Skip to content

Commit

Permalink
index-pack: disable rev-index if index file has non .idx suffix
Browse files Browse the repository at this point in the history
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 <jeffhostetler@github.com>
  • Loading branch information
jeffhostetler authored and mjcheetham committed Jul 29, 2024
1 parent 8530990 commit abc8681
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 4 deletions.
4 changes: 4 additions & 0 deletions builtin/index-pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 3 additions & 4 deletions t/t5300-pack-object.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
'

Expand Down

0 comments on commit abc8681

Please sign in to comment.