Re: [PATCH 2/2] repack -ad: prune the list of shallow commits

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux