Re: [PATCH v8 07/15] bugreport: add git-remote-https version
- From
Emily Shaffer <emilyshaffer@google.com>
- Date
- Feb 20, 2020, 23:28 UTC
- Message-ID
- <20200220232812.GH2447@google.com>
- In-Reply-To
- <xmqqr1yp9eue.fsf@gitster-ct.c.googlers.com>
On Thu, Feb 20, 2020 at 12:35:37PM -0800, Junio C Hamano wrote:
Show 77 quoted lines
> Emily Shaffer <emilyshaffer@google.com> writes:
>
> > +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.
>
> > + 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.I mentioned it in another reply to you at a different point in the v8 conversation; I took away the localization on the boilerplate at your suggestion. Unfortunately I agree that we wouldn't be equipped to handle reports in other languages. Not being able to understand the user-fillable part of the report extends, I think, to not being able to understand the diagnostic info. If the point of the report is to skip back-and-forth, then writing back to say "actually, run all this stuff manually so we can see the output in English" is not going to achieve that goal.
- Emily