Profile specific feature selection.

[withoutboats]

Rendered

[est31]

I think you got a mistake here.

[withoutboats]

yes that should say features 😊

[est31]

What is the difference of this proposal to simply replacing the features table?

Also, the toml standard calls them arrays btw, and calls tables what you call objects.

[withoutboats]

Merging the table means that you don't need to repeat all of its elements. For example:

[features]
foo = []
bar = []

[profile.dev.features]
bar = ["foo"]

If we replaced the table, this wouln't be sensical because there would no longer be a foo feature.

[est31]

Ah, I think I got confused a bit, and thought you meant the features array (the features = ["foo", "bar", "baz"] thing), nvm.

[est31]

Regarding the motivation section

I couldn't find any mentions of concrete use cases in the motivation section, only general "sometimes users wish to control". I'm interested in examples for all profiles, but especially in examples in dev vs release.

Regarding replacing of settings (features table, dependencies table, ...) in the default config

What to do in the case when two profiles are used simultaneously? E.g. test and release? Which setup wins?

Also, replacing settings would be inconsistent with what dev dependencies do. ATM you get an error when you include a crate both into dev dependencies and dependencies, and those two inclusions vary in any way (like source of inclusion, or version).

IMO the only place where we should allow replacements is by the choice of features.

Regarding dev-dependencies

I think if we allow dev-dependencies to be set per profile, this addition would lead to a great deal of confusion. First about whether something should be put into the default dev-dependencies or into the dependencies of a profile, and then second about whether some profile specific dependency should be put into its dev-dependencies or not.

Giving an example: As author of a crate A, you will likely ask yourself whether to put some dependency B which is only needed to test A itself into the dev-dependencies section or into the dependencies section of the test profile. I'd say the dev-dependencies section should be preferred, as in (at least I think so, haven't confirmed it) when you cargo test a crate C with A as (normal) dependency, both A and B will be compiled with a test profile. Then if you put B into the test-profile-dependencies section of A, you'll end up with B included, even if it isn't needed at all.

This example doesn't just expose a source for confusion, but maybe also hints to whether we should have a test-inner profile that non leaf (as in dependency) crates get compiled under.

Another question worth considering is that there is the dev-dependencies use case for deps only needed by the examples you have. Obviously, if we chose to deprecate dev-dependencies, those need a new home as well. Maybe we could introduce an examples profile?

Generally I would support deprecating dev-dependencies overall (as they are mostly a simpler version of the proposed per profile dependencies) as long as the proposed replacement is simple and supports all use cases dev-dependencies supported. I'd like to propose that cargo should error if it encounters both per-profile and dev-dependencies. This is 100% compatible with existing Cargo.toml files, but demotes demote future usage of dev-dependencies, and helps to get rid of confusion regarding that system.

[withoutboats]

I couldn't find any mentions of concrete use cases in the motivation section, only general "sometimes users wish to control". I'm interested in examples for all profiles, but especially in examples in dev vs release.

Yea, the use cases are mentioned in detailed design and should be prefigured in the motivation section. I'll edit.

What to do in the case when two profiles are used simultaneously? E.g. test and release? Which setup wins?

As we discussed in IRC, I don't think this happens today (cargo test --release runs the test harness in the release profile) but if it does, we already have to resolve the two profiles somehow and so this should just be consistent with that.

IMO the only place where we should allow replacements is by the choice of features.

This is the motivation for the proposal, anything else would be quite questionable (such as changing the version of a dependency in a profile). I didn't rule them out because it seemed to me like trying to do those things would be self-deterring & maybe there are valid use cases I haven't imagined.

Will think more about your dev dependencies comments and respond to that later.

[est31]

cargo test --release runs the test harness in the release profile

Repeating what I've pointed out on IRC: if that's the case (very reasonable I think), then the new proposal would break the cargo test --release feature, as you'd only include your deps into the test profile, but not the release profile. I wouldn't like that.

[withoutboats]

@est31 to clarify you mean it doesn't subsume the use case of dev-dependencies, because those run in the test harness (and example harness) regardless of profile?

I think the conclusion I've drawn is that the answer to the question "can we deprecate dev-dependencies?" is "no."

[est31]

you mean it doesn't subsume the use case of dev-dependencies, because those run in the test harness (and example harness) regardless of profile?

Yes. Of course, we could tie the dependencies to the harness and not the profile. But that would obviously be a much bigger change than the one proposed, and be more confusing upfront (although the confusion later on would be lower). I think it would be a bad idea to do it.

I think the conclusion I've drawn is that the answer to the question "can we deprecate dev-dependencies?" is "no."

Okay, but there is still the question whether the dev-dependencies should be included into the profiles.

Thinking this further, with the fact that the harness is separate from the profile, does putting custom features and dependencies, things that actually modify behaviour, into the profile even make sense? I mean essentially it would mean that in profile features and dependencies should only change things like speed or logging verbosity or such, but not actual behaviour or anything that may break the tests. I'm not sure whether we will be able to communicate that, and whether people will actually adopt that change.

We could also maybe solve the situation by deprecating the test, bench and doc profiles, idk.

[Ericson2314]

At first I was really against this. Then I thought some more, and decided the real problem is it's unclear what profiles are supposed to be at all. In any event, my confusion here I think a perfect example of what @eternaleye brought up in #1953 of Cargo's current manifest format
optimizing for the common case and avoiding a combinatory explosion of possibilities---which sounds great until one realizes we're stuck with having few orthogonal concepts and a syntax which is hard to expand sanely.

[eternaleye]

I don't think I'm in favor of this, since it seems like it tries to make an orthogonal axis out of something that really isn't. In particular, tests seem to me like a when (one that can be skipped), rather than a new axis orthogonal to "release". Tests require a superset of what's needed at runtime - both whatever's needed for normal usage, and whatever harness it uses to exercise it.

However, test-time is distinct from build-time - one may need (say) a C compiler, while the other does not. Or testing may require a mocking library that pulls in deps that someone who is building wants to avoid.

Meanwhile, "release" is almost totally unconnected to time. In fact, I'd consider it a significant antipattern to have different dependencies in "release" mode, as then you fundamentally are not testing the same code as you release.

[withoutboats]

@Ericson2314 & @eternaleye both of your points are well taken, but I do have a response to them.

---

@Ericson2314 like you, I am discomfited at the number of not-entirely-orthogonal knobs that cargo exposes to the user. In particular, dev-deps and profiles seem to be overlapping in purpose, with the relationship between different 'harnesses' (tests, examples, doc tests) and profiles extremely muddy (to put a point on it: I discovered, only by reading the source, that cargo test --release puts cargo in the bench profile, not the release profile).

But that's why I wrote this RFC as a minimal extension of an existing feature, with potential to cover multiple use cases, rather than creating a new feature that narrowly covered my own use case (conditional compilation in dev vs not dev). I do not think it would be accurate to describe this RFC as "optimizing for the common case and avoiding a combinatory explosion of possibilities."

If you want to deal with extricating the relationship between profiles, harnesses, and the three different dependency categories, to create a more orthogonal system, that seems like a potentially fruitful endeavor. That's just not what this RFC is for.

---

@eternaleye

Meanwhile, "release" is almost totally unconnected to time. In fact, I'd consider it a significant antipattern to have different dependencies in "release" mode, as then you fundamentally are not testing the same code as you release.

This is certainly a reasonable position. But your position is that conditional compilation is an antipattern. We already have conditional compilation, this RFC is about making it more controllable by end users.

My specific use case is to compile different code in release and dev modes. This is a library. In dev I want to dynamically load certain assets, so that users can edit them without recompiling their project. In release I want to include those assets into the source binary - not only for performance reasons but so that the final artefact is a single binary.

Now this might sound terrible to you, but put that aside - I'm going to do this regardless. But without this RFC, the way to do it is to use debug_assertions as the flag to control whether these assets are dynamic or static. This entangles this behavior with an unrelated feature, makes it hard for users to turn this behavior off, and (worst of all) means tests by default get the dev behavior instead of the release behavior.

I'd much rather create a dynamic-assets feature, turn it on during dev by default, and allow users who disagree with that decision to be able to turn it off in all profiles.

[Ericson2314]

like you, I am discomfited at the number of not-entirely-orthogonal knobs that cargo exposes to the user.

Whew!

I do not think it would be accurate to describe this RFC as "optimizing for the common case and avoiding a combinatory explosion of possibilities."

Sorry I worded that poorly, "my confusion here" is over profiles today, not this RFC. My concern about over-optimizing for the common case is also with the current schema, not this RFC.

That's just not what this RFC is for.

That is where I get worried. I think the current schema is confusing enough that we really ought to back to the drawing board before making any more piecemeal changes. It may be that simple schema changes are appropriate, but I just don't think we have the design foundation to fully justify them at the time. I rather have an overarching roadmap or something for what knobs Cargo ought to expose, and then think about what syntax (e.g. in light of what exists already) those knobs will take.

[Ericson2314]

@withoutboats reading over your written motivation in response to @eternaleye's points, I think I am noticing something that might be useful. Crucially there is a tension between describing a development environment for the library, and packaging a library for reuse. In the former case, as with a binary, the Cargo file is the root crate in the dependency dag, and thus we should be pretty lax about what is allowed so as not to get in the developer's way. In the latter case however, we have to consider the needs of downstream developers, who need not concern themselves with the idiosyncrasies of upstream developers. For their sake, we should really cramp down on what libraries are allowed to do to enforce consistency / best practices.

I think the solution here is a virtual workspace root. The (perhaps single) library in the workspace should not contain any of these sorts of things, and crates.io should enforce that uploaded libraries don't either. The workspace root however would have all these sorts of local developer conveniences that don't affect the library's definition, but do aid it's development. [As an aside, I am saddened that a crate can serve as a workspace root, and that this appears to be the most common usage. If we didn't allow this, the non-normative-stuff-in-workspace-root approach would be more obvious.]

And FWIW, I agree with @eternaleye that, if we indeed ought to have this notion of "profiles", it should be for tweaking "unobservable" things like optimizations, incremental compilation granularity, etc.

[withoutboats]

In the latter case however, we have to consider the needs of downstream developers, who need not concern themselves with the idiosyncrasies of upstream developers.

this is a feature for downstream developers' environment

[Ericson2314]

@withoutboats Hmm? To specifically narrow in on your example, wouldn't downstream always want the assets included into the library, as they are not changing them so no point bothering with reloading on the fly?

[withoutboats]

No these are their assets which the library (a web framework) loads for them (e.g. html/css templates)

[Ericson2314]

Hmm, so transitive usage is very much the intention.

[alexcrichton]

cc rust-lang/cargo#3870 and rust-lang/cargo#1982, another case for more targeted dependencies, in that case for binaries. (I think this syntax would basically work there as well)

[alexcrichton]

Overall this looks pretty good to me, thanks @withoutboats!

I think it may be worth considering the (cargo) target specific dependencies here (e.g. deps specific to one test, binary, benchmark, etc). This'd cover the PR/issue I cc'd above, and I feel like it would pretty naturally fit into this as well.

I sympathize with @eternaleye's concerns about features being different between release/dev in terms of then you're not testing what you're releasing, but I think you also bring up a compelling use case. I continue to want my deploy to "build release mode" and have that do all the right bits by default, and this'd be a good way to configure that.

One question I'd have is how would you use release-specific dependencies? Presumably you'd have to write down something in the code to have an extern crate directive, but what's the appropriate #[cfg] to tag it with?

One part I've never been super happy with in Cargo is how profiles turned out. The profiles used today are pretty muddied (as you've discovered with cargo test --release) and they're slightly sprawling. Eventually we'd also like to support custom profiles! I'm not sure there's much we can do about this though. There's at least a story for what profile is used where today, so we'd at least have that to cover ourselves.

cc @matklad, @wycats, @brson, @carols10cents

[withoutboats]

I sympathize with @eternaleye's concerns about features being different between release/dev in terms of then you're not testing what you're releasing, but I think you also bring up a compelling use case.

For differences like the one I raised, you probably want testing to behave like release (since you're not changing assets during tests!). Of course, someone could still not do that, but that's technically already true (by having significantly different behavior configured on debug_assertions).

One question I'd have is how would you use release-specific dependencies? Presumably you'd have to write down something in the code to have an extern crate directive, but what's the appropriate #[cfg] to tag it with?

One part I've never been super happy with in Cargo is how profiles turned out. The profiles used today are pretty muddied (as you've discovered with cargo test --release) and they're slightly sprawling. Eventually we'd also like to support custom profiles! I'm not sure there's much we can do about this though. There's at least a story for what profile is used where today, so we'd at least have that to cover ourselves.

You raise a good point about profile-specific dependencies, but I think it connects to your next point. It might be worth trying to revamp the profile system before accepting this RFC, possibly connecting it to features, letting you define your own profiles (I could see test being inadequate to cover both ci testing and local testing for example), and making cfg(release) or something like it Just Work.

[matklad]

I would love to see more concrete examples in the text of the RFC itself. The one in #1956 (comment) is great! It would be helpful to document existing workarounds as well.

Why the existing --feature flag is not enough here? I think it could be used for "dynamic assets loading" use case, no?

Let's say you've created a library l, which has [features] dynamic-assets = [] to enable dev-mod asset loading. Then the downstream client of your library can reexport this feature as [features] dynamic-assets = ["l/dynamic-assets"]. So they can run either cargo test or cargo test --feature dynamic-assets and that seems to solve the problem? Or am I missing something?

[alexcrichton]

@withoutboats oh so to be clear I think the case of a framework here is a solid one for different behavior. A local library would likely never want to test differently in release/production, but switching that for a well tested framework from a different party seems totally fine.

It's also true yeah that release differs from debug mode, not only with cfg(debug_assertions) but also optimizations in the compiler! I just share the sentiment of trying to minimize the number of differences as much as possible, and debug_assertions is at least typically seen as an "ok thing to disable" and tends to not cause too many problems (but there's always one...)

I also don't necessarily want to block this RFC on a revamp of the profile system, that's likely to take a long time! Having different sets of default features in different profiles seemed reasonable to me (easy to implement today, doesn't require a revamp) and seems like it would solve a number of your use cases? It was just profile-specific dependencies I wasn't sure how to implement.

[withoutboats]

@matklad The intent would be to make dynamic-assets a default feature, but only in the dev profile.

@alexcrichton

Having different sets of default features in different profiles seemed reasonable to me (easy to implement today, doesn't require a revamp) and seems like it would solve a number of your use cases?

Yes, if we just scoped this RFC to adding features objects (or even just default features) to the profile objects that would be enough for my use case! I agree it makes sense to deal with the holistic issues later.

[withoutboats]

I've redrafted this RFC to only control selecting features in specific profiles, rather than defining features & dependencies. It now has two components.

First, a default-features key in each profile, which overrides the default features when in that profile.

Second, the dependencies.$name.features and dependencies.$name.default-features can now be objects which contain keys for each profile, so that they can be independently specified in each profile.

I've also included a more concrete example in the motivation (the dynamic templates feature I described above).

[matklad]

I also don't necessarily want to block this RFC on a revamp of the profile system, that's likely to take a long time! Having different sets of default features in different profiles seemed reasonable to me (easy to implement today, doesn't require a revamp)

I think we must define and document when exactly the current profiles are activated before we implement this RFC. The current behavior is not documented anywhere as far as I know, and is very surprising.

cargo test will compile integration tests in test, the library they are testing in dev, and then it compile library and it's unit tests again in test.

cargo test --release will compile integration tests in bench, the library they are testing in release, and then it compile library and it's unit tests again in bench.

I would love to try to restrict this feature to only dev and release profiles, but this won't work because of the unit tests I think.

[withoutboats]

@matklad That is very surprising, and despite inspecting the code my belief about what profiles were used was wrong.

I would like to see this changed so that at very least everything is run under a single profile under each command, cargo test under "test" and cargo test --release under either "release" or "bench." @alexcrichton What do you think about making a change like this?

[alexcrichton]

Yeah a good bit of the current behavior around profiles is relatively historical. Now that we cache binaries based on profile configuration the door's more open to doing more flavorful things (and still having a reasonable experience). The main goal was to have cargo build followed by cargo test be a relatively quick turnaround time. Historically that'd thrash the cache if we literally built everything in the test profile for tests and dev profile for builds. Nowadays you'd have a one-time cost of building everything once in dev and once in test, but incremental builds would be much better.

I think it'd be fine to clarify that in this RFC as @matklad was saying, basically saying "these commands map to these profiles" and we can fix the bugs in Cargo in parallel perhaps

[withoutboats]

@alexcrichton Sounds good. Should cargo test --release correspond to release or bench?

[matklad]

we can fix the bugs in Cargo in parallel perhaps

Do you say that we want to change what profiles are activated when by cargo? I definitely want this, but it's not exactly 100% backwards compatible :( And if we do want the change, then we'll need to put some more thought into what profiles are exactly. As test --release question shows, current dev, test and release are not orthogonal, so perhaps we want to deprecate test, bench and doc as suggested by @est31, and which is a topic for a separate RFC.

[alexcrichton]

Oh well I don't necessarily want to open the can of worms here, but I think we'd definitely be up for straightening out the story of profiles in cargo... somehow. Not precisely sure how but surely there's a way to do it!

[mgattozzi]

There's a typo here. suhc should be such

[aturon]

I believe this is effectively blocked on a forthcoming rationalization of profiles. Closing as postponed for now.