[WIP, do not commit] set Cargo.toml configuration from the command line

[froydnj]

This pull request introduces cargo build -c CONFIG for setting Cargo.toml key-value pairs from the command line, suggested in #4349. Any config settings provided on the command line override those set in Cargo.toml. This sort of thing is probably most useful for setting profile.$PROFILE.$X options, but there are a number of other possibilities.

It's not complete--there are a few XXX comments, tests are needed, and it's an open question how many commands should support this--but it is at least in a state where I thought feedback could be provided. @alexcrichton WDYT? Is this heading in the right direction?

[rust-highfive]

r? @matklad

(rust_highfive has picked a reviewer for you, use r? to override)

[alexcrichton]

Thanks for the PR @froydnj!

This seems pretty solid to me as an idea in terms of an implementation, especially b/c it just merges at the level of parsed TOML.

I'd have a few reservations about the overall approach, though:

• This seems pretty powerful, almost too powerful, for what we'd like? For example this can override package names, package build targets, etc. The original motivation is just profile configuration, right?
• I'm worried that this may not be what we'd always want for "overriding things" in the sense of what Cargo.toml does --config apply to? Does it apply to the --manifest-path? Does it apply to the -p argument passed on the command line? The workspace? (etc). This is something that plagues Cargo a lot today where 'targeted arguments' are difficult to say where they're actually being targeted.

In terms of a solution for the problem at hand (especially if you'd like to backport to beta) my preference would be a CARGO_BUILD_LTO env var as it should be even more unobtrusive in terms of future possibilities of it. We'd be pretty likely to deprecate it though!

[alexcrichton]

Ah and also cc @rust-lang/cargo

[froydnj]

This seems pretty powerful, almost too powerful, for what we'd like? For example this can override package names, package build targets, etc. The original motivation is just profile configuration, right?

The original motivation is just profile configuration, yes. It's possible that we could restrict the power somewhat, either by:

• --profile-config "release = { lto = false }" to make it explicit it only applies to profiles; or
• Validating that we're only modifying the profile table once we've parsed all the passed options and keeping the --config argument name. This would leave the door open to loosening that restriction in the future.

I'm worried that this may not be what we'd always want for "overriding things" in the sense of what Cargo.toml does --config apply to? Does it apply to the --manifest-path? Does it apply to the -p argument passed on the command line? The workspace? (etc). This is something that plagues Cargo a lot today where 'targeted arguments' are difficult to say where they're actually being targeted.

Assuming I understand things correctly, --config applies to whatever Cargo.toml we find to build: the workspace of the auto-discovered Cargo.toml or the workspace of the --manifest-path argument. I'm not 100% on this, but given that -p selects something out of the auto-discovered Cargo.toml or --manifest-path, whichever is provided, I think that case is taken care of too. Not sure that answers your question, exactly, but the confusion certainly resonates with your concerns about complication!

CARGO_BUILD_LTO=1 would certainly be more expedient...though rather than adding that single-purpose thing, I think we'd be better served by making rustc accept -C lto for all sorts of compiles: more discoverable, a little more general-purpose, and nothing to deprecate if we do wind up adding a more powerful configuration mechanism later.

[aturon]

The Cargo team plans to discuss this PR, and the broader context, when we meet tomorrow morning. Should have an update for you after that!

[sfackler]

Cargo already looks at environment variables to override bits of .cargo/config (e.g. CARGO_TARGET_X86_64_UNKNOWN_LINUX_MUSL_LINKER=musl-gcc). We could potentially do the same behavior for Cargo.toml? CARGO_TOML_TARGET_RELEASE_LTO=false.

[alexcrichton]

@froydnj hm ok you excellent point! The more I think about this though the more I'm thinking that the only section of Cargo.toml you'd want to change "dynamically" is the profiles section. The reason for this is pretty related to the "what Cargo.toml are we modifying" question.

You mentioned "--config applies to whatever Cargo.toml we find to build" which is I think sort of correct, but it's a bit deeper than this as well. This can lead to some perhaps surprising things though. For example:

# Here we're selecting the package `foo`, but we're actually 
# building something else. Should we modify `bar`'s Cargo.toml?
$ cargo build --manifest-path foo/Cargo.toml -p bar

# Same as above, but now we're building two unrelated packages!
$ cargo build --manifest-path foo/Cargo.toml -p bar  -p baz

# Let's say here that `ws/Cargo.toml` is a workspace configuration. 
# Are we editing the workspace's cargo.toml? the package we're building?
$ cargo build --manifest-path ws/Cargo.toml -p member-of-ws

In a sense the [profile] section is the only "global" one. The build only selects one set of profiles to be used, and those are only taken from one Cargo.toml (the workspace root). In that sense I think it makes a lot of sense for command line overrides of global information. For example we have, as @sfackler mentions, environment-variable based overrides of ~/.cargo/config, and that level of configuration is a global resource.

Basically along those lines I think there's a strong case to be made here for "profiles" specifically being overridden at build time, as they're a global resource. Similarly I think it naturally, to me at least, leads to the conclusion that environment-based Cargo.toml overrides (like @sfackler mentioned) may not work out as it's a global configuration overriding a non-global resource.

(boy does placing [profile] in Cargo.toml seem odd now!)

So in that sense your alternative of --profile-config "release = { lto = false }" sounds pretty appealing to me. I'm led to the conclusion that we'll only ever want to allow command line and/or global configuration of build profiles, not of other fields in Cargo.toml.

Following that line of thinking, I'm actually sort of coming to the conclusion that we should just add flags everywhere! Taking TOML on the command line is a bit odd (seems weird to me at least), but I could imagine us adding something like:

    --opt-level N               Optimization level to use, inferred by default
    --debuginfo N               Debug information level to use, inferred by default
    --lto BOOL                  Enable or disable LTO, inferred by default
    --codegen-units N           Number of codegen units to use, inferred by default
    --debug-assertions BOOL     Enable or disable debug assertions, inferred by default
    --overflow-checks BOOL      Enable or disable overflow-checks, inferred by default

That is, every "reasonable" option in [profile] would have a command line flag that takes precedent over what's listed in Cargo.toml. That way Cargo.toml could be a "reasonable" set of defaults for "ergonomic builds", but build systems like Gecko would pass in all arguments for other sorts of configuration.

Again though I think the most expedient solution would probably be CARGO_BUILD_LTO=true in Cargo, and we can always deprecate that later. At this red-hot second, though, adding more CLI flags seems like the most attractive "long term" and extensible solution!

[bors]

☔ The latest upstream changes (presumably #4400) made this pull request unmergeable. Please resolve the merge conflicts.

[alexcrichton]

I'm going to close this due to inactivity, and I think the various issues were solved or otherwise worked around in the meantime?