Revert "Merge branch 'kn/rev-list-missing-fix' into next"

This reverts commit 539495ec89, reversing
changes made to ea05f2083d.

Documentation/git.txt

@@ -911,15 +911,6 @@ for full details.
 	should not normally need to set this to `0`, but it may be
 	useful when trying to salvage data from a corrupted repository.
 
-`GIT_COMMIT_GRAPH_PARANOIA`::
-	If this Boolean environment variable is set to false, ignore the
-	case where commits exist in the commit graph but not in the
-	object database. Normally, Git will check whether commits loaded
-	from the commit graph exist in the object database to avoid
-	issues with stale commit graphs, but this check comes with a
-	performance penalty. The default is `1` (i.e., be paranoid about
-	stale commits in the commit graph).
-
 `GIT_ALLOW_PROTOCOL`::
 	If set to a colon-separated list of protocols, behave as if
 	`protocol.allow` is set to `never`, and each of the listed

builtin/reflog.c

@@ -298,7 +298,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 		struct rev_info revs;
 
 		repo_init_revisions(the_repository, &revs, prefix);
-		revs.do_not_die_on_missing_objects = 1;
+		revs.do_not_die_on_missing_tree = 1;
 		revs.ignore_missing = 1;
 		revs.ignore_missing_links = 1;
 		if (verbose)

builtin/rev-list.c

@@ -100,48 +100,7 @@ static off_t get_object_disk_usage(struct object *obj)
 	return size;
 }
 
-static inline void finish_object__ma(struct object *obj)
+static void finish_commit(struct commit *commit);
-{
-	/*
-	 * Whether or not we try to dynamically fetch missing objects
-	 * from the server, we currently DO NOT have the object.  We
-	 * can either print, allow (ignore), or conditionally allow
-	 * (ignore) them.
-	 */
-	switch (arg_missing_action) {
-	case MA_ERROR:
-		die("missing %s object '%s'",
-		    type_name(obj->type), oid_to_hex(&obj->oid));
-		return;
-
-	case MA_ALLOW_ANY:
-		return;
-
-	case MA_PRINT:
-		oidset_insert(&missing_objects, &obj->oid);
-		return;
-
-	case MA_ALLOW_PROMISOR:
-		if (is_promisor_object(&obj->oid))
-			return;
-		die("unexpected missing %s object '%s'",
-		    type_name(obj->type), oid_to_hex(&obj->oid));
-		return;
-
-	default:
-		BUG("unhandled missing_action");
-		return;
-	}
-}
-
-static void finish_commit(struct commit *commit)
-{
-	free_commit_list(commit->parents);
-	commit->parents = NULL;
-	free_commit_buffer(the_repository->parsed_objects,
-			   commit);
-}
-
 static void show_commit(struct commit *commit, void *data)
 {
 	struct rev_list_info *info = data;
@@ -149,12 +108,6 @@ static void show_commit(struct commit *commit, void *data)
 
 	display_progress(progress, ++progress_counter);
 
-	if (revs->do_not_die_on_missing_objects &&
-	    oidset_contains(&revs->missing_commits, &commit->object.oid)) {
-		finish_object__ma(&commit->object);
-		return;
-	}
-
 	if (show_disk_usage)
 		total_disk_usage += get_object_disk_usage(&commit->object);
 
@@ -266,6 +219,48 @@ static void show_commit(struct commit *commit, void *data)
 	finish_commit(commit);
 }
 
+static void finish_commit(struct commit *commit)
+{
+	free_commit_list(commit->parents);
+	commit->parents = NULL;
+	free_commit_buffer(the_repository->parsed_objects,
+			   commit);
+}
+
+static inline void finish_object__ma(struct object *obj)
+{
+	/*
+	 * Whether or not we try to dynamically fetch missing objects
+	 * from the server, we currently DO NOT have the object.  We
+	 * can either print, allow (ignore), or conditionally allow
+	 * (ignore) them.
+	 */
+	switch (arg_missing_action) {
+	case MA_ERROR:
+		die("missing %s object '%s'",
+		    type_name(obj->type), oid_to_hex(&obj->oid));
+		return;
+
+	case MA_ALLOW_ANY:
+		return;
+
+	case MA_PRINT:
+		oidset_insert(&missing_objects, &obj->oid);
+		return;
+
+	case MA_ALLOW_PROMISOR:
+		if (is_promisor_object(&obj->oid))
+			return;
+		die("unexpected missing %s object '%s'",
+		    type_name(obj->type), oid_to_hex(&obj->oid));
+		return;
+
+	default:
+		BUG("unhandled missing_action");
+		return;
+	}
+}
+
 static int finish_object(struct object *obj, const char *name UNUSED,
 			 void *cb_data)
 {
@@ -566,7 +561,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	}
 
 	if (arg_missing_action)
-		revs.do_not_die_on_missing_objects = 1;
+		revs.do_not_die_on_missing_tree = 1;
 
 	argc = setup_revisions(argc, argv, &revs, &s_r_opt);
 

commit-graph.c

@@ -1024,18 +1024,14 @@ int repo_find_commit_pos_in_graph(struct repository *r, struct commit *c,
 
 struct commit *lookup_commit_in_graph(struct repository *repo, const struct object_id *id)
 {
-	static int object_paranoia = -1;
 	struct commit *commit;
 	uint32_t pos;
 
-	if (object_paranoia == -1)
-		object_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 1);
-
 	if (!prepare_commit_graph(repo))
 		return NULL;
 	if (!search_commit_pos_in_graph(id, repo->objects->commit_graph, &pos))
 		return NULL;
-	if (object_paranoia && !has_object(repo, id, 0))
+	if (!has_object(repo, id, 0))
 		return NULL;
 
 	commit = lookup_commit(repo, id);

commit-graph.h

@@ -8,12 +8,6 @@
 #define GIT_TEST_COMMIT_GRAPH_DIE_ON_PARSE "GIT_TEST_COMMIT_GRAPH_DIE_ON_PARSE"
 #define GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS "GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS"
 
-/*
- * This environment variable controls whether commits looked up via the
- * commit graph will be double checked to exist in the object database.
- */
-#define GIT_COMMIT_GRAPH_PARANOIA "GIT_COMMIT_GRAPH_PARANOIA"
-
 /*
  * This method is only used to enhance coverage of the commit-graph
  * feature in the test suite with the GIT_TEST_COMMIT_GRAPH and

commit.c

@@ -28,7 +28,6 @@
 #include "shallow.h"
 #include "tree.h"
 #include "hook.h"
-#include "parse.h"
 
 static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);
 
@@ -573,21 +572,8 @@ int repo_parse_commit_internal(struct repository *r,
 		return -1;
 	if (item->object.parsed)
 		return 0;
-	if (use_commit_graph && parse_commit_in_graph(r, item)) {
+	if (use_commit_graph && parse_commit_in_graph(r, item))
-		static int object_paranoia = -1;
-
-		if (object_paranoia == -1)
-			object_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 1);
-
-		if (object_paranoia && !has_object(r, &item->object.oid, 0)) {
-			unparse_commit(r, &item->object.oid);
-			return quiet_on_missing ? -1 :
-				error(_("commit %s exists in commit-graph but not in the object database"),
-				      oid_to_hex(&item->object.oid));
-		}
-
 		return 0;
-	}
 
 	if (oid_object_info_extended(r, &item->object.oid, &oi, flags) < 0)
 		return quiet_on_missing ? -1 :

list-objects.c

@@ -177,7 +177,7 @@ static void process_tree(struct traversal_context *ctx,
 		    is_promisor_object(&obj->oid))
 			return;
 
-		if (!revs->do_not_die_on_missing_objects)
+		if (!revs->do_not_die_on_missing_tree)
 			die("bad tree object %s", oid_to_hex(&obj->oid));
 	}
 
@@ -389,9 +389,6 @@ static void do_traverse(struct traversal_context *ctx)
 		 */
 		if (!ctx->revs->tree_objects)
 			; /* do not bother loading tree */
-		else if (ctx->revs->do_not_die_on_missing_objects &&
-			 oidset_contains(&ctx->revs->missing_commits, &commit->object.oid))
-			;
 		else if (repo_get_commit_tree(the_repository, commit)) {
 			struct tree *tree = repo_get_commit_tree(the_repository,
 								 commit);

revision.c

@@ -6,7 +6,6 @@
 #include "object-name.h"
 #include "object-file.h"
 #include "object-store-ll.h"
-#include "oidset.h"
 #include "tag.h"
 #include "blob.h"
 #include "tree.h"
@@ -1113,9 +1112,6 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
 
 	if (commit->object.flags & ADDED)
 		return 0;
-	if (revs->do_not_die_on_missing_objects &&
-	    oidset_contains(&revs->missing_commits, &commit->object.oid))
-		return 0;
 	commit->object.flags |= ADDED;
 
 	if (revs->include_check &&
@@ -1172,8 +1168,7 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
 	for (parent = commit->parents; parent; parent = parent->next) {
 		struct commit *p = parent->item;
 		int gently = revs->ignore_missing_links ||
-			     revs->exclude_promisor_objects ||
+			     revs->exclude_promisor_objects;
-			     revs->do_not_die_on_missing_objects;
 		if (repo_parse_commit_gently(revs->repo, p, gently) < 0) {
 			if (revs->exclude_promisor_objects &&
 			    is_promisor_object(&p->object.oid)) {
@@ -1181,11 +1176,7 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
 					break;
 				continue;
 			}
-
+			return -1;
-			if (revs->do_not_die_on_missing_objects)
-				oidset_insert(&revs->missing_commits, &p->object.oid);
-			else
-				return -1; /* corrupt repository */
 		}
 		if (revs->sources) {
 			char **slot = revision_sources_at(revs->sources, p);
@@ -3118,7 +3109,6 @@ void release_revisions(struct rev_info *revs)
 	clear_decoration(&revs->merge_simplification, free);
 	clear_decoration(&revs->treesame, free);
 	line_log_free(revs);
-	oidset_clear(&revs->missing_commits);
 }
 
 static void add_child(struct rev_info *revs, struct commit *parent, struct commit *child)
@@ -3810,8 +3800,6 @@ int prepare_revision_walk(struct rev_info *revs)
 				       FOR_EACH_OBJECT_PROMISOR_ONLY);
 	}
 
-	oidset_init(&revs->missing_commits, 0);
-
 	if (!revs->reflog_info)
 		prepare_to_use_bloom_filter(revs);
 	if (!revs->unsorted_input)

revision.h

@@ -4,7 +4,6 @@
 #include "commit.h"
 #include "grep.h"
 #include "notes.h"
-#include "oidset.h"
 #include "pretty.h"
 #include "diff.h"
 #include "commit-slab-decl.h"
@@ -213,19 +212,18 @@ struct rev_info {
 
 			/*
 			 * Blobs are shown without regard for their existence.
-			 * But not so for trees/commits: unless exclude_promisor_objects
+			 * But not so for trees: unless exclude_promisor_objects
 			 * is set and the tree in question is a promisor object;
 			 * OR ignore_missing_links is set, the revision walker
-			 * dies with a "bad <type> object HASH" message when
+			 * dies with a "bad tree object HASH" message when
-			 * encountering a missing object. For callers that can
+			 * encountering a missing tree. For callers that can
-			 * handle missing trees/commits and want them to be filterable
+			 * handle missing trees and want them to be filterable
 			 * and showable, set this to true. The revision walker
-			 * will filter and show such a missing object as usual,
+			 * will filter and show such a missing tree as usual,
-			 * but will not attempt to recurse into this tree/commit
+			 * but will not attempt to recurse into this tree
-			 * object. The revision walker will also set the MISSING
+			 * object.
-			 * flag for such objects.
 			 */
-			do_not_die_on_missing_objects:1,
+			do_not_die_on_missing_tree:1,
 
 			/* for internal use only */
 			exclude_promisor_objects:1;
@@ -374,9 +372,6 @@ struct rev_info {
 
 	/* Location where temporary objects for remerge-diff are written. */
 	struct tmp_objdir *remerge_objdir;
-
-	/* Missing commits to be tracked without failing traversal. */
-	struct oidset missing_commits;
 };
 
 /**

t/t5318-commit-graph.sh

@@ -895,52 +895,4 @@ test_expect_success 'reader notices too-small generations chunk' '
 	test_cmp expect.err err
 '
 
-test_expect_success 'stale commit cannot be parsed when given directly' '
-	test_when_finished "rm -rf repo" &&
-	git init repo &&
-	(
-		cd repo &&
-		test_commit A &&
-		test_commit B &&
-		git commit-graph write --reachable &&
-
-		oid=$(git rev-parse B) &&
-		rm .git/objects/"$(test_oid_to_path "$oid")" &&
-
-		# Verify that it is possible to read the commit from the
-		# commit graph when not being paranoid, ...
-		GIT_COMMIT_GRAPH_PARANOIA=false git rev-list B &&
-		# ... but parsing the commit when double checking that
-		# it actually exists in the object database should fail.
-		test_must_fail git rev-list -1 B
-	)
-'
-
-test_expect_success 'stale commit cannot be parsed when traversing graph' '
-	test_when_finished "rm -rf repo" &&
-	git init repo &&
-	(
-		cd repo &&
-
-		test_commit A &&
-		test_commit B &&
-		test_commit C &&
-		git commit-graph write --reachable &&
-
-		# Corrupt the repository by deleting the intermittent commit
-		# object. Commands should notice that this object is absent and
-		# thus that the repository is corrupt even if the commit graph
-		# exists.
-		oid=$(git rev-parse B) &&
-		rm .git/objects/"$(test_oid_to_path "$oid")" &&
-
-		# Again, we should be able to parse the commit when not
-		# being paranoid about commit graph staleness...
-		GIT_COMMIT_GRAPH_PARANOIA=false git rev-parse HEAD~2 &&
-		# ... but fail when we are paranoid.
-		test_must_fail git rev-parse HEAD~2 2>error &&
-		grep "error: commit $oid exists in commit-graph but not in the object database" error
-	)
-'
-
 test_done

t/t6022-rev-list-missing.sh

@@ -1,74 +0,0 @@
-#!/bin/sh
-
-test_description='handling of missing objects in rev-list'
-
-TEST_PASSES_SANITIZE_LEAK=true
-. ./test-lib.sh
-
-# We setup the repository with two commits, this way HEAD is always
-# available and we can hide commit 1.
-test_expect_success 'create repository and alternate directory' '
-	test_commit 1 &&
-	test_commit 2 &&
-	test_commit 3
-'
-
-for obj in "HEAD~1" "HEAD~1^{tree}" "HEAD:1.t"
-do
-	test_expect_success "rev-list --missing=error fails with missing object $obj" '
-		oid="$(git rev-parse $obj)" &&
-		path=".git/objects/$(test_oid_to_path $oid)" &&
-
-		mv "$path" "$path.hidden" &&
-		test_when_finished "mv $path.hidden $path" &&
-
-		test_must_fail git rev-list --missing=error --objects \
-			--no-object-names HEAD
-	'
-done
-
-for obj in "HEAD~1" "HEAD~1^{tree}" "HEAD:1.t"
-do
-	for action in "allow-any" "print"
-	do
-		test_expect_success "rev-list --missing=$action with missing $obj" '
-			oid="$(git rev-parse $obj)" &&
-			path=".git/objects/$(test_oid_to_path $oid)" &&
-
-			# Before the object is made missing, we use rev-list to
-			# get the expected oids.
-			git rev-list --objects --no-object-names \
-				HEAD ^$obj >expect.raw &&
-
-			# Blobs are shared by all commits, so evethough a commit/tree
-			# might be skipped, its blob must be accounted for.
-			if [ $obj != "HEAD:1.t" ]; then
-				echo $(git rev-parse HEAD:1.t) >>expect.raw &&
-				echo $(git rev-parse HEAD:2.t) >>expect.raw
-			fi &&
-
-			mv "$path" "$path.hidden" &&
-			test_when_finished "mv $path.hidden $path" &&
-
-			git rev-list --missing=$action --objects --no-object-names \
-				HEAD >actual.raw &&
-
-			# When the action is to print, we should also add the missing
-			# oid to the expect list.
-			case $action in
-			allow-any)
-				;;
-			print)
-				grep ?$oid actual.raw &&
-				echo ?$oid >>expect.raw
-				;;
-			esac &&
-
-			sort actual.raw >actual &&
-			sort expect.raw >expect &&
-			test_cmp expect actual
-		'
-	done
-done
-
-test_done