| Message ID | 20191213004312.169753-8-emilyshaffer@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4,01/15] bugreport: add tool to generate debugging info | expand |
Emily Shaffer <emilyshaffer@google.com> 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? 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. > 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? > 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++;
On Fri, Dec 13, 2019 at 01:27:31PM -0800, Junio C Hamano wrote: > Emily Shaffer <emilyshaffer@google.com> 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++;
Hi Emily, On Thu, 12 Dec 2019, Emily Shaffer wrote: > 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() Pretty much all the builds in https://dev.azure.com/gitgitgadget/git/_build/results?buildId=23830&view=logs&jobId=253e5128-1058-5bd4-fdf1-9b556d3207f8&j=fd490c07-0b22-5182-fac9-6d67fe1e939b&t=ce91d5d6-0c55-50f5-8ab9-6695c03ab102 fail because this function definition needs `(void)` instead of `()`. Ciao, Dscho > +{ > + 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++; > -- > 2.24.1.735.g03f4e72817-goog > > >
diff --git a/Documentation/git-http-fetch.txt b/Documentation/git-http-fetch.txt index 666b042679..2894c5e82b 100644 --- a/Documentation/git-http-fetch.txt +++ b/Documentation/git-http-fetch.txt @@ -10,6 +10,7 @@ SYNOPSIS -------- [verse] 'git http-fetch' [-c] [-t] [-a] [-d] [-v] [-w filename] [--recover] [--stdin] <commit> <url> +'git http-fetch' [-V] DESCRIPTION ----------- @@ -30,6 +31,10 @@ commit-id:: -v:: Report what is downloaded. +-V:: + Report information about the version of git-http-fetch, including the + versions of its dependencies. + -w <filename>:: Writes the commit-id into the filename under $GIT_DIR/refs/<filename> on the local end after the transfer is complete. 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"); +} 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() +{ + 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++;
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. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> --- Documentation/git-http-fetch.txt | 5 +++++ bugreport.c | 16 ++++++++++++++++ http-fetch.c | 13 ++++++++++++- 3 files changed, 33 insertions(+), 1 deletion(-)