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

Cargo new does not follow "includeIf" in .gitconfig #8882

Closed
justfortherec opened this issue Nov 22, 2020 · 3 comments · Fixed by #8886
Closed

Cargo new does not follow "includeIf" in .gitconfig #8882

justfortherec opened this issue Nov 22, 2020 · 3 comments · Fixed by #8886
Labels
C-bug Category: bug

Comments

@justfortherec
Copy link
Contributor

justfortherec commented Nov 22, 2020

This is a trivial bug: The name and/or email address of the user may not be set correctly by cargo new.

Problem

cargo new sets the user name and email in the Cargo.toml file based on the values configured for Git. Since version 2.13, .gitconfig can include other configuration files based on the prefix of the repository. I use this to set separate email addresses for private and work projects based on where a repository is located in my file system. See this Stackoverflow answer for details.

Unfortunately, Cargo (version 1.48.0 (65cbdd2dc 2020-10-14)) seems to ignore these include directives.

Steps

  1. Add an includeIf directive to the global .gitconfig:
$ cat << END >> ~/.gitconfig
[includeIf "gitdir:~/test/"]
    path = ~/test/.gitconfig
END
  1. Set the user details in the included file:
$ mkdir ~/test
$ cat << END > ~/test/.gitconfig
[user]
    name = "John Doe"
    email = john@example.com
END
  1. Run cargo new in a directory with the prefix of the includeIf:
$ cd ~/test
$ cargo new example-project

Expected outcome:

The generated file ~/test/example-project/Cargo.toml uses the user details from ~/test/.gitconfig:

authors = ["John Doe <john@example.com"]

Actual outcome:

The Cargo.toml file does not include the set user details:

$ grep authors ~/test/example-project/Cargo.toml
authors = ["my-username"]

Possible Solution(s)

Updating whatever is parsing the .gitconfig files might solve the problem.

Notes

Output of cargo version:

$ cargo --version
cargo 1.48.0 (65cbdd2dc 2020-10-14)
@justfortherec justfortherec added the C-bug Category: bug label Nov 22, 2020
@DJMcNab
Copy link

DJMcNab commented Nov 22, 2020

My understanding is that this behaviour is correct - the includeIf only applies in a git repository's directory; when you run cargo new you are not in one.
E.g. run git config user.name in your ~/test directory

However, in terms of practicality, it's not ideal. If we're lucky changing

Ok(cwd) => GitRepository::discover(cwd)

to get the config in the new directory would fix this.

@justfortherec
Copy link
Contributor Author

My understanding is that this behaviour is correct - the includeIf only applies in a git repository's directory; when you run cargo new you are not in one.

Fair enough.

However, in terms of practicality, it's not ideal. If we're lucky changing get_real_git_config() to get the config in the new directory would fix this.

Yes, this seems correct. It is exemplified by the fact that cargo init works as expected, i.e. it picks up the included configuration because the cwd is a Git repository.

How do you think changing get_real_git_config() should work? It is used by both init and new. I see two possible ways:

  1. In new change the working directory before invoking mk() or
  2. Use the path option instead of using the current working directory.

The second option seems preferable to me (because no implicit global state in form of the current working directory would need to be changed). However, it would require the path as a new parameter for discover_author(), find_git_config(), and find_real_git_config().

Or is there another way that I don't see?

@DJMcNab
Copy link

DJMcNab commented Nov 23, 2020

Yeah, passing the parameters down the call stack would be the correct method. Changing the current directory is very fragile.

bors added a commit that referenced this issue Nov 24, 2020
…lexcrichton

Start searching git config at new path

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.
@bors bors closed this as completed in 35b029c Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants