From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rock.gnat.com (rock.gnat.com [205.232.38.15]) by sourceware.org (Postfix) with ESMTP id B92F1385B835 for ; Fri, 10 Apr 2020 00:02:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org B92F1385B835 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=brobecker@adacore.com Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 999B911672C; Thu, 9 Apr 2020 20:02:00 -0400 (EDT) X-Virus-Scanned: Debian amavisd-new at gnat.com Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id KM12AGkHE3yA; Thu, 9 Apr 2020 20:02:00 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 5823A116708; Thu, 9 Apr 2020 20:02:00 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 88F7583399; Thu, 9 Apr 2020 17:01:58 -0700 (PDT) Date: Thu, 9 Apr 2020 17:01:58 -0700 From: Joel Brobecker To: Andrew Burgess Cc: gdb-patches@sourceware.org, Simon Marchi Subject: Re: [PATCH 2/2] Add git sha information to the gdb version string Message-ID: <20200410000158.GJ9860@adacore.com> References: <47128465-bbae-6241-6b04-929163f15229@simark.ca> <16599d81be47851f9ce53d7da037d9a1cdeba490.1585311874.git.andrew.burgess@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <16599d81be47851f9ce53d7da037d9a1cdeba490.1585311874.git.andrew.burgess@embecosm.com> User-Agent: Mutt/1.9.4 (2018-02-28) X-Spam-Status: No, score=-19.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org 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: Fri, 10 Apr 2020 00:02:02 -0000 Hi Andrew, Thanks for doing this. I finally had a chance to look at the changes you are proposing. First, a point of clarification regarding the discussions that occurred during last year's GNU Cauldron, since it's actually not the same as what is proposed here: What was discussed was the reasons people needed the version date to be updated and checked in every day. The idea was to explore alternate options to achieve the same goal without having to make a commit every single day. Unfortunately, I don't have a solution for that, right now. The two ideas aren't mutually exclusive. We may have to adjust the format of the version ID at some point, but since I don't have a good solution to the daily checkins problem, that's really not a consideration at the moment. I'd like to propose we add a bit more flexibility to this, so vendors can plug their own way of calculating the ID. Without budening you with the details, it wouldn't work well in our production environment, and the only thing that we are really after is the SHA1 of the commit used during the source packaging and the counter. What do you guys think of adding a configure option to pass a "git-tag", for instance, which, when passed, would use that instead of trying to calculate it on the fly? One additional question: As far as I can tell, this create-version.sh creates the version.c files during the build. If so, users building from source tarballs uploaded to ftp each day (using the src-release.sh script) would be building versions of GDB without any git information in them... What do we think we should have for... - official releases; - daily snapshots? Seems like we would want to be consistent regardless of whether it's a snapshot or a release? On Fri, Mar 27, 2020 at 12:27:12PM +0000, Andrew Burgess wrote: > Adds information about the precise git version from which gdb was > built. The new version string will look like this: > > GNU gdb (GDB) 10.0.50.20200307-git (137acc6fbd6-72fbdf834da-2-dirty) > > The first half (up to the '-git' string) is as before, and taken from > the version.in file within the gdb source tree, with the date text > taken from the bfd source tree. > > The second part is new, and is built from the current git version. > This is broken down like this: > > 137acc6fbd6-72fbdf834da-2-dirty > | | | | | | | > '---A-----' '-----B---' C '-D-' > > Where: > A - This is the last git commit sha. This represents the HEAD of the > branch that was built. > B - This is the merge base between the current branch and the > upstream master branch. > C - The number of commits from the merge base to the current sha. > This gives an impression of how diverged the branch is. > D - Is the current git tree fully committed? If not then it is dirty. > > Parts B, C, and D are optional, though B and C will almost always > appear together (see below for details). The following are all valid: > > 137acc6fbd6-72fbdf834da-2-dirty > > Current HEAD is 137acc6fbd6, which is 2 commits from the merge-base > 72fbdf834da (which is in sourceware's master branch), however, the > repository that built this GDB was not fully committed, so 137acc6fbd6 > will not accurately represent the source code that built this GDB. > > 137acc6fbd6-72fbdf834da-2 > > Like the above, but the repository was fully committed, so 137acc6fbd6 > does exactly match the source code that built this version of GDB. > > 72fbdf834da-dirty > > This version of GDB was built directly from sourceware's master > branch (commit 72fbdf834da), however the repository had local changes > at the time of build, so 72fbdf834da does not exactly match the source > code that built this version of GDB. > > 72fbdf834da > > Like the above, but the repository was clean at the time of build, so > 72fbdf834da exactly matches that source code that built this version > of GDB. > > 137acc6fbd6-unknown-dirty > 137acc6fbd6-unknown > > These occur when the version script can't find the remote sourceware > repository, and so is unable to figure out a suitable merge-base, nor > can the script figure out if the current HEAD is in sourceware's > master branch. In this case parts B and C are replaced with the > string 'unknown'. > > I have tried to ensure that if the source code is not a git repository > then the git version token will not be added, so folks building from > tar files should get exactly what they had before. > > 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. > > gdbsupport/ChangeLog: > > * create-version.sh: Add git sha information. > * remote-repository: New file. > > gdbserver/ChangeLog: > > * Makefile.in (version-generated.cc): Add remote-repository file > as a dependency. > > gdb/ChangeLog: > > * Makefile.in (stamp-version): Add remote-repository file as a > dependency. > --- > gdb/ChangeLog | 5 +++ > gdb/Makefile.in | 2 +- > gdbserver/ChangeLog | 5 +++ > gdbserver/Makefile.in | 2 +- > gdbsupport/ChangeLog | 5 +++ > gdbsupport/create-version.sh | 90 ++++++++++++++++++++++++++++++++++++++++++++ > gdbsupport/remote-repository | 2 + > 7 files changed, 109 insertions(+), 2 deletions(-) > create mode 100644 gdbsupport/remote-repository > > diff --git a/gdb/Makefile.in b/gdb/Makefile.in > index 0c331af4bff..9b5ad1ea176 100644 > --- a/gdb/Makefile.in > +++ b/gdb/Makefile.in > @@ -2068,7 +2068,7 @@ $(srcdir)/copying.c: @MAINTAINER_MODE_TRUE@ $(srcdir)/../COPYING3 $(srcdir)/copy > version.c: stamp-version; @true > # Note that the obvious names for the temp file are taken by > # create-version.sh. > -stamp-version: Makefile version.in $(srcdir)/../bfd/version.h $(srcdir)/../gdbsupport/create-version.sh > +stamp-version: Makefile version.in $(srcdir)/../bfd/version.h $(srcdir)/../gdbsupport/create-version.sh $(srcdir)/../gdbsupport/remote-repository > $(ECHO_GEN) $(SHELL) $(srcdir)/../gdbsupport/create-version.sh $(srcdir) \ > $(host_alias) $(target_alias) version-t.t > @$(SHELL) $(srcdir)/../move-if-change version-t.t version.c > diff --git a/gdbserver/Makefile.in b/gdbserver/Makefile.in > index 8c35c169d62..0c67d64b02e 100644 > --- a/gdbserver/Makefile.in > +++ b/gdbserver/Makefile.in > @@ -445,7 +445,7 @@ am--refresh: > > force: > > -version-generated.cc: Makefile $(srcdir)/../gdb/version.in $(srcdir)/../bfd/version.h $(srcdir)/../gdbsupport/create-version.sh > +version-generated.cc: Makefile $(srcdir)/../gdb/version.in $(srcdir)/../bfd/version.h $(srcdir)/../gdbsupport/create-version.sh $(srcdir)/../gdbsupport/remote-repository > $(ECHO_GEN) $(SHELL) $(srcdir)/../gdbsupport/create-version.sh $(srcdir)/../gdb \ > $(host_alias) $(target_alias) $@ > > diff --git a/gdbsupport/create-version.sh b/gdbsupport/create-version.sh > index 6135d219d94..68c6518615f 100755 > --- a/gdbsupport/create-version.sh > +++ b/gdbsupport/create-version.sh > @@ -27,9 +27,99 @@ host_alias="$2" > target_alias="$3" > output="$4" > > +GIT_TAG="" > +if (cd "$srcdir" && git rev-parse --show-toplevel >/dev/null 2>/dev/null); then > + > + # The git version string looks something like this: > + # > + # 137acc6fbd6-72fbdf834da-2-dirty > + # | | | | | | | > + # '---A-----' '-----B---' C '-D-' > + # > + # Where: > + # A - This is the last git commit sha. This represents the HEAD of the > + # branch that was built. > + # B - This is the merge base between the current branch and the > + # upstream master branch. > + # C - The number of commits from the merge base to the current sha. > + # This gives an impression of how diverged the branch is. > + # D - Is the current git tree fully committed? If not then it is dirty. > + # > + # First figure out part A, the short sha for the current head: > + short_head_sha=$(cd "$srcdir" && git rev-parse --short HEAD) > + > + # Now figure out part D, the dirty tag. This might be left as the > + # empty string if the repository has no local changes. > + dirty_mark="" > + (cd "$srcdir" && git update-index -q --refresh) > + test -z "$(cd "$srcdir" && git diff-index --name-only HEAD --)" || > + dirty_mark="-dirty" > + > + # To calculate parts B and C we need to calculate the merge-base > + # between the current branch and a specific remote branch. The > + # local file 'remote-repository' contains the remote repository > + # url, and the branch name we are comparing against. Here we > + # extract that information. > + remote_repo_pattern=$(grep ^pattern: \ > + "$srcdir/../gdbsupport/remote-repository" \ > + | cut -d: -f2-) > + remote_repo_branch=$(grep ^branch: \ > + "$srcdir/../gdbsupport/remote-repository" \ > + | cut -d: -f2-) > + > + # Use the remote url pattern from the remote-repository file to > + # find the local name for the remote git repository. > + remote_name=$( (cd "$srcdir" && git remote -v) \ > + | grep "${remote_repo_pattern}" | grep \(fetch\) \ > + | awk -F '\t' '{print $1}') > + branch_info="" > + if [ -n "${remote_name}" ]; then > + # We found the local name for the remote repository, we can > + # now go ahead and figure out parts B and C of the version > + # string. > + remote_branch="${remote_name}/${remote_repo_branch}" > + # Check to see if the remote branch contains the current HEAD. > + # If it does then the user is building directly from (their > + # checkout of) the upstream branch. > + if ! (cd "$srcdir" && git merge-base \ > + --is-ancestor "${short_head_sha}" \ > + "${remote_branch}"); then > + # The current head 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}") > + > + # Count the commits between the merge base and current head. > + commit_count=$(cd "$srcdir" \ > + && git rev-list --count "${merge_base_sha}"..HEAD) > + > + # Now format the parts B and C of the git version string. > + branch_info="-${short_merge_base_sha}-${commit_count}" > + else > + # The user is building from a commit on the upstream > + # branch. There's no need to include parts B and C of the > + # git version string. > + branch_info="" > + fi > + else > + # We couldn't find the local name for the remote repository. > + # Maybe this user has cloned from some other remote, or has > + # deleted their remotes. > + branch_info="-unknown" > + fi > + GIT_TAG="${short_head_sha}${branch_info}${dirty_mark}" > +fi > + > rm -f version.c-tmp "$output" version.tmp > date=$(sed -n -e 's/^.* BFD_VERSION_DATE \(.*\)$/\1/p' "$srcdir/../bfd/version.h") > sed -e "s/DATE/$date/" < "$srcdir/version.in" > version.tmp > +if [ -n "$GIT_TAG" ]; then > + echo " ($GIT_TAG)" >> version.tmp > +fi > { > echo '#include "gdbsupport/version.h"' > echo 'const char version[] = "'"$(sed q version.tmp)"'";' > diff --git a/gdbsupport/remote-repository b/gdbsupport/remote-repository > new file mode 100644 > index 00000000000..c05f3163e81 > --- /dev/null > +++ b/gdbsupport/remote-repository > @@ -0,0 +1,2 @@ > +pattern:sourceware.org/git/binutils-gdb.git > +branch:master > -- > 2.14.5 -- Joel