Skip to content
New issue

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

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

is_path_ignored not working on windows #334

Closed
Eh2406 opened this issue Jul 19, 2018 · 8 comments · Fixed by #335
Closed

is_path_ignored not working on windows #334

Eh2406 opened this issue Jul 19, 2018 · 8 comments · Fixed by #335

Comments

@Eh2406
Copy link
Contributor

Eh2406 commented Jul 19, 2018

steps to repro:
start a new cargo project C:\> cargo new git2-test then
check that .gitignore includes /target
add git2 = "*" to Cargo.toml and the following to main.rs

extern crate git2;

fn main() {
    let p_exe = std::env::current_exe().unwrap();
    println!("./target/debug == {:?}", p_exe.display());
    let repo = git2::Repository::discover(&p_exe).unwrap();
    println!("./.git == {:?}", repo.path().display());
    let ignored = repo.is_path_ignored(&p_exe).unwrap();
    println!("true == {:?}", ignored);
}

(or unzip the file git2-test.zip) and do cargo run.

output is:

C:\git2-test>cargo run
   Compiling git2-test v0.1.0 (file:///C:/git2-test)
    Finished dev [unoptimized + debuginfo] target(s) in 1.78s
     Running `target\debug\git2-test.exe`
./target/debug == "C:\\git2-test\\target\\debug\\git2-test.exe"
./.git == "C:/git2-test/.git/"
true == false

I expected the last line to be true == true

tested on windows 10 locally, reduced from appveyor testing rust-lang/cargo#5733

@alexcrichton
Copy link
Member

This is likely either a bug in libgit2 or a facet that we need to work around unfortunately :(

@alexcrichton
Copy link
Member

Thanks for diagnosing this though!

@Eh2406
Copy link
Contributor Author

Eh2406 commented Jul 19, 2018

I refined it as far as I can. I don't have the C knowledge to reported up stream.

If you have a suggestion for a workaround, I'm all ears.

@alexcrichton
Copy link
Member

We may need to to_string it and convert \ to / to maybe work, but that's the only workaruond I'd posisbly know what to do :(

@Eh2406
Copy link
Contributor Author

Eh2406 commented Jul 19, 2018

That seems to work locally. Odd that discover did not need that conversion. What is the most reliable way to convert? to_string_lossy?

@Eh2406
Copy link
Contributor Author

Eh2406 commented Jul 20, 2018

Also should we Implement that work around here or in each user of this crates?

@alexcrichton
Copy link
Member

Yeah something like to_string_lossy would be reasonable, and I'd be fine with a fix in this crate itself

@dwijnand
Copy link
Member

The other workaround, probably not worth considering but maybe worth studying, is https://github.com/BurntSushi/ripgrep/tree/master/ignore.

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 a pull request may close this issue.

3 participants