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

Add -C <DIR> flag to cargo to change directory before invoking #10098

Open
lilyball opened this issue Nov 19, 2021 · 9 comments · Fixed by #10952
Open

Add -C <DIR> flag to cargo to change directory before invoking #10098

lilyball opened this issue Nov 19, 2021 · 9 comments · Fixed by #10952
Labels
A-cli Area: Command-line interface, option parsing, etc. A-configuration Area: cargo config files and env vars C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage.

Comments

@lilyball
Copy link
Contributor

Problem

Cargo infers the manifest path and reads config based on the current directory. Plugins may potentially locate other config this same way. Cargo commands offer a --manifest-path flag to override the manifest, but this doesn't override config.

For example, given the following:

> cat bin/foo/.cargo/config.toml
[build]
target = "wasm32-unknown-unknown"
target-dir = "../../target"
> cargo build --manifest-path bin/foo/Cargo.toml

The resulting build will use the host platform instead of wasm32-unknown-unknown and the build artifacts will end up in bin/foo/target. But were I to run cd bin/foo && cargo build it would use the correct target and target dir.

Proposed Solution

Cargo should have a flag -C <DIR> (or --directory <DIR>) that exists at the top level rather than in the subcommands. This flag would cause cargo to change its working directory to the specified one before processing any commands or loading any config. It would be roughly equivalent to ( cd <DIR> && cargo … ).

Besides fixing the issue of loading cargo config, this is also a simpler interface for end users than --manifest-path as it means they don't have to worry about where the specific Cargo.toml file is, they can just run cargo as though it were in a specific directory.

This flag is inspired by both make and git which offer the same -C <DIR> (make has the long --directory form, I don't believe git has a long form). The desired behavior here should be based on git as it also defines how multiple -C flags interact (subsequent ones are interpreted relative to the earlier ones). Like git, any other flags or parameters that take relative paths should then be interpreted relative to the working directory caused by the -C option.

Notes

Regarding rustup and rust-toolchain.toml overrides, I'm inclined to say rustup should honor this flag as well and interpret it prior to locating an override file. The idea here is that saying cargo -C $path … should behave indistinguishably from running cargo within that path.

@lilyball lilyball added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Nov 19, 2021
@lilyball
Copy link
Contributor Author

This request was inspired by my surprised at cargo build --manifest-path bin/foo/Cargo.toml not applying the config from bin/foo/.cargo/config.toml. I considered filing that as a bug, but I'm not sure what the actual correct behavior there should be, whereas a -C flag has an obvious answer (and is easier to use).

@thomcc
Copy link
Member

thomcc commented Nov 30, 2021

Yeah, I hit this issue too. I think ideally --manifest-path made cargo do the right thing here, although I guess that would be a breaking change, and I can see some use cases for it using the current directory.

@ehuss ehuss added A-cli Area: Command-line interface, option parsing, etc. A-configuration Area: cargo config files and env vars labels Dec 6, 2021
@Eh2406
Copy link
Contributor

Eh2406 commented Dec 10, 2021

I believe that @joshtriplett have brought this up at cargo meetings before, but I cannot find notes at this time.

@olson-dan
Copy link

Ran into this today as well, let me just echo that it's pretty unfortunate that using --manifest-path produces a different and potentially incorrect build depending on where it's invoked from. I'd prefer if config location started in the directory of the manifest than a new config option.

Potentially this might not be breaking, because anyone depending on the current behavior would have to be building from the root of the project anyway.

@lilyball
Copy link
Contributor Author

At this point, I think that the current behavior of --manifest-path is actually correct. The .cargo/config.toml file controls the behavior of cargo itself, it's not crate configuration, and so the location I invoke cargo from should be what determines how to find the cargo config. Having a separate -C <DIR> flag solves this. Especially the part about toolchain overrides (e.g. rust-toolchain.yaml) as that is explicitly based on your current path.

@compenguy
Copy link
Contributor

At this point, I think that the current behavior of --manifest-path is actually correct. The .cargo/config.toml file controls the behavior of cargo itself, it's not crate configuration, and so the location I invoke cargo from should be what determines how to find the cargo config.

This is contrary to the cargo config documentation, which says:

With this structure, you can specify configuration per-package, and even possibly check it into version control. You can also specify personal defaults with a configuration file in your home directory.

the per-package .cargo/config.toml checked into version control is being ignored when built via --manifest-path, unless the cwd is currently somewhere within the package directory tree, but the primary use case for --manifest-path is to make build independent of cwd. This is either a cargo bug, or a cargo documentation bug, take your pick, but my vote is for cargo bug.

@epage
Copy link
Contributor

epage commented Jul 26, 2022

btw #2930 is the issue about the config being relative to CWD rather than --manifest-path. This issue is more about acknowledging things as they currently are and providing a mechanism for dealing with it

the per-package .cargo/config.toml checked into version control is being ignored when built via --manifest-path, unless the cwd is currently somewhere within the package directory tree, but the primary use case for --manifest-path is to make build independent of cwd. This is either a cargo bug, or a cargo documentation bug, take your pick, but my vote is for cargo bug.

I could see there being some minor wording tweaks to the documentation. My guess is its written assuming auto-discovery of the manifest in which case, it is correct. That assumption should be made more clear though.

compenguy pushed a commit to compenguy/meta-rust that referenced this issue Jul 26, 2022
Cargo documentation for configuration toml says that a .cargo/config
checked into revision control in the project root should be picked up by
cargo, but in fact cargo only searches relative to cwd.  In order to
achieve the documented result, cargo needs to be invoked with the
current working directory already set to the directory containing the
manifest file.

see rust-lang/cargo#10098
compenguy pushed a commit to compenguy/cargo that referenced this issue Aug 8, 2022
This implements the suggestion in rust-lang#10098 to make cargo change cwd before
completing config processing and starting the build. It is also an
alternative to --manifest-path that resolves the issue described
in rust-lang#2930.
compenguy pushed a commit to compenguy/cargo that referenced this issue Aug 8, 2022
This implements the suggestion in rust-lang#10098 to make cargo change cwd before
completing config processing and starting the build. It is also an
alternative to --manifest-path that resolves the issue described
in rust-lang#2930.
compenguy pushed a commit to compenguy/cargo that referenced this issue Aug 16, 2022
This implements the suggestion in rust-lang#10098 to make cargo change cwd before
completing config processing and starting the build. It is also an
alternative to --manifest-path that resolves the issue described
in rust-lang#2930.
@weihanglo
Copy link
Member

compenguy pushed a commit to compenguy/cargo that referenced this issue Feb 1, 2023
This implements the suggestion in rust-lang#10098 to make cargo change cwd before
completing config processing and starting the build. It is also an
alternative to --manifest-path that resolves the issue described
in rust-lang#2930.
compenguy pushed a commit to compenguy/cargo that referenced this issue Feb 1, 2023
This implements the suggestion in rust-lang#10098 to make cargo change cwd before
completing config processing and starting the build. It is also an
alternative to --manifest-path that resolves the issue described
in rust-lang#2930.
compenguy pushed a commit to compenguy/cargo that referenced this issue Feb 1, 2023
This implements the suggestion in rust-lang#10098 to make cargo change cwd before
completing config processing and starting the build. It is also an
alternative to --manifest-path that resolves the issue described
in rust-lang#2930.
compenguy pushed a commit to compenguy/cargo that referenced this issue Feb 1, 2023
This implements the suggestion in rust-lang#10098 to make cargo change cwd before
completing config processing and starting the build. It is also an
alternative to --manifest-path that resolves the issue described
in rust-lang#2930.
compenguy pushed a commit to compenguy/cargo that referenced this issue Feb 1, 2023
This implements the suggestion in rust-lang#10098 to make cargo change cwd before
completing config processing and starting the build. It is also an
alternative to --manifest-path that resolves the issue described
in rust-lang#2930.
compenguy pushed a commit to compenguy/cargo that referenced this issue Feb 1, 2023
This implements the suggestion in rust-lang#10098 to make cargo change cwd before
completing config processing and starting the build. It is also an
alternative to --manifest-path that resolves the issue described
in rust-lang#2930.
compenguy pushed a commit to compenguy/cargo that referenced this issue Feb 1, 2023
This implements the suggestion in rust-lang#10098 to make cargo change cwd before
completing config processing and starting the build. It is also an
alternative to --manifest-path that resolves the issue described
in rust-lang#2930.
compenguy pushed a commit to compenguy/cargo that referenced this issue Feb 2, 2023
This implements the suggestion in rust-lang#10098 to make cargo change cwd before
completing config processing and starting the build. It is also an
alternative to --manifest-path that resolves the issue described
in rust-lang#2930.
compenguy added a commit to compenguy/cargo that referenced this issue Feb 10, 2023
This implements the suggestion in rust-lang#10098 to make cargo change cwd before
completing config processing and starting the build. It is also an
alternative to --manifest-path that resolves the issue described
in rust-lang#2930.
@bors bors closed this as completed in 7404367 Feb 10, 2023
Ethan-000 pushed a commit to Ethan-000/cargo that referenced this issue Feb 11, 2023
This implements the suggestion in rust-lang#10098 to make cargo change cwd before
completing config processing and starting the build. It is also an
alternative to --manifest-path that resolves the issue described
in rust-lang#2930.
@ehuss
Copy link
Contributor

ehuss commented Apr 12, 2023

Reopening since in #11960 we are planning to mark -C as unstable for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli Area: Command-line interface, option parsing, etc. A-configuration Area: cargo config files and env vars C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants