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

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

[Johannes Schindelin]

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

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

[Johannes Schindelin]

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(+)

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

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

[Jonathan Nieder]

Hi,

Johannes Schindelin wrote:

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

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

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

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

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

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

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

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

[Eric Sunshine]

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

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

[Eric Sunshine]

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:
>>>> @@ -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).

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

[Eric Sunshine]

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.

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(+)

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

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

[Johannes Schindelin]

Hi Eric,

On Sat, 9 Dec 2017, Eric Sunshine wrote:

> 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

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

[Johannes Schindelin]

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(-)

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

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

[Jonathan Nieder]

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

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

[Junio C Hamano]

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.

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

[Johannes Schindelin]

Hi Junio,

On Fri, 8 Dec 2017, Junio C Hamano wrote:

> 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

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

[Johannes Schindelin]

Hi Jonathan,

On Fri, 8 Dec 2017, Jonathan Nieder wrote:

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

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

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

[Johannes Schindelin]

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

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

[Johannes Schindelin]

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(+)

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

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

[Johannes Schindelin]

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(-)

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

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

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

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

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

[Eric Sunshine]

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

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

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

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