| Message ID | 20200214015343.201946-8-emilyshaffer@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v7,01/15] help: move list_config_help to builtin/help | expand |
Hi Emily, On Thu, 13 Feb 2020, Emily Shaffer wrote: > diff --git a/bugreport.c b/bugreport.c > index 4f9101caeb..bfdff33368 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_git_remote_https_version_info(struct strbuf *version_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, version_info, 0)) Let's use RUN_GIT_CMD instead of adding `"git"` explicitly; It documents that we're interested in a Git command, and if we ever build a single-binary version of Git (as some Git for Windows users already asked for), it will make things easier. > + strbuf_addstr(version_info, "'git-remote-https --build-info' not supported\n"); > +} > > static void get_system_info(struct strbuf *sys_info) > { > diff --git a/remote-curl.c b/remote-curl.c > index 350d92a074..c590fbfae3 100644 > --- a/remote-curl.c > +++ b/remote-curl.c > @@ -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])) { The context does not say this, but at this point, we already verified that `argc` is larger than 1. Good. Also, in keeping with the existing code, we would need to use `--build-options` here (this is what `git version` calls the equivalent mode). _However_. I like your `--build-info` a lot more than `--build-options` (because the latter is very misleading: the commit and the date of the build are not "options" at all). Thanks, 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()); > + return 0; > + } > + > /* > * Just report "remote-curl" here (folding all the various aliases > * ("git-remote-http", "git-remote-https", and etc.) here since they > -- > 2.25.0.265.gbab2e86ba0-goog > >
On Wed, Feb 19, 2020 at 03:28:35PM +0100, Johannes Schindelin wrote: > Hi Emily, > > On Thu, 13 Feb 2020, Emily Shaffer wrote: > > > diff --git a/bugreport.c b/bugreport.c > > index 4f9101caeb..bfdff33368 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_git_remote_https_version_info(struct strbuf *version_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, version_info, 0)) > > Let's use RUN_GIT_CMD instead of adding `"git"` explicitly; It documents > that we're interested in a Git command, and if we ever build a > single-binary version of Git (as some Git for Windows users already asked > for), it will make things easier. Hm. RUN_GIT_CMD is an argument used for the run_command_v_opt* family of calls, but it seems that setting child_process.git_cmd has the same effect. Done. > > > + strbuf_addstr(version_info, "'git-remote-https --build-info' not supported\n"); > > +} > > > > static void get_system_info(struct strbuf *sys_info) > > { > > diff --git a/remote-curl.c b/remote-curl.c > > index 350d92a074..c590fbfae3 100644 > > --- a/remote-curl.c > > +++ b/remote-curl.c > > @@ -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])) { > > The context does not say this, but at this point, we already verified that > `argc` is larger than 1. Good. > > Also, in keeping with the existing code, we would need to use > `--build-options` here (this is what `git version` calls the equivalent > mode). > > _However_. > > I like your `--build-info` a lot more than `--build-options` (because the > latter is very misleading: the commit and the date of the build are not > "options" at all). Sure. Thanks for saying so. - Emily
Emily Shaffer <emilyshaffer@google.com> writes: >> Also, in keeping with the existing code, we would need to use >> `--build-options` here (this is what `git version` calls the equivalent >> mode). >> >> _However_. >> >> I like your `--build-info` a lot more than `--build-options` (because the >> latter is very misleading: the commit and the date of the build are not >> "options" at all). > > Sure. Thanks for saying so. I don't think anybody would mind introducing --build-info to "git version" as a synonym and deprecate --build-options from it ;-)
Hi Junio, On Wed, 19 Feb 2020, Junio C Hamano wrote: > Emily Shaffer <emilyshaffer@google.com> writes: > > >> Also, in keeping with the existing code, we would need to use > >> `--build-options` here (this is what `git version` calls the equivalent > >> mode). > >> > >> _However_. > >> > >> I like your `--build-info` a lot more than `--build-options` (because the > >> latter is very misleading: the commit and the date of the build are not > >> "options" at all). > > > > Sure. Thanks for saying so. > > I don't think anybody would mind introducing --build-info to "git > version" as a synonym and deprecate --build-options from it ;-) Heh. I almost suggested this. Almost. Ciao, Dscho
diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt index 8bbc4c960c..33df4dec7f 100644 --- a/Documentation/git-bugreport.txt +++ b/Documentation/git-bugreport.txt @@ -28,6 +28,7 @@ The following information is captured automatically: - 'git version --build-options' - uname sysname, release, version, and machine strings - Compiler-specific info string + - 'git remote-https --build-info' OPTIONS ------- diff --git a/bugreport.c b/bugreport.c index 4f9101caeb..bfdff33368 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_git_remote_https_version_info(struct strbuf *version_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, 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 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
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. Today, "git-remote-http" and "git-remote-https" are aliased to "git-remote-curl"; but in case we rely on a different library than cURL in the future, let's not explicitly reference cURL from bugreport. 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 <emilyshaffer@google.com> --- Documentation/git-bugreport.txt | 1 + bugreport.c | 16 ++++++++++++++++ remote-curl.c | 8 ++++++++ 3 files changed, 25 insertions(+)