Merge pull request git-for-windows#168: commit: remove parse_commit_no_graph()

The parse_commit_no_graph() method was added in 43d3561 ("commit-graph
write: don't die if the existing graph is corrupt" 2019-03-25) as a way
to avoid persisting bad data across commit-graph files. That is, if the
commit-graph file has undetected corrupt data -- such as a flipped bit
in a parent int-id value -- then that data will persist to the next
commit-graph file. The parse_commit_no_graph() method was used to always
use the pack data directly instead.

Unfortunately, this comes at a significant performance cost. In both
time and memory, parsing from pack files is much slower than parsing
from the commit-graph file. In a repository with 4.5 million commits,
this can lead to Git taking up to 11gb of memory to rewrite the file.

Now that the incremental commit-graph file format exists, we can rely
on the quality of the commit-graph file if we follow the two-step
pattern of (1) write a commit-graph with "--split" and (2) run "git
commit-graph verify --shallow" to verify the tip file.

Author: derrickstolee

commit-graph.c

@@ -468,10 +468,6 @@ static int prepare_commit_graph(struct repository *r)
 	struct object_directory *odb;
 	int config_value;
 
-	if (git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD, 0))
-		die("dying as requested by the '%s' variable on commit-graph load!",
-		    GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD);
-
 	if (r->objects->commit_graph_attempted)
 		return !!r->objects->commit_graph;
 	r->objects->commit_graph_attempted = 1;
@@ -841,7 +837,7 @@ static void write_graph_chunk_data(struct hashfile *f, int hash_len,
 		uint32_t packedDate[2];
 		display_progress(ctx->progress, ++ctx->progress_cnt);
 
-		parse_commit_no_graph(*list);
+		parse_commit(*list);
 		hashwrite(f, get_commit_tree_oid(*list)->hash, hash_len);
 
 		parent = (*list)->parents;
@@ -1059,7 +1055,7 @@ static void close_reachable(struct write_commit_graph_context *ctx)
 			if (!parse_commit(commit) &&
 			    commit->graph_pos == COMMIT_NOT_FROM_GRAPH)
 				add_missing_parents(ctx, commit);
-		} else if (!parse_commit_no_graph(commit))
+		} else if (!parse_commit(commit))
 			add_missing_parents(ctx, commit);
 	}
 	stop_progress(&ctx->progress);
@@ -1295,7 +1291,7 @@ static void copy_oids_to_commits(struct write_commit_graph_context *ctx)
 		    ctx->commits.list[ctx->commits.nr]->graph_pos != COMMIT_NOT_FROM_GRAPH)
 			continue;
 
-		parse_commit_no_graph(ctx->commits.list[ctx->commits.nr]);
+		parse_commit(ctx->commits.list[ctx->commits.nr]);
 
 		for (parent = ctx->commits.list[ctx->commits.nr]->parents;
 		     parent; parent = parent->next)

commit-graph.h

@@ -7,7 +7,6 @@
 #include "cache.h"
 
 #define GIT_TEST_COMMIT_GRAPH "GIT_TEST_COMMIT_GRAPH"
-#define GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD "GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD"
 
 struct commit;
 

commit.h

@@ -89,12 +89,6 @@ static inline int repo_parse_commit(struct repository *r, struct commit *item)
 {
 	return repo_parse_commit_gently(r, item, 0);
 }
-
-static inline int parse_commit_no_graph(struct commit *commit)
-{
-	return repo_parse_commit_internal(the_repository, commit, 0, 0);
-}
-
 #ifndef NO_THE_REPOSITORY_COMPATIBILITY_MACROS
 #define parse_commit_internal(item, quiet, use) repo_parse_commit_internal(the_repository, item, quiet, use)
 #define parse_commit_gently(item, quiet) repo_parse_commit_gently(the_repository, item, quiet)

t/t5318-commit-graph.sh

@@ -385,13 +385,7 @@ corrupt_graph_verify() {
 	test_must_fail git commit-graph verify 2>test_err &&
 	grep -v "^+" test_err >err &&
 	test_i18ngrep "$grepstr" err &&
-	if test "$2" != "no-copy"
-	then
-		cp $objdir/info/commit-graph commit-graph-pre-write-test
-	fi &&
-	git status --short &&
-	GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD=true git commit-graph write &&
-	git commit-graph verify
+	git status --short
 }
 
 # usage: corrupt_graph_and_verify <position> <data> <string> [<zero_pos>]
@@ -415,9 +409,10 @@ corrupt_graph_and_verify() {
 }
 
 test_expect_success POSIXPERM,SANITY 'detect permission problem' '
+	test_when_finished chmod 666 $objdir/info/commit-graph &&
 	corrupt_graph_setup &&
 	chmod 000 $objdir/info/commit-graph &&
-	corrupt_graph_verify "Could not open" "no-copy"
+	corrupt_graph_verify "Could not open"
 '
 
 test_expect_success 'detect too small' '
@@ -536,7 +531,6 @@ test_expect_success 'git fsck (checks commit-graph)' '
 	git fsck &&
 	corrupt_graph_and_verify $GRAPH_BYTE_FOOTER "\00" \
 		"incorrect checksum" &&
-	cp commit-graph-pre-write-test $objdir/info/commit-graph &&
 	test_must_fail git fsck
 '