Start searching git config at new path

[justfortherec]

This lets cargo new follow includeIf blocks inside .gitignore.

Fixes #8882

I am not sure if removing the __CARGO_TEST_ROOT environment variable has any bad side effects. My quick grep of the repository didn't highlight anything in particular.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[justfortherec]

I am not entirely sure what causes the failed test on Windows (I have no way of reproducing it locally):

failures:

---- new::finds_git_author_in_included_config stdout ----
running `D:\a\cargo\cargo\target\debug\cargo.exe new foo/bar`
thread 'new::finds_git_author_in_included_config' panicked at '
Expected: execs
    but: exited with exit code: 101
--- stdout

--- stderr
error: Failed to create package `bar` at `D:\a\cargo\cargo\target\cit\t1230\foo/bar`

Caused by:
  invalid escape at a\cargo\cargo\target\cit\t1230\foo/.gitconfig; class=Config (7)
', crates\cargo-test-support\src\lib.rs:729:13

The way how the new test crafts paths matches what surrounding tests use (e.g. subpackage_git_with_gitignore). Still, something seems to go wrong with the backwards slashes on Windows. It could be the path in .gitconfig. I will try using explicit calls to join instead of one call with several path components in one argument.

[justfortherec]

The test failed again ... with the same error. I assume the culprit is the forward slash in "gitdir:{}/". The block is ignored without the trailing slash on my local machine (Linux). I have pushed another attempt to fix this.

BTW: I am not force pushing the attempts to fix the problem to make it easier to review. Before merging I'd like to squash the (supposed) fixes into the first commit.

[justfortherec]

This is progress: A new error.

failed to parse config file: unexpected text after closing quotes (in D:/a/cargo/cargo/target/cit/t1230/home/.gitconfig:2, column 47); class=Config (7)

libgit2 fails with this error message if the parsed element does not end in "]

From config_parse.c at line 145:

	if (line[rpos] != '"' || line[rpos + 1] != ']') {
		set_parse_error(reader, rpos, "unexpected text after closing quotes");
		git_buf_dispose(&buf);
		return -1;
	}

It's hard for me to see why this would not be the case in this test because I lack access to a Windows environment. Does anyone with more experience have a lead? @alexcrichton

[DJMcNab]

I use gitdir/i:{} (for ignore case IIRC) in my personal setup which uses this feature on windows - that might be worth a try.

[justfortherec]

I use gitdir/i:{} (for ignore case IIRC) in my personal setup which uses this feature on windows - that might be worth a try.

Thanks, @DJMcNab, I'll give it a try.

[alexcrichton]

Looks good to me and seems like the right fix, thanks!

[DJMcNab]

Yeah, I've just debugged it to the same thing @alexcrichton found.

I got it working by adding

.to_string()
.replace('\\', "/"),

to both paths
Which obviously is very not ideal, but at least it works.

[justfortherec]

Yeah, I've just debugged it to the same thing @alexcrichton found.

I got it working by adding

.to_string()
.replace('\\', "/"),

to both paths
Which obviously is very not ideal, but at least it works.

Thanks a lot for helping out with debugging. So in fact it is still the same problem that I had supposedly fixed earlier.

I was under the assumption that .display() would be platform specific, i.e. use \ on Windows.

How do I go about this now? Just including .to_string().replace('\\', "/") would then probably break the tests on other platforms. Can a write specific tests for different platforms?

[DJMcNab]

I don't think it would matter on other platforms, since they wouldn't have \ in their paths.

[justfortherec]

I am not super happy with the hack but I can live with it. I'll squash the commits and rebase on master. Then this PR is ready to be merged from my side.

[alexcrichton]

@bors: r+

Sounds reasonable to me!

[bors]

📌 Commit 35b029c has been approved by alexcrichton

[bors]

⌛ Testing commit 35b029c with merge 4e32fba...

[bors]

☀️ Test successful - checks-actions
Approved by: alexcrichton
Pushing 4e32fba to master...