{"thread":{"id":"61524","subject":"[PATCH] dir: fix treat_leading_path() to return false on non-directories","startedAt":"2024-05-21T15:48:56Z","lastAt":"2024-05-21T15:48:56Z","messageCount":1,"participants":["Ivan Tse via GitGitGadget"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"495172","messageId":"pull.1723.git.git.1716306532869.gitgitgadget@gmail.com","threadId":"61524","inReplyTo":null,"subject":"[PATCH] dir: fix treat_leading_path() to return false on non-directories","fromName":"Ivan Tse via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-05-21T15:48:52Z","receivedAt":"2024-05-21T15:48:56Z","isPatch":true,"sender":{"key":"ivan.tse1@gmail.com","avatar":"https://avatars.githubusercontent.com/u/4662860?v=4"},"body":"From: Ivan Tse <ivan.tse1@gmail.com>\n\nIf treat_leading_path() encounters a non-directory in its loop over each\nleading path component, it should return false. This bug was introduced\nin commit b9670c1f5e (\"dir: fix checks on common prefix directory\",\n2019-12-19) where the check for `is_directory(sb.buf)` still breaks out\nof the loop but doesn't return false anymore. Instead, the loop is\nbroken out and the last state result in the loop is used in the return\nconditional: `state == path_recurse`.\n\nThis prevents the warning \"warning: could not open directory\" errors\nfrom occurring in `git status` or `git ls-files -o` calls on\nnon-directory pathspecs. The warning was introduced in commit b673155074\n(\"dir.c: stop ignoring opendir() error in open_cached_dir()\",\n2018-02-02).\n\nSigned-off-by: Ivan Tse <ivan.tse1@gmail.com>\n---\n    dir: fix treat_leading_path() to return false on non-directories\n    \n    Hi there!\n    \n    I wanted to provide some extra context on the behavior I'm trying to\n    fix. I noticed in one of my git hook scripts that it was showing a\n    warning error message when running git status on a deleted directory.\n    You can reproduce with the following on the git repo:\n    \n    $ git status --short t/non_existent_directory/\n    warning: could not open directory 't/non_existent_directory/': No such file or directory\n    \n    \n    However, this warning doesn't show if you give a non-directory from the\n    root of the repo:\n    \n    $ git status --short non_existent_directory/\n    \n    \n    Also doesn't show if you give it a directory in gitignore (/bin is in\n    t/unit-tests/.gitignore)\n    \n    $ git status --short t/unit-tests/bin/non_existent_directory\n    \n    \n    I found it strange that sometimes git is able to detect non-directories\n    without warnings. Even stranger, an older version of git didn't show\n    this warning in the first example.\n    \n    After running git bisect, I was able to track down the commit that\n    introduced this behavior: b9670c1 (\"dir: fix checks on common prefix\n    directory\", 2019-12-19).\n    \n    Before, that change, !is_directory(sb.buf) would break out of the loop\n    and return 0 since the branch of code that changes rc to 1 hasn't been\n    reached yet. The change introduces the conditional state == path_recurse\n    as the new return statement. This state variable could be from a\n    previous iteration of the loop which is causing this new behavior. The\n    fix is to ensure the is_directory conditional results in the overall\n    method returning false.\n    \n    I'm not sure if this is considered a bug or not since it's just a\n    warning message. If this isn't noteworthy, then feel free to ignore this\n    pull request!\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1723%2Fivantsepp%2Fitse_untracked_warnings-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1723/ivantsepp/itse_untracked_warnings-v1\nPull-Request: https://github.com/git/git/pull/1723\n\n dir.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex 2d83f3311a7..5cb56f3a3e2 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -2771,8 +2771,10 @@ static int treat_leading_path(struct dir_struct *dir,\n \t\t\tbaselen = cp - path;\n \t\tstrbuf_reset(&sb);\n \t\tstrbuf_add(&sb, path, baselen);\n-\t\tif (!is_directory(sb.buf))\n+\t\tif (!is_directory(sb.buf)) {\n+\t\t\tstate = path_none;\n \t\t\tbreak;\n+\t\t}\n \t\tstrbuf_reset(&sb);\n \t\tstrbuf_add(&sb, path, prevlen);\n \t\tstrbuf_reset(&subdir);\n\nbase-commit: 4365c6fcf96caac73dcc412aa25db34cf8df48d5\n-- \ngitgitgadget\n"}]}