threads / patch / 47412

patch, 2 partsOffer more information in `git version --build-options`'s output

Subject: [PATCH 0/2] Offer more information in `git version --build-options`'s output

## tl;dr

19 messages between Dec 8, 2017 and Dec 15, 2017. Diffs are folded; open one to read it.

replies: 18people: 4as markdown or json

Johannes Schindelin· Dec 8, 2017, 17:01 UTC · lore

In Git for Windows, we ask users to paste the output of said command into their bug reports, with the idea that this frequently helps identify where the problems are coming from.

There are some obvious missing bits of information in said output, though, and this patch series tries to fill the gaps at least a little.

Adric Norris (1):
  git version --build-options: report the build platform, too
Johannes Schindelin (1):
  version --build-options: report commit, too, if possible
 Makefile  |  4 +++-
 help.c    |  4 ++++
 help.h    | 13 +++++++++++++
 version.c |  1 +
 version.h |  1 +
 5 files changed, 22 insertions(+), 1 deletion(-)
base-commit: 95ec6b1b3393eb6e26da40c565520a8db9796e9f
Published-As: https://github.com/dscho/git/releases/tag/built-from-commit-v1
Fetch-It-Via: git fetch https://github.com/dscho/git built-from-commit-v1
-- 
2.15.1.windows.2
Johannes Schindelin· Dec 8, 2017, 17:02 UTC · re: Johannes Schindelin · lore

[PATCH 1/2] git version --build-options: report the build platform, too

From: Adric Norris <landstander668@gmail.com>

When asking for bug reports to include the output of `git version --build-options`, the idea is that we get a better idea of the environment where said bug occurs. In this context, it is useful to distinguish between 32 and 64-bit builds.

We start by distinguishing between x86 and x86_64 (hoping that interested parties will extend this to other architectures in the future). In addition, all 32-bit variants (i686, i586, etc.) are collapsed into `x86'. An example of the output is:

   $ git version --build-options
   git version 2.9.3.windows.2.826.g06c0f2f
   sizeof-long: 4
   machine: x86_64

The label of `machine' was chosen so the new information will approximate the output of `uname -m'.

Signed-off-by: Adric Norris <landstander668@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 help.c |  2 ++
 help.h | 13 +++++++++++++
 2 files changed, 15 insertions(+)
Show changes to 2 files +15 −0

help.c, help.h

diff --git a/help.c b/help.c
index 88a3aeaeb9f..df1332fa3c9 100644
--- a/help.c
+++ b/help.c
@@ -390,6 +390,7 @@ const char *help_unknown_cmd(const char *cmd)
 
 int cmd_version(int argc, const char **argv, const char *prefix)
 {
+	static char build_platform[] = GIT_BUILD_PLATFORM;
 	int build_options = 0;
 	const char * const usage[] = {
 		N_("git version [<options>]"),
@@ -413,6 +414,7 @@ int cmd_version(int argc, const char **argv, const char *prefix)
 
 	if (build_options) {
 		printf("sizeof-long: %d\n", (int)sizeof(long));
+		printf("machine: %s\n", build_platform);
 		/* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */
 	}
 	return 0;
diff --git a/help.h b/help.h
index b21d7c94e8c..42dd9852194 100644
--- a/help.h
+++ b/help.h
@@ -33,3 +33,16 @@ extern void list_commands(unsigned int colopts, struct cmdnames *main_cmds, stru
  */
 extern void help_unknown_ref(const char *ref, const char *cmd, const char *error);
 #endif /* HELP_H */
+
+/*
+ * identify build platform
+ */
+#ifndef GIT_BUILD_PLATFORM
+	#if defined __x86__ || defined __i386__ || defined __i586__ || defined __i686__
+		#define GIT_BUILD_PLATFORM "x86"
+	#elif defined __x86_64__
+		#define GIT_BUILD_PLATFORM "x86_64"
+	#else
+		#define GIT_BUILD_PLATFORM "unknown"
+	#endif
+#endif
-- 
2.15.1.windows.2
Jonathan Nieder· Dec 8, 2017, 17:23 UTC · re: Johannes Schindelin · lore

Re: [PATCH 1/2] git version --build-options: report the build platform, too

Hi,
Johannes Schindelin wrote:
Show 6 quoted lines
> From: Adric Norris <landstander668@gmail.com>
>
> When asking for bug reports to include the output of `git version
> --build-options`, the idea is that we get a better idea of the
> environment where said bug occurs. In this context, it is useful to
> distinguish between 32 and 64-bit builds.
Neat!
The cover letter gives a clearer idea of the motivation:
	In Git for Windows, we ask users to paste the output of said command
	into their bug reports, with the idea that this frequently helps
	identify where the problems are coming from.
	There are some obvious missing bits of information in said output,
	though, and this patch series tries to fill the gaps at least a little.
Could some of that text go here, too?
[...]
Show 15 quoted lines
> --- a/help.c
> +++ b/help.c
> @@ -390,6 +390,7 @@ const char *help_unknown_cmd(const char *cmd)
>  
>  int cmd_version(int argc, const char **argv, const char *prefix)
>  {
> +	static char build_platform[] = GIT_BUILD_PLATFORM;
>  	int build_options = 0;
>  	const char * const usage[] = {
>  		N_("git version [<options>]"),
> @@ -413,6 +414,7 @@ int cmd_version(int argc, const char **argv, const char *prefix)
>  
>  	if (build_options) {
>  		printf("sizeof-long: %d\n", (int)sizeof(long));
> +		printf("machine: %s\n", build_platform);

Can this use GIT_BUILD_PLATFORM directly instead of going via the indirection of a mutable static string? That is, something like

		printf("machine: %s\n", GIT_BUILD_PLATFORM);
>  		/* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */

What do you think of this idea? uname_M isn't one of the variables in that file, though, so that's orthogonal to this patch.

[...]
Show 19 quoted lines
> --- a/help.h
> +++ b/help.h
> @@ -33,3 +33,16 @@ extern void list_commands(unsigned int colopts, struct cmdnames *main_cmds, stru
>   */
>  extern void help_unknown_ref(const char *ref, const char *cmd, const char *error);
>  #endif /* HELP_H */
> +
> +/*
> + * identify build platform
> + */
> +#ifndef GIT_BUILD_PLATFORM
> +	#if defined __x86__ || defined __i386__ || defined __i586__ || defined __i686__
> +		#define GIT_BUILD_PLATFORM "x86"
> +	#elif defined __x86_64__
> +		#define GIT_BUILD_PLATFORM "x86_64"
> +	#else
> +		#define GIT_BUILD_PLATFORM "unknown"
> +	#endif
> +#endif
This code needs to be inside the HELP_H header guard.

Thanks and hope that helps, Jonathan

Junio C Hamano· Dec 8, 2017, 17:43 UTC · re: Jonathan Nieder · lore

Re: [PATCH 1/2] git version --build-options: report the build platform, too

Jonathan Nieder <jrnieder@gmail.com> writes:
Show 18 quoted lines
>> @@ -390,6 +390,7 @@ const char *help_unknown_cmd(const char *cmd)
>>  
>>  int cmd_version(int argc, const char **argv, const char *prefix)
>>  {
>> +	static char build_platform[] = GIT_BUILD_PLATFORM;
>>  	int build_options = 0;
>>  	const char * const usage[] = {
>>  		N_("git version [<options>]"),
>> @@ -413,6 +414,7 @@ int cmd_version(int argc, const char **argv, const char *prefix)
>>  
>>  	if (build_options) {
>>  		printf("sizeof-long: %d\n", (int)sizeof(long));
>> +		printf("machine: %s\n", build_platform);
>
> Can this use GIT_BUILD_PLATFORM directly instead of going via the indirection
> of a mutable static string?  That is, something like
>
> 		printf("machine: %s\n", GIT_BUILD_PLATFORM);

Good point. And if this is externally identified as "machine", probably the macro should also use the same word, not "platform". We can go either way, as long as we are consistent, though.

Show 21 quoted lines
>> --- a/help.h
>> +++ b/help.h
>> @@ -33,3 +33,16 @@ extern void list_commands(unsigned int colopts, struct cmdnames *main_cmds, stru
>>   */
>>  extern void help_unknown_ref(const char *ref, const char *cmd, const char *error);
>>  #endif /* HELP_H */
>> +
>> +/*
>> + * identify build platform
>> + */
>> +#ifndef GIT_BUILD_PLATFORM
>> +	#if defined __x86__ || defined __i386__ || defined __i586__ || defined __i686__
>> +		#define GIT_BUILD_PLATFORM "x86"
>> +	#elif defined __x86_64__
>> +		#define GIT_BUILD_PLATFORM "x86_64"
>> +	#else
>> +		#define GIT_BUILD_PLATFORM "unknown"
>> +	#endif
>> +#endif
>
> This code needs to be inside the HELP_H header guard.
Certainly.
Thanks.
Eric Sunshine· Dec 8, 2017, 21:17 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/2] git version --build-options: report the build platform, too

On Fri, Dec 8, 2017 at 12:43 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 15 quoted lines
> Jonathan Nieder <jrnieder@gmail.com> writes:
>>> @@ -413,6 +414,7 @@ int cmd_version(int argc, const char **argv, const char *prefix)
>>>
>>>      if (build_options) {
>>>              printf("sizeof-long: %d\n", (int)sizeof(long));
>>> +            printf("machine: %s\n", build_platform);
>>
>> Can this use GIT_BUILD_PLATFORM directly instead of going via the indirection
>> of a mutable static string?  That is, something like
>>
>>               printf("machine: %s\n", GIT_BUILD_PLATFORM);
>
> Good point.  And if this is externally identified as "machine",
> probably the macro should also use the same word, not "platform".
> We can go either way, as long as we are consistent, though.
In Autoconf parlance, this would be called "host architecture" (GIT_HOST_ARCH).
Eric Sunshine· Dec 8, 2017, 21:19 UTC · re: Eric Sunshine · lore

Re: [PATCH 1/2] git version --build-options: report the build platform, too

On Fri, Dec 8, 2017 at 4:17 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 18 quoted lines
> On Fri, Dec 8, 2017 at 12:43 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>>>> @@ -413,6 +414,7 @@ int cmd_version(int argc, const char **argv, const char *prefix)
>>>>
>>>>      if (build_options) {
>>>>              printf("sizeof-long: %d\n", (int)sizeof(long));
>>>> +            printf("machine: %s\n", build_platform);
>>>
>>> Can this use GIT_BUILD_PLATFORM directly instead of going via the indirection
>>> of a mutable static string?  That is, something like
>>>
>>>               printf("machine: %s\n", GIT_BUILD_PLATFORM);
>>
>> Good point.  And if this is externally identified as "machine",
>> probably the macro should also use the same word, not "platform".
>> We can go either way, as long as we are consistent, though.
>
> In Autoconf parlance, this would be called "host architecture" (GIT_HOST_ARCH).
My bad: "host cpu", rather (GIT_HOST_CPU).
Eric Sunshine· Dec 9, 2017, 09:43 UTC · re: Eric Sunshine · lore

Re: [PATCH 1/2] git version --build-options: report the build platform, too

On Fri, Dec 08, 2017 at 04:19:25PM -0500, Eric Sunshine wrote:
Show 15 quoted lines
> On Fri, Dec 8, 2017 at 4:17 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> > On Fri, Dec 8, 2017 at 12:43 PM, Junio C Hamano <gitster@pobox.com> wrote:
> >> Jonathan Nieder <jrnieder@gmail.com> writes:
> >>> Can this use GIT_BUILD_PLATFORM directly instead of going via the indirection
> >>> of a mutable static string?  That is, something like
> >>>
> >>>               printf("machine: %s\n", GIT_BUILD_PLATFORM);
> >>
> >> Good point.  And if this is externally identified as "machine",
> >> probably the macro should also use the same word, not "platform".
> >> We can go either way, as long as we are consistent, though.
> >
> > In Autoconf parlance, this would be called "host architecture" (GIT_HOST_ARCH).
> 
> My bad: "host cpu", rather (GIT_HOST_CPU).

Dscho, when you re-roll, perhaps replace the current patch with the one below which determines the CPU type automatically rather than having to manually maintain and augment a bunch of #ifdefs in help.h.

I took the liberty of renaming the #define to GIT_HOST_CPU to better match Autoconf parlance since its conceivable that Git might some day support cross-compilation officially via the configure script (which doesn't yet work, though I have some patches which begin addressing that, but that's a separate topic).

The original plan was to keep Adric Norris as author, but by the time the patch was done and the commit message rewritten to match, I realized that none of his work remained, thus ended up dropping his authorship and both of your sign-offs. Sorry.

--- >8 ---
From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sat, 9 Dec 2017 04:09:18 -0500
Subject: [PATCH] help: version --build-options: also report host CPU

It can be helpful for bug reports to include information about the environment in which the bug occurs. "git version --build-options" can help to supplement this information. In addition to the size of 'long' already reported by --build-options, also report the host's CPU type. Example output:

   $ git version --build-options
   git version 2.9.3.windows.2.826.g06c0f2f
   cpu: x86_64
   sizeof-long: 4
New Makefile variable HOST_CPU supports cross-compiling.
Suggested-by: Adric Norris <landstander668@gmail.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 Makefile | 9 +++++++++
 help.c   | 1 +
 2 files changed, 10 insertions(+)
Show changes to 2 files +10 −0

Makefile, help.c

diff --git a/Makefile b/Makefile
index fef9c8d272..5587bccc93 100644
--- a/Makefile
+++ b/Makefile
@@ -425,6 +425,9 @@ all::
 #
 # to say "export LESS=FRX (and LV=-c) if the environment variable
 # LESS (and LV) is not set, respectively".
+#
+# When cross-compiling, define HOST_CPU as the canonical name of the CPU on
+# which the built Git will run (for instance "x86_64").
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1095,6 +1098,12 @@ else
 BROKEN_PATH_FIX = '/^\# @@BROKEN_PATH_FIX@@$$/d'
 endif
 
+ifeq (,$(HOST_CPU))
+	BASIC_CFLAGS += -DGIT_HOST_CPU="\"$(firstword $(subst -, ,$(uname_M)))\""
+else
+	BASIC_CFLAGS += -DGIT_HOST_CPU="\"$(HOST_CPU)\""
+endif
+
 ifneq (,$(INLINE))
 	BASIC_CFLAGS += -Dinline=$(INLINE)
 endif
diff --git a/help.c b/help.c
index 88a3aeaeb9..cbcb159f36 100644
--- a/help.c
+++ b/help.c
@@ -412,6 +412,7 @@ int cmd_version(int argc, const char **argv, const char *prefix)
 	printf("git version %s\n", git_version_string);
 
 	if (build_options) {
+		printf("cpu: %s\n", GIT_HOST_CPU);
 		printf("sizeof-long: %d\n", (int)sizeof(long));
 		/* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */
 	}
-- 
2.15.1.502.gccaef8de57
Johannes Schindelin· Dec 9, 2017, 14:17 UTC · re: Eric Sunshine · lore

Re: [PATCH 1/2] git version --build-options: report the build platform, too

Hi Eric,
On Sat, 9 Dec 2017, Eric Sunshine wrote:
Show 20 quoted lines
> On Fri, Dec 08, 2017 at 04:19:25PM -0500, Eric Sunshine wrote:
> > On Fri, Dec 8, 2017 at 4:17 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> > > On Fri, Dec 8, 2017 at 12:43 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > >> Jonathan Nieder <jrnieder@gmail.com> writes:
> > >>> Can this use GIT_BUILD_PLATFORM directly instead of going via the indirection
> > >>> of a mutable static string?  That is, something like
> > >>>
> > >>>               printf("machine: %s\n", GIT_BUILD_PLATFORM);
> > >>
> > >> Good point.  And if this is externally identified as "machine",
> > >> probably the macro should also use the same word, not "platform".
> > >> We can go either way, as long as we are consistent, though.
> > >
> > > In Autoconf parlance, this would be called "host architecture" (GIT_HOST_ARCH).
> > 
> > My bad: "host cpu", rather (GIT_HOST_CPU).
> 
> Dscho, when you re-roll, perhaps replace the current patch with the
> one below which determines the CPU type automatically rather than
> having to manually maintain and augment a bunch of #ifdefs in help.h.
Will do!

Ciao, Dscho

Johannes Schindelin· Dec 8, 2017, 17:02 UTC · re: Johannes Schindelin · lore

[PATCH 2/2] version --build-options: report commit, too, if possible

In particular when local tags are used (or tags that are pushed to some fork) to build Git, it is very hard to figure out from which particular revision a particular Git executable was built.

Let's just report that in our build options.

We need to be careful, though, to report when the current commit cannot be determined, e.g. when building from a tarball without any associated Git repository.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Makefile  | 4 +++-
 help.c    | 2 ++
 version.c | 1 +
 version.h | 1 +
 4 files changed, 7 insertions(+), 1 deletion(-)
Show changes to 4 files +7 −1

Makefile, help.c, version.c, version.h

diff --git a/Makefile b/Makefile
index fef9c8d2725..92a0ae3d8e3 100644
--- a/Makefile
+++ b/Makefile
@@ -1893,7 +1893,9 @@ builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
 version.sp version.s version.o: GIT-VERSION-FILE GIT-USER-AGENT
 version.sp version.s version.o: EXTRA_CPPFLAGS = \
 	'-DGIT_VERSION="$(GIT_VERSION)"' \
-	'-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)'
+	'-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)' \
+	'-DGIT_BUILT_FROM_COMMIT="$(shell git rev-parse -q --verify HEAD || \
+		echo "(unknown)")"'
 
 $(BUILT_INS): git$X
 	$(QUIET_BUILT_IN)$(RM) $@ && \
diff --git a/help.c b/help.c
index df1332fa3c9..6ebea665780 100644
--- a/help.c
+++ b/help.c
@@ -413,6 +413,8 @@ int cmd_version(int argc, const char **argv, const char *prefix)
 	printf("git version %s\n", git_version_string);
 
 	if (build_options) {
+		printf("built from commit: %s\n",
+		       git_built_from_commit_string);
 		printf("sizeof-long: %d\n", (int)sizeof(long));
 		printf("machine: %s\n", build_platform);
 		/* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */
diff --git a/version.c b/version.c
index 6106a8098c8..41b718c29e1 100644
--- a/version.c
+++ b/version.c
@@ -3,6 +3,7 @@
 #include "strbuf.h"
 
 const char git_version_string[] = GIT_VERSION;
+const char git_built_from_commit_string[] = GIT_BUILT_FROM_COMMIT;
 
 const char *git_user_agent(void)
 {
diff --git a/version.h b/version.h
index 6911a4f1558..7c62e805771 100644
--- a/version.h
+++ b/version.h
@@ -2,6 +2,7 @@
 #define VERSION_H
 
 extern const char git_version_string[];
+extern const char git_built_from_commit_string[];
 
 const char *git_user_agent(void);
 const char *git_user_agent_sanitized(void);
-- 
2.15.1.windows.2
Jonathan Nieder· Dec 8, 2017, 17:27 UTC · re: Johannes Schindelin · lore

Re: [PATCH 2/2] version --build-options: report commit, too, if possible

Hi,
Johannes Schindelin wrote:
> In particular when local tags are used (or tags that are pushed to some
> fork) to build Git, it is very hard to figure out from which particular
> revision a particular Git executable was built.

Hm, can you say more about how this comes up in practice? I wonder if we should always augment the version string with the commit hash. E.g. I am running

	git version 2.15.1.424.g9478a66081

now, which already includes the commit hash because it disambiguates the .424 thing, but depending on the motivation, maybe we would also want

	git version 2.15.1.0.g9b185bef0c15

for people running v2.15.1 (or at least when such a tag is not a well known, published tag).

> We need to be careful, though, to report when the current commit cannot be
> determined, e.g. when building from a tarball without any associated Git
> repository.
This means that on Debian, it would always print
	built from commit: (unknown)

Maybe I shouldn't care, but I wonder if there's a way to improve on that. E.g. should there be a makefile knob to allow Debian to specify what to put there?

Thanks, Jonathan

Junio C Hamano· Dec 8, 2017, 17:49 UTC · re: Jonathan Nieder · lore

Re: [PATCH 2/2] version --build-options: report commit, too, if possible

Jonathan Nieder <jrnieder@gmail.com> writes:
Show 11 quoted lines
>> We need to be careful, though, to report when the current commit cannot be
>> determined, e.g. when building from a tarball without any associated Git
>> repository.
>
> This means that on Debian, it would always print
>
> 	built from commit: (unknown)
>
> Maybe I shouldn't care, but I wonder if there's a way to improve on
> that. E.g. should there be a makefile knob to allow Debian to specify
> what to put there?

Another "interesting" possibility is to build from a tarball extracted into a directory hierarchy that is controlled by an unrelated Git repository. E.g. "my $HOME is under $HOME/.git repository, and then I have a tarball extract in $HOME/src/git". We shouldn't embed the HEAD commit of that $HOME directory project in the resulting executable in such a case.

We should be able to do this by being a bit more careful than the presented patch. Make sure that the toplevel is at the same directory as we assumed to be (i.e. where we found that Makefile) and trust rev-parse output only when that is the case, or something like that.

Johannes Schindelin· Dec 14, 2017, 23:27 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] version --build-options: report commit, too, if possible

Hi Junio,
On Fri, 8 Dec 2017, Junio C Hamano wrote:
Show 26 quoted lines
> Jonathan Nieder <jrnieder@gmail.com> writes:
> 
> >> We need to be careful, though, to report when the current commit cannot be
> >> determined, e.g. when building from a tarball without any associated Git
> >> repository.
> >
> > This means that on Debian, it would always print
> >
> > 	built from commit: (unknown)
> >
> > Maybe I shouldn't care, but I wonder if there's a way to improve on
> > that. E.g. should there be a makefile knob to allow Debian to specify
> > what to put there?
> 
> Another "interesting" possibility is to build from a tarball
> extracted into a directory hierarchy that is controlled by an
> unrelated Git repository.  E.g. "my $HOME is under $HOME/.git
> repository, and then I have a tarball extract in $HOME/src/git".
> We shouldn't embed the HEAD commit of that $HOME directory project
> in the resulting executable in such a case.
> 
> We should be able to do this by being a bit more careful than the
> presented patch.  Make sure that the toplevel is at the same
> directory as we assumed to be (i.e. where we found that Makefile)
> and trust rev-parse output only when that is the case, or something
> like that.
Cute.
I added specific handling for that.

Ciao, Dscho

Johannes Schindelin· Dec 14, 2017, 23:25 UTC · re: Jonathan Nieder · lore

Re: [PATCH 2/2] version --build-options: report commit, too, if possible

Hi Jonathan,
On Fri, 8 Dec 2017, Jonathan Nieder wrote:
Show 7 quoted lines
> Johannes Schindelin wrote:
> 
> > In particular when local tags are used (or tags that are pushed to some
> > fork) to build Git, it is very hard to figure out from which particular
> > revision a particular Git executable was built.
> 
> Hm, can you say more about how this comes up in practice?

I recently saw a version string on this here list (in a generated patch) that looked something like "git v2.14.0.chrome".

I sometimes build custom Git for Windows snapshots from commits that I keep in my own fork. I would expect other people to do the same.

With this patch, at least I can verify very easily whether I have access to the corresponding commit or not.

> I wonder if we should always augment the version string with the commit
> hash.
That would probably be more confusing than helpful to the end-users.
> E.g. I am running
> 
> 	git version 2.15.1.424.g9478a66081

which is currently good enough, but in the future may clash with another object, maybe even a commit. Unabbreviated commit names are what I am after.

Show 11 quoted lines
> > We need to be careful, though, to report when the current commit cannot be
> > determined, e.g. when building from a tarball without any associated Git
> > repository.
> 
> This means that on Debian, it would always print
> 
> 	built from commit: (unknown)
> 
> Maybe I shouldn't care, but I wonder if there's a way to improve on
> that. E.g. should there be a makefile knob to allow Debian to specify
> what to put there?

I changed the text to "no commit associated with this build". Does that suffice? If not, what "UI" would you suggest (most likely a new Makefile variable? What name would you prefer?).

Ciao, Dscho

Johannes Schindelin· Dec 14, 2017, 23:34 UTC · re: Johannes Schindelin · lore

[PATCH v2 0/2] Offer more information in `git version --build-options`'s output

In Git for Windows, we ask users to paste the output of said command into their bug reports, with the idea that this frequently helps identify where the problems are coming from.

There are some obvious missing bits of information in said output, though, and this patch series tries to fill the gaps at least a little.

Changes since v1:
- replaced 1/2 by Eric's proposed alternative patch
- when no commit can be determined, it now says
	no commit associated with this build
  instead of
	built from commit: (unknown)
- the code is now careful not to look further than Git's top-level
  directory for a Git repository from which to determine the current
  commit. As Junio pointed out, some developer may extract Git's source
  code from a tarball into a worktree of a completely different project.
Eric Sunshine (1):
  version --build-options: also report host CPU
Johannes Schindelin (1):
  version --build-options: report commit, too, if possible
 Makefile  | 13 ++++++++++++-
 help.c    |  6 ++++++
 version.c |  1 +
 version.h |  1 +
 4 files changed, 20 insertions(+), 1 deletion(-)
base-commit: 95ec6b1b3393eb6e26da40c565520a8db9796e9f
Published-As: https://github.com/dscho/git/releases/tag/built-from-commit-v2
Fetch-It-Via: git fetch https://github.com/dscho/git built-from-commit-v2
Interdiff vs v1:
 diff --git a/Makefile b/Makefile
 index 92a0ae3d8e3..2ce70d205d9 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -425,6 +425,9 @@ all::
  #
  # to say "export LESS=FRX (and LV=-c) if the environment variable
  # LESS (and LV) is not set, respectively".
 +#
 +# When cross-compiling, define HOST_CPU as the canonical name of the CPU on
 +# which the built Git will run (for instance "x86_64").
  
  GIT-VERSION-FILE: FORCE
  	@$(SHELL_PATH) ./GIT-VERSION-GEN
 @@ -1095,6 +1098,12 @@ else
  BROKEN_PATH_FIX = '/^\# @@BROKEN_PATH_FIX@@$$/d'
  endif
  
 +ifeq (,$(HOST_CPU))
 +	BASIC_CFLAGS += -DGIT_HOST_CPU="\"$(firstword $(subst -, ,$(uname_M)))\""
 +else
 +	BASIC_CFLAGS += -DGIT_HOST_CPU="\"$(HOST_CPU)\""
 +endif
 +
  ifneq (,$(INLINE))
  	BASIC_CFLAGS += -Dinline=$(INLINE)
  endif
 @@ -1894,8 +1903,8 @@ version.sp version.s version.o: GIT-VERSION-FILE GIT-USER-AGENT
  version.sp version.s version.o: EXTRA_CPPFLAGS = \
  	'-DGIT_VERSION="$(GIT_VERSION)"' \
  	'-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)' \
 -	'-DGIT_BUILT_FROM_COMMIT="$(shell git rev-parse -q --verify HEAD || \
 -		echo "(unknown)")"'
 +	'-DGIT_BUILT_FROM_COMMIT="$(shell GIT_CEILING_DIRECTORIES=\"$(CURDIR)/..\" \
 +		git rev-parse -q --verify HEAD || :)"'
  
  $(BUILT_INS): git$X
  	$(QUIET_BUILT_IN)$(RM) $@ && \
 diff --git a/help.c b/help.c
 index 6ebea665780..60071a9beaa 100644
 --- a/help.c
 +++ b/help.c
 @@ -390,7 +390,6 @@ const char *help_unknown_cmd(const char *cmd)
  
  int cmd_version(int argc, const char **argv, const char *prefix)
  {
 -	static char build_platform[] = GIT_BUILD_PLATFORM;
  	int build_options = 0;
  	const char * const usage[] = {
  		N_("git version [<options>]"),
 @@ -413,10 +412,13 @@ int cmd_version(int argc, const char **argv, const char *prefix)
  	printf("git version %s\n", git_version_string);
  
  	if (build_options) {
 -		printf("built from commit: %s\n",
 -		       git_built_from_commit_string);
 +		printf("cpu: %s\n", GIT_HOST_CPU);
 +		if (git_built_from_commit_string[0])
 +			printf("built from commit: %s\n",
 +			       git_built_from_commit_string);
 +		else
 +			printf("no commit associated with this build\n");
  		printf("sizeof-long: %d\n", (int)sizeof(long));
 -		printf("machine: %s\n", build_platform);
  		/* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */
  	}
  	return 0;
 diff --git a/help.h b/help.h
 index 42dd9852194..b21d7c94e8c 100644
 --- a/help.h
 +++ b/help.h
 @@ -33,16 +33,3 @@ extern void list_commands(unsigned int colopts, struct cmdnames *main_cmds, stru
   */
  extern void help_unknown_ref(const char *ref, const char *cmd, const char *error);
  #endif /* HELP_H */
 -
 -/*
 - * identify build platform
 - */
 -#ifndef GIT_BUILD_PLATFORM
 -	#if defined __x86__ || defined __i386__ || defined __i586__ || defined __i686__
 -		#define GIT_BUILD_PLATFORM "x86"
 -	#elif defined __x86_64__
 -		#define GIT_BUILD_PLATFORM "x86_64"
 -	#else
 -		#define GIT_BUILD_PLATFORM "unknown"
 -	#endif
 -#endif
-- 
2.15.1.windows.2
Johannes Schindelin· Dec 14, 2017, 23:34 UTC · re: Johannes Schindelin · lore

[PATCH v2 1/2] version --build-options: also report host CPU

From: Eric Sunshine <sunshine@sunshineco.com>

It can be helpful for bug reports to include information about the environment in which the bug occurs. "git version --build-options" can help to supplement this information. In addition to the size of 'long' already reported by --build-options, also report the host's CPU type. Example output:

   $ git version --build-options
   git version 2.9.3.windows.2.826.g06c0f2f
   cpu: x86_64
   sizeof-long: 4
New Makefile variable HOST_CPU supports cross-compiling.
Suggested-by: Adric Norris <landstander668@gmail.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Makefile | 9 +++++++++
 help.c   | 1 +
 2 files changed, 10 insertions(+)
Show changes to 2 files +10 −0

Makefile, help.c

diff --git a/Makefile b/Makefile
index fef9c8d2725..5587bccc932 100644
--- a/Makefile
+++ b/Makefile
@@ -425,6 +425,9 @@ all::
 #
 # to say "export LESS=FRX (and LV=-c) if the environment variable
 # LESS (and LV) is not set, respectively".
+#
+# When cross-compiling, define HOST_CPU as the canonical name of the CPU on
+# which the built Git will run (for instance "x86_64").
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1095,6 +1098,12 @@ else
 BROKEN_PATH_FIX = '/^\# @@BROKEN_PATH_FIX@@$$/d'
 endif
 
+ifeq (,$(HOST_CPU))
+	BASIC_CFLAGS += -DGIT_HOST_CPU="\"$(firstword $(subst -, ,$(uname_M)))\""
+else
+	BASIC_CFLAGS += -DGIT_HOST_CPU="\"$(HOST_CPU)\""
+endif
+
 ifneq (,$(INLINE))
 	BASIC_CFLAGS += -Dinline=$(INLINE)
 endif
diff --git a/help.c b/help.c
index 88a3aeaeb9f..cbcb159f367 100644
--- a/help.c
+++ b/help.c
@@ -412,6 +412,7 @@ int cmd_version(int argc, const char **argv, const char *prefix)
 	printf("git version %s\n", git_version_string);
 
 	if (build_options) {
+		printf("cpu: %s\n", GIT_HOST_CPU);
 		printf("sizeof-long: %d\n", (int)sizeof(long));
 		/* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */
 	}
-- 
2.15.1.windows.2
Johannes Schindelin· Dec 14, 2017, 23:34 UTC · re: Johannes Schindelin · lore

[PATCH v2 2/2] version --build-options: report commit, too, if possible

In particular when local tags are used (or tags that are pushed to some fork) to build Git, it is very hard to figure out from which particular revision a particular Git executable was built. It gets worse when those tags are deleted, or even updated.

Let's just report an exact, unabbreviated commit name in our build options.

We need to be careful, though, to report when the current commit cannot be determined, e.g. when building from a tarball without any associated Git repository. This could be the case also when extracting Git's source code into an unrelated Git worktree.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Makefile  | 4 +++-
 help.c    | 5 +++++
 version.c | 1 +
 version.h | 1 +
 4 files changed, 10 insertions(+), 1 deletion(-)
Show changes to 4 files +10 −1

Makefile, help.c, version.c, version.h

diff --git a/Makefile b/Makefile
index 5587bccc932..2ce70d205d9 100644
--- a/Makefile
+++ b/Makefile
@@ -1902,7 +1902,9 @@ builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
 version.sp version.s version.o: GIT-VERSION-FILE GIT-USER-AGENT
 version.sp version.s version.o: EXTRA_CPPFLAGS = \
 	'-DGIT_VERSION="$(GIT_VERSION)"' \
-	'-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)'
+	'-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)' \
+	'-DGIT_BUILT_FROM_COMMIT="$(shell GIT_CEILING_DIRECTORIES=\"$(CURDIR)/..\" \
+		git rev-parse -q --verify HEAD || :)"'
 
 $(BUILT_INS): git$X
 	$(QUIET_BUILT_IN)$(RM) $@ && \
diff --git a/help.c b/help.c
index cbcb159f367..60071a9beaa 100644
--- a/help.c
+++ b/help.c
@@ -413,6 +413,11 @@ int cmd_version(int argc, const char **argv, const char *prefix)
 
 	if (build_options) {
 		printf("cpu: %s\n", GIT_HOST_CPU);
+		if (git_built_from_commit_string[0])
+			printf("built from commit: %s\n",
+			       git_built_from_commit_string);
+		else
+			printf("no commit associated with this build\n");
 		printf("sizeof-long: %d\n", (int)sizeof(long));
 		/* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */
 	}
diff --git a/version.c b/version.c
index 6106a8098c8..41b718c29e1 100644
--- a/version.c
+++ b/version.c
@@ -3,6 +3,7 @@
 #include "strbuf.h"
 
 const char git_version_string[] = GIT_VERSION;
+const char git_built_from_commit_string[] = GIT_BUILT_FROM_COMMIT;
 
 const char *git_user_agent(void)
 {
diff --git a/version.h b/version.h
index 6911a4f1558..7c62e805771 100644
--- a/version.h
+++ b/version.h
@@ -2,6 +2,7 @@
 #define VERSION_H
 
 extern const char git_version_string[];
+extern const char git_built_from_commit_string[];
 
 const char *git_user_agent(void);
 const char *git_user_agent_sanitized(void);
-- 
2.15.1.windows.2
Junio C Hamano· Dec 15, 2017, 17:45 UTC · re: Johannes Schindelin · lore

Re: [PATCH v2 0/2] Offer more information in `git version --build-options`'s output

Johannes Schindelin <johannes.schindelin@gmx.de> writes:
Show 6 quoted lines
> - when no commit can be determined, it now says
>
> 	no commit associated with this build
>   instead of
>
> 	built from commit: (unknown)

I find this output somewhat klunky for machine parsing (and it is inconsistent with the style used for "sizeof-long", which hints that these are "<token> <colon> <value>" lines where whitespaces are avoided in a <token>), but hopefully this is primarily for human consumption and scrypts that are trying to find a specific piece of information would know how to use 'grep', so the inconsistency does not make much of a difference in practice anyway.

Queued.  Thanks.
Eric Sunshine· Dec 15, 2017, 18:03 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 0/2] Offer more information in `git version --build-options`'s output

On Fri, Dec 15, 2017 at 12:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 10 quoted lines
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>>       no commit associated with this build
>
> I find this output somewhat klunky for machine parsing (and it is
> inconsistent with the style used for "sizeof-long", which hints that
> these are "<token> <colon> <value>" lines where whitespaces are
> avoided in a <token>), but hopefully this is primarily for human
> consumption and scrypts that are trying to find a specific piece of
> information would know how to use 'grep', so the inconsistency does
> not make much of a difference in practice anyway.

Simply omitting that line from the output of --build-options would also be an option if the commit can not be determined...

Junio C Hamano· Dec 15, 2017, 18:12 UTC · re: Eric Sunshine · lore

Re: [PATCH v2 0/2] Offer more information in `git version --build-options`'s output

Eric Sunshine <sunshine@sunshineco.com> writes:
Show 14 quoted lines
> On Fri, Dec 15, 2017 at 12:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>>>       no commit associated with this build
>>
>> I find this output somewhat klunky for machine parsing (and it is
>> inconsistent with the style used for "sizeof-long", which hints that
>> these are "<token> <colon> <value>" lines where whitespaces are
>> avoided in a <token>), but hopefully this is primarily for human
>> consumption and scrypts that are trying to find a specific piece of
>> information would know how to use 'grep', so the inconsistency does
>> not make much of a difference in practice anyway.
>
> Simply omitting that line from the output of --build-options would
> also be an option if the commit can not be determined...

What we see in Dscho's patch is better than that option, though. At least, explicitly checking for "no commit associated" would tell you if the version of Git is too old to have "built from commit:" line or it is new enough but was built from a tarball.

← back to recent threads