From: Elijah Newren Date: Wed, 25 Aug 2010 23:57:06 GMT Subject: Re: [PATCH 5/7] diff_tree(): Skip skip_uninteresting() when all remaining paths interesting Message-ID: In-Reply-To: <7v1v9mp95o.fsf@alter.siamese.dyndns.org> On Wed, Aug 25, 2010 at 5:25 PM, Junio C Hamano 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.)