Overhaul how cargo treats profiles #1441

alexcrichton · 2015-03-23T18:58:08Z

This commit is a complete overhaul of how Cargo handles compilation profiles
internally. The external interface of Cargo is not affected by this change.

Previously each Target had a Profile embedded within it. Conceptually a Target
is an entry in the manifest (a binary, benchmark, etc) and a Profile controlled
various flags (e.g. --test, -C opt-level, etc). Each Package then contained many
profiles for each possible compilation mode. For example a Package would have
one target for testing a library, benchmarking a library, documenting a library,
etc. When it came to building these targets, Cargo would filter out the targets
listed to determine what needed to be built.

This filtering was largely done based off an "environment" represented as a
string. Each mode of compilation got a separate environment string like "test"
or "bench". Altogether, however, this approach had a number of drawbacks:

Examples in release mode do not currently work. This is due to how examples
are classified and how release mode is handled (e.g. the "release" environment
where examples are meant to be built in the "test" environment).
It is not trivial to implement cargo test --release today.
It is not trivial to implement cargo build --bin foo where only the binary
foo is built. The same is true for examples.
It is not trivial to add selective building of a number of
binaries/examples/etc.
Filtering the list of targets to build has some pretty hokey logic that
involves pseudo-environments like "doc-all" vs "doc". This logic is duplicated
in a few places and in general is quite brittle.
The TOML parser greatly affects compilation due to the time at which profiles
are generated, which seems somewhat backwards.
Profiles must be overridden, but only partially, at compile time becuase only
the top-level package's profile is applied.

In general, this system just needed an overhaul. This commit made a single
change of separating Profile from Target and then basically hit make until
all the tests passed again. The other large architectural changes are:

Environment strings are now entirely gone.
Filters are implemented in a much more robust fashion.
Release mode is now handled much more gracefully.
The unit of compilation in the backend is no longer (package, target) but
rather (package, target, profile). This change had to be propagated many
location in the cargo_rustc module.
The backend does not filter targets to build at all. All filtering now
happens entirely in the frontend.

I'll test issues after this change lands, but the motivation behind this is to
open the door to quickly fixing a number of outstanding issues against Cargo.
This change itself is not intended to close many bugs.

This commit is a complete overhaul of how Cargo handles compilation profiles internally. The external interface of Cargo is not affected by this change. Previously each Target had a Profile embedded within it. Conceptually a Target is an entry in the manifest (a binary, benchmark, etc) and a Profile controlled various flags (e.g. --test, -C opt-level, etc). Each Package then contained many profiles for each possible compilation mode. For example a Package would have one target for testing a library, benchmarking a library, documenting a library, etc. When it came to building these targets, Cargo would filter out the targets listed to determine what needed to be built. This filtering was largely done based off an "environment" represented as a string. Each mode of compilation got a separate environment string like `"test"` or `"bench"`. Altogether, however, this approach had a number of drawbacks: * Examples in release mode do not currently work. This is due to how examples are classified and how release mode is handled (e.g. the "release" environment where examples are meant to be built in the "test" environment). * It is not trivial to implement `cargo test --release` today. * It is not trivial to implement `cargo build --bin foo` where *only* the binary `foo` is built. The same is true for examples. * It is not trivial to add selective building of a number of binaries/examples/etc. * Filtering the list of targets to build has some pretty hokey logic that involves pseudo-environments like "doc-all" vs "doc". This logic is duplicated in a few places and in general is quite brittle. * The TOML parser greatly affects compilation due to the time at which profiles are generated, which seems somewhat backwards. * Profiles must be overridden, but only partially, at compile time becuase only the top-level package's profile is applied. In general, this system just needed an overhaul. This commit made a single change of separating `Profile` from `Target` and then basically hit `make` until all the tests passed again. The other large architectural changes are: * Environment strings are now entirely gone. * Filters are implemented in a much more robust fashion. * Release mode is now handled much more gracefully. * The unit of compilation in the backend is no longer (package, target) but rather (package, target, profile). This change had to be propagated many location in the `cargo_rustc` module. * The backend does not filter targets to build *at all*. All filtering now happens entirely in the frontend. I'll test issues after this change lands, but the motivation behind this is to open the door to quickly fixing a number of outstanding issues against Cargo. This change itself is not intended to close many bugs.

This adds a new `--bin` flag to `cargo test` to specifically say that a binary should be tested. Additionally the dependencies are tweaked such that binaries to not depend on themselves being available.

If a profile's `test` flag is set to true, then it will always generate a unit test so the `is_test` check is redundant.

The flag now only applies to integration tests, not all targets with the specified name.

Libraries are no longer built if they aren't doctested, so something else needed to be present to trigger the library being built.

If no filter is supplied, then only consider binaries as candidates, not examples as well.

This is required for `cargo run` to work

Binaries are no longer always built when examples are built, so update the test accordingly.

The dependency is just there to get them to compile, not to actually induce recompiles.

Both tweak how deep documentation is compiled as well as fixing up the dependencies for a documented target.

rust-highfive · 2015-03-23T18:58:19Z

r? @huonw

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

alexcrichton · 2015-03-23T18:58:34Z

This is me finally following up to #1325

alexcrichton · 2015-03-23T18:59:21Z

cc @wycats

alexcrichton · 2015-03-23T19:01:43Z

This will also unblock some work related to rust-lang/rust#23533 I've got pending.

tamird · 2015-03-23T21:41:25Z

src/bin/bench.rs

@@ -50,20 +50,31 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
    let root = try!(find_root_manifest_for_cwd(options.flag_manifest_path));
    config.shell().set_verbose(options.flag_verbose);

+    let mut benches = Vec::new();


this vector looks unnecessary - seems you could use options.flag_bench.as_slice()

I'm currently trying to keep Cargo on stable Rust as much as possible, and unfortunately these methods are currently unstable. I had not considered this though, and it's a good idea! I may try to facilitate the stability of as_slice now...

alexcrichton · 2015-03-23T23:23:27Z

I can also squash any number of commits as desired or break up the first commit if necessary. I started by lumping as much as possible into one commit but eventually started making one fix per mistake, so the first commit is somewhat biasedly large!

brson · 2015-03-23T23:34:52Z

@bors r+

bors · 2015-03-23T23:34:53Z

📌 Commit 3ce79da has been approved by brson

bors · 2015-03-23T23:34:54Z

⌛ Testing commit 3ce79da with merge 0428b88...

bors · 2015-03-23T23:53:08Z

💔 Test failed - cargo-win-32

alexcrichton · 2015-03-23T23:57:09Z

@bors: r=brson acd1304

bors · 2015-03-23T23:57:10Z

⌛ Testing commit acd1304 with merge 060c59f...

bors · 2015-03-24T00:07:02Z

💔 Test failed - cargo-win-32

alexcrichton · 2015-03-24T00:15:19Z

@bors: r=bson 0c28783

bors · 2015-03-24T00:15:21Z

⌛ Testing commit 0c28783 with merge c6b9324...

This commit is a complete overhaul of how Cargo handles compilation profiles internally. The external interface of Cargo is not affected by this change. Previously each Target had a Profile embedded within it. Conceptually a Target is an entry in the manifest (a binary, benchmark, etc) and a Profile controlled various flags (e.g. --test, -C opt-level, etc). Each Package then contained many profiles for each possible compilation mode. For example a Package would have one target for testing a library, benchmarking a library, documenting a library, etc. When it came to building these targets, Cargo would filter out the targets listed to determine what needed to be built. This filtering was largely done based off an "environment" represented as a string. Each mode of compilation got a separate environment string like `"test"` or `"bench"`. Altogether, however, this approach had a number of drawbacks: * Examples in release mode do not currently work. This is due to how examples are classified and how release mode is handled (e.g. the "release" environment where examples are meant to be built in the "test" environment). * It is not trivial to implement `cargo test --release` today. * It is not trivial to implement `cargo build --bin foo` where *only* the binary `foo` is built. The same is true for examples. * It is not trivial to add selective building of a number of binaries/examples/etc. * Filtering the list of targets to build has some pretty hokey logic that involves pseudo-environments like "doc-all" vs "doc". This logic is duplicated in a few places and in general is quite brittle. * The TOML parser greatly affects compilation due to the time at which profiles are generated, which seems somewhat backwards. * Profiles must be overridden, but only partially, at compile time becuase only the top-level package's profile is applied. In general, this system just needed an overhaul. This commit made a single change of separating `Profile` from `Target` and then basically hit `make` until all the tests passed again. The other large architectural changes are: * Environment strings are now entirely gone. * Filters are implemented in a much more robust fashion. * Release mode is now handled much more gracefully. * The unit of compilation in the backend is no longer (package, target) but rather (package, target, profile). This change had to be propagated many location in the `cargo_rustc` module. * The backend does not filter targets to build *at all*. All filtering now happens entirely in the frontend. I'll test issues after this change lands, but the motivation behind this is to open the door to quickly fixing a number of outstanding issues against Cargo. This change itself is not intended to close many bugs.

bors · 2015-03-24T00:26:04Z

☀️ Test successful - cargo-linux-32, cargo-linux-64, cargo-mac-32, cargo-mac-64, cargo-win-32, cargo-win-64

bors · 2015-03-24T00:26:06Z

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

alexcrichton added 22 commits March 20, 2015 17:20

Fix cargo test filtering for binaries

26cf004

This adds a new `--bin` flag to `cargo test` to specifically say that a binary should be tested. Additionally the dependencies are tweaked such that binaries to not depend on themselves being available.

Remove an extraneous is_test condition

5408fd7

If a profile's `test` flag is set to true, then it will always generate a unit test so the `is_test` check is redundant.

Fix test for --test filtering

1a452fb

The flag now only applies to integration tests, not all targets with the specified name.

Fix cargo test --no-run

07b53ec

Fix configuring lib targets with profile options

0a187e7

Only doctest libs by default

eb2b6f0

Fix a subtesting test

a54d240

Libraries are no longer built if they aren't doctested, so something else needed to be present to trigger the library being built.

Don't fail cargo run with bins + examples

48a5d75

If no filter is supplied, then only consider binaries as candidates, not examples as well.

List examples in binaries built

fb8a3e3

This is required for `cargo run` to work

Fix test for bins/examples

8989d82

Binaries are no longer always built when examples are built, so update the test accordingly.

Update error message for missing examples

c18158b

Fix cargo run with no binaries

fc9212c

Don't compile build scripts if they aren't run

891da62

Update some error messages in tests

a83a50f

Fix fingerprints of build scripts that aren't compiled

9adbdef

Don't factor bins into fingerprint calculations

201aa61

The dependency is just there to get them to compile, not to actually induce recompiles.

Update tests for tweaked error messages

7d1419d

Fix cargo doc

8350cd1

Both tweak how deep documentation is compiled as well as fixing up the dependencies for a documented target.

Fix documenting binaries with libraries

1273003

Fix trying to document build scripts

8bebb3e

Fix printing Fresh for crates

23d3def

rust-highfive assigned huonw Mar 23, 2015

tamird reviewed Mar 23, 2015
View reviewed changes

rust-highfive assigned brson and unassigned huonw Mar 23, 2015

alexcrichton mentioned this pull request Mar 23, 2015

Normalize hyphens to underscores in crate names #1443

Merged

len() == 0 => is_empty()

3ce79da

Fix tests for windows

0c28783

alexcrichton force-pushed the issue-1323 branch from acd1304 to 0c28783 Compare March 24, 2015 00:15

bors merged commit 0c28783 into rust-lang:master Mar 24, 2015

alexcrichton deleted the issue-1323 branch March 27, 2015 00:09

dcuddeback mentioned this pull request Mar 29, 2015

Doc test link failure with native dependencies #1469

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Overhaul how cargo treats profiles #1441

Overhaul how cargo treats profiles #1441

alexcrichton commented Mar 23, 2015

rust-highfive commented Mar 23, 2015

alexcrichton commented Mar 23, 2015

alexcrichton commented Mar 23, 2015

alexcrichton commented Mar 23, 2015

tamird Mar 23, 2015

alexcrichton Mar 23, 2015

alexcrichton commented Mar 23, 2015

brson commented Mar 23, 2015

bors commented Mar 23, 2015

bors commented Mar 23, 2015

bors commented Mar 23, 2015

alexcrichton commented Mar 23, 2015

bors commented Mar 23, 2015

bors commented Mar 24, 2015

alexcrichton commented Mar 24, 2015

bors commented Mar 24, 2015

bors commented Mar 24, 2015

bors commented Mar 24, 2015

Overhaul how cargo treats profiles #1441

Overhaul how cargo treats profiles #1441

Conversation

alexcrichton commented Mar 23, 2015

rust-highfive commented Mar 23, 2015

alexcrichton commented Mar 23, 2015

alexcrichton commented Mar 23, 2015

alexcrichton commented Mar 23, 2015

tamird Mar 23, 2015

Choose a reason for hiding this comment

alexcrichton Mar 23, 2015

Choose a reason for hiding this comment

alexcrichton commented Mar 23, 2015

brson commented Mar 23, 2015

bors commented Mar 23, 2015

bors commented Mar 23, 2015

bors commented Mar 23, 2015

alexcrichton commented Mar 23, 2015

bors commented Mar 23, 2015

bors commented Mar 24, 2015

alexcrichton commented Mar 24, 2015

bors commented Mar 24, 2015

bors commented Mar 24, 2015

bors commented Mar 24, 2015