build.rustflags is passed twice (and rustc errors) #3070
Comments
|
The bug here appears to be pretty deep in Cargo when we're parsing config files. The logic there is to walk the directory tree up to the root of the file system, picking up configuration as we go along. At the end, however, we specifically look in the home directory as well in case a build isn't hierarchically below that. This logic has a guard to try to not parse a config file twice, but unfortunately that's not actually the right condition. As a result, configuration in the home directory is getting parsed twice, and it's duplicating lists. The fix here is to probably just keep a hash set of visited directories, and check that for the home directory instead of a |
|
Probably would be a good idea to canonicalize each visited path to ensure you don't visit the same config twice due to symbolic links. |
|
Hi! I'm pretty new to rust but I would like to try to contribute to cargo by fixing this issue if nobody else is working on it. |
|
I went ahead and made the fix. If you have something better coming feel free to close it. I hope it helps #3078 |
|
Thanks @jhbabon! |
Fix: Don't parse the home directory more than once This PR tries to resolve this issue #3070. The problem is that the `walk_tree` method in the `src/util/config.rs` module was parsing more than once the contents of the config file in the home directory (the file `~/.cargo/config`). The biggest problem with this is with options that can accept multiple values, like `build.rustflags`. If you parse the file twice, the same option can end with duplicated values (e.g: `rustflags=["-Z", "foo", "-Z", "foo"]`). I made the fix following the comments in the issue. In the fix I keep track of all the parsed config files in a `HashSet` so I can know if a file has been parsed already. ~~I'm also using `std::fs::canonicalize`, as suggested in the issue, to prevent parsing files behind symbolic links more than once.~~ **UPDATE:** I removed the call to `fs::canonicalize` as suggested in the comments. Now the fix is way simpler, which means less code and less possibilities to add a new bug.
|
I think this issue can be closed, the PR has been merged. |
|
Thanks @jhbabon! |
|
Thank you! |
If I use this in ~/.cargo/config, the flag is passed twice:
This does not apply to configuration directories stored directly under my project's root (using
projectname/.cargo/config, the option is passed only once). There is only a single cargo config file involved in each scenario.For this particular flag, it stops the build with an error:
Also related: #3052, but this issue is for the specific bug of doubling the flag.
The text was updated successfully, but these errors were encountered: