Re: [PATCH v2 2/2] rebase: validate -C<n> and --whitespace=<mode> parameters early

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ævar,

On Mon, 19 Nov 2018, Ævar Arnfjörð Bjarmason wrote:

> 
> On Wed, Nov 14 2018, Johannes Schindelin via GitGitGadget wrote:
> 
> > From: Johannes Schindelin <johannes.schindelin@xxxxxx>
> >
> > It is a good idea to error out early upon seeing, say, `-Cbad`, rather
> > than starting the rebase only to have the `--am` backend complain later.
> >
> > Let's do this.
> >
> > The only options accepting parameters which we pass through to `git am`
> > (which may, or may not, forward them to `git apply`) are `-C` and
> > `--whitespace`. The other options we pass through do not accept
> > parameters, so we do not have to validate them here.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> > ---
> >  builtin/rebase.c          | 12 +++++++++++-
> >  t/t3406-rebase-message.sh |  7 +++++++
> >  2 files changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/builtin/rebase.c b/builtin/rebase.c
> > index 96ffa80b71..571cf899d5 100644
> > --- a/builtin/rebase.c
> > +++ b/builtin/rebase.c
> > @@ -1064,12 +1064,22 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
> >  	}
> >
> >  	for (i = 0; i < options.git_am_opts.argc; i++) {
> > -		const char *option = options.git_am_opts.argv[i];
> > +		const char *option = options.git_am_opts.argv[i], *p;
> >  		if (!strcmp(option, "--committer-date-is-author-date") ||
> >  		    !strcmp(option, "--ignore-date") ||
> >  		    !strcmp(option, "--whitespace=fix") ||
> >  		    !strcmp(option, "--whitespace=strip"))
> >  			options.flags |= REBASE_FORCE;
> > +		else if (skip_prefix(option, "-C", &p)) {
> > +			while (*p)
> > +				if (!isdigit(*(p++)))
> > +					die(_("switch `C' expects a "
> > +					      "numerical value"));
> > +		} else if (skip_prefix(option, "--whitespace=", &p)) {
> > +			if (*p && strcmp(p, "warn") && strcmp(p, "nowarn") &&
> > +			    strcmp(p, "error") && strcmp(p, "error-all"))
> > +				die("Invalid whitespace option: '%s'", p);
> > +		}
> >  	}
> >
> >  	if (!(options.flags & REBASE_NO_QUIET))
> > diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
> > index 0392e36d23..2c79eed4fe 100755
> > --- a/t/t3406-rebase-message.sh
> > +++ b/t/t3406-rebase-message.sh
> > @@ -84,4 +84,11 @@ test_expect_success 'rebase --onto outputs the invalid ref' '
> >  	test_i18ngrep "invalid-ref" err
> >  '
> >
> > +test_expect_success 'error out early upon -C<n> or --whitespace=<bad>' '
> > +	test_must_fail git rebase -Cnot-a-number HEAD 2>err &&
> > +	test_i18ngrep "numerical value" err &&
> > +	test_must_fail git rebase --whitespace=bad HEAD 2>err &&
> > +	test_i18ngrep "Invalid whitespace option" err
> > +'
> > +
> 
> The combination of this gitster/js/rebase-am-options and my
> gitster/ab/rebase-in-c-escape-hatch breaks tests under
> GIT_TEST_REBASE_USE_BUILTIN=false for obvious reasons. The C version is
> now more strict.

Maybe you can concoct a prereq for this test? Something like

test_lazy_prereq BUILTIN_REBASE '
	test true = "${GIT_TEST_REBASE_USE_BUILTIN:-true}"
'

Ciao,
Dscho

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux