index-pack: handle --strict checks of non-repo packs

Commit 73c3f0f704 (index-pack: check .gitmodules files with
--strict, 2018-05-04) added a call to add_packed_git(), with
the intent that the newly-indexed objects would be available
to the process when we run fsck_finish().  But that's not
what add_packed_git() does. It only allocates the struct,
and you must install_packed_git() on the result. So that
call was effectively doing nothing (except leaking a
struct).

But wait, we passed all of the tests! Does that mean we
don't need the call at all?

For normal cases, no. When we run "index-pack --stdin"
inside a repository, we write the new pack into the object
directory. If fsck_finish() needs to access one of the new
objects, then our initial lookup will fail to find it, but
we'll follow up by running reprepare_packed_git() and
looking again. That logic was meant to handle somebody else
repacking simultaneously, but it ends up working for us
here.

But there is a case that does need this, that we were not
testing. You can run "git index-pack foo.pack" on any file,
even when it is not inside the object directory. Or you may
not even be in a repository at all! This case fails without
doing the proper install_packed_git() call.

We can make this work by adding the install call.

Note that we should be prepared to handle add_packed_git()
failing. We can just silently ignore this case, though. If
fsck_finish() later needs the objects and they're not
available, it will complain itself. And if it doesn't
(because we were able to resolve the whole fsck in the first
pass), then it actually isn't an interesting error at all.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 7b2f7c04703f5eff8a1eda06ae59c0d22a8a1ab5..7b399478dd43684c1e4405b5be7588781e3e7180 100644 (file)
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1480,8 +1480,12 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
        } else
                chmod(final_index_name, 0444);
 
-       if (do_fsck_object)
-               add_packed_git(final_index_name, strlen(final_index_name), 0);
+       if (do_fsck_object) {
+               struct packed_git *p;
+               p = add_packed_git(final_index_name, strlen(final_index_name), 0);
+               if (p)
+                       install_packed_git(the_repository, p);
+       }
 
        if (!from_stdin) {
                printf("%s\n", sha1_to_hex(hash));

diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh
index a770d92a5592e5aab1760d0ff509f5ce3cc8e5d0..4157e1a134e15678e8b271a036230ad6a8ffc257 100755 (executable)
--- a/t/t7415-submodule-names.sh
+++ b/t/t7415-submodule-names.sh
@@ -122,6 +122,16 @@ test_expect_success 'transfer.fsckObjects handles odd pack (index)' '
        test_must_fail git -C dst.git index-pack --strict --stdin <odd.pack
 '
 
+test_expect_success 'index-pack --strict works for non-repo pack' '
+       rm -rf dst.git &&
+       git init --bare dst.git &&
+       cp odd.pack dst.git &&
+       test_must_fail git -C dst.git index-pack --strict odd.pack 2>output &&
+       # Make sure we fail due to bad gitmodules content, not because we
+       # could not read the blob in the first place.
+       grep gitmodulesName output
+'
+
 test_expect_success 'fsck detects symlinked .gitmodules file' '
        git init symlink &&
        (