{"thread":{"id":"57706","subject":"[RFC PATCH 0/6][GSoC] iterate dirs before or after their contents","startedAt":"2022-04-10T11:19:29Z","lastAt":"2022-04-27T17:30:28Z","messageCount":22,"participants":["Plato Kiorpelidis","Ævar Arnfjörð Bjarmason","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"453375","messageId":"20220410111852.2097418-1-kioplato@gmail.com","threadId":"57706","inReplyTo":null,"subject":"[RFC PATCH 0/6][GSoC] iterate dirs before or after their contents","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-04-10T11:18:46Z","receivedAt":"2022-04-10T11:19:29Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"Hello. Some time ago I worked on converting opendir/readdir calls to use\ndir-iterator [1], however this cleanup required to iterate directory paths after\ntheir contents. Matheus pointed me out to a previous patch series[2] that\nattempted to implement such functionality in dir-iterator. From it, I mainly\nused Michael's feedback and feature requests and tried to include it in my work.\n\nMy fork: https://github.com/kioplato/git/tree/dir-iterator\nCI: https://github.com/kioplato/git/actions/runs/2141288008\nThere are some memleaks, I'll track them down in v2.\n\nI aim to implement more functionality in dir-iterator, my goal being to simplify\nthe codebase by introducing an abstraction layer for iterating directories.\nI would like to eventually simplify read_directory_recursive(). I wanted to\ncheck in with you to make sure I'm heading in the right direction with what I've\nimplemented.\n  * Are my tests overly exhaustive?\n  * As of now we can't thoroughly test dir-iterator on directories with complex\n  structure since readdir produce dirents with undefined order in a directory.\n  I thought about introducing a tool for generating permutations with stable\n  parts in test-lib. Is there a need to something like this for other tests?\n  Or maybe should I sort each level iterated by dir-iterator inside\n  test-dir-iterator before printing to stdout? In these patches I did enumerate\n  the path permutations for some tests by hand, but that's not viable really.\n  * We also don't test for deleted entries between dir_iterator_advance() calls.\n  * Are my comments too much? Throughout git, .c files don't have many comments,\n  should I remove mine as well? I think they provide better context when reading\n  through the source code.\n\nI do understand that it probably is too early to worry about most of these.\nHowever I wanted to communicate my thoughts and setup for the following\nversions.\n\nWhile I wait for review, I'll implement/fix:\n  * DIR_ITERATOR_RECURSE proposed here[3], but with finer control. Instead of a\n  flag I'll introduce a new integer parameter in dir_iterator_begin(), which\n  will specify the maximum iteration depth.\n    * Supplying 0 will have the same behavior as DIR_ITERATOR_RECURSE i.e. it\n    will iterate over the entries of the root directory.\n    * Supplying -1 will iterate to maximum depth. This is the default right now.\n  * DIR_ITERATOR_DIRS_ONLY proposed here[4]. Enabling this, dir-iterator will\n  show only directories. Failing to enable DIR_ITERATOR_DIRS_BEFORE and/or\n  DIR_ITERATOR_DIRS_AFTER will result in dir_iterator_begin() returning NULL.\n  Is this a good way to encode \"show only directories\" in the flags?\n\nI'll include them along with feedback and suggestions from this version in the\nnext one.\n\nI didn't refactor entry.c to use dir-iterator. It's a good first issue for\nsomeone else to become introduced to the community. I applied my patch[1] and it\ndoes not pass t/, as it used to, because of 15th test in t1092. Should I work on\nentry.c in my next version or leave it alone for a newcomer?\n\nThis serves as my microproject for GSoC. Could my future work on dir-iterator\nand cleanup of read_directory_recursive() and other customers of dir-iterator\nbecome a seperate GSoC project I could undertake?\n\n[1]: https://public-inbox.org/git/20191208180439.19018-1-otalpster@gmail.com/\n[2]: https://public-inbox.org/git/1493226219-33423-1-git-send-email-bnmvco@gmail.com/\n[3]: https://public-inbox.org/git/CACsJy8DBa-oH3i+5P=iVr9NhJwsicZ43DO89WmvpYEQu90RrMw@mail.gmail.com/\n[4]: https://public-inbox.org/git/xmqqmvc265hk.fsf@gitster.mtv.corp.google.com/\n\nPlato Kiorpelidis (6):\n  t0066: improve readablity of dir-iterator tests\n  t0066: better test coverage for dir-iterator\n  dir-iterator: refactor dir_iterator_advance()\n  dir-iterator: iterate dirs before or after their contents\n  t0066: remove redundant tests\n  test-dir-iterator: handle EACCES errno by dir-iterator\n\n builtin/clone.c              |    4 +-\n dir-iterator.c               |  302 ++++++---\n dir-iterator.h               |   34 +-\n refs/files-backend.c         |    2 +-\n t/helper/test-dir-iterator.c |   23 +-\n t/t0066-dir-iterator.sh      | 1202 +++++++++++++++++++++++++++++++---\n 6 files changed, 1371 insertions(+), 196 deletions(-)\n\n-- \n2.35.1\n\n"},{"id":"453376","messageId":"20220410111852.2097418-2-kioplato@gmail.com","threadId":"57706","inReplyTo":"20220410111852.2097418-1-kioplato@gmail.com","subject":"[RFC PATCH 1/6] t0066: improve readablity of dir-iterator tests","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-04-10T11:18:47Z","receivedAt":"2022-04-10T11:20:08Z","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\nSigned-off-by: Plato Kiorpelidis <kioplato@gmail.com>\n---\n t/t0066-dir-iterator.sh | 227 +++++++++++++++++++++-------------------\n 1 file changed, 118 insertions(+), 109 deletions(-)\n\ndiff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\nindex 63a1a45cd3..fb20219487 100755\n--- a/t/t0066-dir-iterator.sh\n+++ b/t/t0066-dir-iterator.sh\n@@ -5,145 +5,154 @@ test_description='Test the dir-iterator functionality'\n TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n-test_expect_success 'setup' '\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-\tmkdir -p dir2/a/b/c/ &&\n-\t>dir2/a/b/c/d\n+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+\n+\tcat >expected-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+\tEOF\n+'\n+test_expect_success 'iteration of dir w/ three nested dirs w/ file' '\n+\ttest-tool dir-iterator ./dir6 >actual-out &&\n+\ttest_cmp expected-out actual-out\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-\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+test_expect_success 'setup -- dir w/ complex structure w/o symlinks' '\n+\tmkdir -p dir11/a/b/c/ &&\n+\t>dir11/b &&\n+\t>dir11/c &&\n+\t>dir11/a/e &&\n+\t>dir11/a/b/c/d &&\n+\tmkdir -p dir11/d/e/d/ &&\n+\t>dir11/d/e/d/a &&\n+\n+\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+test_expect_success 'iteration of dir w/ complex structure w/o symlinks' '\n+\ttest-tool dir-iterator ./dir11 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n \n-\ttest-tool dir-iterator ./dir >out &&\n-\tsort out >./actual-iteration-sorted-output &&\n+\ttest_cmp expected-sorted-out actual-sorted-out\n+'\n \n-\ttest_cmp expected-iteration-sorted-output actual-iteration-sorted-output\n+test_expect_success 'dir_iterator_begin() should fail upon inexistent paths' '\n+\techo \"dir_iterator_begin failure: ENOENT\" >expected-inexistent-path-out &&\n+\n+\ttest_must_fail test-tool dir-iterator ./inexistent-path >actual-out &&\n+\ttest_cmp expected-inexistent-path-out actual-out\n '\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-\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+test_expect_success 'dir_iterator_begin() should fail upon non directory paths' '\n+\t>some-file &&\n \n-\ttest-tool dir-iterator ./dir2 >actual-pre-order-output &&\n \n-\ttest_cmp expected-pre-order-output actual-pre-order-output\n-'\n+\techo \"dir_iterator_begin failure: ENOTDIR\" >expected-non-dir-out &&\n \n-test_expect_success 'begin should fail upon inexistent paths' '\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-\ttest_cmp expected-inexistent-path-output actual-inexistent-path-output\n-'\n+\ttest_must_fail test-tool dir-iterator ./some-file >actual-out &&\n+\ttest_cmp expected-non-dir-out actual-out &&\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-\ttest_cmp expected-non-dir-output actual-non-dir-output\n+\ttest_must_fail test-tool dir-iterator --pedantic ./some-file >actual-out &&\n+\ttest_cmp expected-non-dir-out actual-out\n '\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+test_expect_success POSIXPERM,SANITY \\\n+'dir_iterator_advance() should not fail on errors by default' '\n \n-\tmkdir -p dir3/a &&\n-\t>dir3/a/b &&\n-\tchmod 0 dir3/a &&\n+\tmkdir -p dir13/a &&\n+\t>dir13/a/b &&\n+\tchmod 0 dir13/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-'\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+\tcat >expected-no-permissions-out <<-EOF &&\n+\t[d] (a) [a] ./dir13/a\n \tEOF\n \n-\tmkdir -p dir3/a &&\n-\t>dir3/a/b &&\n-\tchmod 0 dir3/a &&\n+\ttest-tool dir-iterator ./dir13 >actual-out &&\n+\ttest_cmp expected-no-permissions-out actual-out &&\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 dir13/a &&\n+\trm -rf dir13\n '\n \n-test_expect_success SYMLINKS 'setup dirs with symlinks' '\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-'\n+test_expect_success POSIXPERM,SANITY \\\n+'dir_iterator_advance() should fail on errors, w/ pedantic flag' '\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-\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+\tmkdir -p dir13/a &&\n+\t>dir13/a/b &&\n+\tchmod 0 dir13/a &&\n+\n+\n+\tcat >expected-no-permissions-pedantic-out <<-EOF &&\n+\t[d] (a) [a] ./dir13/a\n+\tdir_iterator_advance failure\n \tEOF\n \n-\ttest-tool dir-iterator ./dir4 >out &&\n-\tsort out >actual-no-follow-sorted-output &&\n+\ttest_must_fail test-tool dir-iterator --pedantic ./dir13 >actual-out &&\n+\ttest_cmp expected-no-permissions-pedantic-out actual-out &&\n \n-\ttest_cmp expected-no-follow-sorted-output actual-no-follow-sorted-output\n+\tchmod 755 dir13/a &&\n+\trm -rf dir13\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-\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+test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/o cycle' '\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+\n+\tcat >expected-dont-follow-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+\tcat >expected-follow-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+test_expect_success SYMLINKS \\\n+'dont-follow-symlinks of dir w/ symlinks w/o cycle' '\n+\n+\ttest-tool dir-iterator ./dir14 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-dont-follow-sorted-out actual-sorted-out\n+'\n+test_expect_success SYMLINKS \\\n+'follow-symlinks of dir w/ symlinks w/o cycle' '\n \n-\ttest-tool dir-iterator --follow-symlinks ./dir4 >out &&\n-\tsort out >actual-follow-sorted-output &&\n+\ttest-tool dir-iterator --follow-symlinks ./dir14 >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-follow-sorted-out actual-sorted-out\n '\n \n test_done\n-- \n2.35.1\n\n"},{"id":"453377","messageId":"20220410111852.2097418-3-kioplato@gmail.com","threadId":"57706","inReplyTo":"20220410111852.2097418-1-kioplato@gmail.com","subject":"[RFC PATCH 2/6] t0066: better test coverage for dir-iterator","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-04-10T11:18:48Z","receivedAt":"2022-04-10T11:21:57Z","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\nSigned-off-by: Plato Kiorpelidis <kioplato@gmail.com>\n---\n t/t0066-dir-iterator.sh | 291 ++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 291 insertions(+)\n\ndiff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\nindex fb20219487..0a98dd54ba 100755\n--- a/t/t0066-dir-iterator.sh\n+++ b/t/t0066-dir-iterator.sh\n@@ -5,6 +5,94 @@ 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+\n+\tcat >expected-out <<-EOF\n+\t[f] (a) [a] ./dir1/a\n+\tEOF\n+'\n+test_expect_success 'iteration of dir with a file' '\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 dir2/a &&\n+\n+\n+\tcat >expected-out <<-EOF\n+\t[d] (a) [a] ./dir2/a\n+\tEOF\n+'\n+test_expect_success 'iteration of dir with a single dir' '\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 dir3/a &&\n+\tchmod 0 dir3/a &&\n+\n+\n+\tcat >expected-out <<-EOF &&\n+\t[d] (a) [a] ./dir3/a\n+\tEOF\n+\tcat >expected-pedantic-out <<-EOF\n+\t[d] (a) [a] ./dir3/a\n+\tdir_iterator_advance failure\n+\tEOF\n+'\n+test_expect_success POSIXPERM,SANITY 'iteration of dir w/ dir w/o perms' '\n+\ttest-tool dir-iterator ./dir3/ >actual-out &&\n+\ttest_cmp expected-out actual-out\n+'\n+test_expect_success POSIXPERM,SANITY 'pedantic iteration of dir w/ dir w/o perms' '\n+\ttest_must_fail test-tool dir-iterator --pedantic ./dir3/ >actual-out &&\n+\ttest_cmp expected-pedantic-out actual-out\n+'\n+\n+test_expect_success 'setup -- dir w/ five files' '\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+\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+test_expect_success 'iteration of dir w/ five files' '\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 dir5/a &&\n+\t>dir5/a/b &&\n+\n+\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+test_expect_success 'iteration of dir w/ dir w/ a file' '\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 dir6/a/b/c &&\n \t>dir6/a/b/c/d &&\n@@ -22,6 +110,149 @@ test_expect_success 'iteration of dir w/ three nested dirs w/ file' '\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+\tcat >expected-out <<-EOF &&\n+\t[d] (a) [a] ./dir7/a\n+\t[d] (a/b) [b] ./dir7/a/b\n+\tEOF\n+\tcat >expected-pedantic-out <<-EOF\n+\t[d] (a) [a] ./dir7/a\n+\t[d] (a/b) [b] ./dir7/a/b\n+\tdir_iterator_advance failure\n+\tEOF\n+'\n+test_expect_success POSIXPERM,SANITY \\\n+'iteration of dir w/ three nested dirs w/ file, second w/o perms' '\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+\tchmod 0 dir7/a/b &&\n+\n+\ttest_must_fail test-tool dir-iterator --pedantic ./dir7 >actual-out &&\n+\ttest_cmp expected-pedantic-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+\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+test_expect_success 'iteration of dir w/ two dirs each w/ file' '\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+\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+test_expect_success \\\n+'iteration of dir w/ three dirs, one w/ two, one w/ one and one w/ none files' '\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+\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+'\n+test_expect_success 'iteration of dir w/ two nested dirs, each w/ file' '\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 'setup -- dir w/ complex structure w/o symlinks' '\n \tmkdir -p dir11/a/b/c/ &&\n \t>dir11/b &&\n@@ -53,6 +284,28 @@ test_expect_success 'iteration of dir w/ complex structure w/o symlinks' '\n \ttest_cmp expected-sorted-out actual-sorted-out\n '\n \n+test_expect_success POSIXPERM,SANITY \\\n+'dir_iterator_advance() should fail on root dir w/o perms' '\n+\n+\tmkdir -p dir12/a &&\n+\t>dir12/a/b &&\n+\tchmod 0 dir12 &&\n+\n+\n+\tcat >expected-no-permissions-out <<-EOF &&\n+\tdir_iterator_advance failure\n+\tEOF\n+\n+\ttest_must_fail test-tool dir-iterator ./dir12 >actual-out &&\n+\ttest_cmp expected-no-permissions-out actual-out &&\n+\n+\ttest_must_fail test-tool dir-iterator --pedantic ./dir12 >actual-out &&\n+\ttest_cmp expected-no-permissions-out actual-out &&\n+\n+\tchmod 755 dir12 &&\n+\trm -rf dir12\n+'\n+\n test_expect_success 'dir_iterator_begin() should fail upon inexistent paths' '\n \techo \"dir_iterator_begin failure: ENOENT\" >expected-inexistent-path-out &&\n \n@@ -155,4 +408,42 @@ test_expect_success SYMLINKS \\\n \ttest_cmp expected-follow-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+\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+\tcat >expected-dont-follow-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+\tcat >expected-pedantic-follow-tailed-out <<-EOF\n+\tdir_iterator_advance failure\n+\tEOF\n+'\n+test_expect_success SYMLINKS \\\n+'dont-follow-symlinks of dir w/ symlinks w/ cycle' '\n+\n+\ttest-tool dir-iterator ./dir15 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-dont-follow-sorted-out actual-sorted-out\n+'\n+test_expect_success SYMLINKS \\\n+'pedantic follow-symlinks of dir w/ symlinks w/ cycle' '\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-pedantic-follow-tailed-out actual-tailed-out\n+'\n+\n test_done\n-- \n2.35.1\n\n"},{"id":"453378","messageId":"20220410111852.2097418-4-kioplato@gmail.com","threadId":"57706","inReplyTo":"20220410111852.2097418-1-kioplato@gmail.com","subject":"[RFC PATCH 3/6] dir-iterator: refactor dir_iterator_advance()","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-04-10T11:18:49Z","receivedAt":"2022-04-10T11:22:05Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"Simplify dir_iterator_advance by switch from iterative to recursive\nimplementation. In each recursive step one action is performed.\n\nThis makes dir-iterator easier to work with, understand and introduce\nnew functionality.\n\nSigned-off-by: Plato Kiorpelidis <kioplato@gmail.com>\n---\n dir-iterator.c          | 225 ++++++++++++++++++++++++++--------------\n t/t0066-dir-iterator.sh |   4 +-\n 2 files changed, 148 insertions(+), 81 deletions(-)\n\ndiff --git a/dir-iterator.c b/dir-iterator.c\nindex b17e9f970a..0f9ed95958 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@@ -45,34 +45,53 @@ struct dir_iterator_int {\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)) return 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.\n+ *\n+ * Return 1 on success.\n+ * Return -1 on failure when errno == ENOENT, leaving the stack unchanged.\n+ * Return -2 on failure when errno != ENOENT, leaving the stack unchanged.\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 1;\n+\n+\tif ((level->dir = opendir(iter->base.path.buf)) != NULL)\n+\t\treturn 1;\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 -2;\n \t}\n-\n-\treturn 0;\n+\terrno = saved_errno;\n+\treturn -1;\n }\n \n /*\n@@ -81,12 +100,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 +111,121 @@ 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+ * 1 on successful exposure of the provided entry.\n+ * -1 on failed exposure because entry does not exist.\n+ * -2 on failed exposure because of error 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 -2;  // Stat failed not with ENOENT.\n+\t} else if (stat_err && errno == ENOENT)\n+\t\treturn -1;  // Stat failed with ENOENT.\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 1;\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-\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\t\tgoto error_out;\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+\n+\tstruct dirent *dir_entry = NULL;\n+\n+\tint expose_err, activate_err;\n+\n+\t/* For shorter code width-wise, more readable */\n+\tunsigned int PEDANTIC = iter->flags & DIR_ITERATOR_PEDANTIC;\n+\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 == -2 && PEDANTIC)\n+\t\tgoto error_out;\n+\telse if (activate_err == -2 || activate_err == -1) {\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\t--iter->levels_nr;\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-\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+\tif (dir_entry == NULL) {\n+\t\tif (errno) {\n+\t\t\twarning_errno(\"errno reading dir '%s'\", iter->base.path.buf);\n+\t\t\tif (iter->flags & DIR_ITERATOR_PEDANTIC) goto error_out;\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 == -2 && PEDANTIC)\n+\t\tgoto error_out;\n+\n+\tif (expose_err == 1)\n+\t\tpush_level(iter);\n+\n+\tif (expose_err == -1)\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 +263,7 @@ 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+\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 +283,16 @@ 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) != 1) {\n+\t\tsaved_errno = ENOTDIR;\n+\t\tgoto error_out;\n+\t}\n+\n+\tif (activate_level(iter) != 1) {\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 0a98dd54ba..2437ab21c4 100755\n--- a/t/t0066-dir-iterator.sh\n+++ b/t/t0066-dir-iterator.sh\n@@ -285,7 +285,7 @@ test_expect_success 'iteration of dir w/ complex structure w/o symlinks' '\n '\n \n test_expect_success POSIXPERM,SANITY \\\n-'dir_iterator_advance() should fail on root dir w/o perms' '\n+'dir_iterator_begin() should fail on root dir w/o perms' '\n \n \tmkdir -p dir12/a &&\n \t>dir12/a/b &&\n@@ -293,7 +293,7 @@ test_expect_success POSIXPERM,SANITY \\\n \n \n \tcat >expected-no-permissions-out <<-EOF &&\n-\tdir_iterator_advance failure\n+\tdir_iterator_begin failure: ESOMETHINGELSE\n \tEOF\n \n \ttest_must_fail test-tool dir-iterator ./dir12 >actual-out &&\n-- \n2.35.1\n\n"},{"id":"453379","messageId":"20220410111852.2097418-6-kioplato@gmail.com","threadId":"57706","inReplyTo":"20220410111852.2097418-1-kioplato@gmail.com","subject":"[RFC PATCH 5/6] t0066: remove redundant tests","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-04-10T11:18:51Z","receivedAt":"2022-04-10T11:22:14Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"Remove dir-iterator redundant tests since the new tests introduced in\n045738486f (t0066: better test coverage for dir-iterator, 2022-04-07)\nsupersede them.\n\nSigned-off-by: Plato Kiorpelidis <kioplato@gmail.com>\n---\n t/t0066-dir-iterator.sh | 40 ----------------------------------------\n 1 file changed, 40 deletions(-)\n\ndiff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\nindex 24a4f1afef..974bb13092 100755\n--- a/t/t0066-dir-iterator.sh\n+++ b/t/t0066-dir-iterator.sh\n@@ -894,46 +894,6 @@ test_expect_success 'dir_iterator_begin() should fail upon non directory paths'\n \ttest_cmp expected-non-dir-out actual-out\n '\n \n-test_expect_success POSIXPERM,SANITY \\\n-'dir_iterator_advance() should not fail on errors by default' '\n-\n-\tmkdir -p dir13/a &&\n-\t>dir13/a/b &&\n-\tchmod 0 dir13/a &&\n-\n-\n-\tcat >expected-no-permissions-out <<-EOF &&\n-\t[d] (a) [a] ./dir13/a\n-\tEOF\n-\n-\ttest-tool dir-iterator --dirs-before ./dir13 >actual-out &&\n-\ttest_cmp expected-no-permissions-out actual-out &&\n-\n-\tchmod 755 dir13/a &&\n-\trm -rf dir13\n-'\n-\n-test_expect_success POSIXPERM,SANITY \\\n-'dir_iterator_advance() should fail on errors, w/ pedantic flag' '\n-\n-\tmkdir -p dir13/a &&\n-\t>dir13/a/b &&\n-\tchmod 0 dir13/a &&\n-\n-\n-\tcat >expected-no-permissions-pedantic-out <<-EOF &&\n-\t[d] (a) [a] ./dir13/a\n-\tdir_iterator_advance failure\n-\tEOF\n-\n-\ttest_must_fail test-tool dir-iterator --dirs-before \\\n-\t\t--pedantic ./dir13 >actual-out &&\n-\ttest_cmp expected-no-permissions-pedantic-out actual-out &&\n-\n-\tchmod 755 dir13/a &&\n-\trm -rf dir13\n-'\n-\n test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/o cycle' '\n \tmkdir -p dir14/a &&\n \tmkdir -p dir14/b/c &&\n-- \n2.35.1\n\n"},{"id":"453381","messageId":"20220410111852.2097418-5-kioplato@gmail.com","threadId":"57706","inReplyTo":"20220410111852.2097418-1-kioplato@gmail.com","subject":"[RFC PATCH 4/6] dir-iterator: iterate dirs before or after their contents","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-04-10T11:18:50Z","receivedAt":"2022-04-10T11:22:16Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"Introduce a new feature to dir-iterator, using dir_iterator_begin()\nflags parameter, allowing to control whether or not directories will\nappear before or after their contents.\n\nThe new default behavior (i.e. flags set to 0) does not iterate over\ndirectories. Flag DIR_ITERATOR_DIRS_BEFORE iterates over a directory\nbefore doing so over its contents. Flag DIR_ITERATOR_DIRS_AFTER\niterates over a directory after doing so over its contents. These flags\ndo not conflict with each other and may be used simultaneously:\n  * ignore directories\n  (DIR_ITERATOR_DIRS_BEFORE and DIR_ITERATOR_DIRS_AFTER are both false)\n  * expose them before opening and after exhausting them\n  (DIR_ITERATOR_DIRS_BEFORE and DIR_ITERATOR_DIRS_AFTER are both true).\n\nAmend a call to dir_iterator_begin() in refs/files-backend.c and\nbuiltin/clone.c to enable DIR_ITERATOR_DIRS_BEFORE, since they need\niteration over a directory before doing so over its contents.\n\nUpdate t/t0066-dir-iterator.sh and t/helper/test-dir-iterator.c to test\nthe new flags DIR_ITERATOR_DIRS_BEFORE, DIR_ITERATOR_DIRS_AFTER.\n\nSigned-off-by: Plato Kiorpelidis <kioplato@gmail.com>\n---\n builtin/clone.c              |   4 +-\n dir-iterator.c               | 101 ++++-\n dir-iterator.h               |  34 +-\n refs/files-backend.c         |   2 +-\n t/helper/test-dir-iterator.c |  15 +-\n t/t0066-dir-iterator.sh      | 844 ++++++++++++++++++++++++++++++++---\n 6 files changed, 923 insertions(+), 77 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 5231656379..87ee764ba3 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 0f9ed95958..41a1481bb4 100644\n--- a/dir-iterator.c\n+++ b/dir-iterator.c\n@@ -116,16 +116,26 @@ static int pop_level(struct dir_iterator_int *iter)\n  *\n  * Return values:\n  * 1 on successful exposure of the provided entry.\n+ * 0 on failed exposure because entry is directory and flags don't allow it.\n  * -1 on failed exposure because entry does not exist.\n  * -2 on failed exposure because of error 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 \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@@ -137,6 +147,11 @@ static int expose_entry(struct dir_iterator_int *iter, char *d_name)\n \t} else if (stat_err && errno == ENOENT)\n \t\treturn -1;  // Stat failed with ENOENT.\n \n+\tif (S_ISDIR(iter->base.st.st_mode)) {\n+\t\tif (!DIRS_BEFORE && !strcmp(dir_state, \"before\")) return 0;\n+\t\tif (!DIRS_AFTER && !strcmp(dir_state, \"after\")) return 0;\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@@ -150,6 +165,21 @@ static int expose_entry(struct dir_iterator_int *iter, char *d_name)\n \treturn 1;\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, '/') + 1;\n+\tchar *basename = calloc(1, strlen(start) + 1);\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@@ -180,9 +210,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 == -2 && PEDANTIC)\n+\t\t\tgoto error_out;\n+\t\telse if (expose_err == -1 || expose_err == 0)\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@@ -198,12 +247,41 @@ 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\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\tif (pop_level(iter) == 0) {\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 == -2 && PEDANTIC)\n+\t\t\t\tgoto error_out;\n+\t\t\telse if (expose_err == -1 || expose_err == 0)\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 \n@@ -212,17 +290,26 @@ 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 == -2 && PEDANTIC)\n \t\tgoto error_out;\n \n-\tif (expose_err == 1)\n+\tif (expose_err == 0 || expose_err == 1)\n \t\tpush_level(iter);\n \n-\tif (expose_err == -1)\n+\tif (expose_err == 0 || expose_err == -1)\n \t\treturn dir_iterator_advance(dir_iterator);\n \n \treturn ITER_OK;\ndiff --git a/dir-iterator.h b/dir-iterator.h\nindex 08229157c6..dea4fc8f3e 100644\n--- a/dir-iterator.h\n+++ b/dir-iterator.h\n@@ -8,19 +8,21 @@\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.\n+ * Note that the 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\n+ * their 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 directories themselves not being exposed, only their contents will be.\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 +63,25 @@\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 before\n+ *   iterating through that directory's contents.\n+ *\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  * 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+#define DIR_ITERATOR_DIRS_AFTER (1 << 3)\n \n struct dir_iterator {\n \t/* The current path: */\n@@ -97,12 +112,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 paths,\n+ *    but will include non-directory directory contents.\n  */\n struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags);\n \n@@ -110,6 +126,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 the dir_iterator 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 +136,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 95acab78ee..951a673e71 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2247,7 +2247,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 659b6bfa81..c92616bd69 100644\n--- a/t/helper/test-dir-iterator.c\n+++ b/t/helper/test-dir-iterator.c\n@@ -15,7 +15,16 @@ 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+ *\t--dirs-after\n+ *\n+ * example:\n+ * test-tool dir-iterator --pedantic --dirs-before --dirs-after ./somedir\n  */\n int cmd__dir_iterator(int argc, const char **argv)\n {\n@@ -28,6 +37,10 @@ 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 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 2437ab21c4..24a4f1afef 100755\n--- a/t/t0066-dir-iterator.sh\n+++ b/t/t0066-dir-iterator.sh\n@@ -14,44 +14,157 @@ test_expect_success 'setup -- dir with a single file' '\n \t[f] (a) [a] ./dir1/a\n \tEOF\n '\n-test_expect_success 'iteration of dir with a file' '\n+test_expect_success 'dirs-ignore 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+\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+\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+\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 \n \n-\tcat >expected-out <<-EOF\n+\tcat >expected-ignore-out <<-EOF &&\n+\tEOF\n+\n+\tcat >expected-before-out <<-EOF &&\n+\t[d] (a) [a] ./dir2/a\n+\tEOF\n+\n+\tcat expected-before-out >expected-after-out &&\n+\n+\tcat >expected-before-after-out <<-EOF\n+\t[d] (a) [a] ./dir2/a\n \t[d] (a) [a] ./dir2/a\n \tEOF\n '\n-test_expect_success 'iteration of dir with a single dir' '\n+test_expect_success 'dirs-ignore of dir with a single dir' '\n \ttest-tool dir-iterator ./dir2 >actual-out &&\n-\ttest_cmp expected-out actual-out\n+\ttest_cmp expected-ignore-out actual-out\n+'\n+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-before-out actual-out\n+'\n+test_expect_success 'dirs-after of dir with a single dir' '\n+\ttest-tool dir-iterator --dirs-after ./dir2 >actual-out &&\n+\ttest_cmp expected-after-out actual-out\n+'\n+test_expect_success 'dirs-before/dirs-after of dir with a single dir' '\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir2 >actual-out &&\n+\ttest_cmp expected-before-after-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-\tchmod 0 dir3/a &&\n \n \n-\tcat >expected-out <<-EOF &&\n+\tcat >expected-ignore-out <<-EOF &&\n+\tEOF\n+\tcat >expected-pedantic-ignore-out <<-EOF &&\n+\tdir_iterator_advance failure\n+\tEOF\n+\n+\tcat >expected-before-out <<-EOF &&\n \t[d] (a) [a] ./dir3/a\n \tEOF\n-\tcat >expected-pedantic-out <<-EOF\n+\tcat >expected-pedantic-before-out <<-EOF &&\n \t[d] (a) [a] ./dir3/a\n \tdir_iterator_advance failure\n \tEOF\n+\n+\tcat expected-before-out >expected-after-out &&\n+\tcat >expected-pedantic-after-out <<-EOF &&\n+\tdir_iterator_advance failure\n+\tEOF\n+\n+\tcat >expected-before-after-out <<-EOF &&\n+\t[d] (a) [a] ./dir3/a\n+\t[d] (a) [a] ./dir3/a\n+\tEOF\n+\tcat expected-pedantic-before-out >expected-pedantic-before-after-out\n '\n-test_expect_success POSIXPERM,SANITY 'iteration of dir w/ dir w/o perms' '\n+test_expect_success POSIXPERM,SANITY 'dirs-ignore of dir w/ dir w/o perms' '\n+\tchmod 0 dir3/a &&\n+\n \ttest-tool dir-iterator ./dir3/ >actual-out &&\n-\ttest_cmp expected-out actual-out\n+\ttest_cmp expected-ignore-out actual-out &&\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-ignore of dir w/ dir w/o perms' '\n+\tchmod 0 dir3/a &&\n+\n \ttest_must_fail test-tool dir-iterator --pedantic ./dir3/ >actual-out &&\n-\ttest_cmp expected-pedantic-out actual-out\n+\ttest_cmp expected-pedantic-ignore-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+\tchmod 0 dir3/a &&\n+\n+\ttest-tool dir-iterator --dirs-before ./dir3/ >actual-out &&\n+\ttest_cmp expected-before-out actual-out &&\n+\n+\tchmod 755 dir3/a\n+'\n+test_expect_success POSIXPERM,SANITY 'pedantic dirs-before of dir w/ dir w/o perms' '\n+\tchmod 0 dir3/a &&\n+\n+\ttest_must_fail test-tool dir-iterator --dirs-before \\\n+\t\t--pedantic ./dir3/ >actual-out &&\n+\ttest_cmp expected-pedantic-before-out actual-out &&\n+\n+\tchmod 755 dir3/a\n+'\n+test_expect_success POSIXPERM,SANITY 'dirs-after of dir w/ dir w/o perms' '\n+\tchmod 0 dir3/a &&\n+\n+\ttest-tool dir-iterator --dirs-after ./dir3/ >actual-out &&\n+\ttest_cmp expected-after-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+\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-pedantic-after-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+\tchmod 0 dir3/a &&\n+\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir3/ >actual-out &&\n+\ttest_cmp expected-before-after-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+\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-pedantic-before-after-out actual-out &&\n+\n+\tchmod 755 dir3/a\n '\n \n test_expect_success 'setup -- dir w/ five files' '\n@@ -71,26 +184,71 @@ test_expect_success 'setup -- dir w/ five files' '\n \t[f] (e) [e] ./dir4/e\n \tEOF\n '\n-test_expect_success 'iteration of dir w/ five files' '\n+test_expect_success 'dirs-ignore of dir w/ five files' '\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+test_expect_success 'dirs-before of dir w/ five files' '\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+test_expect_success 'dirs-after of dir w/ five files' '\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+\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 \t>dir5/a/b &&\n \n \n-\tcat >expected-out <<-EOF\n+\tcat >expected-ignore-out <<-EOF &&\n+\t[f] (a/b) [b] ./dir5/a/b\n+\tEOF\n+\n+\tcat >expected-before-out <<-EOF &&\n+\t[d] (a) [a] ./dir5/a\n+\t[f] (a/b) [b] ./dir5/a/b\n+\tEOF\n+\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+\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-test_expect_success 'iteration of dir w/ dir w/ a file' '\n+test_expect_success 'dirs-ignore of dir w/ dir w/ a file' '\n \ttest-tool dir-iterator ./dir5 >actual-out &&\n-\ttest_cmp expected-out actual-out\n+\ttest_cmp expected-ignore-out actual-out\n+'\n+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-before-out actual-out\n+'\n+test_expect_success 'dirs-after of dir w/ dir w/ a file' '\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+\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@@ -98,16 +256,49 @@ test_expect_success 'setup -- dir w/ three nested dirs w/ file' '\n \t>dir6/a/b/c/d &&\n \n \n-\tcat >expected-out <<-EOF\n+\tcat >expected-ignore-out <<-EOF &&\n+\t[f] (a/b/c/d) [d] ./dir6/a/b/c/d\n+\tEOF\n+\n+\tcat >expected-before-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 \tEOF\n+\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+\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-test_expect_success 'iteration of dir w/ three nested dirs w/ file' '\n+test_expect_success 'dirs-ignore of dir w/ three nested dirs w/ file' '\n \ttest-tool dir-iterator ./dir6 >actual-out &&\n-\ttest_cmp expected-out actual-out\n+\ttest_cmp expected-ignore-out actual-out\n+'\n+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-before-out actual-out\n+'\n+test_expect_success 'dirs-after of dir w/ three nested dirs w/ file' '\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+\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@@ -116,33 +307,123 @@ test_expect_success POSIXPERM,SANITY \\\n \tmkdir -p dir7/a/b/c &&\n \t>dir7/a/b/c/d &&\n \n-\tcat >expected-out <<-EOF &&\n+\n+\tcat >expected-ignore-out <<-EOF &&\n+\tEOF\n+\tcat >expected-pedantic-ignore-out <<-EOF &&\n+\tdir_iterator_advance failure\n+\tEOF\n+\n+\tcat >expected-before-out <<-EOF &&\n+\t[d] (a) [a] ./dir7/a\n+\t[d] (a/b) [b] ./dir7/a/b\n+\tEOF\n+\tcat >expected-pedantic-before-out <<-EOF &&\n+\t[d] (a) [a] ./dir7/a\n+\t[d] (a/b) [b] ./dir7/a/b\n+\tdir_iterator_advance failure\n+\tEOF\n+\n+\tcat >expected-after-out <<-EOF &&\n+\t[d] (a/b) [b] ./dir7/a/b\n+\t[d] (a) [a] ./dir7/a\n+\tEOF\n+\tcat >expected-pedantic-after-out <<-EOF &&\n+\tdir_iterator_advance failure\n+\tEOF\n+\n+\tcat >expected-before-after-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-\tcat >expected-pedantic-out <<-EOF\n+\tcat >expected-pedantic-before-after-out <<-EOF\n \t[d] (a) [a] ./dir7/a\n \t[d] (a/b) [b] ./dir7/a/b\n \tdir_iterator_advance failure\n \tEOF\n '\n test_expect_success POSIXPERM,SANITY \\\n-'iteration of dir w/ three nested dirs w/ file, second w/o perms' '\n+'dirs-ignore of dir w/ three nested dirs w/ file, second w/o perms' '\n \n \tchmod 0 dir7/a/b &&\n \n \ttest-tool dir-iterator ./dir7 >actual-out &&\n-\ttest_cmp expected-out actual-out &&\n+\ttest_cmp expected-ignore-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+'pedantic dirs-ignore of dir w/ three nested dirs w/ file, second w/o perms' '\n \n \tchmod 0 dir7/a/b &&\n \n \ttest_must_fail test-tool dir-iterator --pedantic ./dir7 >actual-out &&\n-\ttest_cmp expected-pedantic-out actual-out &&\n+\ttest_cmp expected-pedantic-ignore-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+\tchmod 0 dir7/a/b &&\n+\n+\ttest-tool dir-iterator --dirs-before ./dir7 >actual-out &&\n+\ttest_cmp expected-before-out actual-out &&\n+\n+\tchmod 755 dir7/a/b\n+'\n+test_expect_success POSIXPERM,SANITY \\\n+'pedantic dirs-before of dir w/ three nested dirs w/ file, second w/o perms' '\n+\n+\tchmod 0 dir7/a/b &&\n+\n+\ttest_must_fail test-tool dir-iterator --dirs-before \\\n+\t\t--pedantic ./dir7 >actual-out &&\n+\ttest_cmp expected-pedantic-before-out actual-out &&\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+\tchmod 0 dir7/a/b &&\n+\n+\ttest-tool dir-iterator --dirs-after ./dir7 >actual-out &&\n+\ttest_cmp expected-after-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+\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-pedantic-after-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+\tchmod 0 dir7/a/b &&\n+\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir7 >actual-out &&\n+\ttest_cmp expected-before-after-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+\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-pedantic-before-after-out actual-out &&\n \n \tchmod 755 dir7/a/b\n '\n@@ -154,24 +435,84 @@ test_expect_success 'setup -- dir w/ two dirs each w/ file' '\n \t>dir8/c/d &&\n \n \n-\tcat >expected-out1 <<-EOF &&\n+\tcat >expected-ignore-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-ignore-out2 <<-EOF &&\n+\t[f] (c/d) [d] ./dir8/c/d\n+\t[f] (a/b) [b] ./dir8/a/b\n+\tEOF\n+\n+\tcat >expected-before-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+\tcat >expected-before-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+\tcat >expected-after-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-after-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+\tcat >expected-before-after-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-before-after-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-test_expect_success 'iteration of dir w/ two dirs each w/ file' '\n+test_expect_success 'dirs-ignore of dir w/ two dirs each w/ file' '\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\ttest_cmp expected-ignore-out1 actual-out ||\n+\t\ttest_cmp expected-ignore-out2 actual-out\n+\t)\n+'\n+test_expect_success 'dirs-before of dir w/ two dirs each w/ file' '\n+\ttest-tool dir-iterator --dirs-before ./dir8 >actual-out &&\n+\t(\n+\t\ttest_cmp expected-before-out1 actual-out ||\n+\t\ttest_cmp expected-before-out2 actual-out\n+\t)\n+'\n+test_expect_success 'dirs-after of dir w/ two dirs each w/ file' '\n+\ttest-tool dir-iterator --dirs-after ./dir8 >actual-out &&\n+\t(\n+\t\ttest_cmp expected-after-out1 actual-out ||\n+\t\ttest_cmp expected-after-out2 actual-out\n+\t)\n+'\n+test_expect_success 'dirs-before/dirs-after of dir w/ two dirs each w/ file' '\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir8 >actual-out &&\n+\t(\n+\t\ttest_cmp expected-before-after-out1 actual-out ||\n+\t\ttest_cmp expected-before-after-out2 actual-out\n \t)\n '\n \n@@ -183,44 +524,164 @@ test_expect_success 'setup -- dir w/ two dirs, one w/ two and one w/ one files'\n \t>dir9/d/e &&\n \n \n-\tcat >expected-out1 <<-EOF &&\n+\tcat >expected-ignore-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-ignore-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-ignore-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-ignore-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+\tcat >expected-before-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+\tcat >expected-before-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+\tcat >expected-before-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+\tcat >expected-before-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+\tcat >expected-after-out1 <<-EOF &&\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-after-out2 <<-EOF &&\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+\tcat >expected-after-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-after-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+\tcat >expected-before-after-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-before-after-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-before-after-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-before-after-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 test_expect_success \\\n-'iteration of dir w/ three dirs, one w/ two, one w/ one and one w/ none files' '\n+'dirs-ignore of dir w/ three dirs, one w/ two, one w/ one and one w/ none files' '\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\ttest_cmp expected-ignore-out1 actual-out ||\n+\t\ttest_cmp expected-ignore-out2 actual-out ||\n+\t\ttest_cmp expected-ignore-out3 actual-out ||\n+\t\ttest_cmp expected-ignore-out4 actual-out\n+\t)\n+'\n+test_expect_success \\\n+'dirs-before of dir w/ three dirs, one w/ two, one w/ one and one w/ none files' '\n+\n+\ttest-tool dir-iterator --dirs-before ./dir9 >actual-out &&\n+\t(\n+\t\ttest_cmp expected-before-out1 actual-out ||\n+\t\ttest_cmp expected-before-out2 actual-out ||\n+\t\ttest_cmp expected-before-out3 actual-out ||\n+\t\ttest_cmp expected-before-out4 actual-out\n+\t)\n+'\n+test_expect_success \\\n+'dirs-after of dir w/ three dirs, one w/ two, one w/ one and one w/ none files' '\n+\n+\ttest-tool dir-iterator --dirs-after ./dir9 >actual-out &&\n+\t(\n+\t\ttest_cmp expected-after-out1 actual-out ||\n+\t\ttest_cmp expected-after-out2 actual-out ||\n+\t\ttest_cmp expected-after-out3 actual-out ||\n+\t\ttest_cmp expected-after-out4 actual-out\n+\t)\n+'\n+test_expect_success \\\n+'dirs-before/dirs-after of dir w/ three dirs, one w/ two, one w/ one and one w/ none files' '\n+\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir9 >actual-out &&\n+\t(\n+\t\ttest_cmp expected-before-after-out1 actual-out ||\n+\t\ttest_cmp expected-before-after-out2 actual-out ||\n+\t\ttest_cmp expected-before-after-out3 actual-out ||\n+\t\ttest_cmp expected-before-after-out4 actual-out\n \t)\n '\n \n@@ -231,25 +692,84 @@ test_expect_success 'setup -- dir w/ two nested dirs, each w/ file' '\n \t>dir10/a/c/d &&\n \n \n-\tcat >expected-out1 <<-EOF &&\n+\tcat >expected-ignore-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-ignore-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+\tcat >expected-before-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+\tcat >expected-before-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+\tcat >expected-after-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-after-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+\tcat >expected-before-after-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-before-after-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-test_expect_success 'iteration of dir w/ two nested dirs, each w/ file' '\n+test_expect_success 'dirs-ignore of dir w/ two nested dirs, each w/ file' '\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\ttest_cmp expected-ignore-out1 actual-out ||\n+\t\ttest_cmp expected-ignore-out2 actual-out\n+\t)\n+'\n+test_expect_success 'dirs-before of dir w/ two nested dirs, each w/ file' '\n+\ttest-tool dir-iterator --dirs-before ./dir10/ >actual-out &&\n+\t(\n+\t\ttest_cmp expected-before-out1 actual-out ||\n+\t\ttest_cmp expected-before-out2 actual-out\n+\t)\n+'\n+test_expect_success 'dirs-after of dir w/ two nested dirs, each w/ file' '\n+\ttest-tool dir-iterator --dirs-after ./dir10/ >actual-out &&\n+\t(\n+\t\ttest_cmp expected-after-out1 actual-out ||\n+\t\ttest_cmp expected-after-out2 actual-out\n+\t)\n+'\n+test_expect_success 'dirs-before/dirs-after of dir w/ two nested dirs, each w/ file' '\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir10 >actual-out &&\n+\t(\n+\t\ttest_cmp expected-before-after-out1 actual-out ||\n+\t\ttest_cmp expected-before-after-out2 actual-out\n \t)\n '\n \n@@ -263,7 +783,15 @@ test_expect_success 'setup -- dir w/ complex structure w/o symlinks' '\n \t>dir11/d/e/d/a &&\n \n \n-\tcat >expected-sorted-out <<-EOF\n+\tcat >expected-ignore-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+\tcat >expected-before-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@@ -276,12 +804,52 @@ test_expect_success 'setup -- dir w/ complex structure w/o symlinks' '\n \t[f] (c) [c] ./dir11/c\n \t[f] (d/e/d/a) [a] ./dir11/d/e/d/a\n \tEOF\n+\n+\tcat expected-before-sorted-out >expected-after-sorted-out &&\n+\n+\tcat >expected-before-after-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-test_expect_success 'iteration of dir w/ complex structure w/o symlinks' '\n+test_expect_success 'dirs-ignore of dir w/ complex structure w/o symlinks' '\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+\ttest_cmp expected-ignore-sorted-out actual-sorted-out\n+'\n+test_expect_success 'dirs-before of dir w/ complex structure w/o symlinks' '\n+\ttest-tool dir-iterator --dirs-before ./dir11 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-before-sorted-out actual-sorted-out\n+'\n+test_expect_success 'dirs-after of dir w/ complex structure w/o symlinks' '\n+\ttest-tool dir-iterator --dirs-after ./dir11 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-after-sorted-out actual-sorted-out\n+'\n+test_expect_success 'dirs-before/dirs-after of dir w/ complex structure w/o symlinks' '\n+\ttest-tool dir-iterator --dirs-before --dirs-after ./dir11 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-before-after-sorted-out actual-sorted-out\n '\n \n test_expect_success POSIXPERM,SANITY \\\n@@ -338,7 +906,7 @@ test_expect_success POSIXPERM,SANITY \\\n \t[d] (a) [a] ./dir13/a\n \tEOF\n \n-\ttest-tool dir-iterator ./dir13 >actual-out &&\n+\ttest-tool dir-iterator --dirs-before ./dir13 >actual-out &&\n \ttest_cmp expected-no-permissions-out actual-out &&\n \n \tchmod 755 dir13/a &&\n@@ -358,7 +926,8 @@ test_expect_success POSIXPERM,SANITY \\\n \tdir_iterator_advance failure\n \tEOF\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-no-permissions-pedantic-out actual-out &&\n \n \tchmod 755 dir13/a &&\n@@ -373,7 +942,17 @@ test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/o cycle' '\n \tln -s ../b dir14/a/f &&\n \n \n-\tcat >expected-dont-follow-sorted-out <<-EOF &&\n+\tcat >expected-dont-follow-ignore-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+\tcat >expected-follow-ignore-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+\tcat >expected-dont-follow-before-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@@ -381,7 +960,7 @@ test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/o cycle' '\n \t[s] (a/e) [e] ./dir14/a/e\n \t[s] (a/f) [f] ./dir14/a/f\n \tEOF\n-\tcat >expected-follow-sorted-out <<-EOF\n+\tcat >expected-follow-before-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@@ -390,22 +969,99 @@ test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/o cycle' '\n \t[f] (a/d) [d] ./dir14/a/d\n \t[f] (a/e) [e] ./dir14/a/e\n \tEOF\n+\n+\tcat expected-dont-follow-before-sorted-out >expected-dont-follow-after-sorted-out &&\n+\tcat expected-follow-before-sorted-out >expected-follow-after-sorted-out &&\n+\n+\tcat >expected-dont-follow-before-after-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+\tcat >expected-follow-before-after-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 test_expect_success SYMLINKS \\\n-'dont-follow-symlinks of dir w/ symlinks w/o cycle' '\n+'dont-follow-symlinks dirs-ignore of dir w/ symlinks w/o cycle' '\n \n \ttest-tool dir-iterator ./dir14 >actual-out &&\n \tsort actual-out >actual-sorted-out &&\n \n-\ttest_cmp expected-dont-follow-sorted-out actual-sorted-out\n+\ttest_cmp expected-dont-follow-ignore-sorted-out actual-sorted-out\n '\n test_expect_success SYMLINKS \\\n-'follow-symlinks of dir w/ symlinks w/o cycle' '\n+'follow-symlinks dirs-ignore of dir w/ symlinks w/o cycle' '\n \n \ttest-tool dir-iterator --follow-symlinks ./dir14 >actual-out &&\n \tsort actual-out >actual-sorted-out &&\n \n-\ttest_cmp expected-follow-sorted-out actual-sorted-out\n+\ttest_cmp expected-follow-ignore-sorted-out actual-sorted-out\n+'\n+test_expect_success SYMLINKS \\\n+'dont-follow-symlinks dirs-before of dir w/ symlinks w/o cycle' '\n+\n+\ttest-tool dir-iterator --dirs-before ./dir14 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-dont-follow-before-sorted-out actual-sorted-out\n+'\n+test_expect_success SYMLINKS \\\n+'follow-symlinks dirs-before of dir w/ symlinks w/o cycle' '\n+\n+\ttest-tool dir-iterator --dirs-before --follow-symlinks ./dir14 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-follow-before-sorted-out actual-sorted-out\n+'\n+test_expect_success SYMLINKS \\\n+'dont-follow-symlinks dirs-after of dir w/ symlinks w/o cycle' '\n+\n+\ttest-tool dir-iterator --dirs-after ./dir14 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-dont-follow-after-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+\ttest-tool dir-iterator --dirs-after --follow-symlinks ./dir14 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-follow-after-sorted-out actual-sorted-out\n+'\n+test_expect_success SYMLINKS \\\n+'dont-follow-symlinks dirs-before/dirs-after of dir w/ symlinks w/o cycle' '\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-dont-follow-before-after-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+\ttest-tool dir-iterator --dirs-before --dirs-after --follow-symlinks ./dir14 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-follow-before-after-sorted-out actual-sorted-out\n '\n \n test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/ cycle' '\n@@ -415,7 +1071,14 @@ 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-\tcat >expected-dont-follow-sorted-out <<-EOF &&\n+\n+\tcat >expected-dont-follow-ignore-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+\tcat >expected-dont-follow-before-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@@ -424,22 +1087,87 @@ test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/ cycle' '\n \t[s] (a/b/f) [f] ./dir15/a/b/f\n \tEOF\n \n+\tcat expected-dont-follow-before-sorted-out >expected-dont-follow-after-sorted-out &&\n+\n+\tcat >expected-dont-follow-before-after-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 \tcat >expected-pedantic-follow-tailed-out <<-EOF\n \tdir_iterator_advance failure\n \tEOF\n '\n test_expect_success SYMLINKS \\\n-'dont-follow-symlinks of dir w/ symlinks w/ cycle' '\n+'dont-follow-symlinks dirs-ignore of dir w/ symlinks w/ cycle' '\n \n \ttest-tool dir-iterator ./dir15 >actual-out &&\n \tsort actual-out >actual-sorted-out &&\n \n-\ttest_cmp expected-dont-follow-sorted-out actual-sorted-out\n+\ttest_cmp expected-dont-follow-ignore-sorted-out actual-sorted-out\n '\n test_expect_success SYMLINKS \\\n-'pedantic follow-symlinks of dir w/ symlinks w/ cycle' '\n+'pedantic follow-symlinks dirs-ignore of dir w/ symlinks w/ cycle' '\n \n \ttest_must_fail test-tool dir-iterator \\\n+\t\t--follow-symlinks --pedantic ./dir15 >actual-out &&\n+\ttail -n 1 actual-out >actual-tailed-out &&\n+\n+\ttest_cmp expected-pedantic-follow-tailed-out actual-tailed-out\n+'\n+test_expect_success SYMLINKS \\\n+'dont-follow-symlinks dirs-before of dir w/ symlinks w/ cycle' '\n+\n+\ttest-tool dir-iterator --dirs-before ./dir15 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-dont-follow-before-sorted-out actual-sorted-out\n+'\n+test_expect_success SYMLINKS \\\n+'pedantic follow-symlinks dirs-before of dir w/ symlinks w/ cycle' '\n+\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+\ttest_cmp expected-pedantic-follow-tailed-out actual-tailed-out\n+'\n+test_expect_success SYMLINKS \\\n+'dont-follow-symlinks dirs-after of dir w/ symlinks w/ cycle' '\n+\n+\ttest-tool dir-iterator --dirs-after ./dir15 >actual-out &&\n+\tsort actual-out >actual-sorted-out &&\n+\n+\ttest_cmp expected-dont-follow-after-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+\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-pedantic-follow-tailed-out actual-tailed-out\n+'\n+test_expect_success SYMLINKS \\\n+'dont-follow-symlinks dirs-before/dirs-after of dir w/ symlinks w/ cycle' '\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-dont-follow-before-after-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+\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-- \n2.35.1\n\n"},{"id":"453380","messageId":"20220410111852.2097418-7-kioplato@gmail.com","threadId":"57706","inReplyTo":"20220410111852.2097418-1-kioplato@gmail.com","subject":"[RFC PATCH 6/6] test-dir-iterator: handle EACCES errno by dir-iterator","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-04-10T11:18:52Z","receivedAt":"2022-04-10T11:22:17Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"Handle EACCES errno returned by dir_iterator_begin() by printing the\n\"EACCES\" string instead of printing \"ESOMETHINGELSE\".\n\nSigned-off-by: Plato Kiorpelidis <kioplato@gmail.com>\n---\n t/helper/test-dir-iterator.c | 8 +++++---\n t/t0066-dir-iterator.sh      | 2 +-\n 2 files changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/t/helper/test-dir-iterator.c b/t/helper/test-dir-iterator.c\nindex c92616bd69..fd07429f90 100644\n--- a/t/helper/test-dir-iterator.c\n+++ b/t/helper/test-dir-iterator.c\n@@ -7,9 +7,11 @@\n static const char *error_name(int error_number)\n {\n \tswitch (error_number) {\n-\tcase ENOENT: return \"ENOENT\";\n-\tcase ENOTDIR: return \"ENOTDIR\";\n-\tdefault: return \"ESOMETHINGELSE\";\n+\t\tcase ENOENT: return \"ENOENT\";\n+\t\tcase ENOTDIR: return \"ENOTDIR\";\n+\t\tcase EACCES: return \"EACCES\";\n+\n+\t\tdefault: return \"ESOMETHINGELSE\";\n \t}\n }\n \ndiff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\nindex 974bb13092..4bf6456735 100755\n--- a/t/t0066-dir-iterator.sh\n+++ b/t/t0066-dir-iterator.sh\n@@ -861,7 +861,7 @@ test_expect_success POSIXPERM,SANITY \\\n \n \n \tcat >expected-no-permissions-out <<-EOF &&\n-\tdir_iterator_begin failure: ESOMETHINGELSE\n+\tdir_iterator_begin failure: EACCES\n \tEOF\n \n \ttest_must_fail test-tool dir-iterator ./dir12 >actual-out &&\n-- \n2.35.1\n\n"},{"id":"453399","messageId":"220411.86wnfvj2q6.gmgdl@evledraar.gmail.com","threadId":"57706","inReplyTo":"20220410111852.2097418-7-kioplato@gmail.com","subject":"Re: [RFC PATCH 6/6] test-dir-iterator: handle EACCES errno by dir-iterator","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-11T11:04:48Z","receivedAt":"2022-04-11T11:11:30Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Apr 10 2022, Plato Kiorpelidis wrote:\n\n> Handle EACCES errno returned by dir_iterator_begin() by printing the\n> \"EACCES\" string instead of printing \"ESOMETHINGELSE\".\n>\n> Signed-off-by: Plato Kiorpelidis <kioplato@gmail.com>\n> ---\n>  t/helper/test-dir-iterator.c | 8 +++++---\n>  t/t0066-dir-iterator.sh      | 2 +-\n>  2 files changed, 6 insertions(+), 4 deletions(-)\n>\n> diff --git a/t/helper/test-dir-iterator.c b/t/helper/test-dir-iterator.c\n> index c92616bd69..fd07429f90 100644\n> --- a/t/helper/test-dir-iterator.c\n> +++ b/t/helper/test-dir-iterator.c\n> @@ -7,9 +7,11 @@\n>  static const char *error_name(int error_number)\n>  {\n>  \tswitch (error_number) {\n> -\tcase ENOENT: return \"ENOENT\";\n> -\tcase ENOTDIR: return \"ENOTDIR\";\n> -\tdefault: return \"ESOMETHINGELSE\";\n> +\t\tcase ENOENT: return \"ENOENT\";\n> +\t\tcase ENOTDIR: return \"ENOTDIR\";\n> +\t\tcase EACCES: return \"EACCES\";\n> +\n> +\t\tdefault: return \"ESOMETHINGELSE\";\n>  \t}\n>  }\n\nPlease stick with the git coding style, see\nDocumentation/CodingGuidelines.\n\nIt's forgivable to \"fix style while at it\" to some extent, but in this\ncase it's both moving away from our normal style (by indenting case\nlabels), and in doing so making the diff larger/worse.\n\nThis should be a one-line addition of your new EACCES case.\n\nI haven't read through the rest yet, but please self-review those\npatches with a keen eye to diff size & seeing if there's similar\nissues. E.g. are they the same lines changed under \"git show -w\", if not\nare you sure you're correct in making the style change?\n\n> diff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\n> index 974bb13092..4bf6456735 100755\n> --- a/t/t0066-dir-iterator.sh\n> +++ b/t/t0066-dir-iterator.sh\n> @@ -861,7 +861,7 @@ test_expect_success POSIXPERM,SANITY \\\n>  \n>  \n>  \tcat >expected-no-permissions-out <<-EOF &&\n> -\tdir_iterator_begin failure: ESOMETHINGELSE\n> +\tdir_iterator_begin failure: EACCES\n>  \tEOF\n>  \n>  \ttest_must_fail test-tool dir-iterator ./dir12 >actual-out &&\n\nI see this is changing the ESOMETHINGELSE added in your 3/6, if you make\nthis patch come first then presumably we won't have that churn.\n\nAnd if we don't have a test that's relying on ESOMETHINGELSE (maybe we\ndo, but don't test_cmp it, I haven't checked) shouldn't this \"default\"\ncase just be:\n\n    BUG(\"unknown errno %d: %s\", errno_number, strerror(errno_number))\n\nI.e. if we have OK coverage here we'd presumably fail the test case\nitself anyway, so shouldn't we fail here too?\n"},{"id":"453400","messageId":"220411.86sfqjj2o0.gmgdl@evledraar.gmail.com","threadId":"57706","inReplyTo":"20220410111852.2097418-6-kioplato@gmail.com","subject":"Re: [RFC PATCH 5/6] t0066: remove redundant tests","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-11T11:10:10Z","receivedAt":"2022-04-11T11:11:33Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Apr 10 2022, Plato Kiorpelidis wrote:\n\n> Remove dir-iterator redundant tests since the new tests introduced in\n> 045738486f (t0066: better test coverage for dir-iterator, 2022-04-07)\n> supersede them.\n\nThis is presumably a forward-reference to the OID for your local version\nof 2/6. Let's instead squash this change into that commit.\n"},{"id":"453401","messageId":"220411.86o817j2dt.gmgdl@evledraar.gmail.com","threadId":"57706","inReplyTo":"20220410111852.2097418-4-kioplato@gmail.com","subject":"Re: [RFC PATCH 3/6] dir-iterator: refactor dir_iterator_advance()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-11T11:11:06Z","receivedAt":"2022-04-11T11:17:10Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Apr 10 2022, Plato Kiorpelidis wrote:\n\n> Simplify dir_iterator_advance by switch from iterative to recursive\n> implementation. In each recursive step one action is performed.\n>\n> This makes dir-iterator easier to work with, understand and introduce\n> new functionality.\n>\n> Signed-off-by: Plato Kiorpelidis <kioplato@gmail.com>\n> ---\n>  dir-iterator.c          | 225 ++++++++++++++++++++++++++--------------\n>  t/t0066-dir-iterator.sh |   4 +-\n>  2 files changed, 148 insertions(+), 81 deletions(-)\n>\n> diff --git a/dir-iterator.c b/dir-iterator.c\n> index b17e9f970a..0f9ed95958 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> @@ -45,34 +45,53 @@ struct dir_iterator_int {\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)) return 0;\n\nstyle: missing \\n before \"return\".\n\nAlso, the one existing caller before this patch is:\n\n    if (S_ISDIR(iter->base.st.st_mode) && push_level(iter))\n\nWhy not continue to do that?\n\n> +/*\n> + * Activate most recent pushed level.\n> + *\n> + * Return 1 on success.\n> + * Return -1 on failure when errno == ENOENT, leaving the stack unchanged.\n> + * Return -2 on failure when errno != ENOENT, leaving the stack unchanged.\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 1;\n> +\n> +\tif ((level->dir = opendir(iter->base.path.buf)) != NULL)\n> +\t\treturn 1;\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 -2;\n\nPerhaps we should just add an enum for these return values instead of\nadding more negative/positive values here. That makes your later calls\nof activate_level() more idiomaic. E.g. !activate_level() instead of\nactivate_level() == 1.\n\n>  \t\twarning_errno(\"failed to stat '%s'\", iter->base.path.buf);\n> +\t\treturn -2;  // Stat failed not with ENOENT.\n\nDon't use // comments, use /* .. */\n> +\t} else if (stat_err && errno == ENOENT)\n> +\t\treturn -1;  // Stat failed with ENOENT.\n\nMissing {} here for the else if.\n\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> +\n> +\tstruct dirent *dir_entry = NULL;\n> +\n> +\tint expose_err, activate_err;\n> +\n> +\t/* For shorter code width-wise, more readable */\n> +\tunsigned int PEDANTIC = iter->flags & DIR_ITERATOR_PEDANTIC;\n\nWe usually don't add \\n\\n in the middle of variable decls.\n\n> +\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 == -2 && PEDANTIC)\n> +\t\tgoto error_out;\n> +\telse if (activate_err == -2 || activate_err == -1) {\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\t--iter->levels_nr;\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> -\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> +\tif (dir_entry == NULL) {\n\nDon't compare against NULL, use !dir_entry.\n\nAlso: Manually resetting \"errno\" before isn't needed. It will be (re)set\nif readdir() returns NULL().\n\n> +\t\tif (errno) {\n> +\t\t\twarning_errno(\"errno reading dir '%s'\", iter->base.path.buf);\n> +\t\t\tif (iter->flags & DIR_ITERATOR_PEDANTIC) goto error_out;\n\nmore missing \\n before goto/return.\n"},{"id":"453405","messageId":"ed6656e0-a865-319e-0f56-23ab1da3eef3@gmail.com","threadId":"57706","inReplyTo":"20220410111852.2097418-2-kioplato@gmail.com","subject":"Re: [RFC PATCH 1/6] t0066: improve readablity of dir-iterator tests","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-04-11T13:16:19Z","receivedAt":"2022-04-11T13:16:37Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Plato\n\nOn 10/04/2022 12:18, Plato Kiorpelidis wrote:\n> Be consistent throughout the dir-iterator tests regarding the order in\n> which we:\n>    * create test directories\n>    * create expected outputs\n>    * test if actual and expected outputs differ\n\nWhen writing a commit message it is important to explain why you are \nmaking the changes in the patch, rather than just describe the changes \nthemselves.\n\nIn general modernizing or standardizing existing tests before adding new \nones is a good idea. However it is important to do it in a way that lets \nreviewers see there are no unintended changes. In this patch so much is \nchanged it's really hard to tell if you have changed any of the tests or \nnot. It would be much easier if you had not renamed all the directories \nand files that are created and renamed the tests as well.\n\nI think that having separate setup tests makes sense as we're going to \nwant to test different iteration schemes over the same hierarchy but it \nwould be helpful to have the \"expect\" files written in the same test \nthat calls test_cmp. That way it is much easier to debug test failures \nin the CI as it one can see the contents of \"expect\" as well as the diff \nfrom test_cmp in the CI output.\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Plato Kiorpelidis <kioplato@gmail.com>\n> ---\n>   t/t0066-dir-iterator.sh | 227 +++++++++++++++++++++-------------------\n>   1 file changed, 118 insertions(+), 109 deletions(-)\n> \n> diff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\n> index 63a1a45cd3..fb20219487 100755\n> --- a/t/t0066-dir-iterator.sh\n> +++ b/t/t0066-dir-iterator.sh\n> @@ -5,145 +5,154 @@ test_description='Test the dir-iterator functionality'\n>   TEST_PASSES_SANITIZE_LEAK=true\n>   . ./test-lib.sh\n>   \n> -test_expect_success 'setup' '\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> -\tmkdir -p dir2/a/b/c/ &&\n> -\t>dir2/a/b/c/d\n> +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> +\n> +\tcat >expected-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> +\tEOF\n> +'\n> +test_expect_success 'iteration of dir w/ three nested dirs w/ file' '\n> +\ttest-tool dir-iterator ./dir6 >actual-out &&\n> +\ttest_cmp expected-out actual-out\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> -\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> +test_expect_success 'setup -- dir w/ complex structure w/o symlinks' '\n> +\tmkdir -p dir11/a/b/c/ &&\n> +\t>dir11/b &&\n> +\t>dir11/c &&\n> +\t>dir11/a/e &&\n> +\t>dir11/a/b/c/d &&\n> +\tmkdir -p dir11/d/e/d/ &&\n> +\t>dir11/d/e/d/a &&\n> +\n> +\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> +test_expect_success 'iteration of dir w/ complex structure w/o symlinks' '\n> +\ttest-tool dir-iterator ./dir11 >actual-out &&\n> +\tsort actual-out >actual-sorted-out &&\n>   \n> -\ttest-tool dir-iterator ./dir >out &&\n> -\tsort out >./actual-iteration-sorted-output &&\n> +\ttest_cmp expected-sorted-out actual-sorted-out\n> +'\n>   \n> -\ttest_cmp expected-iteration-sorted-output actual-iteration-sorted-output\n> +test_expect_success 'dir_iterator_begin() should fail upon inexistent paths' '\n> +\techo \"dir_iterator_begin failure: ENOENT\" >expected-inexistent-path-out &&\n> +\n> +\ttest_must_fail test-tool dir-iterator ./inexistent-path >actual-out &&\n> +\ttest_cmp expected-inexistent-path-out actual-out\n>   '\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> -\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> +test_expect_success 'dir_iterator_begin() should fail upon non directory paths' '\n> +\t>some-file &&\n>   \n> -\ttest-tool dir-iterator ./dir2 >actual-pre-order-output &&\n>   \n> -\ttest_cmp expected-pre-order-output actual-pre-order-output\n> -'\n> +\techo \"dir_iterator_begin failure: ENOTDIR\" >expected-non-dir-out &&\n>   \n> -test_expect_success 'begin should fail upon inexistent paths' '\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> -\ttest_cmp expected-inexistent-path-output actual-inexistent-path-output\n> -'\n> +\ttest_must_fail test-tool dir-iterator ./some-file >actual-out &&\n> +\ttest_cmp expected-non-dir-out actual-out &&\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> -\ttest_cmp expected-non-dir-output actual-non-dir-output\n> +\ttest_must_fail test-tool dir-iterator --pedantic ./some-file >actual-out &&\n> +\ttest_cmp expected-non-dir-out actual-out\n>   '\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> +test_expect_success POSIXPERM,SANITY \\\n> +'dir_iterator_advance() should not fail on errors by default' '\n>   \n> -\tmkdir -p dir3/a &&\n> -\t>dir3/a/b &&\n> -\tchmod 0 dir3/a &&\n> +\tmkdir -p dir13/a &&\n> +\t>dir13/a/b &&\n> +\tchmod 0 dir13/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> -'\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> +\tcat >expected-no-permissions-out <<-EOF &&\n> +\t[d] (a) [a] ./dir13/a\n>   \tEOF\n>   \n> -\tmkdir -p dir3/a &&\n> -\t>dir3/a/b &&\n> -\tchmod 0 dir3/a &&\n> +\ttest-tool dir-iterator ./dir13 >actual-out &&\n> +\ttest_cmp expected-no-permissions-out actual-out &&\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 dir13/a &&\n> +\trm -rf dir13\n>   '\n>   \n> -test_expect_success SYMLINKS 'setup dirs with symlinks' '\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> -'\n> +test_expect_success POSIXPERM,SANITY \\\n> +'dir_iterator_advance() should fail on errors, w/ pedantic flag' '\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> -\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> +\tmkdir -p dir13/a &&\n> +\t>dir13/a/b &&\n> +\tchmod 0 dir13/a &&\n> +\n> +\n> +\tcat >expected-no-permissions-pedantic-out <<-EOF &&\n> +\t[d] (a) [a] ./dir13/a\n> +\tdir_iterator_advance failure\n>   \tEOF\n>   \n> -\ttest-tool dir-iterator ./dir4 >out &&\n> -\tsort out >actual-no-follow-sorted-output &&\n> +\ttest_must_fail test-tool dir-iterator --pedantic ./dir13 >actual-out &&\n> +\ttest_cmp expected-no-permissions-pedantic-out actual-out &&\n>   \n> -\ttest_cmp expected-no-follow-sorted-output actual-no-follow-sorted-output\n> +\tchmod 755 dir13/a &&\n> +\trm -rf dir13\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> -\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> +test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/o cycle' '\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> +\n> +\tcat >expected-dont-follow-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> +\tcat >expected-follow-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> +test_expect_success SYMLINKS \\\n> +'dont-follow-symlinks of dir w/ symlinks w/o cycle' '\n> +\n> +\ttest-tool dir-iterator ./dir14 >actual-out &&\n> +\tsort actual-out >actual-sorted-out &&\n> +\n> +\ttest_cmp expected-dont-follow-sorted-out actual-sorted-out\n> +'\n> +test_expect_success SYMLINKS \\\n> +'follow-symlinks of dir w/ symlinks w/o cycle' '\n>   \n> -\ttest-tool dir-iterator --follow-symlinks ./dir4 >out &&\n> -\tsort out >actual-follow-sorted-output &&\n> +\ttest-tool dir-iterator --follow-symlinks ./dir14 >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-follow-sorted-out actual-sorted-out\n>   '\n>   \n>   test_done\n\n"},{"id":"453406","messageId":"35160d46-d337-2110-f968-238abb7e9f0e@gmail.com","threadId":"57706","inReplyTo":"20220410111852.2097418-4-kioplato@gmail.com","subject":"Re: [RFC PATCH 3/6] dir-iterator: refactor dir_iterator_advance()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-04-11T13:26:29Z","receivedAt":"2022-04-11T13:26:49Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Plato\n\nOn 10/04/2022 12:18, Plato Kiorpelidis wrote:\n> Simplify dir_iterator_advance by switch from iterative to recursive\n> implementation. In each recursive step one action is performed.\n> \n> This makes dir-iterator easier to work with, understand and introduce\n> new functionality.\n\nI think that this message could explain a bit more about why it is \nnecessary to use a recursive approach. As we are implementing an \niterator we cannot keep state in the stack between iterations so how \ndoes using recursive approach help us?\n\nI reviewed this series as a way to learn the directory iterator code as \nI'd not looked at it before. Having read the existing code I think the \nit is fairly easy to follow. I'm not sure the changes here improve that. \nI also found these changes harder to follow because they renamed \nexisting functions and variables in addition to moving the code around.\n\nIn order in implement returning the directory name after its contents I \nthink all we need is to remember whether we've returned the name when we \nreaddir() returns NULL so we don't pop it too soon. It's not clear that \nwe need a lot of refactoring to do that, especially as the patches you \nlinked to in the cover letter seem to avoid that.\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Plato Kiorpelidis <kioplato@gmail.com>\n> ---\n>   dir-iterator.c          | 225 ++++++++++++++++++++++++++--------------\n>   t/t0066-dir-iterator.sh |   4 +-\n>   2 files changed, 148 insertions(+), 81 deletions(-)\n> \n> diff --git a/dir-iterator.c b/dir-iterator.c\n> index b17e9f970a..0f9ed95958 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> @@ -45,34 +45,53 @@ struct dir_iterator_int {\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)) return 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.\n> + *\n> + * Return 1 on success.\n> + * Return -1 on failure when errno == ENOENT, leaving the stack unchanged.\n> + * Return -2 on failure when errno != ENOENT, leaving the stack unchanged.\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 1;\n> +\n> +\tif ((level->dir = opendir(iter->base.path.buf)) != NULL)\n> +\t\treturn 1;\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 -2;\n>   \t}\n> -\n> -\treturn 0;\n> +\terrno = saved_errno;\n> +\treturn -1;\n>   }\n>   \n>   /*\n> @@ -81,12 +100,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 +111,121 @@ 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> + * 1 on successful exposure of the provided entry.\n> + * -1 on failed exposure because entry does not exist.\n> + * -2 on failed exposure because of error 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 -2;  // Stat failed not with ENOENT.\n> +\t} else if (stat_err && errno == ENOENT)\n> +\t\treturn -1;  // Stat failed with ENOENT.\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 1;\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> -\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\t\tgoto error_out;\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> +\n> +\tstruct dirent *dir_entry = NULL;\n> +\n> +\tint expose_err, activate_err;\n> +\n> +\t/* For shorter code width-wise, more readable */\n> +\tunsigned int PEDANTIC = iter->flags & DIR_ITERATOR_PEDANTIC;\n> +\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 == -2 && PEDANTIC)\n> +\t\tgoto error_out;\n> +\telse if (activate_err == -2 || activate_err == -1) {\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\t--iter->levels_nr;\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> -\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> +\tif (dir_entry == NULL) {\n> +\t\tif (errno) {\n> +\t\t\twarning_errno(\"errno reading dir '%s'\", iter->base.path.buf);\n> +\t\t\tif (iter->flags & DIR_ITERATOR_PEDANTIC) goto error_out;\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 == -2 && PEDANTIC)\n> +\t\tgoto error_out;\n> +\n> +\tif (expose_err == 1)\n> +\t\tpush_level(iter);\n> +\n> +\tif (expose_err == -1)\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 +263,7 @@ 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> +\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 +283,16 @@ 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) != 1) {\n> +\t\tsaved_errno = ENOTDIR;\n> +\t\tgoto error_out;\n> +\t}\n> +\n> +\tif (activate_level(iter) != 1) {\n> +\t\tsaved_errno = errno;\n> +\t\tgoto error_out;\n> +\t}\n> +\n>   \treturn dir_iterator;\n>   \n>   error_out:\n> diff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\n> index 0a98dd54ba..2437ab21c4 100755\n> --- a/t/t0066-dir-iterator.sh\n> +++ b/t/t0066-dir-iterator.sh\n> @@ -285,7 +285,7 @@ test_expect_success 'iteration of dir w/ complex structure w/o symlinks' '\n>   '\n>   \n>   test_expect_success POSIXPERM,SANITY \\\n> -'dir_iterator_advance() should fail on root dir w/o perms' '\n> +'dir_iterator_begin() should fail on root dir w/o perms' '\n>   \n>   \tmkdir -p dir12/a &&\n>   \t>dir12/a/b &&\n> @@ -293,7 +293,7 @@ test_expect_success POSIXPERM,SANITY \\\n>   \n>   \n>   \tcat >expected-no-permissions-out <<-EOF &&\n> -\tdir_iterator_advance failure\n> +\tdir_iterator_begin failure: ESOMETHINGELSE\n>   \tEOF\n>   \n>   \ttest_must_fail test-tool dir-iterator ./dir12 >actual-out &&\n\n"},{"id":"453407","messageId":"b75aaee8-c037-e8e0-6ee0-729922059352@gmail.com","threadId":"57706","inReplyTo":"20220410111852.2097418-5-kioplato@gmail.com","subject":"Re: [RFC PATCH 4/6] dir-iterator: iterate dirs before or after their contents","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-04-11T13:31:39Z","receivedAt":"2022-04-11T13:31:58Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Plato\n\nOn 10/04/2022 12:18, Plato Kiorpelidis wrote:\n> Introduce a new feature to dir-iterator, using dir_iterator_begin()\n> flags parameter, allowing to control whether or not directories will\n> appear before or after their contents.\n> \n> The new default behavior (i.e. flags set to 0) does not iterate over\n> directories.\n\nThis is introducing two changes at once adding the \"AFTER\" flag and \nchanging the default behavior. I think it wolud make sence to separate \nthose into two separate commits. You could change the default adding the \n\"BEFORE\" flag and adjusting the existing callers first and then \nimplement the \"AFTER\" flag in a second commit.\n\nIt would also be nice to see the \"AFTER\" flag being used by changing \nremove_dir_recursively() for example.\n\nBest Wishes\n\nPhillip\n\n  Flag DIR_ITERATOR_DIRS_BEFORE iterates over a directory\n> before doing so over its contents. Flag DIR_ITERATOR_DIRS_AFTER\n> iterates over a directory after doing so over its contents. These flags\n> do not conflict with each other and may be used simultaneously:\n>    * ignore directories\n>    (DIR_ITERATOR_DIRS_BEFORE and DIR_ITERATOR_DIRS_AFTER are both false)\n>    * expose them before opening and after exhausting them\n>    (DIR_ITERATOR_DIRS_BEFORE and DIR_ITERATOR_DIRS_AFTER are both true).\n> \n> Amend a call to dir_iterator_begin() in refs/files-backend.c and\n> builtin/clone.c to enable DIR_ITERATOR_DIRS_BEFORE, since they need\n> iteration over a directory before doing so over its contents.\n> \n> Update t/t0066-dir-iterator.sh and t/helper/test-dir-iterator.c to test\n> the new flags DIR_ITERATOR_DIRS_BEFORE, DIR_ITERATOR_DIRS_AFTER.\n> \n> Signed-off-by: Plato Kiorpelidis <kioplato@gmail.com>\n> ---\n>   builtin/clone.c              |   4 +-\n>   dir-iterator.c               | 101 ++++-\n>   dir-iterator.h               |  34 +-\n>   refs/files-backend.c         |   2 +-\n>   t/helper/test-dir-iterator.c |  15 +-\n>   t/t0066-dir-iterator.sh      | 844 ++++++++++++++++++++++++++++++++---\n>   6 files changed, 923 insertions(+), 77 deletions(-)\n> \n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index 5231656379..87ee764ba3 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 0f9ed95958..41a1481bb4 100644\n> --- a/dir-iterator.c\n> +++ b/dir-iterator.c\n> @@ -116,16 +116,26 @@ static int pop_level(struct dir_iterator_int *iter)\n>    *\n>    * Return values:\n>    * 1 on successful exposure of the provided entry.\n> + * 0 on failed exposure because entry is directory and flags don't allow it.\n>    * -1 on failed exposure because entry does not exist.\n>    * -2 on failed exposure because of error 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>   \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> @@ -137,6 +147,11 @@ static int expose_entry(struct dir_iterator_int *iter, char *d_name)\n>   \t} else if (stat_err && errno == ENOENT)\n>   \t\treturn -1;  // Stat failed with ENOENT.\n>   \n> +\tif (S_ISDIR(iter->base.st.st_mode)) {\n> +\t\tif (!DIRS_BEFORE && !strcmp(dir_state, \"before\")) return 0;\n> +\t\tif (!DIRS_AFTER && !strcmp(dir_state, \"after\")) return 0;\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> @@ -150,6 +165,21 @@ static int expose_entry(struct dir_iterator_int *iter, char *d_name)\n>   \treturn 1;\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, '/') + 1;\n> +\tchar *basename = calloc(1, strlen(start) + 1);\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> @@ -180,9 +210,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 == -2 && PEDANTIC)\n> +\t\t\tgoto error_out;\n> +\t\telse if (expose_err == -1 || expose_err == 0)\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> @@ -198,12 +247,41 @@ 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\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\tif (pop_level(iter) == 0) {\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 == -2 && PEDANTIC)\n> +\t\t\t\tgoto error_out;\n> +\t\t\telse if (expose_err == -1 || expose_err == 0)\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>   \n> @@ -212,17 +290,26 @@ 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 == -2 && PEDANTIC)\n>   \t\tgoto error_out;\n>   \n> -\tif (expose_err == 1)\n> +\tif (expose_err == 0 || expose_err == 1)\n>   \t\tpush_level(iter);\n>   \n> -\tif (expose_err == -1)\n> +\tif (expose_err == 0 || expose_err == -1)\n>   \t\treturn dir_iterator_advance(dir_iterator);\n>   \n>   \treturn ITER_OK;\n> diff --git a/dir-iterator.h b/dir-iterator.h\n> index 08229157c6..dea4fc8f3e 100644\n> --- a/dir-iterator.h\n> +++ b/dir-iterator.h\n> @@ -8,19 +8,21 @@\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.\n> + * Note that the 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\n> + * their 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 directories themselves not being exposed, only their contents will be.\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 +63,25 @@\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 before\n> + *   iterating through that directory's contents.\n> + *\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>    * 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> +#define DIR_ITERATOR_DIRS_AFTER (1 << 3)\n>   \n>   struct dir_iterator {\n>   \t/* The current path: */\n> @@ -97,12 +112,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 paths,\n> + *    but will include non-directory directory contents.\n>    */\n>   struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags);\n>   \n> @@ -110,6 +126,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 the dir_iterator 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 +136,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 95acab78ee..951a673e71 100644\n> --- a/refs/files-backend.c\n> +++ b/refs/files-backend.c\n> @@ -2247,7 +2247,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 659b6bfa81..c92616bd69 100644\n> --- a/t/helper/test-dir-iterator.c\n> +++ b/t/helper/test-dir-iterator.c\n> @@ -15,7 +15,16 @@ 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> + *\t--dirs-after\n> + *\n> + * example:\n> + * test-tool dir-iterator --pedantic --dirs-before --dirs-after ./somedir\n>    */\n>   int cmd__dir_iterator(int argc, const char **argv)\n>   {\n> @@ -28,6 +37,10 @@ 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 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}\n> diff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\n> index 2437ab21c4..24a4f1afef 100755\n> --- a/t/t0066-dir-iterator.sh\n> +++ b/t/t0066-dir-iterator.sh\n> @@ -14,44 +14,157 @@ test_expect_success 'setup -- dir with a single file' '\n>   \t[f] (a) [a] ./dir1/a\n>   \tEOF\n>   '\n> -test_expect_success 'iteration of dir with a file' '\n> +test_expect_success 'dirs-ignore 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> +\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> +\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> +\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>   \n>   \n> -\tcat >expected-out <<-EOF\n> +\tcat >expected-ignore-out <<-EOF &&\n> +\tEOF\n> +\n> +\tcat >expected-before-out <<-EOF &&\n> +\t[d] (a) [a] ./dir2/a\n> +\tEOF\n> +\n> +\tcat expected-before-out >expected-after-out &&\n> +\n> +\tcat >expected-before-after-out <<-EOF\n> +\t[d] (a) [a] ./dir2/a\n>   \t[d] (a) [a] ./dir2/a\n>   \tEOF\n>   '\n> -test_expect_success 'iteration of dir with a single dir' '\n> +test_expect_success 'dirs-ignore of dir with a single dir' '\n>   \ttest-tool dir-iterator ./dir2 >actual-out &&\n> -\ttest_cmp expected-out actual-out\n> +\ttest_cmp expected-ignore-out actual-out\n> +'\n> +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-before-out actual-out\n> +'\n> +test_expect_success 'dirs-after of dir with a single dir' '\n> +\ttest-tool dir-iterator --dirs-after ./dir2 >actual-out &&\n> +\ttest_cmp expected-after-out actual-out\n> +'\n> +test_expect_success 'dirs-before/dirs-after of dir with a single dir' '\n> +\ttest-tool dir-iterator --dirs-before --dirs-after ./dir2 >actual-out &&\n> +\ttest_cmp expected-before-after-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> -\tchmod 0 dir3/a &&\n>   \n>   \n> -\tcat >expected-out <<-EOF &&\n> +\tcat >expected-ignore-out <<-EOF &&\n> +\tEOF\n> +\tcat >expected-pedantic-ignore-out <<-EOF &&\n> +\tdir_iterator_advance failure\n> +\tEOF\n> +\n> +\tcat >expected-before-out <<-EOF &&\n>   \t[d] (a) [a] ./dir3/a\n>   \tEOF\n> -\tcat >expected-pedantic-out <<-EOF\n> +\tcat >expected-pedantic-before-out <<-EOF &&\n>   \t[d] (a) [a] ./dir3/a\n>   \tdir_iterator_advance failure\n>   \tEOF\n> +\n> +\tcat expected-before-out >expected-after-out &&\n> +\tcat >expected-pedantic-after-out <<-EOF &&\n> +\tdir_iterator_advance failure\n> +\tEOF\n> +\n> +\tcat >expected-before-after-out <<-EOF &&\n> +\t[d] (a) [a] ./dir3/a\n> +\t[d] (a) [a] ./dir3/a\n> +\tEOF\n> +\tcat expected-pedantic-before-out >expected-pedantic-before-after-out\n>   '\n> -test_expect_success POSIXPERM,SANITY 'iteration of dir w/ dir w/o perms' '\n> +test_expect_success POSIXPERM,SANITY 'dirs-ignore of dir w/ dir w/o perms' '\n> +\tchmod 0 dir3/a &&\n> +\n>   \ttest-tool dir-iterator ./dir3/ >actual-out &&\n> -\ttest_cmp expected-out actual-out\n> +\ttest_cmp expected-ignore-out actual-out &&\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-ignore of dir w/ dir w/o perms' '\n> +\tchmod 0 dir3/a &&\n> +\n>   \ttest_must_fail test-tool dir-iterator --pedantic ./dir3/ >actual-out &&\n> -\ttest_cmp expected-pedantic-out actual-out\n> +\ttest_cmp expected-pedantic-ignore-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> +\tchmod 0 dir3/a &&\n> +\n> +\ttest-tool dir-iterator --dirs-before ./dir3/ >actual-out &&\n> +\ttest_cmp expected-before-out actual-out &&\n> +\n> +\tchmod 755 dir3/a\n> +'\n> +test_expect_success POSIXPERM,SANITY 'pedantic dirs-before of dir w/ dir w/o perms' '\n> +\tchmod 0 dir3/a &&\n> +\n> +\ttest_must_fail test-tool dir-iterator --dirs-before \\\n> +\t\t--pedantic ./dir3/ >actual-out &&\n> +\ttest_cmp expected-pedantic-before-out actual-out &&\n> +\n> +\tchmod 755 dir3/a\n> +'\n> +test_expect_success POSIXPERM,SANITY 'dirs-after of dir w/ dir w/o perms' '\n> +\tchmod 0 dir3/a &&\n> +\n> +\ttest-tool dir-iterator --dirs-after ./dir3/ >actual-out &&\n> +\ttest_cmp expected-after-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> +\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-pedantic-after-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> +\tchmod 0 dir3/a &&\n> +\n> +\ttest-tool dir-iterator --dirs-before --dirs-after ./dir3/ >actual-out &&\n> +\ttest_cmp expected-before-after-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> +\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-pedantic-before-after-out actual-out &&\n> +\n> +\tchmod 755 dir3/a\n>   '\n>   \n>   test_expect_success 'setup -- dir w/ five files' '\n> @@ -71,26 +184,71 @@ test_expect_success 'setup -- dir w/ five files' '\n>   \t[f] (e) [e] ./dir4/e\n>   \tEOF\n>   '\n> -test_expect_success 'iteration of dir w/ five files' '\n> +test_expect_success 'dirs-ignore of dir w/ five files' '\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> +test_expect_success 'dirs-before of dir w/ five files' '\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> +test_expect_success 'dirs-after of dir w/ five files' '\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> +\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>   \t>dir5/a/b &&\n>   \n>   \n> -\tcat >expected-out <<-EOF\n> +\tcat >expected-ignore-out <<-EOF &&\n> +\t[f] (a/b) [b] ./dir5/a/b\n> +\tEOF\n> +\n> +\tcat >expected-before-out <<-EOF &&\n> +\t[d] (a) [a] ./dir5/a\n> +\t[f] (a/b) [b] ./dir5/a/b\n> +\tEOF\n> +\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> +\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> -test_expect_success 'iteration of dir w/ dir w/ a file' '\n> +test_expect_success 'dirs-ignore of dir w/ dir w/ a file' '\n>   \ttest-tool dir-iterator ./dir5 >actual-out &&\n> -\ttest_cmp expected-out actual-out\n> +\ttest_cmp expected-ignore-out actual-out\n> +'\n> +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-before-out actual-out\n> +'\n> +test_expect_success 'dirs-after of dir w/ dir w/ a file' '\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> +\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> @@ -98,16 +256,49 @@ test_expect_success 'setup -- dir w/ three nested dirs w/ file' '\n>   \t>dir6/a/b/c/d &&\n>   \n>   \n> -\tcat >expected-out <<-EOF\n> +\tcat >expected-ignore-out <<-EOF &&\n> +\t[f] (a/b/c/d) [d] ./dir6/a/b/c/d\n> +\tEOF\n> +\n> +\tcat >expected-before-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>   \tEOF\n> +\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> +\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> -test_expect_success 'iteration of dir w/ three nested dirs w/ file' '\n> +test_expect_success 'dirs-ignore of dir w/ three nested dirs w/ file' '\n>   \ttest-tool dir-iterator ./dir6 >actual-out &&\n> -\ttest_cmp expected-out actual-out\n> +\ttest_cmp expected-ignore-out actual-out\n> +'\n> +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-before-out actual-out\n> +'\n> +test_expect_success 'dirs-after of dir w/ three nested dirs w/ file' '\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> +\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> @@ -116,33 +307,123 @@ test_expect_success POSIXPERM,SANITY \\\n>   \tmkdir -p dir7/a/b/c &&\n>   \t>dir7/a/b/c/d &&\n>   \n> -\tcat >expected-out <<-EOF &&\n> +\n> +\tcat >expected-ignore-out <<-EOF &&\n> +\tEOF\n> +\tcat >expected-pedantic-ignore-out <<-EOF &&\n> +\tdir_iterator_advance failure\n> +\tEOF\n> +\n> +\tcat >expected-before-out <<-EOF &&\n> +\t[d] (a) [a] ./dir7/a\n> +\t[d] (a/b) [b] ./dir7/a/b\n> +\tEOF\n> +\tcat >expected-pedantic-before-out <<-EOF &&\n> +\t[d] (a) [a] ./dir7/a\n> +\t[d] (a/b) [b] ./dir7/a/b\n> +\tdir_iterator_advance failure\n> +\tEOF\n> +\n> +\tcat >expected-after-out <<-EOF &&\n> +\t[d] (a/b) [b] ./dir7/a/b\n> +\t[d] (a) [a] ./dir7/a\n> +\tEOF\n> +\tcat >expected-pedantic-after-out <<-EOF &&\n> +\tdir_iterator_advance failure\n> +\tEOF\n> +\n> +\tcat >expected-before-after-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> -\tcat >expected-pedantic-out <<-EOF\n> +\tcat >expected-pedantic-before-after-out <<-EOF\n>   \t[d] (a) [a] ./dir7/a\n>   \t[d] (a/b) [b] ./dir7/a/b\n>   \tdir_iterator_advance failure\n>   \tEOF\n>   '\n>   test_expect_success POSIXPERM,SANITY \\\n> -'iteration of dir w/ three nested dirs w/ file, second w/o perms' '\n> +'dirs-ignore of dir w/ three nested dirs w/ file, second w/o perms' '\n>   \n>   \tchmod 0 dir7/a/b &&\n>   \n>   \ttest-tool dir-iterator ./dir7 >actual-out &&\n> -\ttest_cmp expected-out actual-out &&\n> +\ttest_cmp expected-ignore-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> +'pedantic dirs-ignore of dir w/ three nested dirs w/ file, second w/o perms' '\n>   \n>   \tchmod 0 dir7/a/b &&\n>   \n>   \ttest_must_fail test-tool dir-iterator --pedantic ./dir7 >actual-out &&\n> -\ttest_cmp expected-pedantic-out actual-out &&\n> +\ttest_cmp expected-pedantic-ignore-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> +\tchmod 0 dir7/a/b &&\n> +\n> +\ttest-tool dir-iterator --dirs-before ./dir7 >actual-out &&\n> +\ttest_cmp expected-before-out actual-out &&\n> +\n> +\tchmod 755 dir7/a/b\n> +'\n> +test_expect_success POSIXPERM,SANITY \\\n> +'pedantic dirs-before of dir w/ three nested dirs w/ file, second w/o perms' '\n> +\n> +\tchmod 0 dir7/a/b &&\n> +\n> +\ttest_must_fail test-tool dir-iterator --dirs-before \\\n> +\t\t--pedantic ./dir7 >actual-out &&\n> +\ttest_cmp expected-pedantic-before-out actual-out &&\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> +\tchmod 0 dir7/a/b &&\n> +\n> +\ttest-tool dir-iterator --dirs-after ./dir7 >actual-out &&\n> +\ttest_cmp expected-after-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> +\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-pedantic-after-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> +\tchmod 0 dir7/a/b &&\n> +\n> +\ttest-tool dir-iterator --dirs-before --dirs-after ./dir7 >actual-out &&\n> +\ttest_cmp expected-before-after-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> +\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-pedantic-before-after-out actual-out &&\n>   \n>   \tchmod 755 dir7/a/b\n>   '\n> @@ -154,24 +435,84 @@ test_expect_success 'setup -- dir w/ two dirs each w/ file' '\n>   \t>dir8/c/d &&\n>   \n>   \n> -\tcat >expected-out1 <<-EOF &&\n> +\tcat >expected-ignore-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-ignore-out2 <<-EOF &&\n> +\t[f] (c/d) [d] ./dir8/c/d\n> +\t[f] (a/b) [b] ./dir8/a/b\n> +\tEOF\n> +\n> +\tcat >expected-before-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> +\tcat >expected-before-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> +\tcat >expected-after-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-after-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> +\tcat >expected-before-after-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-before-after-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> -test_expect_success 'iteration of dir w/ two dirs each w/ file' '\n> +test_expect_success 'dirs-ignore of dir w/ two dirs each w/ file' '\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\ttest_cmp expected-ignore-out1 actual-out ||\n> +\t\ttest_cmp expected-ignore-out2 actual-out\n> +\t)\n> +'\n> +test_expect_success 'dirs-before of dir w/ two dirs each w/ file' '\n> +\ttest-tool dir-iterator --dirs-before ./dir8 >actual-out &&\n> +\t(\n> +\t\ttest_cmp expected-before-out1 actual-out ||\n> +\t\ttest_cmp expected-before-out2 actual-out\n> +\t)\n> +'\n> +test_expect_success 'dirs-after of dir w/ two dirs each w/ file' '\n> +\ttest-tool dir-iterator --dirs-after ./dir8 >actual-out &&\n> +\t(\n> +\t\ttest_cmp expected-after-out1 actual-out ||\n> +\t\ttest_cmp expected-after-out2 actual-out\n> +\t)\n> +'\n> +test_expect_success 'dirs-before/dirs-after of dir w/ two dirs each w/ file' '\n> +\ttest-tool dir-iterator --dirs-before --dirs-after ./dir8 >actual-out &&\n> +\t(\n> +\t\ttest_cmp expected-before-after-out1 actual-out ||\n> +\t\ttest_cmp expected-before-after-out2 actual-out\n>   \t)\n>   '\n>   \n> @@ -183,44 +524,164 @@ test_expect_success 'setup -- dir w/ two dirs, one w/ two and one w/ one files'\n>   \t>dir9/d/e &&\n>   \n>   \n> -\tcat >expected-out1 <<-EOF &&\n> +\tcat >expected-ignore-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-ignore-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-ignore-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-ignore-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> +\tcat >expected-before-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> +\tcat >expected-before-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> +\tcat >expected-before-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> +\tcat >expected-before-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> +\tcat >expected-after-out1 <<-EOF &&\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-after-out2 <<-EOF &&\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> +\tcat >expected-after-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-after-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> +\tcat >expected-before-after-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-before-after-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-before-after-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-before-after-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>   test_expect_success \\\n> -'iteration of dir w/ three dirs, one w/ two, one w/ one and one w/ none files' '\n> +'dirs-ignore of dir w/ three dirs, one w/ two, one w/ one and one w/ none files' '\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\ttest_cmp expected-ignore-out1 actual-out ||\n> +\t\ttest_cmp expected-ignore-out2 actual-out ||\n> +\t\ttest_cmp expected-ignore-out3 actual-out ||\n> +\t\ttest_cmp expected-ignore-out4 actual-out\n> +\t)\n> +'\n> +test_expect_success \\\n> +'dirs-before of dir w/ three dirs, one w/ two, one w/ one and one w/ none files' '\n> +\n> +\ttest-tool dir-iterator --dirs-before ./dir9 >actual-out &&\n> +\t(\n> +\t\ttest_cmp expected-before-out1 actual-out ||\n> +\t\ttest_cmp expected-before-out2 actual-out ||\n> +\t\ttest_cmp expected-before-out3 actual-out ||\n> +\t\ttest_cmp expected-before-out4 actual-out\n> +\t)\n> +'\n> +test_expect_success \\\n> +'dirs-after of dir w/ three dirs, one w/ two, one w/ one and one w/ none files' '\n> +\n> +\ttest-tool dir-iterator --dirs-after ./dir9 >actual-out &&\n> +\t(\n> +\t\ttest_cmp expected-after-out1 actual-out ||\n> +\t\ttest_cmp expected-after-out2 actual-out ||\n> +\t\ttest_cmp expected-after-out3 actual-out ||\n> +\t\ttest_cmp expected-after-out4 actual-out\n> +\t)\n> +'\n> +test_expect_success \\\n> +'dirs-before/dirs-after of dir w/ three dirs, one w/ two, one w/ one and one w/ none files' '\n> +\n> +\ttest-tool dir-iterator --dirs-before --dirs-after ./dir9 >actual-out &&\n> +\t(\n> +\t\ttest_cmp expected-before-after-out1 actual-out ||\n> +\t\ttest_cmp expected-before-after-out2 actual-out ||\n> +\t\ttest_cmp expected-before-after-out3 actual-out ||\n> +\t\ttest_cmp expected-before-after-out4 actual-out\n>   \t)\n>   '\n>   \n> @@ -231,25 +692,84 @@ test_expect_success 'setup -- dir w/ two nested dirs, each w/ file' '\n>   \t>dir10/a/c/d &&\n>   \n>   \n> -\tcat >expected-out1 <<-EOF &&\n> +\tcat >expected-ignore-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-ignore-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> +\tcat >expected-before-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> +\tcat >expected-before-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> +\tcat >expected-after-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-after-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> +\tcat >expected-before-after-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-before-after-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> -test_expect_success 'iteration of dir w/ two nested dirs, each w/ file' '\n> +test_expect_success 'dirs-ignore of dir w/ two nested dirs, each w/ file' '\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\ttest_cmp expected-ignore-out1 actual-out ||\n> +\t\ttest_cmp expected-ignore-out2 actual-out\n> +\t)\n> +'\n> +test_expect_success 'dirs-before of dir w/ two nested dirs, each w/ file' '\n> +\ttest-tool dir-iterator --dirs-before ./dir10/ >actual-out &&\n> +\t(\n> +\t\ttest_cmp expected-before-out1 actual-out ||\n> +\t\ttest_cmp expected-before-out2 actual-out\n> +\t)\n> +'\n> +test_expect_success 'dirs-after of dir w/ two nested dirs, each w/ file' '\n> +\ttest-tool dir-iterator --dirs-after ./dir10/ >actual-out &&\n> +\t(\n> +\t\ttest_cmp expected-after-out1 actual-out ||\n> +\t\ttest_cmp expected-after-out2 actual-out\n> +\t)\n> +'\n> +test_expect_success 'dirs-before/dirs-after of dir w/ two nested dirs, each w/ file' '\n> +\ttest-tool dir-iterator --dirs-before --dirs-after ./dir10 >actual-out &&\n> +\t(\n> +\t\ttest_cmp expected-before-after-out1 actual-out ||\n> +\t\ttest_cmp expected-before-after-out2 actual-out\n>   \t)\n>   '\n>   \n> @@ -263,7 +783,15 @@ test_expect_success 'setup -- dir w/ complex structure w/o symlinks' '\n>   \t>dir11/d/e/d/a &&\n>   \n>   \n> -\tcat >expected-sorted-out <<-EOF\n> +\tcat >expected-ignore-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> +\tcat >expected-before-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> @@ -276,12 +804,52 @@ test_expect_success 'setup -- dir w/ complex structure w/o symlinks' '\n>   \t[f] (c) [c] ./dir11/c\n>   \t[f] (d/e/d/a) [a] ./dir11/d/e/d/a\n>   \tEOF\n> +\n> +\tcat expected-before-sorted-out >expected-after-sorted-out &&\n> +\n> +\tcat >expected-before-after-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> -test_expect_success 'iteration of dir w/ complex structure w/o symlinks' '\n> +test_expect_success 'dirs-ignore of dir w/ complex structure w/o symlinks' '\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> +\ttest_cmp expected-ignore-sorted-out actual-sorted-out\n> +'\n> +test_expect_success 'dirs-before of dir w/ complex structure w/o symlinks' '\n> +\ttest-tool dir-iterator --dirs-before ./dir11 >actual-out &&\n> +\tsort actual-out >actual-sorted-out &&\n> +\n> +\ttest_cmp expected-before-sorted-out actual-sorted-out\n> +'\n> +test_expect_success 'dirs-after of dir w/ complex structure w/o symlinks' '\n> +\ttest-tool dir-iterator --dirs-after ./dir11 >actual-out &&\n> +\tsort actual-out >actual-sorted-out &&\n> +\n> +\ttest_cmp expected-after-sorted-out actual-sorted-out\n> +'\n> +test_expect_success 'dirs-before/dirs-after of dir w/ complex structure w/o symlinks' '\n> +\ttest-tool dir-iterator --dirs-before --dirs-after ./dir11 >actual-out &&\n> +\tsort actual-out >actual-sorted-out &&\n> +\n> +\ttest_cmp expected-before-after-sorted-out actual-sorted-out\n>   '\n>   \n>   test_expect_success POSIXPERM,SANITY \\\n> @@ -338,7 +906,7 @@ test_expect_success POSIXPERM,SANITY \\\n>   \t[d] (a) [a] ./dir13/a\n>   \tEOF\n>   \n> -\ttest-tool dir-iterator ./dir13 >actual-out &&\n> +\ttest-tool dir-iterator --dirs-before ./dir13 >actual-out &&\n>   \ttest_cmp expected-no-permissions-out actual-out &&\n>   \n>   \tchmod 755 dir13/a &&\n> @@ -358,7 +926,8 @@ test_expect_success POSIXPERM,SANITY \\\n>   \tdir_iterator_advance failure\n>   \tEOF\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-no-permissions-pedantic-out actual-out &&\n>   \n>   \tchmod 755 dir13/a &&\n> @@ -373,7 +942,17 @@ test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/o cycle' '\n>   \tln -s ../b dir14/a/f &&\n>   \n>   \n> -\tcat >expected-dont-follow-sorted-out <<-EOF &&\n> +\tcat >expected-dont-follow-ignore-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> +\tcat >expected-follow-ignore-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> +\tcat >expected-dont-follow-before-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> @@ -381,7 +960,7 @@ test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/o cycle' '\n>   \t[s] (a/e) [e] ./dir14/a/e\n>   \t[s] (a/f) [f] ./dir14/a/f\n>   \tEOF\n> -\tcat >expected-follow-sorted-out <<-EOF\n> +\tcat >expected-follow-before-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> @@ -390,22 +969,99 @@ test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/o cycle' '\n>   \t[f] (a/d) [d] ./dir14/a/d\n>   \t[f] (a/e) [e] ./dir14/a/e\n>   \tEOF\n> +\n> +\tcat expected-dont-follow-before-sorted-out >expected-dont-follow-after-sorted-out &&\n> +\tcat expected-follow-before-sorted-out >expected-follow-after-sorted-out &&\n> +\n> +\tcat >expected-dont-follow-before-after-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> +\tcat >expected-follow-before-after-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>   test_expect_success SYMLINKS \\\n> -'dont-follow-symlinks of dir w/ symlinks w/o cycle' '\n> +'dont-follow-symlinks dirs-ignore of dir w/ symlinks w/o cycle' '\n>   \n>   \ttest-tool dir-iterator ./dir14 >actual-out &&\n>   \tsort actual-out >actual-sorted-out &&\n>   \n> -\ttest_cmp expected-dont-follow-sorted-out actual-sorted-out\n> +\ttest_cmp expected-dont-follow-ignore-sorted-out actual-sorted-out\n>   '\n>   test_expect_success SYMLINKS \\\n> -'follow-symlinks of dir w/ symlinks w/o cycle' '\n> +'follow-symlinks dirs-ignore of dir w/ symlinks w/o cycle' '\n>   \n>   \ttest-tool dir-iterator --follow-symlinks ./dir14 >actual-out &&\n>   \tsort actual-out >actual-sorted-out &&\n>   \n> -\ttest_cmp expected-follow-sorted-out actual-sorted-out\n> +\ttest_cmp expected-follow-ignore-sorted-out actual-sorted-out\n> +'\n> +test_expect_success SYMLINKS \\\n> +'dont-follow-symlinks dirs-before of dir w/ symlinks w/o cycle' '\n> +\n> +\ttest-tool dir-iterator --dirs-before ./dir14 >actual-out &&\n> +\tsort actual-out >actual-sorted-out &&\n> +\n> +\ttest_cmp expected-dont-follow-before-sorted-out actual-sorted-out\n> +'\n> +test_expect_success SYMLINKS \\\n> +'follow-symlinks dirs-before of dir w/ symlinks w/o cycle' '\n> +\n> +\ttest-tool dir-iterator --dirs-before --follow-symlinks ./dir14 >actual-out &&\n> +\tsort actual-out >actual-sorted-out &&\n> +\n> +\ttest_cmp expected-follow-before-sorted-out actual-sorted-out\n> +'\n> +test_expect_success SYMLINKS \\\n> +'dont-follow-symlinks dirs-after of dir w/ symlinks w/o cycle' '\n> +\n> +\ttest-tool dir-iterator --dirs-after ./dir14 >actual-out &&\n> +\tsort actual-out >actual-sorted-out &&\n> +\n> +\ttest_cmp expected-dont-follow-after-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> +\ttest-tool dir-iterator --dirs-after --follow-symlinks ./dir14 >actual-out &&\n> +\tsort actual-out >actual-sorted-out &&\n> +\n> +\ttest_cmp expected-follow-after-sorted-out actual-sorted-out\n> +'\n> +test_expect_success SYMLINKS \\\n> +'dont-follow-symlinks dirs-before/dirs-after of dir w/ symlinks w/o cycle' '\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-dont-follow-before-after-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> +\ttest-tool dir-iterator --dirs-before --dirs-after --follow-symlinks ./dir14 >actual-out &&\n> +\tsort actual-out >actual-sorted-out &&\n> +\n> +\ttest_cmp expected-follow-before-after-sorted-out actual-sorted-out\n>   '\n>   \n>   test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/ cycle' '\n> @@ -415,7 +1071,14 @@ 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> -\tcat >expected-dont-follow-sorted-out <<-EOF &&\n> +\n> +\tcat >expected-dont-follow-ignore-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> +\tcat >expected-dont-follow-before-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> @@ -424,22 +1087,87 @@ test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/ cycle' '\n>   \t[s] (a/b/f) [f] ./dir15/a/b/f\n>   \tEOF\n>   \n> +\tcat expected-dont-follow-before-sorted-out >expected-dont-follow-after-sorted-out &&\n> +\n> +\tcat >expected-dont-follow-before-after-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>   \tcat >expected-pedantic-follow-tailed-out <<-EOF\n>   \tdir_iterator_advance failure\n>   \tEOF\n>   '\n>   test_expect_success SYMLINKS \\\n> -'dont-follow-symlinks of dir w/ symlinks w/ cycle' '\n> +'dont-follow-symlinks dirs-ignore of dir w/ symlinks w/ cycle' '\n>   \n>   \ttest-tool dir-iterator ./dir15 >actual-out &&\n>   \tsort actual-out >actual-sorted-out &&\n>   \n> -\ttest_cmp expected-dont-follow-sorted-out actual-sorted-out\n> +\ttest_cmp expected-dont-follow-ignore-sorted-out actual-sorted-out\n>   '\n>   test_expect_success SYMLINKS \\\n> -'pedantic follow-symlinks of dir w/ symlinks w/ cycle' '\n> +'pedantic follow-symlinks dirs-ignore of dir w/ symlinks w/ cycle' '\n>   \n>   \ttest_must_fail test-tool dir-iterator \\\n> +\t\t--follow-symlinks --pedantic ./dir15 >actual-out &&\n> +\ttail -n 1 actual-out >actual-tailed-out &&\n> +\n> +\ttest_cmp expected-pedantic-follow-tailed-out actual-tailed-out\n> +'\n> +test_expect_success SYMLINKS \\\n> +'dont-follow-symlinks dirs-before of dir w/ symlinks w/ cycle' '\n> +\n> +\ttest-tool dir-iterator --dirs-before ./dir15 >actual-out &&\n> +\tsort actual-out >actual-sorted-out &&\n> +\n> +\ttest_cmp expected-dont-follow-before-sorted-out actual-sorted-out\n> +'\n> +test_expect_success SYMLINKS \\\n> +'pedantic follow-symlinks dirs-before of dir w/ symlinks w/ cycle' '\n> +\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> +\ttest_cmp expected-pedantic-follow-tailed-out actual-tailed-out\n> +'\n> +test_expect_success SYMLINKS \\\n> +'dont-follow-symlinks dirs-after of dir w/ symlinks w/ cycle' '\n> +\n> +\ttest-tool dir-iterator --dirs-after ./dir15 >actual-out &&\n> +\tsort actual-out >actual-sorted-out &&\n> +\n> +\ttest_cmp expected-dont-follow-after-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> +\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-pedantic-follow-tailed-out actual-tailed-out\n> +'\n> +test_expect_success SYMLINKS \\\n> +'dont-follow-symlinks dirs-before/dirs-after of dir w/ symlinks w/ cycle' '\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-dont-follow-before-after-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> +\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\n"},{"id":"453408","messageId":"df287d4f-e9da-4ce0-d7e9-1b1fe7671aab@gmail.com","threadId":"57706","inReplyTo":"20220410111852.2097418-1-kioplato@gmail.com","subject":"Re: [RFC PATCH 0/6][GSoC] iterate dirs before or after their contents","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-04-11T13:37:00Z","receivedAt":"2022-04-11T13:37:13Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Plato\n\nI think this would be a useful addition to git, thanks for working on \nit. I've left some comments on the individual patches. In general I \nthink this series would benefit from trying to work with the existing \ncode and change one aspect at a time in stages rather than rewriting \nlarge chunks all at once.\n\nBest Wishes\n\nPhillip\n\nOn 10/04/2022 12:18, Plato Kiorpelidis wrote:\n> Hello. Some time ago I worked on converting opendir/readdir calls to use\n> dir-iterator [1], however this cleanup required to iterate directory paths after\n> their contents. Matheus pointed me out to a previous patch series[2] that\n> attempted to implement such functionality in dir-iterator. From it, I mainly\n> used Michael's feedback and feature requests and tried to include it in my work.\n> \n> My fork: https://github.com/kioplato/git/tree/dir-iterator\n> CI: https://github.com/kioplato/git/actions/runs/2141288008\n> There are some memleaks, I'll track them down in v2.\n> \n> I aim to implement more functionality in dir-iterator, my goal being to simplify\n> the codebase by introducing an abstraction layer for iterating directories.\n> I would like to eventually simplify read_directory_recursive(). I wanted to\n> check in with you to make sure I'm heading in the right direction with what I've\n> implemented.\n>    * Are my tests overly exhaustive?\n>    * As of now we can't thoroughly test dir-iterator on directories with complex\n>    structure since readdir produce dirents with undefined order in a directory.\n>    I thought about introducing a tool for generating permutations with stable\n>    parts in test-lib. Is there a need to something like this for other tests?\n>    Or maybe should I sort each level iterated by dir-iterator inside\n>    test-dir-iterator before printing to stdout? In these patches I did enumerate\n>    the path permutations for some tests by hand, but that's not viable really.\n>    * We also don't test for deleted entries between dir_iterator_advance() calls.\n>    * Are my comments too much? Throughout git, .c files don't have many comments,\n>    should I remove mine as well? I think they provide better context when reading\n>    through the source code.\n> \n> I do understand that it probably is too early to worry about most of these.\n> However I wanted to communicate my thoughts and setup for the following\n> versions.\n> \n> While I wait for review, I'll implement/fix:\n>    * DIR_ITERATOR_RECURSE proposed here[3], but with finer control. Instead of a\n>    flag I'll introduce a new integer parameter in dir_iterator_begin(), which\n>    will specify the maximum iteration depth.\n>      * Supplying 0 will have the same behavior as DIR_ITERATOR_RECURSE i.e. it\n>      will iterate over the entries of the root directory.\n>      * Supplying -1 will iterate to maximum depth. This is the default right now.\n>    * DIR_ITERATOR_DIRS_ONLY proposed here[4]. Enabling this, dir-iterator will\n>    show only directories. Failing to enable DIR_ITERATOR_DIRS_BEFORE and/or\n>    DIR_ITERATOR_DIRS_AFTER will result in dir_iterator_begin() returning NULL.\n>    Is this a good way to encode \"show only directories\" in the flags?\n> \n> I'll include them along with feedback and suggestions from this version in the\n> next one.\n> \n> I didn't refactor entry.c to use dir-iterator. It's a good first issue for\n> someone else to become introduced to the community. I applied my patch[1] and it\n> does not pass t/, as it used to, because of 15th test in t1092. Should I work on\n> entry.c in my next version or leave it alone for a newcomer?\n> \n> This serves as my microproject for GSoC. Could my future work on dir-iterator\n> and cleanup of read_directory_recursive() and other customers of dir-iterator\n> become a seperate GSoC project I could undertake?\n> \n> [1]: https://public-inbox.org/git/20191208180439.19018-1-otalpster@gmail.com/\n> [2]: https://public-inbox.org/git/1493226219-33423-1-git-send-email-bnmvco@gmail.com/\n> [3]: https://public-inbox.org/git/CACsJy8DBa-oH3i+5P=iVr9NhJwsicZ43DO89WmvpYEQu90RrMw@mail.gmail.com/\n> [4]: https://public-inbox.org/git/xmqqmvc265hk.fsf@gitster.mtv.corp.google.com/\n> \n> Plato Kiorpelidis (6):\n>    t0066: improve readablity of dir-iterator tests\n>    t0066: better test coverage for dir-iterator\n>    dir-iterator: refactor dir_iterator_advance()\n>    dir-iterator: iterate dirs before or after their contents\n>    t0066: remove redundant tests\n>    test-dir-iterator: handle EACCES errno by dir-iterator\n> \n>   builtin/clone.c              |    4 +-\n>   dir-iterator.c               |  302 ++++++---\n>   dir-iterator.h               |   34 +-\n>   refs/files-backend.c         |    2 +-\n>   t/helper/test-dir-iterator.c |   23 +-\n>   t/t0066-dir-iterator.sh      | 1202 +++++++++++++++++++++++++++++++---\n>   6 files changed, 1371 insertions(+), 196 deletions(-)\n> \n\n"},{"id":"453409","messageId":"9e5495d0-c117-2242-6718-46042e9664bf@gmail.com","threadId":"57706","inReplyTo":"220411.86o817j2dt.gmgdl@evledraar.gmail.com","subject":"Re: [RFC PATCH 3/6] dir-iterator: refactor dir_iterator_advance()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-04-11T13:40:24Z","receivedAt":"2022-04-11T13:40:43Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 11/04/2022 12:11, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Sun, Apr 10 2022, Plato Kiorpelidis wrote:\n> \n>> +\n>> +\terrno = 0;\n>> +\tdir_entry = readdir(level->dir);\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>> +\tif (dir_entry == NULL) {\n> \n> Don't compare against NULL, use !dir_entry.\n> \n> Also: Manually resetting \"errno\" before isn't needed. It will be (re)set\n> if readdir() returns NULL().\n\nThat's not what the man page says\n\n     If the end of the directory stream is reached, NULL is returned \n\n     and errno is not changed.  If an error occurs, NULL is returned \n\n     and errno is set to indicate the error.  To distinguish end of \n\n     stream from an error, set errno to zero before calling readdir() \n\n     and then check the value of errno if NULL is returned. \n\n\nBest Wishes\n\nPhillip\n\n"},{"id":"453925","messageId":"CAO2gv81bM2srW0jxFwYQQ6xbt3ZrExNjf7_Y2Dxyfydc71r27w@mail.gmail.com","threadId":"57706","inReplyTo":"df287d4f-e9da-4ce0-d7e9-1b1fe7671aab@gmail.com","subject":"Re: [RFC PATCH 0/6][GSoC] iterate dirs before or after their contents","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-04-19T13:06:44Z","receivedAt":"2022-04-19T13:07:26Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"On Mon, Apr 11, 2022 at 4:37 PM Phillip Wood <phillip.wood123@gmail.com> wrote:\n>\n> Hi Plato\n\nHey Phillip,\n\nThanks for reviewing my patches, your input is welcomed! :)\nI'll reply your reviews and Ævar's in the following days. I need some\ntime to read dir-iterator again in combination with your comments.\n\n>\n> I think this would be a useful addition to git, thanks for working on\n> it. I've left some comments on the individual patches. In general I\n> think this series would benefit from trying to work with the existing\n> code and change one aspect at a time in stages rather than rewriting\n> large chunks all at once.\n\nAgreed, I'll change one aspect at a time. First, I'll include\nsuggested changes from this version and also convert entry.c to use\ndir-iterator instead of opendir, readdir, closedir API and submit v2.\nThen I'll spend some time understanding read_directory_recursive() and\nsearch for possible parts that can be cleaned up using dir-iterator.\nI'll compile a set of possible customer candidates for dir-iterator as\nreference. This will take some time since I'll prepare for the\nupcoming GSoC as a possible contributor.\n\nThanks,\nPlato\n\n>\n> Best Wishes\n>\n> Phillip\n>\n> On 10/04/2022 12:18, Plato Kiorpelidis wrote:\n> [...]\n"},{"id":"454361","messageId":"20220424192538.q3gazpbmxh27svmg@compass","threadId":"57706","inReplyTo":"ed6656e0-a865-319e-0f56-23ab1da3eef3@gmail.com","subject":"Re: [RFC PATCH 1/6] t0066: improve readablity of dir-iterator tests","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-04-24T19:25:38Z","receivedAt":"2022-04-24T19:25:55Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"On 22/04/11 02:16PM, Phillip Wood wrote:\n> Hi Plato\n> \n> On 10/04/2022 12:18, Plato Kiorpelidis wrote:\n> > Be consistent throughout the dir-iterator tests regarding the order in\n> > which we:\n> >    * create test directories\n> >    * create expected outputs\n> >    * test if actual and expected outputs differ\n> \n> When writing a commit message it is important to explain why you are making\n> the changes in the patch, rather than just describe the changes themselves.\n> \n> In general modernizing or standardizing existing tests before adding new\n> ones is a good idea. However it is important to do it in a way that lets\n> reviewers see there are no unintended changes. In this patch so much is\n> changed it's really hard to tell if you have changed any of the tests or\n> not. It would be much easier if you had not renamed all the directories and\n> files that are created and renamed the tests as well.\n> \n> I think that having separate setup tests makes sense as we're going to want\n> to test different iteration schemes over the same hierarchy but it would be\n> helpful to have the \"expect\" files written in the same test that calls\n> test_cmp. That way it is much easier to debug test failures in the CI as it\n> one can see the contents of \"expect\" as well as the diff from test_cmp in\n> the CI output.\n\nI agree. All three points have been adopted in the upcoming v2:\n1. Explain _why_ I'm making these changes.\n2. Create targeted commits with smaller diffs. (This commit has been splitted)\n3. Write \"expected-*\" files in the same test that calls test_cmp.\n\nThanks,\nPlato\n\n> \n> Best Wishes\n> \n> Phillip\n> \n> > Signed-off-by: Plato Kiorpelidis <kioplato@gmail.com>\n> > ---\n> >   t/t0066-dir-iterator.sh | 227 +++++++++++++++++++++-------------------\n> >   1 file changed, 118 insertions(+), 109 deletions(-)\n> > \n> > diff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\n> > index 63a1a45cd3..fb20219487 100755\n> > --- a/t/t0066-dir-iterator.sh\n> > +++ b/t/t0066-dir-iterator.sh\n> > @@ -5,145 +5,154 @@ test_description='Test the dir-iterator functionality'\n> >   TEST_PASSES_SANITIZE_LEAK=true\n> >   . ./test-lib.sh\n> > -test_expect_success 'setup' '\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> > -\tmkdir -p dir2/a/b/c/ &&\n> > -\t>dir2/a/b/c/d\n> > +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> > +\n> > +\tcat >expected-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> > +\tEOF\n> > +'\n> > +test_expect_success 'iteration of dir w/ three nested dirs w/ file' '\n> > +\ttest-tool dir-iterator ./dir6 >actual-out &&\n> > +\ttest_cmp expected-out actual-out\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> > -\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> > +test_expect_success 'setup -- dir w/ complex structure w/o symlinks' '\n> > +\tmkdir -p dir11/a/b/c/ &&\n> > +\t>dir11/b &&\n> > +\t>dir11/c &&\n> > +\t>dir11/a/e &&\n> > +\t>dir11/a/b/c/d &&\n> > +\tmkdir -p dir11/d/e/d/ &&\n> > +\t>dir11/d/e/d/a &&\n> > +\n> > +\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> > +test_expect_success 'iteration of dir w/ complex structure w/o symlinks' '\n> > +\ttest-tool dir-iterator ./dir11 >actual-out &&\n> > +\tsort actual-out >actual-sorted-out &&\n> > -\ttest-tool dir-iterator ./dir >out &&\n> > -\tsort out >./actual-iteration-sorted-output &&\n> > +\ttest_cmp expected-sorted-out actual-sorted-out\n> > +'\n> > -\ttest_cmp expected-iteration-sorted-output actual-iteration-sorted-output\n> > +test_expect_success 'dir_iterator_begin() should fail upon inexistent paths' '\n> > +\techo \"dir_iterator_begin failure: ENOENT\" >expected-inexistent-path-out &&\n> > +\n> > +\ttest_must_fail test-tool dir-iterator ./inexistent-path >actual-out &&\n> > +\ttest_cmp expected-inexistent-path-out actual-out\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> > -\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> > +test_expect_success 'dir_iterator_begin() should fail upon non directory paths' '\n> > +\t>some-file &&\n> > -\ttest-tool dir-iterator ./dir2 >actual-pre-order-output &&\n> > -\ttest_cmp expected-pre-order-output actual-pre-order-output\n> > -'\n> > +\techo \"dir_iterator_begin failure: ENOTDIR\" >expected-non-dir-out &&\n> > -test_expect_success 'begin should fail upon inexistent paths' '\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> > -\ttest_cmp expected-inexistent-path-output actual-inexistent-path-output\n> > -'\n> > +\ttest_must_fail test-tool dir-iterator ./some-file >actual-out &&\n> > +\ttest_cmp expected-non-dir-out actual-out &&\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> > -\ttest_cmp expected-non-dir-output actual-non-dir-output\n> > +\ttest_must_fail test-tool dir-iterator --pedantic ./some-file >actual-out &&\n> > +\ttest_cmp expected-non-dir-out actual-out\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> > +test_expect_success POSIXPERM,SANITY \\\n> > +'dir_iterator_advance() should not fail on errors by default' '\n> > -\tmkdir -p dir3/a &&\n> > -\t>dir3/a/b &&\n> > -\tchmod 0 dir3/a &&\n> > +\tmkdir -p dir13/a &&\n> > +\t>dir13/a/b &&\n> > +\tchmod 0 dir13/a &&\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> > -'\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> > +\tcat >expected-no-permissions-out <<-EOF &&\n> > +\t[d] (a) [a] ./dir13/a\n> >   \tEOF\n> > -\tmkdir -p dir3/a &&\n> > -\t>dir3/a/b &&\n> > -\tchmod 0 dir3/a &&\n> > +\ttest-tool dir-iterator ./dir13 >actual-out &&\n> > +\ttest_cmp expected-no-permissions-out actual-out &&\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 dir13/a &&\n> > +\trm -rf dir13\n> >   '\n> > -test_expect_success SYMLINKS 'setup dirs with symlinks' '\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> > -'\n> > +test_expect_success POSIXPERM,SANITY \\\n> > +'dir_iterator_advance() should fail on errors, w/ pedantic flag' '\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> > -\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> > +\tmkdir -p dir13/a &&\n> > +\t>dir13/a/b &&\n> > +\tchmod 0 dir13/a &&\n> > +\n> > +\n> > +\tcat >expected-no-permissions-pedantic-out <<-EOF &&\n> > +\t[d] (a) [a] ./dir13/a\n> > +\tdir_iterator_advance failure\n> >   \tEOF\n> > -\ttest-tool dir-iterator ./dir4 >out &&\n> > -\tsort out >actual-no-follow-sorted-output &&\n> > +\ttest_must_fail test-tool dir-iterator --pedantic ./dir13 >actual-out &&\n> > +\ttest_cmp expected-no-permissions-pedantic-out actual-out &&\n> > -\ttest_cmp expected-no-follow-sorted-output actual-no-follow-sorted-output\n> > +\tchmod 755 dir13/a &&\n> > +\trm -rf dir13\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> > -\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> > +test_expect_success SYMLINKS 'setup -- dir w/ symlinks w/o cycle' '\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> > +\n> > +\tcat >expected-dont-follow-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> > +\tcat >expected-follow-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> > +test_expect_success SYMLINKS \\\n> > +'dont-follow-symlinks of dir w/ symlinks w/o cycle' '\n> > +\n> > +\ttest-tool dir-iterator ./dir14 >actual-out &&\n> > +\tsort actual-out >actual-sorted-out &&\n> > +\n> > +\ttest_cmp expected-dont-follow-sorted-out actual-sorted-out\n> > +'\n> > +test_expect_success SYMLINKS \\\n> > +'follow-symlinks of dir w/ symlinks w/o cycle' '\n> > -\ttest-tool dir-iterator --follow-symlinks ./dir4 >out &&\n> > -\tsort out >actual-follow-sorted-output &&\n> > +\ttest-tool dir-iterator --follow-symlinks ./dir14 >actual-out &&\n> > +\tsort actual-out >actual-sorted-out &&\n> > -\ttest_cmp expected-follow-sorted-output actual-follow-sorted-output\n> > +\ttest_cmp expected-follow-sorted-out actual-sorted-out\n> >   '\n> >   test_done\n> \n"},{"id":"454498","messageId":"20220427143206.6cqksztbk4exift3@compass","threadId":"57706","inReplyTo":"35160d46-d337-2110-f968-238abb7e9f0e@gmail.com","subject":"Re: [RFC PATCH 3/6] dir-iterator: refactor dir_iterator_advance()","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-04-27T14:32:06Z","receivedAt":"2022-04-27T14:32:20Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"On 22/04/11 02:26PM, Phillip Wood wrote:\n> Hi Plato\n> \n> On 10/04/2022 12:18, Plato Kiorpelidis wrote:\n> > Simplify dir_iterator_advance by switch from iterative to recursive\n> > implementation. In each recursive step one action is performed.\n> > \n> > This makes dir-iterator easier to work with, understand and introduce\n> > new functionality.\n> \n> I think that this message could explain a bit more about why it is necessary\n> to use a recursive approach. As we are implementing an iterator we cannot\n> keep state in the stack between iterations so how does using recursive\n> approach help us?\n\nIt's not necessary from a functioning perspective. This iterator works on tree\nlike structures (exception cyclic symlinks), performing pre-order and post-order\ntraversal on some vertices, in our case directories. Traditionally, these\noperations, i.e. pre-order, in-order, post-order, in textbooks, lectures and\nonline sources are implemented recursively and not iteratively. Therefore it\nhelps reduce mental load for the reader, since it's easier to follow as it\nreminds of the same code paradigm, e.g. post-order:\n  * Recurse left child\n  * Recurse right child\n  * Visit subroot\n\nIn general, graph algorithms are more readable when implemented recursively.\nIn each recursive step a set of instructions are executed and recursion lays out\nthe code in such a way that these instructions stand out symmetrically for each\nrecursive step.\n\nAs a result, recursive dir-iterator is more maintainable, as its code structure\nis more similar to other graph algorithms and to what we're already used to,\nthan its iterative implementation.\n\nA nice side effect of recursion is that it requires one less indentation depth.\nThis further simplifies the code.\n\n> I reviewed this series as a way to learn the directory iterator code as I'd\n> not looked at it before. Having read the existing code I think the it is\n> fairly easy to follow. I'm not sure the changes here improve that. I also\n> found these changes harder to follow because they renamed existing functions\n> and variables in addition to moving the code around.\n\nMichael Haggerty said that dir-iterator code is quite intricate[1], I agree and\ntried to simplify it. My conversion from iterative to recursive implementation\ntries to deal with this problem. The proposed advantages are specified above.\n\n> In order in implement returning the directory name after its contents I\n> think all we need is to remember whether we've returned the name when we\n> readdir() returns NULL so we don't pop it too soon. It's not clear that we\n> need a lot of refactoring to do that, especially as the patches you linked\n> to in the cover letter seem to avoid that.\n\nIt's one more thing we need to keep track of, that's not required and can be\navoided. It further tangles the code, introducing more cases. The recursive\nimplementation that I propose, does not need to keep track of this information.\nProbably there's an equal iterative implementation that also does not need to\nkeep track of this information.\n\n\n[1]: https://public-inbox.org/git/7a665631-da6a-4b9f-b9e7-750f2504eccd@alum.mit.edu/\n\nThanks,\nPlato\n\n> Best Wishes\n> \n> Phillip\n> \n> > [...]\n"},{"id":"454499","messageId":"20220427145711.uwoa6n7kgq4okgr7@compass","threadId":"57706","inReplyTo":"b75aaee8-c037-e8e0-6ee0-729922059352@gmail.com","subject":"Re: [RFC PATCH 4/6] dir-iterator: iterate dirs before or after their contents","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-04-27T14:57:11Z","receivedAt":"2022-04-27T14:57:30Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"On 22/04/11 02:31PM, Phillip Wood wrote:\n> Hi Plato\n> \n> On 10/04/2022 12:18, Plato Kiorpelidis wrote:\n> > Introduce a new feature to dir-iterator, using dir_iterator_begin()\n> > flags parameter, allowing to control whether or not directories will\n> > appear before or after their contents.\n> > \n> > The new default behavior (i.e. flags set to 0) does not iterate over\n> > directories.\n> \n> This is introducing two changes at once adding the \"AFTER\" flag and changing\n> the default behavior. I think it would make sence to separate those into two\n> separate commits. You could change the default adding the \"BEFORE\" flag and\n> adjusting the existing callers first and then implement the \"AFTER\" flag in\n> a second commit.\n\nNice, I'll split this commit into two, like you suggest.\n\n> It would also be nice to see the \"AFTER\" flag being used by changing\n> remove_dir_recursively() for example.\n\nI'll make use of \"AFTER\" flag to convert remove_subtree() function in entry.c\nand include the conversion in v2.\n\nTackling remove_dir_recursively() will come later.\n\nThanks Phillip,\nPlato\n\n> Best Wishes\n> \n> Phillip\n> \n> > [...]\n"},{"id":"454501","messageId":"20220427154526.uuhpkoee322l7kmz@compass","threadId":"57706","inReplyTo":"220411.86o817j2dt.gmgdl@evledraar.gmail.com","subject":"Re: [RFC PATCH 3/6] dir-iterator: refactor dir_iterator_advance()","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-04-27T15:45:26Z","receivedAt":"2022-04-27T15:46:28Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"On 22/04/11 01:11PM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Sun, Apr 10 2022, Plato Kiorpelidis wrote:\n> \n> > Simplify dir_iterator_advance by switch from iterative to recursive\n> > implementation. In each recursive step one action is performed.\n> >\n> > This makes dir-iterator easier to work with, understand and introduce\n> > new functionality.\n> >\n> > Signed-off-by: Plato Kiorpelidis <kioplato@gmail.com>\n> > ---\n> >\n> > [...]\n> >\n> > @@ -45,34 +45,53 @@ struct dir_iterator_int {\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)) return 0;\n> \n> style: missing \\n before \"return\".\n> \n> Also, the one existing caller before this patch is:\n> \n>     if (S_ISDIR(iter->base.st.st_mode) && push_level(iter))\n> \n> Why not continue to do that?\n\nIn this patch I also fixed a problem that's subtle. Previously, when we called\ndir_iterator_begin() and the specified path is invalid the call won't return\nNULL. Instead the call succeeds and the first call to dir_iterator_advance()\nfails. I believe that's unexpected. The expected behavior would be to return\nNULL from dir_iterator_begin() if the specified path is invalid. Successful call\nto dir_iterator_begin() suggests that the root path is valid.\n\nTo deal with that I introduced two states for the most recent directory. The\nfirst is just pushed into the levels stack and the other pushed and activated.\nThis way we can \"activate\" the root directory in dir_iterator_begin(), but we\nalso need to reorder the calls to push_level() and activate_level() as a result.\nWe need to push directories after we read them and activate the most recent one\nwhen dir_iterator_advance() is called.\n\nI'll document that change in the related commit in v2.\n\nDo you have any objection about this change?\n\n> > +/*\n> > + * Activate most recent pushed level.\n> > + *\n> > + * Return 1 on success.\n> > + * Return -1 on failure when errno == ENOENT, leaving the stack unchanged.\n> > + * Return -2 on failure when errno != ENOENT, leaving the stack unchanged.\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 1;\n> > +\n> > +\tif ((level->dir = opendir(iter->base.path.buf)) != NULL)\n> > +\t\treturn 1;\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 -2;\n>\n> Perhaps we should just add an enum for these return values instead of\n> adding more negative/positive values here. That makes your later calls\n> of activate_level() more idiomaic. E.g. !activate_level() instead of\n> activate_level() == 1.\n\nYes, I agree. It was bothering me while I was writing these parts. I'm happy you\nsuggested a way to make this cleaner by using an enum.\n\n> >  \t\twarning_errno(\"failed to stat '%s'\", iter->base.path.buf);\n> > +\t\treturn -2;  // Stat failed not with ENOENT.\n> \n> Don't use // comments, use /* .. */\n> > +\t} else if (stat_err && errno == ENOENT)\n> > +\t\treturn -1;  // Stat failed with ENOENT.\n> \n> Missing {} here for the else if.\n> \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> > +\n> > +\tstruct dirent *dir_entry = NULL;\n> > +\n> > +\tint expose_err, activate_err;\n> > +\n> > +\t/* For shorter code width-wise, more readable */\n> > +\tunsigned int PEDANTIC = iter->flags & DIR_ITERATOR_PEDANTIC;\n> \n> We usually don't add \\n\\n in the middle of variable decls.\n> \n> > [...]\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> > +\tif (dir_entry == NULL) {\n> \n> Don't compare against NULL, use !dir_entry.\n> \n> [...]\n> \n> > +\t\tif (errno) {\n> > +\t\t\twarning_errno(\"errno reading dir '%s'\", iter->base.path.buf);\n> > +\t\t\tif (iter->flags & DIR_ITERATOR_PEDANTIC) goto error_out;\n> \n> more missing \\n before goto/return.\n\nI'll fix the coding style in these parts in v2. I'm sorry about that.\n"},{"id":"454504","messageId":"20220427160040.v5qj3oua2ayetfow@compass","threadId":"57706","inReplyTo":"220411.86sfqjj2o0.gmgdl@evledraar.gmail.com","subject":"Re: [RFC PATCH 5/6] t0066: remove redundant tests","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-04-27T16:00:40Z","receivedAt":"2022-04-27T16:02:47Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"On 22/04/11 01:10PM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Sun, Apr 10 2022, Plato Kiorpelidis wrote:\n> \n> > Remove dir-iterator redundant tests since the new tests introduced in\n> > 045738486f (t0066: better test coverage for dir-iterator, 2022-04-07)\n> > supersede them.\n> \n> This is presumably a forward-reference to the OID for your local version\n> of 2/6. Let's instead squash this change into that commit.\n\nWill do.\n"},{"id":"454518","messageId":"20220427173002.srn7cidveao7pk55@compass","threadId":"57706","inReplyTo":"220411.86wnfvj2q6.gmgdl@evledraar.gmail.com","subject":"Re: [RFC PATCH 6/6] test-dir-iterator: handle EACCES errno by dir-iterator","fromName":"Plato Kiorpelidis","fromEmail":"kioplato@gmail.com","sentAt":"2022-04-27T17:30:02Z","receivedAt":"2022-04-27T17:30:28Z","isPatch":true,"sender":{"key":"kioplato@gmail.com","avatar":null},"body":"On 22/04/11 01:04PM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Sun, Apr 10 2022, Plato Kiorpelidis wrote:\n> \n> > Handle EACCES errno returned by dir_iterator_begin() by printing the\n> > \"EACCES\" string instead of printing \"ESOMETHINGELSE\".\n> >\n> > Signed-off-by: Plato Kiorpelidis <kioplato@gmail.com>\n> > ---\n> >  t/helper/test-dir-iterator.c | 8 +++++---\n> >  t/t0066-dir-iterator.sh      | 2 +-\n> >  2 files changed, 6 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/t/helper/test-dir-iterator.c b/t/helper/test-dir-iterator.c\n> > index c92616bd69..fd07429f90 100644\n> > --- a/t/helper/test-dir-iterator.c\n> > +++ b/t/helper/test-dir-iterator.c\n> > @@ -7,9 +7,11 @@\n> >  static const char *error_name(int error_number)\n> >  {\n> >  \tswitch (error_number) {\n> > -\tcase ENOENT: return \"ENOENT\";\n> > -\tcase ENOTDIR: return \"ENOTDIR\";\n> > -\tdefault: return \"ESOMETHINGELSE\";\n> > +\t\tcase ENOENT: return \"ENOENT\";\n> > +\t\tcase ENOTDIR: return \"ENOTDIR\";\n> > +\t\tcase EACCES: return \"EACCES\";\n> > +\n> > +\t\tdefault: return \"ESOMETHINGELSE\";\n> >  \t}\n> >  }\n> \n> Please stick with the git coding style, see\n> Documentation/CodingGuidelines.\n> \n> It's forgivable to \"fix style while at it\" to some extent, but in this\n> case it's both moving away from our normal style (by indenting case\n> labels), and in doing so making the diff larger/worse.\n> \n> This should be a one-line addition of your new EACCES case.\n\nThat's true. I should've consulted Documentation/CodingGuidelines in more\ndetail. I mainly copied the local style and mindlessly indented these case\nlabels. I'll be more mindful of the coding style. Thanks!\n\n> I haven't read through the rest yet, but please self-review those\n> patches with a keen eye to diff size & seeing if there's similar\n> issues. E.g. are they the same lines changed under \"git show -w\", if not\n> are you sure you're correct in making the style change?\n\nThat's great, I'll make the use of \"git show -w\" habitual.\n\n> > diff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh\n> > index 974bb13092..4bf6456735 100755\n> > --- a/t/t0066-dir-iterator.sh\n> > +++ b/t/t0066-dir-iterator.sh\n> > @@ -861,7 +861,7 @@ test_expect_success POSIXPERM,SANITY \\\n> >  \n> >  \n> >  \tcat >expected-no-permissions-out <<-EOF &&\n> > -\tdir_iterator_begin failure: ESOMETHINGELSE\n> > +\tdir_iterator_begin failure: EACCES\n> >  \tEOF\n> >  \n> >  \ttest_must_fail test-tool dir-iterator ./dir12 >actual-out &&\n> \n> I see this is changing the ESOMETHINGELSE added in your 3/6, if you make\n> this patch come first then presumably we won't have that churn.\n\nYes. I'll make this patch come first to avoid that churn.\n\n> And if we don't have a test that's relying on ESOMETHINGELSE (maybe we\n> do, but don't test_cmp it, I haven't checked) shouldn't this \"default\"\n> case just be:\n> \n>     BUG(\"unknown errno %d: %s\", errno_number, strerror(errno_number))\n> \n> I.e. if we have OK coverage here we'd presumably fail the test case\n> itself anyway, so shouldn't we fail here too?\n\nWe can probably do that. I don't think we've got a test whose output isn't\ntest_cmp'ed anyways. There are two reasons I'm hesitant about this:\n  1. I'm not sure whether or not the test suite t/ behaves correctly in case\n  this macro triggers. I suppose you are positive it does since you suggested\n  it. I need to check and figure this out myself because I haven't used this\n  macro before.\n  2. How does this improve testing? If ESOMETHINGELSE is returned when some\n  other errno is expected then test_cmp will fail and the test suite will output\n  an error. How will the introduction of BUG() macro as the default case for the\n  switch statement in test-dir-iterator improve that?\n\nThanks,\nPlato\n"}]}