Re: [PATCH v4 5/6] rev-list: add list-objects filtering support

From: Jonathan Tan <jonathantanmy@google.com>
Date: Nov 16, 2017, 20:43 UTC
Message-ID: <20171116124354.3d800cc27dbe3ea668c64519@google.com>
In-Reply-To: <20171116180743.61353-6-git@jeffhostetler.com>

On Thu, 16 Nov 2017 18:07:42 +0000 Jeff Hostetler <git@jeffhostetler.com> wrote:

Show 24 quoted lines

> From: Jeff Hostetler <jeffhost@microsoft.com>
> 
> Teach rev-list to use the filtering provided by the
> traverse_commit_list_filtered() interface to omit
> unwanted objects from the result.  This feature is
> intended to help with partial clone.
> 
> Object filtering is only allowed when one of the "--objects*"
> options are used.
> 
> When the "--filter-print-omitted" option is used, the omitted
> objects are printed at the end.  These are marked with a "~".
> This option can be combined with "--quiet" to get a list of
> just the omitted objects.
> 
> Added "--missing=(error|print|omit)" argument to specify how
> rev-list should behave when it encounters a missing object
> (presumably from a prior partial clone).
> 
> When "--missing=print" is used, rev-list will print a list of
> any missing objects that should have been included in the output.
> These are marked with a "?".
> 
> Add t6112 test.

The patch itself looks good, except that I have a nagging feeling about the usage of the term "partial clone" in the commit message, documentation, and test description. I feel that the usage here leads one to believe that partial clones haphazardly leave repositories without random objects (and at the point that this patch is merged, there will not be any patch in the main repo contradicting this viewpoint), contrary to the fact that we will have a tracking mechanism to track which missing objects are expected to be missing. (If I'm the only one feeling this way, though, then I'll just let it slide.)

If it were up to me, I would remove all existing mentions of "partial clone" and explain the presence of the "--missing" argument as follows:

    In the future, we will introduce a "partial clone" mechanism wherein
    an object in a repo, obtained from a remote, may reference a missing
    object that can be dynamically fetched from that remote once needed.
    This "partial clone" mechanism will have a way, sometimes slow, of
    determining if a missing link is one of the links expected to be
    produced by this mechanism.

    This patch introduces handling of missing objects to help debugging
    and development of the "partial clone" mechanism, and once the
    mechanism is implemented, for a power user to perform operations
    that are missing-object-aware without incurring the cost of checking
    if a missing link is expected.