Re: [PATCH 02/10] revision: learn '--no-kept-objects'

[Junio C Hamano <gitster@pobox.com>]

Taylor Blau <me@ttaylorr.com> writes:

> Some callers want to perform a reachability traversal that terminates
> when an object is found in a kept pack. The closest existing option is
> '--honor-pack-keep', but this isn't quite what we want. Instead of
> halting the traversal midway through, a full traversal is always
> performed, and the results are only trimmed afterwords.

True.  

Is there a reason to keep both kinds?  It is obvious that stopping
traversal once we hit a kept pack would be more time and space
efficient (I presume that the reason why .kept pack matters is
because we are repacking everything else) to enumerate the objects
that need to be repacked than traversing all the way and filtering
out objects that appear in .kept packs, but would there be some
correctness implications to replace the existing use of
"--honor-pack-keep" with "--no-kept-objects=on-disk"?  

What it means to be excluded by the former is quite clear: any
object that appears in a kept pack, whether another copy of it
appears elsewhere, is excluded from getting enumerated for
repacking.  It is quite unclear what it means to enumerate objects
with "--no-kept-objects".  It is clear from the implementation side
of the thing (stop traversal at objects that appear in any kept
pack), but it is totally unclear what such a meaning defined
operationally affects the resulting enumeration.  We know that the
enumerated objects do not appear in any of the kept pack, but it
does not mean all objects that are reachable/in-use that are not in
any kept packs are enumerated.

> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> index 002379056a..817419d552 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -856,6 +856,13 @@ ifdef::git-rev-list[]
>  	Only useful with `--objects`; print the object IDs that are not
>  	in packs.
>  
> +--no-kept-objects[=<kind>]::
> +	Halts the traversal as soon as an object in a kept pack is
> +	found. If `<kind>` is `on-disk`, only packs with a corresponding
> +	`*.keep` file are ignored. If `<kind>` is `in-core`, only packs
> +	with their in-core kept state set are ignored. Otherwise, both
> +	kinds of kept packs are ignored.

Is it explained anywhere how "in-core kept state" is bootstrapped,
modified and maintained?

The patch to C-part itself is a trivially correct implementation of
"stop at an object that can be found in a kept pack", and there is
no comment, but it is not clear to me what we want to achieve by
this.  Is the underlying assumption that no objects in .kept pack
would refer to outside world, either loose or packs that are not
kept?  How are we guaranteeing it?