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

From: Elijah Newren <newren@gmail.com>
Date: Aug 25, 2010, 23:57 UTC
Message-ID: <AANLkTi=Z0nZ7dPqZisPkbpGUBvoFbu9UVxkMq0hucGGg@mail.gmail.com>
In-Reply-To: <7v1v9mp95o.fsf@alter.siamese.dyndns.org>

On Wed, Aug 25, 2010 at 5:25 PM, Junio C Hamano <gitster@pobox.com> wrote:

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

Thanks.

Show 20 quoted lines

> 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.)