{"thread":{"id":"46580","subject":"Bug?: git archive exclude pathspec and gitattributes export-ignore","startedAt":"2017-08-13T05:04:09Z","lastAt":"2017-09-13T14:56:50Z","messageCount":17,"participants":["David Adam","René Scharfe","Junio C Hamano","Jeff King","Stefan Beller"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"326233","messageId":"alpine.DEB.2.11.1708131240360.15538@motsugo.ucc.gu.uwa.edu.au","threadId":"46580","inReplyTo":null,"subject":"Bug?: git archive exclude pathspec and gitattributes export-ignore","fromName":"David Adam","fromEmail":"zanchey@ucc.gu.uwa.edu.au","sentAt":"2017-08-13T04:53:58Z","receivedAt":"2017-08-13T05:04:09Z","isPatch":false,"sender":{"key":"zanchey@ucc.gu.uwa.edu.au","avatar":null},"body":"Hi all,\n\nI think I have a bug in git (tested 2.11.0 on Debian 8, 2.14.1 on OS X and \n2.14.1.145.gb3622a4 on OS X).\n\nGiven a repository with an export-ignore directive for a subdirectory in \n.gitattributes, `git archive` with a pathspec that excludes a different \nsubdirectory produces no output file and git exits with -1 as the return \nstatus.\n\nAs shown:\n\n   > git init foo && cd foo\n   Initialized empty Git repository in /Users/david/src/foo/.git/\n   > mkdir a b\n   > touch {a,b}/somefile\n   > echo \"/a export-ignore\" >> .gitattributes\n   > git add .\n   > git commit -m \"Initial commit\"\n   [master (root-commit) 53527a7] Initial commit\n    3 files changed, 1 insertion(+)\n    create mode 100644 .gitattributes\n    create mode 100644 a/somefile\n    create mode 100644 b/somefile\n   > git archive --verbose master ':(top)' ':(exclude)b*'\n   .gitattributes\n   > echo $?\n   255\n\nIf this is intended behaviour, is there any way of achieving the goal of \nexcluding a subdirectory not listed as export-ignore? Using the exclude \npathspec \":(exclude)b\" produces an empty subdirectory b in the output, \nwhich I would like to avoid.\n\nThis is a reduced testcase; my goal is to end up with two archives, one \ncontaining directory b only, and one containing everything except for \ndirectory b - so I can't just add 'b export-ignore' to gitattributes.\n\nThanks\n\nDavid Adam\nzanchey@ucc.gu.uwa.edu.au\n"},{"id":"326274","messageId":"ae893c19-652d-1c8f-50ba-1242b95be84e@web.de","threadId":"46580","inReplyTo":"alpine.DEB.2.11.1708131240360.15538@motsugo.ucc.gu.uwa.edu.au","subject":"Re: Bug?: git archive exclude pathspec and gitattributes export-ignore","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-08-14T16:43:46Z","receivedAt":"2017-08-14T16:44:16Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 13.08.2017 um 06:53 schrieb David Adam:\n> Hi all,\n> \n> I think I have a bug in git (tested 2.11.0 on Debian 8, 2.14.1 on OS X and\n> 2.14.1.145.gb3622a4 on OS X).\n> \n> Given a repository with an export-ignore directive for a subdirectory in\n> .gitattributes, `git archive` with a pathspec that excludes a different\n> subdirectory produces no output file and git exits with -1 as the return\n> status.\n> \n> As shown:\n> \n>     > git init foo && cd foo\n>     Initialized empty Git repository in /Users/david/src/foo/.git/\n>     > mkdir a b\n>     > touch {a,b}/somefile\n>     > echo \"/a export-ignore\" >> .gitattributes\n>     > git add .\n>     > git commit -m \"Initial commit\"\n>     [master (root-commit) 53527a7] Initial commit\n>      3 files changed, 1 insertion(+)\n>      create mode 100644 .gitattributes\n>      create mode 100644 a/somefile\n>      create mode 100644 b/somefile\n>     > git archive --verbose master ':(top)' ':(exclude)b*'\n>     .gitattributes\n>     > echo $?\n>     255\n> \n> If this is intended behaviour, is there any way of achieving the goal of\n> excluding a subdirectory not listed as export-ignore? Using the exclude\n> pathspec \":(exclude)b\" produces an empty subdirectory b in the output,\n> which I would like to avoid.\n> \n> This is a reduced testcase; my goal is to end up with two archives, one\n> containing directory b only, and one containing everything except for\n> directory b - so I can't just add 'b export-ignore' to gitattributes.\n\nThanks for the thoughtful bug report!\n\nThe problem seems to be that archive.c::write_archive_entry() returns 0\ninstead of READ_TREE_RECURSIVE for directories with the attribute\n\"export-ignore\", and archive.c::write_directory() gets caught by\nsurprise by that and returns -1, which ends up causing git archive to\nexit with return code 255 without actually writing anything.\n\nThis should only happen if you use wildcards like \"*\", i.e. git archive\nshould behave as expected if you spell out the full name of the\ndirectory.  Can you confirm that?\n\nThe real solution is probably to teach tree-walk.c::do_match() how to\nhandle attributes and then inject \":(attr:-export-ignore)\" as a default\ninternal pathspec in archive.c::parse_pathspec_arg() instead of handling\nit in archive.c::write_archive_entry().\n\n@Duy: What do you think?\n\nThanks,\nRené\n"},{"id":"326360","messageId":"alpine.DEB.2.11.1708151027350.9062@motsugo.ucc.gu.uwa.edu.au","threadId":"46580","inReplyTo":"ae893c19-652d-1c8f-50ba-1242b95be84e@web.de","subject":"Re: Bug?: git archive exclude pathspec and gitattributes export-ignore","fromName":"David Adam","fromEmail":"zanchey@ucc.gu.uwa.edu.au","sentAt":"2017-08-15T02:39:29Z","receivedAt":"2017-08-15T02:40:07Z","isPatch":false,"sender":{"key":"zanchey@ucc.gu.uwa.edu.au","avatar":null},"body":"On Mon, 14 Aug 2017, René Scharfe wrote:\n> Am 13.08.2017 um 06:53 schrieb David Adam:\n> > I think I have a bug in git (tested 2.11.0 on Debian 8, 2.14.1 on OS X and\n> > 2.14.1.145.gb3622a4 on OS X).\n> > \n> > Given a repository with an export-ignore directive for a subdirectory in\n> > .gitattributes, `git archive` with a pathspec that excludes a different\n> > subdirectory produces no output file and git exits with -1 as the return\n> > status.\n> \n> Thanks for the thoughtful bug report!\n> \n> The problem seems to be that archive.c::write_archive_entry() returns 0\n> instead of READ_TREE_RECURSIVE for directories with the attribute\n> \"export-ignore\", and archive.c::write_directory() gets caught by\n> surprise by that and returns -1, which ends up causing git archive to\n> exit with return code 255 without actually writing anything.\n> \n> This should only happen if you use wildcards like \"*\", i.e. git archive\n> should behave as expected if you spell out the full name of the\n> directory.  Can you confirm that?\n\nYes - that's definitely the case.\n\nThe reason I am trying to use the wildcard is that using an \":(exclude)b\" \npathspec excludes the contents of, but not the actual b directory \nitself:\n\n    > git archive HEAD ':(top)' ':(exclude)b' | tar t\n    .gitattributes\n    b/\n\nWhereas I would like to export the archive without the b directory \nentirely.\n\n> The real solution is probably to teach tree-walk.c::do_match() how to\n> handle attributes and then inject \":(attr:-export-ignore)\" as a default\n> internal pathspec in archive.c::parse_pathspec_arg() instead of handling\n> it in archive.c::write_archive_entry().\n\nMany thanks\n\nDavid Adam\nzanchey@ucc.gu.uwa.edu.au"},{"id":"326754","messageId":"887652a4-3f03-e2dd-2c68-cff4f7194898@web.de","threadId":"46580","inReplyTo":"ae893c19-652d-1c8f-50ba-1242b95be84e@web.de","subject":"Re: Bug?: git archive exclude pathspec and gitattributes export-ignore","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-08-19T05:26:34Z","receivedAt":"2017-08-19T05:26:59Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 14.08.2017 um 18:43 schrieb René Scharfe:\n> The real solution is probably to teach tree-walk.c::do_match() how to\n> handle attributes and then inject \":(attr:-export-ignore)\" as a default\n> internal pathspec in archive.c::parse_pathspec_arg() instead of handling\n> it in archive.c::write_archive_entry().\n\nThat's complicated and I'm not sure anymore if it's even a good idea.\nLet's solve this in git archive for now.\n\n  t5001: add tests for export-ignore attributes and exclude pathspecs\n  archive: factor out helper functions for handling attributes\n  archive: don't queue excluded directories\n  archive: queue directories for all types of pathspecs\n\n archive.c               | 49 +++++++++++++++++++++++++++++++++++++++++--------\n t/t5001-archive-attr.sh | 47 ++++++++++++++++++++++++++++++++++++++++++++---\n 2 files changed, 85 insertions(+), 11 deletions(-)\n\n-- \n2.14.1\n"},{"id":"326755","messageId":"f65809c4-ce94-559a-855a-bbd9b3b1c936@web.de","threadId":"46580","inReplyTo":"887652a4-3f03-e2dd-2c68-cff4f7194898@web.de","subject":"[PATCH 1/4] t5001: add tests for export-ignore attributes and exclude pathspecs","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-08-19T05:28:54Z","receivedAt":"2017-08-19T05:29:27Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Demonstrate mishandling of the attribute export-ignore by git archive\nwhen used together with pathspecs.  Wildcard pathspecs can even cause it\nto abort.  And a directory excluded without a wildcard is still included\nas an empty folder in the archive.\n\nTest-case-by: David Adam <zanchey@ucc.gu.uwa.edu.au>\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n t/t5001-archive-attr.sh | 47 ++++++++++++++++++++++++++++++++++++++++++++---\n 1 file changed, 44 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t5001-archive-attr.sh b/t/t5001-archive-attr.sh\nindex b04d955bfa..063622bc71 100755\n--- a/t/t5001-archive-attr.sh\n+++ b/t/t5001-archive-attr.sh\n@@ -7,11 +7,15 @@ test_description='git archive attribute tests'\n SUBSTFORMAT='%H (%h)%n'\n \n test_expect_exists() {\n-\ttest_expect_success \" $1 exists\" \"test -e $1\"\n+\ttest_expect_${2:-success} \" $1 exists\" \"test -e $1\"\n }\n \n test_expect_missing() {\n-\ttest_expect_success \" $1 does not exist\" \"test ! -e $1\"\n+\ttest_expect_${2:-success} \" $1 does not exist\" \"test ! -e $1\"\n+}\n+\n+extract_tar_to_dir () {\n+\t(mkdir \"$1\" && cd \"$1\" && \"$TAR\" xf -) <\"$1.tar\"\n }\n \n test_expect_success 'setup' '\n@@ -21,12 +25,19 @@ test_expect_success 'setup' '\n \n \techo ignored by tree >ignored-by-tree &&\n \techo ignored-by-tree export-ignore >.gitattributes &&\n-\tgit add ignored-by-tree .gitattributes &&\n+\tmkdir ignored-by-tree.d &&\n+\t>ignored-by-tree.d/file &&\n+\techo ignored-by-tree.d export-ignore >>.gitattributes &&\n+\tgit add ignored-by-tree ignored-by-tree.d .gitattributes &&\n \n \techo ignored by worktree >ignored-by-worktree &&\n \techo ignored-by-worktree export-ignore >.gitattributes &&\n \tgit add ignored-by-worktree &&\n \n+\tmkdir excluded-by-pathspec.d &&\n+\t>excluded-by-pathspec.d/file &&\n+\tgit add excluded-by-pathspec.d &&\n+\n \tprintf \"A\\$Format:%s\\$O\" \"$SUBSTFORMAT\" >nosubstfile &&\n \tprintf \"A\\$Format:%s\\$O\" \"$SUBSTFORMAT\" >substfile1 &&\n \tprintf \"A not substituted O\" >substfile2 &&\n@@ -46,7 +57,37 @@ test_expect_success 'git archive' '\n \n test_expect_missing\tarchive/ignored\n test_expect_missing\tarchive/ignored-by-tree\n+test_expect_missing\tarchive/ignored-by-tree.d\n+test_expect_missing\tarchive/ignored-by-tree.d/file\n test_expect_exists\tarchive/ignored-by-worktree\n+test_expect_exists\tarchive/excluded-by-pathspec.d\n+test_expect_exists\tarchive/excluded-by-pathspec.d/file\n+\n+test_expect_success 'git archive with pathspec' '\n+\tgit archive HEAD \":!excluded-by-pathspec.d\" >archive-pathspec.tar &&\n+\textract_tar_to_dir archive-pathspec\n+'\n+\n+test_expect_missing\tarchive-pathspec/ignored\n+test_expect_missing\tarchive-pathspec/ignored-by-tree\n+test_expect_missing\tarchive-pathspec/ignored-by-tree.d\n+test_expect_missing\tarchive-pathspec/ignored-by-tree.d/file\n+test_expect_exists\tarchive-pathspec/ignored-by-worktree\n+test_expect_missing\tarchive-pathspec/excluded-by-pathspec.d failure\n+test_expect_missing\tarchive-pathspec/excluded-by-pathspec.d/file\n+\n+test_expect_failure 'git archive with wildcard pathspec' '\n+\tgit archive HEAD \":!excluded-by-p*\" >archive-pathspec-wildcard.tar &&\n+\textract_tar_to_dir archive-pathspec-wildcard\n+'\n+\n+test_expect_missing\tarchive-pathspec-wildcard/ignored\n+test_expect_missing\tarchive-pathspec-wildcard/ignored-by-tree\n+test_expect_missing\tarchive-pathspec-wildcard/ignored-by-tree.d\n+test_expect_missing\tarchive-pathspec-wildcard/ignored-by-tree.d/file\n+test_expect_exists\tarchive-pathspec-wildcard/ignored-by-worktree failure\n+test_expect_missing\tarchive-pathspec-wildcard/excluded-by-pathspec.d\n+test_expect_missing\tarchive-pathspec-wildcard/excluded-by-pathspec.d/file\n \n test_expect_success 'git archive with worktree attributes' '\n \tgit archive --worktree-attributes HEAD >worktree.tar &&\n-- \n2.14.1\n"},{"id":"326756","messageId":"aaf9c857-76b9-5766-59ce-cffc1b65015f@web.de","threadId":"46580","inReplyTo":"887652a4-3f03-e2dd-2c68-cff4f7194898@web.de","subject":"[PATCH 2/4] archive: factor out helper functions for handling attributes","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-08-19T05:29:43Z","receivedAt":"2017-08-19T05:30:02Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Add helpers for accessing attributes that encapsulate the details of how\nto retrieve their values.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n archive.c | 31 +++++++++++++++++++++++--------\n 1 file changed, 23 insertions(+), 8 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 557dd2db85..8e5f632912 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -103,12 +103,30 @@ struct archiver_context {\n \tstruct directory *bottom;\n };\n \n+static const struct attr_check *get_archive_attrs(const char *path)\n+{\n+\tstatic struct attr_check *check;\n+\tif (!check)\n+\t\tcheck = attr_check_initl(\"export-ignore\", \"export-subst\", NULL);\n+\treturn git_check_attr(path, check) ? NULL : check;\n+}\n+\n+static int check_attr_export_ignore(const struct attr_check *check)\n+{\n+\treturn check && ATTR_TRUE(check->items[0].value);\n+}\n+\n+static int check_attr_export_subst(const struct attr_check *check)\n+{\n+\treturn check && ATTR_TRUE(check->items[1].value);\n+}\n+\n static int write_archive_entry(const unsigned char *sha1, const char *base,\n \t\tint baselen, const char *filename, unsigned mode, int stage,\n \t\tvoid *context)\n {\n \tstatic struct strbuf path = STRBUF_INIT;\n-\tstatic struct attr_check *check;\n+\tconst struct attr_check *check;\n \tstruct archiver_context *c = context;\n \tstruct archiver_args *args = c->args;\n \twrite_archive_entry_fn_t write_entry = c->write_entry;\n@@ -125,13 +143,10 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,\n \t\tstrbuf_addch(&path, '/');\n \tpath_without_prefix = path.buf + args->baselen;\n \n-\tif (!check)\n-\t\tcheck = attr_check_initl(\"export-ignore\", \"export-subst\", NULL);\n-\tif (!git_check_attr(path_without_prefix, check)) {\n-\t\tif (ATTR_TRUE(check->items[0].value))\n-\t\t\treturn 0;\n-\t\targs->convert = ATTR_TRUE(check->items[1].value);\n-\t}\n+\tcheck = get_archive_attrs(path_without_prefix);\n+\tif (check_attr_export_ignore(check))\n+\t\treturn 0;\n+\targs->convert = check_attr_export_subst(check);\n \n \tif (S_ISDIR(mode) || S_ISGITLINK(mode)) {\n \t\tif (args->verbose)\n-- \n2.14.1\n"},{"id":"326757","messageId":"0432c7cc-a2a5-12bd-bf65-a16186313d5b@web.de","threadId":"46580","inReplyTo":"887652a4-3f03-e2dd-2c68-cff4f7194898@web.de","subject":"[PATCH 4/4] archive: queue directories for all types of pathspecs","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-08-19T05:33:50Z","receivedAt":"2017-08-19T05:35:55Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"When read_tree_recursive() encounters a directory excluded by a pathspec\nthen it enters it anyway because it might contain included entries.  It\ncalls the callback function before it is able to decide if the directory\nis actually needed.\n\nFor that reason git archive adds directories to a queue and writes\nentries for them only when it encounters the first child item -- but\nonly if pathspecs with wildcards are used.  Do the same for literal\npathspecs as well, as the reasoning above applies to them, too.  This\nprevents git archive from writing entries for excluded directories.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n archive.c               | 2 +-\n t/t5001-archive-attr.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 1ab8d3a1d7..174c0555b6 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -123,7 +123,7 @@ static int check_attr_export_subst(const struct attr_check *check)\n \n static int should_queue_directories(const struct archiver_args *args)\n {\n-\treturn args->pathspec.has_wildcard;\n+\treturn args->pathspec.nr;\n }\n \n static int write_archive_entry(const unsigned char *sha1, const char *base,\ndiff --git a/t/t5001-archive-attr.sh b/t/t5001-archive-attr.sh\nindex 897f6f06d5..e9aa97117a 100755\n--- a/t/t5001-archive-attr.sh\n+++ b/t/t5001-archive-attr.sh\n@@ -73,7 +73,7 @@ test_expect_missing\tarchive-pathspec/ignored-by-tree\n test_expect_missing\tarchive-pathspec/ignored-by-tree.d\n test_expect_missing\tarchive-pathspec/ignored-by-tree.d/file\n test_expect_exists\tarchive-pathspec/ignored-by-worktree\n-test_expect_missing\tarchive-pathspec/excluded-by-pathspec.d failure\n+test_expect_missing\tarchive-pathspec/excluded-by-pathspec.d\n test_expect_missing\tarchive-pathspec/excluded-by-pathspec.d/file\n \n test_expect_success 'git archive with wildcard pathspec' '\n-- \n2.14.1\n"},{"id":"326758","messageId":"781919c3-1656-46e1-5fb6-87494e2a449a@web.de","threadId":"46580","inReplyTo":"887652a4-3f03-e2dd-2c68-cff4f7194898@web.de","subject":"[PATCH 3/4] archive: don't queue excluded directories","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-08-19T05:32:37Z","receivedAt":"2017-08-19T05:37:59Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Reject directories with the attribute export-ignore already while\nqueuing them.  This prevents read_tree_recursive() from descending into\nthem and this avoids write_archive_entry() rejecting them later on,\nwhich queue_or_write_archive_entry() is not prepared for.\n\nBorrow the existing strbuf to build the full path to avoid string\ncopies and extra allocations; just make sure we restore the original\nvalue before moving on.\n\nKeep checking any other attributes in write_archive_entry() as before,\nbut avoid checking them twice.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n archive.c               | 32 +++++++++++++++++++++++++-------\n t/t5001-archive-attr.sh |  4 ++--\n 2 files changed, 27 insertions(+), 9 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 8e5f632912..1ab8d3a1d7 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -121,17 +121,21 @@ static int check_attr_export_subst(const struct attr_check *check)\n \treturn check && ATTR_TRUE(check->items[1].value);\n }\n \n+static int should_queue_directories(const struct archiver_args *args)\n+{\n+\treturn args->pathspec.has_wildcard;\n+}\n+\n static int write_archive_entry(const unsigned char *sha1, const char *base,\n \t\tint baselen, const char *filename, unsigned mode, int stage,\n \t\tvoid *context)\n {\n \tstatic struct strbuf path = STRBUF_INIT;\n-\tconst struct attr_check *check;\n \tstruct archiver_context *c = context;\n \tstruct archiver_args *args = c->args;\n \twrite_archive_entry_fn_t write_entry = c->write_entry;\n-\tconst char *path_without_prefix;\n \tint err;\n+\tconst char *path_without_prefix;\n \n \targs->convert = 0;\n \tstrbuf_reset(&path);\n@@ -143,10 +147,13 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,\n \t\tstrbuf_addch(&path, '/');\n \tpath_without_prefix = path.buf + args->baselen;\n \n-\tcheck = get_archive_attrs(path_without_prefix);\n-\tif (check_attr_export_ignore(check))\n-\t\treturn 0;\n-\targs->convert = check_attr_export_subst(check);\n+\tif (!S_ISDIR(mode) || !should_queue_directories(args)) {\n+\t\tconst struct attr_check *check;\n+\t\tcheck = get_archive_attrs(path_without_prefix);\n+\t\tif (check_attr_export_ignore(check))\n+\t\t\treturn 0;\n+\t\targs->convert = check_attr_export_subst(check);\n+\t}\n \n \tif (S_ISDIR(mode) || S_ISGITLINK(mode)) {\n \t\tif (args->verbose)\n@@ -219,6 +226,17 @@ static int queue_or_write_archive_entry(const unsigned char *sha1,\n \t}\n \n \tif (S_ISDIR(mode)) {\n+\t\tsize_t baselen = base->len;\n+\t\tconst struct attr_check *check;\n+\n+\t\t/* Borrow base, but restore its original value when done. */\n+\t\tstrbuf_addstr(base, filename);\n+\t\tstrbuf_addch(base, '/');\n+\t\tcheck = get_archive_attrs(base->buf);\n+\t\tstrbuf_setlen(base, baselen);\n+\n+\t\tif (check_attr_export_ignore(check))\n+\t\t\treturn 0;\n \t\tqueue_directory(sha1, base, filename,\n \t\t\t\tmode, stage, c);\n \t\treturn READ_TREE_RECURSIVE;\n@@ -272,7 +290,7 @@ int write_archive_entries(struct archiver_args *args,\n \t}\n \n \terr = read_tree_recursive(args->tree, \"\", 0, 0, &args->pathspec,\n-\t\t\t\t  args->pathspec.has_wildcard ?\n+\t\t\t\t  should_queue_directories(args) ?\n \t\t\t\t  queue_or_write_archive_entry :\n \t\t\t\t  write_archive_entry_buf,\n \t\t\t\t  &context);\ndiff --git a/t/t5001-archive-attr.sh b/t/t5001-archive-attr.sh\nindex 063622bc71..897f6f06d5 100755\n--- a/t/t5001-archive-attr.sh\n+++ b/t/t5001-archive-attr.sh\n@@ -76,7 +76,7 @@ test_expect_exists\tarchive-pathspec/ignored-by-worktree\n test_expect_missing\tarchive-pathspec/excluded-by-pathspec.d failure\n test_expect_missing\tarchive-pathspec/excluded-by-pathspec.d/file\n \n-test_expect_failure 'git archive with wildcard pathspec' '\n+test_expect_success 'git archive with wildcard pathspec' '\n \tgit archive HEAD \":!excluded-by-p*\" >archive-pathspec-wildcard.tar &&\n \textract_tar_to_dir archive-pathspec-wildcard\n '\n@@ -85,7 +85,7 @@ test_expect_missing\tarchive-pathspec-wildcard/ignored\n test_expect_missing\tarchive-pathspec-wildcard/ignored-by-tree\n test_expect_missing\tarchive-pathspec-wildcard/ignored-by-tree.d\n test_expect_missing\tarchive-pathspec-wildcard/ignored-by-tree.d/file\n-test_expect_exists\tarchive-pathspec-wildcard/ignored-by-worktree failure\n+test_expect_exists\tarchive-pathspec-wildcard/ignored-by-worktree\n test_expect_missing\tarchive-pathspec-wildcard/excluded-by-pathspec.d\n test_expect_missing\tarchive-pathspec-wildcard/excluded-by-pathspec.d/file\n \n-- \n2.14.1\n"},{"id":"326775","messageId":"33fa4f08-8f06-5a98-e492-3f05cc742555@web.de","threadId":"46580","inReplyTo":"0432c7cc-a2a5-12bd-bf65-a16186313d5b@web.de","subject":"Re: [PATCH 4/4] archive: queue directories for all types of pathspecs","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-08-19T16:53:26Z","receivedAt":"2017-08-19T16:54:03Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 19.08.2017 um 07:33 schrieb René Scharfe:\n> When read_tree_recursive() encounters a directory excluded by a pathspec\n> then it enters it anyway because it might contain included entries.  It\n> calls the callback function before it is able to decide if the directory\n> is actually needed.\n> \n> For that reason git archive adds directories to a queue and writes\n> entries for them only when it encounters the first child item -- but\n> only if pathspecs with wildcards are used.  Do the same for literal\n> pathspecs as well, as the reasoning above applies to them, too.  This\n> prevents git archive from writing entries for excluded directories.\n\nThis breaks the test \"archive empty subtree with no pathspec\" in t5004 by\nomitting the empty directory from the archive.  Sorry for missing that!\n\nThis is kind of a bonus patch, so please discard it for now; the first\nthree are OK IMHO.\n\nA better version of this patch would at least update t5004 as well, but\nthere might be a better way.\n\n> \n> Signed-off-by: Rene Scharfe <l.s.r@web.de>\n> ---\n>   archive.c               | 2 +-\n>   t/t5001-archive-attr.sh | 2 +-\n>   2 files changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/archive.c b/archive.c\n> index 1ab8d3a1d7..174c0555b6 100644\n> --- a/archive.c\n> +++ b/archive.c\n> @@ -123,7 +123,7 @@ static int check_attr_export_subst(const struct attr_check *check)\n>   \n>   static int should_queue_directories(const struct archiver_args *args)\n>   {\n> -\treturn args->pathspec.has_wildcard;\n> +\treturn args->pathspec.nr;\n>   }\n>   \n>   static int write_archive_entry(const unsigned char *sha1, const char *base,\n> diff --git a/t/t5001-archive-attr.sh b/t/t5001-archive-attr.sh\n> index 897f6f06d5..e9aa97117a 100755\n> --- a/t/t5001-archive-attr.sh\n> +++ b/t/t5001-archive-attr.sh\n> @@ -73,7 +73,7 @@ test_expect_missing\tarchive-pathspec/ignored-by-tree\n>   test_expect_missing\tarchive-pathspec/ignored-by-tree.d\n>   test_expect_missing\tarchive-pathspec/ignored-by-tree.d/file\n>   test_expect_exists\tarchive-pathspec/ignored-by-worktree\n> -test_expect_missing\tarchive-pathspec/excluded-by-pathspec.d failure\n> +test_expect_missing\tarchive-pathspec/excluded-by-pathspec.d\n>   test_expect_missing\tarchive-pathspec/excluded-by-pathspec.d/file\n>   \n>   test_expect_success 'git archive with wildcard pathspec' '\n> \n"},{"id":"326776","messageId":"xmqq378na40x.fsf@gitster.mtv.corp.google.com","threadId":"46580","inReplyTo":"0432c7cc-a2a5-12bd-bf65-a16186313d5b@web.de","subject":"Re: [PATCH 4/4] archive: queue directories for all types of pathspecs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-19T16:58:06Z","receivedAt":"2017-08-19T16:58:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> When read_tree_recursive() encounters a directory excluded by a pathspec\n> then it enters it anyway because it might contain included entries.  It\n> calls the callback function before it is able to decide if the directory\n> is actually needed.\n>\n> For that reason git archive adds directories to a queue and writes\n> entries for them only when it encounters the first child item -- but\n> only if pathspecs with wildcards are used.  Do the same for literal\n> pathspecs as well, as the reasoning above applies to them, too.  This\n> prevents git archive from writing entries for excluded directories.\n>\n> Signed-off-by: Rene Scharfe <l.s.r@web.de>\n> ---\n>  archive.c               | 2 +-\n>  t/t5001-archive-attr.sh | 2 +-\n>  2 files changed, 2 insertions(+), 2 deletions(-)\n\nThis seems to break t/t5004-archive-corner-cases.sh though...\n\n    expecting success:\n            git archive --format=tar $root_tree -- sub >subtree-path.tar &&\n            make_dir extract &&\n            \"$TAR\" xf subtree-path.tar -C extract &&\n            check_dir extract sub\n\n    --- expect      2017-08-19 16:56:49.761513537 +0000\n    +++ actual      2017-08-19 16:56:49.769513535 +0000\n    @@ -1,2 +1 @@\n     extract\n    -extract/sub\n    not ok 10 - archive empty subtree by direct pathspec\n\n> diff --git a/archive.c b/archive.c\n> index 1ab8d3a1d7..174c0555b6 100644\n> --- a/archive.c\n> +++ b/archive.c\n> @@ -123,7 +123,7 @@ static int check_attr_export_subst(const struct attr_check *check)\n>  \n>  static int should_queue_directories(const struct archiver_args *args)\n>  {\n> -\treturn args->pathspec.has_wildcard;\n> +\treturn args->pathspec.nr;\n>  }\n>  \n>  static int write_archive_entry(const unsigned char *sha1, const char *base,\n> diff --git a/t/t5001-archive-attr.sh b/t/t5001-archive-attr.sh\n> index 897f6f06d5..e9aa97117a 100755\n> --- a/t/t5001-archive-attr.sh\n> +++ b/t/t5001-archive-attr.sh\n> @@ -73,7 +73,7 @@ test_expect_missing\tarchive-pathspec/ignored-by-tree\n>  test_expect_missing\tarchive-pathspec/ignored-by-tree.d\n>  test_expect_missing\tarchive-pathspec/ignored-by-tree.d/file\n>  test_expect_exists\tarchive-pathspec/ignored-by-worktree\n> -test_expect_missing\tarchive-pathspec/excluded-by-pathspec.d failure\n> +test_expect_missing\tarchive-pathspec/excluded-by-pathspec.d\n>  test_expect_missing\tarchive-pathspec/excluded-by-pathspec.d/file\n>  \n>  test_expect_success 'git archive with wildcard pathspec' '\n"},{"id":"326777","messageId":"798212ae-ffb8-2ffb-8b59-fa307424b48a@web.de","threadId":"46580","inReplyTo":"33fa4f08-8f06-5a98-e492-3f05cc742555@web.de","subject":"Re: [PATCH 4/4] archive: queue directories for all types of pathspecs","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-08-19T16:58:11Z","receivedAt":"2017-08-19T16:58:32Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 19.08.2017 um 18:53 schrieb René Scharfe:\n> Am 19.08.2017 um 07:33 schrieb René Scharfe:\n>> When read_tree_recursive() encounters a directory excluded by a pathspec\n>> then it enters it anyway because it might contain included entries.  It\n>> calls the callback function before it is able to decide if the directory\n>> is actually needed.\n>>\n>> For that reason git archive adds directories to a queue and writes\n>> entries for them only when it encounters the first child item -- but\n>> only if pathspecs with wildcards are used.  Do the same for literal\n>> pathspecs as well, as the reasoning above applies to them, too.  This\n>> prevents git archive from writing entries for excluded directories.\n> \n> This breaks the test \"archive empty subtree with no pathspec\" in t5004 by\n\nNo, it's \"archive empty subtree by direct pathspec\" that's broken.  Gah!\n\n> omitting the empty directory from the archive.  Sorry for missing that!\n> \n> This is kind of a bonus patch, so please discard it for now; the first\n> three are OK IMHO.\n> \n> A better version of this patch would at least update t5004 as well, but\n> there might be a better way.\n> \n>>\n>> Signed-off-by: Rene Scharfe <l.s.r@web.de>\n>> ---\n>>    archive.c               | 2 +-\n>>    t/t5001-archive-attr.sh | 2 +-\n>>    2 files changed, 2 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/archive.c b/archive.c\n>> index 1ab8d3a1d7..174c0555b6 100644\n>> --- a/archive.c\n>> +++ b/archive.c\n>> @@ -123,7 +123,7 @@ static int check_attr_export_subst(const struct attr_check *check)\n>>    \n>>    static int should_queue_directories(const struct archiver_args *args)\n>>    {\n>> -\treturn args->pathspec.has_wildcard;\n>> +\treturn args->pathspec.nr;\n>>    }\n>>    \n>>    static int write_archive_entry(const unsigned char *sha1, const char *base,\n>> diff --git a/t/t5001-archive-attr.sh b/t/t5001-archive-attr.sh\n>> index 897f6f06d5..e9aa97117a 100755\n>> --- a/t/t5001-archive-attr.sh\n>> +++ b/t/t5001-archive-attr.sh\n>> @@ -73,7 +73,7 @@ test_expect_missing\tarchive-pathspec/ignored-by-tree\n>>    test_expect_missing\tarchive-pathspec/ignored-by-tree.d\n>>    test_expect_missing\tarchive-pathspec/ignored-by-tree.d/file\n>>    test_expect_exists\tarchive-pathspec/ignored-by-worktree\n>> -test_expect_missing\tarchive-pathspec/excluded-by-pathspec.d failure\n>> +test_expect_missing\tarchive-pathspec/excluded-by-pathspec.d\n>>    test_expect_missing\tarchive-pathspec/excluded-by-pathspec.d/file\n>>    \n>>    test_expect_success 'git archive with wildcard pathspec' '\n>>\n"},{"id":"326779","messageId":"xmqqtw138nem.fsf@gitster.mtv.corp.google.com","threadId":"46580","inReplyTo":"798212ae-ffb8-2ffb-8b59-fa307424b48a@web.de","subject":"Re: [PATCH 4/4] archive: queue directories for all types of pathspecs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-19T17:42:25Z","receivedAt":"2017-08-19T17:42:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> No, it's \"archive empty subtree by direct pathspec\" that's broken.  Gah!\n>\n>> omitting the empty directory from the archive.  Sorry for missing that!\n>> \n>> This is kind of a bonus patch, so please discard it for now; the first\n>> three are OK IMHO.\n\nAh, our mails crossed.  Thanks; will drop the last one for now.\n"},{"id":"326804","messageId":"20170820090629.tumvqwzkromcykjf@sigill.intra.peff.net","threadId":"46580","inReplyTo":"33fa4f08-8f06-5a98-e492-3f05cc742555@web.de","subject":"Re: [PATCH 4/4] archive: queue directories for all types of pathspecs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-20T09:06:30Z","receivedAt":"2017-08-20T09:06:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Aug 19, 2017 at 06:53:26PM +0200, René Scharfe wrote:\n\n> Am 19.08.2017 um 07:33 schrieb René Scharfe:\n> > When read_tree_recursive() encounters a directory excluded by a pathspec\n> > then it enters it anyway because it might contain included entries.  It\n> > calls the callback function before it is able to decide if the directory\n> > is actually needed.\n> > \n> > For that reason git archive adds directories to a queue and writes\n> > entries for them only when it encounters the first child item -- but\n> > only if pathspecs with wildcards are used.  Do the same for literal\n> > pathspecs as well, as the reasoning above applies to them, too.  This\n> > prevents git archive from writing entries for excluded directories.\n> \n> This breaks the test \"archive empty subtree with no pathspec\" in t5004 by\n> omitting the empty directory from the archive.  Sorry for missing that!\n> \n> This is kind of a bonus patch, so please discard it for now; the first\n> three are OK IMHO.\n> \n> A better version of this patch would at least update t5004 as well, but\n> there might be a better way.\n\nI actually think it would be reasonable to omit the empty directory in\nt5004. The main thing we care about there is that we produce an archive\nwith no files rather than barfing. Checking that the empty directory is\nactually there was mostly \"this is what it happens to produce\" rather\nthan any conscious decision, I think.\n\nIf the new rule is \"we omit empty directories\", then it would make sense\nfor us to follow that, regardless of whether it happened by pathspec\nlimiting or if the tree was empty in the first place (and such an\nempty tree is insane anyway; Git tries hard not to create them).\n\n-Peff\n"},{"id":"326897","messageId":"CAGZ79kZm31jYZs5BXh8pwV2sLnhsy0AcQzoBWv1UZKtDFfYhkw@mail.gmail.com","threadId":"46580","inReplyTo":"33fa4f08-8f06-5a98-e492-3f05cc742555@web.de","subject":"Re: [PATCH 4/4] archive: queue directories for all types of pathspecs","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-21T18:17:09Z","receivedAt":"2017-08-21T18:17:15Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Aug 19, 2017 at 9:53 AM, René Scharfe <l.s.r@web.de> wrote:\n> Am 19.08.2017 um 07:33 schrieb René Scharfe:\n>> When read_tree_recursive() encounters a directory excluded by a pathspec\n>> then it enters it anyway because it might contain included entries.  It\n>> calls the callback function before it is able to decide if the directory\n>> is actually needed.\n>>\n>> For that reason git archive adds directories to a queue and writes\n>> entries for them only when it encounters the first child item -- but\n>> only if pathspecs with wildcards are used.  Do the same for literal\n>> pathspecs as well, as the reasoning above applies to them, too.  This\n>> prevents git archive from writing entries for excluded directories.\n>\n> This breaks the test \"archive empty subtree with no pathspec\" in t5004 by\n> omitting the empty directory from the archive.  Sorry for missing that!\n>\n> This is kind of a bonus patch, so please discard it for now; the first\n> three are OK IMHO.\n\nI thought so up to reading this patch. I found the naming and\ncontent of should_queue_directories not obvious at first read.\nThis patch confused me even more for that name to be chosen.\nI have no alternative to propose, though.\n\nThanks,\nStefan\n"},{"id":"327906","messageId":"837ff6fe-a645-97fe-4b21-ec36f662ce19@web.de","threadId":"46580","inReplyTo":"20170820090629.tumvqwzkromcykjf@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] archive: queue directories for all types of pathspecs","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-09-12T22:43:57Z","receivedAt":"2017-09-12T22:44:29Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 20.08.2017 um 11:06 schrieb Jeff King:\n> I actually think it would be reasonable to omit the empty directory in\n> t5004. The main thing we care about there is that we produce an archive\n> with no files rather than barfing. Checking that the empty directory is\n> actually there was mostly \"this is what it happens to produce\" rather\n> than any conscious decision, I think.\n> \n> If the new rule is \"we omit empty directories\", then it would make sense\n> for us to follow that, regardless of whether it happened by pathspec\n> limiting or if the tree was empty in the first place (and such an\n> empty tree is insane anyway; Git tries hard not to create them).\n\nRight.\n\n-- >8 --\nSubject: [PATCH] archive: don't add empty directories to archives\n\nWhile git doesn't track empty directories, git archive can be tricked\ninto putting some into archives.  One way is to construct an empty tree\nobject, as t5004 does.  While that is supported by the object database,\nit can't be represented in the index and thus it's unlikely to occur in\nthe wild.\n\nAnother way is using the literal name of a directory in an exclude\npathspec -- its contents are are excluded, but the directory stub is\nincluded.  That's inconsistent: exclude pathspecs containing wildcards\ndon't leave empty directories in the archive.\n\nYet another way is have a few levels of nested subdirectories (e.g.\nd1/d2/d3/file1) and ignoring the entries at the leaved (e.g. file1).\nThe directories with the ignored content are ignored as well (e.g. d3),\nbut their empty parents are included (e.g. d2).\n\nAs empty directories are not supported by git, they should also not be\nwritten into archives.  If an empty directory is really needed then it\ncan be tracked and archived by placing an empty .gitignore file in it.\n\nThere already is a mechanism in place for suppressing empty directories.\nWhen read_tree_recursive() encounters a directory excluded by a pathspec\nthen it enters it anyway because it might contain included entries.  It\ncalls the callback function before it is able to decide if the directory\nis actually needed.  For that reason git archive adds directories to a\nqueue and writes entries for them only when it encounters the first\nchild item -- but currently only if pathspecs with wildcards are used.\n\nQueue *all* directories, no matter if there even are pathspecs present.\nThis prevents git archive from writing entries for empty directories in\nall cases.\n\nSuggested-by: Jeff King <peff@peff.net>\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n archive.c                       | 19 ++-----------------\n t/t5001-archive-attr.sh         |  2 +-\n t/t5002-archive-attr-pattern.sh |  2 +-\n t/t5004-archive-corner-cases.sh |  4 ++--\n 4 files changed, 6 insertions(+), 21 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 1ab8d3a1d7..1e41f4bbeb 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -121,11 +121,6 @@ static int check_attr_export_subst(const struct attr_check *check)\n \treturn check && ATTR_TRUE(check->items[1].value);\n }\n \n-static int should_queue_directories(const struct archiver_args *args)\n-{\n-\treturn args->pathspec.has_wildcard;\n-}\n-\n static int write_archive_entry(const unsigned char *sha1, const char *base,\n \t\tint baselen, const char *filename, unsigned mode, int stage,\n \t\tvoid *context)\n@@ -147,7 +142,7 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,\n \t\tstrbuf_addch(&path, '/');\n \tpath_without_prefix = path.buf + args->baselen;\n \n-\tif (!S_ISDIR(mode) || !should_queue_directories(args)) {\n+\tif (!S_ISDIR(mode)) {\n \t\tconst struct attr_check *check;\n \t\tcheck = get_archive_attrs(path_without_prefix);\n \t\tif (check_attr_export_ignore(check))\n@@ -169,14 +164,6 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,\n \treturn write_entry(args, sha1, path.buf, path.len, mode);\n }\n \n-static int write_archive_entry_buf(const unsigned char *sha1, struct strbuf *base,\n-\t\tconst char *filename, unsigned mode, int stage,\n-\t\tvoid *context)\n-{\n-\treturn write_archive_entry(sha1, base->buf, base->len,\n-\t\t\t\t     filename, mode, stage, context);\n-}\n-\n static void queue_directory(const unsigned char *sha1,\n \t\tstruct strbuf *base, const char *filename,\n \t\tunsigned mode, int stage, struct archiver_context *c)\n@@ -290,9 +277,7 @@ int write_archive_entries(struct archiver_args *args,\n \t}\n \n \terr = read_tree_recursive(args->tree, \"\", 0, 0, &args->pathspec,\n-\t\t\t\t  should_queue_directories(args) ?\n-\t\t\t\t  queue_or_write_archive_entry :\n-\t\t\t\t  write_archive_entry_buf,\n+\t\t\t\t  queue_or_write_archive_entry,\n \t\t\t\t  &context);\n \tif (err == READ_TREE_RECURSIVE)\n \t\terr = 0;\ndiff --git a/t/t5001-archive-attr.sh b/t/t5001-archive-attr.sh\nindex 897f6f06d5..e9aa97117a 100755\n--- a/t/t5001-archive-attr.sh\n+++ b/t/t5001-archive-attr.sh\n@@ -73,7 +73,7 @@ test_expect_missing\tarchive-pathspec/ignored-by-tree\n test_expect_missing\tarchive-pathspec/ignored-by-tree.d\n test_expect_missing\tarchive-pathspec/ignored-by-tree.d/file\n test_expect_exists\tarchive-pathspec/ignored-by-worktree\n-test_expect_missing\tarchive-pathspec/excluded-by-pathspec.d failure\n+test_expect_missing\tarchive-pathspec/excluded-by-pathspec.d\n test_expect_missing\tarchive-pathspec/excluded-by-pathspec.d/file\n \n test_expect_success 'git archive with wildcard pathspec' '\ndiff --git a/t/t5002-archive-attr-pattern.sh b/t/t5002-archive-attr-pattern.sh\nindex 6667d159ab..bda6d7d7e9 100755\n--- a/t/t5002-archive-attr-pattern.sh\n+++ b/t/t5002-archive-attr-pattern.sh\n@@ -76,7 +76,7 @@ test_expect_missing\tarchive/deep/and/slashless/ &&\n test_expect_missing\tarchive/deep/and/slashless/foo &&\n test_expect_missing\tarchive/deep/with/wildcard/ &&\n test_expect_missing\tarchive/deep/with/wildcard/foo &&\n-test_expect_exists\tarchive/one-level-lower/\n+test_expect_missing\tarchive/one-level-lower/\n test_expect_missing\tarchive/one-level-lower/two-levels-lower/ignored-only-if-dir/\n test_expect_missing\tarchive/one-level-lower/two-levels-lower/ignored-ony-if-dir/ignored-by-ignored-dir\n \ndiff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh\nindex f6207f42b5..ced44355ca 100755\n--- a/t/t5004-archive-corner-cases.sh\n+++ b/t/t5004-archive-corner-cases.sh\n@@ -108,14 +108,14 @@ test_expect_success 'archive empty subtree with no pathspec' '\n \tgit archive --format=tar $root_tree >subtree-all.tar &&\n \tmake_dir extract &&\n \t\"$TAR\" xf subtree-all.tar -C extract &&\n-\tcheck_dir extract sub\n+\tcheck_dir extract\n '\n \n test_expect_success 'archive empty subtree by direct pathspec' '\n \tgit archive --format=tar $root_tree -- sub >subtree-path.tar &&\n \tmake_dir extract &&\n \t\"$TAR\" xf subtree-path.tar -C extract &&\n-\tcheck_dir extract sub\n+\tcheck_dir extract\n '\n \n ZIPINFO=zipinfo\n-- \n2.14.1\n"},{"id":"327929","messageId":"20170913125312.awtlbndv5j6kkcmz@sigill.intra.peff.net","threadId":"46580","inReplyTo":"837ff6fe-a645-97fe-4b21-ec36f662ce19@web.de","subject":"Re: [PATCH 4/4] archive: queue directories for all types of pathspecs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-09-13T12:53:12Z","receivedAt":"2017-09-13T12:53:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 13, 2017 at 12:43:57AM +0200, René Scharfe wrote:\n\n> -- >8 --\n> Subject: [PATCH] archive: don't add empty directories to archives\n> \n> While git doesn't track empty directories, git archive can be tricked\n> into putting some into archives.  One way is to construct an empty tree\n> object, as t5004 does.  While that is supported by the object database,\n> it can't be represented in the index and thus it's unlikely to occur in\n> the wild.\n> \n> Another way is using the literal name of a directory in an exclude\n> pathspec -- its contents are are excluded, but the directory stub is\n> included.  That's inconsistent: exclude pathspecs containing wildcards\n> don't leave empty directories in the archive.\n> \n> Yet another way is have a few levels of nested subdirectories (e.g.\n> d1/d2/d3/file1) and ignoring the entries at the leaved (e.g. file1).\n\ns/leaved/leaves/ ?\n\n> The directories with the ignored content are ignored as well (e.g. d3),\n> but their empty parents are included (e.g. d2).\n> \n> As empty directories are not supported by git, they should also not be\n> written into archives.  If an empty directory is really needed then it\n> can be tracked and archived by placing an empty .gitignore file in it.\n> \n> There already is a mechanism in place for suppressing empty directories.\n> When read_tree_recursive() encounters a directory excluded by a pathspec\n> then it enters it anyway because it might contain included entries.  It\n> calls the callback function before it is able to decide if the directory\n> is actually needed.  For that reason git archive adds directories to a\n> queue and writes entries for them only when it encounters the first\n> child item -- but currently only if pathspecs with wildcards are used.\n> \n> Queue *all* directories, no matter if there even are pathspecs present.\n> This prevents git archive from writing entries for empty directories in\n> all cases.\n\nNicely explained, and this seems like the right level to be handling it.\nSimple, and it will catch the cases we know about _and_ and any new ones\nwhich pop up.\n\n> ---\n>  archive.c                       | 19 ++-----------------\n>  t/t5001-archive-attr.sh         |  2 +-\n>  t/t5002-archive-attr-pattern.sh |  2 +-\n>  t/t5004-archive-corner-cases.sh |  4 ++--\n>  4 files changed, 6 insertions(+), 21 deletions(-)\n\nI'm not too familiar with this part of the archive code, but it seemed\npretty easy to follow. The patch looks good to me.\n\n-Peff\n"},{"id":"327941","messageId":"03efb857-ed92-8815-3730-767cd5d6896a@web.de","threadId":"46580","inReplyTo":"20170913125312.awtlbndv5j6kkcmz@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] archive: queue directories for all types of pathspecs","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-09-13T14:56:24Z","receivedAt":"2017-09-13T14:56:50Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 13.09.2017 um 14:53 schrieb Jeff King:\n> On Wed, Sep 13, 2017 at 12:43:57AM +0200, René Scharfe wrote:\n>> Yet another way is have a few levels of nested subdirectories (e.g.\n>> d1/d2/d3/file1) and ignoring the entries at the leaved (e.g. file1).\n> \n> s/leaved/leaves/ ?\n\nIndeed.\n\nRené\n"}]}