Re: [PATCH v7 07/15] bugreport: add git-remote-https version
- From
Emily Shaffer <emilyshaffer@google.com>
- Date
- Feb 19, 2020, 22:28 UTC
- Message-ID
- <20200219222807.GB26221@google.com>
- In-Reply-To
- <nycvar.QRO.7.76.6.2002191524460.46@tvgsbejvaqbjf.bet>
On Wed, Feb 19, 2020 at 03:28:35PM +0100, Johannes Schindelin wrote:
Show 27 quoted lines
> 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.
Show 28 quoted lines
>
> > + 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