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

2 messages, 2 authors, 2020-02-20 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2020-02-20 20:35:45

Emily Shaffer [off-list ref] writes:
quoted hunk
+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.

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

From: Emily Shaffer <hidden>
Date: 2020-02-20 23:28:19

On Thu, Feb 20, 2020 at 12:35:37PM -0800, Junio C Hamano wrote:
Emily Shaffer [off-list ref] writes:
quoted
+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.
quoted
+		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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help