Subject: Re: [PATCH v4 07/15] bugreport: add curl version

From: Emily Shaffer <[email-masked]>
Date: Mon, 16 Dec 2019 14:49:28 -0800

On Fri, Dec 13, 2019 at 01:27:31PM -0800, Junio C Hamano wrote:
> Emily Shaffer <[email-masked]> writes:
> 
> > It's possible for git-http* to be built separately from git; in that
> > case we want to know what version of cURL is used by git-http*, not
> > necessarily which version was present at git-bugreport's build time.
> > So instead, ask git-http-fetch for the version information it knows
> > about.
> >
> > git-http-fetch was chosen as git-http-backend was described as a
> > server-side implementation, and as an accidental fetch in case of
> > problems was considered less harmful than an accidental push.
> >
> > Since it could have been built at a different time, also report the
> > version and built-from commit of git-http-fetch alongside the cURL info.
> 
> One possible issue I have is that I was hoping that eventually we
> can discard "git http-fetch" altogether sometime in the future.
> Does anybody still use the dumb HTTP transport seriously?  

Oh, interesting. I was about to say, "I still use it to fetch, when I
don't really care" - but that isn't even true, as I fetch via https (and
have so much muscle memory to type https that I don't even notice).`

> 
> And the first move in that direction would be to allow the system be
> built without http-fetch, even if git-remote-curl (and its aliases)
> would still be built to access smart-http transports.
> 
> So, I am not sure.  This is just the matter of adding an out-of-line
> hidden option used only for environment inspection, so if it can be
> done to git-remote-curl, that would probably be much more future
> proof.

Ok. I'll move it.

> 
> > diff --git a/bugreport.c b/bugreport.c
> > index af715dc157..f5598513d9 100644
> > --- a/bugreport.c
> > +++ b/bugreport.c
> > @@ -5,6 +5,18 @@
> >  #include "time.h"
> >  #include "help.h"
> >  #include <gnu/libc-version.h>
> > +#include "run-command.h"
> > +
> > +static void get_http_version_info(struct strbuf *http_info)
> > +{
> > +	struct child_process cp = CHILD_PROCESS_INIT;
> > +
> > +	argv_array_push(&cp.args, "git");
> > +	argv_array_push(&cp.args, "http-fetch");
> > +	argv_array_push(&cp.args, "-V");
> > +	if (capture_command(&cp, http_info, 0))
> > +	    strbuf_addstr(http_info, "'git-http-fetch -V' not supported\n");
> 
> OK.  We probably can also take the compile-time "NO_CURL" into account,
> so that we can tell a misconfigured installation that wanted to have
> CURL but failed to install a usable http-fetch and an installation
> that deliberately omitted anything cURL?

Oh, interesting idea! I'll add that.

> 
> >  static void get_system_info(struct strbuf *sys_info)
> >  {
> > @@ -32,6 +44,10 @@ static void get_system_info(struct strbuf *sys_info)
> >  	strbuf_addstr(sys_info, "glibc version: ");
> >  	strbuf_addstr(sys_info, gnu_get_libc_version());
> >  	strbuf_complete_line(sys_info);
> > +
> > +	strbuf_addstr(sys_info, "git-http-fetch -V:\n");
> > +	get_http_version_info(sys_info);
> > +	strbuf_complete_line(sys_info);
> >  }
> >  
> >  static const char * const bugreport_usage[] = {
> > diff --git a/http-fetch.c b/http-fetch.c
> > index a32ac118d9..31844812a1 100644
> > --- a/http-fetch.c
> > +++ b/http-fetch.c
> > @@ -3,9 +3,18 @@
> >  #include "exec-cmd.h"
> >  #include "http.h"
> >  #include "walker.h"
> > +#include "version.h"
> >  
> >  static const char http_fetch_usage[] = "git http-fetch "
> > -"[-c] [-t] [-a] [-v] [--recover] [-w ref] [--stdin] commit-id url";
> > +"[-c] [-t] [-a] [-v] [-V] [--recover] [-w ref] [--stdin] commit-id url";
> > +
> > +void NORETURN version_info()
> 
> void NORETURN version_info(void)
> 
> 
> > +{
> > +	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());
> > +	exit(0);
> > +}
> >  
> >  int cmd_main(int argc, const char **argv)
> >  {
> > @@ -26,6 +35,8 @@ int cmd_main(int argc, const char **argv)
> >  		} else if (argv[arg][1] == 'a') {
> >  		} else if (argv[arg][1] == 'v') {
> >  			get_verbosely = 1;
> > +		} else if (argv[arg][1] == 'V') {
> > +			version_info();
> >  		} else if (argv[arg][1] == 'w') {
> >  			write_ref = &argv[arg + 1];
> >  			arg++;