index-pack: disable rev-index if index file has non .idx suffix

Teach index-pack to silently omit the reverse index if the
index file does not have the standard ".idx" suffix.

In e37d0b8 (builtin/index-pack.c: write reverse indexes, 2021-01-25)
we learned to create `.rev` reverse indexes in addition to `.idx` index
files.  The `.rev` file pathname is constructed by replacing the suffix
on the `.idx` file.  The code assumes a hard-coded "idx" suffix.

In a8dd7e0 (config: enable `pack.writeReverseIndex` by default, 2023-04-12)
reverse indexes were enabled by default.

If the `-o <idx-path>` argument is used, the index file may have a
different suffix.  This causes an error when it tries to create the
reverse index pathname.

Since we do not know why the user requested a non-standard suffix for
the index, we cannot guess what the proper corresponding suffix should
be for the reverse index.  So we disable it.

The t5300 test has been updated to verify that we no longer error
out and that the .rev file is not created.

TODO We could warn the user that we skipped it (perhaps only if they
TODO explicitly requested `--rev-index` on the command line).
TODO
TODO Ideally, we should add an `--rev-index-path=<path>` argument
TODO or change `--rev-index` to take a pathname.
TODO
TODO I'll leave these questions for a future series.

Signed-off-by: Jeff Hostetler <[email protected]>

Author: jeffhostetler

builtin/index-pack.c

@@ -1734,6 +1734,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
 	unsigned foreign_nr = 1;	/* zero is a "good" value, assume bad */
 	int report_end_of_input = 0;
 	int hash_algo = 0;
+	int dash_o = 0;
 
 	/*
 	 * index-pack never needs to fetch missing objects except when
@@ -1827,6 +1828,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
 				if (index_name || (i+1) >= argc)
 					usage(index_pack_usage);
 				index_name = argv[++i];
+				dash_o = 1;
 			} else if (starts_with(arg, "--index-version=")) {
 				char *c;
 				opts.version = strtoul(arg + 16, &c, 10);
@@ -1869,6 +1871,8 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
 		index_name = derive_filename(pack_name, "pack", "idx", &index_name_buf);
 
 	opts.flags &= ~(WRITE_REV | WRITE_REV_VERIFY);
+	if (rev_index && dash_o && !ends_with(index_name, ".idx"))
+		rev_index = 0;
 	if (rev_index) {
 		opts.flags |= verify ? WRITE_REV_VERIFY : WRITE_REV;
 		if (index_name)

t/t5300-pack-object.sh

@@ -372,11 +372,10 @@ test_expect_success 'complain about index name' '
 	test -f test-complain-0.idx &&
 	test -f test-complain-0.rev &&
 
-	# Non .idx suffix
+	# Non .idx suffix -- implicitly omits the .rev
 	cat test-1-${packname_1}.pack >test-complain-1.pack &&
-	test_must_fail git index-pack -o test-complain-1.idx-suffix --rev-index test-complain-1.pack 2>err &&
-	grep "does not end" err &&
-	! test -f test-complain-1.idx-suffix &&
+	git index-pack -o test-complain-1.idx-suffix --rev-index test-complain-1.pack &&
+	test -f test-complain-1.idx-suffix &&
 	! test -f test-complain-1.rev
 '