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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Dec 8, 2017, 17:23 UTC
Message-ID: <20171208172324.GB14261@aiede.mtv.corp.google.com>
In-Reply-To: <978adf12e85ec6c4d407ba09da82945a08ee1f8c.1512752468.git.johannes.schindelin@gmx.de>

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