{"thread":{"id":"33334","subject":"check-attr doesn't respect recursive definitions","startedAt":"2013-03-30T09:45:51Z","lastAt":"2013-04-05T12:04:21Z","messageCount":13,"participants":["Jan Larres","Jeff King","Junio C Hamano","Duy Nguyen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"212699","messageId":"slrnkldd3g.1l4.jan@majutsushi.net","threadId":"33334","inReplyTo":null,"subject":"check-attr doesn't respect recursive definitions","fromName":"Jan Larres","fromEmail":"jan@majutsushi.net","sentAt":"2013-03-30T09:45:51Z","receivedAt":"2013-03-30T09:45:51Z","isPatch":false,"sender":{"key":"jan@majutsushi.net","avatar":null},"body":"Hi,\n\nI am trying to write a custom archiving script that checks the\nexport-ignore attribute to know which files from an ls-files output it\nshould skip. Through this I noticed that for files in directories for\nwhich the export-ignore (or any other) attribute is set, check-attr\nstill reports 'unspecified'. More precisely:\n\n$ git init test\nInitialized empty Git repository in /home/jan/test/.git/\n$ cd test\n$ mkdir foo\n$ touch foo/bar\n$ echo \"foo export-ignore\" > .gitattributes\n$ git check-attr export-ignore foo\nfoo: export-ignore: set\n$ git check-attr export-ignore foo/bar\nfoo/bar: export-ignore: unspecified\n\nI would expect the last command to also report 'set'. I've also tried\nother patterns like 'foo/' and 'foo*', but it didn't make any\ndifference. Is this expected behaviour? It does make checking the\nattributes of single files somewhat more difficult.\n\ngit-archive ignores the directory as expected, but unfortunately it\ndoesn't have an option to just list the files it would archive instead\nof actually archiving them.\n\nThis is with git version 1.7.10.4.\n\n-Jan\n"},{"id":"212881","messageId":"20130402143130.GC23828@sigill.intra.peff.net","threadId":"33334","inReplyTo":"slrnkldd3g.1l4.jan@majutsushi.net","subject":"Re: check-attr doesn't respect recursive definitions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-02T14:31:30Z","receivedAt":"2013-04-02T14:31:30Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 30, 2013 at 09:45:51AM +0000, Jan Larres wrote:\n\n> I am trying to write a custom archiving script that checks the\n> export-ignore attribute to know which files from an ls-files output it\n> should skip. Through this I noticed that for files in directories for\n> which the export-ignore (or any other) attribute is set, check-attr\n> still reports 'unspecified'. More precisely:\n> \n> $ git init test\n> Initialized empty Git repository in /home/jan/test/.git/\n> $ cd test\n> $ mkdir foo\n> $ touch foo/bar\n> $ echo \"foo export-ignore\" > .gitattributes\n> $ git check-attr export-ignore foo\n> foo: export-ignore: set\n> $ git check-attr export-ignore foo/bar\n> foo/bar: export-ignore: unspecified\n> \n> I would expect the last command to also report 'set'. I've also tried\n> other patterns like 'foo/' and 'foo*', but it didn't make any\n> difference. Is this expected behaviour? It does make checking the\n> attributes of single files somewhat more difficult.\n\nYes, it is the expected behavior, though I cannot offhand think of\nanything that would break if we did apply it recursively.\n\n> git-archive ignores the directory as expected, but unfortunately it\n> doesn't have an option to just list the files it would archive instead\n> of actually archiving them.\n\nYes, git-archive feeds the directories into the attribute machinery as\nit traverses the tree, so it actually checks for attributes of \"foo\"\nbefore recursing. You can do the same, but I agree it is quite a bit\nmore annoying than just piping \"ls-files -z\" into \"check-attr --stdin\n-z\".\n\n-Peff\n"},{"id":"212884","messageId":"20130402144814.GD23828@sigill.intra.peff.net","threadId":"33334","inReplyTo":"20130402143130.GC23828@sigill.intra.peff.net","subject":"Re: check-attr doesn't respect recursive definitions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-02T14:48:14Z","receivedAt":"2013-04-02T14:48:14Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 02, 2013 at 10:31:30AM -0400, Jeff King wrote:\n\n> On Sat, Mar 30, 2013 at 09:45:51AM +0000, Jan Larres wrote:\n> \n> > I am trying to write a custom archiving script that checks the\n> > export-ignore attribute to know which files from an ls-files output it\n> > should skip. Through this I noticed that for files in directories for\n> > which the export-ignore (or any other) attribute is set, check-attr\n> > still reports 'unspecified'. More precisely:\n> > \n> > $ git init test\n> > Initialized empty Git repository in /home/jan/test/.git/\n> > $ cd test\n> > $ mkdir foo\n> > $ touch foo/bar\n> > $ echo \"foo export-ignore\" > .gitattributes\n> > $ git check-attr export-ignore foo\n> > foo: export-ignore: set\n> > $ git check-attr export-ignore foo/bar\n> > foo/bar: export-ignore: unspecified\n> > \n> > I would expect the last command to also report 'set'. I've also tried\n> > other patterns like 'foo/' and 'foo*', but it didn't make any\n> > difference. Is this expected behaviour? It does make checking the\n> > attributes of single files somewhat more difficult.\n> \n> Yes, it is the expected behavior, though I cannot offhand think of\n> anything that would break if we did apply it recursively.\n\nNaively, such a patch might look something like this:\n\ndiff --git a/attr.c b/attr.c\nindex de170ff..ac04188 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -791,8 +791,18 @@ static void collect_all_attrs(const char *path)\n \t\tcheck_all_attr[i].value = ATTR__UNKNOWN;\n \n \trem = attr_nr;\n-\tfor (stk = attr_stack; 0 < rem && stk; stk = stk->prev)\n+\tfor (stk = attr_stack; 0 < rem && stk; stk = stk->prev) {\n+\t\tlast_slash = path;\n+\t\tfor (cp = path; *cp; cp++) {\n+\t\t\tif (*cp == '/') {\n+\t\t\t\trem = fill(path, cp - path + 1,\n+\t\t\t\t\t last_slash - path,\n+\t\t\t\t\t stk, rem);\n+\t\t\t\tlast_slash = cp;\n+\t\t\t}\n+\t\t}\n \t\trem = fill(path, pathlen, basename_offset, stk, rem);\n+\t}\n }\n \n int git_check_attr(const char *path, int num, struct git_attr_check *check)\n\nwhich is based on current \"next\" (because it has some other related\nfixes for handling directories). It seems to work for me, but I suspect\nwe could do it more optimally. If you have N files in \"foo/\", this will\ncheck \"foo/\" against the attribute stack N times, and we should be able\nto amortize that work.\n\nHmm. Actually, thinking on it more, I'm not sure this is even going to\ngive the right answer anyway, as it will check \"foo\" against the\n\"foo/.gitattributes\", which is not right. I think we'd need to collect\nattributes as we walk the stack in prepare_attr_stack.\n\nSo take this as a \"this is not how to do it\" patch. :)\n\n-Peff\n"},{"id":"212895","messageId":"7vtxnogbft.fsf@alter.siamese.dyndns.org","threadId":"33334","inReplyTo":"20130402143130.GC23828@sigill.intra.peff.net","subject":"Re: check-attr doesn't respect recursive definitions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-02T16:11:02Z","receivedAt":"2013-04-02T16:11:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Yes, it is the expected behavior, though I cannot offhand think of\n> anything that would break if we did apply it recursively.\n\nConceptually that breaks our brain. \"All files in doc/ directories\nare text\" and \"doc/ directory is text\" are two different things, no?\n"},{"id":"212898","messageId":"20130402163034.GA19252@sigill.intra.peff.net","threadId":"33334","inReplyTo":"7vtxnogbft.fsf@alter.siamese.dyndns.org","subject":"Re: check-attr doesn't respect recursive definitions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-02T16:30:34Z","receivedAt":"2013-04-02T16:30:34Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 02, 2013 at 09:11:02AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Yes, it is the expected behavior, though I cannot offhand think of\n> > anything that would break if we did apply it recursively.\n> \n> Conceptually that breaks our brain. \"All files in doc/ directories\n> are text\" and \"doc/ directory is text\" are two different things, no?\n\nIn some systems, yes, but git does not have any notion of \"doc/\" as an\nitem (after all, we track content in files, not directories), so I do\nnot see what it means to specify a directory except to say \"everything\nunder it has this property\".\n\nWe already treat \"foo/\" in gitignore this way (i.e., \"everything under\nfoo is ignored\"). And we treat export-ignore the same way in\ngit-archive.\n\nI agree that saying \"Documentation diff=text\" is not something I would\nexpect people to do, but what _should_ it do? Recursively applying under\nthe directory seems a sane option. I would not want to paint us into a\ncorner, but I cannot think of any other reasonable thing to do with it\n(which is why I phrased it as \"I cannot think of anything that would\nbreak\", and not \"this is a great idea\"; counterpoints are welcome).\n\nI am sympathetic to Jan's issue, though. If git were consistent about\napplying attributes only to paths, then the repo owner would have\nwritten \"dir/* export-ignore\", not \"dir\". But it is not, and git-archive\ndoes ask for attributes on directories, but without providing a good\nmechanism for doing those attribute lookups from external programs.\n\n-Peff\n"},{"id":"212901","messageId":"7vhajog9xp.fsf@alter.siamese.dyndns.org","threadId":"33334","inReplyTo":"20130402163034.GA19252@sigill.intra.peff.net","subject":"Re: check-attr doesn't respect recursive definitions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-02T16:43:30Z","receivedAt":"2013-04-02T16:43:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Apr 02, 2013 at 09:11:02AM -0700, Junio C Hamano wrote:\n>\n>> Jeff King writes:\n>> \n>> > Yes, it is the expected behavior, though I cannot offhand think of\n>> > anything that would break if we did apply it recursively.\n>> \n>> Conceptually that breaks our brain. \"All files in doc/ directories\n>> are text\" and \"doc/ directory is text\" are two different things, no?\n>\n> In some systems, yes, but git does not have any notion of \"doc/\" as an\n> item (after all, we track content in files, not directories), so I do\n> not see what it means to specify a directory except to say \"everything\n> under it has this property\".\n\nThat was true back when gitattributes (and ignore) was defined to\napply only to the paths we track. But export-ignore abuses the\nattrtibute system, allows a directory to be specified in the match\npattern, and we declared that is a kosher use by the patch that\ncaused 1.8.1.X regression, no? So \"Git does not have any notion of\n\"doc/' as an item\" is no longer true, I think.\n"},{"id":"212903","messageId":"20130402165128.GA19712@sigill.intra.peff.net","threadId":"33334","inReplyTo":"7vhajog9xp.fsf@alter.siamese.dyndns.org","subject":"Re: check-attr doesn't respect recursive definitions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-02T16:51:28Z","receivedAt":"2013-04-02T16:51:28Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 02, 2013 at 09:43:30AM -0700, Junio C Hamano wrote:\n\n> > In some systems, yes, but git does not have any notion of \"doc/\" as an\n> > item (after all, we track content in files, not directories), so I do\n> > not see what it means to specify a directory except to say \"everything\n> > under it has this property\".\n> \n> That was true back when gitattributes (and ignore) was defined to\n> apply only to the paths we track. But export-ignore abuses the\n> attrtibute system, allows a directory to be specified in the match\n> pattern, and we declared that is a kosher use by the patch that\n> caused 1.8.1.X regression, no? So \"Git does not have any notion of\n> \"doc/' as an item\" is no longer true, I think.\n\nYes, but as I explained later, the meaning of \"apply an attribute to\ndir\" in such cases is always equivalent to \"apply attribute\nrecursively to dir/*\". So I do not think we are violating that rule to\nrecursively apply all attributes.\n\nBut let's take a step back. I think Jan is trying to do a very\nreasonable thing: come up with the same set of paths that git-archive\nwould. What's the best way to solve that? Recursive application of\nattributes is one way, but is there another way we could help with\nsolving that?\n\nUsing:\n\n git ls-tree --name-only -zrt HEAD |\n git check-attr --stdin -z export-ignore\n\nmeans we can find out that \"foo/\" is ignored. But he would have to\nmanually post-process the output to see that \"foo/bar\" is below \"foo\".\nNot impossible, but I just wonder if git can be more helpful in figuring\nthis out.\n\n-Peff\n"},{"id":"212913","messageId":"20130402171557.GA24698@sigill.intra.peff.net","threadId":"33334","inReplyTo":"20130402165128.GA19712@sigill.intra.peff.net","subject":"Re: check-attr doesn't respect recursive definitions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-02T17:15:57Z","receivedAt":"2013-04-02T17:15:57Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 02, 2013 at 12:51:28PM -0400, Jeff King wrote:\n\n> But let's take a step back. I think Jan is trying to do a very\n> reasonable thing: come up with the same set of paths that git-archive\n> would. What's the best way to solve that? Recursive application of\n> attributes is one way, but is there another way we could help with\n> solving that?\n> \n> Using:\n> \n> git ls-tree --name-only -zrt HEAD |\n> git check-attr --stdin -z export-ignore\n> \n> means we can find out that \"foo/\" is ignored. But he would have to\n> manually post-process the output to see that \"foo/bar\" is below \"foo\".\n> Not impossible, but I just wonder if git can be more helpful in figuring\n> this out.\n\nOne way to solve the problem is something like the patch below, which\nallows \"git archive --format=lstree\" to give an lstree-like listing, but\nwith export-ignore attributes applied.\n\nIt feels weirdly specific, though, like there should be a more general\nsolution.\n\n---\n Makefile | 1 +\n archive-lstree.c | 43 +++++++++++++++++++++++++++++++++++\n archive.c | 1 +\n archive.h | 1 +\n quote.c | 4 ++--\n quote.h | 1 +\n 6 files changed, 49 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex e9749ca..5e63d72 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -772,6 +772,7 @@ LIB_OBJS += archive.o\n LIB_OBJS += alias.o\n LIB_OBJS += alloc.o\n LIB_OBJS += archive.o\n+LIB_OBJS += archive-lstree.o\n LIB_OBJS += archive-tar.o\n LIB_OBJS += archive-zip.o\n LIB_OBJS += argv-array.o\ndiff --git a/archive-lstree.c b/archive-lstree.c\nnew file mode 100644\nindex 0000000..5ca1bbc\n--- /dev/null\n+++ b/archive-lstree.c\n@@ -0,0 +1,43 @@\n+#include \"cache.h\"\n+#include \"archive.h\"\n+#include \"quote.h\"\n+#include \"commit.h\"\n+#include \"blob.h\"\n+\n+static int write_lstree_entry(struct archiver_args *args,\n+\t\t\t const unsigned char *sha1,\n+\t\t\t const char *path, size_t pathlen,\n+\t\t\t unsigned int mode)\n+{\n+\tconst char *type;\n+\n+\tif (S_ISDIR(mode))\n+\t\treturn 0;\n+\telse if (S_ISGITLINK(mode))\n+\t\ttype = commit_type;\n+\telse\n+\t\ttype = blob_type;\n+\n+\tprintf(\"%06o %s %s\\t\", mode, type, sha1_to_hex(sha1));\n+\tquote_c_style_counted(path, pathlen, NULL, stdout, 0);\n+\tprintf(\"\\n\");\n+\n+\treturn 0;\n+}\n+\n+static int write_lstree_archive(const struct archiver *ar,\n+\t\t\t\tstruct archiver_args *args)\n+{\n+\treturn write_archive_entries(args, write_lstree_entry);\n+}\n+\n+static struct archiver lstree_archiver = {\n+\t\"lstree\",\n+\twrite_lstree_archive,\n+\tARCHIVER_REMOTE\n+};\n+\n+void init_lstree_archiver(void)\n+{\n+\tregister_archiver(&lstree_archiver);\n+}\ndiff --git a/archive.c b/archive.c\nindex 3f00da6..4966db7 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -420,6 +420,7 @@ int write_archive(int argc, const char **argv, const char *prefix,\n \tgit_config(git_default_config, NULL);\n \tinit_tar_archiver();\n \tinit_zip_archiver();\n+\tinit_lstree_archiver();\n \n \targc = parse_archive_args(argc, argv, &ar, &args, name_hint, remote);\n \tif (nongit) {\ndiff --git a/archive.h b/archive.h\nindex 895afcd..1428fc4 100644\n--- a/archive.h\n+++ b/archive.h\n@@ -27,6 +27,7 @@ extern void init_zip_archiver(void);\n \n extern void init_tar_archiver(void);\n extern void init_zip_archiver(void);\n+extern void init_lstree_archiver(void);\n \n typedef int (*write_archive_entry_fn_t)(struct archiver_args *args,\n \t\t\t\t\tconst unsigned char *sha1,\ndiff --git a/quote.c b/quote.c\nindex 911229f..da6b7e4 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -206,8 +206,8 @@ static size_t next_quote_pos(const char *s, ssize_t maxlen)\n * of name, enclosed with double quotes if asked and needed only.\n * Return value is the same as in (1).\n */\n-static size_t quote_c_style_counted(const char *name, ssize_t maxlen,\n- struct strbuf *sb, FILE *fp, int no_dq)\n+size_t quote_c_style_counted(const char *name, ssize_t maxlen,\n+\t\t\t struct strbuf *sb, FILE *fp, int no_dq)\n {\n #undef EMIT\n #define EMIT(c) \\\ndiff --git a/quote.h b/quote.h\nindex 133155a..f2b8acb 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -55,6 +55,7 @@ extern size_t quote_c_style(const char *name, struct strbuf *, FILE *, int no_dq\n \n extern int unquote_c_style(struct strbuf *, const char *quoted, const char **endp);\n extern size_t quote_c_style(const char *name, struct strbuf *, FILE *, int no_dq);\n+extern size_t quote_c_style_counted(const char *name, ssize_t len, struct strbuf *, FILE *, int no_dq);\n extern void quote_two_c_style(struct strbuf *, const char *, const char *, int);\n \n extern void write_name_quoted(const char *name, FILE *, int terminator);\n"},{"id":"212914","messageId":"7v1uasg8e0.fsf@alter.siamese.dyndns.org","threadId":"33334","inReplyTo":"20130402165128.GA19712@sigill.intra.peff.net","subject":"Re: check-attr doesn't respect recursive definitions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-02T17:16:55Z","receivedAt":"2013-04-02T17:16:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Apr 02, 2013 at 09:43:30AM -0700, Junio C Hamano wrote:\n>\n>> > In some systems, yes, but git does not have any notion of \"doc/\" as an\n>> > item (after all, we track content in files, not directories), so I do\n>> > not see what it means to specify a directory except to say \"everything\n>> > under it has this property\".\n>> \n>> That was true back when gitattributes (and ignore) was defined to\n>> apply only to the paths we track. But export-ignore abuses the\n>> attrtibute system, allows a directory to be specified in the match\n>> pattern, and we declared that is a kosher use by the patch that\n>> caused 1.8.1.X regression, no? So \"Git does not have any notion of\n>> \"doc/' as an item\" is no longer true, I think.\n>\n> Yes, but as I explained later, the meaning of \"apply an attribute to\n> dir\" in such cases is always equivalent to \"apply attribute\n> recursively to dir/*\". So I do not think we are violating that rule to\n> recursively apply all attributes.\n\nI think this is where we disagree. Attribute does not recursively\napply in general. It was _never_ designed to (and that is the\nauthoritative answer, as I was who designed it in Apr 2007 ;-)).\n\nIt is not even true to say that \"archive\" applies export-ignore to\nthe directory recursively, with or without the recent change. Would\nit allow everything but dir/file to be excluded and still dir/file\nto be included in the archive if you have a .gitattribute file like\nthis?\n\n\tdir/ export-ignore\n dir/file !export-ignore\n\nI do not think so.\n\nIf we _were_ living in an alternate universe where we did not have\nthe .gitignore file and instead expressed what it does with an\n\"ignore\" attribute, then having \"dir\" in the top-level .gitignore\nfile and \"!file\" in the .gitignore file in \"dir/\" directory may be\nequivalent to having\n\n\tdir/ ignore\n dir/file -ignore\n\nin your .gitattribute, and we would want to ignore dir/other while\nincluding dir/file in the \"may be interesting\" paths.\n\nThe point is, yes, we could choose to define individual attribute\nkeys to apply recursively, but in general attributes were designed\nnot to recurse, and no existing attribute recurses.\n\n> But let's take a step back. I think Jan is trying to do a very\n> reasonable thing: come up with the same set of paths that git-archive\n> would. What's the best way to solve that?\n\nBecause the attribute does not recursively apply in general, and it\nis entirely up to the application and a particular attribute key to\ndecide how the key is applied in the context of the application,\n\"check-attr\" by itself cannot know. You need to know how \"archive\"\ntreats \"export-ignore\" attribute and then use \"check-attr\" with that\nknowledge.\n\n> Using:\n>\n> git ls-tree --name-only -zrt HEAD |\n> git check-attr --stdin -z export-ignore\n>\n> means we can find out that \"foo/\" is ignored.\n\nYes, and after learning that, we need to reject everything foo/*\nusing the knowledge that \"git archive\" cuts all subpaths of a path\nthat has \"export-ignore\" attribute is attached to.\n"},{"id":"212960","messageId":"20130402191641.GB17784@sigill.intra.peff.net","threadId":"33334","inReplyTo":"7v1uasg8e0.fsf@alter.siamese.dyndns.org","subject":"Re: check-attr doesn't respect recursive definitions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-02T19:16:41Z","receivedAt":"2013-04-02T19:16:41Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 02, 2013 at 10:16:55AM -0700, Junio C Hamano wrote:\n\n> > Yes, but as I explained later, the meaning of \"apply an attribute to\n> > dir\" in such cases is always equivalent to \"apply attribute\n> > recursively to dir/*\". So I do not think we are violating that rule to\n> > recursively apply all attributes.\n> \n> I think this is where we disagree. Attribute does not recursively\n> apply in general. It was _never_ designed to (and that is the\n> authoritative answer, as I was who designed it in Apr 2007 ;-)).\n\nMy argument was not that they apply recursively in general, but that we\nfollowed an \"as if\" rule where what happens in the gitignore and\nexport-ignore cases are indistinguishable from recursive application.\nBut...\n\n> It is not even true to say that \"archive\" applies export-ignore to\n> the directory recursively, with or without the recent change. Would\n> it allow everything but dir/file to be excluded and still dir/file\n> to be included in the archive if you have a .gitattribute file like\n> this?\n> \n> \tdir/ export-ignore\n> dir/file !export-ignore\n> \n> I do not think so.\n\nThis is a perfect counter-example. The current behavior _is_\ndistinguishable from recursive application, and my \"as if\" above is not\ncorrect.\n\nThanks for the clarification.\n\n> > But let's take a step back. I think Jan is trying to do a very\n> > reasonable thing: come up with the same set of paths that git-archive\n> > would. What's the best way to solve that?\n> \n> Because the attribute does not recursively apply in general, and it\n> is entirely up to the application and a particular attribute key to\n> decide how the key is applied in the context of the application,\n> \"check-attr\" by itself cannot know. You need to know how \"archive\"\n> treats \"export-ignore\" attribute and then use \"check-attr\" with that\n> knowledge.\n\nYeah, I had the same thought. So you would have to either:\n\n 1. Hook the feature into git-archive, which knows about how it\n recurses, and can report the correct set of paths.\n\nor\n\n 2. Tell check-attr (or some post-processor) to apply the attribute to\n elements below the path (or possibly to prune out such paths). This\n is not the same as recursive application, because you cannot negate\n it (i.e., you actually find out the final attrs for both \"foo\" and\n \"foo/bar\", and then say \"the attr for 'foo' overrides the attr for\n 'foo/bar'\".\n\nI posted a patch for (1), but it felt not-very-general. But (2) also\nfeels gross and not very general. Even though it could in theory be used\nfor things besides git-archive, it is really just applying git-archive's\npruning rule, which other programs likely don't care about.\n\n-Peff\n"},{"id":"213030","messageId":"slrnklnvo6.jdl.jan@majutsushi.net","threadId":"33334","inReplyTo":"20130402191641.GB17784@sigill.intra.peff.net","subject":"Re: check-attr doesn't respect recursive definitions","fromName":"Jan Larres","fromEmail":"jan@majutsushi.net","sentAt":"2013-04-03T10:05:24Z","receivedAt":"2013-04-03T10:05:24Z","isPatch":false,"sender":{"key":"jan@majutsushi.net","avatar":null},"body":"Thanks for the clarifications. Just a quick comment about the summary:\n\nJeff King wrote:\n> Yeah, I had the same thought. So you would have to either:\n>\n> 1. Hook the feature into git-archive, which knows about how it\n> recurses, and can report the correct set of paths.\n>\n> or\n>\n> 2. Tell check-attr (or some post-processor) to apply the attribute to\n> elements below the path (or possibly to prune out such paths). This\n> is not the same as recursive application, because you cannot negate\n> it (i.e., you actually find out the final attrs for both \"foo\" and\n> \"foo/bar\", and then say \"the attr for 'foo' overrides the attr for\n> 'foo/bar'\".\n>\n> I posted a patch for (1), but it felt not-very-general. But (2) also\n> feels gross and not very general. Even though it could in theory be used\n> for things besides git-archive, it is really just applying git-archive's\n> pruning rule, which other programs likely don't care about.\n\nI actually think the first approach is not such a bad idea, it would\nmake \"archive\" more of a general-purpose archiving tool/helper for the\nrepository rather than just something for the special cases of tars and\nzips. But I guess that's somewhat subjective.\n\nCheers,\nJan\n"},{"id":"213226","messageId":"CACsJy8B9s0=EFUazrb9MhDJiou_dSzx6kezFCET+w+jUdBMv=A@mail.gmail.com","threadId":"33334","inReplyTo":"slrnkldd3g.1l4.jan@majutsushi.net","subject":"Re: check-attr doesn't respect recursive definitions","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-04-05T02:09:07Z","receivedAt":"2013-04-05T02:09:07Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Mar 30, 2013 at 8:45 PM, Jan Larres wrote:\n> I would expect the last command to also report 'set'. I've also tried\n> other patterns like 'foo/' and 'foo*', but it didn't make any\n> difference.\n\nTry \"foo/**\". You need 1.8.2 though.\n--\nDuy\n"},{"id":"213366","messageId":"slrnkltff7.mdl.jan@majutsushi.net","threadId":"33334","inReplyTo":"CACsJy8B9s0=EFUazrb9MhDJiou_dSzx6kezFCET+w+jUdBMv=A@mail.gmail.com","subject":"Re: check-attr doesn't respect recursive definitions","fromName":"Jan Larres","fromEmail":"jan@majutsushi.net","sentAt":"2013-04-05T12:04:21Z","receivedAt":"2013-04-05T12:04:21Z","isPatch":false,"sender":{"key":"jan@majutsushi.net","avatar":null},"body":"Duy Nguyen wrote:\n> On Sat, Mar 30, 2013 at 8:45 PM, Jan Larres wrote:\n>> I would expect the last command to also report 'set'. I've also tried\n>> other patterns like 'foo/' and 'foo*', but it didn't make any\n>> difference.\n>\n> Try \"foo/**\". You need 1.8.2 though.\n\nThat indeed works exactly the way I would like! Thanks a lot!\n\nJan\n"}]}