[v2,08/17] mktree.c: do not fail on mismatched submodule type

Commit Message

Victoria Dye June 19, 2024, 9:57 p.m. UTC

From: Victoria Dye <[e-mail]>

Adjust the 'git mktree' tree entry intake logic to no longer fail if an OID
specified with a S_IFGITLINK mode exists in the current repository's object
database with a different type.

While this scenario likely represents a mistake by the user, submodule OIDs
are not validated as part of object writes or in 'git fsck'. In other
commands, any object info would be ignored if such an OID was found in the
current repository with a different type.

Since this check is not needed to avoid creation of a corrupt tree, let's
remove it and make 'git mktree' less opinionated as a result.

Helped-by: Junio C Hamano <[e-mail]>
Signed-off-by: Victoria Dye <[e-mail]>
---
 builtin/mktree.c  | 58 ++++++++++++++++++++++-------------------------
 t/t1010-mktree.sh | 18 ++++++---------
 2 files changed, 34 insertions(+), 42 deletions(-)

Patch

diff --git a/builtin/mktree.c b/builtin/mktree.c
index 03a9899bc11..f509ed1a81f 100644
--- a/builtin/mktree.c
+++ b/builtin/mktree.c
@@ -107,8 +107,6 @@  static int mktree_line(unsigned int mode, struct object_id *oid,
 {
 	struct mktree_line_data *data = cbdata;
 	enum object_type mode_type = object_type(mode);
-	struct object_info oi = OBJECT_INFO_INIT;
-	enum object_type parsed_obj_type;
 
 	if (stage)
 		die(_("path '%s' is unmerged"), path);
@@ -117,36 +115,34 @@  static int mktree_line(unsigned int mode, struct object_id *oid,
 		die("object type (%s) doesn't match mode type (%s)",
 		    type_name(obj_type), type_name(mode_type));
 
-	oi.typep = &parsed_obj_type;
-
-	if (oid_object_info_extended(the_repository, oid, &oi,
-				     OBJECT_INFO_LOOKUP_REPLACE |
+	if (!S_ISGITLINK(mode)) {
+		struct object_info oi = OBJECT_INFO_INIT;
+		enum object_type parsed_obj_type;
+		unsigned int flags = OBJECT_INFO_LOOKUP_REPLACE |
 				     OBJECT_INFO_QUICK |
-				     OBJECT_INFO_SKIP_FETCH_OBJECT) < 0)
-		parsed_obj_type = -1;
-
-	if (parsed_obj_type < 0) {
-		/*
-		 * There are two conditions where the object being missing
-		 * is acceptable:
-		 *
-		 * - We're explicitly allowing it with --missing.
-		 * - The object is a submodule, which we wouldn't expect to
-		 *   be in this repo anyway.
-		 *
-		 * If neither condition is met, die().
-		 */
-		if (!data->allow_missing && !S_ISGITLINK(mode))
-			die("entry '%s' object %s is unavailable", path, oid_to_hex(oid));
-
-	} else if (parsed_obj_type != mode_type) {
-		/*
-		 * The object exists but is of the wrong type.
-		 * This is a problem regardless of allow_missing
-		 * because the new tree entry will never be correct.
-		 */
-		die("entry '%s' object %s is a %s but specified type was (%s)",
-		    path, oid_to_hex(oid), type_name(parsed_obj_type), type_name(mode_type));
+				     OBJECT_INFO_SKIP_FETCH_OBJECT;
+
+		oi.typep = &parsed_obj_type;
+
+		if (oid_object_info_extended(the_repository, oid, &oi, flags) < 0) {
+			/*
+			 * If the object is missing and we aren't explicitly
+			 * allowing missing objects, die(). Otherwise, continue
+			 * without error.
+			 */
+			if (!data->allow_missing)
+				die("entry '%s' object %s is unavailable", path,
+				    oid_to_hex(oid));
+		} else if (parsed_obj_type != mode_type) {
+			/*
+			 * The object exists but is of the wrong type.
+			 * This is a problem regardless of allow_missing
+			 * because the new tree entry will never be correct.
+			 */
+			die("entry '%s' object %s is a %s but specified type was (%s)",
+			    path, oid_to_hex(oid), type_name(parsed_obj_type),
+			    type_name(mode_type));
+		}
 	}
 
 	append_to_tree(mode, oid, path, data->arr);
diff --git a/t/t1010-mktree.sh b/t/t1010-mktree.sh
index 649842fa27c..48fc532e7af 100755
--- a/t/t1010-mktree.sh
+++ b/t/t1010-mktree.sh
@@ -71,17 +71,13 @@  test_expect_success 'allow missing object with --missing' '
 '
 
 test_expect_success 'mktree with invalid submodule OIDs' '
-	# non-existent OID - ok
-	printf "160000 commit $(test_oid numeric)\tA\n" >in &&
-	git mktree <in >tree.actual &&
-	git ls-tree $(cat tree.actual) >actual &&
-	test_cmp in actual &&
-
-	# existing OID, wrong type - error
-	tree_oid="$(cat tree)" &&
-	printf "160000 commit $tree_oid\tA" |
-	test_must_fail git mktree 2>err &&
-	test_grep "object $tree_oid is a tree but specified type was (commit)" err
+	for oid in "$(test_oid numeric)" "$(cat tree)"
+	do
+		printf "160000 commit $oid\tA\n" >in &&
+		git mktree <in >tree.actual &&
+		git ls-tree $(cat tree.actual) >actual &&
+		test_cmp in actual || return 1
+	done
 '
 
 test_expect_success 'mktree refuses to read ls-tree -r output (1)' '