Re: [PATCH 5/7] diff_tree(): Skip skip_uninteresting() when all remaining paths interesting

On Wed, Aug 25, 2010 at 5:25 PM, Junio C Hamano [off-list ref] wrote:

This and the previous patch seem to make sense to me, independent of the
rest.

Thanks.

As to mixing pathspecs with --objects, I would say it is unfair to Linus
to call it a bug that that particular use case has not been supported.  I'd
say it _is_ a bug that we didn't error out when --objects was given with
pathspec at the command line level, though, as that is a combination we
knowingly ignored to support.

It simply hasn't been deemed as a sane operation to produce a pack using
an output from rev-list with pathspec and --objects, as it will leave some
trees and blobs that the pack knows about in the result, without actually
having them in it.  In the context of "narrow clone", these wounds to
trees are deliberate, and the existence of these wounds alone is not the
reason why I called it insane to produce such a pack.

To make use of such a pack, however, you need to somehow cauterize these
deliberate wounds in trees so that fsck, a later run of pack-objects,
fetch-pack and friends will not choke on them.  We didn't plan to have
such an infrastructure so far, and that is what made such a pack with
thousands of cuts "insane".  As soon as "narrow clone" addresses that
issue, mixture of pathspecs with --objects stops being an insane use
case.

Well, I did call it a minor bug, because I figured no one was using it
and so it wasn't hurting anyone.  But saying you knowingly ignored
supporting it is understandable.

I understand that for this combination of options to be useful, all
the other stuff you mention needs to be done.  Duy and I are both
working on separate ways of doing that; I just figured it made sense
to submit any independent pieces early and separately.  I don't think
supporting this would hurt anyone, as I don't think anyone could
accidentally use it at this point.  However, if you'd prefer that I
submitted a patch to just error out with this combination of options
for now, and resubmit this patch later with a full sparse clone
patchset, I can do that.  (Note though that my fix for supporting both
--objects and pathspec isn't correct; there's a bug I'm looking in
to.)