| Message ID | 1464707044-10447-1-git-send-email-lersek@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 05/31/16 17:14, Daniel P. Berrange wrote: > On Tue, May 31, 2016 at 05:04:04PM +0200, Laszlo Ersek wrote: >> When building QEMU from a git working tree (either in-tree or >> out-of-tree), it is useful to capture the working tree status in the QEMU >> binary, for the "-version" option to report. >> >> Daniel suggested using the "pkgversion" variable (tied to the >> "--with-pkgversion" option) of the configure script for this. Downstream >> packagers of QEMU already use this option for customizing their builds, >> plus libvirtd captures "pkgversion" (with the "-version" option) in >> "/var/log/libvirt/qemu/$GUEST.log", whenever a guest is started. >> >> The information we include in "pkgversion" is the output of git-describe, >> with a plus sign (+) appended if there are staged or unstaged changes to >> tracked files, at the time of "configure" being executed. >> >> The content of "pkgversion" is not changed when "--with-pkgversion" is >> used on the command line. >> >> Cc: "Daniel P. Berrange" <berrange@redhat.com> >> Cc: Kashyap Chamarthy <kchamart@redhat.com> >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >> --- >> configure | 38 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 38 insertions(+) >> >> diff --git a/configure b/configure >> index b5aab7257b33..20a7ec5cc0fd 100755 >> --- a/configure >> +++ b/configure >> @@ -4255,6 +4255,44 @@ if have_backend "dtrace"; then >> fi >> >> ########################################## >> +# save git working tree information in pkgversion >> + >> +# If pkgversion has not been set to a non-empty string, fetch the output of >> +# "git describe" into it. If the working tree is unclean (there are staged or >> +# unstaged changes in tracked files), then append a plus sign. >> +# >> +# If we're not building from a git working tree, then pkgversion is not >> +# changed. Otherwise, git errors are fatal. >> + >> +if test -z "$pkgversion" && test -d "$source_path/.git"; then >> + pkgversion=$( >> + export GIT_DIR=$source_path/.git >> + export GIT_WORK_TREE=$source_path >> + >> + git_desc=$(git describe) >> + git_exit=$? >> + if test $git_exit -ne 0; then >> + exit $git_exit >> + fi >> + >> + git_changes= >> + for git_diff_option in "" --staged; do >> + git diff $git_diff_option --quiet >> + git_exit=$? >> + case $git_exit in >> + (0) ;; >> + (1) git_changes=+ >> + ;; >> + (*) exit $git_exit >> + ;; >> + esac >> + done > > An alternative to this would be to jus use > > "git describe --dirty" > > which appends "--dirty" to its output if working tre has uncommitted > changes. Good idea! > Not sure if the --dirty flag is a recent option or whether we can just > assume it always exists. Grepping git's Documentation/RelNotes/ directory, I find: - in "1.6.6.txt": the introduction of --dirty - in "1.7.6.4.txt": an apparently important bugfix for --dirty Version 1.7.6.4 of git was tagged on Sep 23 2011. Does this information help in deciding if we can use --dirty? For example, Debian oldstable ("wheezy"), which I tend to use as one reference point for "ancient but still supported", has 1.7.10.4: https://packages.debian.org/wheezy/git Another example: the latest git package in EPEL-5 is 1.8.2.3: http://koji.fedoraproject.org/koji/buildinfo?buildID=757915 Thanks Laszlo
On 05/31/16 17:43, Daniel P. Berrange wrote: > On Tue, May 31, 2016 at 05:40:38PM +0200, Laszlo Ersek wrote: >> On 05/31/16 17:14, Daniel P. Berrange wrote: >>> On Tue, May 31, 2016 at 05:04:04PM +0200, Laszlo Ersek wrote: >>>> When building QEMU from a git working tree (either in-tree or >>>> out-of-tree), it is useful to capture the working tree status in the QEMU >>>> binary, for the "-version" option to report. >>>> >>>> Daniel suggested using the "pkgversion" variable (tied to the >>>> "--with-pkgversion" option) of the configure script for this. Downstream >>>> packagers of QEMU already use this option for customizing their builds, >>>> plus libvirtd captures "pkgversion" (with the "-version" option) in >>>> "/var/log/libvirt/qemu/$GUEST.log", whenever a guest is started. >>>> >>>> The information we include in "pkgversion" is the output of git-describe, >>>> with a plus sign (+) appended if there are staged or unstaged changes to >>>> tracked files, at the time of "configure" being executed. >>>> >>>> The content of "pkgversion" is not changed when "--with-pkgversion" is >>>> used on the command line. >>>> >>>> Cc: "Daniel P. Berrange" <berrange@redhat.com> >>>> Cc: Kashyap Chamarthy <kchamart@redhat.com> >>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >>>> --- >>>> configure | 38 ++++++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 38 insertions(+) >>>> >>>> diff --git a/configure b/configure >>>> index b5aab7257b33..20a7ec5cc0fd 100755 >>>> --- a/configure >>>> +++ b/configure >>>> @@ -4255,6 +4255,44 @@ if have_backend "dtrace"; then >>>> fi >>>> >>>> ########################################## >>>> +# save git working tree information in pkgversion >>>> + >>>> +# If pkgversion has not been set to a non-empty string, fetch the output of >>>> +# "git describe" into it. If the working tree is unclean (there are staged or >>>> +# unstaged changes in tracked files), then append a plus sign. >>>> +# >>>> +# If we're not building from a git working tree, then pkgversion is not >>>> +# changed. Otherwise, git errors are fatal. >>>> + >>>> +if test -z "$pkgversion" && test -d "$source_path/.git"; then >>>> + pkgversion=$( >>>> + export GIT_DIR=$source_path/.git >>>> + export GIT_WORK_TREE=$source_path >>>> + >>>> + git_desc=$(git describe) >>>> + git_exit=$? >>>> + if test $git_exit -ne 0; then >>>> + exit $git_exit >>>> + fi >>>> + >>>> + git_changes= >>>> + for git_diff_option in "" --staged; do >>>> + git diff $git_diff_option --quiet >>>> + git_exit=$? >>>> + case $git_exit in >>>> + (0) ;; >>>> + (1) git_changes=+ >>>> + ;; >>>> + (*) exit $git_exit >>>> + ;; >>>> + esac >>>> + done >>> >>> An alternative to this would be to jus use >>> >>> "git describe --dirty" >>> >>> which appends "--dirty" to its output if working tre has uncommitted >>> changes. >> >> Good idea! >> >>> Not sure if the --dirty flag is a recent option or whether we can just >>> assume it always exists. >> >> Grepping git's Documentation/RelNotes/ directory, I find: >> - in "1.6.6.txt": the introduction of --dirty >> - in "1.7.6.4.txt": an apparently important bugfix for --dirty >> >> Version 1.7.6.4 of git was tagged on Sep 23 2011. >> >> Does this information help in deciding if we can use --dirty? > > 5 years old sounds new enough for my liking :-) > > I guess we could use --dirty and catch the non-zero exit code and just > re-try without --dirty. But, if we can't use --dirty, I should probably use the plus-sign fallback (we need *something* to mark a dirty state). In which case however, shouldn't we just go with the current patch, which doesn't care about --dirty at all? Otherwise, some build hosts will append "-dirty", and others will append "+". IMO we should either require --dirty, or go with the current patch. Thanks Laszlo
On 05/31/16 19:45, Eric Blake wrote: > On 05/31/2016 11:01 AM, Laszlo Ersek wrote: > >>>> Grepping git's Documentation/RelNotes/ directory, I find: >>>> - in "1.6.6.txt": the introduction of --dirty >>>> - in "1.7.6.4.txt": an apparently important bugfix for --dirty (*) >>>> >>>> Version 1.7.6.4 of git was tagged on Sep 23 2011. >>>> >>>> Does this information help in deciding if we can use --dirty? >>> >>> 5 years old sounds new enough for my liking :-) >>> >>> I guess we could use --dirty and catch the non-zero exit code and just >>> re-try without --dirty. >> >> But, if we can't use --dirty, I should probably use the plus-sign >> fallback (we need *something* to mark a dirty state). >> >> In which case however, shouldn't we just go with the current patch, >> which doesn't care about --dirty at all? Otherwise, some build hosts >> will append "-dirty", and others will append "+". >> >> IMO we should either require --dirty, or go with the current patch. > > Gnulib's build-aux/git-version-gen script doesn't yet use --dirty, but > may be an inspiration for how to generate the same suffix: > > # Test whether to append the "-dirty" suffix only if the version > # string we're using came from git. I.e., skip the test if it's "UNKNOWN" > # or if it came from .tarball-version. > if test "x$v_from_git" != x; then > # Don't declare a version "dirty" merely because a time stamp has changed. > git update-index --refresh > /dev/null 2>&1 ( This is exactly the fix (*) that went into git v1.7.6.4 (and v1.7.7): $ git log --oneline --reverse v1.7.6.3..v1.7.6.4 0f64bfa9567f ls-files: fix pathspec display on error e9d4f7405b6a branch.c: use the parsed branch name 13d6ec913330 read_gitfile_gently(): rename misnamed function to read_gitfile() 9b0ebc722cfc clone: allow to clone from .git file dbc92b072dd7 clone: allow more than one --reference e6baf4a1ae1b clone: clone from a repository with relative alternates 2f633f41d695 check-ref-format --print: Normalize refnames that start with slashes f3738c1ce919 Forbid DEL characters in reference names 385ceec1cb46 t3005: do not assume a particular order of stdout and stderr of git-ls-files dff4b0ef30cd am: format is in $patch_format, not parse_patch e622f41dcd97 git-mergetool: check return value from read 40ffc4987661 Merge branch 'gb/maint-am-patch-format-error-message' into maint 503359f13abc Merge branch 'mg/branch-set-upstream-previous' into maint be5acb3b63af Merge branch 'mh/check-ref-format-print-normalize' into maint 406c1c4dd4a8 Merge branch 'nd/maint-clone-gitdir' into maint 84b051462fca Merge branch 'jc/maint-clone-alternates' into maint 85b3c75f4fd3 describe: Refresh the index when run with --dirty a0b1cb60ab29 Merge branch 'cb/maint-ls-files-error-report' into maint 632052641517 Git 1.7.6.4 --> https://github.com/git/git/commit/85b3c75f4fd3 "describe: Refresh the index when run with --dirty" ) > > dirty=`exec 2>/dev/null;git diff-index --name-only HEAD` || dirty= > case "$dirty" in > '') ;; > *) # Append the suffix only if there isn't one already. > case $v in > *-dirty) ;; > *) v="$v-dirty" ;; > esac ;; > esac > fi This seems to do the right thing, yes. I'll submit a new version later. Thanks! Laszlo
On 05/31/16 19:45, Eric Blake wrote: > On 05/31/2016 11:01 AM, Laszlo Ersek wrote: > >>>> Grepping git's Documentation/RelNotes/ directory, I find: >>>> - in "1.6.6.txt": the introduction of --dirty >>>> - in "1.7.6.4.txt": an apparently important bugfix for --dirty >>>> >>>> Version 1.7.6.4 of git was tagged on Sep 23 2011. >>>> >>>> Does this information help in deciding if we can use --dirty? >>> >>> 5 years old sounds new enough for my liking :-) >>> >>> I guess we could use --dirty and catch the non-zero exit code and just >>> re-try without --dirty. >> >> But, if we can't use --dirty, I should probably use the plus-sign >> fallback (we need *something* to mark a dirty state). >> >> In which case however, shouldn't we just go with the current patch, >> which doesn't care about --dirty at all? Otherwise, some build hosts >> will append "-dirty", and others will append "+". >> >> IMO we should either require --dirty, or go with the current patch. > > Gnulib's build-aux/git-version-gen script doesn't yet use --dirty, but > may be an inspiration for how to generate the same suffix: > > # Test whether to append the "-dirty" suffix only if the version > # string we're using came from git. I.e., skip the test if it's "UNKNOWN" > # or if it came from .tarball-version. > if test "x$v_from_git" != x; then > # Don't declare a version "dirty" merely because a time stamp has changed. > git update-index --refresh > /dev/null 2>&1 > > dirty=`exec 2>/dev/null;git diff-index --name-only HEAD` || dirty= > case "$dirty" in > '') ;; > *) # Append the suffix only if there isn't one already. > case $v in > *-dirty) ;; > *) v="$v-dirty" ;; > esac ;; > esac > fi > BTW, my patch has a functionality bug. Consider the case when you change some of the tracked files, then stage all those changes with "git add", then *undo* the changes in the working tree only. In this case, my patch will report "dirty" ("+"), because there will be both staged changes (relative to the HEAD commit) and working tree changes (relative to the index). But that's incorrect -- the working tree actually matches the HEAD commit, so the build qualifies as "clean". On the other hand, git-diff-index will do the right thing, namely: git-diff-index <tree-ish> compares the <tree-ish> and the files on the filesystem. which is exactly right. The index (= the staged changes) are irrelevant for a build; only the working tree matters. (Anyway, this is moot now; I'll happily leave it to Fam! :)) Thanks Laszlo
diff --git a/configure b/configure index b5aab7257b33..20a7ec5cc0fd 100755 --- a/configure +++ b/configure @@ -4255,6 +4255,44 @@ if have_backend "dtrace"; then fi ########################################## +# save git working tree information in pkgversion + +# If pkgversion has not been set to a non-empty string, fetch the output of +# "git describe" into it. If the working tree is unclean (there are staged or +# unstaged changes in tracked files), then append a plus sign. +# +# If we're not building from a git working tree, then pkgversion is not +# changed. Otherwise, git errors are fatal. + +if test -z "$pkgversion" && test -d "$source_path/.git"; then + pkgversion=$( + export GIT_DIR=$source_path/.git + export GIT_WORK_TREE=$source_path + + git_desc=$(git describe) + git_exit=$? + if test $git_exit -ne 0; then + exit $git_exit + fi + + git_changes= + for git_diff_option in "" --staged; do + git diff $git_diff_option --quiet + git_exit=$? + case $git_exit in + (0) ;; + (1) git_changes=+ + ;; + (*) exit $git_exit + ;; + esac + done + + printf " (%s%s)" "$git_desc" "$git_changes" + ) || error_exit "Failed to get git description, working tree or index status" +fi + +########################################## # check and set a backend for coroutine # We prefer ucontext, but it's not always possible. The fallback
When building QEMU from a git working tree (either in-tree or out-of-tree), it is useful to capture the working tree status in the QEMU binary, for the "-version" option to report. Daniel suggested using the "pkgversion" variable (tied to the "--with-pkgversion" option) of the configure script for this. Downstream packagers of QEMU already use this option for customizing their builds, plus libvirtd captures "pkgversion" (with the "-version" option) in "/var/log/libvirt/qemu/$GUEST.log", whenever a guest is started. The information we include in "pkgversion" is the output of git-describe, with a plus sign (+) appended if there are staged or unstaged changes to tracked files, at the time of "configure" being executed. The content of "pkgversion" is not changed when "--with-pkgversion" is used on the command line. Cc: "Daniel P. Berrange" <berrange@redhat.com> Cc: Kashyap Chamarthy <kchamart@redhat.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- configure | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) -- 1.8.3.1