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

Simon Marchi
2020-03-26T14:56:00Z

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