git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: blees@dcon.de, gitster@pobox.com, kyle@kyleam.com,
	sxlijin@gmail.com, Junio C Hamano <gitster@pobox.com>,
	Elijah Newren <newren@gmail.com>
Subject: [PATCH v5 2/8] Revert "dir.c: make 'git-status --ignored' work within leading directories"
Date: Thu, 19 Dec 2019 21:28:20 +0000	[thread overview]
Message-ID: <79f2b561742372234a52dc7194dd72026c6e96ec.1576790906.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.676.v5.git.git.1576790906.gitgitgadget@gmail.com>

From: Elijah Newren <newren@gmail.com>

Commit be8a84c52669 ("dir.c: make 'git-status --ignored' work within
leading directories", 2013-04-15) noted that
   git status --ignored <SOMEPATH>
would not list ignored files and directories within <SOMEPATH> if
<SOMEPATH> was untracked, and modified the behavior to make it show
them.  However, it did so via a hack that broke consistency; it would
show paths under <SOMEPATH> differently than a simple
   git status --ignored | grep <SOMEPATH>
would show them.  A correct fix is slightly more involved, and
complicated slightly by this hack, so we revert this commit (but keep
corrected versions of the testcases) and will later fix the original
bug with a subsequent patch.

Some history may be helpful:

A very, very similar case to the commit we are reverting was raised in
commit 48ffef966c76 ("ls-files: fix overeager pathspec optimization",
2010-01-08); but it actually went in somewhat the opposite direction.  In
that commit, it mentioned how
   git ls-files -o --exclude-standard t/
used to show untracked files under t/ even when t/ was ignored, and then
changed the behavior to stop showing untracked files under an ignored
directory.  More importantly, this commit considered keeping this
behavior but noted that it would be inconsistent with the behavior when
multiple pathspecs were specified and thus rejected it.

The reason for this whole inconsistency when one pathspec is specified
versus zero or two is because common prefixes of pathspecs are sent
through a different set of checks (in treat_leading_path()) than normal
file/directory traversal (those go through read_directory_recursive()
and treat_path()).  As such, for consistency, one needs to check that
both codepaths produce the same result.

Revert commit be8a84c526691667fc04a8241d93a3de1de298ab, except instead
of removing the testcase it added, modify it to check for correct and
consistent behavior.  A subsequent patch in this series will fix the
testcase.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 dir.c                      | 3 ---
 t/t7061-wtstatus-ignore.sh | 9 +++++++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/dir.c b/dir.c
index 61f559f980..0dd5266629 100644
--- a/dir.c
+++ b/dir.c
@@ -2083,14 +2083,12 @@ static int treat_leading_path(struct dir_struct *dir,
 	struct strbuf sb = STRBUF_INIT;
 	int baselen, rc = 0;
 	const char *cp;
-	int old_flags = dir->flags;
 
 	while (len && path[len - 1] == '/')
 		len--;
 	if (!len)
 		return 1;
 	baselen = 0;
-	dir->flags &= ~DIR_SHOW_OTHER_DIRECTORIES;
 	while (1) {
 		cp = path + baselen + !!baselen;
 		cp = memchr(cp, '/', path + len - cp);
@@ -2113,7 +2111,6 @@ static int treat_leading_path(struct dir_struct *dir,
 		}
 	}
 	strbuf_release(&sb);
-	dir->flags = old_flags;
 	return rc;
 }
 
diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh
index 0c394cf995..84366050da 100755
--- a/t/t7061-wtstatus-ignore.sh
+++ b/t/t7061-wtstatus-ignore.sh
@@ -43,11 +43,16 @@ test_expect_success 'status untracked directory with --ignored -u' '
 	test_cmp expected actual
 '
 cat >expected <<\EOF
-?? untracked/uncommitted
+?? untracked/
 !! untracked/ignored
 EOF
 
-test_expect_success 'status prefixed untracked directory with --ignored' '
+test_expect_failure 'status of untracked directory with --ignored works with or without prefix' '
+	git status --porcelain --ignored >tmp &&
+	grep untracked/ tmp >actual &&
+	rm tmp &&
+	test_cmp expected actual &&
+
 	git status --porcelain --ignored untracked/ >actual &&
 	test_cmp expected actual
 '
-- 
gitgitgadget


  parent reply	other threads:[~2019-12-19 21:28 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-09 20:47 [PATCH 0/8] Directory traversal bugs Elijah Newren via GitGitGadget
2019-12-09 20:47 ` [PATCH 1/8] t3011: demonstrate directory traversal failures Elijah Newren via GitGitGadget
2019-12-09 21:06   ` Denton Liu
2019-12-09 20:47 ` [PATCH 2/8] Revert "dir.c: make 'git-status --ignored' work within leading directories" Elijah Newren via GitGitGadget
2019-12-09 21:32   ` Denton Liu
2019-12-09 21:51     ` Elijah Newren
2019-12-09 22:09     ` Eric Sunshine
2019-12-09 20:47 ` [PATCH 3/8] dir: remove stray quote character in comment Elijah Newren via GitGitGadget
2019-12-09 20:47 ` [PATCH 4/8] dir: exit before wildcard fall-through if there is no wildcard Elijah Newren via GitGitGadget
2019-12-09 20:47 ` [PATCH 5/8] dir: break part of read_directory_recursive() out for reuse Elijah Newren via GitGitGadget
2019-12-09 20:47 ` [PATCH 6/8] dir: fix checks on common prefix directory Elijah Newren via GitGitGadget
2019-12-09 20:47 ` [PATCH 7/8] dir: synchronize treat_leading_path() and read_directory_recursive() Elijah Newren via GitGitGadget
2019-12-09 20:47 ` [PATCH 8/8] dir: consolidate similar code in treat_directory() Elijah Newren via GitGitGadget
2019-12-10 20:00 ` [PATCH v2 0/8] Directory traversal bugs Elijah Newren via GitGitGadget
2019-12-10 20:00   ` [PATCH v2 1/8] t3011: demonstrate directory traversal failures Elijah Newren via GitGitGadget
2019-12-10 20:00   ` [PATCH v2 2/8] Revert "dir.c: make 'git-status --ignored' work within leading directories" Elijah Newren via GitGitGadget
2019-12-10 20:00   ` [PATCH v2 3/8] dir: remove stray quote character in comment Elijah Newren via GitGitGadget
2019-12-10 20:00   ` [PATCH v2 4/8] dir: exit before wildcard fall-through if there is no wildcard Elijah Newren via GitGitGadget
2019-12-10 20:00   ` [PATCH v2 5/8] dir: break part of read_directory_recursive() out for reuse Elijah Newren via GitGitGadget
2019-12-10 20:00   ` [PATCH v2 6/8] dir: fix checks on common prefix directory Elijah Newren via GitGitGadget
2019-12-15 10:29     ` Johannes Schindelin
2019-12-16 13:51       ` Elijah Newren
2019-12-16 16:00         ` Elijah Newren
2019-12-16 18:13           ` Junio C Hamano
2019-12-16 21:08             ` Elijah Newren
2019-12-16 21:25               ` Junio C Hamano
2019-12-16 22:39                 ` Elijah Newren
2019-12-17  0:04           ` Johannes Schindelin
2019-12-17  0:14             ` Junio C Hamano
2019-12-17 11:08               ` Johannes Schindelin
2019-12-17 17:33                 ` Junio C Hamano
2019-12-17 19:32                   ` Johannes Schindelin
2019-12-17  5:26             ` Elijah Newren
2019-12-17 11:15               ` Johannes Schindelin
2019-12-17 16:58                 ` Elijah Newren
2019-12-10 20:00   ` [PATCH v2 7/8] dir: synchronize treat_leading_path() and read_directory_recursive() Elijah Newren via GitGitGadget
2019-12-10 20:00   ` [PATCH v2 8/8] dir: consolidate similar code in treat_directory() Elijah Newren via GitGitGadget
2019-12-17  8:33   ` [PATCH v3 0/3] Directory traversal bugs Elijah Newren via GitGitGadget
2019-12-17  8:33     ` [PATCH v3 1/3] t3011: demonstrate directory traversal failures Elijah Newren via GitGitGadget
2019-12-17  8:33     ` [PATCH v3 2/3] dir: remove stray quote character in comment Elijah Newren via GitGitGadget
2019-12-17  8:33     ` [PATCH v3 3/3] dir: exit before wildcard fall-through if there is no wildcard Elijah Newren via GitGitGadget
2019-12-17 11:18     ` [PATCH v3 0/3] Directory traversal bugs Johannes Schindelin
2019-12-17 18:24       ` Junio C Hamano
2019-12-21 22:05         ` Johannes Schindelin
2019-12-18 19:29     ` [PATCH v4 0/8] " Elijah Newren via GitGitGadget
2019-12-18 19:29       ` [PATCH v4 1/8] t3011: demonstrate directory traversal failures Elijah Newren via GitGitGadget
2019-12-18 19:29       ` [PATCH v4 2/8] Revert "dir.c: make 'git-status --ignored' work within leading directories" Elijah Newren via GitGitGadget
2019-12-18 19:29       ` [PATCH v4 3/8] dir: remove stray quote character in comment Elijah Newren via GitGitGadget
2019-12-18 19:29       ` [PATCH v4 4/8] dir: exit before wildcard fall-through if there is no wildcard Elijah Newren via GitGitGadget
2019-12-18 19:29       ` [PATCH v4 5/8] dir: break part of read_directory_recursive() out for reuse Elijah Newren via GitGitGadget
2019-12-18 19:29       ` [PATCH v4 6/8] dir: fix checks on common prefix directory Elijah Newren via GitGitGadget
2019-12-18 21:29         ` Junio C Hamano
2019-12-19 20:23           ` Elijah Newren
2019-12-19 22:24             ` Jeff King
2019-12-20 17:00               ` Elijah Newren
2019-12-20 21:14                 ` Jeff King
2019-12-20 18:01               ` Junio C Hamano
2019-12-20 21:15                 ` Jeff King
2019-12-18 19:29       ` [PATCH v4 7/8] dir: synchronize treat_leading_path() and read_directory_recursive() Elijah Newren via GitGitGadget
2019-12-18 19:29       ` [PATCH v4 8/8] dir: consolidate similar code in treat_directory() Elijah Newren via GitGitGadget
2019-12-19 21:28       ` [PATCH v5 0/8] Directory traversal bugs Elijah Newren via GitGitGadget
2019-12-19 21:28         ` [PATCH v5 1/8] t3011: demonstrate directory traversal failures Elijah Newren via GitGitGadget
2019-12-19 21:28         ` Elijah Newren via GitGitGadget [this message]
2019-12-19 21:28         ` [PATCH v5 3/8] dir: remove stray quote character in comment Elijah Newren via GitGitGadget
2019-12-19 21:28         ` [PATCH v5 4/8] dir: exit before wildcard fall-through if there is no wildcard Elijah Newren via GitGitGadget
2019-12-19 21:28         ` [PATCH v5 5/8] dir: break part of read_directory_recursive() out for reuse Elijah Newren via GitGitGadget
2019-12-19 21:28         ` [PATCH v5 6/8] dir: fix checks on common prefix directory Elijah Newren via GitGitGadget
2019-12-19 21:28         ` [PATCH v5 7/8] dir: synchronize treat_leading_path() and read_directory_recursive() Elijah Newren via GitGitGadget
2019-12-19 21:28         ` [PATCH v5 8/8] dir: consolidate similar code in treat_directory() Elijah Newren via GitGitGadget

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=79f2b561742372234a52dc7194dd72026c6e96ec.1576790906.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=blees@dcon.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kyle@kyleam.com \
    --cc=newren@gmail.com \
    --cc=sxlijin@gmail.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 NNTP newsgroup(s).