[v5,07/15] bugreport: add curl version

Commit Message

Emily Shaffer Jan. 24, 2020, 3:34 a.m. UTC

From: Emily Shaffer <[e-mail]>

It's possible for git-remote-curl to be built separately from git; in that
case we want to know what version of cURL is used by git-remote-curl, not
necessarily which version was present at git-bugreport's build time.
So instead, ask git-remote-curl for the version information it knows
about.

For longevity purposes, invoke the alias "git-remote-https" instead of
"git-remote-http".

Since it could have been built at a different time, also report the
version and built-from commit of git-remote-curl alongside the cURL info.

Signed-off-by: Emily Shaffer <[e-mail]>
---
 bugreport.c   | 16 ++++++++++++++++
 remote-curl.c |  8 ++++++++
 2 files changed, 24 insertions(+)

Comments

Martin Ågren Jan. 30, 2020, 10:27 p.m. UTC | #1

On Fri, 24 Jan 2020 at 04:41, <[e-mail]> wrote:
> +static void get_curl_version_info(struct strbuf *curl_info)
> +{
> +       struct child_process cp = CHILD_PROCESS_INIT;
> +
> +       argv_array_push(&cp.args, "git");
> +       argv_array_push(&cp.args, "remote-https");
> +       argv_array_push(&cp.args, "--build-info");
> +       if (capture_command(&cp, curl_info, 0))
> +           strbuf_addstr(curl_info, "'git-remote-https --build-info' not supported\n");
> +}
>
>  static void get_system_info(struct strbuf *sys_info)
>  {
> @@ -31,6 +43,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_curl_version_info(sys_info);

The header here looks a lot like an implementation detail of
`get_curl_version_info()`. Or put differently, these risk getting out of
sync. Maybe frame the header a bit more human readable: "curl version".
But is this "curl version", or more like "git-remote-https version"?
There's some discrepancy here.

> +       strbuf_complete_line(sys_info);
>  }


Martin

Emily Shaffer Feb. 4, 2020, 10:54 p.m. UTC | #2

On Thu, Jan 30, 2020 at 11:27:45PM +0100, Martin Ågren wrote:
> On Fri, 24 Jan 2020 at 04:41, <[e-mail]> wrote:
> > +static void get_curl_version_info(struct strbuf *curl_info)
> > +{
> > +       struct child_process cp = CHILD_PROCESS_INIT;
> > +
> > +       argv_array_push(&cp.args, "git");
> > +       argv_array_push(&cp.args, "remote-https");
> > +       argv_array_push(&cp.args, "--build-info");
> > +       if (capture_command(&cp, curl_info, 0))
> > +           strbuf_addstr(curl_info, "'git-remote-https --build-info' not supported\n");
> > +}
> >
> >  static void get_system_info(struct strbuf *sys_info)
> >  {
> > @@ -31,6 +43,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_curl_version_info(sys_info);
> 
> The header here looks a lot like an implementation detail of
> `get_curl_version_info()`. Or put differently, these risk getting out of
> sync. Maybe frame the header a bit more human readable: "curl version".
> But is this "curl version", or more like "git-remote-https version"?
> There's some discrepancy here.

Hm, I think you're saying "If we switch to future-https-lib instead of
cURL for git-remote-https, then this command will be incorrectly named."
Sure, I agree. It's true that with this change git-remote-https also
tells us some info about itself.

Thanks.
 - Emily

Patch

diff --git a/bugreport.c b/bugreport.c
index 818ccb385c..73f6d39517 100644
--- a/bugreport.c
+++ b/bugreport.c
@@ -5,6 +5,18 @@ 
 #include "time.h"
 #include "help.h"
 #include "compat/compiler.h"
+#include "run-command.h"
+
+static void get_curl_version_info(struct strbuf *curl_info)
+{
+	struct child_process cp = CHILD_PROCESS_INIT;
+
+	argv_array_push(&cp.args, "git");
+	argv_array_push(&cp.args, "remote-https");
+	argv_array_push(&cp.args, "--build-info");
+	if (capture_command(&cp, curl_info, 0))
+	    strbuf_addstr(curl_info, "'git-remote-https --build-info' not supported\n");
+}
 
 static void get_system_info(struct strbuf *sys_info)
 {
@@ -31,6 +43,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_curl_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 350d92a074..c590fbfae3 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 */
@@ -1374,6 +1375,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);
+		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