Re: [Qemu-devel] [PATCH] configure: save git working tree information in "pkgversion"

[Laszlo Ersek]

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" <address@hidden>
>>>> Cc: Kashyap Chamarthy <address@hidden>
>>>> Signed-off-by: Laszlo Ersek <address@hidden>
>>>> ---
>>>>  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