Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --version argument to commands #298

Merged
merged 3 commits into from
Dec 21, 2023

Conversation

cfergeau
Copy link
Contributor

The binaries shipped by gvisor-tap-vsock don't have a --version argument.
This is problematic to check which version is being used (for example, podman ships different gvproxy versions if it comes from brew or an installer, if it's running on macOS or Windows).
This PR adds --version to commands which accept arguments.
It's a bit involved as I wanted --version to work:

  • when building from git
  • when building from a github tarball
  • when installing through go install

This unfortunately requires 3 different codepaths, but it works in these 3 cases.

@cfergeau cfergeau force-pushed the versioning branch 2 times, most recently from ac3cc6c to 31be767 Compare November 23, 2023 14:30
@anjannath
Copy link
Member

LGTM, in the first commit log there's a typo,

Correct versioning when installing from tarballs, and through git install will be addressed in the next commits.

s/git/go

# reset LDFLAGS for plugins and gvisor binaries, but ensure gvproxy --version and such is set
LDFLAGS="-X github.com/containers/gvisor-tap-vsock/pkg/types.PackageVersion=%{version}"
# reset LDFLAGS for plugins and gvisor binaries
LDFLAGS=''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We removed this because we are sure that the tarball source for rpm would have gitArchiveVersion set so as part of spec we don't have to provide anything?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another thing which is not clear when removing this is assume that I want to create rpm package for v1.5 so I tag it and then it git describe should show v1.5 ?

Copy link
Contributor Author

@cfergeau cfergeau Dec 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We removed this because we are sure that the tarball source for rpm would have gitArchiveVersion set so as part of spec we don't have to provide anything?

Yes, that's correct, the tarball is assumed to have gitArchiveVersion set. If this is not always true, we can revert back to setting -X github.com/...

Another thing which is not clear when removing this is assume that I want to create rpm package for v1.5 so I tag it and then it git describe should show v1.5 ?

$ git tag -s v0.8.0
$ git describe HEAD
v0.8.0

Then if I push this to github, and generate a release, version.go in the tarball will contain gitArchiveVersion="v0.8.0" instead of gitArchiveVersion = "$Format:%(describe)$"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like when I tried it for crc-org/crc I am not getting expected result. I checked out v2.30.0 tag and describe shows me something unexpected.

 $ git checkout v2.30.0 
Note: switching to 'v2.30.0'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at b6532a cut v2.30.0

$ git describe HEAD
v1.23.0-1666-gb6532a

By going through man git-describe looks like git describe only shows annotated tags. For more information about creating annotated tags see the -a and -s options to git-tag(1) which means we have to create annotated tags otherwise it will fail. Might be mention somewhere so that if someone else from the team creating the tag know that it should be annotated one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be mention somewhere so that if someone else from the team creating the tag know that it should be annotated one.

Even better to create GPG signed tags

gitVersion = ""
// set through .gitattributes when `git archive` is used
// see https://icinga.com/blog/2022/05/25/embedding-git-commit-information-in-go-binaries/
gitArchiveVersion = "$Format:%(describe)$"
Copy link
Contributor

@praveenkumar praveenkumar Dec 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This convert to git describe or git log --pretty=format"$Format:%(describe)$" one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation for git log --pretty=format"$Format:%(describe)"$ says

%(describe[:<options>])
human-readable name, like git-describe(1);

so they are both the same?

@cfergeau
Copy link
Contributor Author

LGTM, in the first commit log there's a typo,

Correct versioning when installing from tarballs, and through git install will be addressed in the next commits.

s/git/go

Thanks, fixed.

This sets the version to `git describe --always --dirty` using the
`-X github.com/containers/gvisor-tap-vsock/pkg/types.ModuleVersion`
linker flag.

This adds a -version flag to all binaries which currently have
command-line flags.

Correct versioning when installing from tarballs, and through
`go install` will be addressed in the next commits.

This fixes containers#295

Signed-off-by: Christophe Fergeau <[email protected]>
This adds correct version information for `gvproxy -version` when
it's installed using:
```
go install github.com/containers/gvisor-tap-vsock/cmd/gvproxy
```

Signed-off-by: Christophe Fergeau <[email protected]>
When building from github tarballs, getting the version from `git
describe` won't work, or worse, will be irrelevant [1]
However, we can make use of $Format$ and .gitattributes as described in
[2] to automatically substitute the correct version in a go variable
when `git archive` is used.
In particular, the correct version number will automatically be
substituted when GitHub creates release tarballs.

[1] rpms/gvisor-tap-vsock.spec uses `%autosetup -Sgit -n %{name}-%{version}` which unpacks
the release tarball in a newly created git repository.

[2] https://icinga.com/blog/2022/05/25/embedding-git-commit-information-in-go-binaries/

Signed-off-by: Christophe Fergeau <[email protected]>
@praveenkumar
Copy link
Contributor

/lgtm
/approve

Copy link
Contributor

openshift-ci bot commented Dec 21, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfergeau, praveenkumar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit f01fd1c into containers:main Dec 21, 2023
18 checks passed
@cfergeau cfergeau deleted the versioning branch December 21, 2023 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants