Re: [RFC] Add git sha information to the gdb version string

[Simon Marchi <simark@simark.ca>]

Hi Andrew,

On 2020-03-25 7:48 a.m., Andrew Burgess wrote:
> I don't know if there is any interest in adding git sha information to
> the GDB version string.  I seem to recall this was discussed briefly
> at 2019's GNU cauldron, but I don't recall if there was interest in
> the idea or not.
> 
> Personally I think having the git information in the binary can be
> helpful, so below is an initial proposal.  I'm happy to change this as
> needed if people have feedback on how to improve it.
> 
> Let me know what you think,
> 
> Thanks,
> Andrew

I am in favor of this, and I have no comments on the user-visible result, I like the
way it looks.

When we discussed this, we had the "build date" and the "git sha1" camps.  I think
they both have their use, so keeping both in the version string is good.

For the shell script, I'd suggest to first make a patch (probably obvious) to make it
shellcheck-clean [1], and then ensure that your patch keeps it clean.  shellcheck finds
so many little gotchas of shell scripting.

[1] https://github.com/koalaman/shellcheck

> The file gdbsupport/remote-repository contains the pattern used to
> identify the remote repository, and the name of the upstream branch
> from that repository which we care about.  For us this will be
> sourceware and master, but by moving these strings into a separate
> file anyone maintaining an out of tree GDB can easily update these to
> point to their repository and branch, and get project specific version
> strings.

What is the advantage of doing this, versus just doing

  git-merge-base master HEAD

... which would just use the local master branch?

I presume that this file will need to be updated to change the branch name
in the stable branches (CCing Joel for that).

> diff --git a/gdbsupport/create-version.sh b/gdbsupport/create-version.sh
> index 81d6dbf8c1f..2f8e5a3e283 100755
> --- a/gdbsupport/create-version.sh
> +++ b/gdbsupport/create-version.sh
> @@ -27,9 +27,54 @@ host_alias="$2"
>  target_alias="$3"
>  output="$4"
>  
> +GIT_TAG=""
> +if $(cd $srcdir && git rev-parse --show-toplevel >/dev/null 2>/dev/null); then
> +    short_head_sha=$(cd $srcdir && git rev-parse --short HEAD)
> +
> +    dirty_mark=""
> +    (cd $srcdir && git update-index -q --refresh)
>
> +    test -z "$(cd $srcdir && git diff-index --name-only HEAD --)" ||
> +	dirty_mark="-dirty"
> +
> +    remote_repo_pattern=`grep ^pattern: \
> +				$srcdir/../gdbsupport/remote-repository \
> +				| cut -d: -f2-`
> +    remote_repo_branch=`grep ^branch: \
> +				$srcdir/../gdbsupport/remote-repository \
> +				| cut -d: -f2-`
> +    branch_info=""
> +    remote_name=`(cd $srcdir && git remote -v) \
> +	| grep ${remote_repo_pattern} | grep \(fetch\) \
> +	| awk -F '\t' '{print $1}'`
> +    if [ -n "${remote_name}" ]; then
> +	remote_branch="${remote_name}/${remote_repo_branch}"
> +	# If the remote branch contains our commit, then we're good.
> +	if ! $(cd $srcdir  && git merge-base \
> +				  --is-ancestor ${short_head_sha} \
> +				  ${remote_branch}); then
> +	    # SHA is not on the remote tracking branch.  We need to figure out
> +	    # the merge base, and the distance from that merge base.
> +	    merge_base_sha=$(cd $srcdir && git merge-base ${short_head_sha} \
> +					       ${remote_branch})
> +	    short_merge_base_sha=$(cd $srcdir \
> +				       && git rev-parse \
> +					      --short ${merge_base_sha})
> +	    commit_count=$(cd $srcdir \
> +			       && git rev-list --count ${merge_base_sha}..HEAD)
> +	    branch_info="-${short_merge_base_sha}-${commit_count}"
> +	fi
> +    else
> +	branch_info="-unknown"
> +    fi
> +    GIT_TAG="${short_head_sha}${branch_info}${dirty_mark}"
> +fi

Could you please comment the code a bit more, to say what each line does?  It will
make it easier to read in the future, if we want to fix/modify it.

Simon