From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id C179C385E014 for ; Thu, 26 Mar 2020 14:56:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C179C385E014 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark@simark.ca Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 2F03F1E60C; Thu, 26 Mar 2020 10:55:59 -0400 (EDT) Subject: Re: [RFC] Add git sha information to the gdb version string To: Andrew Burgess , gdb-patches@sourceware.org, Joel Brobecker References: <20200325114811.6234-1-andrew.burgess@embecosm.com> From: Simon Marchi Message-ID: <47128465-bbae-6241-6b04-929163f15229@simark.ca> Date: Thu, 26 Mar 2020 10:55:59 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <20200325114811.6234-1-andrew.burgess@embecosm.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US-large Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 26 Mar 2020 14:56:00 -0000 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