On Thu, Jul 12, 2018 at 12:23:28AM +0200, Johannes Schindelin via GitGitGadget wrote:
> This is particularly important to keep in mind when looking at the
> `.git/shallow` file: if any commits listed in that file become
> unreachable, it is not a problem, but if they go missing, it *is* a
> problem. One symptom of this problem is that a deepening fetch may now
> fail with
>
> fatal: error in object: unshallow <commit-hash>
>
> To avoid this problem, let's prune the shallow list in `git repack` when
> the `-d` option is passed, unless `-A` is passed, too (which would force
> the now-unreachable objects to be turned into loose objects instead of
> being deleted).
I'm not sure if this covers all cases:
- even with "-A", we may still drop objects subject to
--unpack-unreachable. So if your pack has an old mtime (e.g., because
you haven't packed in a while) I think you'd see the same problem.
- if you use "-adk", we'd keep all objects, and this pruning would not
be necessary
> diff --git a/builtin/repack.c b/builtin/repack.c
> index 6c636e159..45f321b23 100644
> --- a/builtin/repack.c
> +++ b/builtin/repack.c
> @@ -444,6 +444,10 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
> if (!quiet && isatty(2))
> opts |= PRUNE_PACKED_VERBOSE;
> prune_packed_objects(opts);
> +
> + if (!(pack_everything & LOOSEN_UNREACHABLE) &&
> + is_repository_shallow())
> + prune_shallow(0);
> }
I understand how this solves your immediate problem, but it feels like a
weird layering violation (which I think is a result of existing layering
violations ;) ).
I.e., it seems unexpected that "git repack" is going to tweak your
shallow lists. If we were designing from scratch, the sane behavior
seems to me to be:
1. Shallow pruning should be its own separate command (distinct from
either repacking or loose object pruning), and should be triggered
as part of a normal git-gc.
AND ONE OF:
2a. Objects mentions in the shallow file are important, and therefore
_are_ considered reachable on their own. Neither repack nor prune
needs to know or care.
OR
2b. It's OK for shallow objects to be missing, and the shallow code
should be more resilient to missing objects. Neither repack nor
prune needs to know or care.
I don't know how hard it would be to get there from the current code
state, though.
-Peff
