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

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Simon Marchi <simark@simark.ca> [2020-03-26 10:55:59 -0400]:

> 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

Good idea, I'll take a look at doing this.

> 
> > 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'd very much like to not make assumptions about what people have
chosen to name branches in their local repository.

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

Yes, if we wanted the merge-base part to show anything better than the
SHA where the release branch forked from the master branch then this
file would need to be changed when the release branch was created.

> 
> > 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.

Will do.

Thanks,
Andrew