Re: [PATCH v8 07/15] bugreport: add git-remote-https version

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 20, 2020, 20:35 UTC
Message-ID: <xmqqr1yp9eue.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20200220015858.181086-8-emilyshaffer@google.com>

Emily Shaffer <emilyshaffer@google.com> writes:

Show 42 quoted lines

> +static void get_git_remote_https_version_info(struct strbuf *version_info)
> +{
> +	struct child_process cp = CHILD_PROCESS_INIT;
> +
> +	cp.git_cmd = 1;
> +	argv_array_push(&cp.args, "remote-https");
> +	argv_array_push(&cp.args, "--build-info");
> +	if (capture_command(&cp, version_info, 0))
> +	    strbuf_addstr(version_info, "'git-remote-https --build-info' not supported\n");
> +}
>  
>  static void get_system_info(struct strbuf *sys_info)
>  {
> @@ -29,6 +41,10 @@ static void get_system_info(struct strbuf *sys_info)
>  	strbuf_addstr(sys_info, "compiler info: ");
>  	get_compiler_info(sys_info);
>  	strbuf_complete_line(sys_info);
> +
> +	strbuf_addstr(sys_info, "git-remote-https --build-info:\n");
> +	get_git_remote_https_version_info(sys_info);
> +	strbuf_complete_line(sys_info);
>  }
>  
>  static const char * const bugreport_usage[] = {
> diff --git a/remote-curl.c b/remote-curl.c
> index 8eb96152f5..73e52175c0 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -17,6 +17,7 @@
>  #include "protocol.h"
>  #include "quote.h"
>  #include "transport.h"
> +#include "version.h"
>  
>  static struct remote *remote;
>  /* always ends with a trailing slash */
> @@ -1375,6 +1376,13 @@ int cmd_main(int argc, const char **argv)
>  	string_list_init(&options.deepen_not, 1);
>  	string_list_init(&options.push_options, 1);
>  
> +	if (!strcmp("--build-info", argv[1])) {
> +		printf("git-http-fetch version: %s\n", git_version_string);

We are letting bugreport grab this information from remote-curl, and they are different binaries, so git_version_string we see here is that of the remote-curl. Good.

Show 8 quoted lines

> +		printf("built from commit: %s\n", git_built_from_commit_string);
> +		printf("curl version: %s\n", curl_version());
> +		return 0;
> +	}
> +
>  	/*
>  	 * Just report "remote-curl" here (folding all the various aliases
>  	 * ("git-remote-http", "git-remote-https", and etc.) here since they

Makes sense, except that it is not clear what our overall stance on i18n/l10n of the bugreport output.

I think there are two schools of thought.

 - You can stick to C local, with an expectation that developers can
   read reports in C locale.  This will allow developers to avoid
   having to read reports written in a language they do not
   understand, and also makes it easier to mechanically process
   reports.

 - You can make sure what matters in the report is localized to the
   end-users' locale.  This will avoid forcing end-users to send out
   a report that they may not even able to read (which is what
   happens if we did not do i18n/l10n).

I am not sure which way you are aiming at. The boilerplate text we saw in a very early part of the series were marked N_() but some of the messages in later parts of the series are not.