Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Matheus Tavares Bernardino <matheus.tavb@gmail.com>
Cc: Taylor Blau <me@ttaylorr.com>, git@vger.kernel.org
Subject: [PATCH 2/1] t0066: drop setup of "dir5"
Date: Thu, 16 Feb 2023 19:40:29 -0500	[thread overview]
Message-ID: <Y+7M/VLcDRDCXX27@coredump.intra.peff.net> (raw)
In-Reply-To: <CAGdrTFhHBU2BNYdYr7LbOS7i1LOHGjWLw_d5ZJAXxvTKyLiFCA@mail.gmail.com>

On Thu, Feb 16, 2023 at 09:16:50PM -0300, Matheus Tavares Bernardino wrote:

> Oh, it's been some time. But yes, dir5 is vestigial and should indeed
> be removed. It was created back when I wanted to make dir-iterator
> detect and avoid recursive symlinks (see this [1] earlier version of
> the patch, where there was a test grepping for dir5 at the output).
> But this idea ended up being discarded and I must have forgotten to
> remove the dir5.

Thanks! It's not super-important, but while we're thinking about it, it
probably makes sense to do this cleanup (I prepared it on top of
Taylor's patch, since it would otherwise conflict textually):

-- >8 --
Subject: t0066: drop setup of "dir5"

The symlink setup in t0066 makes several directories with links, dir4
through dir6. But ever since dir5 was introduced in fa1da7d2ee
(dir-iterator: add flags parameter to dir_iterator_begin, 2019-07-10),
it has never actually been used. It was left over from an earlier
iteration of the patch which tried to handle recursive symlinks
specially, as seen in:

  https://lore.kernel.org/git/20190502144829.4394-7-matheus.bernardino@usp.br/

It's not hurting any of the existing tests to be there, but the extra
setup is confusing to anybody trying to read and understand the tests.
Let's drop the extra directory, and we'll rename "dir6" to "dir5" so
nobody wonders whether the gap in naming is important.

Helped-by: Matheus Tavares Bernardino <matheus.tavb@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---
 t/t0066-dir-iterator.sh | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh
index 4ed3136b00..7d0a0da8c0 100755
--- a/t/t0066-dir-iterator.sh
+++ b/t/t0066-dir-iterator.sh
@@ -106,13 +106,7 @@ test_expect_success SYMLINKS 'setup dirs with symlinks' '
 	ln -s d dir4/a/e &&
 	ln -s ../b dir4/a/f &&
 
-	mkdir -p dir5/a/b &&
-	mkdir -p dir5/a/c &&
-	ln -s ../c dir5/a/b/d &&
-	ln -s ../ dir5/a/b/e &&
-	ln -s ../../ dir5/a/b/f &&
-
-	ln -s dir4 dir6
+	ln -s dir4 dir5
 '
 
 test_expect_success SYMLINKS 'dir-iterator should not follow symlinks by default' '
@@ -132,7 +126,7 @@ test_expect_success SYMLINKS 'dir-iterator should not follow symlinks by default
 '
 
 test_expect_success SYMLINKS 'dir-iterator does not resolve top-level symlinks' '
-	test_must_fail test-tool dir-iterator ./dir6 >out &&
+	test_must_fail test-tool dir-iterator ./dir5 >out &&
 
 	grep "ENOTDIR" out
 '
-- 
2.39.2.920.gf3beb43ccf


      reply	other threads:[~2023-02-17  0:40 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-16 18:27 [PATCH] dir-iterator: drop unused `DIR_ITERATOR_FOLLOW_SYMLINKS` Taylor Blau
2023-02-16 18:49 ` Jeff King
2023-02-16 20:03   ` Taylor Blau
2023-02-16 20:05     ` Taylor Blau
2023-02-17  0:16       ` Matheus Tavares Bernardino
2023-02-17  0:40         ` Jeff King [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y+7M/VLcDRDCXX27@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=matheus.tavb@gmail.com \
    --cc=me@ttaylorr.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).