Re: check-attr doesn't respect recursive definitions

From: Jeff King <peff@peff.net>
Date: Apr 2, 2013, 19:16 UTC
Message-ID: <20130402191641.GB17784@sigill.intra.peff.net>
In-Reply-To: <7v1uasg8e0.fsf@alter.siamese.dyndns.org>

On Tue, Apr 02, 2013 at 10:16:55AM -0700, Junio C Hamano wrote:

Show 8 quoted lines

> > Yes, but as I explained later, the meaning of "apply an attribute to
> > dir" in such cases is always equivalent to "apply attribute
> > recursively to dir/*". So I do not think we are violating that rule to
> > recursively apply all attributes.
> 
> I think this is where we disagree.  Attribute does not recursively
> apply in general.  It was _never_ designed to (and that is the
> authoritative answer, as I was who designed it in Apr 2007 ;-)).

My argument was not that they apply recursively in general, but that we followed an "as if" rule where what happens in the gitignore and export-ignore cases are indistinguishable from recursive application. But...

Show 10 quoted lines

> It is not even true to say that "archive" applies export-ignore to
> the directory recursively, with or without the recent change.  Would
> it allow everything but dir/file to be excluded and still dir/file
> to be included in the archive if you have a .gitattribute file like
> this?
> 
> 	dir/ export-ignore
>         dir/file !export-ignore
> 
> I do not think so.

This is a perfect counter-example. The current behavior _is_ distinguishable from recursive application, and my "as if" above is not correct.

Thanks for the clarification.

Show 10 quoted lines

> > But let's take a step back. I think Jan is trying to do a very
> > reasonable thing: come up with the same set of paths that git-archive
> > would. What's the best way to solve that?
> 
> Because the attribute does not recursively apply in general, and it
> is entirely up to the application and a particular attribute key to
> decide how the key is applied in the context of the application,
> "check-attr" by itself cannot know.  You need to know how "archive"
> treats "export-ignore" attribute and then use "check-attr" with that
> knowledge.

Yeah, I had the same thought. So you would have to either:

  1. Hook the feature into git-archive, which knows about how it
     recurses, and can report the correct set of paths.

or

  2. Tell check-attr (or some post-processor) to apply the attribute to
     elements below the path (or possibly to prune out such paths). This
     is not the same as recursive application, because you cannot negate
     it (i.e., you actually find out the final attrs for both "foo" and
     "foo/bar", and then say "the attr for 'foo' overrides the attr for
     'foo/bar'".

I posted a patch for (1), but it felt not-very-general. But (2) also feels gross and not very general. Even though it could in theory be used for things besides git-archive, it is really just applying git-archive's pruning rule, which other programs likely don't care about.

-Peff