{"thread":{"id":"57864","subject":"[PATCH v2 00/15][GSoC] iterate dirs before or after their contents","startedAt":"2022-05-09T17:52:18Z","lastAt":"2022-05-20T17:43:27Z","messageCount":31,"participants":["Plato Kiorpelidis","Junio C Hamano","Phillip Wood","rsbecker@nexbridge.com"],"isPatch":true,"patchVersion":2,"patchTotal":15},"messages":[{"id":"455030","messageId":"20220509175159.2948802-1-kioplato@gmail.com","threadId":"57864","inReplyTo":null,"subject":"[PATCH v2 00/15][GSoC] iterate dirs before or after their contents","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-05-09T17:51:44Z","receivedAt":"2022-05-09T17:52:18Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"This is the second version of a patch series which implements pre-order and\npost-order iteration over directories. In this version I use the new iteration\nscheme to convert entry.c remove_subtree() function from using opendir/readdir/\nclosedir API to dir-iterator API.\n\nv1: https://lore.kernel.org/git/20220410111852.2097418-1-kioplato@gmail.com/\n\nFork: https://github.com/kioplato/git/tree/dir-iterator-v2\nCI: https://github.com/kioplato/git/actions/runs/2295239114\nI've ran the full test suite for each commit.\n\nIn comparison to v1 I changed/fixed/improved:\n\n* In this version I followed Phillip's suggestion[1] to explain why I'm making\nthe changes in each commit's description and create smaller, targeted commits.\nIn v1 I did a lot of renaming to directory paths, test descriptions and files.\nIn this version these renames have their own commits.\n\n* Explained why I converted dir_iterator_advance() from iterative implementation\nto recursive in the related commit. Ævar[2] and Phillip[3] asked about why I did\nthis conversion, so I thought it was appropriate to include an explanation in\nthe commit's description.\n\n* I talked about a subtle unexpected behavior of dir_iterator_begin() [4]. I was\nwrong about the two new states for the deepest directory. I introduced those\nstates in order to implement recursive dir_iterator_advance() and not to deal\nwith the unexpected behavior of dir_iterator_begin(). Therefore I splitted the\nconversion of dir_iteartor_advance() and the changes to deal with the unexpected\nbehavior of dir_iteartor_begin() into two different commits. They need their own\ncommits since they are two unrelated changes.\n\n* Split the commit that introduces pre-order and post-order iteration into two.\nLike Phillip suggested here[5] the first changes dir-iterator default behavior\nand the second implements post-order iterator over directories. This helps split\nthe tests introduced, which helps generate smaller diffs.\n\n* Like Phillip suggested [6], I'll work with the existing code and change one\naspect at a time in stages. In this series I converted entry.c remove_subtree()\nto use dir-iterator API instead of opendir/readdir/closedir API.\n\n* The redundant tests [7] in this version were not removed. Instead I kept them\nfor more detailed testing which helps in case a test fails.\n\n* Ævar's idea[8] to change the default swich case in test-dir-iterator made me\nrealize that test-dir-iterator does not print errno codes set by failed calls to\ndir_iterator_advance(). Therefore I made test-dir-iterator print them which\nmakes all tests test_cmp for the errno code set by either dir_iteartor_begin()\nor dir_iterator_advance(). This presumably has the same effect as changing the\ndefault switch case to use BUG() macro. This change also led to two others, one\nthat makes test-dir-iterator explicitly print ELOOP errno and the second that\nimproves readability by consistently returning, C standard constants, either\nEXIT_SUCCESS or EXIT_FAILURE, instead of mixed integers and exit() calls.\n\n* fixed coding style and introduces an enum for returning constants instead of\nmore integers which worsens readability, as suggested by Ævar [9].\n\nI also didn't CC Michael and Matheus in this version, since they weren't\ninterested in v1 where I did CC them.\n\n[1]: https://lore.kernel.org/git/ed6656e0-a865-319e-0f56-23ab1da3eef3@gmail.com/\n[2]: https://lore.kernel.org/git/220411.86o817j2dt.gmgdl@evledraar.gmail.com/\n[3]: https://lore.kernel.org/git/35160d46-d337-2110-f968-238abb7e9f0e@gmail.com/\n[4]: https://lore.kernel.org/git/20220427154526.uuhpkoee322l7kmz@compass/\n[5]: https://lore.kernel.org/git/b75aaee8-c037-e8e0-6ee0-729922059352@gmail.com/\n[6]: https://lore.kernel.org/git/df287d4f-e9da-4ce0-d7e9-1b1fe7671aab@gmail.com/\n[7]: https://lore.kernel.org/git/220411.86sfqjj2o0.gmgdl@evledraar.gmail.com/\n[8]: https://lore.kernel.org/git/220411.86wnfvj2q6.gmgdl@evledraar.gmail.com/\n[9]: https://lore.kernel.org/git/220411.86o817j2dt.gmgdl@evledraar.gmail.com/\n\nPlato Kiorpelidis (15):\n t0066: refactor dir-iterator tests\n t0066: remove dependency between unrelated tests\n t0066: shorter expected and actual output file names\n test-dir-iterator: consistently return EXIT_FAILURE or EXIT_SUCCESS\n test-dir-iterator: print EACCES and ELOOP errno set by dir_iterator\n test-dir-iterator: print errno name set by dir_iterator_advance\n t0066: better test coverage for dir-iterator\n t0066: reorder tests from simple to more complex\n t0066: rename test directories\n dir-iterator: refactor dir_iterator_advance()\n dir-iterator: open root dir in dir_iterator_begin()\n t0066: rename subtest descriptions\n dir-iterator: option to iterate dirs in pre-order\n dir-iterator: option to iterate dirs in post-order\n entry.c: use dir-iterator to avoid explicit dir traversal\n\n builtin/clone.c | 4 +-\n dir-iterator.c | 334 ++++++--\n dir-iterator.h | 36 +-\n entry.c | 38 +-\n refs/files-backend.c | 2 +-\n t/helper/test-dir-iterator.c | 24 +-\n t/t0066-dir-iterator.sh | 1478 +++++++++++++++++++++++++++++++---\n 7 files changed, 1704 insertions(+), 212 deletions(-)\n\n-- \n2.36.1\n\n"},{"id":"455032","messageId":"20220509175159.2948802-2-kioplato@gmail.com","threadId":"57864","inReplyTo":"20220509175159.2948802-1-kioplato@gmail.com","subject":"[PATCH v2 01/15] t0066: refactor dir-iterator tests","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-05-09T17:51:45Z","receivedAt":"2022-05-09T17:54:59Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"Be consistent throughout the dir-iterator tests regarding the order in\nwhich we:\n * create test directories\n * create expected outputs\n * test if actual and expected outputs differ\n\nThese changes improve the readability of dir-iterator tests, separating\nsetup tests as we want to test different iteration schemes over the same\nhierarchy. They also simplify the introduction of new tests that cover\ncurrent and future iteration schemes by providing a standardized\nstructure for testing dir-iterator with various directory hierarchies.\n\nSigned-off-by: Plato Kiorpelidis \n---\n t/t0066-dir-iterator.sh | 55 +++++++++++++++++++++--------------------\n 1 file changed, 28 insertions(+), 27 deletions(-)\n\ndiff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\nindex 63a1a45cd3..807c43d447 100755\n--- a/t/t0066-dir-iterator.sh\n+++ b/t/t0066-dir-iterator.sh\n@@ -5,7 +5,7 @@ test_description='Test the dir-iterator functionality'\n TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n-test_expect_success 'setup' '\n+test_expect_success 'setup -- dir w/ complex structure' '\n \tmkdir -p dir &&\n \tmkdir -p dir/a/b/c/ &&\n \t>dir/b &&\n@@ -13,12 +13,8 @@ test_expect_success 'setup' '\n \tmkdir -p dir/d/e/d/ &&\n \t>dir/a/b/c/d &&\n \t>dir/a/e &&\n-\t>dir/d/e/d/a &&\n-\n-\tmkdir -p dir2/a/b/c/ &&\n-\t>dir2/a/b/c/d\n+\t>dir/d/e/d/a\n '\n-\n test_expect_success 'dir-iterator should iterate through all files' '\n \tcat >expected-iteration-sorted-output <<-EOF &&\n \t[d] (a) [a] ./dir/a\n@@ -40,6 +36,10 @@ test_expect_success 'dir-iterator should iterate through all files' '\n \ttest_cmp expected-iteration-sorted-output actual-iteration-sorted-output\n '\n \n+test_expect_success 'setup -- dir w/ three nested dirs w/ file' '\n+\tmkdir -p dir2/a/b/c &&\n+\t>dir2/a/b/c/d\n+'\n test_expect_success 'dir-iterator should list files in the correct order' '\n \tcat >expected-pre-order-output <<-EOF &&\n \t[d] (a) [a] ./dir2/a\n@@ -54,65 +54,59 @@ test_expect_success 'dir-iterator should list files in the correct order' '\n '\n \n test_expect_success 'begin should fail upon inexistent paths' '\n+\techo \"dir_iterator_begin failure: ENOENT\" >expected-inexistent-path-output &&\n+\n \ttest_must_fail test-tool dir-iterator ./inexistent-path \\\n \t\t>actual-inexistent-path-output &&\n-\techo \"dir_iterator_begin failure: ENOENT\" >expected-inexistent-path-output &&\n+\n \ttest_cmp expected-inexistent-path-output actual-inexistent-path-output\n '\n \n test_expect_success 'begin should fail upon non directory paths' '\n-\ttest_must_fail test-tool dir-iterator ./dir/b >actual-non-dir-output &&\n \techo \"dir_iterator_begin failure: ENOTDIR\" >expected-non-dir-output &&\n+\n+\ttest_must_fail test-tool dir-iterator ./dir/b >actual-non-dir-output &&\n+\n \ttest_cmp expected-non-dir-output actual-non-dir-output\n '\n \n+test_expect_success POSIXPERM,SANITY 'setup -- dir w/ dir w/o perms w/ file' '\n+\tmkdir -p dir3/a &&\n+\t>dir3/a/b\n+'\n test_expect_success POSIXPERM,SANITY 'advance should not fail on errors by default' '\n \tcat >expected-no-permissions-output <<-EOF &&\n \t[d] (a) [a] ./dir3/a\n \tEOF\n \n-\tmkdir -p dir3/a &&\n-\t>dir3/a/b &&\n \tchmod 0 dir3/a &&\n \n \ttest-tool dir-iterator ./dir3 >actual-no-permissions-output &&\n \ttest_cmp expected-no-permissions-output actual-no-permissions-output &&\n-\tchmod 755 dir3/a &&\n-\trm -rf dir3\n+\tchmod 755 dir3/a\n '\n-\n test_expect_success POSIXPERM,SANITY 'advance should fail on errors, w/ pedantic flag' '\n \tcat >expected-no-permissions-pedantic-output <<-EOF &&\n \t[d] (a) [a] ./dir3/a\n \tdir_iterator_advance failure\n \tEOF\n \n-\tmkdir -p dir3/a &&\n-\t>dir3/a/b &&\n \tchmod 0 dir3/a &&\n \n \ttest_must_fail test-tool dir-iterator --pedantic ./dir3 \\\n \t\t>actual-no-permissions-pedantic-output &&\n \ttest_cmp expected-no-permissions-pedantic-output \\\n \t\tactual-no-permissions-pedantic-output &&\n-\tchmod 755 dir3/a &&\n-\trm -rf dir3\n+\tchmod 755 dir3/a\n '\n \n-test_expect_success SYMLINKS 'setup dirs with symlinks' '\n+test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/o cycle' '\n \tmkdir -p dir4/a &&\n \tmkdir -p dir4/b/c &&\n \t>dir4/a/d &&\n \tln -s d dir4/a/e &&\n-\tln -s ../b dir4/a/f &&\n-\n-\tmkdir -p dir5/a/b &&\n-\tmkdir -p dir5/a/c &&\n-\tln -s ../c dir5/a/b/d &&\n-\tln -s ../ dir5/a/b/e &&\n-\tln -s ../../ dir5/a/b/f\n+\tln -s ../b dir4/a/f\n '\n-\n test_expect_success SYMLINKS 'dir-iterator should not follow symlinks by default' '\n \tcat >expected-no-follow-sorted-output <<-EOF &&\n \t[d] (a) [a] ./dir4/a\n@@ -128,7 +122,6 @@ test_expect_success SYMLINKS 'dir-iterator should not follow symlinks by default\n \n \ttest_cmp expected-no-follow-sorted-output actual-no-follow-sorted-output\n '\n-\n test_expect_success SYMLINKS 'dir-iterator should follow symlinks w/ follow flag' '\n \tcat >expected-follow-sorted-output <<-EOF &&\n \t[d] (a) [a] ./dir4/a\n@@ -146,4 +139,12 @@ test_expect_success SYMLINKS 'dir-iterator should follow symlinks w/ follow flag\n \ttest_cmp expected-follow-sorted-output actual-follow-sorted-output\n '\n \n+test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/ cycle' '\n+\tmkdir -p dir5/a/b &&\n+\tmkdir -p dir5/a/c &&\n+\tln -s ../c dir5/a/b/d &&\n+\tln -s ../ dir5/a/b/e &&\n+\tln -s ../../ dir5/a/b/f\n+'\n+\n test_done\n-- \n2.36.1\n\n"},{"id":"455033","messageId":"20220509175159.2948802-3-kioplato@gmail.com","threadId":"57864","inReplyTo":"20220509175159.2948802-1-kioplato@gmail.com","subject":"[PATCH v2 02/15] t0066: remove dependency between unrelated tests","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-05-09T17:51:46Z","receivedAt":"2022-05-09T17:55:06Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"Two unrelated tests were using the same path for testing. That's\nincorrect because if the test that depends on the other test, which\ncreates the directory hierarcy, is ran using the --run flag then it\nwill fail, even though it should pass, because the path doesn't exist.\nUnrelated tests should not have dependencies among them.\n\nSigned-off-by: Plato Kiorpelidis \n---\n t/t0066-dir-iterator.sh | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\nindex 807c43d447..801749eb7e 100755\n--- a/t/t0066-dir-iterator.sh\n+++ b/t/t0066-dir-iterator.sh\n@@ -63,9 +63,11 @@ test_expect_success 'begin should fail upon inexistent paths' '\n '\n \n test_expect_success 'begin should fail upon non directory paths' '\n+\t>some-file &&\n+\n \techo \"dir_iterator_begin failure: ENOTDIR\" >expected-non-dir-output &&\n \n-\ttest_must_fail test-tool dir-iterator ./dir/b >actual-non-dir-output &&\n+\ttest_must_fail test-tool dir-iterator ./some-file >actual-non-dir-output &&\n \n \ttest_cmp expected-non-dir-output actual-non-dir-output\n '\n-- \n2.36.1\n\n"},{"id":"455034","messageId":"20220509175159.2948802-4-kioplato@gmail.com","threadId":"57864","inReplyTo":"20220509175159.2948802-1-kioplato@gmail.com","subject":"[PATCH v2 03/15] t0066: shorter expected and actual output file names","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-05-09T17:51:47Z","receivedAt":"2022-05-09T17:55:12Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"Change the names of expected-* and actual-* files to shorter ones to\nimprove readability, avoiding line breaks. There is no reason to have\ndescriptive expected-* and actual-* file names since subtests\ndescriptions fulfill this need.\n\nIf a test fails we can re-run it with --run flag and read its expected\nand actual outputs, since each subtest shares the same file names for\nexpected and actual outputs.\n\nSigned-off-by: Plato Kiorpelidis \n---\n t/t0066-dir-iterator.sh | 59 ++++++++++++++++++++---------------------\n 1 file changed, 29 insertions(+), 30 deletions(-)\n\ndiff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\nindex 801749eb7e..b21c58ade3 100755\n--- a/t/t0066-dir-iterator.sh\n+++ b/t/t0066-dir-iterator.sh\n@@ -16,7 +16,7 @@ test_expect_success 'setup -- dir w/ complex structure' '\n \t>dir/d/e/d/a\n '\n test_expect_success 'dir-iterator should iterate through all files' '\n-\tcat >expected-iteration-sorted-output <<-EOF &&\n+\tcat >expected-sorted-out <<-EOF &&\n \t[d] (a) [a] ./dir/a\n \t[d] (a/b) [b] ./dir/a/b\n \t[d] (a/b/c) [c] ./dir/a/b/c\n@@ -30,10 +30,10 @@ test_expect_success 'dir-iterator should iterate through all files' '\n \t[f] (d/e/d/a) [a] ./dir/d/e/d/a\n \tEOF\n \n-\ttest-tool dir-iterator ./dir >out &&\n-\tsort out >./actual-iteration-sorted-output &&\n+\ttest-tool dir-iterator ./dir >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n \n-\ttest_cmp expected-iteration-sorted-output actual-iteration-sorted-output\n+\ttest_cmp expected-sorted-out actual-sorted-out\n '\n \n test_expect_success 'setup -- dir w/ three nested dirs w/ file' '\n@@ -41,35 +41,34 @@ test_expect_success 'setup -- dir w/ three nested dirs w/ file' '\n \t>dir2/a/b/c/d\n '\n test_expect_success 'dir-iterator should list files in the correct order' '\n-\tcat >expected-pre-order-output <<-EOF &&\n+\tcat >expected-out <<-EOF &&\n \t[d] (a) [a] ./dir2/a\n \t[d] (a/b) [b] ./dir2/a/b\n \t[d] (a/b/c) [c] ./dir2/a/b/c\n \t[f] (a/b/c/d) [d] ./dir2/a/b/c/d\n \tEOF\n \n-\ttest-tool dir-iterator ./dir2 >actual-pre-order-output &&\n+\ttest-tool dir-iterator ./dir2 >actual-out &&\n \n-\ttest_cmp expected-pre-order-output actual-pre-order-output\n+\ttest_cmp expected-out actual-out\n '\n \n test_expect_success 'begin should fail upon inexistent paths' '\n-\techo \"dir_iterator_begin failure: ENOENT\" >expected-inexistent-path-output &&\n+\techo \"dir_iterator_begin failure: ENOENT\" >expected-out &&\n \n-\ttest_must_fail test-tool dir-iterator ./inexistent-path \\\n-\t\t>actual-inexistent-path-output &&\n+\ttest_must_fail test-tool dir-iterator ./inexistent-path >actual-out &&\n \n-\ttest_cmp expected-inexistent-path-output actual-inexistent-path-output\n+\ttest_cmp expected-out actual-out\n '\n \n test_expect_success 'begin should fail upon non directory paths' '\n \t>some-file &&\n \n-\techo \"dir_iterator_begin failure: ENOTDIR\" >expected-non-dir-output &&\n+\techo \"dir_iterator_begin failure: ENOTDIR\" >expected-out &&\n \n-\ttest_must_fail test-tool dir-iterator ./some-file >actual-non-dir-output &&\n+\ttest_must_fail test-tool dir-iterator ./some-file >actual-out &&\n \n-\ttest_cmp expected-non-dir-output actual-non-dir-output\n+\ttest_cmp expected-out actual-out\n '\n \n test_expect_success POSIXPERM,SANITY 'setup -- dir w/ dir w/o perms w/ file' '\n@@ -77,28 +76,28 @@ test_expect_success POSIXPERM,SANITY 'setup -- dir w/ dir w/o perms w/ file' '\n \t>dir3/a/b\n '\n test_expect_success POSIXPERM,SANITY 'advance should not fail on errors by default' '\n-\tcat >expected-no-permissions-output <<-EOF &&\n+\tcat >expected-out <<-EOF &&\n \t[d] (a) [a] ./dir3/a\n \tEOF\n \n \tchmod 0 dir3/a &&\n \n-\ttest-tool dir-iterator ./dir3 >actual-no-permissions-output &&\n-\ttest_cmp expected-no-permissions-output actual-no-permissions-output &&\n+\ttest-tool dir-iterator ./dir3 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n \tchmod 755 dir3/a\n '\n test_expect_success POSIXPERM,SANITY 'advance should fail on errors, w/ pedantic flag' '\n-\tcat >expected-no-permissions-pedantic-output <<-EOF &&\n+\tcat >expected-out <<-EOF &&\n \t[d] (a) [a] ./dir3/a\n \tdir_iterator_advance failure\n \tEOF\n \n \tchmod 0 dir3/a &&\n \n-\ttest_must_fail test-tool dir-iterator --pedantic ./dir3 \\\n-\t\t>actual-no-permissions-pedantic-output &&\n-\ttest_cmp expected-no-permissions-pedantic-output \\\n-\t\tactual-no-permissions-pedantic-output &&\n+\ttest_must_fail test-tool dir-iterator --pedantic ./dir3 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n \tchmod 755 dir3/a\n '\n \n@@ -110,7 +109,7 @@ test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/o cycle' '\n \tln -s ../b dir4/a/f\n '\n test_expect_success SYMLINKS 'dir-iterator should not follow symlinks by default' '\n-\tcat >expected-no-follow-sorted-output <<-EOF &&\n+\tcat >expected-sorted-out <<-EOF &&\n \t[d] (a) [a] ./dir4/a\n \t[d] (b) [b] ./dir4/b\n \t[d] (b/c) [c] ./dir4/b/c\n@@ -119,13 +118,13 @@ test_expect_success SYMLINKS 'dir-iterator should not follow symlinks by default\n \t[s] (a/f) [f] ./dir4/a/f\n \tEOF\n \n-\ttest-tool dir-iterator ./dir4 >out &&\n-\tsort out >actual-no-follow-sorted-output &&\n+\ttest-tool dir-iterator ./dir4 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n \n-\ttest_cmp expected-no-follow-sorted-output actual-no-follow-sorted-output\n+\ttest_cmp expected-sorted-out actual-sorted-out\n '\n test_expect_success SYMLINKS 'dir-iterator should follow symlinks w/ follow flag' '\n-\tcat >expected-follow-sorted-output <<-EOF &&\n+\tcat >expected-sorted-out <<-EOF &&\n \t[d] (a) [a] ./dir4/a\n \t[d] (a/f) [f] ./dir4/a/f\n \t[d] (a/f/c) [c] ./dir4/a/f/c\n@@ -135,10 +134,10 @@ test_expect_success SYMLINKS 'dir-iterator should follow symlinks w/ follow flag\n \t[f] (a/e) [e] ./dir4/a/e\n \tEOF\n \n-\ttest-tool dir-iterator --follow-symlinks ./dir4 >out &&\n-\tsort out >actual-follow-sorted-output &&\n+\ttest-tool dir-iterator --follow-symlinks ./dir4 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n \n-\ttest_cmp expected-follow-sorted-output actual-follow-sorted-output\n+\ttest_cmp expected-sorted-out actual-sorted-out\n '\n \n test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/ cycle' '\n-- \n2.36.1\n\n"},{"id":"455035","messageId":"20220509175159.2948802-5-kioplato@gmail.com","threadId":"57864","inReplyTo":"20220509175159.2948802-1-kioplato@gmail.com","subject":"[PATCH v2 04/15] test-dir-iterator: consistently return EXIT_FAILURE or EXIT_SUCCESS","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-05-09T17:51:48Z","receivedAt":"2022-05-09T17:55:13Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"Throughout test-dir-iterator.c we were returning/exiting with either\nintegers or EXIT_FAILURE. Improve readability and reduce mental load\nby being consistent with what test-dir-iterator returns through the\ntest-tool. Returning mixed constants and integers could indicate that\nit matters for some reason e.g. architecture of test-tool and cmd__*\nfunctions.\n\nEXIT_SUCCESS and EXIT_FAILURE are specified by the C standard.\nThat makes the code more portable and standardized.\n\nSigned-off-by: Plato Kiorpelidis \n---\n t/helper/test-dir-iterator.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/helper/test-dir-iterator.c b/t/helper/test-dir-iterator.c\nindex 659b6bfa81..81e931673e 100644\n--- a/t/helper/test-dir-iterator.c\n+++ b/t/helper/test-dir-iterator.c\n@@ -39,7 +39,7 @@ int cmd__dir_iterator(int argc, const char **argv)\n \n \tif (!diter) {\n \t\tprintf(\"dir_iterator_begin failure: %s\\n\", error_name(errno));\n-\t\texit(EXIT_FAILURE);\n+\t\treturn EXIT_FAILURE;\n \t}\n \n \twhile ((iter_status = dir_iterator_advance(diter)) == ITER_OK) {\n@@ -58,8 +58,8 @@ int cmd__dir_iterator(int argc, const char **argv)\n \n \tif (iter_status != ITER_DONE) {\n \t\tprintf(\"dir_iterator_advance failure\\n\");\n-\t\treturn 1;\n+\t\treturn EXIT_FAILURE;\n \t}\n \n-\treturn 0;\n+\treturn EXIT_SUCCESS;\n }\n-- \n2.36.1\n\n"},{"id":"455036","messageId":"20220509175159.2948802-6-kioplato@gmail.com","threadId":"57864","inReplyTo":"20220509175159.2948802-1-kioplato@gmail.com","subject":"[PATCH v2 05/15] test-dir-iterator: print EACCES and ELOOP errno set by dir_iterator","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-05-09T17:51:49Z","receivedAt":"2022-05-09T17:55:15Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"Currently, only ENOENT and ENOTDIR errno names are explicitly printed\nwhen an unsuccessful call to dir_iterator_begin occurs. In case any\nother errno happens it's collapsed and ESOMETHINGELSE is printed.\nESOMETHINGELSE collapses a lot of errno error numbers which makes\nexplicitly testing for specific cases of directory hierarcies\nimpossible.\n\nThis commit allows for more thorought testing by seperating EACCES and\nELOOP errno from ESOMETHINGELSE, which makes them explicitly printed\ninstead of just collapsed into ESOMETHINGELSE.\n\nRight now, there isn't any test that depends on ESOMETHINGELSE directly,\nneither in EACCES or ELOOP as a result, since only errno error codes set\nby unsuccessful dir_iterator_begin calls are printed and the ones set by\ndir_iterator_advance are ignored. However, this commit provides the\nrequired support that future tests need to improve test coverage.\n\nSigned-off-by: Plato Kiorpelidis \n---\n t/helper/test-dir-iterator.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/helper/test-dir-iterator.c b/t/helper/test-dir-iterator.c\nindex 81e931673e..cdb9269ad5 100644\n--- a/t/helper/test-dir-iterator.c\n+++ b/t/helper/test-dir-iterator.c\n@@ -9,6 +9,8 @@ static const char *error_name(int error_number)\n \tswitch (error_number) {\n \tcase ENOENT: return \"ENOENT\";\n \tcase ENOTDIR: return \"ENOTDIR\";\n+\tcase EACCES: return \"EACCES\";\n+\tcase ELOOP: return \"ELOOP\";\n \tdefault: return \"ESOMETHINGELSE\";\n \t}\n }\n-- \n2.36.1\n\n"},{"id":"455037","messageId":"20220509175159.2948802-7-kioplato@gmail.com","threadId":"57864","inReplyTo":"20220509175159.2948802-1-kioplato@gmail.com","subject":"[PATCH v2 06/15] test-dir-iterator: print errno name set by dir_iterator_advance","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-05-09T17:51:50Z","receivedAt":"2022-05-09T17:55:17Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"Currently, test-dir-iterator does not check for errno error codes set by\nan unsuccessful dir_iterator_advance() call. We do check for them,\nhowever, in dir_iterator_begin(). Make it such that test-dir-iterator\ndoes checks for them in dir_iterator_advance() as well.\n\nThis improves test coverage because it provides more detailed outputs\nregarding errno error codes set by dir_iterator_advance(), which in turn\nallows for the addition of more thorough tests over dir-iterator.\n\nSigned-off-by: Plato Kiorpelidis \n---\n t/helper/test-dir-iterator.c | 2 +-\n t/t0066-dir-iterator.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/helper/test-dir-iterator.c b/t/helper/test-dir-iterator.c\nindex cdb9269ad5..55d8a58836 100644\n--- a/t/helper/test-dir-iterator.c\n+++ b/t/helper/test-dir-iterator.c\n@@ -59,7 +59,7 @@ int cmd__dir_iterator(int argc, const char **argv)\n \t}\n \n \tif (iter_status != ITER_DONE) {\n-\t\tprintf(\"dir_iterator_advance failure\\n\");\n+\t\tprintf(\"dir_iterator_advance failure: %s\\n\", error_name(errno));\n \t\treturn EXIT_FAILURE;\n \t}\n \ndiff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\nindex b21c58ade3..61b02423d5 100755\n--- a/t/t0066-dir-iterator.sh\n+++ b/t/t0066-dir-iterator.sh\n@@ -90,7 +90,7 @@ test_expect_success POSIXPERM,SANITY 'advance should not fail on errors by defau\n test_expect_success POSIXPERM,SANITY 'advance should fail on errors, w/ pedantic flag' '\n \tcat >expected-out <<-EOF &&\n \t[d] (a) [a] ./dir3/a\n-\tdir_iterator_advance failure\n+\tdir_iterator_advance failure: EACCES\n \tEOF\n \n \tchmod 0 dir3/a &&\n-- \n2.36.1\n\n"},{"id":"455038","messageId":"20220509175159.2948802-10-kioplato@gmail.com","threadId":"57864","inReplyTo":"20220509175159.2948802-1-kioplato@gmail.com","subject":"[PATCH v2 09/15] t0066: rename test directories","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-05-09T17:51:53Z","receivedAt":"2022-05-09T17:55:24Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"Improve readability by renaming directories such that their name\ncorrespond to the order they appear in the file. An out of order\nnaming could suggest that the directory naming plays some role\nin how the tests are structured and performed, which is not true.\n\nSigned-off-by: Plato Kiorpelidis \n---\n t/t0066-dir-iterator.sh | 198 ++++++++++++++++++++--------------------\n 1 file changed, 99 insertions(+), 99 deletions(-)\n\ndiff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\nindex 9b3ce99382..04e51928bc 100755\n--- a/t/t0066-dir-iterator.sh\n+++ b/t/t0066-dir-iterator.sh\n@@ -19,96 +19,96 @@ test_expect_success 'iteration of dir with a file' '\n '\n \n test_expect_success 'setup -- dir with a single dir' '\n-\tmkdir -p dir6/a\n+\tmkdir -p dir2/a\n '\n test_expect_success 'iteration of dir with a single dir' '\n \tcat >expected-out <<-EOF &&\n-\t[d] (a) [a] ./dir6/a\n+\t[d] (a) [a] ./dir2/a\n \tEOF\n \n-\ttest-tool dir-iterator ./dir6 >actual-out &&\n+\ttest-tool dir-iterator ./dir2 >actual-out &&\n \ttest_cmp expected-out actual-out\n '\n \n test_expect_success POSIXPERM,SANITY 'setup -- dir w/ single dir w/o perms' '\n-\tmkdir -p dir13/a\n+\tmkdir -p dir3/a\n '\n test_expect_success POSIXPERM,SANITY 'iteration of dir w/ dir w/o perms' '\n \tcat >expected-out <<-EOF &&\n-\t[d] (a) [a] ./dir13/a\n+\t[d] (a) [a] ./dir3/a\n \tEOF\n \n-\tchmod 0 dir13/a &&\n+\tchmod 0 dir3/a &&\n \n-\ttest-tool dir-iterator ./dir13/ >actual-out &&\n+\ttest-tool dir-iterator ./dir3/ >actual-out &&\n \ttest_cmp expected-out actual-out &&\n \n-\tchmod 755 dir13/a\n+\tchmod 755 dir3/a\n '\n test_expect_success POSIXPERM,SANITY 'pedantic iteration of dir w/ dir w/o perms' '\n \tcat >expected-out <<-EOF &&\n-\t[d] (a) [a] ./dir13/a\n+\t[d] (a) [a] ./dir3/a\n \tdir_iterator_advance failure: EACCES\n \tEOF\n \n-\tchmod 0 dir13/a &&\n+\tchmod 0 dir3/a &&\n \n-\ttest_must_fail test-tool dir-iterator --pedantic ./dir13/ >actual-out &&\n+\ttest_must_fail test-tool dir-iterator --pedantic ./dir3/ >actual-out &&\n \ttest_cmp expected-out actual-out &&\n \n-\tchmod 755 dir13/a\n+\tchmod 755 dir3/a\n '\n \n test_expect_success 'setup -- dir w/ five files' '\n-\tmkdir dir14 &&\n-\t>dir14/a &&\n-\t>dir14/b &&\n-\t>dir14/c &&\n-\t>dir14/d &&\n-\t>dir14/e\n+\tmkdir dir4 &&\n+\t>dir4/a &&\n+\t>dir4/b &&\n+\t>dir4/c &&\n+\t>dir4/d &&\n+\t>dir4/e\n '\n test_expect_success 'iteration of dir w/ five files' '\n \tcat >expected-sorted-out <<-EOF &&\n-\t[f] (a) [a] ./dir14/a\n-\t[f] (b) [b] ./dir14/b\n-\t[f] (c) [c] ./dir14/c\n-\t[f] (d) [d] ./dir14/d\n-\t[f] (e) [e] ./dir14/e\n+\t[f] (a) [a] ./dir4/a\n+\t[f] (b) [b] ./dir4/b\n+\t[f] (c) [c] ./dir4/c\n+\t[f] (d) [d] ./dir4/d\n+\t[f] (e) [e] ./dir4/e\n \tEOF\n \n-\ttest-tool dir-iterator ./dir14 >actual-out &&\n+\ttest-tool dir-iterator ./dir4 >actual-out &&\n \tsort actual-out >actual-sorted-out &&\n \n \ttest_cmp expected-sorted-out actual-sorted-out\n '\n \n test_expect_success 'setup -- dir w/ dir w/ a file' '\n-\tmkdir -p dir15/a &&\n-\t>dir15/a/b\n+\tmkdir -p dir5/a &&\n+\t>dir5/a/b\n '\n test_expect_success 'iteration of dir w/ dir w/ a file' '\n \tcat >expected-out <<-EOF &&\n-\t[d] (a) [a] ./dir15/a\n-\t[f] (a/b) [b] ./dir15/a/b\n+\t[d] (a) [a] ./dir5/a\n+\t[f] (a/b) [b] ./dir5/a/b\n \tEOF\n \n-\ttest-tool dir-iterator ./dir15 >actual-out &&\n+\ttest-tool dir-iterator ./dir5 >actual-out &&\n \ttest_cmp expected-out actual-out\n '\n \n test_expect_success 'setup -- dir w/ three nested dirs w/ file' '\n-\tmkdir -p dir2/a/b/c &&\n-\t>dir2/a/b/c/d\n+\tmkdir -p dir6/a/b/c &&\n+\t>dir6/a/b/c/d\n '\n test_expect_success 'dir-iterator should list files in the correct order' '\n \tcat >expected-out <<-EOF &&\n-\t[d] (a) [a] ./dir2/a\n-\t[d] (a/b) [b] ./dir2/a/b\n-\t[d] (a/b/c) [c] ./dir2/a/b/c\n-\t[f] (a/b/c/d) [d] ./dir2/a/b/c/d\n+\t[d] (a) [a] ./dir6/a\n+\t[d] (a/b) [b] ./dir6/a/b\n+\t[d] (a/b/c) [c] ./dir6/a/b/c\n+\t[f] (a/b/c/d) [d] ./dir6/a/b/c/d\n \tEOF\n \n-\ttest-tool dir-iterator ./dir2 >actual-out &&\n+\ttest-tool dir-iterator ./dir6 >actual-out &&\n \ttest_cmp expected-out actual-out\n '\n \n@@ -253,31 +253,31 @@ test_expect_success 'iteration of dir w/ two nested dirs, each w/ file' '\n '\n \n test_expect_success 'setup -- dir w/ complex structure' '\n-\tmkdir -p dir &&\n-\tmkdir -p dir/a/b/c/ &&\n-\t>dir/b &&\n-\t>dir/c &&\n-\tmkdir -p dir/d/e/d/ &&\n-\t>dir/a/b/c/d &&\n-\t>dir/a/e &&\n-\t>dir/d/e/d/a\n+\tmkdir -p dir11 &&\n+\tmkdir -p dir11/a/b/c/ &&\n+\t>dir11/b &&\n+\t>dir11/c &&\n+\tmkdir -p dir11/d/e/d/ &&\n+\t>dir11/a/b/c/d &&\n+\t>dir11/a/e &&\n+\t>dir11/d/e/d/a\n '\n test_expect_success 'dir-iterator should iterate through all files' '\n \tcat >expected-sorted-out <<-EOF &&\n-\t[d] (a) [a] ./dir/a\n-\t[d] (a/b) [b] ./dir/a/b\n-\t[d] (a/b/c) [c] ./dir/a/b/c\n-\t[d] (d) [d] ./dir/d\n-\t[d] (d/e) [e] ./dir/d/e\n-\t[d] (d/e/d) [d] ./dir/d/e/d\n-\t[f] (a/b/c/d) [d] ./dir/a/b/c/d\n-\t[f] (a/e) [e] ./dir/a/e\n-\t[f] (b) [b] ./dir/b\n-\t[f] (c) [c] ./dir/c\n-\t[f] (d/e/d/a) [a] ./dir/d/e/d/a\n+\t[d] (a) [a] ./dir11/a\n+\t[d] (a/b) [b] ./dir11/a/b\n+\t[d] (a/b/c) [c] ./dir11/a/b/c\n+\t[d] (d) [d] ./dir11/d\n+\t[d] (d/e) [e] ./dir11/d/e\n+\t[d] (d/e/d) [d] ./dir11/d/e/d\n+\t[f] (a/b/c/d) [d] ./dir11/a/b/c/d\n+\t[f] (a/e) [e] ./dir11/a/e\n+\t[f] (b) [b] ./dir11/b\n+\t[f] (c) [c] ./dir11/c\n+\t[f] (d/e/d/a) [a] ./dir11/d/e/d/a\n \tEOF\n \n-\ttest-tool dir-iterator ./dir >actual-out &&\n+\ttest-tool dir-iterator ./dir11 >actual-out &&\n \tsort actual-out >actual-sorted-out &&\n \n \ttest_cmp expected-sorted-out actual-sorted-out\n@@ -331,93 +331,93 @@ test_expect_success 'begin should fail upon non directory paths' '\n '\n \n test_expect_success POSIXPERM,SANITY 'setup -- dir w/ dir w/o perms w/ file' '\n-\tmkdir -p dir3/a &&\n-\t>dir3/a/b\n+\tmkdir -p dir13/a &&\n+\t>dir13/a/b\n '\n test_expect_success POSIXPERM,SANITY 'advance should not fail on errors by default' '\n \tcat >expected-out <<-EOF &&\n-\t[d] (a) [a] ./dir3/a\n+\t[d] (a) [a] ./dir13/a\n \tEOF\n \n-\tchmod 0 dir3/a &&\n+\tchmod 0 dir13/a &&\n \n-\ttest-tool dir-iterator ./dir3 >actual-out &&\n+\ttest-tool dir-iterator ./dir13 >actual-out &&\n \ttest_cmp expected-out actual-out &&\n \n-\tchmod 755 dir3/a\n+\tchmod 755 dir13/a\n '\n test_expect_success POSIXPERM,SANITY 'advance should fail on errors, w/ pedantic flag' '\n \tcat >expected-out <<-EOF &&\n-\t[d] (a) [a] ./dir3/a\n+\t[d] (a) [a] ./dir13/a\n \tdir_iterator_advance failure: EACCES\n \tEOF\n \n-\tchmod 0 dir3/a &&\n+\tchmod 0 dir13/a &&\n \n-\ttest_must_fail test-tool dir-iterator --pedantic ./dir3 >actual-out &&\n+\ttest_must_fail test-tool dir-iterator --pedantic ./dir13 >actual-out &&\n \ttest_cmp expected-out actual-out &&\n \n-\tchmod 755 dir3/a\n+\tchmod 755 dir13/a\n '\n \n test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/o cycle' '\n-\tmkdir -p dir4/a &&\n-\tmkdir -p dir4/b/c &&\n-\t>dir4/a/d &&\n-\tln -s d dir4/a/e &&\n-\tln -s ../b dir4/a/f\n+\tmkdir -p dir14/a &&\n+\tmkdir -p dir14/b/c &&\n+\t>dir14/a/d &&\n+\tln -s d dir14/a/e &&\n+\tln -s ../b dir14/a/f\n '\n test_expect_success SYMLINKS 'dir-iterator should not follow symlinks by default' '\n \tcat >expected-sorted-out <<-EOF &&\n-\t[d] (a) [a] ./dir4/a\n-\t[d] (b) [b] ./dir4/b\n-\t[d] (b/c) [c] ./dir4/b/c\n-\t[f] (a/d) [d] ./dir4/a/d\n-\t[s] (a/e) [e] ./dir4/a/e\n-\t[s] (a/f) [f] ./dir4/a/f\n+\t[d] (a) [a] ./dir14/a\n+\t[d] (b) [b] ./dir14/b\n+\t[d] (b/c) [c] ./dir14/b/c\n+\t[f] (a/d) [d] ./dir14/a/d\n+\t[s] (a/e) [e] ./dir14/a/e\n+\t[s] (a/f) [f] ./dir14/a/f\n \tEOF\n \n-\ttest-tool dir-iterator ./dir4 >actual-out &&\n+\ttest-tool dir-iterator ./dir14 >actual-out &&\n \tsort actual-out >actual-sorted-out &&\n \n \ttest_cmp expected-sorted-out actual-sorted-out\n '\n test_expect_success SYMLINKS 'dir-iterator should follow symlinks w/ follow flag' '\n \tcat >expected-sorted-out <<-EOF &&\n-\t[d] (a) [a] ./dir4/a\n-\t[d] (a/f) [f] ./dir4/a/f\n-\t[d] (a/f/c) [c] ./dir4/a/f/c\n-\t[d] (b) [b] ./dir4/b\n-\t[d] (b/c) [c] ./dir4/b/c\n-\t[f] (a/d) [d] ./dir4/a/d\n-\t[f] (a/e) [e] ./dir4/a/e\n+\t[d] (a) [a] ./dir14/a\n+\t[d] (a/f) [f] ./dir14/a/f\n+\t[d] (a/f/c) [c] ./dir14/a/f/c\n+\t[d] (b) [b] ./dir14/b\n+\t[d] (b/c) [c] ./dir14/b/c\n+\t[f] (a/d) [d] ./dir14/a/d\n+\t[f] (a/e) [e] ./dir14/a/e\n \tEOF\n \n-\ttest-tool dir-iterator --follow-symlinks ./dir4 >actual-out &&\n+\ttest-tool dir-iterator --follow-symlinks ./dir14 >actual-out &&\n \tsort actual-out >actual-sorted-out &&\n \n \ttest_cmp expected-sorted-out actual-sorted-out\n '\n \n test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/ cycle' '\n-\tmkdir -p dir5/a/b &&\n-\tmkdir -p dir5/a/c &&\n-\tln -s ../c dir5/a/b/d &&\n-\tln -s ../ dir5/a/b/e &&\n-\tln -s ../../ dir5/a/b/f\n+\tmkdir -p dir15/a/b &&\n+\tmkdir -p dir15/a/c &&\n+\tln -s ../c dir15/a/b/d &&\n+\tln -s ../ dir15/a/b/e &&\n+\tln -s ../../ dir15/a/b/f\n '\n test_expect_success SYMLINKS 'iteration of dir w/ symlinks w/ cycle' '\n \n \tcat >expected-sorted-out <<-EOF &&\n-\t[d] (a) [a] ./dir5/a\n-\t[d] (a/b) [b] ./dir5/a/b\n-\t[d] (a/c) [c] ./dir5/a/c\n-\t[s] (a/b/d) [d] ./dir5/a/b/d\n-\t[s] (a/b/e) [e] ./dir5/a/b/e\n-\t[s] (a/b/f) [f] ./dir5/a/b/f\n+\t[d] (a) [a] ./dir15/a\n+\t[d] (a/b) [b] ./dir15/a/b\n+\t[d] (a/c) [c] ./dir15/a/c\n+\t[s] (a/b/d) [d] ./dir15/a/b/d\n+\t[s] (a/b/e) [e] ./dir15/a/b/e\n+\t[s] (a/b/f) [f] ./dir15/a/b/f\n \tEOF\n \n-\ttest-tool dir-iterator ./dir5 >actual-out &&\n+\ttest-tool dir-iterator ./dir15 >actual-out &&\n \tsort actual-out >actual-sorted-out &&\n \n \ttest_cmp expected-sorted-out actual-sorted-out\n@@ -430,7 +430,7 @@ test_expect_success SYMLINKS \\\n \tEOF\n \n \ttest_must_fail test-tool dir-iterator \\\n-\t\t--pedantic --follow-symlinks ./dir5 >actual-out &&\n+\t\t--pedantic --follow-symlinks ./dir15 >actual-out &&\n \ttail -n 1 actual-out >actual-tailed-out &&\n \n \ttest_cmp expected-tailed-out actual-tailed-out\n-- \n2.36.1\n\n"},{"id":"455039","messageId":"20220509175159.2948802-9-kioplato@gmail.com","threadId":"57864","inReplyTo":"20220509175159.2948802-1-kioplato@gmail.com","subject":"[PATCH v2 08/15] t0066: reorder tests from simple to more complex","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-05-09T17:51:52Z","receivedAt":"2022-05-09T17:55:27Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"Gradually increase the complexity of the tests. This reordering helps\nwhen testing dir-iterator, explicitly or by running the full test suite.\nIn case a test fails it provides additional aid searching for and finding\nthe part of dir-iterator that causes the test to fail, making it easier.\n\nSigned-off-by: Plato Kiorpelidis \n---\n t/t0066-dir-iterator.sh | 62 ++++++++++++++++++++---------------------\n 1 file changed, 31 insertions(+), 31 deletions(-)\n\ndiff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\nindex 932b1f291c..9b3ce99382 100755\n--- a/t/t0066-dir-iterator.sh\n+++ b/t/t0066-dir-iterator.sh\n@@ -96,37 +96,6 @@ test_expect_success 'iteration of dir w/ dir w/ a file' '\n \ttest_cmp expected-out actual-out\n '\n \n-test_expect_success 'setup -- dir w/ complex structure' '\n-\tmkdir -p dir &&\n-\tmkdir -p dir/a/b/c/ &&\n-\t>dir/b &&\n-\t>dir/c &&\n-\tmkdir -p dir/d/e/d/ &&\n-\t>dir/a/b/c/d &&\n-\t>dir/a/e &&\n-\t>dir/d/e/d/a\n-'\n-test_expect_success 'dir-iterator should iterate through all files' '\n-\tcat >expected-sorted-out <<-EOF &&\n-\t[d] (a) [a] ./dir/a\n-\t[d] (a/b) [b] ./dir/a/b\n-\t[d] (a/b/c) [c] ./dir/a/b/c\n-\t[d] (d) [d] ./dir/d\n-\t[d] (d/e) [e] ./dir/d/e\n-\t[d] (d/e/d) [d] ./dir/d/e/d\n-\t[f] (a/b/c/d) [d] ./dir/a/b/c/d\n-\t[f] (a/e) [e] ./dir/a/e\n-\t[f] (b) [b] ./dir/b\n-\t[f] (c) [c] ./dir/c\n-\t[f] (d/e/d/a) [a] ./dir/d/e/d/a\n-\tEOF\n-\n-\ttest-tool dir-iterator ./dir >actual-out &&\n-\tsort actual-out >actual-sorted-out &&\n-\n-\ttest_cmp expected-sorted-out actual-sorted-out\n-'\n-\n test_expect_success 'setup -- dir w/ three nested dirs w/ file' '\n \tmkdir -p dir2/a/b/c &&\n \t>dir2/a/b/c/d\n@@ -283,6 +252,37 @@ test_expect_success 'iteration of dir w/ two nested dirs, each w/ file' '\n \t)\n '\n \n+test_expect_success 'setup -- dir w/ complex structure' '\n+\tmkdir -p dir &&\n+\tmkdir -p dir/a/b/c/ &&\n+\t>dir/b &&\n+\t>dir/c &&\n+\tmkdir -p dir/d/e/d/ &&\n+\t>dir/a/b/c/d &&\n+\t>dir/a/e &&\n+\t>dir/d/e/d/a\n+'\n+test_expect_success 'dir-iterator should iterate through all files' '\n+\tcat >expected-sorted-out <<-EOF &&\n+\t[d] (a) [a] ./dir/a\n+\t[d] (a/b) [b] ./dir/a/b\n+\t[d] (a/b/c) [c] ./dir/a/b/c\n+\t[d] (d) [d] ./dir/d\n+\t[d] (d/e) [e] ./dir/d/e\n+\t[d] (d/e/d) [d] ./dir/d/e/d\n+\t[f] (a/b/c/d) [d] ./dir/a/b/c/d\n+\t[f] (a/e) [e] ./dir/a/e\n+\t[f] (b) [b] ./dir/b\n+\t[f] (c) [c] ./dir/c\n+\t[f] (d/e/d/a) [a] ./dir/d/e/d/a\n+\tEOF\n+\n+\ttest-tool dir-iterator ./dir >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-sorted-out actual-sorted-out\n+'\n+\n test_expect_success POSIXPERM,SANITY 'setup -- dir w/o perms' '\n \tmkdir -p dir12/a &&\n \t>dir12/a/b\n-- \n2.36.1\n\n"},{"id":"455040","messageId":"20220509175159.2948802-8-kioplato@gmail.com","threadId":"57864","inReplyTo":"20220509175159.2948802-1-kioplato@gmail.com","subject":"[PATCH v2 07/15] t0066: better test coverage for dir-iterator","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-05-09T17:51:51Z","receivedAt":"2022-05-09T17:55:29Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"dir-iterator is lacking test coverage for some cases:\n * root directory and/or subdirectories w/o perms\n * pedantic runs on directories with cyclical symlinks\n * more thorough tests without sorting expected and actual outputs\n\nImprove test coverage by introducing tests that test ELOOP and EACCES\nerrno and various directory hierarchies that cover most cases.\n\nSigned-off-by: Plato Kiorpelidis \n---\n t/t0066-dir-iterator.sh | 290 +++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 289 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\nindex 61b02423d5..932b1f291c 100755\n--- a/t/t0066-dir-iterator.sh\n+++ b/t/t0066-dir-iterator.sh\n@@ -5,6 +5,97 @@ test_description='Test the dir-iterator functionality'\n TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n+test_expect_success 'setup -- dir with a single file' '\n+\tmkdir dir1 &&\n+\t>dir1/a\n+'\n+test_expect_success 'iteration of dir with a file' '\n+\tcat >expected-out <<-EOF &&\n+\t[f] (a) [a] ./dir1/a\n+\tEOF\n+\n+\ttest-tool dir-iterator ./dir1 >actual-out &&\n+\ttest_cmp expected-out actual-out\n+'\n+\n+test_expect_success 'setup -- dir with a single dir' '\n+\tmkdir -p dir6/a\n+'\n+test_expect_success 'iteration of dir with a single dir' '\n+\tcat >expected-out <<-EOF &&\n+\t[d] (a) [a] ./dir6/a\n+\tEOF\n+\n+\ttest-tool dir-iterator ./dir6 >actual-out &&\n+\ttest_cmp expected-out actual-out\n+'\n+\n+test_expect_success POSIXPERM,SANITY 'setup -- dir w/ single dir w/o perms' '\n+\tmkdir -p dir13/a\n+'\n+test_expect_success POSIXPERM,SANITY 'iteration of dir w/ dir w/o perms' '\n+\tcat >expected-out <<-EOF &&\n+\t[d] (a) [a] ./dir13/a\n+\tEOF\n+\n+\tchmod 0 dir13/a &&\n+\n+\ttest-tool dir-iterator ./dir13/ >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir13/a\n+'\n+test_expect_success POSIXPERM,SANITY 'pedantic iteration of dir w/ dir w/o perms' '\n+\tcat >expected-out <<-EOF &&\n+\t[d] (a) [a] ./dir13/a\n+\tdir_iterator_advance failure: EACCES\n+\tEOF\n+\n+\tchmod 0 dir13/a &&\n+\n+\ttest_must_fail test-tool dir-iterator --pedantic ./dir13/ >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir13/a\n+'\n+\n+test_expect_success 'setup -- dir w/ five files' '\n+\tmkdir dir14 &&\n+\t>dir14/a &&\n+\t>dir14/b &&\n+\t>dir14/c &&\n+\t>dir14/d &&\n+\t>dir14/e\n+'\n+test_expect_success 'iteration of dir w/ five files' '\n+\tcat >expected-sorted-out <<-EOF &&\n+\t[f] (a) [a] ./dir14/a\n+\t[f] (b) [b] ./dir14/b\n+\t[f] (c) [c] ./dir14/c\n+\t[f] (d) [d] ./dir14/d\n+\t[f] (e) [e] ./dir14/e\n+\tEOF\n+\n+\ttest-tool dir-iterator ./dir14 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-sorted-out actual-sorted-out\n+'\n+\n+test_expect_success 'setup -- dir w/ dir w/ a file' '\n+\tmkdir -p dir15/a &&\n+\t>dir15/a/b\n+'\n+test_expect_success 'iteration of dir w/ dir w/ a file' '\n+\tcat >expected-out <<-EOF &&\n+\t[d] (a) [a] ./dir15/a\n+\t[f] (a/b) [b] ./dir15/a/b\n+\tEOF\n+\n+\ttest-tool dir-iterator ./dir15 >actual-out &&\n+\ttest_cmp expected-out actual-out\n+'\n+\n test_expect_success 'setup -- dir w/ complex structure' '\n \tmkdir -p dir &&\n \tmkdir -p dir/a/b/c/ &&\n@@ -49,10 +140,178 @@ test_expect_success 'dir-iterator should list files in the correct order' '\n \tEOF\n \n \ttest-tool dir-iterator ./dir2 >actual-out &&\n-\n \ttest_cmp expected-out actual-out\n '\n \n+test_expect_success POSIXPERM,SANITY \\\n+'setup -- dir w/ three nested dirs w/ file, second nested dir w/o perms' '\n+\n+\tmkdir -p dir7/a/b/c &&\n+\t>dir7/a/b/c/d\n+'\n+test_expect_success POSIXPERM,SANITY \\\n+'iteration of dir w/ three nested dirs w/ file, second w/o perms' '\n+\n+\tcat >expected-out <<-EOF &&\n+\t[d] (a) [a] ./dir7/a\n+\t[d] (a/b) [b] ./dir7/a/b\n+\tEOF\n+\n+\tchmod 0 dir7/a/b &&\n+\n+\ttest-tool dir-iterator ./dir7 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir7/a/b\n+'\n+test_expect_success POSIXPERM,SANITY \\\n+'pedantic iteration of dir w/ three nested dirs w/ file, second w/o perms' '\n+\n+\tcat >expected-out <<-EOF &&\n+\t[d] (a) [a] ./dir7/a\n+\t[d] (a/b) [b] ./dir7/a/b\n+\tdir_iterator_advance failure: EACCES\n+\tEOF\n+\n+\tchmod 0 dir7/a/b &&\n+\n+\ttest_must_fail test-tool dir-iterator --pedantic ./dir7 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir7/a/b\n+'\n+\n+test_expect_success 'setup -- dir w/ two dirs each w/ file' '\n+\tmkdir -p dir8/a &&\n+\t>dir8/a/b &&\n+\tmkdir dir8/c &&\n+\t>dir8/c/d\n+'\n+test_expect_success 'iteration of dir w/ two dirs each w/ file' '\n+\tcat >expected-out1 <<-EOF &&\n+\t[d] (a) [a] ./dir8/a\n+\t[f] (a/b) [b] ./dir8/a/b\n+\t[d] (c) [c] ./dir8/c\n+\t[f] (c/d) [d] ./dir8/c/d\n+\tEOF\n+\tcat >expected-out2 <<-EOF &&\n+\t[d] (c) [c] ./dir8/c\n+\t[f] (c/d) [d] ./dir8/c/d\n+\t[d] (a) [a] ./dir8/a\n+\t[f] (a/b) [b] ./dir8/a/b\n+\tEOF\n+\n+\ttest-tool dir-iterator ./dir8 >actual-out &&\n+\t(\n+\t\ttest_cmp expected-out1 actual-out ||\n+\t\ttest_cmp expected-out2 actual-out\n+\t)\n+'\n+\n+test_expect_success 'setup -- dir w/ two dirs, one w/ two and one w/ one files' '\n+\tmkdir -p dir9/a &&\n+\t>dir9/a/b &&\n+\t>dir9/a/c &&\n+\tmkdir dir9/d &&\n+\t>dir9/d/e\n+'\n+test_expect_success \\\n+'iteration of dir w/ two dirs, one w/ two and one w/ one files' '\n+\n+\tcat >expected-out1 <<-EOF &&\n+\t[d] (a) [a] ./dir9/a\n+\t[f] (a/b) [b] ./dir9/a/b\n+\t[f] (a/c) [c] ./dir9/a/c\n+\t[d] (d) [d] ./dir9/d\n+\t[f] (d/e) [e] ./dir9/d/e\n+\tEOF\n+\tcat >expected-out2 <<-EOF &&\n+\t[d] (a) [a] ./dir9/a\n+\t[f] (a/c) [c] ./dir9/a/c\n+\t[f] (a/b) [b] ./dir9/a/b\n+\t[d] (d) [d] ./dir9/d\n+\t[f] (d/e) [e] ./dir9/d/e\n+\tEOF\n+\tcat >expected-out3 <<-EOF &&\n+\t[d] (d) [d] ./dir9/d\n+\t[f] (d/e) [e] ./dir9/d/e\n+\t[d] (a) [a] ./dir9/a\n+\t[f] (a/b) [b] ./dir9/a/b\n+\t[f] (a/c) [c] ./dir9/a/c\n+\tEOF\n+\tcat >expected-out4 <<-EOF &&\n+\t[d] (d) [d] ./dir9/d\n+\t[f] (d/e) [e] ./dir9/d/e\n+\t[d] (a) [a] ./dir9/a\n+\t[f] (a/c) [c] ./dir9/a/c\n+\t[f] (a/b) [b] ./dir9/a/b\n+\tEOF\n+\n+\ttest-tool dir-iterator ./dir9 >actual-out &&\n+\t(\n+\t\ttest_cmp expected-out1 actual-out ||\n+\t\ttest_cmp expected-out2 actual-out ||\n+\t\ttest_cmp expected-out3 actual-out ||\n+\t\ttest_cmp expected-out4 actual-out\n+\t)\n+'\n+\n+test_expect_success 'setup -- dir w/ two nested dirs, each w/ file' '\n+\tmkdir -p dir10/a &&\n+\t>dir10/a/b &&\n+\tmkdir dir10/a/c &&\n+\t>dir10/a/c/d\n+'\n+test_expect_success 'iteration of dir w/ two nested dirs, each w/ file' '\n+\tcat >expected-out1 <<-EOF &&\n+\t[d] (a) [a] ./dir10/a\n+\t[f] (a/b) [b] ./dir10/a/b\n+\t[d] (a/c) [c] ./dir10/a/c\n+\t[f] (a/c/d) [d] ./dir10/a/c/d\n+\tEOF\n+\tcat >expected-out2 <<-EOF &&\n+\t[d] (a) [a] ./dir10/a\n+\t[d] (a/c) [c] ./dir10/a/c\n+\t[f] (a/c/d) [d] ./dir10/a/c/d\n+\t[f] (a/b) [b] ./dir10/a/b\n+\tEOF\n+\n+\ttest-tool dir-iterator ./dir10/ >actual-out &&\n+\t(\n+\t\ttest_cmp expected-out1 actual-out ||\n+\t\ttest_cmp expected-out2 actual-out\n+\t)\n+'\n+\n+test_expect_success POSIXPERM,SANITY 'setup -- dir w/o perms' '\n+\tmkdir -p dir12/a &&\n+\t>dir12/a/b\n+'\n+test_expect_success POSIXPERM,SANITY 'iteration of root dir w/o perms' '\n+\tcat >expected-out <<-EOF &&\n+\tdir_iterator_advance failure: EACCES\n+\tEOF\n+\n+\tchmod 0 dir12 &&\n+\n+\ttest_must_fail test-tool dir-iterator ./dir12 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir12\n+'\n+test_expect_success POSIXPERM,SANITY 'pedantic iteration of root dir w/o perms' '\n+\tcat >expected-out <<-EOF &&\n+\tdir_iterator_advance failure: EACCES\n+\tEOF\n+\n+\tchmod 0 dir12 &&\n+\n+\ttest_must_fail test-tool dir-iterator --pedantic ./dir12 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir12\n+'\n+\n test_expect_success 'begin should fail upon inexistent paths' '\n \techo \"dir_iterator_begin failure: ENOENT\" >expected-out &&\n \n@@ -147,5 +406,34 @@ test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/ cycle' '\n \tln -s ../ dir5/a/b/e &&\n \tln -s ../../ dir5/a/b/f\n '\n+test_expect_success SYMLINKS 'iteration of dir w/ symlinks w/ cycle' '\n+\n+\tcat >expected-sorted-out <<-EOF &&\n+\t[d] (a) [a] ./dir5/a\n+\t[d] (a/b) [b] ./dir5/a/b\n+\t[d] (a/c) [c] ./dir5/a/c\n+\t[s] (a/b/d) [d] ./dir5/a/b/d\n+\t[s] (a/b/e) [e] ./dir5/a/b/e\n+\t[s] (a/b/f) [f] ./dir5/a/b/f\n+\tEOF\n+\n+\ttest-tool dir-iterator ./dir5 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-sorted-out actual-sorted-out\n+'\n+test_expect_success SYMLINKS \\\n+'pedantic follow-symlinks iteration of dir w/ symlinks w/ cycle' '\n+\n+\tcat >expected-tailed-out <<-EOF &&\n+\tdir_iterator_advance failure: ELOOP\n+\tEOF\n+\n+\ttest_must_fail test-tool dir-iterator \\\n+\t\t--pedantic --follow-symlinks ./dir5 >actual-out &&\n+\ttail -n 1 actual-out >actual-tailed-out &&\n+\n+\ttest_cmp expected-tailed-out actual-tailed-out\n+'\n \n test_done\n-- \n2.36.1\n\n"},{"id":"455041","messageId":"20220509175159.2948802-11-kioplato@gmail.com","threadId":"57864","inReplyTo":"20220509175159.2948802-1-kioplato@gmail.com","subject":"[PATCH v2 10/15] dir-iterator: refactor dir_iterator_advance()","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-05-09T17:51:54Z","receivedAt":"2022-05-09T17:55:33Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"Simplify dir_iterator_advance() by converting from iterative to\nrecursive implementation. This conversion makes dir-iterator more\nmaintainable for the following reasons:\n * dir-iterator iterates the file-system, which is a tree structure.\n Traditionally, tree traversals, in textbooks, lectures and online\n sources are implemented recursively and not iteratively. Therefore\n it helps reduce mental load for readers, since it's easier to follow\n as it reminds of the same tree traversals we use on tree structures.\n * recursion requires one less indentation depth because we get rid of\n the while loop and instead recurse, using the program's stack.\n * in each recursive step a set of instructions are executed and\n recursion lays out the code structurally in a better way, such that\n these instructions stand out symmetrically for each recursive step.\n\nThis makes dir-iterator easier to work with, understand and introduce\nnew functionality, like post-order on some directory entries, because it\nreminds us of the same operations we use to traverse tree structures.\n\nSigned-off-by: Plato Kiorpelidis \n---\n dir-iterator.c | 223 ++++++++++++++++++++++++++++++++-----------------\n 1 file changed, 146 insertions(+), 77 deletions(-)\n\ndiff --git a/dir-iterator.c b/dir-iterator.c\nindex b17e9f970a..3adcfbc966 100644\n--- a/dir-iterator.c\n+++ b/dir-iterator.c\n@@ -7,8 +7,7 @@ struct dir_iterator_level {\n \tDIR *dir;\n \n \t/*\n-\t * The length of the directory part of path at this level\n-\t * (including a trailing '/'):\n+\t * The length of the directory part of path at this level.\n \t */\n \tsize_t prefix_len;\n };\n@@ -34,8 +33,9 @@ struct dir_iterator_int {\n \tsize_t levels_alloc;\n \n \t/*\n-\t * A stack of levels. levels[0] is the uppermost directory\n-\t * that will be included in this iteration.\n+\t * A stack of levels. levels[0] is the root directory.\n+\t * It won't be included in the iteration, but iteration will happen\n+\t * inside it's subdirectories.\n \t */\n \tstruct dir_iterator_level *levels;\n \n@@ -43,36 +43,63 @@ struct dir_iterator_int {\n \tunsigned int flags;\n };\n \n+enum {\n+\tOK,\n+\tFAIL_ENOENT,\n+\tFAIL_NOT_ENOENT,\n+};\n+\n /*\n * Push a level in the iter stack and initialize it with information from\n- * the directory pointed by iter->base->path. It is assumed that this\n- * strbuf points to a valid directory path. Return 0 on success and -1\n- * otherwise, setting errno accordingly and leaving the stack unchanged.\n+ * the directory pointed by iter->base->path. Don't open the directory.\n+ *\n+ * Return 1 on success.\n+ * Return 0 when `iter->base->path` isn't a directory.\n */\n static int push_level(struct dir_iterator_int *iter)\n {\n \tstruct dir_iterator_level *level;\n \n+\tif (!S_ISDIR(iter->base.st.st_mode))\n+\t\treturn 0;\n+\n \tALLOC_GROW(iter->levels, iter->levels_nr + 1, iter->levels_alloc);\n \tlevel = &iter->levels[iter->levels_nr++];\n \n-\tif (!is_dir_sep(iter->base.path.buf[iter->base.path.len - 1]))\n-\t\tstrbuf_addch(&iter->base.path, '/');\n+\tlevel->dir = NULL;\n+\n \tlevel->prefix_len = iter->base.path.len;\n \n-\tlevel->dir = opendir(iter->base.path.buf);\n-\tif (!level->dir) {\n-\t\tint saved_errno = errno;\n-\t\tif (errno != ENOENT) {\n-\t\t\twarning_errno(\"error opening directory '%s'\",\n-\t\t\t\t iter->base.path.buf);\n-\t\t}\n-\t\titer->levels_nr--;\n+\treturn 1;\n+}\n+\n+/*\n+ * Activate most recent pushed level. Stack is unchanged.\n+ *\n+ * Return values:\n+ * OK on success.\n+ * FAIL_ENOENT on failed exposure because entry does not exist.\n+ * FAIL_NOT_ENOENT on failed exposure because of errno other than ENOENT.\n+ */\n+static int activate_level(struct dir_iterator_int *iter)\n+{\n+\tstruct dir_iterator_level *level = &iter->levels[iter->levels_nr - 1];\n+\tint saved_errno;\n+\n+\tif (level->dir)\n+\t\treturn OK;\n+\n+\tif ((level->dir = opendir(iter->base.path.buf)) != NULL)\n+\t\treturn OK;\n+\n+\tsaved_errno = errno;\n+\tif (errno != ENOENT) {\n+\t\twarning_errno(\"error opening directory '%s'\", iter->base.path.buf);\n \t\terrno = saved_errno;\n-\t\treturn -1;\n+\t\treturn FAIL_NOT_ENOENT;\n \t}\n-\n-\treturn 0;\n+\terrno = saved_errno;\n+\treturn FAIL_ENOENT;\n }\n \n /*\n@@ -81,12 +108,10 @@ static int push_level(struct dir_iterator_int *iter)\n */\n static int pop_level(struct dir_iterator_int *iter)\n {\n-\tstruct dir_iterator_level *level =\n-\t\t&iter->levels[iter->levels_nr - 1];\n+\tstruct dir_iterator_level *level = &iter->levels[iter->levels_nr - 1];\n \n \tif (level->dir && closedir(level->dir))\n-\t\twarning_errno(\"error closing directory '%s'\",\n-\t\t\t iter->base.path.buf);\n+\t\twarning_errno(\"error closing directory '%s'\", iter->base.path.buf);\n \tlevel->dir = NULL;\n \n \treturn --iter->levels_nr;\n@@ -94,82 +119,119 @@ static int pop_level(struct dir_iterator_int *iter)\n \n /*\n * Populate iter->base with the necessary information on the next iteration\n- * entry, represented by the given dirent de. Return 0 on success and -1\n- * otherwise, setting errno accordingly.\n+ * entry, represented by the given relative path to the lowermost directory,\n+ * d_name.\n+ *\n+ * Return values:\n+ * OK on successful exposure of the provided entry.\n+ * FAIL_ENOENT on failed exposure because entry does not exist.\n+ * FAIL_NOT_ENOENT on failed exposure because of errno other than ENOENT.\n */\n-static int prepare_next_entry_data(struct dir_iterator_int *iter,\n-\t\t\t\t struct dirent *de)\n+static int expose_entry(struct dir_iterator_int *iter, char *d_name)\n {\n-\tint err, saved_errno;\n+\tint stat_err;\n \n-\tstrbuf_addstr(&iter->base.path, de->d_name);\n-\t/*\n-\t * We have to reset these because the path strbuf might have\n-\t * been realloc()ed at the previous strbuf_addstr().\n-\t */\n-\titer->base.relative_path = iter->base.path.buf +\n-\t\t\t\t iter->levels[0].prefix_len;\n-\titer->base.basename = iter->base.path.buf +\n-\t\t\t iter->levels[iter->levels_nr - 1].prefix_len;\n+\tstrbuf_addch(&iter->base.path, '/');\n+\tstrbuf_addstr(&iter->base.path, d_name);\n \n \tif (iter->flags & DIR_ITERATOR_FOLLOW_SYMLINKS)\n-\t\terr = stat(iter->base.path.buf, &iter->base.st);\n+\t\tstat_err = stat(iter->base.path.buf, &iter->base.st);\n \telse\n-\t\terr = lstat(iter->base.path.buf, &iter->base.st);\n+\t\tstat_err = lstat(iter->base.path.buf, &iter->base.st);\n \n-\tsaved_errno = errno;\n-\tif (err && errno != ENOENT)\n+\tif (stat_err && errno != ENOENT) {\n \t\twarning_errno(\"failed to stat '%s'\", iter->base.path.buf);\n+\t\treturn FAIL_NOT_ENOENT;\n+\t} else if (stat_err && errno == ENOENT) {\n+\t\treturn FAIL_ENOENT;\n+\t}\n \n-\terrno = saved_errno;\n-\treturn err;\n+\t/*\n+\t * We have to reset relative path and basename because the path strbuf\n+\t * might have been realloc()'ed at the previous strbuf_addstr().\n+\t */\n+\n+\titer->base.relative_path =\n+\t\titer->base.path.buf + iter->levels[0].prefix_len + 1;\n+\titer->base.basename =\n+\t\titer->base.path.buf + iter->levels[iter->levels_nr - 1].prefix_len + 1;\n+\n+\treturn OK;\n }\n \n int dir_iterator_advance(struct dir_iterator *dir_iterator)\n {\n-\tstruct dir_iterator_int *iter =\n-\t\t(struct dir_iterator_int *)dir_iterator;\n+\tstruct dir_iterator_int *iter = (struct dir_iterator_int *)dir_iterator;\n+\tstruct dir_iterator_level *level = &iter->levels[iter->levels_nr - 1];\n+\tstruct dirent *dir_entry = NULL;\n+\tint expose_err, activate_err;\n+\t/* For shorter code width-wise, more readable */\n+\tunsigned int PEDANTIC = iter->flags & DIR_ITERATOR_PEDANTIC;\n \n-\tif (S_ISDIR(iter->base.st.st_mode) && push_level(iter)) {\n-\t\tif (errno != ENOENT && iter->flags & DIR_ITERATOR_PEDANTIC)\n-\t\t\tgoto error_out;\n-\t\tif (iter->levels_nr == 0)\n+\t/*\n+\t * Attempt to open the directory of the last level if not opened yet.\n+\t *\n+\t * Remember that we ignore ENOENT errors so that the user of this API\n+\t * can remove entries between calls to `dir_iterator_advance()`.\n+\t * We care for errors other than ENOENT only when PEDANTIC is enabled.\n+\t */\n+\n+\tactivate_err = activate_level(iter);\n+\n+\tif (activate_err == FAIL_NOT_ENOENT && PEDANTIC) {\n+\t\tgoto error_out;\n+\t} else if (activate_err != OK) {\n+\t\t--iter->levels_nr;\n+\n+\t\tif (iter->levels_nr == 0) /* Failed to open root directory */\n \t\t\tgoto error_out;\n+\n+\t\treturn dir_iterator_advance(dir_iterator);\n \t}\n \n-\t/* Loop until we find an entry that we can give back to the caller. */\n-\twhile (1) {\n-\t\tstruct dirent *de;\n-\t\tstruct dir_iterator_level *level =\n-\t\t\t&iter->levels[iter->levels_nr - 1];\n+\tstrbuf_setlen(&iter->base.path, level->prefix_len);\n+\n+\terrno = 0;\n+\tdir_entry = readdir(level->dir);\n+\n+\tif (!dir_entry) {\n+\t\tif (errno) {\n+\t\t\twarning_errno(\"errno reading dir '%s'\", iter->base.path.buf);\n+\t\t\tif (PEDANTIC)\n+\t\t\t\tgoto error_out;\n \n-\t\tstrbuf_setlen(&iter->base.path, level->prefix_len);\n-\t\terrno = 0;\n-\t\tde = readdir(level->dir);\n-\n-\t\tif (!de) {\n-\t\t\tif (errno) {\n-\t\t\t\twarning_errno(\"error reading directory '%s'\",\n-\t\t\t\t\t iter->base.path.buf);\n-\t\t\t\tif (iter->flags & DIR_ITERATOR_PEDANTIC)\n-\t\t\t\t\tgoto error_out;\n-\t\t\t} else if (pop_level(iter) == 0) {\n+\t\t\treturn dir_iterator_advance(dir_iterator);\n+\t\t} else {\n+\t\t\t/*\n+\t\t\t * Current directory has been iterated through.\n+\t\t\t */\n+\n+\t\t\tif (pop_level(iter) == 0)\n \t\t\t\treturn dir_iterator_abort(dir_iterator);\n-\t\t\t}\n-\t\t\tcontinue;\n+\n+\t\t\treturn dir_iterator_advance(dir_iterator);\n \t\t}\n+\t}\n \n-\t\tif (is_dot_or_dotdot(de->d_name))\n-\t\t\tcontinue;\n+\tif (is_dot_or_dotdot(dir_entry->d_name))\n+\t\treturn dir_iterator_advance(dir_iterator);\n \n-\t\tif (prepare_next_entry_data(iter, de)) {\n-\t\t\tif (errno != ENOENT && iter->flags & DIR_ITERATOR_PEDANTIC)\n-\t\t\t\tgoto error_out;\n-\t\t\tcontinue;\n-\t\t}\n+\t/*\n+\t * Successfully read entry from current directory level.\n+\t */\n \n-\t\treturn ITER_OK;\n-\t}\n+\texpose_err = expose_entry(iter, dir_entry->d_name);\n+\n+\tif (expose_err == FAIL_NOT_ENOENT && PEDANTIC)\n+\t\tgoto error_out;\n+\n+\tif (expose_err == OK)\n+\t\tpush_level(iter);\n+\n+\tif (expose_err != OK)\n+\t\treturn dir_iterator_advance(dir_iterator);\n+\n+\treturn ITER_OK;\n \n error_out:\n \tdir_iterator_abort(dir_iterator);\n@@ -207,6 +269,8 @@ struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags)\n \n \tstrbuf_init(&iter->base.path, PATH_MAX);\n \tstrbuf_addstr(&iter->base.path, path);\n+\t/* expose_entry() appends dir seperator before exposing an entry */\n+\tstrbuf_trim_trailing_dir_sep(&iter->base.path);\n \n \tALLOC_GROW(iter->levels, 10, iter->levels_alloc);\n \titer->levels_nr = 0;\n@@ -226,6 +290,11 @@ struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags)\n \t\tgoto error_out;\n \t}\n \n+\tif (!push_level(iter)) {\n+\t\tsaved_errno = ENOTDIR;\n+\t\tgoto error_out;\n+\t}\n+\n \treturn dir_iterator;\n \n error_out:\n-- \n2.36.1\n\n"},{"id":"455042","messageId":"20220509175159.2948802-12-kioplato@gmail.com","threadId":"57864","inReplyTo":"20220509175159.2948802-1-kioplato@gmail.com","subject":"[PATCH v2 11/15] dir-iterator: open root dir in dir_iterator_begin()","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-05-09T17:51:55Z","receivedAt":"2022-05-09T17:55:34Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"When we call dir_iterator_begin() and the provided path is invalid, the\ncall won't fail and return NULL. Instead, the call succeeds, and the\nfirst call dir_iterator_advance() fails. That's unexpected behavior from\nthe perspective of a user of this API. The expected behavior would be to\nfail and return NULL from dir_iterator_begin() if the provided path is\ninvalid. Successful call to dir_iterator_begin() suggests that the root\npath is valid.\n\nTo deal with this, call activate_level() in dir_iterator_begin() which\nopens the provided root path and confirms it's a valid directory path.\nBy doing this we can return NULL from dir_iterator_begin() in case the\nprovided root path isn't valid, implementing the expected behavior.\n\nSigned-off-by: Plato Kiorpelidis \n---\n dir-iterator.c | 13 ++++++++++---\n t/t0066-dir-iterator.sh | 4 ++--\n 2 files changed, 12 insertions(+), 5 deletions(-)\n\ndiff --git a/dir-iterator.c b/dir-iterator.c\nindex 3adcfbc966..c36f549a78 100644\n--- a/dir-iterator.c\n+++ b/dir-iterator.c\n@@ -181,10 +181,12 @@ int dir_iterator_advance(struct dir_iterator *dir_iterator)\n \tif (activate_err == FAIL_NOT_ENOENT && PEDANTIC) {\n \t\tgoto error_out;\n \t} else if (activate_err != OK) {\n-\t\t--iter->levels_nr;\n+\t\t/*\n+\t\t * We activate the root level at `dir_iterator_begin()`.\n+\t\t * Therefore, there isn't any case to run out of levels.\n+\t\t */\n \n-\t\tif (iter->levels_nr == 0) /* Failed to open root directory */\n-\t\t\tgoto error_out;\n+\t\t--iter->levels_nr;\n \n \t\treturn dir_iterator_advance(dir_iterator);\n \t}\n@@ -295,6 +297,11 @@ struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags)\n \t\tgoto error_out;\n \t}\n \n+\tif (activate_level(iter) != OK) {\n+\t\tsaved_errno = errno;\n+\t\tgoto error_out;\n+\t}\n+\n \treturn dir_iterator;\n \n error_out:\ndiff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\nindex 04e51928bc..52b0217bde 100755\n--- a/t/t0066-dir-iterator.sh\n+++ b/t/t0066-dir-iterator.sh\n@@ -289,7 +289,7 @@ test_expect_success POSIXPERM,SANITY 'setup -- dir w/o perms' '\n '\n test_expect_success POSIXPERM,SANITY 'iteration of root dir w/o perms' '\n \tcat >expected-out <<-EOF &&\n-\tdir_iterator_advance failure: EACCES\n+\tdir_iterator_begin failure: EACCES\n \tEOF\n \n \tchmod 0 dir12 &&\n@@ -301,7 +301,7 @@ test_expect_success POSIXPERM,SANITY 'iteration of root dir w/o perms' '\n '\n test_expect_success POSIXPERM,SANITY 'pedantic iteration of root dir w/o perms' '\n \tcat >expected-out <<-EOF &&\n-\tdir_iterator_advance failure: EACCES\n+\tdir_iterator_begin failure: EACCES\n \tEOF\n \n \tchmod 0 dir12 &&\n-- \n2.36.1\n\n"},{"id":"455043","messageId":"20220509175159.2948802-13-kioplato@gmail.com","threadId":"57864","inReplyTo":"20220509175159.2948802-1-kioplato@gmail.com","subject":"[PATCH v2 12/15] t0066: rename subtest descriptions","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-05-09T17:51:56Z","receivedAt":"2022-05-09T17:55:40Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"Improve the structure of dir-iterator tests by renaming subtest\ndescriptions. This makes the introduction of new tests for future\niteration schemes easier, as they will share similar naming, only\ndifferentiating over the iteration scheme part of the description\nthat the subtest is testing for.\n\nThese structured subtest descriptions also help when running the tests.\nIt's easier to understand what's tested and in case a test fails find\nout which iteration scheme failed.\n\nSigned-off-by: Plato Kiorpelidis \n---\n t/t0066-dir-iterator.sh | 49 +++++++++++++++++++++++------------------\n 1 file changed, 27 insertions(+), 22 deletions(-)\n\ndiff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\nindex 52b0217bde..8ab8811fb5 100755\n--- a/t/t0066-dir-iterator.sh\n+++ b/t/t0066-dir-iterator.sh\n@@ -9,7 +9,7 @@ test_expect_success 'setup -- dir with a single file' '\n \tmkdir dir1 &&\n \t>dir1/a\n '\n-test_expect_success 'iteration of dir with a file' '\n+test_expect_success 'dirs-before of dir with a file' '\n \tcat >expected-out <<-EOF &&\n \t[f] (a) [a] ./dir1/a\n \tEOF\n@@ -21,7 +21,7 @@ test_expect_success 'iteration of dir with a file' '\n test_expect_success 'setup -- dir with a single dir' '\n \tmkdir -p dir2/a\n '\n-test_expect_success 'iteration of dir with a single dir' '\n+test_expect_success 'dirs-before of dir with a single dir' '\n \tcat >expected-out <<-EOF &&\n \t[d] (a) [a] ./dir2/a\n \tEOF\n@@ -33,7 +33,7 @@ test_expect_success 'iteration of dir with a single dir' '\n test_expect_success POSIXPERM,SANITY 'setup -- dir w/ single dir w/o perms' '\n \tmkdir -p dir3/a\n '\n-test_expect_success POSIXPERM,SANITY 'iteration of dir w/ dir w/o perms' '\n+test_expect_success POSIXPERM,SANITY 'dirs-before of dir w/ dir w/o perms' '\n \tcat >expected-out <<-EOF &&\n \t[d] (a) [a] ./dir3/a\n \tEOF\n@@ -45,7 +45,7 @@ test_expect_success POSIXPERM,SANITY 'iteration of dir w/ dir w/o perms' '\n \n \tchmod 755 dir3/a\n '\n-test_expect_success POSIXPERM,SANITY 'pedantic iteration of dir w/ dir w/o perms' '\n+test_expect_success POSIXPERM,SANITY 'pedantic dirs-before of dir w/ dir w/o perms' '\n \tcat >expected-out <<-EOF &&\n \t[d] (a) [a] ./dir3/a\n \tdir_iterator_advance failure: EACCES\n@@ -67,7 +67,7 @@ test_expect_success 'setup -- dir w/ five files' '\n \t>dir4/d &&\n \t>dir4/e\n '\n-test_expect_success 'iteration of dir w/ five files' '\n+test_expect_success 'dirs-before of dir w/ five files' '\n \tcat >expected-sorted-out <<-EOF &&\n \t[f] (a) [a] ./dir4/a\n \t[f] (b) [b] ./dir4/b\n@@ -86,7 +86,7 @@ test_expect_success 'setup -- dir w/ dir w/ a file' '\n \tmkdir -p dir5/a &&\n \t>dir5/a/b\n '\n-test_expect_success 'iteration of dir w/ dir w/ a file' '\n+test_expect_success 'dirs-before of dir w/ dir w/ a file' '\n \tcat >expected-out <<-EOF &&\n \t[d] (a) [a] ./dir5/a\n \t[f] (a/b) [b] ./dir5/a/b\n@@ -100,7 +100,7 @@ test_expect_success 'setup -- dir w/ three nested dirs w/ file' '\n \tmkdir -p dir6/a/b/c &&\n \t>dir6/a/b/c/d\n '\n-test_expect_success 'dir-iterator should list files in the correct order' '\n+test_expect_success 'dirs-before of dir w/ three nested dirs w/ file' '\n \tcat >expected-out <<-EOF &&\n \t[d] (a) [a] ./dir6/a\n \t[d] (a/b) [b] ./dir6/a/b\n@@ -119,7 +119,7 @@ test_expect_success POSIXPERM,SANITY \\\n \t>dir7/a/b/c/d\n '\n test_expect_success POSIXPERM,SANITY \\\n-'iteration of dir w/ three nested dirs w/ file, second w/o perms' '\n+'dirs-before of dir w/ three nested dirs w/ file, second w/o perms' '\n \n \tcat >expected-out <<-EOF &&\n \t[d] (a) [a] ./dir7/a\n@@ -134,7 +134,7 @@ test_expect_success POSIXPERM,SANITY \\\n \tchmod 755 dir7/a/b\n '\n test_expect_success POSIXPERM,SANITY \\\n-'pedantic iteration of dir w/ three nested dirs w/ file, second w/o perms' '\n+'pedantic dirs-before of dir w/ three nested dirs w/ file, second w/o perms' '\n \n \tcat >expected-out <<-EOF &&\n \t[d] (a) [a] ./dir7/a\n@@ -156,7 +156,7 @@ test_expect_success 'setup -- dir w/ two dirs each w/ file' '\n \tmkdir dir8/c &&\n \t>dir8/c/d\n '\n-test_expect_success 'iteration of dir w/ two dirs each w/ file' '\n+test_expect_success 'dirs-before of dir w/ two dirs each w/ file' '\n \tcat >expected-out1 <<-EOF &&\n \t[d] (a) [a] ./dir8/a\n \t[f] (a/b) [b] ./dir8/a/b\n@@ -185,7 +185,7 @@ test_expect_success 'setup -- dir w/ two dirs, one w/ two and one w/ one files'\n \t>dir9/d/e\n '\n test_expect_success \\\n-'iteration of dir w/ two dirs, one w/ two and one w/ one files' '\n+'dirs-before of dir w/ two dirs, one w/ two and one w/ one files' '\n \n \tcat >expected-out1 <<-EOF &&\n \t[d] (a) [a] ./dir9/a\n@@ -231,7 +231,7 @@ test_expect_success 'setup -- dir w/ two nested dirs, each w/ file' '\n \tmkdir dir10/a/c &&\n \t>dir10/a/c/d\n '\n-test_expect_success 'iteration of dir w/ two nested dirs, each w/ file' '\n+test_expect_success 'dirs-before of dir w/ two nested dirs, each w/ file' '\n \tcat >expected-out1 <<-EOF &&\n \t[d] (a) [a] ./dir10/a\n \t[f] (a/b) [b] ./dir10/a/b\n@@ -262,7 +262,7 @@ test_expect_success 'setup -- dir w/ complex structure' '\n \t>dir11/a/e &&\n \t>dir11/d/e/d/a\n '\n-test_expect_success 'dir-iterator should iterate through all files' '\n+test_expect_success 'dirs-before of dir w/ complex structure' '\n \tcat >expected-sorted-out <<-EOF &&\n \t[d] (a) [a] ./dir11/a\n \t[d] (a/b) [b] ./dir11/a/b\n@@ -287,7 +287,7 @@ test_expect_success POSIXPERM,SANITY 'setup -- dir w/o perms' '\n \tmkdir -p dir12/a &&\n \t>dir12/a/b\n '\n-test_expect_success POSIXPERM,SANITY 'iteration of root dir w/o perms' '\n+test_expect_success POSIXPERM,SANITY 'dirs-before of root dir w/o perms' '\n \tcat >expected-out <<-EOF &&\n \tdir_iterator_begin failure: EACCES\n \tEOF\n@@ -299,7 +299,7 @@ test_expect_success POSIXPERM,SANITY 'iteration of root dir w/o perms' '\n \n \tchmod 755 dir12\n '\n-test_expect_success POSIXPERM,SANITY 'pedantic iteration of root dir w/o perms' '\n+test_expect_success POSIXPERM,SANITY 'pedantic dirs-before of root dir w/o perms' '\n \tcat >expected-out <<-EOF &&\n \tdir_iterator_begin failure: EACCES\n \tEOF\n@@ -334,7 +334,9 @@ test_expect_success POSIXPERM,SANITY 'setup -- dir w/ dir w/o perms w/ file' '\n \tmkdir -p dir13/a &&\n \t>dir13/a/b\n '\n-test_expect_success POSIXPERM,SANITY 'advance should not fail on errors by default' '\n+test_expect_success POSIXPERM,SANITY \\\n+'dirs-before of dir w/ dir w/o perms w/ file' '\n+\n \tcat >expected-out <<-EOF &&\n \t[d] (a) [a] ./dir13/a\n \tEOF\n@@ -346,7 +348,9 @@ test_expect_success POSIXPERM,SANITY 'advance should not fail on errors by defau\n \n \tchmod 755 dir13/a\n '\n-test_expect_success POSIXPERM,SANITY 'advance should fail on errors, w/ pedantic flag' '\n+test_expect_success POSIXPERM,SANITY \\\n+'pedantic dirs-before of dir w/ dir w/o perms w/ file' '\n+\n \tcat >expected-out <<-EOF &&\n \t[d] (a) [a] ./dir13/a\n \tdir_iterator_advance failure: EACCES\n@@ -367,7 +371,7 @@ test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/o cycle' '\n \tln -s d dir14/a/e &&\n \tln -s ../b dir14/a/f\n '\n-test_expect_success SYMLINKS 'dir-iterator should not follow symlinks by default' '\n+test_expect_success SYMLINKS 'dirs-before of dir w/ symlinks w/o cycle' '\n \tcat >expected-sorted-out <<-EOF &&\n \t[d] (a) [a] ./dir14/a\n \t[d] (b) [b] ./dir14/b\n@@ -382,7 +386,9 @@ test_expect_success SYMLINKS 'dir-iterator should not follow symlinks by default\n \n \ttest_cmp expected-sorted-out actual-sorted-out\n '\n-test_expect_success SYMLINKS 'dir-iterator should follow symlinks w/ follow flag' '\n+test_expect_success SYMLINKS \\\n+'follow-symlinks dirs-before of dir w/ symlinks w/o cycle' '\n+\n \tcat >expected-sorted-out <<-EOF &&\n \t[d] (a) [a] ./dir14/a\n \t[d] (a/f) [f] ./dir14/a/f\n@@ -406,8 +412,7 @@ test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/ cycle' '\n \tln -s ../ dir15/a/b/e &&\n \tln -s ../../ dir15/a/b/f\n '\n-test_expect_success SYMLINKS 'iteration of dir w/ symlinks w/ cycle' '\n-\n+test_expect_success SYMLINKS 'dirs-before of dir w/ symlinks w/ cycle' '\n \tcat >expected-sorted-out <<-EOF &&\n \t[d] (a) [a] ./dir15/a\n \t[d] (a/b) [b] ./dir15/a/b\n@@ -423,7 +428,7 @@ test_expect_success SYMLINKS 'iteration of dir w/ symlinks w/ cycle' '\n \ttest_cmp expected-sorted-out actual-sorted-out\n '\n test_expect_success SYMLINKS \\\n-'pedantic follow-symlinks iteration of dir w/ symlinks w/ cycle' '\n+'pedantic follow-symlinks dirs-before of dir w/ symlinks w/ cycle' '\n \n \tcat >expected-tailed-out <<-EOF &&\n \tdir_iterator_advance failure: ELOOP\n-- \n2.36.1\n\n"},{"id":"455044","messageId":"20220509175159.2948802-16-kioplato@gmail.com","threadId":"57864","inReplyTo":"20220509175159.2948802-1-kioplato@gmail.com","subject":"[PATCH v2 15/15] entry.c: use dir-iterator to avoid explicit dir traversal","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-05-09T17:51:59Z","receivedAt":"2022-05-09T17:55:52Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"Replace usage of opendir/readdir/closedir API to traverse directories\nrecursively, at remove_subtree() function, by the dir-iterator API. This\nsimplifies the code and avoids recursive calls to remove_subtree().\n\nSigned-off-by: Plato Kiorpelidis \n---\n entry.c | 38 +++++++++++++++++++-------------------\n 1 file changed, 19 insertions(+), 19 deletions(-)\n\ndiff --git a/entry.c b/entry.c\nindex 1c9df62b30..46ef145f97 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -8,6 +8,8 @@\n #include \"fsmonitor.h\"\n #include \"entry.h\"\n #include \"parallel-checkout.h\"\n+#include \"iterator.h\"\n+#include \"dir-iterator.h\"\n \n static void create_directories(const char *path, int path_len,\n \t\t\t const struct checkout *state)\n@@ -52,26 +54,24 @@ static void create_directories(const char *path, int path_len,\n \n static void remove_subtree(struct strbuf *path)\n {\n-\tDIR *dir = opendir(path->buf);\n-\tstruct dirent *de;\n-\tint origlen = path->len;\n-\n-\tif (!dir)\n-\t\tdie_errno(\"cannot opendir '%s'\", path->buf);\n-\twhile ((de = readdir_skip_dot_and_dotdot(dir)) != NULL) {\n-\t\tstruct stat st;\n-\n-\t\tstrbuf_addch(path, '/');\n-\t\tstrbuf_addstr(path, de->d_name);\n-\t\tif (lstat(path->buf, &st))\n-\t\t\tdie_errno(\"cannot lstat '%s'\", path->buf);\n-\t\tif (S_ISDIR(st.st_mode))\n-\t\t\tremove_subtree(path);\n-\t\telse if (unlink(path->buf))\n-\t\t\tdie_errno(\"cannot unlink '%s'\", path->buf);\n-\t\tstrbuf_setlen(path, origlen);\n+\tunsigned int flags = DIR_ITERATOR_DIRS_AFTER;\n+\tstruct dir_iterator *iter = NULL;\n+\tint ok;\n+\n+\tif (!(iter = dir_iterator_begin(path->buf, flags)))\n+\t\tdie_errno(\"cannot open directory '%s'\", path->buf);\n+\n+\twhile ((ok = dir_iterator_advance(iter)) == ITER_OK) {\n+\t\tif (S_ISDIR(iter->st.st_mode) && rmdir(iter->path.buf))\n+\t\t\tdie_errno(\"cannot rmdir '%s'\", iter->path.buf);\n+\n+\t\tif (!S_ISDIR(iter->st.st_mode) && unlink(iter->path.buf))\n+\t\t\tdie_errno(\"cannot unlink '%s'\", iter->path.buf);\n \t}\n-\tclosedir(dir);\n+\n+\tif (ok != ITER_DONE)\n+\t\tdie_errno(\"failed to iterate over directory '%s'\", path->buf);\n+\n \tif (rmdir(path->buf))\n \t\tdie_errno(\"cannot rmdir '%s'\", path->buf);\n }\n-- \n2.36.1\n\n"},{"id":"455045","messageId":"20220509175159.2948802-14-kioplato@gmail.com","threadId":"57864","inReplyTo":"20220509175159.2948802-1-kioplato@gmail.com","subject":"[PATCH v2 13/15] dir-iterator: option to iterate dirs in pre-order","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-05-09T17:51:57Z","receivedAt":"2022-05-09T17:55:54Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"Introduce a new option to dir-iterator, using dir_iterator_begin()\nflags parameter, allowing to control whether or not directories will\nbe exposed before their contents. In essence, pre-order traversal over\nfile system entries that are directories.\n\nThis changes the default behavior of the dir-iterator API. Instead\nof iterating directories before doing so over their contents, the new\ndefault behavior does not expose directories at all. Iteration is still\nperformed, however, within directories, iterating over any other entry.\nOnly directory paths are ignored.\n\nTo iterate over directories in pre-order, reproducing the previous\ndefault behavior, enable the new flag DIR_ITERATOR_DIRS_BEFORE in the\nflags parameter of dir_iterator_begin():\n * ignore directories by not setting DIR_ITERATOR_DIRS_BEFORE\n * iterate directories pre-order by enabling DIR_ITERATOR_DIRS_BEFORE\n\nAdjust existing callers, in refs/files-backend.c and builtin/clone.c\nto enable DIR_ITERATOR_DIRS_BEFORE since these callers require iteration\nover directories before doing so over their contents.\n\nUpdate t/t0066-dir-iterator.sh and t/helper/test-dir-iterator.c to test\nthe new iteration scheme, which is the new default behavior, and the new\nflag DIR_ITERATOR_DIRS_BEFORE which reproduces the old default behavior.\n\nSigned-off-by: Plato Kiorpelidis \n---\n builtin/clone.c | 4 +-\n dir-iterator.c | 29 +++-\n dir-iterator.h | 29 +++-\n refs/files-backend.c | 2 +-\n t/helper/test-dir-iterator.c | 12 +-\n t/t0066-dir-iterator.sh | 321 ++++++++++++++++++++++++++++++++---\n 6 files changed, 361 insertions(+), 36 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 194d50f75f..68787623e7 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -321,7 +321,9 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,\n \n \tmkdir_if_missing(dest->buf, 0777);\n \n-\tflags = DIR_ITERATOR_PEDANTIC | DIR_ITERATOR_FOLLOW_SYMLINKS;\n+\tflags = DIR_ITERATOR_DIRS_BEFORE |\n+\t\tDIR_ITERATOR_PEDANTIC |\n+\t\tDIR_ITERATOR_FOLLOW_SYMLINKS;\n \titer = dir_iterator_begin(src->buf, flags);\n \n \tif (!iter)\ndiff --git a/dir-iterator.c b/dir-iterator.c\nindex c36f549a78..c1475add27 100644\n--- a/dir-iterator.c\n+++ b/dir-iterator.c\n@@ -47,6 +47,7 @@ enum {\n \tOK,\n \tFAIL_ENOENT,\n \tFAIL_NOT_ENOENT,\n+\tFAIL_IGN_DIRS,\n };\n \n /*\n@@ -124,12 +125,14 @@ static int pop_level(struct dir_iterator_int *iter)\n *\n * Return values:\n * OK on successful exposure of the provided entry.\n+ * FAIL_IGN_DIR on failed exposure because entry is dir and flags don't allow it.\n * FAIL_ENOENT on failed exposure because entry does not exist.\n * FAIL_NOT_ENOENT on failed exposure because of errno other than ENOENT.\n */\n-static int expose_entry(struct dir_iterator_int *iter, char *d_name)\n+static int expose_entry(struct dir_iterator_int *iter, char *d_name, char *dir_state)\n {\n \tint stat_err;\n+\tunsigned int DIRS_BEFORE = iter->flags & DIR_ITERATOR_DIRS_BEFORE;\n \n \tstrbuf_addch(&iter->base.path, '/');\n \tstrbuf_addstr(&iter->base.path, d_name);\n@@ -146,6 +149,17 @@ static int expose_entry(struct dir_iterator_int *iter, char *d_name)\n \t\treturn FAIL_ENOENT;\n \t}\n \n+\t/*\n+\t * We've got to check whether or not this is a directory. We need to\n+\t * perform this check since the user could've requested to ignore\n+\t * directory entries.\n+\t */\n+\n+\tif (S_ISDIR(iter->base.st.st_mode)) {\n+\t\tif (!DIRS_BEFORE && !strcmp(dir_state, \"before\"))\n+\t\t\treturn FAIL_IGN_DIRS;\n+\t}\n+\n \t/*\n \t * We have to reset relative path and basename because the path strbuf\n \t * might have been realloc()'ed at the previous strbuf_addstr().\n@@ -220,14 +234,23 @@ int dir_iterator_advance(struct dir_iterator *dir_iterator)\n \n \t/*\n \t * Successfully read entry from current directory level.\n+\t * In case it's a directory, we need to check, before exposing it, if\n+\t * it's allowed because of DIRS_BEFORE. In any case - allowed or not -\n+\t * we must push the directory to the levels stack, so the next call will\n+\t * read from it.\n+\t */\n+\n+\t/*\n+\t * 'expose_entry()' function needs to know whether\n+\t * the exposure call is about DIRS_BEFORE or DIRS_AFTER.\n \t */\n \n-\texpose_err = expose_entry(iter, dir_entry->d_name);\n+\texpose_err = expose_entry(iter, dir_entry->d_name, \"before\");\n \n \tif (expose_err == FAIL_NOT_ENOENT && PEDANTIC)\n \t\tgoto error_out;\n \n-\tif (expose_err == OK)\n+\tif (expose_err == OK || expose_err == FAIL_IGN_DIRS)\n \t\tpush_level(iter);\n \n \tif (expose_err != OK)\ndiff --git a/dir-iterator.h b/dir-iterator.h\nindex 08229157c6..c1d16a8c6d 100644\n--- a/dir-iterator.h\n+++ b/dir-iterator.h\n@@ -8,19 +8,22 @@\n *\n * Iterate over a directory tree, recursively, including paths of all\n * types and hidden paths. Skip \".\" and \"..\" entries and don't follow\n- * symlinks except for the original path. Note that the original path\n- * is not included in the iteration.\n+ * symlinks except when DIR_ITERATOR_FOLLOW_SYMLINKS is set. If root\n+ * path is a symlink it's followed regardless of flags. Note that the\n+ * original path is not included in the iteration.\n *\n * Every time dir_iterator_advance() is called, update the members of\n * the dir_iterator structure to reflect the next path in the\n * iteration. The order that paths are iterated over within a\n- * directory is undefined, directory paths are always given before\n- * their contents.\n+ * directory is undefined. Directory paths are given before their\n+ * contents when DIR_ITERATOR_DIRS_BEFORE is set. Failure to set this\n+ * flag results in directory paths not being exposed. Instead, iteration\n+ * will happen within directories. Their contents will be exposed.\n *\n * A typical iteration looks like this:\n *\n * int ok;\n- * unsigned int flags = DIR_ITERATOR_PEDANTIC;\n+ * unsigned int flags = DIR_ITERATOR_PEDANTIC | DIR_ITERATOR_DIRS_BEFORE;\n * struct dir_iterator *iter = dir_iterator_begin(path, flags);\n *\n * if (!iter)\n@@ -61,12 +64,19 @@\n * not the symlinks themselves, which is the default behavior. Broken\n * symlinks are ignored.\n *\n+ * - DIR_ITERATOR_DIRS_BEFORE: make dir-iterator expose a directory path\n+ * before iterating through that directory's contents.\n+ *\n+ * Note: Activating none of the flags will iterate through directories'\n+ * contents but won't expose the directory paths.\n+ *\n * Warning: circular symlinks are also followed when\n * DIR_ITERATOR_FOLLOW_SYMLINKS is set. The iteration may end up with\n * an ELOOP if they happen and DIR_ITERATOR_PEDANTIC is set.\n */\n #define DIR_ITERATOR_PEDANTIC (1 << 0)\n #define DIR_ITERATOR_FOLLOW_SYMLINKS (1 << 1)\n+#define DIR_ITERATOR_DIRS_BEFORE (1 << 2)\n \n struct dir_iterator {\n \t/* The current path: */\n@@ -97,12 +107,13 @@ struct dir_iterator {\n * failure, return NULL and set errno accordingly.\n *\n * The iteration includes all paths under path, not including path\n- * itself and not including \".\" or \"..\" entries.\n+ * itself, \".\" or \"..\" entries and directories according to specified flags.\n *\n * Parameters are:\n * - path is the starting directory. An internal copy will be made.\n * - flags is a combination of the possible flags to initialize a\n- * dir-iterator or 0 for default behavior.\n+ * dir-iterator or 0 for default behavior which will ignore directory\n+ * paths, but will include the rest directory contents.\n */\n struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags);\n \n@@ -110,6 +121,7 @@ struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags);\n * Advance the iterator to the first or next item and return ITER_OK.\n * If the iteration is exhausted, free the dir_iterator and any\n * resources associated with it and return ITER_DONE.\n+ * On error, free dir_iterator memory and return ITER_ERROR.\n *\n * It is a bug to use iterator or call this function again after it\n * has returned ITER_DONE or ITER_ERROR (which may be returned iff\n@@ -119,8 +131,7 @@ int dir_iterator_advance(struct dir_iterator *iterator);\n \n /*\n * End the iteration before it has been exhausted. Free the\n- * dir_iterator and any associated resources and return ITER_DONE. On\n- * error, free the dir_iterator and return ITER_ERROR.\n+ * dir_iterator and any associated resources and return ITER_DONE.\n */\n int dir_iterator_abort(struct dir_iterator *iterator);\n \ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 8db7882aac..812f00c0f4 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2237,7 +2237,7 @@ static struct ref_iterator *reflog_iterator_begin(struct ref_store *ref_store,\n \n \tstrbuf_addf(&sb, \"%s/logs\", gitdir);\n \n-\tditer = dir_iterator_begin(sb.buf, 0);\n+\tditer = dir_iterator_begin(sb.buf, DIR_ITERATOR_DIRS_BEFORE);\n \tif (!diter) {\n \t\tstrbuf_release(&sb);\n \t\treturn empty_ref_iterator_begin();\ndiff --git a/t/helper/test-dir-iterator.c b/t/helper/test-dir-iterator.c\nindex 55d8a58836..f05d5fde9d 100644\n--- a/t/helper/test-dir-iterator.c\n+++ b/t/helper/test-dir-iterator.c\n@@ -17,7 +17,15 @@ static const char *error_name(int error_number)\n \n /*\n * usage:\n- * tool-test dir-iterator [--follow-symlinks] [--pedantic] directory_path\n+ * test-tool dir-iterator [OPTIONS] directory_path\n+ *\n+ * OPTIONS\n+ *\t--follow-symlinks\n+ *\t--pedantic\n+ *\t--dirs-before\n+ *\n+ * example:\n+ * test-tool dir-iterator --pedantic --dirs-before ./somedir\n */\n int cmd__dir_iterator(int argc, const char **argv)\n {\n@@ -30,6 +38,8 @@ int cmd__dir_iterator(int argc, const char **argv)\n \t\t\tflags |= DIR_ITERATOR_FOLLOW_SYMLINKS;\n \t\telse if (strcmp(*argv, \"--pedantic\") == 0)\n \t\t\tflags |= DIR_ITERATOR_PEDANTIC;\n+\t\telse if (strcmp(*argv, \"--dirs-before\") == 0)\n+\t\t\tflags |= DIR_ITERATOR_DIRS_BEFORE;\n \t\telse\n \t\t\tdie(\"invalid option '%s'\", *argv);\n \t}\ndiff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\nindex 8ab8811fb5..badd82d8a4 100755\n--- a/t/t0066-dir-iterator.sh\n+++ b/t/t0066-dir-iterator.sh\n@@ -9,7 +9,7 @@ test_expect_success 'setup -- dir with a single file' '\n \tmkdir dir1 &&\n \t>dir1/a\n '\n-test_expect_success 'dirs-before of dir with a file' '\n+test_expect_success 'dirs-ignore of dir with a file' '\n \tcat >expected-out <<-EOF &&\n \t[f] (a) [a] ./dir1/a\n \tEOF\n@@ -17,22 +17,60 @@ test_expect_success 'dirs-before of dir with a file' '\n \ttest-tool dir-iterator ./dir1 >actual-out &&\n \ttest_cmp expected-out actual-out\n '\n+test_expect_success 'dirs-before of dir with a file' '\n+\tcat >expected-out <<-EOF &&\n+\t[f] (a) [a] ./dir1/a\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-before ./dir1 >actual-out &&\n+\ttest_cmp expected-out actual-out\n+'\n \n test_expect_success 'setup -- dir with a single dir' '\n \tmkdir -p dir2/a\n '\n+test_expect_success 'dirs-ignore of dir with a single dir' '\n+\tcat >expected-out <<-EOF &&\n+\tEOF\n+\n+\ttest-tool dir-iterator ./dir2 >actual-out &&\n+\ttest_cmp expected-out actual-out\n+'\n test_expect_success 'dirs-before of dir with a single dir' '\n \tcat >expected-out <<-EOF &&\n \t[d] (a) [a] ./dir2/a\n \tEOF\n \n-\ttest-tool dir-iterator ./dir2 >actual-out &&\n+\ttest-tool dir-iterator --dirs-before ./dir2 >actual-out &&\n \ttest_cmp expected-out actual-out\n '\n \n test_expect_success POSIXPERM,SANITY 'setup -- dir w/ single dir w/o perms' '\n \tmkdir -p dir3/a\n '\n+test_expect_success POSIXPERM,SANITY 'dirs-ignore of dir w/ dir w/o perms' '\n+\tcat >expected-out <<-EOF &&\n+\tEOF\n+\n+\tchmod 0 dir3/a &&\n+\n+\ttest-tool dir-iterator ./dir3/ >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir3/a\n+'\n+test_expect_success POSIXPERM,SANITY 'pedantic dirs-ignore of dir w/ dir w/o perms' '\n+\tcat >expected-out <<-EOF &&\n+\tdir_iterator_advance failure: EACCES\n+\tEOF\n+\n+\tchmod 0 dir3/a &&\n+\n+\ttest_must_fail test-tool dir-iterator --pedantic ./dir3/ >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir3/a\n+'\n test_expect_success POSIXPERM,SANITY 'dirs-before of dir w/ dir w/o perms' '\n \tcat >expected-out <<-EOF &&\n \t[d] (a) [a] ./dir3/a\n@@ -40,7 +78,7 @@ test_expect_success POSIXPERM,SANITY 'dirs-before of dir w/ dir w/o perms' '\n \n \tchmod 0 dir3/a &&\n \n-\ttest-tool dir-iterator ./dir3/ >actual-out &&\n+\ttest-tool dir-iterator --dirs-before ./dir3/ >actual-out &&\n \ttest_cmp expected-out actual-out &&\n \n \tchmod 755 dir3/a\n@@ -53,7 +91,8 @@ test_expect_success POSIXPERM,SANITY 'pedantic dirs-before of dir w/ dir w/o per\n \n \tchmod 0 dir3/a &&\n \n-\ttest_must_fail test-tool dir-iterator --pedantic ./dir3/ >actual-out &&\n+\ttest_must_fail test-tool dir-iterator --dirs-before \\\n+\t\t--pedantic ./dir3/ >actual-out &&\n \ttest_cmp expected-out actual-out &&\n \n \tchmod 755 dir3/a\n@@ -67,7 +106,7 @@ test_expect_success 'setup -- dir w/ five files' '\n \t>dir4/d &&\n \t>dir4/e\n '\n-test_expect_success 'dirs-before of dir w/ five files' '\n+test_expect_success 'dirs-ignore of dir w/ five files' '\n \tcat >expected-sorted-out <<-EOF &&\n \t[f] (a) [a] ./dir4/a\n \t[f] (b) [b] ./dir4/b\n@@ -81,18 +120,40 @@ test_expect_success 'dirs-before of dir w/ five files' '\n \n \ttest_cmp expected-sorted-out actual-sorted-out\n '\n+test_expect_success 'dirs-before of dir w/ five files' '\n+\tcat >expected-sorted-out <<-EOF &&\n+\t[f] (a) [a] ./dir4/a\n+\t[f] (b) [b] ./dir4/b\n+\t[f] (c) [c] ./dir4/c\n+\t[f] (d) [d] ./dir4/d\n+\t[f] (e) [e] ./dir4/e\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-before ./dir4 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-sorted-out actual-sorted-out\n+'\n \n test_expect_success 'setup -- dir w/ dir w/ a file' '\n \tmkdir -p dir5/a &&\n \t>dir5/a/b\n '\n+test_expect_success 'dirs-ignore of dir w/ dir w/ a file' '\n+\tcat >expected-out <<-EOF &&\n+\t[f] (a/b) [b] ./dir5/a/b\n+\tEOF\n+\n+\ttest-tool dir-iterator ./dir5 >actual-out &&\n+\ttest_cmp expected-out actual-out\n+'\n test_expect_success 'dirs-before of dir w/ dir w/ a file' '\n \tcat >expected-out <<-EOF &&\n \t[d] (a) [a] ./dir5/a\n \t[f] (a/b) [b] ./dir5/a/b\n \tEOF\n \n-\ttest-tool dir-iterator ./dir5 >actual-out &&\n+\ttest-tool dir-iterator --dirs-before ./dir5 >actual-out &&\n \ttest_cmp expected-out actual-out\n '\n \n@@ -100,6 +161,14 @@ test_expect_success 'setup -- dir w/ three nested dirs w/ file' '\n \tmkdir -p dir6/a/b/c &&\n \t>dir6/a/b/c/d\n '\n+test_expect_success 'dirs-ignore of dir w/ three nested dirs w/ file' '\n+\tcat >expected-out <<-EOF &&\n+\t[f] (a/b/c/d) [d] ./dir6/a/b/c/d\n+\tEOF\n+\n+\ttest-tool dir-iterator ./dir6 >actual-out &&\n+\ttest_cmp expected-out actual-out\n+'\n test_expect_success 'dirs-before of dir w/ three nested dirs w/ file' '\n \tcat >expected-out <<-EOF &&\n \t[d] (a) [a] ./dir6/a\n@@ -108,7 +177,7 @@ test_expect_success 'dirs-before of dir w/ three nested dirs w/ file' '\n \t[f] (a/b/c/d) [d] ./dir6/a/b/c/d\n \tEOF\n \n-\ttest-tool dir-iterator ./dir6 >actual-out &&\n+\ttest-tool dir-iterator --dirs-before ./dir6 >actual-out &&\n \ttest_cmp expected-out actual-out\n '\n \n@@ -119,6 +188,33 @@ test_expect_success POSIXPERM,SANITY \\\n \t>dir7/a/b/c/d\n '\n test_expect_success POSIXPERM,SANITY \\\n+'dirs-ignore of dir w/ three nested dirs w/ file, second w/o perms' '\n+\n+\tcat >expected-out <<-EOF &&\n+\tEOF\n+\n+\tchmod 0 dir7/a/b &&\n+\n+\ttest-tool dir-iterator ./dir7 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir7/a/b\n+'\n+test_expect_success POSIXPERM,SANITY \\\n+'pedantic dirs-ignore of dir w/ three nested dirs w/ file, second w/o perms' '\n+\n+\tcat >expected-out <<-EOF &&\n+\tdir_iterator_advance failure: EACCES\n+\tEOF\n+\n+\tchmod 0 dir7/a/b &&\n+\n+\ttest_must_fail test-tool dir-iterator --pedantic ./dir7 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir7/a/b\n+'\n+test_expect_success POSIXPERM,SANITY \\\n 'dirs-before of dir w/ three nested dirs w/ file, second w/o perms' '\n \n \tcat >expected-out <<-EOF &&\n@@ -128,7 +224,7 @@ test_expect_success POSIXPERM,SANITY \\\n \n \tchmod 0 dir7/a/b &&\n \n-\ttest-tool dir-iterator ./dir7 >actual-out &&\n+\ttest-tool dir-iterator --dirs-before ./dir7 >actual-out &&\n \ttest_cmp expected-out actual-out &&\n \n \tchmod 755 dir7/a/b\n@@ -144,7 +240,8 @@ test_expect_success POSIXPERM,SANITY \\\n \n \tchmod 0 dir7/a/b &&\n \n-\ttest_must_fail test-tool dir-iterator --pedantic ./dir7 >actual-out &&\n+\ttest_must_fail test-tool dir-iterator --dirs-before \\\n+\t\t--pedantic ./dir7 >actual-out &&\n \ttest_cmp expected-out actual-out &&\n \n \tchmod 755 dir7/a/b\n@@ -156,6 +253,22 @@ test_expect_success 'setup -- dir w/ two dirs each w/ file' '\n \tmkdir dir8/c &&\n \t>dir8/c/d\n '\n+test_expect_success 'dirs-ignore of dir w/ two dirs each w/ file' '\n+\tcat >expected-out1 <<-EOF &&\n+\t[f] (a/b) [b] ./dir8/a/b\n+\t[f] (c/d) [d] ./dir8/c/d\n+\tEOF\n+\tcat >expected-out2 <<-EOF &&\n+\t[f] (c/d) [d] ./dir8/c/d\n+\t[f] (a/b) [b] ./dir8/a/b\n+\tEOF\n+\n+\ttest-tool dir-iterator ./dir8 >actual-out &&\n+\t(\n+\t\ttest_cmp expected-out1 actual-out ||\n+\t\ttest_cmp expected-out2 actual-out\n+\t)\n+'\n test_expect_success 'dirs-before of dir w/ two dirs each w/ file' '\n \tcat >expected-out1 <<-EOF &&\n \t[d] (a) [a] ./dir8/a\n@@ -170,7 +283,7 @@ test_expect_success 'dirs-before of dir w/ two dirs each w/ file' '\n \t[f] (a/b) [b] ./dir8/a/b\n \tEOF\n \n-\ttest-tool dir-iterator ./dir8 >actual-out &&\n+\ttest-tool dir-iterator --dirs-before ./dir8 >actual-out &&\n \t(\n \t\ttest_cmp expected-out1 actual-out ||\n \t\ttest_cmp expected-out2 actual-out\n@@ -185,6 +298,38 @@ test_expect_success 'setup -- dir w/ two dirs, one w/ two and one w/ one files'\n \t>dir9/d/e\n '\n test_expect_success \\\n+'dirs-ignore of dir w/ two dirs, one w/ two and one w/ one files' '\n+\n+\tcat >expected-out1 <<-EOF &&\n+\t[f] (a/b) [b] ./dir9/a/b\n+\t[f] (a/c) [c] ./dir9/a/c\n+\t[f] (d/e) [e] ./dir9/d/e\n+\tEOF\n+\tcat >expected-out2 <<-EOF &&\n+\t[f] (a/c) [c] ./dir9/a/c\n+\t[f] (a/b) [b] ./dir9/a/b\n+\t[f] (d/e) [e] ./dir9/d/e\n+\tEOF\n+\tcat >expected-out3 <<-EOF &&\n+\t[f] (d/e) [e] ./dir9/d/e\n+\t[f] (a/b) [b] ./dir9/a/b\n+\t[f] (a/c) [c] ./dir9/a/c\n+\tEOF\n+\tcat >expected-out4 <<-EOF &&\n+\t[f] (d/e) [e] ./dir9/d/e\n+\t[f] (a/c) [c] ./dir9/a/c\n+\t[f] (a/b) [b] ./dir9/a/b\n+\tEOF\n+\n+\ttest-tool dir-iterator ./dir9 >actual-out &&\n+\t(\n+\t\ttest_cmp expected-out1 actual-out ||\n+\t\ttest_cmp expected-out2 actual-out ||\n+\t\ttest_cmp expected-out3 actual-out ||\n+\t\ttest_cmp expected-out4 actual-out\n+\t)\n+'\n+test_expect_success \\\n 'dirs-before of dir w/ two dirs, one w/ two and one w/ one files' '\n \n \tcat >expected-out1 <<-EOF &&\n@@ -216,7 +361,7 @@ test_expect_success \\\n \t[f] (a/b) [b] ./dir9/a/b\n \tEOF\n \n-\ttest-tool dir-iterator ./dir9 >actual-out &&\n+\ttest-tool dir-iterator --dirs-before ./dir9 >actual-out &&\n \t(\n \t\ttest_cmp expected-out1 actual-out ||\n \t\ttest_cmp expected-out2 actual-out ||\n@@ -231,6 +376,22 @@ test_expect_success 'setup -- dir w/ two nested dirs, each w/ file' '\n \tmkdir dir10/a/c &&\n \t>dir10/a/c/d\n '\n+test_expect_success 'dirs-ignore of dir w/ two nested dirs, each w/ file' '\n+\tcat >expected-out1 <<-EOF &&\n+\t[f] (a/b) [b] ./dir10/a/b\n+\t[f] (a/c/d) [d] ./dir10/a/c/d\n+\tEOF\n+\tcat >expected-out2 <<-EOF &&\n+\t[f] (a/c/d) [d] ./dir10/a/c/d\n+\t[f] (a/b) [b] ./dir10/a/b\n+\tEOF\n+\n+\ttest-tool dir-iterator ./dir10/ >actual-out &&\n+\t(\n+\t\ttest_cmp expected-out1 actual-out ||\n+\t\ttest_cmp expected-out2 actual-out\n+\t)\n+'\n test_expect_success 'dirs-before of dir w/ two nested dirs, each w/ file' '\n \tcat >expected-out1 <<-EOF &&\n \t[d] (a) [a] ./dir10/a\n@@ -245,7 +406,7 @@ test_expect_success 'dirs-before of dir w/ two nested dirs, each w/ file' '\n \t[f] (a/b) [b] ./dir10/a/b\n \tEOF\n \n-\ttest-tool dir-iterator ./dir10/ >actual-out &&\n+\ttest-tool dir-iterator --dirs-before ./dir10/ >actual-out &&\n \t(\n \t\ttest_cmp expected-out1 actual-out ||\n \t\ttest_cmp expected-out2 actual-out\n@@ -262,6 +423,20 @@ test_expect_success 'setup -- dir w/ complex structure' '\n \t>dir11/a/e &&\n \t>dir11/d/e/d/a\n '\n+test_expect_success 'dirs-ignore of dir w/ complex structure' '\n+\tcat >expected-sorted-out <<-EOF &&\n+\t[f] (a/b/c/d) [d] ./dir11/a/b/c/d\n+\t[f] (a/e) [e] ./dir11/a/e\n+\t[f] (b) [b] ./dir11/b\n+\t[f] (c) [c] ./dir11/c\n+\t[f] (d/e/d/a) [a] ./dir11/d/e/d/a\n+\tEOF\n+\n+\ttest-tool dir-iterator ./dir11 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-sorted-out actual-sorted-out\n+'\n test_expect_success 'dirs-before of dir w/ complex structure' '\n \tcat >expected-sorted-out <<-EOF &&\n \t[d] (a) [a] ./dir11/a\n@@ -277,7 +452,7 @@ test_expect_success 'dirs-before of dir w/ complex structure' '\n \t[f] (d/e/d/a) [a] ./dir11/d/e/d/a\n \tEOF\n \n-\ttest-tool dir-iterator ./dir11 >actual-out &&\n+\ttest-tool dir-iterator --dirs-before ./dir11 >actual-out &&\n \tsort actual-out >actual-sorted-out &&\n \n \ttest_cmp expected-sorted-out actual-sorted-out\n@@ -287,7 +462,7 @@ test_expect_success POSIXPERM,SANITY 'setup -- dir w/o perms' '\n \tmkdir -p dir12/a &&\n \t>dir12/a/b\n '\n-test_expect_success POSIXPERM,SANITY 'dirs-before of root dir w/o perms' '\n+test_expect_success POSIXPERM,SANITY 'dirs-ignore of root dir w/o perms' '\n \tcat >expected-out <<-EOF &&\n \tdir_iterator_begin failure: EACCES\n \tEOF\n@@ -299,7 +474,7 @@ test_expect_success POSIXPERM,SANITY 'dirs-before of root dir w/o perms' '\n \n \tchmod 755 dir12\n '\n-test_expect_success POSIXPERM,SANITY 'pedantic dirs-before of root dir w/o perms' '\n+test_expect_success POSIXPERM,SANITY 'pedantic dirs-ignore of root dir w/o perms' '\n \tcat >expected-out <<-EOF &&\n \tdir_iterator_begin failure: EACCES\n \tEOF\n@@ -311,6 +486,31 @@ test_expect_success POSIXPERM,SANITY 'pedantic dirs-before of root dir w/o perms\n \n \tchmod 755 dir12\n '\n+test_expect_success POSIXPERM,SANITY 'dirs-before of root dir w/o perms' '\n+\tcat >expected-out <<-EOF &&\n+\tdir_iterator_begin failure: EACCES\n+\tEOF\n+\n+\tchmod 0 dir12 &&\n+\n+\ttest_must_fail test-tool dir-iterator --dirs-before ./dir12 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir12\n+'\n+test_expect_success POSIXPERM,SANITY 'pedantic dirs-before of root dir w/o perms' '\n+\tcat >expected-out <<-EOF &&\n+\tdir_iterator_begin failure: EACCES\n+\tEOF\n+\n+\tchmod 0 dir12 &&\n+\n+\ttest_must_fail test-tool dir-iterator --dirs-before \\\n+\t\t--pedantic ./dir12 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir12\n+'\n \n test_expect_success 'begin should fail upon inexistent paths' '\n \techo \"dir_iterator_begin failure: ENOENT\" >expected-out &&\n@@ -335,6 +535,33 @@ test_expect_success POSIXPERM,SANITY 'setup -- dir w/ dir w/o perms w/ file' '\n \t>dir13/a/b\n '\n test_expect_success POSIXPERM,SANITY \\\n+'dirs-ignore of dir w/ dir w/o perms w/ file' '\n+\n+\tcat >expected-out <<-EOF &&\n+\tEOF\n+\n+\tchmod 0 dir13/a &&\n+\n+\ttest-tool dir-iterator ./dir13 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir13/a\n+'\n+test_expect_success POSIXPERM,SANITY \\\n+'pedantic dirs-ignore of dir w/ dir w/o perms w/ file' '\n+\n+\tcat >expected-out <<-EOF &&\n+\tdir_iterator_advance failure: EACCES\n+\tEOF\n+\n+\tchmod 0 dir13/a &&\n+\n+\ttest_must_fail test-tool dir-iterator --pedantic ./dir13 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir13/a\n+'\n+test_expect_success POSIXPERM,SANITY \\\n 'dirs-before of dir w/ dir w/o perms w/ file' '\n \n \tcat >expected-out <<-EOF &&\n@@ -343,7 +570,7 @@ test_expect_success POSIXPERM,SANITY \\\n \n \tchmod 0 dir13/a &&\n \n-\ttest-tool dir-iterator ./dir13 >actual-out &&\n+\ttest-tool dir-iterator --dirs-before ./dir13 >actual-out &&\n \ttest_cmp expected-out actual-out &&\n \n \tchmod 755 dir13/a\n@@ -358,7 +585,8 @@ test_expect_success POSIXPERM,SANITY \\\n \n \tchmod 0 dir13/a &&\n \n-\ttest_must_fail test-tool dir-iterator --pedantic ./dir13 >actual-out &&\n+\ttest_must_fail test-tool dir-iterator --dirs-before \\\n+\t\t--pedantic ./dir13 >actual-out &&\n \ttest_cmp expected-out actual-out &&\n \n \tchmod 755 dir13/a\n@@ -371,6 +599,31 @@ test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/o cycle' '\n \tln -s d dir14/a/e &&\n \tln -s ../b dir14/a/f\n '\n+test_expect_success SYMLINKS 'dirs-ignore of dir w/ symlinks w/o cycle' '\n+\tcat >expected-sorted-out <<-EOF &&\n+\t[f] (a/d) [d] ./dir14/a/d\n+\t[s] (a/e) [e] ./dir14/a/e\n+\t[s] (a/f) [f] ./dir14/a/f\n+\tEOF\n+\n+\ttest-tool dir-iterator ./dir14 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-sorted-out actual-sorted-out\n+'\n+test_expect_success SYMLINKS \\\n+'follow-symlinks dirs-ignore of dir w/ symlinks, w/o cycle' '\n+\n+\tcat >expected-sorted-out <<-EOF &&\n+\t[f] (a/d) [d] ./dir14/a/d\n+\t[f] (a/e) [e] ./dir14/a/e\n+\tEOF\n+\n+\ttest-tool dir-iterator --follow-symlinks ./dir14 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-sorted-out actual-sorted-out\n+'\n test_expect_success SYMLINKS 'dirs-before of dir w/ symlinks w/o cycle' '\n \tcat >expected-sorted-out <<-EOF &&\n \t[d] (a) [a] ./dir14/a\n@@ -381,7 +634,7 @@ test_expect_success SYMLINKS 'dirs-before of dir w/ symlinks w/o cycle' '\n \t[s] (a/f) [f] ./dir14/a/f\n \tEOF\n \n-\ttest-tool dir-iterator ./dir14 >actual-out &&\n+\ttest-tool dir-iterator --dirs-before ./dir14 >actual-out &&\n \tsort actual-out >actual-sorted-out &&\n \n \ttest_cmp expected-sorted-out actual-sorted-out\n@@ -399,7 +652,7 @@ test_expect_success SYMLINKS \\\n \t[f] (a/e) [e] ./dir14/a/e\n \tEOF\n \n-\ttest-tool dir-iterator --follow-symlinks ./dir14 >actual-out &&\n+\ttest-tool dir-iterator --dirs-before --follow-symlinks ./dir14 >actual-out &&\n \tsort actual-out >actual-sorted-out &&\n \n \ttest_cmp expected-sorted-out actual-sorted-out\n@@ -412,7 +665,33 @@ test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/ cycle' '\n \tln -s ../ dir15/a/b/e &&\n \tln -s ../../ dir15/a/b/f\n '\n+test_expect_success SYMLINKS 'dirs-ignore of dir w/ symlinks w/ cycle' '\n+\tcat >expected-sorted-out <<-EOF &&\n+\t[s] (a/b/d) [d] ./dir15/a/b/d\n+\t[s] (a/b/e) [e] ./dir15/a/b/e\n+\t[s] (a/b/f) [f] ./dir15/a/b/f\n+\tEOF\n+\n+\ttest-tool dir-iterator ./dir15 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-sorted-out actual-sorted-out\n+'\n+test_expect_success SYMLINKS \\\n+'pedantic follow-symlinks dirs-ignore of dir w/ symlinks w/ cycle' '\n+\n+\tcat >expected-tailed-out <<-EOF &&\n+\tdir_iterator_advance failure: ELOOP\n+\tEOF\n+\n+\ttest_must_fail test-tool dir-iterator \\\n+\t\t--pedantic --follow-symlinks ./dir15 >actual-out &&\n+\ttail -n 1 actual-out >actual-tailed-out &&\n+\n+\ttest_cmp expected-tailed-out actual-tailed-out\n+'\n test_expect_success SYMLINKS 'dirs-before of dir w/ symlinks w/ cycle' '\n+\n \tcat >expected-sorted-out <<-EOF &&\n \t[d] (a) [a] ./dir15/a\n \t[d] (a/b) [b] ./dir15/a/b\n@@ -422,7 +701,7 @@ test_expect_success SYMLINKS 'dirs-before of dir w/ symlinks w/ cycle' '\n \t[s] (a/b/f) [f] ./dir15/a/b/f\n \tEOF\n \n-\ttest-tool dir-iterator ./dir15 >actual-out &&\n+\ttest-tool dir-iterator --dirs-before ./dir15 >actual-out &&\n \tsort actual-out >actual-sorted-out &&\n \n \ttest_cmp expected-sorted-out actual-sorted-out\n@@ -434,7 +713,7 @@ test_expect_success SYMLINKS \\\n \tdir_iterator_advance failure: ELOOP\n \tEOF\n \n-\ttest_must_fail test-tool dir-iterator \\\n+\ttest_must_fail test-tool dir-iterator --dirs-before \\\n \t\t--pedantic --follow-symlinks ./dir15 >actual-out &&\n \ttail -n 1 actual-out >actual-tailed-out &&\n \n-- \n2.36.1\n\n"},{"id":"455046","messageId":"20220509175159.2948802-15-kioplato@gmail.com","threadId":"57864","inReplyTo":"20220509175159.2948802-1-kioplato@gmail.com","subject":"[PATCH v2 14/15] dir-iterator: option to iterate dirs in post-order","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-05-09T17:51:58Z","receivedAt":"2022-05-09T17:55:56Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"Introduce a new option to dir-iterator, using dir_iterator_begin()\nflags parameter, allowing to control whether or not directories will\nbe exposed after their contents. In essence, post-order traversal over\nfile system entries that are directories.\n\nThis new iteration scheme can be enabled with DIR_ITERATOR_DIRS_AFTER\nflag which iterates over a directory after doing so over its contents.\nThe existing flag DIR_ITERATOR_DIRS_BEFORE can be combined with the new\nflag DIR_ITERATOR_DIRS_AFTER in any way. These flags do not conflict\nwith each other and can be combined to:\n * ignore directories by not setting either of them\n * iterate directories pre-order and post-order by enabling both\n * iterate directories pre-order by enabling DIR_ITERATOR_DIRS_BEFORE\n * iterate directories post-order by enabling DIR_ITERATOR_DIRS_AFTER\n\nUpdate t/t0066-dir-iterator.sh and t/helper/test-dir-iterator.c to test\nthe new iteration scheme the new flag DIR_ITERATOR_DIRS_AFTER enables.\n\nSigned-off-by: Plato Kiorpelidis \n---\n dir-iterator.c | 95 ++++-\n dir-iterator.h | 17 +-\n t/helper/test-dir-iterator.c | 6 +-\n t/t0066-dir-iterator.sh | 744 +++++++++++++++++++++++++++++++++--\n 4 files changed, 828 insertions(+), 34 deletions(-)\n\ndiff --git a/dir-iterator.c b/dir-iterator.c\nindex c1475add27..dec15317f0 100644\n--- a/dir-iterator.c\n+++ b/dir-iterator.c\n@@ -133,10 +133,17 @@ static int expose_entry(struct dir_iterator_int *iter, char *d_name, char *dir_s\n {\n \tint stat_err;\n \tunsigned int DIRS_BEFORE = iter->flags & DIR_ITERATOR_DIRS_BEFORE;\n+\tunsigned int DIRS_AFTER = iter->flags & DIR_ITERATOR_DIRS_AFTER;\n \n \tstrbuf_addch(&iter->base.path, '/');\n \tstrbuf_addstr(&iter->base.path, d_name);\n \n+\t/*\n+\t * We've got to check whether or not this is a directory.\n+\t * We need to perform this check since the user could've requested\n+\t * to ignore directory entries.\n+\t */\n+\n \tif (iter->flags & DIR_ITERATOR_FOLLOW_SYMLINKS)\n \t\tstat_err = stat(iter->base.path.buf, &iter->base.st);\n \telse\n@@ -158,6 +165,9 @@ static int expose_entry(struct dir_iterator_int *iter, char *d_name, char *dir_s\n \tif (S_ISDIR(iter->base.st.st_mode)) {\n \t\tif (!DIRS_BEFORE && !strcmp(dir_state, \"before\"))\n \t\t\treturn FAIL_IGN_DIRS;\n+\n+\t\tif (!DIRS_AFTER && !strcmp(dir_state, \"after\"))\n+\t\t\treturn FAIL_IGN_DIRS;\n \t}\n \n \t/*\n@@ -173,6 +183,36 @@ static int expose_entry(struct dir_iterator_int *iter, char *d_name, char *dir_s\n \treturn OK;\n }\n \n+/*\n+ * Get the basename of the current directory.\n+ *\n+ * Using iter->base.path.buf, find the current dir basename.\n+ */\n+static char *current_dir_basename(struct dir_iterator_int *iter)\n+{\n+\tchar *start = strrchr(iter->base.path.buf, '/');\n+\tchar *basename = NULL;\n+\n+\tif (!start) {\n+\t\t/*\n+\t\t * dir-iterator's implementation searches for '/' characters to\n+\t\t * figure out the \"active\" directory part. Therefore, in this\n+\t\t * case, the current path is the current directory part.\n+\t\t */\n+\n+\t\tstart = iter->base.path.buf;\n+\t} else {\n+\t\tstart += 1; /* Skip '/' character */\n+\t}\n+\n+\tif (!(basename = calloc(1, strlen(start) + 1)))\n+\t\tdie_errno(\"calloc\");\n+\n+\tmemcpy(basename, start, strlen(start));\n+\n+\treturn basename;\n+}\n+\n int dir_iterator_advance(struct dir_iterator *dir_iterator)\n {\n \tstruct dir_iterator_int *iter = (struct dir_iterator_int *)dir_iterator;\n@@ -200,9 +240,28 @@ int dir_iterator_advance(struct dir_iterator *dir_iterator)\n \t\t * Therefore, there isn't any case to run out of levels.\n \t\t */\n \n+\t\t/*\n+\t\t * We need to make sure, in case DIRS_AFTER is enabled, to\n+\t\t * expose the entry in order to be consistent with what\n+\t\t * DIRS_BEFORE exposes in case of failed `opendir()` call.\n+\t\t */\n+\n+\t\tchar *d_name = current_dir_basename(iter);\n+\n \t\t--iter->levels_nr;\n \n-\t\treturn dir_iterator_advance(dir_iterator);\n+\t\tlevel = &iter->levels[iter->levels_nr - 1];\n+\t\tstrbuf_setlen(&iter->base.path, level->prefix_len);\n+\n+\t\texpose_err = expose_entry(iter, d_name, \"after\");\n+\t\tfree(d_name);\n+\n+\t\tif (expose_err == FAIL_NOT_ENOENT && PEDANTIC)\n+\t\t\tgoto error_out;\n+\t\telse if (expose_err != OK)\n+\t\t\treturn dir_iterator_advance(dir_iterator);\n+\t\telse\n+\t\t\treturn ITER_OK;\n \t}\n \n \tstrbuf_setlen(&iter->base.path, level->prefix_len);\n@@ -220,12 +279,42 @@ int dir_iterator_advance(struct dir_iterator *dir_iterator)\n \t\t} else {\n \t\t\t/*\n \t\t\t * Current directory has been iterated through.\n+\t\t\t * We need to check if we need to expose current dir\n+\t\t\t * because of DIRS_AFTER flag.\n \t\t\t */\n \n-\t\t\tif (pop_level(iter) == 0)\n+\t\t\tchar* d_name = current_dir_basename(iter);\n+\n+\t\t\t/*\n+\t\t\t * We don't care to expose the root directory.\n+\t\t\t * Users of this API know when iteration starts on root\n+\t\t\t * directory - they call `dir_iterator_begin()` - and\n+\t\t\t * when ITER_DONE is returned they know when it's over.\n+\t\t\t */\n+\n+\t\t\t/*\n+\t\t\t * Call to `pop_level()` needs to preceed call to\n+\t\t\t * `expose_entry()` because `expose_entry()` appends to\n+\t\t\t * current `iter->base` and we need to set it up.\n+\t\t\t */\n+\n+\t\t\tif (pop_level(iter) == 0) {\n+\t\t\t\tfree(d_name);\n \t\t\t\treturn dir_iterator_abort(dir_iterator);\n+\t\t\t}\n \n-\t\t\treturn dir_iterator_advance(dir_iterator);\n+\t\t\tlevel = &iter->levels[iter->levels_nr - 1];\n+\t\t\tstrbuf_setlen(&iter->base.path, level->prefix_len);\n+\n+\t\t\texpose_err = expose_entry(iter, d_name, \"after\");\n+\t\t\tfree(d_name);\n+\n+\t\t\tif (expose_err == FAIL_NOT_ENOENT && PEDANTIC)\n+\t\t\t\tgoto error_out;\n+\t\t\telse if (expose_err != OK)\n+\t\t\t\treturn dir_iterator_advance(dir_iterator);\n+\t\t\telse\n+\t\t\t\treturn ITER_OK;\n \t\t}\n \t}\n \ndiff --git a/dir-iterator.h b/dir-iterator.h\nindex c1d16a8c6d..825db6b9b8 100644\n--- a/dir-iterator.h\n+++ b/dir-iterator.h\n@@ -16,9 +16,10 @@\n * the dir_iterator structure to reflect the next path in the\n * iteration. The order that paths are iterated over within a\n * directory is undefined. Directory paths are given before their\n- * contents when DIR_ITERATOR_DIRS_BEFORE is set. Failure to set this\n- * flag results in directory paths not being exposed. Instead, iteration\n- * will happen within directories. Their contents will be exposed.\n+ * contents when DIR_ITERATOR_DIRS_BEFORE is set and after when\n+ * DIR_ITERATOR_DIRS_AFTER is set. Failure to set any of them results\n+ * in directory paths not being exposed. Instead, iteration will happen\n+ * within directories. Their contents will be exposed.\n *\n * A typical iteration looks like this:\n *\n@@ -67,8 +68,13 @@\n * - DIR_ITERATOR_DIRS_BEFORE: make dir-iterator expose a directory path\n * before iterating through that directory's contents.\n *\n- * Note: Activating none of the flags will iterate through directories'\n- * contents but won't expose the directory paths.\n+ * - DIR_ITERATOR_DIRS_AFTER: make dir-iterator expose a directory path after\n+ * iterating through that directory's contents.\n+ *\n+ * Note: any combination of DIR_ITERATOR_BEFORE and DIR_ITERATOR_AFTER works.\n+ * Activating both of them will expose directories when descending into one and\n+ * when it's been exhausted. Activating none will iterate through directories'\n+ * contents but won't expose the directories themselves.\n *\n * Warning: circular symlinks are also followed when\n * DIR_ITERATOR_FOLLOW_SYMLINKS is set. The iteration may end up with\n@@ -77,6 +83,7 @@\n #define DIR_ITERATOR_PEDANTIC (1 << 0)\n #define DIR_ITERATOR_FOLLOW_SYMLINKS (1 << 1)\n #define DIR_ITERATOR_DIRS_BEFORE (1 << 2)\n+#define DIR_ITERATOR_DIRS_AFTER (1 << 3)\n \n struct dir_iterator {\n \t/* The current path: */\ndiff --git a/t/helper/test-dir-iterator.c b/t/helper/test-dir-iterator.c\nindex f05d5fde9d..ac5f368335 100644\n--- a/t/helper/test-dir-iterator.c\n+++ b/t/helper/test-dir-iterator.c\n@@ -23,9 +23,9 @@ static const char *error_name(int error_number)\n *\t--follow-symlinks\n *\t--pedantic\n *\t--dirs-before\n+ *\t--dirs-after\n *\n- * example:\n- * test-tool dir-iterator --pedantic --dirs-before ./somedir\n+ * test-tool dir-iterator --pedantic --dirs-before --dirs-after ./somedir\n */\n int cmd__dir_iterator(int argc, const char **argv)\n {\n@@ -40,6 +40,8 @@ int cmd__dir_iterator(int argc, const char **argv)\n \t\t\tflags |= DIR_ITERATOR_PEDANTIC;\n \t\telse if (strcmp(*argv, \"--dirs-before\") == 0)\n \t\t\tflags |= DIR_ITERATOR_DIRS_BEFORE;\n+\t\telse if (strcmp(*argv, \"--dirs-after\") == 0)\n+\t\t\tflags |= DIR_ITERATOR_DIRS_AFTER;\n \t\telse\n \t\t\tdie(\"invalid option '%s'\", *argv);\n \t}\ndiff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\nindex badd82d8a4..63d31b8f7d 100755\n--- a/t/t0066-dir-iterator.sh\n+++ b/t/t0066-dir-iterator.sh\n@@ -25,6 +25,22 @@ test_expect_success 'dirs-before of dir with a file' '\n \ttest-tool dir-iterator --dirs-before ./dir1 >actual-out &&\n \ttest_cmp expected-out actual-out\n '\n+test_expect_success 'dirs-after of dir with a file' '\n+\tcat >expected-out <<-EOF &&\n+\t[f] (a) [a] ./dir1/a\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-after ./dir1 >actual-out &&\n+\ttest_cmp expected-out actual-out\n+'\n+test_expect_success 'dirs-before/dirs-after of dir with a file' '\n+\tcat >expected-out <<-EOF &&\n+\t[f] (a) [a] ./dir1/a\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir1 >actual-out &&\n+\ttest_cmp expected-out actual-out\n+'\n \n test_expect_success 'setup -- dir with a single dir' '\n \tmkdir -p dir2/a\n@@ -44,6 +60,23 @@ test_expect_success 'dirs-before of dir with a single dir' '\n \ttest-tool dir-iterator --dirs-before ./dir2 >actual-out &&\n \ttest_cmp expected-out actual-out\n '\n+test_expect_success 'dirs-after of dir with a single dir' '\n+\tcat >expected-out <<-EOF &&\n+\t[d] (a) [a] ./dir2/a\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-after ./dir2 >actual-out &&\n+\ttest_cmp expected-out actual-out\n+'\n+test_expect_success 'dirs-before/dirs-after of dir with a single dir' '\n+\tcat >expected-out <<-EOF &&\n+\t[d] (a) [a] ./dir2/a\n+\t[d] (a) [a] ./dir2/a\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir2 >actual-out &&\n+\ttest_cmp expected-out actual-out\n+'\n \n test_expect_success POSIXPERM,SANITY 'setup -- dir w/ single dir w/o perms' '\n \tmkdir -p dir3/a\n@@ -97,6 +130,62 @@ test_expect_success POSIXPERM,SANITY 'pedantic dirs-before of dir w/ dir w/o per\n \n \tchmod 755 dir3/a\n '\n+test_expect_success POSIXPERM,SANITY 'dirs-after of dir w/ dir w/o perms' '\n+\tcat >expected-out <<-EOF &&\n+\t[d] (a) [a] ./dir3/a\n+\tEOF\n+\n+\tchmod 0 dir3/a &&\n+\n+\ttest-tool dir-iterator --dirs-after ./dir3/ >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir3/a\n+'\n+test_expect_success POSIXPERM,SANITY 'pedantic dirs-after of dir w/ dir w/o perms' '\n+\tcat >expected-out <<-EOF &&\n+\tdir_iterator_advance failure: EACCES\n+\tEOF\n+\n+\tchmod 0 dir3/a &&\n+\n+\ttest_must_fail test-tool dir-iterator --dirs-after \\\n+\t\t--pedantic ./dir3/ >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir3/a\n+'\n+test_expect_success POSIXPERM,SANITY \\\n+'dirs-before/dirs-after of dir w/ dir w/o perms' '\n+\n+\tcat >expected-out <<-EOF &&\n+\t[d] (a) [a] ./dir3/a\n+\t[d] (a) [a] ./dir3/a\n+\tEOF\n+\n+\tchmod 0 dir3/a &&\n+\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir3/ >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir3/a\n+'\n+test_expect_success POSIXPERM,SANITY \\\n+'pedantic dirs-before/dirs-after of dir w/ dir w/o perms' '\n+\n+\tcat >expected-out <<-EOF &&\n+\t[d] (a) [a] ./dir3/a\n+\tdir_iterator_advance failure: EACCES\n+\tEOF\n+\n+\tchmod 0 dir3/a &&\n+\n+\ttest_must_fail test-tool dir-iterator --dirs-before --dirs-after \\\n+\t\t--pedantic ./dir3/ >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir3/a\n+'\n \n test_expect_success 'setup -- dir w/ five files' '\n \tmkdir dir4 &&\n@@ -134,6 +223,34 @@ test_expect_success 'dirs-before of dir w/ five files' '\n \n \ttest_cmp expected-sorted-out actual-sorted-out\n '\n+test_expect_success 'dirs-after of dir w/ five files' '\n+\tcat >expected-sorted-out <<-EOF &&\n+\t[f] (a) [a] ./dir4/a\n+\t[f] (b) [b] ./dir4/b\n+\t[f] (c) [c] ./dir4/c\n+\t[f] (d) [d] ./dir4/d\n+\t[f] (e) [e] ./dir4/e\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-after ./dir4 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-sorted-out actual-sorted-out\n+'\n+test_expect_success 'dirs-before/dirs-after of dir w/ five files' '\n+\tcat >expected-sorted-out <<-EOF &&\n+\t[f] (a) [a] ./dir4/a\n+\t[f] (b) [b] ./dir4/b\n+\t[f] (c) [c] ./dir4/c\n+\t[f] (d) [d] ./dir4/d\n+\t[f] (e) [e] ./dir4/e\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir4 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-sorted-out actual-sorted-out\n+'\n \n test_expect_success 'setup -- dir w/ dir w/ a file' '\n \tmkdir -p dir5/a &&\n@@ -156,6 +273,25 @@ test_expect_success 'dirs-before of dir w/ dir w/ a file' '\n \ttest-tool dir-iterator --dirs-before ./dir5 >actual-out &&\n \ttest_cmp expected-out actual-out\n '\n+test_expect_success 'dirs-after of dir w/ dir w/ a file' '\n+\tcat >expected-after-out <<-EOF &&\n+\t[f] (a/b) [b] ./dir5/a/b\n+\t[d] (a) [a] ./dir5/a\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-after ./dir5 >actual-out &&\n+\ttest_cmp expected-after-out actual-out\n+'\n+test_expect_success 'dirs-before/dirs-after of dir w/ dir w/ a file' '\n+\tcat >expected-before-after-out <<-EOF &&\n+\t[d] (a) [a] ./dir5/a\n+\t[f] (a/b) [b] ./dir5/a/b\n+\t[d] (a) [a] ./dir5/a\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir5 >actual-out &&\n+\ttest_cmp expected-before-after-out actual-out\n+'\n \n test_expect_success 'setup -- dir w/ three nested dirs w/ file' '\n \tmkdir -p dir6/a/b/c &&\n@@ -180,6 +316,31 @@ test_expect_success 'dirs-before of dir w/ three nested dirs w/ file' '\n \ttest-tool dir-iterator --dirs-before ./dir6 >actual-out &&\n \ttest_cmp expected-out actual-out\n '\n+test_expect_success 'dirs-after of dir w/ three nested dirs w/ file' '\n+\tcat >expected-after-out <<-EOF &&\n+\t[f] (a/b/c/d) [d] ./dir6/a/b/c/d\n+\t[d] (a/b/c) [c] ./dir6/a/b/c\n+\t[d] (a/b) [b] ./dir6/a/b\n+\t[d] (a) [a] ./dir6/a\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-after ./dir6 >actual-out &&\n+\ttest_cmp expected-after-out actual-out\n+'\n+test_expect_success 'dirs-before/dirs-after of dir w/ three nested dirs w/ file' '\n+\tcat >expected-before-after-out <<-EOF &&\n+\t[d] (a) [a] ./dir6/a\n+\t[d] (a/b) [b] ./dir6/a/b\n+\t[d] (a/b/c) [c] ./dir6/a/b/c\n+\t[f] (a/b/c/d) [d] ./dir6/a/b/c/d\n+\t[d] (a/b/c) [c] ./dir6/a/b/c\n+\t[d] (a/b) [b] ./dir6/a/b\n+\t[d] (a) [a] ./dir6/a\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir6 >actual-out &&\n+\ttest_cmp expected-before-after-out actual-out\n+'\n \n test_expect_success POSIXPERM,SANITY \\\n 'setup -- dir w/ three nested dirs w/ file, second nested dir w/o perms' '\n@@ -246,6 +407,70 @@ test_expect_success POSIXPERM,SANITY \\\n \n \tchmod 755 dir7/a/b\n '\n+test_expect_success POSIXPERM,SANITY \\\n+'dirs-after of dir w/ three nested dirs w/ file, second w/o perms' '\n+\n+\tcat >expected-out <<-EOF &&\n+\t[d] (a/b) [b] ./dir7/a/b\n+\t[d] (a) [a] ./dir7/a\n+\tEOF\n+\n+\tchmod 0 dir7/a/b &&\n+\n+\ttest-tool dir-iterator --dirs-after ./dir7 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir7/a/b\n+'\n+test_expect_success POSIXPERM,SANITY \\\n+'pedantic dirs-after of dir w/ three nested dirs w/ file, second w/o perms' '\n+\n+\tcat >expected-out <<-EOF &&\n+\tdir_iterator_advance failure: EACCES\n+\tEOF\n+\n+\tchmod 0 dir7/a/b &&\n+\n+\ttest_must_fail test-tool dir-iterator --dirs-after \\\n+\t\t--pedantic ./dir7 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir7/a/b\n+'\n+test_expect_success POSIXPERM,SANITY \\\n+'dirs-before/dirs-after of dir w/ three nested dirs w/ file, second w/o perms' '\n+\n+\tcat >expected-out <<-EOF &&\n+\t[d] (a) [a] ./dir7/a\n+\t[d] (a/b) [b] ./dir7/a/b\n+\t[d] (a/b) [b] ./dir7/a/b\n+\t[d] (a) [a] ./dir7/a\n+\tEOF\n+\n+\tchmod 0 dir7/a/b &&\n+\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir7 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir7/a/b\n+'\n+test_expect_success POSIXPERM,SANITY \\\n+'pedantic dirs-before/dirs-after of dir w/ three nested dirs w/ file, second w/o perms' '\n+\n+\tcat >expected-out <<-EOF &&\n+\t[d] (a) [a] ./dir7/a\n+\t[d] (a/b) [b] ./dir7/a/b\n+\tdir_iterator_advance failure: EACCES\n+\tEOF\n+\n+\tchmod 0 dir7/a/b &&\n+\n+\ttest_must_fail test-tool dir-iterator --dirs-before --dirs-after \\\n+\t\t--pedantic ./dir7 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir7/a/b\n+'\n \n test_expect_success 'setup -- dir w/ two dirs each w/ file' '\n \tmkdir -p dir8/a &&\n@@ -289,6 +514,50 @@ test_expect_success 'dirs-before of dir w/ two dirs each w/ file' '\n \t\ttest_cmp expected-out2 actual-out\n \t)\n '\n+test_expect_success 'dirs-after of dir w/ two dirs each w/ file' '\n+\tcat >expected-out1 <<-EOF &&\n+\t[f] (a/b) [b] ./dir8/a/b\n+\t[d] (a) [a] ./dir8/a\n+\t[f] (c/d) [d] ./dir8/c/d\n+\t[d] (c) [c] ./dir8/c\n+\tEOF\n+\tcat >expected-out2 <<-EOF &&\n+\t[f] (c/d) [d] ./dir8/c/d\n+\t[d] (c) [c] ./dir8/c\n+\t[f] (a/b) [b] ./dir8/a/b\n+\t[d] (a) [a] ./dir8/a\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-after ./dir8 >actual-out &&\n+\t(\n+\t\ttest_cmp expected-out1 actual-out ||\n+\t\ttest_cmp expected-out2 actual-out\n+\t)\n+'\n+test_expect_success 'dirs-before/dirs-after of dir w/ two dirs each w/ file' '\n+\tcat >expected-out1 <<-EOF &&\n+\t[d] (a) [a] ./dir8/a\n+\t[f] (a/b) [b] ./dir8/a/b\n+\t[d] (a) [a] ./dir8/a\n+\t[d] (c) [c] ./dir8/c\n+\t[f] (c/d) [d] ./dir8/c/d\n+\t[d] (c) [c] ./dir8/c\n+\tEOF\n+\tcat >expected-out2 <<-EOF &&\n+\t[d] (c) [c] ./dir8/c\n+\t[f] (c/d) [d] ./dir8/c/d\n+\t[d] (c) [c] ./dir8/c\n+\t[d] (a) [a] ./dir8/a\n+\t[f] (a/b) [b] ./dir8/a/b\n+\t[d] (a) [a] ./dir8/a\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir8 >actual-out &&\n+\t(\n+\t\ttest_cmp expected-out1 actual-out ||\n+\t\ttest_cmp expected-out2 actual-out\n+\t)\n+'\n \n test_expect_success 'setup -- dir w/ two dirs, one w/ two and one w/ one files' '\n \tmkdir -p dir9/a &&\n@@ -369,35 +638,123 @@ test_expect_success \\\n \t\ttest_cmp expected-out4 actual-out\n \t)\n '\n+test_expect_success \\\n+'dirs-after of dir w/ two dirs, one w/ two and one w/ one files' '\n \n-test_expect_success 'setup -- dir w/ two nested dirs, each w/ file' '\n-\tmkdir -p dir10/a &&\n-\t>dir10/a/b &&\n-\tmkdir dir10/a/c &&\n-\t>dir10/a/c/d\n-'\n-test_expect_success 'dirs-ignore of dir w/ two nested dirs, each w/ file' '\n \tcat >expected-out1 <<-EOF &&\n-\t[f] (a/b) [b] ./dir10/a/b\n-\t[f] (a/c/d) [d] ./dir10/a/c/d\n+\t[f] (a/b) [b] ./dir9/a/b\n+\t[f] (a/c) [c] ./dir9/a/c\n+\t[d] (a) [a] ./dir9/a\n+\t[f] (d/e) [e] ./dir9/d/e\n+\t[d] (d) [d] ./dir9/d\n \tEOF\n \tcat >expected-out2 <<-EOF &&\n-\t[f] (a/c/d) [d] ./dir10/a/c/d\n-\t[f] (a/b) [b] ./dir10/a/b\n+\t[f] (a/c) [c] ./dir9/a/c\n+\t[f] (a/b) [b] ./dir9/a/b\n+\t[d] (a) [a] ./dir9/a\n+\t[f] (d/e) [e] ./dir9/d/e\n+\t[d] (d) [d] ./dir9/d\n \tEOF\n-\n-\ttest-tool dir-iterator ./dir10/ >actual-out &&\n-\t(\n-\t\ttest_cmp expected-out1 actual-out ||\n-\t\ttest_cmp expected-out2 actual-out\n-\t)\n-'\n-test_expect_success 'dirs-before of dir w/ two nested dirs, each w/ file' '\n-\tcat >expected-out1 <<-EOF &&\n-\t[d] (a) [a] ./dir10/a\n-\t[f] (a/b) [b] ./dir10/a/b\n-\t[d] (a/c) [c] ./dir10/a/c\n-\t[f] (a/c/d) [d] ./dir10/a/c/d\n+\tcat >expected-out3 <<-EOF &&\n+\t[f] (d/e) [e] ./dir9/d/e\n+\t[d] (d) [d] ./dir9/d\n+\t[f] (a/b) [b] ./dir9/a/b\n+\t[f] (a/c) [c] ./dir9/a/c\n+\t[d] (a) [a] ./dir9/a\n+\tEOF\n+\tcat >expected-out4 <<-EOF &&\n+\t[f] (d/e) [e] ./dir9/d/e\n+\t[d] (d) [d] ./dir9/d\n+\t[f] (a/c) [c] ./dir9/a/c\n+\t[f] (a/b) [b] ./dir9/a/b\n+\t[d] (a) [a] ./dir9/a\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-after ./dir9 >actual-out &&\n+\t(\n+\t\ttest_cmp expected-out1 actual-out ||\n+\t\ttest_cmp expected-out2 actual-out ||\n+\t\ttest_cmp expected-out3 actual-out ||\n+\t\ttest_cmp expected-out4 actual-out\n+\t)\n+'\n+test_expect_success \\\n+'dirs-before/dirs-after of dir w/ two dirs, one w/ two and one w/ one files' '\n+\n+\tcat >expected-out1 <<-EOF &&\n+\t[d] (a) [a] ./dir9/a\n+\t[f] (a/b) [b] ./dir9/a/b\n+\t[f] (a/c) [c] ./dir9/a/c\n+\t[d] (a) [a] ./dir9/a\n+\t[d] (d) [d] ./dir9/d\n+\t[f] (d/e) [e] ./dir9/d/e\n+\t[d] (d) [d] ./dir9/d\n+\tEOF\n+\tcat >expected-out2 <<-EOF &&\n+\t[d] (a) [a] ./dir9/a\n+\t[f] (a/c) [c] ./dir9/a/c\n+\t[f] (a/b) [b] ./dir9/a/b\n+\t[d] (a) [a] ./dir9/a\n+\t[d] (d) [d] ./dir9/d\n+\t[f] (d/e) [e] ./dir9/d/e\n+\t[d] (d) [d] ./dir9/d\n+\tEOF\n+\tcat >expected-out3 <<-EOF &&\n+\t[d] (d) [d] ./dir9/d\n+\t[f] (d/e) [e] ./dir9/d/e\n+\t[d] (d) [d] ./dir9/d\n+\t[d] (a) [a] ./dir9/a\n+\t[f] (a/b) [b] ./dir9/a/b\n+\t[f] (a/c) [c] ./dir9/a/c\n+\t[d] (a) [a] ./dir9/a\n+\tEOF\n+\tcat >expected-out4 <<-EOF &&\n+\t[d] (d) [d] ./dir9/d\n+\t[f] (d/e) [e] ./dir9/d/e\n+\t[d] (d) [d] ./dir9/d\n+\t[d] (a) [a] ./dir9/a\n+\t[f] (a/c) [c] ./dir9/a/c\n+\t[f] (a/b) [b] ./dir9/a/b\n+\t[d] (a) [a] ./dir9/a\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir9 >actual-out &&\n+\t(\n+\t\ttest_cmp expected-out1 actual-out ||\n+\t\ttest_cmp expected-out2 actual-out ||\n+\t\ttest_cmp expected-out3 actual-out ||\n+\t\ttest_cmp expected-out4 actual-out\n+\t)\n+'\n+\n+test_expect_success 'setup -- dir w/ two nested dirs, each w/ file' '\n+\tmkdir -p dir10/a &&\n+\t>dir10/a/b &&\n+\tmkdir dir10/a/c &&\n+\t>dir10/a/c/d\n+'\n+test_expect_success 'dirs-ignore of dir w/ two nested dirs, each w/ file' '\n+\tcat >expected-out1 <<-EOF &&\n+\t[f] (a/b) [b] ./dir10/a/b\n+\t[f] (a/c/d) [d] ./dir10/a/c/d\n+\tEOF\n+\tcat >expected-out2 <<-EOF &&\n+\t[f] (a/c/d) [d] ./dir10/a/c/d\n+\t[f] (a/b) [b] ./dir10/a/b\n+\tEOF\n+\n+\ttest-tool dir-iterator ./dir10/ >actual-out &&\n+\t(\n+\t\ttest_cmp expected-out1 actual-out ||\n+\t\ttest_cmp expected-out2 actual-out\n+\t)\n+'\n+test_expect_success 'dirs-before of dir w/ two nested dirs, each w/ file' '\n+\tcat >expected-out1 <<-EOF &&\n+\t[d] (a) [a] ./dir10/a\n+\t[f] (a/b) [b] ./dir10/a/b\n+\t[d] (a/c) [c] ./dir10/a/c\n+\t[f] (a/c/d) [d] ./dir10/a/c/d\n \tEOF\n \tcat >expected-out2 <<-EOF &&\n \t[d] (a) [a] ./dir10/a\n@@ -412,6 +769,50 @@ test_expect_success 'dirs-before of dir w/ two nested dirs, each w/ file' '\n \t\ttest_cmp expected-out2 actual-out\n \t)\n '\n+test_expect_success 'dirs-after of dir w/ two nested dirs, each w/ file' '\n+\tcat >expected-out1 <<-EOF &&\n+\t[f] (a/b) [b] ./dir10/a/b\n+\t[f] (a/c/d) [d] ./dir10/a/c/d\n+\t[d] (a/c) [c] ./dir10/a/c\n+\t[d] (a) [a] ./dir10/a\n+\tEOF\n+\tcat >expected-out2 <<-EOF &&\n+\t[f] (a/c/d) [d] ./dir10/a/c/d\n+\t[d] (a/c) [c] ./dir10/a/c\n+\t[f] (a/b) [b] ./dir10/a/b\n+\t[d] (a) [a] ./dir10/a\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-after ./dir10/ >actual-out &&\n+\t(\n+\t\ttest_cmp expected-out1 actual-out ||\n+\t\ttest_cmp expected-out2 actual-out\n+\t)\n+'\n+test_expect_success 'dirs-before/dirs-after of dir w/ two nested dirs, each w/ file' '\n+\tcat >expected-out1 <<-EOF &&\n+\t[d] (a) [a] ./dir10/a\n+\t[f] (a/b) [b] ./dir10/a/b\n+\t[d] (a/c) [c] ./dir10/a/c\n+\t[f] (a/c/d) [d] ./dir10/a/c/d\n+\t[d] (a/c) [c] ./dir10/a/c\n+\t[d] (a) [a] ./dir10/a\n+\tEOF\n+\tcat >expected-out2 <<-EOF &&\n+\t[d] (a) [a] ./dir10/a\n+\t[d] (a/c) [c] ./dir10/a/c\n+\t[f] (a/c/d) [d] ./dir10/a/c/d\n+\t[d] (a/c) [c] ./dir10/a/c\n+\t[f] (a/b) [b] ./dir10/a/b\n+\t[d] (a) [a] ./dir10/a\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir10 >actual-out &&\n+\t(\n+\t\ttest_cmp expected-out1 actual-out ||\n+\t\ttest_cmp expected-out2 actual-out\n+\t)\n+'\n \n test_expect_success 'setup -- dir w/ complex structure' '\n \tmkdir -p dir11 &&\n@@ -457,6 +858,52 @@ test_expect_success 'dirs-before of dir w/ complex structure' '\n \n \ttest_cmp expected-sorted-out actual-sorted-out\n '\n+test_expect_success 'dirs-after of dir w/ complex structure' '\n+\tcat >expected-sorted-out <<-EOF &&\n+\t[d] (a) [a] ./dir11/a\n+\t[d] (a/b) [b] ./dir11/a/b\n+\t[d] (a/b/c) [c] ./dir11/a/b/c\n+\t[d] (d) [d] ./dir11/d\n+\t[d] (d/e) [e] ./dir11/d/e\n+\t[d] (d/e/d) [d] ./dir11/d/e/d\n+\t[f] (a/b/c/d) [d] ./dir11/a/b/c/d\n+\t[f] (a/e) [e] ./dir11/a/e\n+\t[f] (b) [b] ./dir11/b\n+\t[f] (c) [c] ./dir11/c\n+\t[f] (d/e/d/a) [a] ./dir11/d/e/d/a\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-after ./dir11 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-sorted-out actual-sorted-out\n+'\n+test_expect_success 'dirs-before/dirs-after of dir w/ complex structure' '\n+\tcat >expected-sorted-out <<-EOF &&\n+\t[d] (a) [a] ./dir11/a\n+\t[d] (a) [a] ./dir11/a\n+\t[d] (a/b) [b] ./dir11/a/b\n+\t[d] (a/b) [b] ./dir11/a/b\n+\t[d] (a/b/c) [c] ./dir11/a/b/c\n+\t[d] (a/b/c) [c] ./dir11/a/b/c\n+\t[d] (d) [d] ./dir11/d\n+\t[d] (d) [d] ./dir11/d\n+\t[d] (d/e) [e] ./dir11/d/e\n+\t[d] (d/e) [e] ./dir11/d/e\n+\t[d] (d/e/d) [d] ./dir11/d/e/d\n+\t[d] (d/e/d) [d] ./dir11/d/e/d\n+\t[f] (a/b/c/d) [d] ./dir11/a/b/c/d\n+\t[f] (a/e) [e] ./dir11/a/e\n+\t[f] (b) [b] ./dir11/b\n+\t[f] (c) [c] ./dir11/c\n+\t[f] (d/e/d/a) [a] ./dir11/d/e/d/a\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir11 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-sorted-out actual-sorted-out\n+'\n \n test_expect_success POSIXPERM,SANITY 'setup -- dir w/o perms' '\n \tmkdir -p dir12/a &&\n@@ -511,6 +958,61 @@ test_expect_success POSIXPERM,SANITY 'pedantic dirs-before of root dir w/o perms\n \n \tchmod 755 dir12\n '\n+test_expect_success POSIXPERM,SANITY 'dirs-after of root dir w/o perms' '\n+\tcat >expected-out <<-EOF &&\n+\tdir_iterator_begin failure: EACCES\n+\tEOF\n+\n+\tchmod 0 dir12 &&\n+\n+\ttest_must_fail test-tool dir-iterator --dirs-after ./dir12 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir12\n+'\n+test_expect_success POSIXPERM,SANITY 'pedantic dirs-after of root dir w/o perms' '\n+\tcat >expected-out <<-EOF &&\n+\tdir_iterator_begin failure: EACCES\n+\tEOF\n+\n+\tchmod 0 dir12 &&\n+\n+\ttest_must_fail test-tool dir-iterator --dirs-after \\\n+\t\t--pedantic ./dir12 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir12\n+'\n+test_expect_success POSIXPERM,SANITY \\\n+'dirs-before/dirs-after of root dir w/o perms' '\n+\n+\tcat >expected-out <<-EOF &&\n+\tdir_iterator_begin failure: EACCES\n+\tEOF\n+\n+\tchmod 0 dir12 &&\n+\n+\ttest_must_fail test-tool dir-iterator --dirs-before --dirs-after \\\n+\t\t./dir12 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir12\n+'\n+test_expect_success POSIXPERM,SANITY \\\n+'pedantic dirs-before/dirs-after of root dir w/o perms' '\n+\n+\tcat >expected-out <<-EOF &&\n+\tdir_iterator_begin failure: EACCES\n+\tEOF\n+\n+\tchmod 0 dir12 &&\n+\n+\ttest_must_fail test-tool dir-iterator --dirs-before --dirs-after \\\n+\t\t--pedantic ./dir12 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir12\n+'\n \n test_expect_success 'begin should fail upon inexistent paths' '\n \techo \"dir_iterator_begin failure: ENOENT\" >expected-out &&\n@@ -591,6 +1093,66 @@ test_expect_success POSIXPERM,SANITY \\\n \n \tchmod 755 dir13/a\n '\n+test_expect_success POSIXPERM,SANITY \\\n+'dirs-after of dir w/ dir w/o perms w/ file' '\n+\n+\tcat >expected-out <<-EOF &&\n+\t[d] (a) [a] ./dir13/a\n+\tEOF\n+\n+\tchmod 0 dir13/a &&\n+\n+\ttest-tool dir-iterator --dirs-after ./dir13 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir13/a\n+'\n+test_expect_success POSIXPERM,SANITY \\\n+'pedantic dirs-after of dir w/ dir w/o perms w/ file' '\n+\n+\tcat >expected-out <<-EOF &&\n+\tdir_iterator_advance failure: EACCES\n+\tEOF\n+\n+\tchmod 0 dir13/a &&\n+\n+\ttest_must_fail test-tool dir-iterator --dirs-after \\\n+\t\t--pedantic ./dir13 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir13/a\n+'\n+test_expect_success POSIXPERM,SANITY \\\n+'dirs-before/dirs-after of dir w/ dir w/o perms w/ file' '\n+\n+\tcat >expected-out <<-EOF &&\n+\t[d] (a) [a] ./dir13/a\n+\t[d] (a) [a] ./dir13/a\n+\tEOF\n+\n+\tchmod 0 dir13/a &&\n+\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir13 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir13/a\n+'\n+test_expect_success POSIXPERM,SANITY \\\n+'pedantic dirs-before/dirs-after of dir w/ dir w/o perms w/ file' '\n+\n+\tcat >expected-out <<-EOF &&\n+\t[d] (a) [a] ./dir13/a\n+\tdir_iterator_advance failure: EACCES\n+\tEOF\n+\n+\tchmod 0 dir13/a &&\n+\n+\ttest_must_fail test-tool dir-iterator --dirs-before --dirs-after \\\n+\t\t--pedantic ./dir13 >actual-out &&\n+\ttest_cmp expected-out actual-out &&\n+\n+\tchmod 755 dir13/a\n+'\n \n test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/o cycle' '\n \tmkdir -p dir14/a &&\n@@ -657,6 +1219,81 @@ test_expect_success SYMLINKS \\\n \n \ttest_cmp expected-sorted-out actual-sorted-out\n '\n+test_expect_success SYMLINKS 'dirs-after of dir w/ symlinks w/o cycle' '\n+\tcat >expected-sorted-out <<-EOF &&\n+\t[d] (a) [a] ./dir14/a\n+\t[d] (b) [b] ./dir14/b\n+\t[d] (b/c) [c] ./dir14/b/c\n+\t[f] (a/d) [d] ./dir14/a/d\n+\t[s] (a/e) [e] ./dir14/a/e\n+\t[s] (a/f) [f] ./dir14/a/f\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-after ./dir14 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-sorted-out actual-sorted-out\n+'\n+test_expect_success SYMLINKS \\\n+'follow-symlinks dirs-after of dir w/ symlinks w/o cycle' '\n+\n+\tcat >expected-sorted-out <<-EOF &&\n+\t[d] (a) [a] ./dir14/a\n+\t[d] (a/f) [f] ./dir14/a/f\n+\t[d] (a/f/c) [c] ./dir14/a/f/c\n+\t[d] (b) [b] ./dir14/b\n+\t[d] (b/c) [c] ./dir14/b/c\n+\t[f] (a/d) [d] ./dir14/a/d\n+\t[f] (a/e) [e] ./dir14/a/e\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-after --follow-symlinks ./dir14 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-sorted-out actual-sorted-out\n+'\n+test_expect_success SYMLINKS 'dirs-before/dirs-after of dir w/ symlinks w/o cycle' '\n+\tcat >expected-sorted-out <<-EOF &&\n+\t[d] (a) [a] ./dir14/a\n+\t[d] (a) [a] ./dir14/a\n+\t[d] (b) [b] ./dir14/b\n+\t[d] (b) [b] ./dir14/b\n+\t[d] (b/c) [c] ./dir14/b/c\n+\t[d] (b/c) [c] ./dir14/b/c\n+\t[f] (a/d) [d] ./dir14/a/d\n+\t[s] (a/e) [e] ./dir14/a/e\n+\t[s] (a/f) [f] ./dir14/a/f\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir14 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-sorted-out actual-sorted-out\n+'\n+test_expect_success SYMLINKS \\\n+'follow-symlinks dirs-before/dirs-after of dir w/ symlinks w/o cycle' '\n+\n+\tcat >expected-sorted-out <<-EOF &&\n+\t[d] (a) [a] ./dir14/a\n+\t[d] (a) [a] ./dir14/a\n+\t[d] (a/f) [f] ./dir14/a/f\n+\t[d] (a/f) [f] ./dir14/a/f\n+\t[d] (a/f/c) [c] ./dir14/a/f/c\n+\t[d] (a/f/c) [c] ./dir14/a/f/c\n+\t[d] (b) [b] ./dir14/b\n+\t[d] (b) [b] ./dir14/b\n+\t[d] (b/c) [c] ./dir14/b/c\n+\t[d] (b/c) [c] ./dir14/b/c\n+\t[f] (a/d) [d] ./dir14/a/d\n+\t[f] (a/e) [e] ./dir14/a/e\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-before --dirs-after \\\n+\t\t--follow-symlinks ./dir14 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-sorted-out actual-sorted-out\n+'\n \n test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/ cycle' '\n \tmkdir -p dir15/a/b &&\n@@ -719,5 +1356,64 @@ test_expect_success SYMLINKS \\\n \n \ttest_cmp expected-tailed-out actual-tailed-out\n '\n+test_expect_success SYMLINKS 'dirs-after of dir w/ symlinks w/ cycle' '\n+\tcat >expected-sorted-out <<-EOF &&\n+\t[d] (a) [a] ./dir15/a\n+\t[d] (a/b) [b] ./dir15/a/b\n+\t[d] (a/c) [c] ./dir15/a/c\n+\t[s] (a/b/d) [d] ./dir15/a/b/d\n+\t[s] (a/b/e) [e] ./dir15/a/b/e\n+\t[s] (a/b/f) [f] ./dir15/a/b/f\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-after ./dir15 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-sorted-out actual-sorted-out\n+'\n+test_expect_success SYMLINKS \\\n+'pedantic follow-symlinks dirs-after of dir w/ symlinks w/ cycle' '\n+\n+\tcat >expected-tailed-out <<-EOF &&\n+\tdir_iterator_advance failure: ELOOP\n+\tEOF\n+\n+\ttest_must_fail test-tool dir-iterator --dirs-after \\\n+\t\t--pedantic --follow-symlinks ./dir15 >actual-out &&\n+\ttail -n 1 actual-out >actual-tailed-out &&\n+\n+\ttest_cmp expected-tailed-out actual-tailed-out\n+'\n+test_expect_success SYMLINKS 'dirs-before/dirs-after of dir w/ symlinks w/ cycle' '\n+\tcat >expected-sorted-out <<-EOF &&\n+\t[d] (a) [a] ./dir15/a\n+\t[d] (a) [a] ./dir15/a\n+\t[d] (a/b) [b] ./dir15/a/b\n+\t[d] (a/b) [b] ./dir15/a/b\n+\t[d] (a/c) [c] ./dir15/a/c\n+\t[d] (a/c) [c] ./dir15/a/c\n+\t[s] (a/b/d) [d] ./dir15/a/b/d\n+\t[s] (a/b/e) [e] ./dir15/a/b/e\n+\t[s] (a/b/f) [f] ./dir15/a/b/f\n+\tEOF\n+\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir15 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-sorted-out actual-sorted-out\n+'\n+test_expect_success SYMLINKS \\\n+'pedantic follow-symlinks dirs-before/dirs-after of dir w/ symlinks w/ cycle' '\n+\n+\tcat >expected-tailed-out <<-EOF &&\n+\tdir_iterator_advance failure: ELOOP\n+\tEOF\n+\n+\ttest_must_fail test-tool dir-iterator --dirs-before --dirs-after \\\n+\t\t--pedantic --follow-symlinks ./dir15 >actual-out &&\n+\ttail -n 1 actual-out >actual-tailed-out &&\n+\n+\ttest_cmp expected-tailed-out actual-tailed-out\n+'\n \n test_done\n-- \n2.36.1\n\n"},{"id":"455053","messageId":"xmqq35hictaa.fsf@gitster.g","threadId":"57864","inReplyTo":"20220509175159.2948802-5-kioplato@gmail.com","subject":"Re: [PATCH v2 04/15] test-dir-iterator: consistently return EXIT_FAILURE or EXIT_SUCCESS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-09T21:03:57Z","receivedAt":"2022-05-09T21:04:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Plato Kiorpelidis writes:\n\n> Throughout test-dir-iterator.c we were returning/exiting with either\n> integers or EXIT_FAILURE. Improve readability and reduce mental load\n> by being consistent with what test-dir-iterator returns through the\n> test-tool. Returning mixed constants and integers could indicate that\n> it matters for some reason e.g. architecture of test-tool and cmd__*\n> functions.\n>\n> EXIT_SUCCESS and EXIT_FAILURE are specified by the C standard.\n> That makes the code more portable and standardized.\n\nAnd less portable for the invoking process of \"git\". The invoking\nprocess used to be able to depend ou getting WEXITSTATUS() from our\nexit status to obtain \"1\" when we exited with 1; if we start exiting\nwith EXIT_FAILURE, there is no guarantee what non-zero value is used.\n\nSo, I am not sure if this is a good direction to go in.\n\n> Signed-off-by: Plato Kiorpelidis \n> ---\n> t/helper/test-dir-iterator.c | 6 +++---\n> 1 file changed, 3 insertions(+), 3 deletions(-)\n\nEspecially given that this is a helper for testing, we may want to\nreturn/exit with different non-zero value at different places to\nmake it easier for the test script to tell where in the program we\ndecided to exit a failure. IOW, if we return not EXIT_FAILURE but 2\n(or whatever value that is not used elsewhere) in the first hunk,\nand let the second hunk continue to return 1, then the calling test\nscript can tell which one decided to fail.\n\nAs to EXIT_SUCCESS, I do not have a strong opinion against it, but\nbecause EXIT_FAILURE is a bad idea, we probably should avoid it for\nconsistency.\n\nThanks.\n\n> diff --git a/t/helper/test-dir-iterator.c b/t/helper/test-dir-iterator.c\n> index 659b6bfa81..81e931673e 100644\n> --- a/t/helper/test-dir-iterator.c\n> +++ b/t/helper/test-dir-iterator.c\n> @@ -39,7 +39,7 @@ int cmd__dir_iterator(int argc, const char **argv)\n> \n> \tif (!diter) {\n> \t\tprintf(\"dir_iterator_begin failure: %s\\n\", error_name(errno));\n> -\t\texit(EXIT_FAILURE);\n> +\t\treturn EXIT_FAILURE;\n> \t}\n> \n> \twhile ((iter_status = dir_iterator_advance(diter)) == ITER_OK) {\n> @@ -58,8 +58,8 @@ int cmd__dir_iterator(int argc, const char **argv)\n> \n> \tif (iter_status != ITER_DONE) {\n> \t\tprintf(\"dir_iterator_advance failure\\n\");\n> -\t\treturn 1;\n> +\t\treturn EXIT_FAILURE;\n> \t}\n> \n> -\treturn 0;\n> +\treturn EXIT_SUCCESS;\n> }\n"},{"id":"455055","messageId":"xmqqr152be4n.fsf@gitster.g","threadId":"57864","inReplyTo":"20220509175159.2948802-11-kioplato@gmail.com","subject":"Re: [PATCH v2 10/15] dir-iterator: refactor dir_iterator_advance()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-09T21:16:40Z","receivedAt":"2022-05-09T21:16:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Plato Kiorpelidis writes:\n\n> Simplify dir_iterator_advance() by converting from iterative to\n> recursive implementation. This conversion makes dir-iterator more\n> maintainable for the following reasons:\n> * dir-iterator iterates the file-system, which is a tree structure.\n> Traditionally, tree traversals, in textbooks, lectures and online\n> sources are implemented recursively and not iteratively. Therefore\n> it helps reduce mental load for readers, since it's easier to follow\n> as it reminds of the same tree traversals we use on tree structures.\n\nCareful.\n\nMany algorithms that are taught in the recursive form in textbooks\nare turned into iterative in production systems for a reason. To\navoid too deep a recursion wasting too much stack space. A loop\nwith management of work items using in-program data structures like\nstack or queue often makes a large traversal bearable.\n\nThe most obvious example is our history traversal code. History\nstored in Git is a tree structure, but no sane reimplementation of\nGit (well, at least those that want to be able to deal with a\nhistory larger than a toy project) would implement \"git log\" using a\nrecursive algorithm.\n\n\n"},{"id":"455077","messageId":"e21eaf9f-8784-88e1-e4d0-49135db1189b@gmail.com","threadId":"57864","inReplyTo":"20220509175159.2948802-11-kioplato@gmail.com","subject":"Re: [PATCH v2 10/15] dir-iterator: refactor dir_iterator_advance()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-05-10T13:04:18Z","receivedAt":"2022-05-10T13:04:25Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Plato\n\nOn 09/05/2022 18:51, Plato Kiorpelidis wrote:\n> Simplify dir_iterator_advance() by converting from iterative to\n> recursive implementation. This conversion makes dir-iterator more\n> maintainable for the following reasons:\n> * dir-iterator iterates the file-system, which is a tree structure.\n> Traditionally, tree traversals, in textbooks, lectures and online\n> sources are implemented recursively and not iteratively. Therefore\n> it helps reduce mental load for readers, since it's easier to follow\n> as it reminds of the same tree traversals we use on tree structures.\n\nIn a traditional recursive tree walk the function that walks the tree \napplies a user supplied function to each node, the function that walks \nthe tree does not return until every node has been visited. The \nrecursive implementation relies on the initial call not returning until \nall the nodes have been visited and as Junio pointed out will exhaust \nthe stack if the tree is too deep. This is not a traditional recursive \ntree walk though because we're implementing a iterator so our function \nreturns each node that it visits rather than applying a user supplied \nfunction to the node thereby loosing any benefit offered by a recursive \nimplementation.\n\n\n> * recursion requires one less indentation depth because we get rid of\n> the while loop and instead recurse, using the program's stack.\n> * in each recursive step a set of instructions are executed and\n> recursion lays out the code structurally in a better way, such that\n> these instructions stand out symmetrically for each recursive step.\n> \n> This makes dir-iterator easier to work with, understand and introduce\n> new functionality, like post-order on some directory entries, because it\n> reminds us of the same operations we use to traverse tree structures.\n\nI'm afraid I'm still not convinced the end result is easier to \nunderstand or use. It is unusual for a patch that adds twice as many \nlines as it removes to make the code simpler.\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Plato Kiorpelidis \n> ---\n> dir-iterator.c | 223 ++++++++++++++++++++++++++++++++-----------------\n> 1 file changed, 146 insertions(+), 77 deletions(-)\n> \n> diff --git a/dir-iterator.c b/dir-iterator.c\n> index b17e9f970a..3adcfbc966 100644\n> --- a/dir-iterator.c\n> +++ b/dir-iterator.c\n> @@ -7,8 +7,7 @@ struct dir_iterator_level {\n> \tDIR *dir;\n> \n> \t/*\n> -\t * The length of the directory part of path at this level\n> -\t * (including a trailing '/'):\n> +\t * The length of the directory part of path at this level.\n> \t */\n> \tsize_t prefix_len;\n> };\n> @@ -34,8 +33,9 @@ struct dir_iterator_int {\n> \tsize_t levels_alloc;\n> \n> \t/*\n> -\t * A stack of levels. levels[0] is the uppermost directory\n> -\t * that will be included in this iteration.\n> +\t * A stack of levels. levels[0] is the root directory.\n> +\t * It won't be included in the iteration, but iteration will happen\n> +\t * inside it's subdirectories.\n> \t */\n> \tstruct dir_iterator_level *levels;\n> \n> @@ -43,36 +43,63 @@ struct dir_iterator_int {\n> \tunsigned int flags;\n> };\n> \n> +enum {\n> +\tOK,\n> +\tFAIL_ENOENT,\n> +\tFAIL_NOT_ENOENT,\n> +};\n> +\n> /*\n> * Push a level in the iter stack and initialize it with information from\n> - * the directory pointed by iter->base->path. It is assumed that this\n> - * strbuf points to a valid directory path. Return 0 on success and -1\n> - * otherwise, setting errno accordingly and leaving the stack unchanged.\n> + * the directory pointed by iter->base->path. Don't open the directory.\n> + *\n> + * Return 1 on success.\n> + * Return 0 when `iter->base->path` isn't a directory.\n> */\n> static int push_level(struct dir_iterator_int *iter)\n> {\n> \tstruct dir_iterator_level *level;\n> \n> +\tif (!S_ISDIR(iter->base.st.st_mode))\n> +\t\treturn 0;\n> +\n> \tALLOC_GROW(iter->levels, iter->levels_nr + 1, iter->levels_alloc);\n> \tlevel = &iter->levels[iter->levels_nr++];\n> \n> -\tif (!is_dir_sep(iter->base.path.buf[iter->base.path.len - 1]))\n> -\t\tstrbuf_addch(&iter->base.path, '/');\n> +\tlevel->dir = NULL;\n> +\n> \tlevel->prefix_len = iter->base.path.len;\n> \n> -\tlevel->dir = opendir(iter->base.path.buf);\n> -\tif (!level->dir) {\n> -\t\tint saved_errno = errno;\n> -\t\tif (errno != ENOENT) {\n> -\t\t\twarning_errno(\"error opening directory '%s'\",\n> -\t\t\t\t iter->base.path.buf);\n> -\t\t}\n> -\t\titer->levels_nr--;\n> +\treturn 1;\n> +}\n> +\n> +/*\n> + * Activate most recent pushed level. Stack is unchanged.\n> + *\n> + * Return values:\n> + * OK on success.\n> + * FAIL_ENOENT on failed exposure because entry does not exist.\n> + * FAIL_NOT_ENOENT on failed exposure because of errno other than ENOENT.\n> + */\n> +static int activate_level(struct dir_iterator_int *iter)\n> +{\n> +\tstruct dir_iterator_level *level = &iter->levels[iter->levels_nr - 1];\n> +\tint saved_errno;\n> +\n> +\tif (level->dir)\n> +\t\treturn OK;\n> +\n> +\tif ((level->dir = opendir(iter->base.path.buf)) != NULL)\n> +\t\treturn OK;\n> +\n> +\tsaved_errno = errno;\n> +\tif (errno != ENOENT) {\n> +\t\twarning_errno(\"error opening directory '%s'\", iter->base.path.buf);\n> \t\terrno = saved_errno;\n> -\t\treturn -1;\n> +\t\treturn FAIL_NOT_ENOENT;\n> \t}\n> -\n> -\treturn 0;\n> +\terrno = saved_errno;\n> +\treturn FAIL_ENOENT;\n> }\n> \n> /*\n> @@ -81,12 +108,10 @@ static int push_level(struct dir_iterator_int *iter)\n> */\n> static int pop_level(struct dir_iterator_int *iter)\n> {\n> -\tstruct dir_iterator_level *level =\n> -\t\t&iter->levels[iter->levels_nr - 1];\n> +\tstruct dir_iterator_level *level = &iter->levels[iter->levels_nr - 1];\n> \n> \tif (level->dir && closedir(level->dir))\n> -\t\twarning_errno(\"error closing directory '%s'\",\n> -\t\t\t iter->base.path.buf);\n> +\t\twarning_errno(\"error closing directory '%s'\", iter->base.path.buf);\n> \tlevel->dir = NULL;\n> \n> \treturn --iter->levels_nr;\n> @@ -94,82 +119,119 @@ static int pop_level(struct dir_iterator_int *iter)\n> \n> /*\n> * Populate iter->base with the necessary information on the next iteration\n> - * entry, represented by the given dirent de. Return 0 on success and -1\n> - * otherwise, setting errno accordingly.\n> + * entry, represented by the given relative path to the lowermost directory,\n> + * d_name.\n> + *\n> + * Return values:\n> + * OK on successful exposure of the provided entry.\n> + * FAIL_ENOENT on failed exposure because entry does not exist.\n> + * FAIL_NOT_ENOENT on failed exposure because of errno other than ENOENT.\n> */\n> -static int prepare_next_entry_data(struct dir_iterator_int *iter,\n> -\t\t\t\t struct dirent *de)\n> +static int expose_entry(struct dir_iterator_int *iter, char *d_name)\n> {\n> -\tint err, saved_errno;\n> +\tint stat_err;\n> \n> -\tstrbuf_addstr(&iter->base.path, de->d_name);\n> -\t/*\n> -\t * We have to reset these because the path strbuf might have\n> -\t * been realloc()ed at the previous strbuf_addstr().\n> -\t */\n> -\titer->base.relative_path = iter->base.path.buf +\n> -\t\t\t\t iter->levels[0].prefix_len;\n> -\titer->base.basename = iter->base.path.buf +\n> -\t\t\t iter->levels[iter->levels_nr - 1].prefix_len;\n> +\tstrbuf_addch(&iter->base.path, '/');\n> +\tstrbuf_addstr(&iter->base.path, d_name);\n> \n> \tif (iter->flags & DIR_ITERATOR_FOLLOW_SYMLINKS)\n> -\t\terr = stat(iter->base.path.buf, &iter->base.st);\n> +\t\tstat_err = stat(iter->base.path.buf, &iter->base.st);\n> \telse\n> -\t\terr = lstat(iter->base.path.buf, &iter->base.st);\n> +\t\tstat_err = lstat(iter->base.path.buf, &iter->base.st);\n> \n> -\tsaved_errno = errno;\n> -\tif (err && errno != ENOENT)\n> +\tif (stat_err && errno != ENOENT) {\n> \t\twarning_errno(\"failed to stat '%s'\", iter->base.path.buf);\n> +\t\treturn FAIL_NOT_ENOENT;\n> +\t} else if (stat_err && errno == ENOENT) {\n> +\t\treturn FAIL_ENOENT;\n> +\t}\n> \n> -\terrno = saved_errno;\n> -\treturn err;\n> +\t/*\n> +\t * We have to reset relative path and basename because the path strbuf\n> +\t * might have been realloc()'ed at the previous strbuf_addstr().\n> +\t */\n> +\n> +\titer->base.relative_path =\n> +\t\titer->base.path.buf + iter->levels[0].prefix_len + 1;\n> +\titer->base.basename =\n> +\t\titer->base.path.buf + iter->levels[iter->levels_nr - 1].prefix_len + 1;\n> +\n> +\treturn OK;\n> }\n> \n> int dir_iterator_advance(struct dir_iterator *dir_iterator)\n> {\n> -\tstruct dir_iterator_int *iter =\n> -\t\t(struct dir_iterator_int *)dir_iterator;\n> +\tstruct dir_iterator_int *iter = (struct dir_iterator_int *)dir_iterator;\n> +\tstruct dir_iterator_level *level = &iter->levels[iter->levels_nr - 1];\n> +\tstruct dirent *dir_entry = NULL;\n> +\tint expose_err, activate_err;\n> +\t/* For shorter code width-wise, more readable */\n> +\tunsigned int PEDANTIC = iter->flags & DIR_ITERATOR_PEDANTIC;\n> \n> -\tif (S_ISDIR(iter->base.st.st_mode) && push_level(iter)) {\n> -\t\tif (errno != ENOENT && iter->flags & DIR_ITERATOR_PEDANTIC)\n> -\t\t\tgoto error_out;\n> -\t\tif (iter->levels_nr == 0)\n> +\t/*\n> +\t * Attempt to open the directory of the last level if not opened yet.\n> +\t *\n> +\t * Remember that we ignore ENOENT errors so that the user of this API\n> +\t * can remove entries between calls to `dir_iterator_advance()`.\n> +\t * We care for errors other than ENOENT only when PEDANTIC is enabled.\n> +\t */\n> +\n> +\tactivate_err = activate_level(iter);\n> +\n> +\tif (activate_err == FAIL_NOT_ENOENT && PEDANTIC) {\n> +\t\tgoto error_out;\n> +\t} else if (activate_err != OK) {\n> +\t\t--iter->levels_nr;\n> +\n> +\t\tif (iter->levels_nr == 0) /* Failed to open root directory */\n> \t\t\tgoto error_out;\n> +\n> +\t\treturn dir_iterator_advance(dir_iterator);\n> \t}\n> \n> -\t/* Loop until we find an entry that we can give back to the caller. */\n> -\twhile (1) {\n> -\t\tstruct dirent *de;\n> -\t\tstruct dir_iterator_level *level =\n> -\t\t\t&iter->levels[iter->levels_nr - 1];\n> +\tstrbuf_setlen(&iter->base.path, level->prefix_len);\n> +\n> +\terrno = 0;\n> +\tdir_entry = readdir(level->dir);\n> +\n> +\tif (!dir_entry) {\n> +\t\tif (errno) {\n> +\t\t\twarning_errno(\"errno reading dir '%s'\", iter->base.path.buf);\n> +\t\t\tif (PEDANTIC)\n> +\t\t\t\tgoto error_out;\n> \n> -\t\tstrbuf_setlen(&iter->base.path, level->prefix_len);\n> -\t\terrno = 0;\n> -\t\tde = readdir(level->dir);\n> -\n> -\t\tif (!de) {\n> -\t\t\tif (errno) {\n> -\t\t\t\twarning_errno(\"error reading directory '%s'\",\n> -\t\t\t\t\t iter->base.path.buf);\n> -\t\t\t\tif (iter->flags & DIR_ITERATOR_PEDANTIC)\n> -\t\t\t\t\tgoto error_out;\n> -\t\t\t} else if (pop_level(iter) == 0) {\n> +\t\t\treturn dir_iterator_advance(dir_iterator);\n> +\t\t} else {\n> +\t\t\t/*\n> +\t\t\t * Current directory has been iterated through.\n> +\t\t\t */\n> +\n> +\t\t\tif (pop_level(iter) == 0)\n> \t\t\t\treturn dir_iterator_abort(dir_iterator);\n> -\t\t\t}\n> -\t\t\tcontinue;\n> +\n> +\t\t\treturn dir_iterator_advance(dir_iterator);\n> \t\t}\n> +\t}\n> \n> -\t\tif (is_dot_or_dotdot(de->d_name))\n> -\t\t\tcontinue;\n> +\tif (is_dot_or_dotdot(dir_entry->d_name))\n> +\t\treturn dir_iterator_advance(dir_iterator);\n> \n> -\t\tif (prepare_next_entry_data(iter, de)) {\n> -\t\t\tif (errno != ENOENT && iter->flags & DIR_ITERATOR_PEDANTIC)\n> -\t\t\t\tgoto error_out;\n> -\t\t\tcontinue;\n> -\t\t}\n> +\t/*\n> +\t * Successfully read entry from current directory level.\n> +\t */\n> \n> -\t\treturn ITER_OK;\n> -\t}\n> +\texpose_err = expose_entry(iter, dir_entry->d_name);\n> +\n> +\tif (expose_err == FAIL_NOT_ENOENT && PEDANTIC)\n> +\t\tgoto error_out;\n> +\n> +\tif (expose_err == OK)\n> +\t\tpush_level(iter);\n> +\n> +\tif (expose_err != OK)\n> +\t\treturn dir_iterator_advance(dir_iterator);\n> +\n> +\treturn ITER_OK;\n> \n> error_out:\n> \tdir_iterator_abort(dir_iterator);\n> @@ -207,6 +269,8 @@ struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags)\n> \n> \tstrbuf_init(&iter->base.path, PATH_MAX);\n> \tstrbuf_addstr(&iter->base.path, path);\n> +\t/* expose_entry() appends dir seperator before exposing an entry */\n> +\tstrbuf_trim_trailing_dir_sep(&iter->base.path);\n> \n> \tALLOC_GROW(iter->levels, 10, iter->levels_alloc);\n> \titer->levels_nr = 0;\n> @@ -226,6 +290,11 @@ struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags)\n> \t\tgoto error_out;\n> \t}\n> \n> +\tif (!push_level(iter)) {\n> +\t\tsaved_errno = ENOTDIR;\n> +\t\tgoto error_out;\n> +\t}\n> +\n> \treturn dir_iterator;\n> \n> error_out:\n\n"},{"id":"455078","messageId":"88421b18-0fa0-236a-b74b-c5ee3ef53279@gmail.com","threadId":"57864","inReplyTo":"20220509175159.2948802-14-kioplato@gmail.com","subject":"Re: [PATCH v2 13/15] dir-iterator: option to iterate dirs in pre-order","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-05-10T13:07:51Z","receivedAt":"2022-05-10T13:08:00Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Plato\n\nOn 09/05/2022 18:51, Plato Kiorpelidis wrote:\n> Introduce a new option to dir-iterator, using dir_iterator_begin()\n> flags parameter, allowing to control whether or not directories will\n> be exposed before their contents. In essence, pre-order traversal over\n> file system entries that are directories.\n> \n> This changes the default behavior of the dir-iterator API. Instead\n> of iterating directories before doing so over their contents, the new\n> default behavior does not expose directories at all. Iteration is still\n> performed, however, within directories, iterating over any other entry.\n> Only directory paths are ignored.\n> \n> To iterate over directories in pre-order, reproducing the previous\n> default behavior, enable the new flag DIR_ITERATOR_DIRS_BEFORE in the\n> flags parameter of dir_iterator_begin():\n> * ignore directories by not setting DIR_ITERATOR_DIRS_BEFORE\n> * iterate directories pre-order by enabling DIR_ITERATOR_DIRS_BEFORE\n> \n> Adjust existing callers, in refs/files-backend.c and builtin/clone.c\n> to enable DIR_ITERATOR_DIRS_BEFORE since these callers require iteration\n> over directories before doing so over their contents.\n> \n> Update t/t0066-dir-iterator.sh and t/helper/test-dir-iterator.c to test\n> the new iteration scheme, which is the new default behavior, and the new\n> flag DIR_ITERATOR_DIRS_BEFORE which reproduces the old default behavior.\n\nIt's great that you've split this change out from the next patch. I had \nnot realized when I looked at the last round that all the existing \ncallers require pre-order traversal. Given that is the case I'm finding \nit hard to see how changing the default behavior to one that no caller \nis using is an improvement.\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Plato Kiorpelidis \n> ---\n> builtin/clone.c | 4 +-\n> dir-iterator.c | 29 +++-\n> dir-iterator.h | 29 +++-\n> refs/files-backend.c | 2 +-\n> t/helper/test-dir-iterator.c | 12 +-\n> t/t0066-dir-iterator.sh | 321 ++++++++++++++++++++++++++++++++---\n> 6 files changed, 361 insertions(+), 36 deletions(-)\n> \n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index 194d50f75f..68787623e7 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -321,7 +321,9 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,\n> \n> \tmkdir_if_missing(dest->buf, 0777);\n> \n> -\tflags = DIR_ITERATOR_PEDANTIC | DIR_ITERATOR_FOLLOW_SYMLINKS;\n> +\tflags = DIR_ITERATOR_DIRS_BEFORE |\n> +\t\tDIR_ITERATOR_PEDANTIC |\n> +\t\tDIR_ITERATOR_FOLLOW_SYMLINKS;\n> \titer = dir_iterator_begin(src->buf, flags);\n> \n> \tif (!iter)\n> diff --git a/dir-iterator.c b/dir-iterator.c\n> index c36f549a78..c1475add27 100644\n> --- a/dir-iterator.c\n> +++ b/dir-iterator.c\n> @@ -47,6 +47,7 @@ enum {\n> \tOK,\n> \tFAIL_ENOENT,\n> \tFAIL_NOT_ENOENT,\n> +\tFAIL_IGN_DIRS,\n> };\n> \n> /*\n> @@ -124,12 +125,14 @@ static int pop_level(struct dir_iterator_int *iter)\n> *\n> * Return values:\n> * OK on successful exposure of the provided entry.\n> + * FAIL_IGN_DIR on failed exposure because entry is dir and flags don't allow it.\n> * FAIL_ENOENT on failed exposure because entry does not exist.\n> * FAIL_NOT_ENOENT on failed exposure because of errno other than ENOENT.\n> */\n> -static int expose_entry(struct dir_iterator_int *iter, char *d_name)\n> +static int expose_entry(struct dir_iterator_int *iter, char *d_name, char *dir_state)\n> {\n> \tint stat_err;\n> +\tunsigned int DIRS_BEFORE = iter->flags & DIR_ITERATOR_DIRS_BEFORE;\n> \n> \tstrbuf_addch(&iter->base.path, '/');\n> \tstrbuf_addstr(&iter->base.path, d_name);\n> @@ -146,6 +149,17 @@ static int expose_entry(struct dir_iterator_int *iter, char *d_name)\n> \t\treturn FAIL_ENOENT;\n> \t}\n> \n> +\t/*\n> +\t * We've got to check whether or not this is a directory. We need to\n> +\t * perform this check since the user could've requested to ignore\n> +\t * directory entries.\n> +\t */\n> +\n> +\tif (S_ISDIR(iter->base.st.st_mode)) {\n> +\t\tif (!DIRS_BEFORE && !strcmp(dir_state, \"before\"))\n> +\t\t\treturn FAIL_IGN_DIRS;\n> +\t}\n> +\n> \t/*\n> \t * We have to reset relative path and basename because the path strbuf\n> \t * might have been realloc()'ed at the previous strbuf_addstr().\n> @@ -220,14 +234,23 @@ int dir_iterator_advance(struct dir_iterator *dir_iterator)\n> \n> \t/*\n> \t * Successfully read entry from current directory level.\n> +\t * In case it's a directory, we need to check, before exposing it, if\n> +\t * it's allowed because of DIRS_BEFORE. In any case - allowed or not -\n> +\t * we must push the directory to the levels stack, so the next call will\n> +\t * read from it.\n> +\t */\n> +\n> +\t/*\n> +\t * 'expose_entry()' function needs to know whether\n> +\t * the exposure call is about DIRS_BEFORE or DIRS_AFTER.\n> \t */\n> \n> -\texpose_err = expose_entry(iter, dir_entry->d_name);\n> +\texpose_err = expose_entry(iter, dir_entry->d_name, \"before\");\n> \n> \tif (expose_err == FAIL_NOT_ENOENT && PEDANTIC)\n> \t\tgoto error_out;\n> \n> -\tif (expose_err == OK)\n> +\tif (expose_err == OK || expose_err == FAIL_IGN_DIRS)\n> \t\tpush_level(iter);\n> \n> \tif (expose_err != OK)\n> diff --git a/dir-iterator.h b/dir-iterator.h\n> index 08229157c6..c1d16a8c6d 100644\n> --- a/dir-iterator.h\n> +++ b/dir-iterator.h\n> @@ -8,19 +8,22 @@\n> *\n> * Iterate over a directory tree, recursively, including paths of all\n> * types and hidden paths. Skip \".\" and \"..\" entries and don't follow\n> - * symlinks except for the original path. Note that the original path\n> - * is not included in the iteration.\n> + * symlinks except when DIR_ITERATOR_FOLLOW_SYMLINKS is set. If root\n> + * path is a symlink it's followed regardless of flags. Note that the\n> + * original path is not included in the iteration.\n> *\n> * Every time dir_iterator_advance() is called, update the members of\n> * the dir_iterator structure to reflect the next path in the\n> * iteration. The order that paths are iterated over within a\n> - * directory is undefined, directory paths are always given before\n> - * their contents.\n> + * directory is undefined. Directory paths are given before their\n> + * contents when DIR_ITERATOR_DIRS_BEFORE is set. Failure to set this\n> + * flag results in directory paths not being exposed. Instead, iteration\n> + * will happen within directories. Their contents will be exposed.\n> *\n> * A typical iteration looks like this:\n> *\n> * int ok;\n> - * unsigned int flags = DIR_ITERATOR_PEDANTIC;\n> + * unsigned int flags = DIR_ITERATOR_PEDANTIC | DIR_ITERATOR_DIRS_BEFORE;\n> * struct dir_iterator *iter = dir_iterator_begin(path, flags);\n> *\n> * if (!iter)\n> @@ -61,12 +64,19 @@\n> * not the symlinks themselves, which is the default behavior. Broken\n> * symlinks are ignored.\n> *\n> + * - DIR_ITERATOR_DIRS_BEFORE: make dir-iterator expose a directory path\n> + * before iterating through that directory's contents.\n> + *\n> + * Note: Activating none of the flags will iterate through directories'\n> + * contents but won't expose the directory paths.\n> + *\n> * Warning: circular symlinks are also followed when\n> * DIR_ITERATOR_FOLLOW_SYMLINKS is set. The iteration may end up with\n> * an ELOOP if they happen and DIR_ITERATOR_PEDANTIC is set.\n> */\n> #define DIR_ITERATOR_PEDANTIC (1 << 0)\n> #define DIR_ITERATOR_FOLLOW_SYMLINKS (1 << 1)\n> +#define DIR_ITERATOR_DIRS_BEFORE (1 << 2)\n> \n> struct dir_iterator {\n> \t/* The current path: */\n> @@ -97,12 +107,13 @@ struct dir_iterator {\n> * failure, return NULL and set errno accordingly.\n> *\n> * The iteration includes all paths under path, not including path\n> - * itself and not including \".\" or \"..\" entries.\n> + * itself, \".\" or \"..\" entries and directories according to specified flags.\n> *\n> * Parameters are:\n> * - path is the starting directory. An internal copy will be made.\n> * - flags is a combination of the possible flags to initialize a\n> - * dir-iterator or 0 for default behavior.\n> + * dir-iterator or 0 for default behavior which will ignore directory\n> + * paths, but will include the rest directory contents.\n> */\n> struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags);\n> \n> @@ -110,6 +121,7 @@ struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags);\n> * Advance the iterator to the first or next item and return ITER_OK.\n> * If the iteration is exhausted, free the dir_iterator and any\n> * resources associated with it and return ITER_DONE.\n> + * On error, free dir_iterator memory and return ITER_ERROR.\n> *\n> * It is a bug to use iterator or call this function again after it\n> * has returned ITER_DONE or ITER_ERROR (which may be returned iff\n> @@ -119,8 +131,7 @@ int dir_iterator_advance(struct dir_iterator *iterator);\n> \n> /*\n> * End the iteration before it has been exhausted. Free the\n> - * dir_iterator and any associated resources and return ITER_DONE. On\n> - * error, free the dir_iterator and return ITER_ERROR.\n> + * dir_iterator and any associated resources and return ITER_DONE.\n> */\n> int dir_iterator_abort(struct dir_iterator *iterator);\n> \n> diff --git a/refs/files-backend.c b/refs/files-backend.c\n> index 8db7882aac..812f00c0f4 100644\n> --- a/refs/files-backend.c\n> +++ b/refs/files-backend.c\n> @@ -2237,7 +2237,7 @@ static struct ref_iterator *reflog_iterator_begin(struct ref_store *ref_store,\n> \n> \tstrbuf_addf(&sb, \"%s/logs\", gitdir);\n> \n> -\tditer = dir_iterator_begin(sb.buf, 0);\n> +\tditer = dir_iterator_begin(sb.buf, DIR_ITERATOR_DIRS_BEFORE);\n> \tif (!diter) {\n> \t\tstrbuf_release(&sb);\n> \t\treturn empty_ref_iterator_begin();\n> diff --git a/t/helper/test-dir-iterator.c b/t/helper/test-dir-iterator.c\n> index 55d8a58836..f05d5fde9d 100644\n> --- a/t/helper/test-dir-iterator.c\n> +++ b/t/helper/test-dir-iterator.c\n> @@ -17,7 +17,15 @@ static const char *error_name(int error_number)\n> \n> /*\n> * usage:\n> - * tool-test dir-iterator [--follow-symlinks] [--pedantic] directory_path\n> + * test-tool dir-iterator [OPTIONS] directory_path\n> + *\n> + * OPTIONS\n> + *\t--follow-symlinks\n> + *\t--pedantic\n> + *\t--dirs-before\n> + *\n> + * example:\n> + * test-tool dir-iterator --pedantic --dirs-before ./somedir\n> */\n> int cmd__dir_iterator(int argc, const char **argv)\n> {\n> @@ -30,6 +38,8 @@ int cmd__dir_iterator(int argc, const char **argv)\n> \t\t\tflags |= DIR_ITERATOR_FOLLOW_SYMLINKS;\n> \t\telse if (strcmp(*argv, \"--pedantic\") == 0)\n> \t\t\tflags |= DIR_ITERATOR_PEDANTIC;\n> +\t\telse if (strcmp(*argv, \"--dirs-before\") == 0)\n> +\t\t\tflags |= DIR_ITERATOR_DIRS_BEFORE;\n> \t\telse\n> \t\t\tdie(\"invalid option '%s'\", *argv);\n> \t}\n> diff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\n> index 8ab8811fb5..badd82d8a4 100755\n> --- a/t/t0066-dir-iterator.sh\n> +++ b/t/t0066-dir-iterator.sh\n> @@ -9,7 +9,7 @@ test_expect_success 'setup -- dir with a single file' '\n> \tmkdir dir1 &&\n> \t>dir1/a\n> '\n> -test_expect_success 'dirs-before of dir with a file' '\n> +test_expect_success 'dirs-ignore of dir with a file' '\n> \tcat >expected-out <<-EOF &&\n> \t[f] (a) [a] ./dir1/a\n> \tEOF\n> @@ -17,22 +17,60 @@ test_expect_success 'dirs-before of dir with a file' '\n> \ttest-tool dir-iterator ./dir1 >actual-out &&\n> \ttest_cmp expected-out actual-out\n> '\n> +test_expect_success 'dirs-before of dir with a file' '\n> +\tcat >expected-out <<-EOF &&\n> +\t[f] (a) [a] ./dir1/a\n> +\tEOF\n> +\n> +\ttest-tool dir-iterator --dirs-before ./dir1 >actual-out &&\n> +\ttest_cmp expected-out actual-out\n> +'\n> \n> test_expect_success 'setup -- dir with a single dir' '\n> \tmkdir -p dir2/a\n> '\n> +test_expect_success 'dirs-ignore of dir with a single dir' '\n> +\tcat >expected-out <<-EOF &&\n> +\tEOF\n> +\n> +\ttest-tool dir-iterator ./dir2 >actual-out &&\n> +\ttest_cmp expected-out actual-out\n> +'\n> test_expect_success 'dirs-before of dir with a single dir' '\n> \tcat >expected-out <<-EOF &&\n> \t[d] (a) [a] ./dir2/a\n> \tEOF\n> \n> -\ttest-tool dir-iterator ./dir2 >actual-out &&\n> +\ttest-tool dir-iterator --dirs-before ./dir2 >actual-out &&\n> \ttest_cmp expected-out actual-out\n> '\n> \n> test_expect_success POSIXPERM,SANITY 'setup -- dir w/ single dir w/o perms' '\n> \tmkdir -p dir3/a\n> '\n> +test_expect_success POSIXPERM,SANITY 'dirs-ignore of dir w/ dir w/o perms' '\n> +\tcat >expected-out <<-EOF &&\n> +\tEOF\n> +\n> +\tchmod 0 dir3/a &&\n> +\n> +\ttest-tool dir-iterator ./dir3/ >actual-out &&\n> +\ttest_cmp expected-out actual-out &&\n> +\n> +\tchmod 755 dir3/a\n> +'\n> +test_expect_success POSIXPERM,SANITY 'pedantic dirs-ignore of dir w/ dir w/o perms' '\n> +\tcat >expected-out <<-EOF &&\n> +\tdir_iterator_advance failure: EACCES\n> +\tEOF\n> +\n> +\tchmod 0 dir3/a &&\n> +\n> +\ttest_must_fail test-tool dir-iterator --pedantic ./dir3/ >actual-out &&\n> +\ttest_cmp expected-out actual-out &&\n> +\n> +\tchmod 755 dir3/a\n> +'\n> test_expect_success POSIXPERM,SANITY 'dirs-before of dir w/ dir w/o perms' '\n> \tcat >expected-out <<-EOF &&\n> \t[d] (a) [a] ./dir3/a\n> @@ -40,7 +78,7 @@ test_expect_success POSIXPERM,SANITY 'dirs-before of dir w/ dir w/o perms' '\n> \n> \tchmod 0 dir3/a &&\n> \n> -\ttest-tool dir-iterator ./dir3/ >actual-out &&\n> +\ttest-tool dir-iterator --dirs-before ./dir3/ >actual-out &&\n> \ttest_cmp expected-out actual-out &&\n> \n> \tchmod 755 dir3/a\n> @@ -53,7 +91,8 @@ test_expect_success POSIXPERM,SANITY 'pedantic dirs-before of dir w/ dir w/o per\n> \n> \tchmod 0 dir3/a &&\n> \n> -\ttest_must_fail test-tool dir-iterator --pedantic ./dir3/ >actual-out &&\n> +\ttest_must_fail test-tool dir-iterator --dirs-before \\\n> +\t\t--pedantic ./dir3/ >actual-out &&\n> \ttest_cmp expected-out actual-out &&\n> \n> \tchmod 755 dir3/a\n> @@ -67,7 +106,7 @@ test_expect_success 'setup -- dir w/ five files' '\n> \t>dir4/d &&\n> \t>dir4/e\n> '\n> -test_expect_success 'dirs-before of dir w/ five files' '\n> +test_expect_success 'dirs-ignore of dir w/ five files' '\n> \tcat >expected-sorted-out <<-EOF &&\n> \t[f] (a) [a] ./dir4/a\n> \t[f] (b) [b] ./dir4/b\n> @@ -81,18 +120,40 @@ test_expect_success 'dirs-before of dir w/ five files' '\n> \n> \ttest_cmp expected-sorted-out actual-sorted-out\n> '\n> +test_expect_success 'dirs-before of dir w/ five files' '\n> +\tcat >expected-sorted-out <<-EOF &&\n> +\t[f] (a) [a] ./dir4/a\n> +\t[f] (b) [b] ./dir4/b\n> +\t[f] (c) [c] ./dir4/c\n> +\t[f] (d) [d] ./dir4/d\n> +\t[f] (e) [e] ./dir4/e\n> +\tEOF\n> +\n> +\ttest-tool dir-iterator --dirs-before ./dir4 >actual-out &&\n> +\tsort actual-out >actual-sorted-out &&\n> +\n> +\ttest_cmp expected-sorted-out actual-sorted-out\n> +'\n> \n> test_expect_success 'setup -- dir w/ dir w/ a file' '\n> \tmkdir -p dir5/a &&\n> \t>dir5/a/b\n> '\n> +test_expect_success 'dirs-ignore of dir w/ dir w/ a file' '\n> +\tcat >expected-out <<-EOF &&\n> +\t[f] (a/b) [b] ./dir5/a/b\n> +\tEOF\n> +\n> +\ttest-tool dir-iterator ./dir5 >actual-out &&\n> +\ttest_cmp expected-out actual-out\n> +'\n> test_expect_success 'dirs-before of dir w/ dir w/ a file' '\n> \tcat >expected-out <<-EOF &&\n> \t[d] (a) [a] ./dir5/a\n> \t[f] (a/b) [b] ./dir5/a/b\n> \tEOF\n> \n> -\ttest-tool dir-iterator ./dir5 >actual-out &&\n> +\ttest-tool dir-iterator --dirs-before ./dir5 >actual-out &&\n> \ttest_cmp expected-out actual-out\n> '\n> \n> @@ -100,6 +161,14 @@ test_expect_success 'setup -- dir w/ three nested dirs w/ file' '\n> \tmkdir -p dir6/a/b/c &&\n> \t>dir6/a/b/c/d\n> '\n> +test_expect_success 'dirs-ignore of dir w/ three nested dirs w/ file' '\n> +\tcat >expected-out <<-EOF &&\n> +\t[f] (a/b/c/d) [d] ./dir6/a/b/c/d\n> +\tEOF\n> +\n> +\ttest-tool dir-iterator ./dir6 >actual-out &&\n> +\ttest_cmp expected-out actual-out\n> +'\n> test_expect_success 'dirs-before of dir w/ three nested dirs w/ file' '\n> \tcat >expected-out <<-EOF &&\n> \t[d] (a) [a] ./dir6/a\n> @@ -108,7 +177,7 @@ test_expect_success 'dirs-before of dir w/ three nested dirs w/ file' '\n> \t[f] (a/b/c/d) [d] ./dir6/a/b/c/d\n> \tEOF\n> \n> -\ttest-tool dir-iterator ./dir6 >actual-out &&\n> +\ttest-tool dir-iterator --dirs-before ./dir6 >actual-out &&\n> \ttest_cmp expected-out actual-out\n> '\n> \n> @@ -119,6 +188,33 @@ test_expect_success POSIXPERM,SANITY \\\n> \t>dir7/a/b/c/d\n> '\n> test_expect_success POSIXPERM,SANITY \\\n> +'dirs-ignore of dir w/ three nested dirs w/ file, second w/o perms' '\n> +\n> +\tcat >expected-out <<-EOF &&\n> +\tEOF\n> +\n> +\tchmod 0 dir7/a/b &&\n> +\n> +\ttest-tool dir-iterator ./dir7 >actual-out &&\n> +\ttest_cmp expected-out actual-out &&\n> +\n> +\tchmod 755 dir7/a/b\n> +'\n> +test_expect_success POSIXPERM,SANITY \\\n> +'pedantic dirs-ignore of dir w/ three nested dirs w/ file, second w/o perms' '\n> +\n> +\tcat >expected-out <<-EOF &&\n> +\tdir_iterator_advance failure: EACCES\n> +\tEOF\n> +\n> +\tchmod 0 dir7/a/b &&\n> +\n> +\ttest_must_fail test-tool dir-iterator --pedantic ./dir7 >actual-out &&\n> +\ttest_cmp expected-out actual-out &&\n> +\n> +\tchmod 755 dir7/a/b\n> +'\n> +test_expect_success POSIXPERM,SANITY \\\n> 'dirs-before of dir w/ three nested dirs w/ file, second w/o perms' '\n> \n> \tcat >expected-out <<-EOF &&\n> @@ -128,7 +224,7 @@ test_expect_success POSIXPERM,SANITY \\\n> \n> \tchmod 0 dir7/a/b &&\n> \n> -\ttest-tool dir-iterator ./dir7 >actual-out &&\n> +\ttest-tool dir-iterator --dirs-before ./dir7 >actual-out &&\n> \ttest_cmp expected-out actual-out &&\n> \n> \tchmod 755 dir7/a/b\n> @@ -144,7 +240,8 @@ test_expect_success POSIXPERM,SANITY \\\n> \n> \tchmod 0 dir7/a/b &&\n> \n> -\ttest_must_fail test-tool dir-iterator --pedantic ./dir7 >actual-out &&\n> +\ttest_must_fail test-tool dir-iterator --dirs-before \\\n> +\t\t--pedantic ./dir7 >actual-out &&\n> \ttest_cmp expected-out actual-out &&\n> \n> \tchmod 755 dir7/a/b\n> @@ -156,6 +253,22 @@ test_expect_success 'setup -- dir w/ two dirs each w/ file' '\n> \tmkdir dir8/c &&\n> \t>dir8/c/d\n> '\n> +test_expect_success 'dirs-ignore of dir w/ two dirs each w/ file' '\n> +\tcat >expected-out1 <<-EOF &&\n> +\t[f] (a/b) [b] ./dir8/a/b\n> +\t[f] (c/d) [d] ./dir8/c/d\n> +\tEOF\n> +\tcat >expected-out2 <<-EOF &&\n> +\t[f] (c/d) [d] ./dir8/c/d\n> +\t[f] (a/b) [b] ./dir8/a/b\n> +\tEOF\n> +\n> +\ttest-tool dir-iterator ./dir8 >actual-out &&\n> +\t(\n> +\t\ttest_cmp expected-out1 actual-out ||\n> +\t\ttest_cmp expected-out2 actual-out\n> +\t)\n> +'\n> test_expect_success 'dirs-before of dir w/ two dirs each w/ file' '\n> \tcat >expected-out1 <<-EOF &&\n> \t[d] (a) [a] ./dir8/a\n> @@ -170,7 +283,7 @@ test_expect_success 'dirs-before of dir w/ two dirs each w/ file' '\n> \t[f] (a/b) [b] ./dir8/a/b\n> \tEOF\n> \n> -\ttest-tool dir-iterator ./dir8 >actual-out &&\n> +\ttest-tool dir-iterator --dirs-before ./dir8 >actual-out &&\n> \t(\n> \t\ttest_cmp expected-out1 actual-out ||\n> \t\ttest_cmp expected-out2 actual-out\n> @@ -185,6 +298,38 @@ test_expect_success 'setup -- dir w/ two dirs, one w/ two and one w/ one files'\n> \t>dir9/d/e\n> '\n> test_expect_success \\\n> +'dirs-ignore of dir w/ two dirs, one w/ two and one w/ one files' '\n> +\n> +\tcat >expected-out1 <<-EOF &&\n> +\t[f] (a/b) [b] ./dir9/a/b\n> +\t[f] (a/c) [c] ./dir9/a/c\n> +\t[f] (d/e) [e] ./dir9/d/e\n> +\tEOF\n> +\tcat >expected-out2 <<-EOF &&\n> +\t[f] (a/c) [c] ./dir9/a/c\n> +\t[f] (a/b) [b] ./dir9/a/b\n> +\t[f] (d/e) [e] ./dir9/d/e\n> +\tEOF\n> +\tcat >expected-out3 <<-EOF &&\n> +\t[f] (d/e) [e] ./dir9/d/e\n> +\t[f] (a/b) [b] ./dir9/a/b\n> +\t[f] (a/c) [c] ./dir9/a/c\n> +\tEOF\n> +\tcat >expected-out4 <<-EOF &&\n> +\t[f] (d/e) [e] ./dir9/d/e\n> +\t[f] (a/c) [c] ./dir9/a/c\n> +\t[f] (a/b) [b] ./dir9/a/b\n> +\tEOF\n> +\n> +\ttest-tool dir-iterator ./dir9 >actual-out &&\n> +\t(\n> +\t\ttest_cmp expected-out1 actual-out ||\n> +\t\ttest_cmp expected-out2 actual-out ||\n> +\t\ttest_cmp expected-out3 actual-out ||\n> +\t\ttest_cmp expected-out4 actual-out\n> +\t)\n> +'\n> +test_expect_success \\\n> 'dirs-before of dir w/ two dirs, one w/ two and one w/ one files' '\n> \n> \tcat >expected-out1 <<-EOF &&\n> @@ -216,7 +361,7 @@ test_expect_success \\\n> \t[f] (a/b) [b] ./dir9/a/b\n> \tEOF\n> \n> -\ttest-tool dir-iterator ./dir9 >actual-out &&\n> +\ttest-tool dir-iterator --dirs-before ./dir9 >actual-out &&\n> \t(\n> \t\ttest_cmp expected-out1 actual-out ||\n> \t\ttest_cmp expected-out2 actual-out ||\n> @@ -231,6 +376,22 @@ test_expect_success 'setup -- dir w/ two nested dirs, each w/ file' '\n> \tmkdir dir10/a/c &&\n> \t>dir10/a/c/d\n> '\n> +test_expect_success 'dirs-ignore of dir w/ two nested dirs, each w/ file' '\n> +\tcat >expected-out1 <<-EOF &&\n> +\t[f] (a/b) [b] ./dir10/a/b\n> +\t[f] (a/c/d) [d] ./dir10/a/c/d\n> +\tEOF\n> +\tcat >expected-out2 <<-EOF &&\n> +\t[f] (a/c/d) [d] ./dir10/a/c/d\n> +\t[f] (a/b) [b] ./dir10/a/b\n> +\tEOF\n> +\n> +\ttest-tool dir-iterator ./dir10/ >actual-out &&\n> +\t(\n> +\t\ttest_cmp expected-out1 actual-out ||\n> +\t\ttest_cmp expected-out2 actual-out\n> +\t)\n> +'\n> test_expect_success 'dirs-before of dir w/ two nested dirs, each w/ file' '\n> \tcat >expected-out1 <<-EOF &&\n> \t[d] (a) [a] ./dir10/a\n> @@ -245,7 +406,7 @@ test_expect_success 'dirs-before of dir w/ two nested dirs, each w/ file' '\n> \t[f] (a/b) [b] ./dir10/a/b\n> \tEOF\n> \n> -\ttest-tool dir-iterator ./dir10/ >actual-out &&\n> +\ttest-tool dir-iterator --dirs-before ./dir10/ >actual-out &&\n> \t(\n> \t\ttest_cmp expected-out1 actual-out ||\n> \t\ttest_cmp expected-out2 actual-out\n> @@ -262,6 +423,20 @@ test_expect_success 'setup -- dir w/ complex structure' '\n> \t>dir11/a/e &&\n> \t>dir11/d/e/d/a\n> '\n> +test_expect_success 'dirs-ignore of dir w/ complex structure' '\n> +\tcat >expected-sorted-out <<-EOF &&\n> +\t[f] (a/b/c/d) [d] ./dir11/a/b/c/d\n> +\t[f] (a/e) [e] ./dir11/a/e\n> +\t[f] (b) [b] ./dir11/b\n> +\t[f] (c) [c] ./dir11/c\n> +\t[f] (d/e/d/a) [a] ./dir11/d/e/d/a\n> +\tEOF\n> +\n> +\ttest-tool dir-iterator ./dir11 >actual-out &&\n> +\tsort actual-out >actual-sorted-out &&\n> +\n> +\ttest_cmp expected-sorted-out actual-sorted-out\n> +'\n> test_expect_success 'dirs-before of dir w/ complex structure' '\n> \tcat >expected-sorted-out <<-EOF &&\n> \t[d] (a) [a] ./dir11/a\n> @@ -277,7 +452,7 @@ test_expect_success 'dirs-before of dir w/ complex structure' '\n> \t[f] (d/e/d/a) [a] ./dir11/d/e/d/a\n> \tEOF\n> \n> -\ttest-tool dir-iterator ./dir11 >actual-out &&\n> +\ttest-tool dir-iterator --dirs-before ./dir11 >actual-out &&\n> \tsort actual-out >actual-sorted-out &&\n> \n> \ttest_cmp expected-sorted-out actual-sorted-out\n> @@ -287,7 +462,7 @@ test_expect_success POSIXPERM,SANITY 'setup -- dir w/o perms' '\n> \tmkdir -p dir12/a &&\n> \t>dir12/a/b\n> '\n> -test_expect_success POSIXPERM,SANITY 'dirs-before of root dir w/o perms' '\n> +test_expect_success POSIXPERM,SANITY 'dirs-ignore of root dir w/o perms' '\n> \tcat >expected-out <<-EOF &&\n> \tdir_iterator_begin failure: EACCES\n> \tEOF\n> @@ -299,7 +474,7 @@ test_expect_success POSIXPERM,SANITY 'dirs-before of root dir w/o perms' '\n> \n> \tchmod 755 dir12\n> '\n> -test_expect_success POSIXPERM,SANITY 'pedantic dirs-before of root dir w/o perms' '\n> +test_expect_success POSIXPERM,SANITY 'pedantic dirs-ignore of root dir w/o perms' '\n> \tcat >expected-out <<-EOF &&\n> \tdir_iterator_begin failure: EACCES\n> \tEOF\n> @@ -311,6 +486,31 @@ test_expect_success POSIXPERM,SANITY 'pedantic dirs-before of root dir w/o perms\n> \n> \tchmod 755 dir12\n> '\n> +test_expect_success POSIXPERM,SANITY 'dirs-before of root dir w/o perms' '\n> +\tcat >expected-out <<-EOF &&\n> +\tdir_iterator_begin failure: EACCES\n> +\tEOF\n> +\n> +\tchmod 0 dir12 &&\n> +\n> +\ttest_must_fail test-tool dir-iterator --dirs-before ./dir12 >actual-out &&\n> +\ttest_cmp expected-out actual-out &&\n> +\n> +\tchmod 755 dir12\n> +'\n> +test_expect_success POSIXPERM,SANITY 'pedantic dirs-before of root dir w/o perms' '\n> +\tcat >expected-out <<-EOF &&\n> +\tdir_iterator_begin failure: EACCES\n> +\tEOF\n> +\n> +\tchmod 0 dir12 &&\n> +\n> +\ttest_must_fail test-tool dir-iterator --dirs-before \\\n> +\t\t--pedantic ./dir12 >actual-out &&\n> +\ttest_cmp expected-out actual-out &&\n> +\n> +\tchmod 755 dir12\n> +'\n> \n> test_expect_success 'begin should fail upon inexistent paths' '\n> \techo \"dir_iterator_begin failure: ENOENT\" >expected-out &&\n> @@ -335,6 +535,33 @@ test_expect_success POSIXPERM,SANITY 'setup -- dir w/ dir w/o perms w/ file' '\n> \t>dir13/a/b\n> '\n> test_expect_success POSIXPERM,SANITY \\\n> +'dirs-ignore of dir w/ dir w/o perms w/ file' '\n> +\n> +\tcat >expected-out <<-EOF &&\n> +\tEOF\n> +\n> +\tchmod 0 dir13/a &&\n> +\n> +\ttest-tool dir-iterator ./dir13 >actual-out &&\n> +\ttest_cmp expected-out actual-out &&\n> +\n> +\tchmod 755 dir13/a\n> +'\n> +test_expect_success POSIXPERM,SANITY \\\n> +'pedantic dirs-ignore of dir w/ dir w/o perms w/ file' '\n> +\n> +\tcat >expected-out <<-EOF &&\n> +\tdir_iterator_advance failure: EACCES\n> +\tEOF\n> +\n> +\tchmod 0 dir13/a &&\n> +\n> +\ttest_must_fail test-tool dir-iterator --pedantic ./dir13 >actual-out &&\n> +\ttest_cmp expected-out actual-out &&\n> +\n> +\tchmod 755 dir13/a\n> +'\n> +test_expect_success POSIXPERM,SANITY \\\n> 'dirs-before of dir w/ dir w/o perms w/ file' '\n> \n> \tcat >expected-out <<-EOF &&\n> @@ -343,7 +570,7 @@ test_expect_success POSIXPERM,SANITY \\\n> \n> \tchmod 0 dir13/a &&\n> \n> -\ttest-tool dir-iterator ./dir13 >actual-out &&\n> +\ttest-tool dir-iterator --dirs-before ./dir13 >actual-out &&\n> \ttest_cmp expected-out actual-out &&\n> \n> \tchmod 755 dir13/a\n> @@ -358,7 +585,8 @@ test_expect_success POSIXPERM,SANITY \\\n> \n> \tchmod 0 dir13/a &&\n> \n> -\ttest_must_fail test-tool dir-iterator --pedantic ./dir13 >actual-out &&\n> +\ttest_must_fail test-tool dir-iterator --dirs-before \\\n> +\t\t--pedantic ./dir13 >actual-out &&\n> \ttest_cmp expected-out actual-out &&\n> \n> \tchmod 755 dir13/a\n> @@ -371,6 +599,31 @@ test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/o cycle' '\n> \tln -s d dir14/a/e &&\n> \tln -s ../b dir14/a/f\n> '\n> +test_expect_success SYMLINKS 'dirs-ignore of dir w/ symlinks w/o cycle' '\n> +\tcat >expected-sorted-out <<-EOF &&\n> +\t[f] (a/d) [d] ./dir14/a/d\n> +\t[s] (a/e) [e] ./dir14/a/e\n> +\t[s] (a/f) [f] ./dir14/a/f\n> +\tEOF\n> +\n> +\ttest-tool dir-iterator ./dir14 >actual-out &&\n> +\tsort actual-out >actual-sorted-out &&\n> +\n> +\ttest_cmp expected-sorted-out actual-sorted-out\n> +'\n> +test_expect_success SYMLINKS \\\n> +'follow-symlinks dirs-ignore of dir w/ symlinks, w/o cycle' '\n> +\n> +\tcat >expected-sorted-out <<-EOF &&\n> +\t[f] (a/d) [d] ./dir14/a/d\n> +\t[f] (a/e) [e] ./dir14/a/e\n> +\tEOF\n> +\n> +\ttest-tool dir-iterator --follow-symlinks ./dir14 >actual-out &&\n> +\tsort actual-out >actual-sorted-out &&\n> +\n> +\ttest_cmp expected-sorted-out actual-sorted-out\n> +'\n> test_expect_success SYMLINKS 'dirs-before of dir w/ symlinks w/o cycle' '\n> \tcat >expected-sorted-out <<-EOF &&\n> \t[d] (a) [a] ./dir14/a\n> @@ -381,7 +634,7 @@ test_expect_success SYMLINKS 'dirs-before of dir w/ symlinks w/o cycle' '\n> \t[s] (a/f) [f] ./dir14/a/f\n> \tEOF\n> \n> -\ttest-tool dir-iterator ./dir14 >actual-out &&\n> +\ttest-tool dir-iterator --dirs-before ./dir14 >actual-out &&\n> \tsort actual-out >actual-sorted-out &&\n> \n> \ttest_cmp expected-sorted-out actual-sorted-out\n> @@ -399,7 +652,7 @@ test_expect_success SYMLINKS \\\n> \t[f] (a/e) [e] ./dir14/a/e\n> \tEOF\n> \n> -\ttest-tool dir-iterator --follow-symlinks ./dir14 >actual-out &&\n> +\ttest-tool dir-iterator --dirs-before --follow-symlinks ./dir14 >actual-out &&\n> \tsort actual-out >actual-sorted-out &&\n> \n> \ttest_cmp expected-sorted-out actual-sorted-out\n> @@ -412,7 +665,33 @@ test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/ cycle' '\n> \tln -s ../ dir15/a/b/e &&\n> \tln -s ../../ dir15/a/b/f\n> '\n> +test_expect_success SYMLINKS 'dirs-ignore of dir w/ symlinks w/ cycle' '\n> +\tcat >expected-sorted-out <<-EOF &&\n> +\t[s] (a/b/d) [d] ./dir15/a/b/d\n> +\t[s] (a/b/e) [e] ./dir15/a/b/e\n> +\t[s] (a/b/f) [f] ./dir15/a/b/f\n> +\tEOF\n> +\n> +\ttest-tool dir-iterator ./dir15 >actual-out &&\n> +\tsort actual-out >actual-sorted-out &&\n> +\n> +\ttest_cmp expected-sorted-out actual-sorted-out\n> +'\n> +test_expect_success SYMLINKS \\\n> +'pedantic follow-symlinks dirs-ignore of dir w/ symlinks w/ cycle' '\n> +\n> +\tcat >expected-tailed-out <<-EOF &&\n> +\tdir_iterator_advance failure: ELOOP\n> +\tEOF\n> +\n> +\ttest_must_fail test-tool dir-iterator \\\n> +\t\t--pedantic --follow-symlinks ./dir15 >actual-out &&\n> +\ttail -n 1 actual-out >actual-tailed-out &&\n> +\n> +\ttest_cmp expected-tailed-out actual-tailed-out\n> +'\n> test_expect_success SYMLINKS 'dirs-before of dir w/ symlinks w/ cycle' '\n> +\n> \tcat >expected-sorted-out <<-EOF &&\n> \t[d] (a) [a] ./dir15/a\n> \t[d] (a/b) [b] ./dir15/a/b\n> @@ -422,7 +701,7 @@ test_expect_success SYMLINKS 'dirs-before of dir w/ symlinks w/ cycle' '\n> \t[s] (a/b/f) [f] ./dir15/a/b/f\n> \tEOF\n> \n> -\ttest-tool dir-iterator ./dir15 >actual-out &&\n> +\ttest-tool dir-iterator --dirs-before ./dir15 >actual-out &&\n> \tsort actual-out >actual-sorted-out &&\n> \n> \ttest_cmp expected-sorted-out actual-sorted-out\n> @@ -434,7 +713,7 @@ test_expect_success SYMLINKS \\\n> \tdir_iterator_advance failure: ELOOP\n> \tEOF\n> \n> -\ttest_must_fail test-tool dir-iterator \\\n> +\ttest_must_fail test-tool dir-iterator --dirs-before \\\n> \t\t--pedantic --follow-symlinks ./dir15 >actual-out &&\n> \ttail -n 1 actual-out >actual-tailed-out &&\n> \n\n"},{"id":"455079","messageId":"091db194-2e6e-8c15-2e84-5f873fd08b55@gmail.com","threadId":"57864","inReplyTo":"20220509175159.2948802-16-kioplato@gmail.com","subject":"Re: [PATCH v2 15/15] entry.c: use dir-iterator to avoid explicit dir traversal","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-05-10T13:10:07Z","receivedAt":"2022-05-10T13:10:13Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Plato\n\nOn 09/05/2022 18:51, Plato Kiorpelidis wrote:\n> Replace usage of opendir/readdir/closedir API to traverse directories\n> recursively, at remove_subtree() function, by the dir-iterator API. This\n> simplifies the code and avoids recursive calls to remove_subtree().\n\nThanks for adding this conversion, it is nice to see the post-order \ntraversal being used and the end result is an improvement on the original.\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Plato Kiorpelidis \n> ---\n> entry.c | 38 +++++++++++++++++++-------------------\n> 1 file changed, 19 insertions(+), 19 deletions(-)\n> \n> diff --git a/entry.c b/entry.c\n> index 1c9df62b30..46ef145f97 100644\n> --- a/entry.c\n> +++ b/entry.c\n> @@ -8,6 +8,8 @@\n> #include \"fsmonitor.h\"\n> #include \"entry.h\"\n> #include \"parallel-checkout.h\"\n> +#include \"iterator.h\"\n> +#include \"dir-iterator.h\"\n> \n> static void create_directories(const char *path, int path_len,\n> \t\t\t const struct checkout *state)\n> @@ -52,26 +54,24 @@ static void create_directories(const char *path, int path_len,\n> \n> static void remove_subtree(struct strbuf *path)\n> {\n> -\tDIR *dir = opendir(path->buf);\n> -\tstruct dirent *de;\n> -\tint origlen = path->len;\n> -\n> -\tif (!dir)\n> -\t\tdie_errno(\"cannot opendir '%s'\", path->buf);\n> -\twhile ((de = readdir_skip_dot_and_dotdot(dir)) != NULL) {\n> -\t\tstruct stat st;\n> -\n> -\t\tstrbuf_addch(path, '/');\n> -\t\tstrbuf_addstr(path, de->d_name);\n> -\t\tif (lstat(path->buf, &st))\n> -\t\t\tdie_errno(\"cannot lstat '%s'\", path->buf);\n> -\t\tif (S_ISDIR(st.st_mode))\n> -\t\t\tremove_subtree(path);\n> -\t\telse if (unlink(path->buf))\n> -\t\t\tdie_errno(\"cannot unlink '%s'\", path->buf);\n> -\t\tstrbuf_setlen(path, origlen);\n> +\tunsigned int flags = DIR_ITERATOR_DIRS_AFTER;\n> +\tstruct dir_iterator *iter = NULL;\n> +\tint ok;\n> +\n> +\tif (!(iter = dir_iterator_begin(path->buf, flags)))\n> +\t\tdie_errno(\"cannot open directory '%s'\", path->buf);\n> +\n> +\twhile ((ok = dir_iterator_advance(iter)) == ITER_OK) {\n> +\t\tif (S_ISDIR(iter->st.st_mode) && rmdir(iter->path.buf))\n> +\t\t\tdie_errno(\"cannot rmdir '%s'\", iter->path.buf);\n> +\n> +\t\tif (!S_ISDIR(iter->st.st_mode) && unlink(iter->path.buf))\n> +\t\t\tdie_errno(\"cannot unlink '%s'\", iter->path.buf);\n> \t}\n> -\tclosedir(dir);\n> +\n> +\tif (ok != ITER_DONE)\n> +\t\tdie_errno(\"failed to iterate over directory '%s'\", path->buf);\n> +\n> \tif (rmdir(path->buf))\n> \t\tdie_errno(\"cannot rmdir '%s'\", path->buf);\n> }\n\n"},{"id":"455080","messageId":"f6a5f7b5-a4e6-6e64-6e2e-e089618fbd80@gmail.com","threadId":"57864","inReplyTo":"20220509175159.2948802-1-kioplato@gmail.com","subject":"Re: [PATCH v2 00/15][GSoC] iterate dirs before or after their contents","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-05-10T13:13:29Z","receivedAt":"2022-05-10T13:18:28Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 09/05/2022 18:51, Plato Kiorpelidis wrote:\n> This is the second version of a patch series which implements pre-order and\n> post-order iteration over directories. In this version I use the new iteration\n> scheme to convert entry.c remove_subtree() function from using opendir/readdir/\n> closedir API to dir-iterator API.\n> \n> v1: https://lore.kernel.org/git/20220410111852.2097418-1-kioplato@gmail.com/\n> \n> Fork: https://github.com/kioplato/git/tree/dir-iterator-v2\n> CI: https://github.com/kioplato/git/actions/runs/2295239114\n> I've ran the full test suite for each commit.\n> \n> In comparison to v1 I changed/fixed/improved:\n> \n> * In this version I followed Phillip's suggestion[1] to explain why I'm making\n> the changes in each commit's description and create smaller, targeted commits.\n> In v1 I did a lot of renaming to directory paths, test descriptions and files.\n> In this version these renames have their own commits.\n> \n> * Explained why I converted dir_iterator_advance() from iterative implementation\n> to recursive in the related commit. Ævar[2] and Phillip[3] asked about why I did\n> this conversion, so I thought it was appropriate to include an explanation in\n> the commit's description.\n> \n> * I talked about a subtle unexpected behavior of dir_iterator_begin() [4]. I was\n> wrong about the two new states for the deepest directory. I introduced those\n> states in order to implement recursive dir_iterator_advance() and not to deal\n> with the unexpected behavior of dir_iterator_begin(). Therefore I splitted the\n> conversion of dir_iteartor_advance() and the changes to deal with the unexpected\n> behavior of dir_iteartor_begin() into two different commits. They need their own\n> commits since they are two unrelated changes.\n> \n> * Split the commit that introduces pre-order and post-order iteration into two.\n> Like Phillip suggested here[5] the first changes dir-iterator default behavior\n> and the second implements post-order iterator over directories. This helps split\n> the tests introduced, which helps generate smaller diffs.\n> \n> * Like Phillip suggested [6], I'll work with the existing code and change one\n> aspect at a time in stages. In this series I converted entry.c remove_subtree()\n> to use dir-iterator API instead of opendir/readdir/closedir API.\n> \n> * The redundant tests [7] in this version were not removed. Instead I kept them\n> for more detailed testing which helps in case a test fails.\n> \n> * Ævar's idea[8] to change the default swich case in test-dir-iterator made me\n> realize that test-dir-iterator does not print errno codes set by failed calls to\n> dir_iterator_advance(). Therefore I made test-dir-iterator print them which\n> makes all tests test_cmp for the errno code set by either dir_iteartor_begin()\n> or dir_iterator_advance(). This presumably has the same effect as changing the\n> default switch case to use BUG() macro. This change also led to two others, one\n> that makes test-dir-iterator explicitly print ELOOP errno and the second that\n> improves readability by consistently returning, C standard constants, either\n> EXIT_SUCCESS or EXIT_FAILURE, instead of mixed integers and exit() calls.\n> \n> * fixed coding style and introduces an enum for returning constants instead of\n> more integers which worsens readability, as suggested by Ævar [9].\n> \n> I also didn't CC Michael and Matheus in this version, since they weren't\n> interested in v1 where I did CC them.\n\nThanks for the detailed change description. I've commented on the \nimplementation but I've not had time to look at the test changes. I'm \nafraid I still don't see the initial refactoring of dir-iterator.c as am \nimprovement. It is nice to see the post order traversal being used in \nremove_subtree(). Just to let you know, I'm going to off the list for \nthe next couple of weeks, I'll try and catch up with this series later.\n\nBest Wishes\n\nPhillip\n\n> [1]: https://lore.kernel.org/git/ed6656e0-a865-319e-0f56-23ab1da3eef3@gmail.com/\n> [2]: https://lore.kernel.org/git/220411.86o817j2dt.gmgdl@evledraar.gmail.com/\n> [3]: https://lore.kernel.org/git/35160d46-d337-2110-f968-238abb7e9f0e@gmail.com/\n> [4]: https://lore.kernel.org/git/20220427154526.uuhpkoee322l7kmz@compass/\n> [5]: https://lore.kernel.org/git/b75aaee8-c037-e8e0-6ee0-729922059352@gmail.com/\n> [6]: https://lore.kernel.org/git/df287d4f-e9da-4ce0-d7e9-1b1fe7671aab@gmail.com/\n> [7]: https://lore.kernel.org/git/220411.86sfqjj2o0.gmgdl@evledraar.gmail.com/\n> [8]: https://lore.kernel.org/git/220411.86wnfvj2q6.gmgdl@evledraar.gmail.com/\n> [9]: https://lore.kernel.org/git/220411.86o817j2dt.gmgdl@evledraar.gmail.com/\n> \n> Plato Kiorpelidis (15):\n> t0066: refactor dir-iterator tests\n> t0066: remove dependency between unrelated tests\n> t0066: shorter expected and actual output file names\n> test-dir-iterator: consistently return EXIT_FAILURE or EXIT_SUCCESS\n> test-dir-iterator: print EACCES and ELOOP errno set by dir_iterator\n> test-dir-iterator: print errno name set by dir_iterator_advance\n> t0066: better test coverage for dir-iterator\n> t0066: reorder tests from simple to more complex\n> t0066: rename test directories\n> dir-iterator: refactor dir_iterator_advance()\n> dir-iterator: open root dir in dir_iterator_begin()\n> t0066: rename subtest descriptions\n> dir-iterator: option to iterate dirs in pre-order\n> dir-iterator: option to iterate dirs in post-order\n> entry.c: use dir-iterator to avoid explicit dir traversal\n> \n> builtin/clone.c | 4 +-\n> dir-iterator.c | 334 ++++++--\n> dir-iterator.h | 36 +-\n> entry.c | 38 +-\n> refs/files-backend.c | 2 +-\n> t/helper/test-dir-iterator.c | 24 +-\n> t/t0066-dir-iterator.sh | 1478 +++++++++++++++++++++++++++++++---\n> 7 files changed, 1704 insertions(+), 212 deletions(-)\n> \n\n"},{"id":"455085","messageId":"xmqqilqd8i33.fsf@gitster.g","threadId":"57864","inReplyTo":"20220509175159.2948802-1-kioplato@gmail.com","subject":"Re: [PATCH v2 00/15][GSoC] iterate dirs before or after their contents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-10T16:31:44Z","receivedAt":"2022-05-10T16:31:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Plato Kiorpelidis writes:\n\n> This is the second version of a patch series which implements pre-order and\n> post-order iteration over directories. In this version I use the new iteration\n> scheme to convert entry.c remove_subtree() function from using opendir/readdir/\n> closedir API to dir-iterator API.\n\nThis cover letter seems to be written specifically for those who\nhave read v1. It is not very advisable way to write your cover\nletter, unless you are aiming to shrink your reviewer base in each\niteration.\n\nDo not assume that everybody who didn't give reviews to the previous\nround is not interested in the topic.\n\nInstead, at least repeat the justification and the motivation enough\nin each iteration to make it easier for those who were not involved\nin earlier rounds to join the discussion.\n\n\"implements pre-order and post-order\" is \"what\" and not \"why\". Even\nin this version, \"convert remove_subtree()\" is only a \"what\" and not\n\"why\". The reviewers need to learn why helping these patches is a\nworthwhile thing to do before they decide to devote their time, so\nhelping them would in return help your cause.\n\nYou highlighted the difference since the previous round very well\n(omitted here). It would be very appreciated by those who were in\nthe first round. Keep up the good work ;-)\n\n> I also didn't CC Michael and Matheus in this version, since they weren't\n> interested in v1 where I did CC them.\n\nThis is OK as long as the message goes to the list, but again, \"no\nresponses\" should not be taken as more than \"no information\".\nSeeing no review comments from somebody on CC: is not a vote of \"no\"\nby the recipient (nor it is not a vote of \"yes\").\n\nThanks for working on the topic.\n\n"},{"id":"455400","messageId":"20220518141321.uynaxzaoivlthi7b@compass","threadId":"57864","inReplyTo":"xmqq35hictaa.fsf@gitster.g","subject":"Re: [PATCH v2 04/15] test-dir-iterator: consistently return EXIT_FAILURE or EXIT_SUCCESS","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-05-18T14:13:21Z","receivedAt":"2022-05-18T14:13:41Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"On 22/05/09 02:03PM, Junio C Hamano wrote:\n> Plato Kiorpelidis writes:\n> \n> > Throughout test-dir-iterator.c we were returning/exiting with either\n> > integers or EXIT_FAILURE. Improve readability and reduce mental load\n> > by being consistent with what test-dir-iterator returns through the\n> > test-tool. Returning mixed constants and integers could indicate that\n> > it matters for some reason e.g. architecture of test-tool and cmd__*\n> > functions.\n> >\n> > EXIT_SUCCESS and EXIT_FAILURE are specified by the C standard.\n> > That makes the code more portable and standardized.\n> \n> And less portable for the invoking process of \"git\". The invoking\n> process used to be able to depend ou getting WEXITSTATUS() from our\n> exit status to obtain \"1\" when we exited with 1; if we start exiting\n> with EXIT_FAILURE, there is no guarantee what non-zero value is used.\n> \n> So, I am not sure if this is a good direction to go in.\n\nFrom what I understand, this is a point about why EXIT_FAILURE and EXIT_SUCCESS\nare a bad idea in Git's case in general; not specifically in test-tool's case.\nThe test-tool doesn't use child processes, therefore it doesn't use the macro\nWEXITSTATUS. As a result, supposedly, we could use EXIT_FAILURE and EXIT_SUCCESS\nconstants in this case. However, we don't want to use them in order to stay\nconsistent throughtout Git's implementation. Is my understanding correct?\n\n> > Signed-off-by: Plato Kiorpelidis \n> > ---\n> > t/helper/test-dir-iterator.c | 6 +++---\n> > 1 file changed, 3 insertions(+), 3 deletions(-)\n> \n> Especially given that this is a helper for testing, we may want to\n> return/exit with different non-zero value at different places to\n> make it easier for the test script to tell where in the program we\n> decided to exit a failure. IOW, if we return not EXIT_FAILURE but 2\n> (or whatever value that is not used elsewhere) in the first hunk,\n> and let the second hunk continue to return 1, then the calling test\n> script can tell which one decided to fail.\n> \n> As to EXIT_SUCCESS, I do not have a strong opinion against it, but\n> because EXIT_FAILURE is a bad idea, we probably should avoid it for\n> consistency.\n\nThis improves upon my attempt to be consistent in what we return, by also giving\nthe option to tell where in the program we exited a failure. I'll adopt this in\nthe next iteration of these series, v3.\n\n> Thanks.\n\nThanks,\nPlato\n"},{"id":"455401","messageId":"20220518153951.ebmcd5nsnwwdrebz@compass","threadId":"57864","inReplyTo":"xmqqr152be4n.fsf@gitster.g","subject":"Re: [PATCH v2 10/15] dir-iterator: refactor dir_iterator_advance()","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-05-18T15:39:51Z","receivedAt":"2022-05-18T15:40:39Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"On 22/05/09 02:16PM, Junio C Hamano wrote:\n> Plato Kiorpelidis writes:\n> \n> > Simplify dir_iterator_advance() by converting from iterative to\n> > recursive implementation. This conversion makes dir-iterator more\n> > maintainable for the following reasons:\n> > * dir-iterator iterates the file-system, which is a tree structure.\n> > Traditionally, tree traversals, in textbooks, lectures and online\n> > sources are implemented recursively and not iteratively. Therefore\n> > it helps reduce mental load for readers, since it's easier to follow\n> > as it reminds of the same tree traversals we use on tree structures.\n> \n> Careful.\n> \n> Many algorithms that are taught in the recursive form in textbooks\n> are turned into iterative in production systems for a reason. To\n> avoid too deep a recursion wasting too much stack space. A loop\n> with management of work items using in-program data structures like\n> stack or queue often makes a large traversal bearable.\n> \n> The most obvious example is our history traversal code. History\n> stored in Git is a tree structure, but no sane reimplementation of\n> Git (well, at least those that want to be able to deal with a\n> history larger than a toy project) would implement \"git log\" using a\n> recursive algorithm.\n\nThat's a good point, I didn't think about that. It's also something that's\nobjective, so it's easier to reach a conclusion. This whole refactoring of\ndir_iterator_advance() is a matter of opinion on what's more readable or not.\n\nIn this case, however, the dir_iterator_advance() is performing tail recursion\nwhich doesn't require stack space. It reuses the stack frame from the current\ncall for the next call. Does this still pose a problem and why? If it does, I've\ngot no problem to work with the existing iterative implementation.\n\nThanks,\nPlato\n"},{"id":"455407","messageId":"20220518173947.4qabalu6mjmzumen@compass","threadId":"57864","inReplyTo":"88421b18-0fa0-236a-b74b-c5ee3ef53279@gmail.com","subject":"Re: [PATCH v2 13/15] dir-iterator: option to iterate dirs in pre-order","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-05-18T17:40:52Z","receivedAt":"2022-05-18T17:41:20Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"On 22/05/10 02:07PM, Phillip Wood wrote:\n> Hi Plato\n> \n> On 09/05/2022 18:51, Plato Kiorpelidis wrote:\n> > Introduce a new option to dir-iterator, using dir_iterator_begin()\n> > flags parameter, allowing to control whether or not directories will\n> > be exposed before their contents. In essence, pre-order traversal over\n> > file system entries that are directories.\n> > \n> > This changes the default behavior of the dir-iterator API. Instead\n> > of iterating directories before doing so over their contents, the new\n> > default behavior does not expose directories at all. Iteration is still\n> > performed, however, within directories, iterating over any other entry.\n> > Only directory paths are ignored.\n> > \n> > To iterate over directories in pre-order, reproducing the previous\n> > default behavior, enable the new flag DIR_ITERATOR_DIRS_BEFORE in the\n> > flags parameter of dir_iterator_begin():\n> > * ignore directories by not setting DIR_ITERATOR_DIRS_BEFORE\n> > * iterate directories pre-order by enabling DIR_ITERATOR_DIRS_BEFORE\n> > \n> > Adjust existing callers, in refs/files-backend.c and builtin/clone.c\n> > to enable DIR_ITERATOR_DIRS_BEFORE since these callers require iteration\n> > over directories before doing so over their contents.\n> > \n> > Update t/t0066-dir-iterator.sh and t/helper/test-dir-iterator.c to test\n> > the new iteration scheme, which is the new default behavior, and the new\n> > flag DIR_ITERATOR_DIRS_BEFORE which reproduces the old default behavior.\n> \n> It's great that you've split this change out from the next patch. I had not\n> realized when I looked at the last round that all the existing callers\n> require pre-order traversal. Given that is the case I'm finding it hard to\n> see how changing the default behavior to one that no caller is using is an\n> improvement.\n\nChanging the default behavior is required to simplify entry.c remove_subtree().\nI would have kept dir-iterator's default iteration scheme as is, but how are we\ngoing to deal with remove_subtree()? remove_subtree() requires iterating dirs\nafter their contents. We need to find a flag encoding that is a good design\nchoice and serves both existing and remove_subtree(), without limiting future\ndir-iterator customers.\n\nThis encoding of flags was heavily discussed in the patch series that my work\nis based on[1], most notably here[2].\n\n[1]: https://lore.kernel.org/git/1493226219-33423-1-git-send-email-bnmvco@gmail.com/\n[2]: https://public-inbox.org/git/1751d788-d1f1-1c97-b33b-f53dab78ef86@alum.mit.edu/\n\nThanks,\nPlato\n"},{"id":"455408","messageId":"016201d86adf$5a4e1eb0$0eea5c10$@nexbridge.com","threadId":"57864","inReplyTo":"20220518173947.4qabalu6mjmzumen@compass","subject":"RE: [PATCH v2 13/15] dir-iterator: option to iterate dirs in pre-order","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2022-05-18T17:47:27Z","receivedAt":"2022-05-18T17:47:42Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On May 18, 2022 1:41 PM, Plato Kiorpelidis wrote:\n>On 22/05/10 02:07PM, Phillip Wood wrote:\n>> Hi Plato\n>>\n>> On 09/05/2022 18:51, Plato Kiorpelidis wrote:\n>> > Introduce a new option to dir-iterator, using dir_iterator_begin()\n>> > flags parameter, allowing to control whether or not directories will\n>> > be exposed before their contents. In essence, pre-order traversal\n>> > over file system entries that are directories.\n>> >\n>> > This changes the default behavior of the dir-iterator API. Instead\n>> > of iterating directories before doing so over their contents, the\n>> > new default behavior does not expose directories at all. Iteration\n>> > is still performed, however, within directories, iterating over any\nother entry.\n>> > Only directory paths are ignored.\n>> >\n>> > To iterate over directories in pre-order, reproducing the previous\n>> > default behavior, enable the new flag DIR_ITERATOR_DIRS_BEFORE in\n>> > the flags parameter of dir_iterator_begin():\n>> > * ignore directories by not setting DIR_ITERATOR_DIRS_BEFORE\n>> > * iterate directories pre-order by enabling\n>> > DIR_ITERATOR_DIRS_BEFORE\n>> >\n>> > Adjust existing callers, in refs/files-backend.c and builtin/clone.c\n>> > to enable DIR_ITERATOR_DIRS_BEFORE since these callers require\n>> > iteration over directories before doing so over their contents.\n>> >\n>> > Update t/t0066-dir-iterator.sh and t/helper/test-dir-iterator.c to\n>> > test the new iteration scheme, which is the new default behavior,\n>> > and the new flag DIR_ITERATOR_DIRS_BEFORE which reproduces the old\n>default behavior.\n>>\n>> It's great that you've split this change out from the next patch. I\n>> had not realized when I looked at the last round that all the existing\n>> callers require pre-order traversal. Given that is the case I'm\n>> finding it hard to see how changing the default behavior to one that\n>> no caller is using is an improvement.\n>\n>Changing the default behavior is required to simplify entry.c\nremove_subtree().\n>I would have kept dir-iterator's default iteration scheme as is, but how\nare we\n>going to deal with remove_subtree()? remove_subtree() requires iterating\ndirs\n>after their contents. We need to find a flag encoding that is a good design\nchoice\n>and serves both existing and remove_subtree(), without limiting future dir-\n>iterator customers.\n>\n>This encoding of flags was heavily discussed in the patch series that my\nwork is\n>based on[1], most notably here[2].\n>\n>[1]: https://lore.kernel.org/git/1493226219-33423-1-git-send-email-\n>bnmvco@gmail.com/\n>[2]: https://public-inbox.org/git/1751d788-d1f1-1c97-b33b-\n>f53dab78ef86@alum.mit.edu/\n\nCould this be a fallback position where nftw() is not available? I am not\nhow broadly nftw() is supported but it appears to do what you are looking\nfor.\n--Randall\n\n"},{"id":"455409","messageId":"xmqq7d6iwwnx.fsf@gitster.g","threadId":"57864","inReplyTo":"20220518141321.uynaxzaoivlthi7b@compass","subject":"Re: [PATCH v2 04/15] test-dir-iterator: consistently return EXIT_FAILURE or EXIT_SUCCESS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-18T17:57:54Z","receivedAt":"2022-05-18T17:58:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Plato Kiorpelidis writes:\n\n>> And less portable for the invoking process of \"git\". The invoking\n>> process used to be able to depend ou getting WEXITSTATUS() from our\n>> exit status to obtain \"1\" when we exited with 1; if we start exiting\n>> with EXIT_FAILURE, there is no guarantee what non-zero value is used.\n>> \n>> So, I am not sure if this is a good direction to go in.\n>\n> From what I understand, this is a point about why EXIT_FAILURE and EXIT_SUCCESS\n> are a bad idea in Git's case in general; not specifically in test-tool's case.\n> The test-tool doesn't use child processes, therefore it doesn't use the macro\n> WEXITSTATUS. As a result, supposedly, we could use EXIT_FAILURE and EXIT_SUCCESS\n> constants in this case. However, we don't want to use them in order to stay\n> consistent throughtout Git's implementation. Is my understanding correct?\n\nYes, it is a bad idea for any tool (not limited to Git) whose\ndocumentation does not say \"on failure, it exits with non-zero exit\nstatus\", but signals what kind of failure with different non-zero\nexit status values. Perhaps the calling test scripts of test-tool\nmay only care about exit status being (or not being) zero, so we\ncould use EXIT_FAILURE vs EXIT_SUCCESS, as long as EXIT_FAILURE does\nnot exit with a value that confuses test_must_fail, but by design,\nthe code that uses EXIT_FAILURE cannot guarantee that---the whole\npoint of EXIT_FAILURE macro is to hide which exact value is used.\nYou are correct to assume that we'd want to avoid using EXIT_FAILURE\nin test-tool because we do not want to tempt people to\ncopy-and-paste.\n\nThanks.\n"},{"id":"455410","messageId":"xmqq35h6ww55.fsf@gitster.g","threadId":"57864","inReplyTo":"016201d86adf$5a4e1eb0$0eea5c10$@nexbridge.com","subject":"Re: [PATCH v2 13/15] dir-iterator: option to iterate dirs in pre-order","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-18T18:09:10Z","receivedAt":"2022-05-18T18:09:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":" writes:\n\n> Could this be a fallback position where nftw() is not available? I am not\n> how broadly nftw() is supported but it appears to do what you are looking\n> for.\n\nYeah, nftw() sounds like a better approach than what we have been\ndoing for the past decade by writing our own iterator.\n\n\n"},{"id":"455411","messageId":"016b01d86ae6$40b2ed80$c218c880$@nexbridge.com","threadId":"57864","inReplyTo":"xmqq35h6ww55.fsf@gitster.g","subject":"RE: [PATCH v2 13/15] dir-iterator: option to iterate dirs in pre-order","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2022-05-18T18:36:50Z","receivedAt":"2022-05-18T18:37:06Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On May 18, 2022 2:09 PM Junio C Hamano wrote:\n> writes:\n>\n>> Could this be a fallback position where nftw() is not available? I am\n>> not how broadly nftw() is supported but it appears to do what you are\n>> looking for.\n>\n>Yeah, nftw() sounds like a better approach than what we have been doing for the\n>past decade by writing our own iterator.\n\nNot to mention being blindingly faster and lower resource usage on some \"exotic\" platforms 😊\n\n"},{"id":"455581","messageId":"20220520174310.2worceevbqkqn6oo@compass","threadId":"57864","inReplyTo":"xmqqilqd8i33.fsf@gitster.g","subject":"Re: [PATCH v2 00/15][GSoC] iterate dirs before or after their contents","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-05-20T17:43:10Z","receivedAt":"2022-05-20T17:43:27Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"On 22/05/10 09:31AM, Junio C Hamano wrote:\n> Plato Kiorpelidis writes:\n> \n> > This is the second version of a patch series which implements pre-order and\n> > post-order iteration over directories. In this version I use the new iteration\n> > scheme to convert entry.c remove_subtree() function from using opendir/readdir/\n> > closedir API to dir-iterator API.\n> \n> This cover letter seems to be written specifically for those who\n> have read v1. It is not very advisable way to write your cover\n> letter, unless you are aiming to shrink your reviewer base in each\n> iteration.\n> \n> Do not assume that everybody who didn't give reviews to the previous\n> round is not interested in the topic.\n> \n> Instead, at least repeat the justification and the motivation enough\n> in each iteration to make it easier for those who were not involved\n> in earlier rounds to join the discussion.\n> \n> \"implements pre-order and post-order\" is \"what\" and not \"why\". Even\n> in this version, \"convert remove_subtree()\" is only a \"what\" and not\n> \"why\". The reviewers need to learn why helping these patches is a\n> worthwhile thing to do before they decide to devote their time, so\n> helping them would in return help your cause.\n> \n> You highlighted the difference since the previous round very well\n> (omitted here). It would be very appreciated by those who were in\n> the first round. Keep up the good work ;-)\n> \n> > I also didn't CC Michael and Matheus in this version, since they weren't\n> > interested in v1 where I did CC them.\n> \n> This is OK as long as the message goes to the list, but again, \"no\n> responses\" should not be taken as more than \"no information\".\n> Seeing no review comments from somebody on CC: is not a vote of \"no\"\n> by the recipient (nor it is not a vote of \"yes\").\n> \n> Thanks for working on the topic.\n\nThat's great advice, thank you. Indeed, I did assume the above. I'll\nfollow this up with a patch on how to write better cover letters in\nthe related section and file of Documentation/. I've seen these patch\nseries[1] and I've read through them in detail. I'll figure out how\nto incorporate such information in them.\n\n[1]: https://lore.kernel.org/git/pull.1226.v3.git.1652399017.gitgitgadget@gmail.com/\n\nThanks,\nPlato\n"}]}