git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Nov 19, 2018, 12:38 UTC
Message-ID
<874lcd1bub.fsf@evledraar.gmail.com>
In-Reply-To
<4c2ba5276636097a1c3bb2207c7537831e9079bc.1542212726.git.gitgitgadget@gmail.com>
On Wed, Nov 14 2018, Johannes Schindelin via GitGitGadget wrote:
Show 61 quoted lines
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> 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@gmx.de>
> ---
>  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.

Previous: Johannes SchindelinNext: Johannes Schindelin
Message 23 of 26 in “rebase: understand -C again, refactor”
  1. 0/1 rebase: understand -C again, refactorJohannes Schindelin via GitGitGadget, Nov 13, 2018
  2. 1/1 rebase: really just passthru the `git am` optionsJohannes Schindelin via GitGitGadget, Nov 13, 2018
  3. Junio C HamanoNov 13, 2018
  4. Phillip WoodNov 13, 2018
  5. Johannes SchindelinNov 13, 2018
  6. Phillip WoodNov 13, 2018
  7. rebase-in-C stability for 2.20Ævar Arnfjörð Bjarmason, Nov 13, 2018
  8. Stefan BellerNov 14, 2018
  9. 0/2 rebase.useBuiltin doc & test modeÆvar Arnfjörð Bjarmason, Nov 14, 2018
  10. Johannes SchindelinNov 14, 2018
  11. 1/2 rebase doc: document rebase.useBuiltinÆvar Arnfjörð Bjarmason, Nov 14, 2018
  12. 2/2 tests: add a special setup where rebase.useBuiltin is offÆvar Arnfjörð Bjarmason, Nov 14, 2018
  13. Elijah NewrenNov 14, 2018
  14. Junio C HamanoNov 14, 2018
  15. Johannes SchindelinNov 14, 2018
  16. Jeff KingNov 14, 2018
  17. Johannes SchindelinNov 14, 2018
  18. 0/2 rebase: understand -C again, refactorJohannes Schindelin via GitGitGadget, Nov 14, 2018
  19. 1/2 rebase: really just passthru the `git am` optionsJohannes Schindelin via GitGitGadget, Nov 14, 2018
  20. 2/2 rebase: validate -C<n> and --whitespace=<mode> parameters earlyJohannes Schindelin via GitGitGadget, Nov 14, 2018
  21. Phillip WoodNov 14, 2018
  22. Johannes SchindelinNov 14, 2018
  23. Ævar Arnfjörð BjarmasonNov 19, 2018
  24. Johannes SchindelinNov 20, 2018
  25. rebase: mark a test as failing with rebase.useBuiltin=falseÆvar Arnfjörð Bjarmason, Nov 20, 2018
  26. Johannes SchindelinNov 20, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.