rebase: understand -C again, refactor

Phillip Wood reported a problem where the built-in rebase did not understand options like -C1, i.e. it did not expect the option argument.

While investigating how to address this best, I stumbled upon OPT_PASSTHRU_ARGV (which I was so far happily unaware of).

Instead of just fixing the -C<n> bug, I decided to simply convert all of the options intended for git am (or, eventually, for git apply). This happens to fix that bug, and does so much more: it simplifies the entire logic (and removes more lines than it adds).

Johannes Schindelin (1):
rebase: really just passthru the git am options

 builtin/rebase.c | 98 +++++++++++++++++-------------------------------
 1 file changed, 35 insertions(+), 63 deletions(-)

base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-76%2Fdscho%2Frebase-Cn-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-76/dscho/rebase-Cn-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/76

gitgitgadget

From: Johannes Schindelin <[email protected]>

Currently, we parse the options intended for git am as if we wanted to handle them in git rebase, and then reconstruct them painstakingly to define the git_am_opt variable.

However, there is a much better way (that I was unaware of, at the time when I mentored Pratik to implement these options): OPT_PASSTHRU_ARGV. It is intended for exactly this use case, where command-line options want to be parsed into a separate argv_array.

Let's use this feature.

Incidentally, this also allows us to address a bug discovered by Phillip Wood, where the built-in rebase failed to understand that the -C option takes an optional argument.

---
 builtin/rebase.c | 98 +++++++++++++++++-------------------------------
 1 file changed, 35 insertions(+), 63 deletions(-)

index 0ee06aa363..96ffa80b71 100644
@@ -87,7 +87,7 @@ struct rebase_options {
 		REBASE_FORCE = 1<<3,
 		REBASE_INTERACTIVE_EXPLICIT = 1<<4,
 	} flags;
-	struct strbuf git_am_opt;
+	struct argv_array git_am_opts;
 	const char *action;
 	int signoff;
 	int allow_rerere_autoupdate;

@@ -339,7 +339,7 @@ N_("Resolve all conflicts manually, mark them as resolved with\n"
 static int run_specific_rebase(struct rebase_options *opts)
 {
 	const char *argv[] = { NULL, NULL };
-	struct strbuf script_snippet = STRBUF_INIT;
+	struct strbuf script_snippet = STRBUF_INIT, buf = STRBUF_INIT;
 	int status;
 	const char *backend, *backend_func;
 

@@ -433,7 +433,9 @@ static int run_specific_rebase(struct rebase_options *opts)
 		oid_to_hex(&opts->restrict_revision->object.oid) : NULL);
 	add_var(&script_snippet, "GIT_QUIET",
 		opts->flags & REBASE_NO_QUIET ? "" : "t");
-	add_var(&script_snippet, "git_am_opt", opts->git_am_opt.buf);
+	sq_quote_argv_pretty(&buf, opts->git_am_opts.argv);
+	add_var(&script_snippet, "git_am_opt", buf.buf);
+	strbuf_release(&buf);
 	add_var(&script_snippet, "verbose",
 		opts->flags & REBASE_VERBOSE ? "t" : "");
 	add_var(&script_snippet, "diffstat",

@@ -756,7 +758,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	struct rebase_options options = {
 		.type = REBASE_UNSPECIFIED,
 		.flags = REBASE_NO_QUIET,
-		.git_am_opt = STRBUF_INIT,
+		.git_am_opts = ARGV_ARRAY_INIT,
 		.allow_rerere_autoupdate  = -1,
 		.allow_empty_message = 1,
 		.git_format_patch_opt = STRBUF_INIT,

@@ -777,12 +779,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		ACTION_EDIT_TODO,
 		ACTION_SHOW_CURRENT_PATCH,
 	} action = NO_ACTION;
-	int committer_date_is_author_date = 0;
-	int ignore_date = 0;
-	int ignore_whitespace = 0;
 	const char *gpg_sign = NULL;
-	int opt_c = -1;
-	struct string_list whitespace = STRING_LIST_INIT_NODUP;
 	struct string_list exec = STRING_LIST_INIT_NODUP;
 	const char *rebase_merges = NULL;
 	int fork_point = -1;

@@ -804,15 +801,20 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		{OPTION_NEGBIT, 'n', "no-stat", &options.flags, NULL,
 			N_("do not show diffstat of what changed upstream"),
 			PARSE_OPT_NOARG, NULL, REBASE_DIFFSTAT },
-		OPT_BOOL(0, "ignore-whitespace", &ignore_whitespace,
-			 N_("passed to 'git apply'")),
 		OPT_BOOL(0, "signoff", &options.signoff,
 			 N_("add a Signed-off-by: line to each commit")),
-		OPT_BOOL(0, "committer-date-is-author-date",
-			 &committer_date_is_author_date,
-			 N_("passed to 'git am'")),
-		OPT_BOOL(0, "ignore-date", &ignore_date,
-			 N_("passed to 'git am'")),
+		OPT_PASSTHRU_ARGV(0, "ignore-whitespace", &options.git_am_opts,
+				  NULL, N_("passed to 'git am'"),
+				  PARSE_OPT_NOARG),
+		OPT_PASSTHRU_ARGV(0, "committer-date-is-author-date",
+				  &options.git_am_opts, NULL,
+				  N_("passed to 'git am'"), PARSE_OPT_NOARG),
+		OPT_PASSTHRU_ARGV(0, "ignore-date", &options.git_am_opts, NULL,
+				  N_("passed to 'git am'"), PARSE_OPT_NOARG),
+		OPT_PASSTHRU_ARGV('C', NULL, &options.git_am_opts, N_("n"),
+				  N_("passed to 'git apply'"), 0),
+		OPT_PASSTHRU_ARGV(0, "whitespace", &options.git_am_opts,
+				  N_("action"), N_("passed to 'git apply'"), 0),
 		OPT_BIT('f', "force-rebase", &options.flags,
 			N_("cherry-pick all commits, even if unchanged"),
 			REBASE_FORCE),

@@ -856,10 +858,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		{ OPTION_STRING, 'S', "gpg-sign", &gpg_sign, N_("key-id"),
 			N_("GPG-sign commits"),
 			PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
-		OPT_STRING_LIST(0, "whitespace", &whitespace,
-				N_("whitespace"), N_("passed to 'git apply'")),
-		OPT_SET_INT('C', NULL, &opt_c, N_("passed to 'git apply'"),
-			    REBASE_AM),
 		OPT_BOOL(0, "autostash", &options.autostash,
 			 N_("automatically stash/stash pop before and after")),
 		OPT_STRING_LIST('x', "exec", &exec, N_("exec"),

@@ -884,6 +882,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 			 N_("rebase all reachable commits up to the root(s)")),
 		OPT_END(),
 	};
+	int i;
 
 	/*
 	 * NEEDSWORK: Once the builtin rebase has been tested enough

@@ -1064,22 +1063,17 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		    state_dir_base, cmd_live_rebase, buf.buf);
 	}
 
-	if (!(options.flags & REBASE_NO_QUIET))
-		strbuf_addstr(&options.git_am_opt, " -q");
-
-	if (committer_date_is_author_date) {
-		strbuf_addstr(&options.git_am_opt,
-			      " --committer-date-is-author-date");
-		options.flags |= REBASE_FORCE;
+	for (i = 0; i < options.git_am_opts.argc; i++) {
+		const char *option = options.git_am_opts.argv[i];
+		if (!strcmp(option, "--committer-date-is-author-date") ||
+		    !strcmp(option, "--ignore-date") ||
+		    !strcmp(option, "--whitespace=fix") ||
+		    !strcmp(option, "--whitespace=strip"))
+			options.flags |= REBASE_FORCE;
 	}
 
-	if (ignore_whitespace)
-		strbuf_addstr(&options.git_am_opt, " --ignore-whitespace");
-
-	if (ignore_date) {
-		strbuf_addstr(&options.git_am_opt, " --ignore-date");
-		options.flags |= REBASE_FORCE;
-	}
+	if (!(options.flags & REBASE_NO_QUIET))
+		argv_array_push(&options.git_am_opts, "-q");
 
 	if (options.keep_empty)
 		imply_interactive(&options, "--keep-empty");

@@ -1089,23 +1083,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		options.gpg_sign_opt = xstrfmt("-S%s", gpg_sign);
 	}
 
-	if (opt_c >= 0)
-		strbuf_addf(&options.git_am_opt, " -C%d", opt_c);
-
-	if (whitespace.nr) {
-		int i;
-
-		for (i = 0; i < whitespace.nr; i++) {
-			const char *item = whitespace.items[i].string;
-
-			strbuf_addf(&options.git_am_opt, " --whitespace=%s",
-				    item);
-
-			if ((!strcmp(item, "fix")) || (!strcmp(item, "strip")))
-				options.flags |= REBASE_FORCE;
-		}
-	}
-
 	if (exec.nr) {
 		int i;
 

@@ -1181,23 +1158,18 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		break;
 	}
 
-	if (options.git_am_opt.len) {
-		const char *p;
-
+	if (options.git_am_opts.argc) {
 		/* all am options except -q are compatible only with --am */
-		strbuf_reset(&buf);
-		strbuf_addbuf(&buf, &options.git_am_opt);
-		strbuf_addch(&buf, ' ');
-		while ((p = strstr(buf.buf, " -q ")))
-			strbuf_splice(&buf, p - buf.buf, 4, " ", 1);
-		strbuf_trim(&buf);
+		for (i = options.git_am_opts.argc - 1; i >= 0; i--)
+			if (strcmp(options.git_am_opts.argv[i], "-q"))
+				break;
 
-		if (is_interactive(&options) && buf.len)
+		if (is_interactive(&options) && i >= 0)
 			die(_("error: cannot combine interactive options "
 			      "(--interactive, --exec, --rebase-merges, "
 			      "--preserve-merges, --keep-empty, --root + "
 			      "--onto) with am options (%s)"), buf.buf);
-		if (options.type == REBASE_MERGE && buf.len)
+		if (options.type == REBASE_MERGE && i >= 0)
 			die(_("error: cannot combine merge options (--merge, "
 			      "--strategy, --strategy-option) with am options "
 			      "(%s)"), buf.buf);

@@ -1207,7 +1179,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		if (options.type == REBASE_PRESERVE_MERGES)
 			die("cannot combine '--signoff' with "
 			    "'--preserve-merges'");
-		strbuf_addstr(&options.git_am_opt, " --signoff");
+		argv_array_push(&options.git_am_opts, "--signoff");
 		options.flags |= REBASE_FORCE;
 	}
 

gitgitgadget

"Johannes Schindelin via GitGitGadget" <[email protected]>

The resulting code does show what is going on more clearly. I wonder if we can do the same for -S as well? It needs to be passed to other backends, so I guess it is not such a good idea.

Hi Johannes

Thanks for looking at this. Unfortunately using OPT_PASSTHRU_ARGV seems to break the error reporting

Running
bin/wrappers/git rebase --onto @^^^^ @^^ -Cbad

Gives
git encountered an error while preparing the patches to replay these revisions:

67f673aa4a580b9e407b1ca505abf1f50510ec47...7c3e01a708856885e60bf4051586970e65dd326c

As a result, git cannot rebase them.

If I do

bin/wrappers/git rebase @^^ -Cbad

I get no error, it just tells me that it does not need to rebase (which is true)

Best Wishes

Phillip

Hi Phillip,

Hmm. Isn't this the same behavior as with the scripted version? Also: are we sure that we want to allow options to come *after* the <upstream> argument?

Ciao,
Dscho

Hi Johannes

git 2.19.1 gives

First, rewinding head to replay your work on top of it...
Applying: Ninth batch for 2.20
error: switch `C' expects a numerical value

So it has a clear message as to what the error is, this patch regresses that. It would be better if rebase detected the error before starting though.

Ah you're right the script does not check if the option argument is valid.

Maybe not but the scripted version does. I'm not sure if that is a good idea or not.

Best Wishes

Phillip

According to Junio's calendar we're now 2 days from 2.20-rc0. We have the js/rebase-autostash-detach-fix bug I reported sitting in "pu" still, and then this.

I see we still have rebase.useBuiltin in the code as an escape hatch, but it's undocumented.

Given that we're still finding regressions bugs in the rebase-in-C version should we be considering reverting 5541bd5b8f ("rebase: default to using the builtin rebase", 2018-08-08)?

I love the feature, but fear that the current list of known regressions serve as a canary for a larger list which we'd discover if we held off for another major release (and would re-enable rebase.useBuiltin=true in master right after 2.20 is out the door).

But maybe I'm being overly paranoid. What do those more familiar with this think?

I am not too worried,
* as rebase is a main porcelain, that is even hard to use in a script. so any failures are not deep down in some automation,
but when found exposed quickly (and hopefully reported).
* 5541bd5b8f was merged to next a month ago; internally we distribute the next branch to Googlers (on a weekly basis) and we have not had any bug reports regarding rebase. (Maybe our environment is too strict for the wide range of bugs reported)
* Johannes reported that the rebase is used in GfW
https://public-inbox.org/git/[email protected]/
https://github.com/git-for-windows/git/pull/1800
and from my cursory reading it is part of
2.19.windows, which has a large user base.

That would be fine with me as well, but I'd rather document rebase.useBuiltin instead of flip-flopping the switch around the release.

Have there been any fixes that are only in
the C version (has the shell version already bitrotted)?

Stefan

I do the same at Booking.com (although at a more ad-hoc schedule) and got the report whose fix is now sitting in "pu" noted upthread.

I fear that these sorts of corporate environments, both Google's and Booking's, end up testing a relatively narrow featureset. Most people have similar enough workflows, e.g. just using "git pull --rebase", I'd be surprised if we have more than 2-3 internal users who ever use the --onto option for example.

That's a good question, one which I don't think we knew the answer to before the following patches. As it turns out no, we still run the tests without failures with GIT_TEST_REBASE_USE_BUILTIN=false.

Ævar Arnfjörð Bjarmason (2):
rebase doc: document rebase.useBuiltin
tests: add a special setup where rebase.useBuiltin is off

 Documentation/config/rebase.txt | 14 ++++++++++++++
 builtin/rebase.c                |  5 ++++-
 t/README                        |  3 +++
 3 files changed, 21 insertions(+), 1 deletion(-)

2.19.1.1182.g4ecb1133ce

Hi Ævar,

I pay close attention to git rebase bug reports and patches (obviously), and there have not been any changes going into the built-in rebase/rebase -i that necessitated changes also in the scripted version.

Ciao,
Dscho

The rebase.useBuiltin variable introduced in 55071ea248 ("rebase:
start implementing it as a builtin", 2018-08-07) was turned on by default in 5541bd5b8f ("rebase: default to using the builtin rebase", 2018-08-08), but had no documentation.

Let's document it so that users who run into any stability issues with the C rewrite know there's an escape hatch[1], and make it clear that needing to turn off builtin rebase means you've found a bug in git.

1. https://public-inbox.org/git/[email protected]/

---
 Documentation/config/rebase.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

index 42e1ba7575..f079bf6b7e 100644
@@ -1,3 +1,17 @@
+rebase.useBuiltin::
+	Set to `false` to use the legacy shellscript implementation of
+	linkgit:git-rebase[1]. Is `true` by default, which means use
+	the built-in rewrite of it in C.
++
+The C rewrite is first included with Git version 2.20. This option
+serves an an escape hatch to re-enable the legacy version in case any
+bugs are found in the rewrite. This option and the shellscript version
+of linkgit:git-rebase[1] will be removed in some future release.
++
+If you find some reason to set this option to `false` other than
+one-off testing you should report the behavior difference as a bug in
+git.
+
 rebase.stat::
 	Whether to show a diffstat of what changed upstream since the last
 	rebase. False by default.

2.19.1.1182.g4ecb1133ce

Add a GIT_TEST_REBASE_USE_BUILTIN=false test mode which is equivalent to running with rebase.useBuiltin=false. This is needed to spot that we're not introducing any regressions in the legacy rebase version while we're carrying both it and the new builtin version.

---
 builtin/rebase.c | 5 ++++-
 t/README         | 3 +++
 2 files changed, 7 insertions(+), 1 deletion(-)

index 0ee06aa363..68ad8c1149 100644
@@ -48,7 +48,10 @@ static int use_builtin_rebase(void)
 {
 	struct child_process cp = CHILD_PROCESS_INIT;
 	struct strbuf out = STRBUF_INIT;
-	int ret;
+	int ret, env = git_env_bool("GIT_TEST_REBASE_USE_BUILTIN", -1);
+
+	if (env != -1)
+		return env;
 
 	argv_array_pushl(&cp.args,
 			 "config", "--bool", "rebase.usebuiltin", NULL);

index 242497455f..c719e08414 100644
@@ -348,6 +348,9 @@ GIT_TEST_MULTI_PACK_INDEX=<boolean>, when true, forces the multi-pack-
 index to be written after every 'git repack' command, and overrides the
 'core.multiPackIndex' setting to true.
 
+GIT_TEST_REBASE_USE_BUILTIN<boolean>, when false, disables the builtin
+version of git-rebase. See 'rebase.useBuiltin' in git-config(1).
+
 Naming Tests
 ------------
 

2.19.1.1182.g4ecb1133ce

That date feels slightly misleading; it has a commit date of Oct 11, and only merged to master less than two weeks ago. Given how big the two different rebase in C series were, I'd expect a couple small things to fall out after it hit master, which is what appears to be happening.

I probably don't qualify as more familar, but giving my $0.02 anyway... I'm happy that setting rebase.useBuiltin=false by default exists an escape hatch if things don't settle down as we get closer to the release, but I'd rather wait until further into the RC cycle before going that route, personally.

I was hoping that having it early in GfW 2.19 would have smoked out all the remaining issues, but it seems that those building from the source and testing have usage patterns different enough to find more issues. This is a normal part of the development process, and hopefully the remaining bugs are minor and can be flushed out in the -rc testing period---this kind of thing is the whole reason why we code-freeze and test rcs.

It unfortunately is too late to depend on the rebase.useBuiltin as an escape hatch, as 'master', 'next', or anywhere else had the "rebase and rebase -i in C" series merged without it set to false and used in any seriousness. Quite honestly, I think we can trust the rest of the "rebase and rebase -i in C" series much more than its "use the scripted one even though we have built-in one" part.

So if we have to seriously consider holding back, we may be better off ripping the whole thing out. I do not offhand know how involved such a reversion would be, though, and I am *NOT* looking forward to having do such a major surgery right before the release.

Hi Phillip,

I agree that that would make most sense: why start something when you can know that it will fail later...

Let me try to add that test case that Junio wanted, and some early error handling.

That behavior was never documented, though, was it?

Ciao,
Dscho

I was unaware of it, too. Looking at the OPT_PASSTHRU and its ARGV counterpart, I think the original intent was that you'd pass through normal last-one-wins individual options with OPT_PASSTHRU, and then list-like options with OPT_PASSTHRU_ARGV. But here you've used the latter to pass sets of individual last-one-wins options.

That said, I think what you've done here is way simpler and more readable than using a bunch of OPT_PASSTHRUs would have been. And even if it was not the original intent of the ARGV variant, I can't see any reason to avoid doing it this way.

-Peff

Hi Peff,

Thanks, that makes me feel better ;-)

Thank you, that makes me feel *even* better ;-)

Together with the extra-early error checking for incorrect -C and --whitespace options, I think we're in a much better place now.

Ciao,
Dscho

Phillip Wood reported a problem where the built-in rebase did not understand options like -C1, i.e. it did not expect the option argument.

While investigating how to address this best, I stumbled upon OPT_PASSTHRU_ARGV (which I was so far happily unaware of).

Instead of just fixing the -C<n> bug, I decided to simply convert all of the options intended for git am (or, eventually, for git apply). This happens to fix that bug, and does so much more: it simplifies the entire logic (and removes more lines than it adds).

Change since v1:

* Introduce early parameter validation for the options passed through to git am.

Johannes Schindelin (2):
rebase: really just passthru the git am options
rebase: validate -C<n> and --whitespace=<mode> parameters early

 builtin/rebase.c          | 108 ++++++++++++++++----------------------
 t/t3406-rebase-message.sh |   7 +++
 2 files changed, 52 insertions(+), 63 deletions(-)

base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-76%2Fdscho%2Frebase-Cn-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-76/dscho/rebase-Cn-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/76

Range-diff vs v1:

 1:  dc36a45068 = 1:  dc36a45068 rebase: really just passthru the `git am` options
 -:  ---------- > 2:  4c2ba52766 rebase: validate -C<n> and --whitespace=<mode> parameters early

gitgitgadget

From: Johannes Schindelin <[email protected]>

Currently, we parse the options intended for git am as if we wanted to handle them in git rebase, and then reconstruct them painstakingly to define the git_am_opt variable.

However, there is a much better way (that I was unaware of, at the time when I mentored Pratik to implement these options): OPT_PASSTHRU_ARGV. It is intended for exactly this use case, where command-line options want to be parsed into a separate argv_array.

Let's use this feature.

Incidentally, this also allows us to address a bug discovered by Phillip Wood, where the built-in rebase failed to understand that the -C option takes an optional argument.

---
 builtin/rebase.c | 98 +++++++++++++++++-------------------------------
 1 file changed, 35 insertions(+), 63 deletions(-)

index 0ee06aa363..96ffa80b71 100644
@@ -87,7 +87,7 @@ struct rebase_options {
 		REBASE_FORCE = 1<<3,
 		REBASE_INTERACTIVE_EXPLICIT = 1<<4,
 	} flags;
-	struct strbuf git_am_opt;
+	struct argv_array git_am_opts;
 	const char *action;
 	int signoff;
 	int allow_rerere_autoupdate;

@@ -339,7 +339,7 @@ N_("Resolve all conflicts manually, mark them as resolved with\n"
 static int run_specific_rebase(struct rebase_options *opts)
 {
 	const char *argv[] = { NULL, NULL };
-	struct strbuf script_snippet = STRBUF_INIT;
+	struct strbuf script_snippet = STRBUF_INIT, buf = STRBUF_INIT;
 	int status;
 	const char *backend, *backend_func;
 

@@ -433,7 +433,9 @@ static int run_specific_rebase(struct rebase_options *opts)
 		oid_to_hex(&opts->restrict_revision->object.oid) : NULL);
 	add_var(&script_snippet, "GIT_QUIET",
 		opts->flags & REBASE_NO_QUIET ? "" : "t");
-	add_var(&script_snippet, "git_am_opt", opts->git_am_opt.buf);
+	sq_quote_argv_pretty(&buf, opts->git_am_opts.argv);
+	add_var(&script_snippet, "git_am_opt", buf.buf);
+	strbuf_release(&buf);
 	add_var(&script_snippet, "verbose",
 		opts->flags & REBASE_VERBOSE ? "t" : "");
 	add_var(&script_snippet, "diffstat",

@@ -756,7 +758,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	struct rebase_options options = {
 		.type = REBASE_UNSPECIFIED,
 		.flags = REBASE_NO_QUIET,
-		.git_am_opt = STRBUF_INIT,
+		.git_am_opts = ARGV_ARRAY_INIT,
 		.allow_rerere_autoupdate  = -1,
 		.allow_empty_message = 1,
 		.git_format_patch_opt = STRBUF_INIT,

@@ -777,12 +779,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		ACTION_EDIT_TODO,
 		ACTION_SHOW_CURRENT_PATCH,
 	} action = NO_ACTION;
-	int committer_date_is_author_date = 0;
-	int ignore_date = 0;
-	int ignore_whitespace = 0;
 	const char *gpg_sign = NULL;
-	int opt_c = -1;
-	struct string_list whitespace = STRING_LIST_INIT_NODUP;
 	struct string_list exec = STRING_LIST_INIT_NODUP;
 	const char *rebase_merges = NULL;
 	int fork_point = -1;

@@ -804,15 +801,20 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		{OPTION_NEGBIT, 'n', "no-stat", &options.flags, NULL,
 			N_("do not show diffstat of what changed upstream"),
 			PARSE_OPT_NOARG, NULL, REBASE_DIFFSTAT },
-		OPT_BOOL(0, "ignore-whitespace", &ignore_whitespace,
-			 N_("passed to 'git apply'")),
 		OPT_BOOL(0, "signoff", &options.signoff,
 			 N_("add a Signed-off-by: line to each commit")),
-		OPT_BOOL(0, "committer-date-is-author-date",
-			 &committer_date_is_author_date,
-			 N_("passed to 'git am'")),
-		OPT_BOOL(0, "ignore-date", &ignore_date,
-			 N_("passed to 'git am'")),
+		OPT_PASSTHRU_ARGV(0, "ignore-whitespace", &options.git_am_opts,
+				  NULL, N_("passed to 'git am'"),
+				  PARSE_OPT_NOARG),
+		OPT_PASSTHRU_ARGV(0, "committer-date-is-author-date",
+				  &options.git_am_opts, NULL,
+				  N_("passed to 'git am'"), PARSE_OPT_NOARG),
+		OPT_PASSTHRU_ARGV(0, "ignore-date", &options.git_am_opts, NULL,
+				  N_("passed to 'git am'"), PARSE_OPT_NOARG),
+		OPT_PASSTHRU_ARGV('C', NULL, &options.git_am_opts, N_("n"),
+				  N_("passed to 'git apply'"), 0),
+		OPT_PASSTHRU_ARGV(0, "whitespace", &options.git_am_opts,
+				  N_("action"), N_("passed to 'git apply'"), 0),
 		OPT_BIT('f', "force-rebase", &options.flags,
 			N_("cherry-pick all commits, even if unchanged"),
 			REBASE_FORCE),

@@ -856,10 +858,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		{ OPTION_STRING, 'S', "gpg-sign", &gpg_sign, N_("key-id"),
 			N_("GPG-sign commits"),
 			PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
-		OPT_STRING_LIST(0, "whitespace", &whitespace,
-				N_("whitespace"), N_("passed to 'git apply'")),
-		OPT_SET_INT('C', NULL, &opt_c, N_("passed to 'git apply'"),
-			    REBASE_AM),
 		OPT_BOOL(0, "autostash", &options.autostash,
 			 N_("automatically stash/stash pop before and after")),
 		OPT_STRING_LIST('x', "exec", &exec, N_("exec"),

@@ -884,6 +882,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 			 N_("rebase all reachable commits up to the root(s)")),
 		OPT_END(),
 	};
+	int i;
 
 	/*
 	 * NEEDSWORK: Once the builtin rebase has been tested enough

@@ -1064,22 +1063,17 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		    state_dir_base, cmd_live_rebase, buf.buf);
 	}
 
-	if (!(options.flags & REBASE_NO_QUIET))
-		strbuf_addstr(&options.git_am_opt, " -q");
-
-	if (committer_date_is_author_date) {
-		strbuf_addstr(&options.git_am_opt,
-			      " --committer-date-is-author-date");
-		options.flags |= REBASE_FORCE;
+	for (i = 0; i < options.git_am_opts.argc; i++) {
+		const char *option = options.git_am_opts.argv[i];
+		if (!strcmp(option, "--committer-date-is-author-date") ||
+		    !strcmp(option, "--ignore-date") ||
+		    !strcmp(option, "--whitespace=fix") ||
+		    !strcmp(option, "--whitespace=strip"))
+			options.flags |= REBASE_FORCE;
 	}
 
-	if (ignore_whitespace)
-		strbuf_addstr(&options.git_am_opt, " --ignore-whitespace");
-
-	if (ignore_date) {
-		strbuf_addstr(&options.git_am_opt, " --ignore-date");
-		options.flags |= REBASE_FORCE;
-	}
+	if (!(options.flags & REBASE_NO_QUIET))
+		argv_array_push(&options.git_am_opts, "-q");
 
 	if (options.keep_empty)
 		imply_interactive(&options, "--keep-empty");

@@ -1089,23 +1083,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		options.gpg_sign_opt = xstrfmt("-S%s", gpg_sign);
 	}
 
-	if (opt_c >= 0)
-		strbuf_addf(&options.git_am_opt, " -C%d", opt_c);
-
-	if (whitespace.nr) {
-		int i;
-
-		for (i = 0; i < whitespace.nr; i++) {
-			const char *item = whitespace.items[i].string;
-
-			strbuf_addf(&options.git_am_opt, " --whitespace=%s",
-				    item);
-
-			if ((!strcmp(item, "fix")) || (!strcmp(item, "strip")))
-				options.flags |= REBASE_FORCE;
-		}
-	}
-
 	if (exec.nr) {
 		int i;
 

@@ -1181,23 +1158,18 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		break;
 	}
 
-	if (options.git_am_opt.len) {
-		const char *p;
-
+	if (options.git_am_opts.argc) {
 		/* all am options except -q are compatible only with --am */
-		strbuf_reset(&buf);
-		strbuf_addbuf(&buf, &options.git_am_opt);
-		strbuf_addch(&buf, ' ');
-		while ((p = strstr(buf.buf, " -q ")))
-			strbuf_splice(&buf, p - buf.buf, 4, " ", 1);
-		strbuf_trim(&buf);
+		for (i = options.git_am_opts.argc - 1; i >= 0; i--)
+			if (strcmp(options.git_am_opts.argv[i], "-q"))
+				break;
 
-		if (is_interactive(&options) && buf.len)
+		if (is_interactive(&options) && i >= 0)
 			die(_("error: cannot combine interactive options "
 			      "(--interactive, --exec, --rebase-merges, "
 			      "--preserve-merges, --keep-empty, --root + "
 			      "--onto) with am options (%s)"), buf.buf);
-		if (options.type == REBASE_MERGE && buf.len)
+		if (options.type == REBASE_MERGE && i >= 0)
 			die(_("error: cannot combine merge options (--merge, "
 			      "--strategy, --strategy-option) with am options "
 			      "(%s)"), buf.buf);

@@ -1207,7 +1179,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		if (options.type == REBASE_PRESERVE_MERGES)
 			die("cannot combine '--signoff' with "
 			    "'--preserve-merges'");
-		strbuf_addstr(&options.git_am_opt, " --signoff");
+		argv_array_push(&options.git_am_opts, "--signoff");
 		options.flags |= REBASE_FORCE;
 	}
 

gitgitgadget

From: Johannes Schindelin <[email protected]>

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.

---
 builtin/rebase.c          | 12 +++++++++++-
 t/t3406-rebase-message.sh |  7 +++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

index 96ffa80b71..571cf899d5 100644
@@ -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))

index 0392e36d23..2c79eed4fe 100755
@@ -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
+'
+
 test_done

gitgitgadget

Hi Johannes

Thanks for doing this, I think this patch is good. I've not checked the first patch as I think it is the same as before judging from the covering letter.

Best Wishes

Phillip

Hi Phillip,

Thanks!

Indeed, that's what the range-diff said. Sorry for not stating this explicitly.

Thank you for your review,
Dscho

Hi Ævar,

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

Mark a test added in 04519d7201 ("rebase: validate -C<n> and --whitespace=<mode> parameters early", 2018-11-14) as only succeeding with the builtin version of rebase. It would be nice if the shellscript version had the same fix, but it's on its way out, and the author is not interested in fixing it[1].

This makes the entire test suite pass again with the GIT_TEST_REBASE_USE_BUILTIN=false mode added in my 62c23938fa ("tests:
add a special setup where rebase.useBuiltin is off", 2018-11-14).

1. https://public-inbox.org/git/[email protected]/

---

It's better to just mark the test as needing the prereq turned off. E.g. this is what we do for the split index tests & now for the gettext tests. That way we always run the test, but just indicate that it relies on GIT_TEST_REBASE_USE_BUILTIN being unset.

 t/t3406-rebase-message.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 38bd876cab..77e5bbb3d5 100755
@@ -84,7 +84,8 @@ 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_expect_success 'builtin rebase: error out early upon -C<n> or --whitespace=<bad>' '
+	sane_unset GIT_TEST_REBASE_USE_BUILTIN &&
 	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 &&

2.20.0.rc0.387.gc7a69e6b6c

Hi Ævar,

It's not that I am not interested in fixing it. It's more like I hoped that I could work on Git for Windows v2.20.0-rc0 and rely on you to fix this bug.

Now that Git for Windows v2.20.0-rc0 is out (see
https://github.com/git-for-windows/git/releases/tag/v2.20.0-rc0.windows.1),
expect a patch soon (see https://github.com/gitgitgadget/git/pull/86).

Ciao,
Dscho

Here is a test coverage report for the uncovered lines introduced in v2.20.0-rc0 compared to v2.19.1.

Thanks,

-Stolee

[1] https://dev.azure.com/git/git/_build/results?buildId=263&view=logs

---

apply.c
eccb5a5f3d 4071) return get_oid_hex(p->old_oid_prefix, oid); 517fe807d6 4776) BUG_ON_OPT_NEG(unset);
735ca208c5 4830) return -1;

blame.c
a470beea39  113)  !strcmp(r->index->cache[-1 - pos]->name, path)) a470beea39  272) int pos = index_name_pos(r->index, path, len); a470beea39  274) mode = r->index->cache[pos]->ce_mode;

builtin/add.c
d1664e73ad builtin/add.c 458) die(_("index file corrupt"));

builtin/am.c
2abf350385 1362) repo_init_revisions(the_repository, &rev_info, NULL); fce5664805 2117) *opt_value = PATCH_FORMAT_UNKNOWN;

builtin/blame.c
517fe807d6 builtin/blame.c    759) BUG_ON_OPT_NEG(unset);

builtin/cat-file.c
98f425b453 builtin/cat-file.c  56) die("unable to stream %s to stdout", oid_to_hex(oid)); 0eb8d3767c builtin/cat-file.c 609) return error(_("only one batch option may be specified"));

builtin/checkout.c
fa655d8411 builtin/checkout.c  539) return 0;
fa655d8411 builtin/checkout.c  953) return error(_("index file corrupt"));

builtin/difftool.c
4a7e27e957 441) if (oideq(&loid, &roid))

builtin/fast-export.c
4a7e27e957 builtin/fast-export.c  387) if (oideq(&ospec->oid, &spec->oid) &&

builtin/fetch.c
builtin/fsck.c
b29759d89a builtin/fsck.c 613) fprintf(stderr, "Checking %s link\n", head_ref_name); b29759d89a builtin/fsck.c 618) return error("Invalid %s", head_ref_name); 454ea2e4d7 builtin/fsck.c 769) for (p = get_all_packs(the_repository); p; 66ec0390e7 builtin/fsck.c 888) midx_argv[2] = "--object-dir"; 66ec0390e7 builtin/fsck.c 889) midx_argv[3] = alt->path;
66ec0390e7 builtin/fsck.c 890) if (run_command(&midx_verify)) 66ec0390e7 builtin/fsck.c 891) errors_found |= ERROR_COMMIT_GRAPH;

builtin/gc.c
3029970275 builtin/gc.c 461) ret = error_errno(_("cannot stat '%s'"), gc_log_path); 3029970275 builtin/gc.c 470) ret = error_errno(_("cannot read '%s'"), gc_log_path); fec2ed2187 builtin/gc.c 495) die(FAILED_RUN, pack_refs_cmd.argv[0]); fec2ed2187 builtin/gc.c 498) die(FAILED_RUN, reflog.argv[0]); 3029970275 builtin/gc.c 585) exit(128);
fec2ed2187 builtin/gc.c 637) die(FAILED_RUN, repack.argv[0]); fec2ed2187 builtin/gc.c 647) die(FAILED_RUN, prune.argv[0]); fec2ed2187 builtin/gc.c 654) die(FAILED_RUN, prune_worktrees.argv[0]); fec2ed2187 builtin/gc.c 658) die(FAILED_RUN, rerere.argv[0]);

builtin/grep.c
76e9bdc437 builtin/grep.c  424) grep_read_unlock();
fd6263fb73 builtin/grep.c 1051) warning(_("invalid option combination, ignoring --threads")); fd6263fb73 builtin/grep.c 1057) die(_("invalid number of threads specified (%d)"), num_threads);

builtin/help.c
e6e76baaf4 builtin/help.c 429) if (!exclude_guides || alias[0] == '!') { e6e76baaf4 builtin/help.c 430) printf_ln(_("'%s' is aliased to '%s'"), cmd, alias); e6e76baaf4 builtin/help.c 431) free(alias);
e6e76baaf4 builtin/help.c 432) exit(0);
e6e76baaf4 builtin/help.c 441) fprintf_ln(stderr, _("'%s' is aliased to '%s'"), cmd, alias); e6e76baaf4 builtin/help.c 442) count = split_cmdline(alias, &argv); e6e76baaf4 builtin/help.c 443) if (count < 0)
e6e76baaf4 builtin/help.c 444) die(_("bad alias.%s string: %s"), cmd, e6e76baaf4 builtin/help.c 446) free(argv);
e6e76baaf4 builtin/help.c 448) return alias;

builtin/log.c
517fe807d6 builtin/log.c 1196) BUG_ON_OPT_NEG(unset);
2e6fd71a52 builtin/log.c 1472) die(_("failed to infer range-diff ranges")); ee6cbf712e builtin/log.c 1818) die(_("--interdiff requires --cover-letter or single patch")); 8631bf1cdd builtin/log.c 1828) else if (!rdiff_prev)
8631bf1cdd builtin/log.c 1829) die(_("--creation-factor requires --range-diff")); 40ce41604d builtin/log.c 1833) die(_("--range-diff requires --cover-letter or single patch"));

builtin/multi-pack-index.c
6d68e6a461 35) usage_with_options(builtin_multi_pack_index_usage, 6d68e6a461 39) die(_("too many arguments"));
6d68e6a461 48) die(_("unrecognized verb: %s"), argv[0]);

builtin/pack-objects.c
6a22d52126 builtin/pack-objects.c 1091) continue;
2fa233a554 builtin/pack-objects.c 1512) hashcpy(base_oid.hash, base_sha1); 2fa233a554 builtin/pack-objects.c 1513) if (!in_same_island(&delta->idx.oid, &base_oid)) 2fa233a554 builtin/pack-objects.c 1514) return 0;
28b8a73080 builtin/pack-objects.c 2793) depth++;
108f530385 builtin/pack-objects.c 2797) oe_set_tree_depth(&to_pack, ent, depth); 454ea2e4d7 builtin/pack-objects.c 2981) p = get_all_packs(the_repository);

builtin/pack-redundant.c
454ea2e4d7 builtin/pack-redundant.c 580) struct packed_git *p = get_all_packs(the_repository); 454ea2e4d7 builtin/pack-redundant.c 595) struct packed_git *p = get_all_packs(the_repository);

builtin/pull.c
01a31f3bca 565) die(_("unable to access commit %s"),

builtin/rebase--interactive.c
53bbcfbde7 builtin/rebase--interactive2.c  24) return error(_("no HEAD?"));
53bbcfbde7 builtin/rebase--interactive2.c  51) return error_errno(_("could not create temporary %s"), path_state_dir()); 53bbcfbde7 builtin/rebase--interactive2.c  57) return error_errno(_("could not mark as interactive")); 53bbcfbde7 builtin/rebase--interactive2.c  77) return -1;
53bbcfbde7 builtin/rebase--interactive2.c  81) return -1;
53bbcfbde7 builtin/rebase--interactive2.c  87) free(revisions);
53bbcfbde7 builtin/rebase--interactive2.c  88) free(shortrevisions);
53bbcfbde7 builtin/rebase--interactive2.c  90) return -1;
53bbcfbde7 builtin/rebase--interactive2.c  98) free(revisions);
53bbcfbde7 builtin/rebase--interactive2.c  99) free(shortrevisions);
53bbcfbde7 builtin/rebase--interactive2.c 101) return error_errno(_("could not open %s"), rebase_path_todo()); 53bbcfbde7 builtin/rebase--interactive2.c 106) argv_array_push(&make_script_args, restrict_revision); 53bbcfbde7 builtin/rebase--interactive2.c 114) error(_("could not generate todo list"));
53bbcfbde7 builtin/rebase--interactive2.c 206) usage_with_options(builtin_rebase_interactive_usage, options); 53bbcfbde7 builtin/rebase--interactive2.c 220) warning(_("--[no-]rebase-cousins has no effect without " 0af129b2ed builtin/rebase--interactive2.c 226) die(_("a base commit must be provided with --upstream or --onto")); 34b47315d9 builtin/rebase--interactive.c  261) ret = rearrange_squash();
34b47315d9 builtin/rebase--interactive.c  262) break;
34b47315d9 builtin/rebase--interactive.c  264) ret = sequencer_add_exec_commands(cmd);
34b47315d9 builtin/rebase--interactive.c  265) break;

builtin/rebase.c
62c23938fa   55) return env;
55071ea248   65) strbuf_trim(&out);
55071ea248   66) ret = !strcmp("true", out.buf);
55071ea248   67) strbuf_release(&out);
002ee2fe68  119) die(_("%s requires an interactive rebase"), option); f95736288a  152) return error_errno(_("could not read '%s'"), path); f95736288a  166) return -1;
f95736288a  171) return error(_("could not get 'onto': '%s'"), buf.buf); f95736288a  182) return -1;
f95736288a  183) } else if (read_one(state_dir_path("head", opts), &buf)) f95736288a  184) return -1;
f95736288a  186) return error(_("invalid orig-head: '%s'"), buf.buf); f95736288a  190) return -1;
f95736288a  192) opts->flags &= ~REBASE_NO_QUIET;
73d51ed0a5  200) opts->signoff = 1;
73d51ed0a5  201) opts->flags |= REBASE_FORCE;
ead98c111b  208) return -1;
12026a412c  223) return -1;
ba1905a5fe  231) return -1;
ba1905a5fe  239) return -1;
6defce2b02  259) return error(_("Could not read '%s'"), path); 6defce2b02  277) res = error(_("Cannot store %s"), autostash.buf); 6defce2b02  281) return res;
bc24382c2b  379) argv_array_pushf(&child.args,
bc24382c2b  381) oid_to_hex(&opts->restrict_revision->object.oid)); ac7f467fef  515) struct strbuf dir = STRBUF_INIT;
6defce2b02  517) apply_autostash(opts);
ac7f467fef  518) strbuf_addstr(&dir, opts->state_dir);
ac7f467fef  519) remove_dir_recursively(&dir, 0);
ac7f467fef  520) strbuf_release(&dir);
ac7f467fef  521) die("Nothing to do");
3249c1251e  556) ret = -1;
3249c1251e  557) goto leave_reset_head;
bac2a1e36f  561) ret = error(_("could not determine HEAD revision")); bac2a1e36f  562) goto leave_reset_head;
3249c1251e  580) ret = error(_("could not read index"));
3249c1251e  581) goto leave_reset_head;
bac2a1e36f  585) ret = error(_("failed to find tree of %s"), oid_to_hex(oid)); bac2a1e36f  586) goto leave_reset_head;
3249c1251e  590) ret = error(_("failed to find tree of %s"), oid_to_hex(oid)); 3249c1251e  591) goto leave_reset_head;
ac7f467fef  603) ret = error(_("could not write index"));
3249c1251e  604) goto leave_reset_head;
ac7f467fef  621) } else if (old_orig)
ac7f467fef  622) delete_ref(NULL, "ORIG_HEAD", old_orig, 0); bff014dac7  655) opts->flags &= !REBASE_DIFFSTAT;
9a48a615b4  689) return 1;
9a48a615b4  705) return 0;
55071ea248  916) const char *path = mkpath("%s/git-legacy-rebase", 55071ea248  919) if (sane_execvp(path, (char **)argv) < 0) 55071ea248  920) die_errno(_("could not exec %s"), path);
0eabf4b95c  938) die(_("It looks like 'git am' is in progress. Cannot rebase.")); f28d40d3a9  975) usage_with_options(builtin_rebase_usage,
f95736288a  995) die(_("Cannot read HEAD"));
f95736288a  999) die(_("could not read index"));
f95736288a 1013) exit(1);
122420c295 1026) die(_("could not discard worktree changes")); 122420c295 1029) exit(1);
5e5d96197c 1040) exit(1);
5e5d96197c 1044) die(_("could not move back to %s"),
5a61494539 1055) die(_("could not remove '%s'"), options.state_dir); c54dacb50e 1074) const char *last_slash = strrchr(options.state_dir, '/'); c54dacb50e 1075) const char *state_dir_base =
c54dacb50e 1076) last_slash ? last_slash + 1 : options.state_dir; c54dacb50e 1077) const char *cmd_live_rebase =
c54dacb50e 1079) strbuf_reset(&buf);
c54dacb50e 1080) strbuf_addf(&buf, "rm -fr \"%s\"", options.state_dir); c54dacb50e 1081) die(_("It seems that there is already a %s directory, and\n" 3c3588c7d3 1138) else if (strcmp("no-rebase-cousins", rebase_merges)) 3c3588c7d3 1139) die(_("Unknown mode: %s"), rebase_merges); ba1905a5fe 1161) die(_("--strategy requires --merge or --interactive")); cda614e489 1179) strbuf_addstr(&options.git_format_patch_opt, " --progress"); ac7f467fef 1188) options.state_dir = apply_dir();
ac7f467fef 1189) break;
ac7f467fef 1261) die(_("invalid upstream '%s'"), options.upstream_name); 9dba809a69 1267) die(_("Could not create new root commit")); e65123a71d 1317) die(_("fatal: no such branch/commit '%s'"), ac7f467fef 1325) die(_("No such ref: %s"), "HEAD");
ac7f467fef 1337) die(_("Could not resolve HEAD to a revision")); e0333e5c63 1350) die(_("could not read index"));
6defce2b02 1377) die(_("Cannot autostash"));
6defce2b02 1380) die(_("Unexpected stash response: '%s'"), 6defce2b02 1386) die(_("Could not create directory for '%s'"), 6defce2b02 1392) die(_("could not reset --hard"));
e65123a71d 1436) ret = !!error(_("could not parse '%s'"),
e65123a71d 1438) goto cleanup;
e65123a71d 1447) ret = !!error(_("could not switch to "
1ed9c14ff2 1457)  resolve_ref_unsafe("HEAD", 0, NULL, &flag)) 1ed9c14ff2 1458) puts(_("HEAD is up to date."));
9a48a615b4 1467)  resolve_ref_unsafe("HEAD", 0, NULL, &flag)) 9a48a615b4 1468) puts(_("HEAD is up to date, rebase forced."));

builtin/reflog.c
c9ef0d95eb builtin/reflog.c 580) all_worktrees = 0; c9ef0d95eb builtin/reflog.c 616) continue;

builtin/remote.c
5025425dff builtin/remote.c  864) return error(_("No such remote: '%s'"), name);

builtin/repack.c
16d75fa48d  48) use_delta_islands = git_config_bool(var, value);
16d75fa48d  49) return 0;
2f0c9e9a9b 239) die("repack: Expecting full hex object ID lines only from pack-objects."); 2f0c9e9a9b 411) die("repack: Expecting full hex object ID lines only from pack-objects.");

builtin/rerere.c
2373b65059 builtin/rerere.c  79) warning(_("'git rerere forget' without paths is deprecated")); 2373b65059 builtin/rerere.c 111) die(_("unable to generate diff for '%s'"), rerere_path(id, NULL));

builtin/rev-list.c
7c0fe330d5 builtin/rev-list.c 227) die("unexpected missing %s object '%s'", 7c0fe330d5 builtin/rev-list.c 228)     type_name(obj->type), oid_to_hex(&obj->oid));

builtin/show-branch.c
9001dc2a74 builtin/show-branch.c 430) if (get_oid(refname + ofs, &tmp) || !oideq(&tmp, oid)) 517fe807d6 builtin/show-branch.c 607) BUG_ON_OPT_NEG(unset);

builtin/show-ref.c
517fe807d6 builtin/show-ref.c 154) BUG_ON_OPT_NEG(unset);

builtin/submodule--helper.c
ee69b2a90c 1469) die(_("Invalid update mode '%s' for submodule path '%s'"), ee69b2a90c 1473) die(_("Invalid update mode '%s' configured for submodule path '%s'"), ee69b2a90c 1476) out->type = sub->update_strategy.type;
ee69b2a90c 1477) out->command = sub->update_strategy.command;
ee69b2a90c 1497) die("submodule--helper update-module-clone expects <just-cloned> <path> [<update>]"); e0a862fdaf 1648) url = sub->url;
74d4731da1 2057) die(_("could not get a repository handle for submodule '%s'"), path);

builtin/unpack-objects.c
4a7e27e957 builtin/unpack-objects.c 306) if (oideq(&info->base_oid, &obj_list[nr].oid) ||

builtin/update-index.c
4a7e27e957 builtin/update-index.c  672) if (oideq(&ce_2->oid, &ce_3->oid) &&

builtin/worktree.c
e5353bef55  60) error_errno(_("failed to delete '%s'"), sb.buf);
e19831c94f 251)     die(_("unable to re-add worktree '%s'"), path);
68a6b3a1bd 793) die(_("cannot move a locked working tree, lock reason: %s\nuse 'move -f -f' to override or unlock first"), f4143101cb 906) die(_("cannot remove a locked working tree, lock reason: %s\nuse 'remove -f -f' to override or unlock first"),

bundle.c
2c8ee1f53c 267) error_errno(_("unable to dup bundle descriptor")); 2c8ee1f53c 268) child_process_clear(&pack_objects); 2c8ee1f53c 269) return -1;
2c8ee1f53c 478) rollback_lock_file(&lock);

cache-tree.c
combine-diff.c
0074c9110d  377) state->sline[state->nb-1].p_lno = 0074c9110d  378) xcalloc(state->num_parent, sizeof(unsigned long));

commit-graph.c
20fd6d5799   79) return 0;
6cc017431c  275) return 0;

commit-reach.c
5227c38566 134) return ret;
5227c38566 282) return 1;
5227c38566 314) return ret;
5227c38566 317) return ret;
1d614d41e5 395) return 0;
1d614d41e5 401) return 0;
1d614d41e5 405) return 0;
4fbcca4eff 538) return 1;
b67f6b26e3 559) continue;
b67f6b26e3 570) from->objects[i].item->flags |= assign_flag; b67f6b26e3 571) continue;
b67f6b26e3 577) result = 0;
b67f6b26e3 578) goto cleanup;

config.c
c780b9cfe8 2303) return val;
c780b9cfe8 2306) if (is_bool)
c780b9cfe8 2307) return val ? 0 : 1;
c780b9cfe8 2309) return val;

date.c
c27cc94fad  904) tm->tm_mon = number-1; c27cc94fad  908) else if (number > 69 && number < 100) c27cc94fad  909) tm->tm_year = number; c27cc94fad  910) else if (number < 38) c27cc94fad  911) tm->tm_year = 100 + number; c27cc94fad  952) pending_number(tm, num);

delta-islands.c
c8d521faf7  53) memcpy(b, old, size);
c8d521faf7  73) return 1;
c8d521faf7 118) return 0;
c8d521faf7 130) return 0;
c8d521faf7 187) b->refcount--;
c8d521faf7 188) b = kh_value(island_marks, pos) = island_bitmap_new(b); c8d521faf7 202) continue;
c8d521faf7 212) obj = ((struct tag *)obj)->tagged;
c8d521faf7 213) if (obj) {
c8d521faf7 214) parse_object(the_repository, &obj->oid);
c8d521faf7 215) marks = create_or_get_island_marks(obj);
c8d521faf7 216) island_bitmap_set(marks, island_counter);
c8d521faf7 248) return;
c8d521faf7 268) progress_state = start_progress(_("Propagating island marks"), nr); c8d521faf7 286) die(_("bad tree object %s"), oid_to_hex(&ent->idx.oid)); c8d521faf7 293) continue;
c8d521faf7 297) continue;
c8d521faf7 321) return config_error_nonbool(k);
c8d521faf7 330) die(_("failed to load island regex for '%s': %s"), k, re.buf); c8d521faf7 386) warning(_("island regex from config has "
c8d521faf7 397) strbuf_addch(&island_name, '-');
c8d521faf7 433) continue;
c8d521faf7 436) list[dst] = list[src];

diff-lib.c
9001dc2a74 diff-lib.c 346)     (!oideq(oid, &old_entry->oid) || !oideq(&old_entry->oid, &new_entry->oid))) {

diff.c
b78ea5fc35 4130) add_external_diff_name(o->repo, &argv, other, two);

dir.c
8a2c174677  287) name = to_free = xmemdupz(name, namelen); c46c406ae1 2282) trace_performance_leave("read directory %.*s", len, path);

entry.c
b878579ae7 402) static void mark_colliding_entries(const struct checkout *state, b878579ae7 405) int i, trust_ino = check_stat;
b878579ae7 411) ce->ce_flags |= CE_MATCHED;
b878579ae7 413) for (i = 0; i < state->istate->cache_nr; i++) { b878579ae7 414) struct cache_entry *dup = state->istate->cache[i]; b878579ae7 416) if (dup == ce)
b878579ae7 417) break;
b878579ae7 419) if (dup->ce_flags & (CE_MATCHED | CE_VALID | CE_SKIP_WORKTREE)) b878579ae7 420) continue;
b878579ae7 422) if ((trust_ino && dup->ce_stat_data.sd_ino == st->st_ino) || b878579ae7 423)     (!trust_ino && !fspathcmp(ce->name, dup->name))) { b878579ae7 424) dup->ce_flags |= CE_MATCHED;
b878579ae7 425) break;
b878579ae7 428) }
b878579ae7 488) mark_colliding_entries(state, ce, &st);

fsck.c
fb8952077d  214) die_errno("Could not read '%s'", path);

git.c
a9a60b94cc 322) fprintf_ln(stderr, _("'%s' is aliased to '%s'"), c6d75bc17a 735) string_list_clear(&cmd_list, 0);

gpg-interface.c
4de9394dcb 155) break;

help.c
26c7d06783 help.c         500) static int get_alias(const char *var, const char *value, void *data) 26c7d06783 help.c         502) struct string_list *list = data;
26c7d06783 help.c         504) if (skip_prefix(var, "alias.", &var))
26c7d06783 help.c         505) string_list_append(list, var)->util = xstrdup(value); 26c7d06783 help.c         507) return 0;
26c7d06783 help.c         530) printf("\n%s\n", _("Command aliases"));
26c7d06783 help.c         531) ALLOC_ARRAY(aliases, alias_list.nr + 1);
26c7d06783 help.c         532) for (i = 0; i < alias_list.nr; i++) {
26c7d06783 help.c         533) aliases[i].name = alias_list.items[i].string; 26c7d06783 help.c         534) aliases[i].help = alias_list.items[i].util; 26c7d06783 help.c         535) aliases[i].category = 1;
26c7d06783 help.c         537) aliases[alias_list.nr].name = NULL;
26c7d06783 help.c         538) print_command_list(aliases, 1, longest);
26c7d06783 help.c         539) free(aliases);

http.c
21084e84a4  316) free(http_ssl_backend);
21084e84a4  317) http_ssl_backend = xstrdup_or_null(value);
21084e84a4  318) return 0;
93aef7c79b  322) http_schannel_check_revoke = git_config_bool(var, value); 93aef7c79b  323) return 0;
b67d40adbb  327) http_schannel_use_ssl_cainfo = git_config_bool(var, value); b67d40adbb  328) return 0;
93aef7c79b  833)     !http_schannel_check_revoke) {
93aef7c79b  835) curl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE); b67d40adbb  883)     !http_schannel_use_ssl_cainfo) {
b67d40adbb  884) curl_easy_setopt(result, CURLOPT_CAINFO, NULL);

ident.c
501afcb8b0 172) strbuf_addstr(&git_default_email, email); 501afcb8b0 173) free((char *)email);

list-objects-filter-options.c
bc5975d24f  55) if (errbuf) {
bc5975d24f  56) strbuf_addstr(
bc5975d24f  60) return 1;
cc0b05a4cc  86) if (errbuf)

list-objects-filter.c
list-objects.c
f447a499db 200) ctx->show_object(obj, base->buf, ctx->show_data);

ll-merge.c
d64324cb60 380) marker_size = DEFAULT_CONFLICT_MARKER_SIZE;

log-tree.c
4a7e27e957 477) if (oideq(&parent->item->object.oid, oid))

mailinfo.c
3aa4d81f88  992) len--;
3aa4d81f88  998) handle_filter(mi, prev);
3aa4d81f88  999) strbuf_reset(prev);
3aa4d81f88 1090) handle_filter(mi, &prev);

midx.c
4d80560c54   58) error_errno(_("failed to read %s"), midx_name); 4d80560c54   59) goto cleanup_fail;
4d80560c54   65) error(_("multi-pack-index file %s is too small"), midx_name); 4d80560c54   66) goto cleanup_fail;
0d5b3a5ef7  146) die(_("multi-pack-index missing required OID lookup chunk")); 662148c435  148) die(_("multi-pack-index missing required object offsets chunk")); 4d80560c54  173) munmap(midx_map, midx_size);
4d80560c54  175) close(fd);
1dcd9f2043  184) return;
3715a6335c  266) return 0;
fe1ed56f5e  413) warning(_("failed to open pack-index '%s'"), fe1ed56f5e  415) close_pack(packs->list[packs->nr]);
fe1ed56f5e  416) FREE_AND_NULL(packs->list[packs->nr]);
fe1ed56f5e  417) return;
a40498a126  490) return 1;
fe1ed56f5e  507) die(_("failed to locate object %d in packfile"), cur_object); fc59e74844  769) die_errno(_("unable to create leading directories of %s"), 525e18c04b  943) die(_("failed to clear multi-pack-index at %s"), midx); 56ee7ff156  969) return 0;
cc6af73c02 1010) midx_report(_("failed to load pack-index for packfile %s"), cc6af73c02 1011)     e.p->pack_name);
cc6af73c02 1012) break;

name-hash.c
2179045fd0 532) die(_("unable to create lazy_dir thread: %s"), strerror(err)); 2179045fd0 554) die(_("unable to create lazy_name thread: %s"), strerror(err)); 2179045fd0 560) die(_("unable to join lazy_name thread: %s"), strerror(err));

oidmap.c
cc00e5ce6b 11) return !oideq(&entry_->oid,

oidset.c
8b2f8cbcb1 29) kh_del_oid(&set->set, pos);
8b2f8cbcb1 30) return 1;

pack-bitmap.c
30cdc33fba 1130) return 0;

pack-objects.c
108f530385 172) REALLOC_ARRAY(pdata->tree_depth, pdata->nr_alloc); fe0ac2fb7f 175) REALLOC_ARRAY(pdata->layer, pdata->nr_alloc); 108f530385 192) pdata->tree_depth[pdata->nr_objects - 1] = 0; fe0ac2fb7f 195) pdata->layer[pdata->nr_objects - 1] = 0;

packfile.c
1127a98cce  117) return error("index file %s is too small", path); 1127a98cce  119) return error("empty data");
fe1ed56f5e  211) if (open_pack_index(p))
fe1ed56f5e  212) return 0;
fe1ed56f5e  213) level1_ofs = p->index_data;
17c35c8969  490) break;
17c35c8969  548) return 0;

preload-index.c
ae9af12287  63) struct progress_data *pd = p->progress;
ae9af12287  65) pthread_mutex_lock(&pd->mutex);
ae9af12287  66) pd->n += last_nr - nr;
ae9af12287  67) display_progress(pd->progress, pd->n);
ae9af12287  68) pthread_mutex_unlock(&pd->mutex);
ae9af12287  69) last_nr = nr;
ae9af12287  83) struct progress_data *pd = p->progress;
ae9af12287  85) pthread_mutex_lock(&pd->mutex);
ae9af12287  86) display_progress(pd->progress, pd->n + last_nr);
ae9af12287  87) pthread_mutex_unlock(&pd->mutex);
ae9af12287 118) pd.progress = start_delayed_progress(_("Refreshing index"), index->cache_nr); ae9af12287 119) pthread_mutex_init(&pd.mutex, NULL);
ae9af12287 132) p->progress = &pd;
2179045fd0 137) die(_("unable to create threaded lstat: %s"), strerror(err));

read-cache.c
ae9af12287 1490) progress = start_delayed_progress(_("Refresh index"),
ae9af12287 1491)   istate->cache_nr);
ae9af12287 1539) display_progress(progress, i);
ae9af12287 1572) display_progress(progress, istate->cache_nr);
ae9af12287 1573) stop_progress(&progress);
252d079cbd 1784) const unsigned char *cp = (const unsigned char *)name;
252d079cbd 1788) strip_len = decode_varint(&cp);
77ff1127a4 1789) if (previous_ce) {
77ff1127a4 1790) previous_len = previous_ce->ce_namelen;
77ff1127a4 1791) if (previous_len < strip_len)
252d079cbd 1792) die(_("malformed name field in the index, near path '%s'"),
77ff1127a4 1793) previous_ce->name);
77ff1127a4 1794) copy_len = previous_len - strip_len;
252d079cbd 1796) name = (const char *)cp;
252d079cbd 1802) len += copy_len;
252d079cbd 1823) if (copy_len)
252d079cbd 1824) memcpy(ce->name, previous_ce->name, copy_len);
252d079cbd 1825) memcpy(ce->name + copy_len, name, len + 1 - copy_len);
252d079cbd 1826) *ent_size = (name - ((char *)ondisk)) + len + 1 - copy_len;
abb4bb8384 1959) munmap((void *)p->mmap, p->mmap_size);
abb4bb8384 1960) die(_("index file corrupt"));
77ff1127a4 2001) mem_pool_init(&istate->ce_mem_pool,
77ff1127a4 2039) static void *load_cache_entries_thread(void *_data)
77ff1127a4 2041) struct load_cache_entries_thread_data *p = _data;
77ff1127a4 2045) for (i = p->ieot_start; i < p->ieot_start + p->ieot_blocks; i++) {
77ff1127a4 2046) p->consumed += load_cache_entry_block(p->istate, p->ce_mem_pool,
77ff1127a4 2047) p->offset, p->ieot->entries[i].nr, p->mmap, p->ieot->entries[i].offset, NULL);
77ff1127a4 2048) p->offset += p->ieot->entries[i].nr;
77ff1127a4 2050) return NULL;
77ff1127a4 2053) static unsigned long load_cache_entries_threaded(struct index_state *istate, const char *mmap, size_t mmap_size, 77ff1127a4 2058) unsigned long consumed = 0;
77ff1127a4 2061) if (istate->name_hash_initialized)
77ff1127a4 2064) mem_pool_init(&istate->ce_mem_pool, 0);
77ff1127a4 2067) if (nr_threads > ieot->nr)
77ff1127a4 2068) nr_threads = ieot->nr;
77ff1127a4 2069) data = xcalloc(nr_threads, sizeof(*data));
77ff1127a4 2071) offset = ieot_start = 0;
77ff1127a4 2072) ieot_blocks = DIV_ROUND_UP(ieot->nr, nr_threads);
77ff1127a4 2073) for (i = 0; i < nr_threads; i++) {
77ff1127a4 2074) struct load_cache_entries_thread_data *p = &data[i];
77ff1127a4 2077) if (ieot_start + ieot_blocks > ieot->nr)
77ff1127a4 2078) ieot_blocks = ieot->nr - ieot_start;
77ff1127a4 2080) p->istate = istate;
77ff1127a4 2081) p->offset = offset;
77ff1127a4 2082) p->mmap = mmap;
77ff1127a4 2083) p->ieot = ieot;
77ff1127a4 2084) p->ieot_start = ieot_start;
77ff1127a4 2085) p->ieot_blocks = ieot_blocks;
77ff1127a4 2088) nr = 0;
77ff1127a4 2089) for (j = p->ieot_start; j < p->ieot_start + p->ieot_blocks; j++)
77ff1127a4 2090) nr += p->ieot->entries[j].nr;
77ff1127a4 2091) if (istate->version == 4) {
77ff1127a4 2092) mem_pool_init(&p->ce_mem_pool,
77ff1127a4 2095) mem_pool_init(&p->ce_mem_pool,
77ff1127a4 2099) err = pthread_create(&p->pthread, NULL, load_cache_entries_thread, p);
77ff1127a4 2100) if (err)
77ff1127a4 2101) die(_("unable to create load_cache_entries thread: %s"), strerror(err));
77ff1127a4 2104) for (j = 0; j < ieot_blocks; j++)
77ff1127a4 2105) offset += ieot->entries[ieot_start + j].nr;
77ff1127a4 2106) ieot_start += ieot_blocks;
77ff1127a4 2109) for (i = 0; i < nr_threads; i++) {
77ff1127a4 2110) struct load_cache_entries_thread_data *p = &data[i];
77ff1127a4 2112) err = pthread_join(p->pthread, NULL);
77ff1127a4 2113) if (err)
77ff1127a4 2114) die(_("unable to join load_cache_entries thread: %s"), strerror(err));
77ff1127a4 2115) mem_pool_combine(istate->ce_mem_pool, p->ce_mem_pool);
77ff1127a4 2116) consumed += p->consumed;
77ff1127a4 2119) free(data);
77ff1127a4 2121) return consumed;
abb4bb8384 2193) extension_offset = read_eoie_extension(mmap, mmap_size);
abb4bb8384 2194) if (extension_offset) {
abb4bb8384 2197) p.src_offset = extension_offset;
abb4bb8384 2198) err = pthread_create(&p.pthread, NULL, load_index_extensions, &p);
abb4bb8384 2199) if (err)
abb4bb8384 2200) die(_("unable to create load_index_extensions thread: %s"), strerror(err));
abb4bb8384 2202) nr_threads--;
77ff1127a4 2211) ieot = read_ieot_extension(mmap, mmap_size, extension_offset);
77ff1127a4 2214) src_offset += load_cache_entries_threaded(istate, mmap, mmap_size, src_offset, nr_threads, ieot); 77ff1127a4 2215) free(ieot);
abb4bb8384 2225) int ret = pthread_join(p.pthread, NULL);
abb4bb8384 2226) if (ret)
abb4bb8384 2227) die(_("unable to join load_index_extensions thread: %s"), strerror(ret));
3255089ada 2769) ieot_blocks = nr_threads;
77ff1127a4 2770) if (ieot_blocks > istate->cache_nr)
77ff1127a4 2771) ieot_blocks = istate->cache_nr;
3255089ada 2779) ieot = xcalloc(1, sizeof(struct index_entry_offset_table)
3255089ada 2780) + (ieot_blocks * sizeof(struct index_entry_offset)));
77ff1127a4 2781) ieot_entries = DIV_ROUND_UP(entries, ieot_blocks);
3255089ada 2787) free(ieot);
3b1d9e045e 2788) return -1;
3255089ada 2814) ieot->entries[ieot->nr].nr = nr;
3255089ada 2815) ieot->entries[ieot->nr].offset = offset;
3255089ada 2816) ieot->nr++;
3255089ada 2822) if (previous_name)
3255089ada 2823) previous_name->buf[0] = 0;
3255089ada 2824) nr = 0;
3255089ada 2825) offset = lseek(newfd, 0, SEEK_CUR);
3255089ada 2826) if (offset < 0) {
3255089ada 2827) free(ieot);
3255089ada 2828) return -1;
3255089ada 2830) offset += write_buffer_len;
3255089ada 2840) ieot->entries[ieot->nr].nr = nr;
3255089ada 2841) ieot->entries[ieot->nr].offset = offset;
3255089ada 2842) ieot->nr++;
3255089ada 2854) free(ieot);
3b1d9e045e 2855) return -1;
3255089ada 2868) struct strbuf sb = STRBUF_INIT;
3255089ada 2870) write_ieot_extension(&sb, ieot);
3255089ada 2871) err = write_index_ext_header(&c, &eoie_c, newfd, CACHE_EXT_INDEXENTRYOFFSETTABLE, sb.len) < 0 3255089ada 2872) || ce_write(&c, newfd, sb.buf, sb.len) < 0;
3255089ada 2873) strbuf_release(&sb);
3255089ada 2874) free(ieot);
3255089ada 2875) if (err)
3255089ada 2876) return -1;
3b1d9e045e 3363) static size_t read_eoie_extension(const char *mmap, size_t mmap_size)
3b1d9e045e 3381) if (mmap_size < sizeof(struct cache_header) + EOIE_SIZE_WITH_HEADER + the_hash_algo->rawsz) 3b1d9e045e 3382) return 0;
3b1d9e045e 3385) index = eoie = mmap + mmap_size - EOIE_SIZE_WITH_HEADER - the_hash_algo->rawsz;
3b1d9e045e 3386) if (CACHE_EXT(index) != CACHE_EXT_ENDOFINDEXENTRIES)
3b1d9e045e 3387) return 0;
3b1d9e045e 3388) index += sizeof(uint32_t);
3b1d9e045e 3391) extsize = get_be32(index);
3b1d9e045e 3392) if (extsize != EOIE_SIZE)
3b1d9e045e 3393) return 0;
3b1d9e045e 3394) index += sizeof(uint32_t);
3b1d9e045e 3400) offset = get_be32(index);
3b1d9e045e 3401) if (mmap + offset < mmap + sizeof(struct cache_header))
3b1d9e045e 3402) return 0;
3b1d9e045e 3403) if (mmap + offset >= eoie)
3b1d9e045e 3404) return 0;
3b1d9e045e 3405) index += sizeof(uint32_t);
3b1d9e045e 3416) src_offset = offset;
3b1d9e045e 3417) the_hash_algo->init_fn(&c);
3b1d9e045e 3418) while (src_offset < mmap_size - the_hash_algo->rawsz - EOIE_SIZE_WITH_HEADER) {
3b1d9e045e 3426) memcpy(&extsize, mmap + src_offset + 4, 4);
3b1d9e045e 3427) extsize = ntohl(extsize);
3b1d9e045e 3430) if (src_offset + 8 + extsize < src_offset)
3b1d9e045e 3431) return 0;
3b1d9e045e 3433) the_hash_algo->update_fn(&c, mmap + src_offset, 8);
3b1d9e045e 3435) src_offset += 8;
3b1d9e045e 3436) src_offset += extsize;
3b1d9e045e 3438) the_hash_algo->final_fn(hash, &c);
3b1d9e045e 3439) if (!hasheq(hash, (const unsigned char *)index))
3b1d9e045e 3440) return 0;
3b1d9e045e 3443) if (src_offset != mmap_size - the_hash_algo->rawsz - EOIE_SIZE_WITH_HEADER)
3b1d9e045e 3444) return 0;
3b1d9e045e 3446) return offset;
3255089ada 3465) static struct index_entry_offset_table *read_ieot_extension(const char *mmap, size_t mmap_size, size_t offset) 3255089ada 3467)        const char *index = NULL;
3255089ada 3473)        if (!offset)
3255089ada 3474)        return NULL;
3255089ada 3475)        while (offset <= mmap_size - the_hash_algo->rawsz - 8) {
3255089ada 3476)        extsize = get_be32(mmap + offset + 4);
3255089ada 3477)        if (CACHE_EXT((mmap + offset)) == CACHE_EXT_INDEXENTRYOFFSETTABLE) {
3255089ada 3478)        index = mmap + offset + 4 + 4;
3255089ada 3479)        break;
3255089ada 3481)        offset += 8;
3255089ada 3482)        offset += extsize;
3255089ada 3484)        if (!index)
3255089ada 3485)        return NULL;
3255089ada 3488)        ext_version = get_be32(index);
3255089ada 3489)        if (ext_version != IEOT_VERSION) {
3255089ada 3490)        error("invalid IEOT version %d", ext_version);
3255089ada 3491)        return NULL;
3255089ada 3493)        index += sizeof(uint32_t);
3255089ada 3496)        nr = (extsize - sizeof(uint32_t)) / (sizeof(uint32_t) + sizeof(uint32_t)); 3255089ada 3497)        if (!nr) {
3255089ada 3498)        error("invalid number of IEOT entries %d", nr);
3255089ada 3499)        return NULL;
3255089ada 3501)        ieot = xmalloc(sizeof(struct index_entry_offset_table)
3255089ada 3502)        + (nr * sizeof(struct index_entry_offset)));
3255089ada 3503)        ieot->nr = nr;
3255089ada 3504)        for (i = 0; i < nr; i++) {
3255089ada 3505)        ieot->entries[i].offset = get_be32(index);
3255089ada 3506)        index += sizeof(uint32_t);
3255089ada 3507)        ieot->entries[i].nr = get_be32(index);
3255089ada 3508)        index += sizeof(uint32_t);
3255089ada 3511)        return ieot;
3255089ada 3514) static void write_ieot_extension(struct strbuf *sb, struct index_entry_offset_table *ieot) 3255089ada 3520)        put_be32(&buffer, IEOT_VERSION);
3255089ada 3521)        strbuf_add(sb, &buffer, sizeof(uint32_t));
3255089ada 3524)        for (i = 0; i < ieot->nr; i++) {
3255089ada 3527)        put_be32(&buffer, ieot->entries[i].offset);
3255089ada 3528)        strbuf_add(sb, &buffer, sizeof(uint32_t));
3255089ada 3531)        put_be32(&buffer, ieot->entries[i].nr);
3255089ada 3532)        strbuf_add(sb, &buffer, sizeof(uint32_t));
3255089ada 3534) }

rebase-interactive.c
64a43cbd5d 62) return error_errno(_("could not read '%s'."), todo_file); 64a43cbd5d 66) strbuf_release(&buf);
64a43cbd5d 67) return -1;
a9f5476fbc 75) return error_errno(_("could not read '%s'."), todo_file); a9f5476fbc 79) strbuf_release(&buf);
a9f5476fbc 80) return -1;
64a43cbd5d 86) return -1;

ref-filter.c
f0062d3b74 1039) v->s = xstrdup("");
f0062d3b74 1302) free((char *)to_free);
f0062d3b74 1303) return xstrdup("");
f0062d3b74 1340) free((char *)to_free);
f0062d3b74 1341) return xstrdup("");
f0062d3b74 1391) *s = xstrdup("=");
f0062d3b74 1393) *s = xstrdup("<");
f0062d3b74 1518) ref->symref = xstrdup("");
f0062d3b74 1587) v->s = xstrdup("");

refs.c
3a3b9d8cde  661) return 0;
4a6067cda5 1431) return 0;

refs/files-backend.c
refs/packed-backend.c
9001dc2a74 1163) } else if (!oideq(&update->old_oid, iter->oid)) {

refs/ref-cache.c
9001dc2a74 275) if (!oideq(&ref1->u.value.oid, &ref2->u.value.oid))

remote.c
85daa01f6b 1219) continue;
85daa01f6b 1225) continue;

rerere.c
2373b65059  217) die(_("corrupt MERGE_RR"));
2373b65059  226) die(_("corrupt MERGE_RR"));
2373b65059  229) die(_("corrupt MERGE_RR"));
2373b65059  264) die(_("unable to write rerere record")); 2373b65059  269) die(_("unable to write rerere record")); 4af32207bc  376) break;
4af32207bc  380) strbuf_addbuf(&two, &conflict);
c0f16f8e14  384) break;
c0f16f8e14  388) break;
c0f16f8e14  392) break;
2373b65059  480) return error_errno(_("could not open '%s'"), path); 2373b65059  485) error_errno(_("could not write '%s'"), output); 2373b65059  495) error(_("there were errors while writing '%s' (%s)"), 2373b65059  498) io.io.wrerror = error_errno(_("failed to flush '%s'"), path); 2373b65059  565) return error(_("index file corrupt")); 2373b65059  599) return error(_("index file corrupt")); 2373b65059  684) warning_errno(_("failed utime() on '%s'"), 2373b65059  690) return error_errno(_("could not open '%s'"), path); 2373b65059  692) error_errno(_("could not write '%s'"), path); 2373b65059  694) return error_errno(_("writing '%s' failed"), path); 2373b65059  720) die(_("unable to write new index file")); 2373b65059  803) die_errno(_("cannot unlink stray '%s'"), path); 2373b65059 1057) error(_("failed to update conflicted state in '%s'"), path); 2373b65059 1075) error(_("no remembered resolution for '%s'"), path); 2373b65059 1077) error_errno(_("cannot unlink '%s'"), filename); 2373b65059 1111) return error(_("index file corrupt")); 2373b65059 1199) die_errno(_("unable to open rr-cache directory"));

revision.c
2abf350385 1538) if (ce_path_match(istate, ce, &revs->prune_data, NULL)) { 2abf350385 1544) while ((i+1 < istate->cache_nr) &&
2abf350385 1545)        ce_same_name(ce, istate->cache[i+1])) b45424181e 2942) return;
b45424181e 2945) return;
b45424181e 2951) c->object.flags |= UNINTERESTING;
b45424181e 2954) return;
b45424181e 2957) mark_parents_uninteresting(c);
b45424181e 2980) return;
b45424181e 2983) return;
b45424181e 3048) continue;
f0d9cc4196 3097) if (!revs->ignore_missing_links)
f0d9cc4196 3098) die("Failed to traverse parents of commit %s", f0d9cc4196 3099)     oid_to_hex(&commit->object.oid)); b45424181e 3107) continue;
4a7e27e957 3473)     oideq(&p->item->object.oid, &commit->object.oid))

run-command.c
2179045fd0 1229) error(_("cannot create async thread: %s"), strerror(err));

send-pack.c
c0e40a2d66 207) close(fd[1]);

sequencer.c
bcd33ec25f  683) np = strchrnul(buf, '\n');
bcd33ec25f  684) return error(_("no key present in '%.*s'"), bcd33ec25f  695) return error(_("unable to dequote value of '%s'"), bcd33ec25f  737) goto finish;
bcd33ec25f  742) name_i = error(_("'GIT_AUTHOR_NAME' already given")); bcd33ec25f  747) email_i = error(_("'GIT_AUTHOR_EMAIL' already given")); bcd33ec25f  752) date_i = error(_("'GIT_AUTHOR_DATE' already given")); bcd33ec25f  756) err = error(_("unknown variable '%s'"), bcd33ec25f  761) error(_("missing 'GIT_AUTHOR_NAME'")); bcd33ec25f  763) error(_("missing 'GIT_AUTHOR_EMAIL'")); bcd33ec25f  765) error(_("missing 'GIT_AUTHOR_DATE'")); 65850686cf 2329) return;
65850686cf 2426) write_file(rebase_path_quiet(), "%s\n", quiet); 2c58483a59 3427) return error(_("could not checkout %s"), commit); 4df66c40b0 3441) return error(_("%s: not a valid OID"), orig_head); 71f82465b1 3461) fprintf(stderr, _("Stopped at HEAD\n")); b97e187364 4827) return -1;
b97e187364 4830) return -1;
b97e187364 4836) return error_errno(_("could not read '%s'."), todo_file); b97e187364 4839) todo_list_release(&todo_list);
b97e187364 4840) return error(_("unusable todo list: '%s'"), todo_file); b97e187364 4859) todo_list_release(&todo_list);
b97e187364 4860) return -1;
b97e187364 4864) return error(_("could not copy '%s' to '%s'."), todo_file, b97e187364 4868) return error(_("could not transform the todo list")); b97e187364 4897) return error(_("could not transform the todo list")); b97e187364 4900) return error(_("could not skip unnecessary pick commands")); b97e187364 4906) return -1;

setup.c
58b284a2e9  413) return config_error_nonbool(var);

sha1-file.c
67947c34ae sha1-file.c 2225) if (!hasheq(expected_sha1, real_sha1)) {

sha1-name.c
8aac67a174 sha1-name.c  162) return;

split-index.c
e3d837989e 335) ce->ce_flags |= CE_UPDATE_IN_BASE;

strbuf.c
f95736288a  127) --sb->len;

submodule-config.c
bcbc780d14 739) return CONFIG_INVALID_KEY;
45f5ef3d77 754) warning(_("Could not update .gitmodules entry %s"), key);

trace.c
c46c406ae1 189) now = getnanotime();
c46c406ae1 190) perf_start_times[perf_indent] = now;
c46c406ae1 191) if (perf_indent + 1 < ARRAY_SIZE(perf_start_times)) c46c406ae1 192) perf_indent++;
c46c406ae1 195) return now;
c46c406ae1 211) if (perf_indent >= strlen(space))
c46c406ae1 214) strbuf_addf(&buf, ":%.*s ", perf_indent, space); c46c406ae1 317) void trace_performance_leave_fl(const char *file, int line, c46c406ae1 323) if (perf_indent)
c46c406ae1 324) perf_indent--;
c46c406ae1 326) if (!format) /* Allow callers to leave without tracing anything */ c46c406ae1 327) return;
c46c406ae1 329) since = perf_start_times[perf_indent];
c46c406ae1 330) va_start(ap, format);
c46c406ae1 331) trace_performance_vprintf_fl(file, line, nanos - since, format, ap); c46c406ae1 332) va_end(ap);
c46c406ae1 477) trace_performance_leave("git command:%s", command_line.buf); c46c406ae1 485) if (!command_line.len)
c46c406ae1 490) trace_performance_enter();

transport.c
unpack-trees.c
b878579ae7  360) string_list_append(&list, ce->name);
b878579ae7  361) ce->ce_flags &= ~CE_MATCHED;
b878579ae7  368) warning(_("the following paths have collided (e.g. case-sensitive paths\n" b878579ae7  372) for (i = 0; i < list.nr; i++)
b878579ae7  373) fprintf(stderr, "  '%s'\n", list.items[i].string); f1e11c6510  777) free(tree_ce);
b4da37380b  778) return rc;
b4da37380b  785) printf("Unpacked %d entries from %s to %s using cache-tree\n", b4da37380b  787)        o->src_index->cache[pos]->name,
b4da37380b  788)        o->src_index->cache[pos + nr_entries - 1]->name);

upload-pack.c
1d1243fe63 1403) deepen(INFINITE_DEPTH, data->deepen_relative, &data->shallows,

worktree.c
3a3b9d8cde 495) return -1;
3a3b9d8cde 508) return -1;
3a3b9d8cde 517) return -1;
ab3e1f78ae 537) break;

wt-status.c
f3bd35fa0d  671) s->committable = 1;
73ba5d78b4 1958) if (s->state.rebase_in_progress || 73ba5d78b4 1959)     s->state.rebase_interactive_in_progress) 73ba5d78b4 1960) branch_name = s->state.onto; 73ba5d78b4 1961) else if (s->state.detached_from) 73ba5d78b4 1962) branch_name = s->state.detached_from;

xdiff-interface.c
xdiff/xutils.c
611e42a598 405) return -1;

Commits introducing uncovered code:
Ævar Arnfjörð Bjarmason      62c23938f: tests: add a special setup where rebase.useBuiltin is off Alban Gruin      0af129b2e: rebase--interactive2: rewrite the submodes of interactive rebase in C Alban Gruin      2c58483a5: rebase -i: rewrite setup_reflog_action() in C
Alban Gruin      34b47315d: rebase -i: move rebase--helper modes to rebase--interactive Alban Gruin      4df66c40b: rebase -i: rewrite checkout_onto() in C
Alban Gruin      53bbcfbde: rebase -i: implement the main part of interactive rebase as a builtin Alban Gruin      64a43cbd5: rebase -i: rewrite the edit-todo functionality in C
Alban Gruin      65850686c: rebase -i: rewrite write_basic_state() in C
Alban Gruin      a9f5476fb: sequencer: refactor append_todo_help() to write its message to a buffer Alban Gruin      b97e18736: rebase -i: rewrite complete_action() in C
Antonio Ospite      45f5ef3d7: submodule: factor out a config_set_in_gitmodules_file_gently function Antonio Ospite      76e9bdc43: submodule: support reading .gitmodules when it's not in the working tree Antonio Ospite      bcbc780d1: submodule: add a print_config_from_gitmodules() helper
Ben Peart      3255089ad: ieot: add Index Entry Offset Table (IEOT) extension
Ben Peart      3b1d9e045: eoie: add End of Index Entry (EOIE) extension
Ben Peart      77ff1127a: read-cache: load cache entries on worker threads
Ben Peart      abb4bb838: read-cache: load cache extensions on a worker thread
Ben Peart      c780b9cfe: config: add new index.threads config setting
Ben Peart      d1664e73a: add: speed up cmd_add() by utilizing read_cache_preload()
Ben Peart      fa655d841: checkout: optimize "git checkout -b <new_branch>"
Brendan Forster      93aef7c79: http: add support for disabling SSL revocation checks in cURL brian m. carlson      2f0c9e9a9: builtin/repack: replace hard-coded constants
brian m. carlson      eccb5a5f3: apply: rename new_sha1_prefix and old_sha1_prefix Christian Couder      108f53038: pack-objects: move tree_depth into 'struct packing_data' Christian Couder      fe0ac2fb7: pack-objects: move 'layer' into 'struct packing_data' Derrick Stolee      0d5b3a5ef: midx: write object ids in a chunk
Derrick Stolee      17c35c896: packfile: skip loading index if in multi-pack-index
Derrick Stolee      1d614d41e: commit-reach: move ref_newer from remote.c
Derrick Stolee      1dcd9f204: midx: close multi-pack-index on repack
Derrick Stolee      20fd6d579: commit-graph: not compatible with grafts
Derrick Stolee      3715a6335: midx: read objects from multi-pack-index
Derrick Stolee      454ea2e4d: treewide: use get_all_packs
Derrick Stolee      4d80560c5: multi-pack-index: load into memory
Derrick Stolee      4fbcca4ef: commit-reach: make can_all_from_reach... linear
Derrick Stolee      5227c3856: commit-reach: move walk methods from commit.c
Derrick Stolee      525e18c04: midx: clear midx on repack
Derrick Stolee      56ee7ff15: multi-pack-index: add 'verify' verb
Derrick Stolee      662148c43: midx: write object offsets
Derrick Stolee      66ec0390e: fsck: verify multi-pack-index
Derrick Stolee      6a22d5212: pack-objects: consider packs in multi-pack-index
Derrick Stolee      6cc017431: commit-reach: use can_all_from_reach
Derrick Stolee      6d68e6a46: multi-pack-index: provide more helpful usage info
Derrick Stolee      85daa01f6: remote: make add_missing_tags() linear
Derrick Stolee      8aac67a17: midx: use midx in abbreviation calculations
Derrick Stolee      a40498a12: midx: use existing midx when writing new one
Derrick Stolee      b45424181: revision.c: generation-based topo-order algorithm
Derrick Stolee      b67f6b26e: commit-reach: properly peel tags
Derrick Stolee      cc6af73c0: multi-pack-index: verify object offsets
Derrick Stolee      f0d9cc419: revision.c: begin refactoring --topo-order logic
Derrick Stolee      fc59e7484: midx: write header information to lockfile
Derrick Stolee      fe1ed56f5: midx: sort and deduplicate objects from packfiles
Duy Nguyen      b878579ae: clone: report duplicate entries on case-insensitive filesystems Eric Sunshine      2e6fd71a5: format-patch: extend --range-diff to accept revision range Eric Sunshine      40ce41604: format-patch: allow --range-diff to apply to a lone-patch Eric Sunshine      68a6b3a1b: worktree: teach 'move' to override lock when --force given twice Eric Sunshine      8631bf1cd: format-patch: add --creation-factor tweak for --range-diff Eric Sunshine      e19831c94: worktree: teach 'add' to respect --force for registered but missing path Eric Sunshine      e5353bef5: worktree: move delete_git_dir() earlier in file for upcoming new callers Eric Sunshine      ee6cbf712: format-patch: allow --interdiff to apply to a lone-patch
Eric Sunshine      f4143101c: worktree: teach 'remove' to override lock when --force given twice Jeff King      0074c9110: combine-diff: use an xdiff hunk callback
Jeff King      01a31f3bc: pull: handle --verify-signatures for unborn branch
Jeff King      0eb8d3767: cat-file: report an error on multiple --batch options
Jeff King      16d75fa48: repack: add delta-islands support
Jeff King      28b8a7308: pack-objects: add delta-islands support
Jeff King      2c8ee1f53: bundle: dup() output descriptor closer to point-of-use
Jeff King      2fa233a55: pack-objects: handle island check for "external" delta base
Jeff King      30cdc33fb: pack-bitmap: save "have" bitmap from walk
Jeff King      4a7e27e95: convert "oidcmp() == 0" to oideq()
Jeff King      517fe807d: assert NOARG/NONEG behavior of parse-options callbacks
Jeff King      611e42a59: xdiff: provide a separate emit callback for hunks
Jeff King      67947c34a: convert "hashcmp() != 0" to "!hasheq()"
Jeff King      735ca208c: apply: return -1 from option callback instead of calling exit(1) Jeff King      8a2c17467: pathspec: handle non-terminated strings with :(attr)
Jeff King      9001dc2a7: convert "oidcmp() != 0" to "!oideq()"
Jeff King      98f425b45: cat-file: handle streaming failures consistently
Jeff King      c27cc94fa: approxidate: handle pending number for "specials"
Jeff King      c8d521faf: Add delta-islands.{c,h}
Jeff King      cc00e5ce6: convert hashmap comparison functions to oideq()
Jeff King      fce566480: am: handle --no-patch-format option
Johannes Schindelin      21084e84a: http: add support for selecting SSL backends at runtime Johannes Schindelin      3249c1251: rebase: consolidate clean-up code before leaving reset_head() Johannes Schindelin      501afcb8b: mingw: use domain information for default email Johannes Schindelin      71f82465b: rebase -i: introduce the 'break' command
Johannes Schindelin      b67d40adb: http: when using Secure Channel, ignore sslCAInfo by default Johannes Schindelin      bac2a1e36: built-in rebase: reinstate checkout -q behavior where appropriate Johannes Schindelin      bc24382c2: builtin rebase: prepare for builtin rebase -i
Jonathan Nieder      302997027: gc: do not return error for prior errors in daemonized mode Jonathan Nieder      fec2ed218: gc: exit with status 128 on failure
Jonathan Tan      1d1243fe6: upload-pack: make want_obj not global
Josh Steadmon      1127a98cc: fuzz: add fuzz testing for packfile indices.
Matthew DeVore      7c0fe330d: rev-list: handle missing tree objects properly
Matthew DeVore      bc5975d24: list-objects-filter: implement filter tree:0
Matthew DeVore      cc0b05a4c: list-objects-filter-options: do not over-strbuf_init
Matthew DeVore      f447a499d: list-objects: store common func args in struct
Michał Górny      4de9394dc: gpg-interface.c: obtain primary key fingerprint as well
Nguyễn Thái Ngọc Duy      2179045fd: Clean up pthread_create() error handling
Nguyễn Thái Ngọc Duy      252d079cb: read-cache.c: optimize reading index format v4
Nguyễn Thái Ngọc Duy      26c7d0678: help -a: improve and make --verbose default
Nguyễn Thái Ngọc Duy      2abf35038: revision.c: remove implicit dependency on the_index Nguyễn Thái Ngọc Duy      3a3b9d8cd: refs: new ref types to make per-worktree refs visible to all worktrees Nguyễn Thái Ngọc Duy      58b284a2e: worktree: add per-worktree config files
Nguyễn Thái Ngọc Duy      a470beea3: blame.c: rename "repo" argument to "r"
Nguyễn Thái Ngọc Duy      ab3e1f78a: revision.c: better error reporting on ref from different worktrees Nguyễn Thái Ngọc Duy      ae9af1228: status: show progress bar if refreshing the index takes too long Nguyễn Thái Ngọc Duy      b29759d89: fsck: check HEAD and reflog from other worktrees Nguyễn Thái Ngọc Duy      b4da37380: unpack-trees: optimize walking same trees with cache-tree Nguyễn Thái Ngọc Duy      b78ea5fc3: diff.c: reduce implicit dependency on the_index
Nguyễn Thái Ngọc Duy      c0e40a2d6: send-pack.c: move async's #ifdef NO_PTHREADS back to run-command.c Nguyễn Thái Ngọc Duy      c46c406ae: trace.h: support nested performance tracing
Nguyễn Thái Ngọc Duy      c9ef0d95e: reflog expire: cover reflog from all worktrees
Nguyễn Thái Ngọc Duy      f1e11c651: unpack-trees: reduce malloc in cache-tree walk
Nguyễn Thái Ngọc Duy      fd6263fb7: grep: clean up num_threads handling
Olga Telezhnaya      f0062d3b7: ref-filter: free item->value and item->value->s
Phillip Wood      bcd33ec25: add read_author_script() to libgit
Pratik Karki      002ee2fe6: builtin rebase: support keep-empty option
Pratik Karki      0eabf4b95: builtin rebase: stop if git am is in progress
Pratik Karki      12026a412: builtin rebase: support --gpg-sign option
Pratik Karki      122420c29: builtin rebase: support --skip
Pratik Karki      1ed9c14ff: builtin rebase: support --force-rebase
Pratik Karki      3c3588c7d: builtin rebase: support --rebase-merges[=[no-]rebase-cousins] Pratik Karki      55071ea24: rebase: start implementing it as a builtin
Pratik Karki      5a6149453: builtin rebase: support --quit
Pratik Karki      5e5d96197: builtin rebase: support --abort
Pratik Karki      6defce2b0: builtin rebase: support --autostash option
Pratik Karki      73d51ed0a: builtin rebase: support --signoff
Pratik Karki      9a48a615b: builtin rebase: try to fast forward when possible
Pratik Karki      9dba809a6: builtin rebase: support --root
Pratik Karki      ac7f467fe: builtin/rebase: support running "git rebase <upstream>"
Pratik Karki      ba1905a5f: builtin rebase: add support for custom merge strategies
Pratik Karki      bff014dac: builtin rebase: support the verbose and diffstat options Pratik Karki      c54dacb50: builtin rebase: start a new rebase only if none is in progress Pratik Karki      cda614e48: builtin rebase: show progress when connected to a terminal Pratik Karki      e0333e5c6: builtin rebase: require a clean worktree
Pratik Karki      e65123a71: builtin rebase: support git rebase <upstream> <switch-to> Pratik Karki      ead98c111: builtin rebase: support --rerere-autoupdate
Pratik Karki      f28d40d3a: builtin rebase: support --onto
Pratik Karki      f95736288: builtin rebase: support --continue
Rasmus Villemoes      a9a60b94c: git.c: handle_alias: prepend alias info when first argument is -h Rasmus Villemoes      e6e76baaf: help: redirect to aliased commands for "git cmd --help" René Scharfe      3aa4d81f8: mailinfo: support format=flowed
René Scharfe      8b2f8cbcb: oidset: use khash
René Scharfe      fb8952077: fsck: use strbuf_getline() to read skiplist file
Shulhan      5025425df: builtin/remote: quote remote name on error to display empty name Stefan Beller      4a6067cda: refs.c: migrate internal ref iteration to pass thru repository argument Stefan Beller      74d4731da: submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree Stefan Beller      e0a862fda: submodule helper: convert relative URL to absolute URL if needed Stefan Beller      ee69b2a90: submodule--helper: introduce new update-module-mode helper Stephen P. Smith      73ba5d78b: roll wt_status_state into wt_status and populate in the collect phase Stephen P. Smith      f3bd35fa0: wt-status.c: set the committable flag in the collect phase SZEDER Gábor      e3d837989: split-index: don't compare cached data of entries already marked for split index Thomas Gummerer      2373b6505: rerere: mark strings for translation
Thomas Gummerer      4af32207b: rerere: teach rerere to handle nested conflicts
Thomas Gummerer      c0f16f8e1: rerere: factor out handle_conflict function
Tim Schumacher      c6d75bc17: alias: add support for aliases of an alias
Torsten Bögershausen      d64324cb6: Make git_check_attr() a void function

The test coverage reports started mid-way through this release cycle, so I thought it would be good to do a full review of the new uncovered code since the last release.

I eliminated most of the uncovered code due to the following cases:

1. Code was only moved or refactored.
2. Code was related to unusual error conditions (e.g. open_pack_index() fails)

The comments below are intended only to point out potential directions to improve test coverage. Some of it is for me to do!

Thanks,
-Stolee

There are two things wrong here:

1. Not properly covering multi-pack-index fsck with alternates.
2. the ERROR_COMMIT_GRAPH flag when the multi-pack-index is being checked.

I'll submit a patch to fix this.

These lines are inside a block for the following if statement:

+       /*
+        * Otherwise, reachability bitmaps may tell us if the receiver has it, +        * even if it was buried too deep in history to make it into the +        * packing list.
+        */
+       if (thin && bitmap_has_sha1_in_uninteresting(bitmap_git, base_sha1)) {

Peff: is this difficult to test?

This 'depth' variable is incremented as part of a for loop in this patch:

 static void show_object(struct object *obj, const char *name, void *data)
@@ -2686,6 +2706,19 @@ static void show_object(struct object *obj, const char *name, void *data) add_preferred_base_object(name);
        add_object_entry(&obj->oid, obj->type, name, 0);
        obj->flags |= OBJECT_ADDED;
+
+       if (use_delta_islands) {
+               const char *p;
+               unsigned depth = 0;
+               struct object_entry *ent;
+
+               for (p = strchr(name, '/'); p; p = strchr(p + 1, '/'))
+                       depth++;
+
+               ent = packlist_find(&to_pack, obj->oid.hash, NULL);
+               if (ent && depth > ent->tree_depth)
+                       ent->tree_depth = depth;
+       }
 }

And that 'ent->tree_depth = depth;' line is replaced with the oe_set_tree_depth() call in the report.

Since depth is never incremented, we are not covering this block. Is it possible to test?

This is a simple config option check for "repack.useDeltaIslands". The logic it enables is tested, so this is an OK gap, in my opinion.

This block was introduced by this part of the patch:

+       } else if (sub->update_strategy.type != SM_UPDATE_UNSPECIFIED) { +               trace_printf("loaded thing");
+               out->type = sub->update_strategy.type;
+               out->command = sub->update_strategy.command;

Which seems to be an important case, as the other SM_UPDATE_* types seem like interesting cases.

Stefan: what actions would trigger this block? Is it easy to test?

This memcpy happens when the 'old' island_bitmap is passed to island_bitmap_new(), but...

This block has the only non-NULL caller to island_bitmap_new().

It appears that this block would happen if we placed a tag in the delta island.

This block is inside the following patch:

+       if (matches[ARRAY_SIZE(matches) - 1].rm_so != -1)
+               warning(_("island regex from config has "
+                         "too many capture groups (max=%d)"),
+                       (int)ARRAY_SIZE(matches) - 2);
+
+       for (m = 1; m < ARRAY_SIZE(matches); m++) {
+               regmatch_t *match = &matches[m];
+
+               if (match->rm_so == -1)
+                       continue;
+
+               if (island_name.len)
+                       strbuf_addch(&island_name, '-');
+
+               strbuf_add(&island_name, refname + match->rm_so, match->rm_eo - match->rm_so); +       }

This likely means that ARRAY_SIZE(matches) is never more than two.

These blocks are inside the following nested loop in deduplicate_islands():

+       for (ref = 0; ref + 1 < island_count; ref++) {
+               for (src = ref + 1, dst = src; src < island_count; src++) { +                       if (list[ref]->hash == list[src]->hash) +                               continue;
+
+                       if (src != dst)
+                               list[dst] = list[src];
+
+                       dst++;
+               }
+               island_count = dst;
+       }

This means that our "deduplication" logic is never actually doing anything meaningful.

(there is interesting logic in this method, but it is only enabled on case-insensitive filesystems. This run was done on a case-sensitive file system. Related changes happen in unpack-trees.c.)

This logic introduces alias help in 'git help -a'. This seems like a simple thing for adding a test to ensure that this works now and in the future.

The code in here seems to be logic for Windows-specific SSL backends, so is not covered by this report.

There's a lot of stuff going on with showing progress on index writes. While the commit message states the progress doesn't show up for 3 seconds, perhaps that can be tweaked to be in the millisecond range for a test?

(There's a lot of progress stuff here, too.)

There are a lot of lines introduced by the IEOT extension in these commits:

These blocks are currently unreachable because we do not use the new topo-order logic when there are UNINTERESTING commits. (This will be replaced after we have generation numbers v2.) I could force using this logic in a git log --topo-order A..B query when GIT_TEST_COMMIT_GRAPH is enabled.

A bit.

The two features (thin-packs and delta-islands) would not generally be used together (one is for serving fetches, the other is for optimizing on-disk packs to serve fetches). Something like:

echo HEAD^ | git pack-objects --revs --thin --delta-islands

would probably exercise it (assuming there's a delta in HEAD^ against something in HEAD), but you'd need to construct a very specific scenario if you wanted to do any kind of checks no the output.

Looks like t5320 only has single-level trees. We probably just need to add a deeper tree. A more interesting case is when an object really is found at multiple depths, but constructing a case that cared about that would be quite complicated.

That said, there is much bigger problem with this code, which is that 108f530385 (pack-objects: move tree_depth into 'struct packing_data', 2018-08-16) is totally broken. It works on the trivial repository in the test, but try this (especially under valgrind or ASan) on a real repository:

git repack -adi

which will crash immediately. It doesn't correctly maintain the invariant that if tree_depth is not NULL, it is the same size as the main object array.

I'll see if I can come up with a fix.

-Peff

Just a quick update to prevent anyone else looking at it: I have a fix for this (and another related issue with that commit).

There's an edge case in that depth computation that I think is unhandled, as well. I _think_ it doesn't trigger very often, but I'm running some experiments to verify it. That's S-L-O-W, so I probably won't have results until tomorrow.

-Peff

These should be hit if you run the test suite with GIT_TEST_INDEX_THREADS=2. Without that, the indexes for the various tests are too small to trigger multi-threaded index reads/writes.

From t/README:

GIT_TEST_INDEX_THREADS=<n> enables exercising the multi-threaded loading of the index for the whole test suite by bypassing the default number of cache entries and thread minimums. Setting this to 1 will make the index loading single threaded.