dir: fix treat_leading_path() to return false on non-directories

Commit Message

Ivan Tse May 21, 2024, 3:48 p.m. UTC

From: Ivan Tse <[e-mail]>

If treat_leading_path() encounters a non-directory in its loop over each
leading path component, it should return false. This bug was introduced
in commit b9670c1f5e ("dir: fix checks on common prefix directory",
2019-12-19) where the check for `is_directory(sb.buf)` still breaks out
of the loop but doesn't return false anymore. Instead, the loop is
broken out and the last state result in the loop is used in the return
conditional: `state == path_recurse`.

This prevents the warning "warning: could not open directory" errors
from occurring in `git status` or `git ls-files -o` calls on
non-directory pathspecs. The warning was introduced in commit b673155074
("dir.c: stop ignoring opendir() error in open_cached_dir()",
2018-02-02).

Signed-off-by: Ivan Tse <[e-mail]>
---
    dir: fix treat_leading_path() to return false on non-directories
    
    Hi there!
    
    I wanted to provide some extra context on the behavior I'm trying to
    fix. I noticed in one of my git hook scripts that it was showing a
    warning error message when running git status on a deleted directory.
    You can reproduce with the following on the git repo:
    
    $ git status --short t/non_existent_directory/
    warning: could not open directory 't/non_existent_directory/': No such file or directory
    
    
    However, this warning doesn't show if you give a non-directory from the
    root of the repo:
    
    $ git status --short non_existent_directory/
    
    
    Also doesn't show if you give it a directory in gitignore (/bin is in
    t/unit-tests/.gitignore)
    
    $ git status --short t/unit-tests/bin/non_existent_directory
    
    
    I found it strange that sometimes git is able to detect non-directories
    without warnings. Even stranger, an older version of git didn't show
    this warning in the first example.
    
    After running git bisect, I was able to track down the commit that
    introduced this behavior: b9670c1 ("dir: fix checks on common prefix
    directory", 2019-12-19).
    
    Before, that change, !is_directory(sb.buf) would break out of the loop
    and return 0 since the branch of code that changes rc to 1 hasn't been
    reached yet. The change introduces the conditional state == path_recurse
    as the new return statement. This state variable could be from a
    previous iteration of the loop which is causing this new behavior. The
    fix is to ensure the is_directory conditional results in the overall
    method returning false.
    
    I'm not sure if this is considered a bug or not since it's just a
    warning message. If this isn't noteworthy, then feel free to ignore this
    pull request!

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1723%2Fivantsepp%2Fitse_untracked_warnings-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1723/ivantsepp/itse_untracked_warnings-v1
Pull-Request: https://github.com/git/git/pull/1723

 dir.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


base-commit: 4365c6fcf96caac73dcc412aa25db34cf8df48d5

Patch

diff --git a/dir.c b/dir.c
index 2d83f3311a7..5cb56f3a3e2 100644
--- a/dir.c
+++ b/dir.c
@@ -2771,8 +2771,10 @@  static int treat_leading_path(struct dir_struct *dir,
 			baselen = cp - path;
 		strbuf_reset(&sb);
 		strbuf_add(&sb, path, baselen);
-		if (!is_directory(sb.buf))
+		if (!is_directory(sb.buf)) {
+			state = path_none;
 			break;
+		}
 		strbuf_reset(&sb);
 		strbuf_add(&sb, path, prevlen);
 		strbuf_reset(&subdir);