dir: fix treat_leading_path() to return false on non-directories #1723
+3
−1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Before you submit, you may want to replace your |
Thank you! Great catch! Updated the PR description to use the full command instead of my alias. |
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 b9670c1 ("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 b673155 ("dir.c: stop ignoring opendir() error in open_cached_dir()", 2018-02-02). Signed-off-by: Ivan Tse <ivan.tse1@gmail.com>
ivantsepp
force-pushed
the
itse_untracked_warnings
branch
from
3a1d0a6
to
06462bb
Compare
|
/preview |
|
Preview email sent as pull.1723.git.git.1716301467964.gitgitgadget@gmail.com |
|
/submit |
|
Submitted as pull.1723.git.git.1716306532869.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 statuson a deleted directory. You can reproduce with the following on the git repo:However, this warning doesn't show if you give a non-directory from the root of the repo:
Also doesn't show if you give it a directory in gitignore (
/binis int/unit-tests/.gitignore)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 changesrcto 1 hasn't been reached yet. The change introduces the conditionalstate == path_recurseas the new return statement. Thisstatevariable 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!