Skip to content

Ignore: only treat one leading slash as a root identifier - #5074

Merged
ethomson merged 2 commits into
masterfrom
ethomson/ignore_leading_slash
Jun 6, 2019
Merged

Ignore: only treat one leading slash as a root identifier#5074
ethomson merged 2 commits into
masterfrom
ethomson/ignore_leading_slash

Conversation

@ethomson

Copy link
Copy Markdown
Member

Per #5070, we should not treat multiple leading slashes as identical to a single leading slash. One leading slash indicates the directory root, but only the first should indicate that.

Fixes #5070

ethomson added 2 commits May 19, 2019 12:32
For compatibility with git, only skip the first leading slash in an
ignore file.  That is: `/a.txt` indicates to ignore a file named `a.txt`
at the root.  However `//b.txt` does not indicate that a file named
`b.txt` at the root should be ignored.
@pks-t

pks-t commented May 21, 2019

Copy link
Copy Markdown
Member

This is weird and contrary to how most other tools handle this. Are we sure this is expected behaviour in git.git?

@ethomson

Copy link
Copy Markdown
Member Author

This is weird and contrary to how most other tools handle this. Are we sure this is expected behaviour in git.git?

Shockingly, yes.

@ethomson

Copy link
Copy Markdown
Member Author
hypnotoad:Temp/TestRepos/028% git status --short
?? a.txt
hypnotoad:Temp/TestRepos/028% echo 'a.txt' > .gitignore
hypnotoad:Temp/TestRepos/028% git status --short
?? .gitignore
hypnotoad:Temp/TestRepos/028% echo '/a.txt' > .gitignore
hypnotoad:Temp/TestRepos/028% git status --short
?? .gitignore
hypnotoad:Temp/TestRepos/028% echo '//a.txt' > .gitignore
hypnotoad:Temp/TestRepos/028% git status --short
?? .gitignore
?? a.txt

@pks-t

pks-t commented May 21, 2019

Copy link
Copy Markdown
Member

Well, sometimes there's a mismatch between how git.git works and how it's intended to work :P Do you know if it is documented in any way? If not, I think we shouldn't blindly assume this to be intended and instead ask for clarification on the mailing list. I'd definitely volunteer instead of pushing this additional work on you.

@ethomson

Copy link
Copy Markdown
Member Author

I'd definitely volunteer instead of pushing this additional work on you.

I'm happy if you want to chase this down. :D But even if this wasn't intended, I think they're probably stuck with this behavior now.

@pks-t

pks-t commented May 21, 2019

Copy link
Copy Markdown
Member

But even if this wasn't intended, I think they're probably stuck with this behavior now.

I don't know. It's obviously broken, if you ask me, as such a pattern cannot ever match anything given that files cannot contain slashes.

@ethomson

ethomson commented Jun 5, 2019

Copy link
Copy Markdown
Member Author

@pks-t did you end up asking on the mailing list?

@pks-t

pks-t commented Jun 6, 2019 via email

Copy link
Copy Markdown
Member

@pks-t pks-t left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I didn't send out an email, but I did investigate the issue a bit further and dug into git.git's code. During that I read gitignore(5) several times, at last realizing that the behaviour we're seeing is in fact specified:

  • Otherwise, Git treats the pattern as a shell glob: "*" matches
    anything except "/", "?" matches any one character except "/" and
    "[]" matches one character in a selected range. See fnmatch(3) and
    the FNM_PATHNAME flag for a more detailed description.

  • A leading slash matches the beginning of the pathname. For example,
    "/*.c" matches "cat-file.c" but not "mozilla-sha1/sha1.c".

So the second part is quite clear in that it says "A leading slash" in singular, and the first part essentially says "If none of our custom pattern format rules apply, please have a look at fnmatch(3)". And neither fnmatch("x//x", "x/x", FNM_PATHNAME) nor fnmatch("x/x", "x/xx", FNM_PATHNAME) will in fact match.

Thus I think we should live with the situation, as gitignore(5) does in fact specify this weird behaviour.

@pks-t

pks-t commented Jun 6, 2019

Copy link
Copy Markdown
Member

By the way, I naturally found some different edge cases which we fail to handle correctly while reading gitignore(5) :D I'll dig into those

@ethomson

ethomson commented Jun 6, 2019

Copy link
Copy Markdown
Member Author

Ah, yes, thanks, I should have quoted the docs which do shed light on the fact that this insanity is... well, at least it's documented, if not expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

.gitignore parsing: The parser skips multiple leading /

2 participants