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

include commit hash in status output #35

Merged
merged 5 commits into from Oct 30, 2018

Conversation

Projects
None yet
3 participants
@ansemjo
Copy link
Contributor

commented Oct 29, 2018

This is an alternative to #34 and fixes #22.

  • all commit hash definition logic is done in a subshell in the makefile
  • both the makefile shell and preprocessor directives in main.c fallback to unknown if the git command fails or VERSION is somehow undefined during compilation
  • the subshell uses git describe output when building from a checked-out repository or an inserted commit hash when building from a git-archive (i.e. a downloaded tarball)
  • lines 71-74 should be reusable for other projects and languages
add commit hash information to builds
Signed-off-by: Anton Semjonov <anton@semjonov.de>
@giuseppe

This comment has been minimized.

Copy link
Collaborator

commented on main.c in 740b346 Oct 29, 2018

would it make sense to use PACKAGE_VERSION when VERSION is not present?

This comment has been minimized.

Copy link
Contributor Author

replied Oct 29, 2018

Where is that defined? I can see it's set to 0.1 after running autogen.sh but there is not git tag 0.1 in the repository? So I'd say no, unless you synchronize those two somehow.

This comment has been minimized.

Copy link
Contributor Author

replied Oct 29, 2018

The only thing you might want a fallback for is when it is a checked-out repository but the git describe ... in the makefile fails for some reason. You would be left with Version: $Format:%h$ in that case, which is unhelpful and ugly.

ansemjo added some commits Oct 29, 2018

do commit hash replacement in Makefile.am and improve fallback
Signed-off-by: Anton Semjonov <anton@semjonov.de>
use COMMIT instead of VERSION
this avoids confusing distclean etc. by not interfering with
automake's VERSION or PACKAGE_VERSION

Signed-off-by: Anton Semjonov <anton@semjonov.de>
@AkihiroSuda
Copy link
Member

left a comment

LGTM thanks, but can we move the version info to slirp4netns --version with runc-style machine-parseable format? (I can open PR for --version) https://github.com/opencontainers/runc/blob/f3ce8221ea760f51b5403a9892a69ace43845c2c/main.go#L55-L63

Makefile.am Outdated
# replaced during git-archive creation
COMMIT := $(shell V=$Format:%h$ ; \
expr match "$$V" ormat: >/dev/null \
&& ([ -d .git ] && git describe --always --abbrev || echo unknown) \

This comment has been minimized.

Copy link
@giuseppe

giuseppe Oct 30, 2018

Collaborator

we should be using $abs_srcdir not the build dir. Could you add a cd $abs_srcdir before any other command?

This comment has been minimized.

Copy link
@ansemjo

ansemjo Oct 30, 2018

Author Contributor

do you mean $abs_top_srcdir?

Suggested change
&& ([ -d .git ] && git describe --always --abbrev || echo unknown) \
&& (cd "$$abs_top_srcdir" && [ -d .git ] && git describe --always --abbrev || echo unknown) \
@giuseppe

This comment has been minimized.

Copy link
Collaborator

commented Oct 30, 2018

PACKAGE_VERSION is defined by the configure script. It is then written into the config.h file

@ansemjo

This comment has been minimized.

Copy link
Contributor Author

commented Oct 30, 2018

@AkihiroSuda something like this?

slirp4netns version $PACKAGE_VERSION
commit: $COMMIT_HASH
@AkihiroSuda

This comment has been minimized.

Copy link
Member

commented Oct 30, 2018

Yes, also it would be great if we can have -dirty suffix for dirty build https://github.com/opencontainers/runc/blob/f3ce8221ea760f51b5403a9892a69ace43845c2c/Makefile#L17

move version output to seperate option flag, runc-style
Signed-off-by: Anton Semjonov <anton@semjonov.de>
@ansemjo

This comment has been minimized.

Copy link
Contributor Author

commented Oct 30, 2018

current output:

$ ./slirp4netns -v
./slirp4netns version 0.1
commit: bf62e3b-dirty
Show resolved Hide resolved Makefile.am Outdated
use full commit hashes
Signed-off-by: Anton Semjonov <anton@semjonov.de>
@AkihiroSuda
Copy link
Member

left a comment

Thanks!

LGTM (if CI green)

@giuseppe giuseppe merged commit 777bdcc into rootless-containers:master Oct 30, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@giuseppe

This comment has been minimized.

Copy link
Collaborator

commented Oct 30, 2018

LGTM as well, merged. Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.