rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.83k stars 2.42k forks source link

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

Open lilyball opened 3 years ago

lilyball commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

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.

Eh2406 commented 2 years ago

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

olson-dan commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

btw https://github.com/rust-lang/cargo/issues/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.

weihanglo commented 1 year ago

Related issue:

ehuss commented 1 year ago

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