8000 Configuration file: Incorrect matching of conditional includes · Issue #5068 · libgit2/libgit2 · GitHub
[go: up one dir, main page]
Skip to content

Configuration file: Incorrect matching of conditional includes #5068

New issue

Have a question about this project? Sign up for a free GitHub accoun 8000 t to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
tmat opened this issue May 15, 2019 · 3 comments
Closed

Configuration file: Incorrect matching of conditional includes #5068

tmat opened this issue May 15, 2019 · 3 comments

Comments

@tmat
Copy link
tmat commented May 15, 2019

Reproduction steps

libgit2 uses FNM_PATHNAME|FNM_LEADING_DIR fnmatch flags to match the conditional include pattern, but it seems that git does not do so.

  1. libgit2 matches a directory prefix of git directory while git does not.
  2. libgit2 does not match wildcards ?, * and [] in the pattern to / in the path, while git does.

Consider repository with git directory D:\repository\.git with config file that contains

[includeIf "gitdir:D:/repository"] 
  path = cfg1
[includeIf "gitdir:D:/repository?.git"] 
  path = cfg2

and configuration files D:\repository\.git\cfg1 and D:\repository\.git\cfg2 with content:

[a]
  b = 1

and

[a]
  c = 1

respectively.

git does not include cfg file:

> git config --list
...
includeif.gitdir:D:/repository.path=cfg1
includeif.gitdir:D:/repository?.git.path=cfg2
a.c=1

Version of libgit2 (release number or SHA1)

a904fc6

Operating system(s) tested

Windows
@pks-t
Copy link
Member
pks-t commented Jun 14, 2019

Yeah, our "gitdir:" includes have a heavy mismatch in semantics compared to git.git. I accidentally stumbled upon this issue yesterday when taking a look at why our gitignore parser behaves differently: turns out git.git has migrated to use wildmatch instead of fnmatch in 2014, which caused quite a lot of incompatibilities.

So I've migrated our code to use wildmatch, too, in #5110. This PR fixes several issues in our config parser as well as in our conditional include handling.

I'm a bit confused, though. The first includeIf was obviously a fault on our side, and that issue got fixed via #5110. But the second condition "D:/repository?.git" shouldn't match at all, but from your git config --list I can see that "a.c=1" is in fact included. libgit2 does the right thing with #5110, and furthermore I'm unable to reproduce this on my (Linux) system with git v2.22.0:

$ git config --list
...
includeif.gitdir:/tmp/test.path=cfg1
includeif.gitdir:/tmp/test?.git.path=cfg2

Which version of git were you using?

@pks-t pks-t mentioned this issue Jun 14, 2019
Merged
@tmat
Copy link
Author
tmat commented Jun 14, 2019

Using git version 2.17.1.windows.2

@pks-t
Copy link
Member
pks-t commented Jun 17, 2019

I can reproduce this behaviour with v2.17.1.windows.2, but in fact it's fixed in v2.22.0.windows.1: none of the conditional includes get included. So with #5110 being merged, libgit2 now behaves the same as Git for Windows v2.22.0.

Closing -- please feel free to reopen if you feel like your issue hasn't been fixed after all.

@pks-t pks-t closed this as completed Jun 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@tmat @pks-t
0