Auto merge of #5300 - djc:namespaced-features, r=alexcrichton
Introduction of namespaced features (see #1286)
I think this basically covers all of the plans from #1286, although it still needs a bunch of tests and documentation updates. Submitting this PR to get some early feedback on the direction.
As discussed at https://github.com/rust-lang/cargo/issues/5364, the new behavior unfortunately causes real-life breakage, so we have to revert it.
This is kinda sad, this is a part of the larger issue with feature selection, which, at the moment, has a behavior which I would classify (loosely speaking) as unsound:
* `cargo build -p foo` and `cargo build -p foo -p bar` might produce different artifacts for `foo` ([repro](https://github.com/matklad/workspace-vs-feaures))
* `cargo build -p foo` might produce different artifacts, depending on cwd ([repro](https://github.com/matklad/features-cwd))
The new feature behavior specifically addressed the second point.
It is unclear what we could do with this... One option, instead of flatly erroring out, as the revreted commit does, is to print a warning, but change the set of activated features. It will still be a breaking change, but it at least has a chance of working by accident.
Take feature namespace into account while building summary (fixes #1286)
Here's an attempt at a table to cover the different cases:
Feature
Old (must be in features table)
Continue
Namespaced (might be stray value)
In features table: Check that Crate dependency is in the list
-> Non-optional dependency: Bail [PREVIOUSLY: bailed for non-optional dependency]
-> Optional dependency: Insert feature of this name
-> Else: Bail [PREVIOUSLY: bailed for unknown dependency or feature]
Crate
Old (might be stray value)
Non-optional dependency: Bail
No dependency found: Bail
Namespaced
Non-optional dependency: Bail
No dependency found: Bail
CrateFeature
Old
No dependency found: Bail
Namespaced
No dependency found: Bail
Auto merge of #5384 - ehuss:profile-override, r=matklad
Profile Overrides (RFC #2282 Part 1)
Profile Overrides (RFC #2282 Part 1)
WIP: Putting this up before I dig into writing tests, but should be mostly complete. I also have a variety of questions below.
This implements the ability to override profiles for dependencies and build scripts. This includes a general rework of how profiles work internally. Closes #5298.
Profile overrides are available with `profile-overrides` set in `cargo-features` in the manifest.
Part 2 is to implement profiles in config files (to be in a separate PR).
General overview of changes:
- `Profiles` moved to `core/profiles.rs`. All profile selection is centralized there.
- Removed Profile flags `test`, `doc`, `run_custom_build`, and `check`.
- Removed `Profile` from `Unit` and replaced it with two enums: `CompileMode` and `ProfileFor`. This is the minimum information needed to compute profiles at a later stage.
- Also removed `rustc_args`/`rustdoc_args` from `Profile` and place them in `Context`. This is currently not very elegant because it is a special case, but it works. An alternate solution I considered was to leave them in the `Profile` and add a special uber-override layer. Let me know if you think it should change.
- Did some general cleanup in `generate_targets`.
## Misc Fixes
- `cargo check` now honors the `--release` flag. Fixes #5218.
- `cargo build --test` will set `panic` correctly for dependences. Fixes #5369.
- `cargo check --tests` will no longer include bins twice (once as a normal check, once as a `--test` check). It only does `--test` check now.
- Similarly, `cargo check --test name` no longer implicitly checks bins.
- Examples are no longer considered a "test". (See #5397). Consequences:
- `cargo test` will continue to build examples as a regular build (no change).
- `cargo test --tests` will no longer build examples at all.
- `cargo test --all-targets` will no longer build examples as tests, but instead build them as a regular build (now matches `cargo test` behavior).
- `cargo check --all-targets` will no longer check examples twice (once as
normal, once as `--test`). It now only checks it once as a normal
target.
## Questions
- Thumbs up/down on the general approach?
- The method to detect if a package is a member of a workspace should probably be redone. I'm uncertain of the best approach. Maybe `Workspace.members` could be a set?
- `Hash` and `PartialEq` are implemented manually for `Profile` only to avoid matching on the `name` field. The `name` field is only there for debug purposes. Is it worth it to keep `name`? Maybe useful for future use (like #4140)?
- I'm unhappy with the `Finished` line summary that displays `[unoptimized + debuginfo]`. It doesn't actually show what was compiled. Currently it just picks the base "dev" or "release" profile. I'm not sure what a good solution is (to be accurate it would need to potentially display a list of different options). Is it ok? (See also #4140 for the wrong profile name being printed.)
- Build-dependencies use different profiles based on whether or not `--release` flag is given. This means that if you want build-dependencies to always use a specific set of settings, you have to specify both `[profile.dev.build_override]` and `[profile.release.build_override]`. Is that reasonable (for now)? I've noticed some issues (like #1774, #2234, #2424) discussing having more control over how build-dependencies are handled.
- `build --bench xxx` or `--benches` builds dependencies with dev profile, which may be surprising. `--release` does the correct thing. Perhaps print a warning when using `cargo build` that builds benchmark deps in dev mode?
- Should it warn/error if you have an override for a package that does not exist?
- Should it warn/error if you attempt to set `panic` on the `test` or `bench` profile?
## TODO
- I have a long list of tests to add.
- Address a few "TODO" comments left behind.
Auto merge of #5415 - alexcrichton:rename-same-dep, r=matklad
Fix renaming crates as they come from 2 sources
Previously there was a verification in manifest parsing that the same dependency
must come from the same source, but this erroneously triggered an error to get
emitted when the `package` key was used to rename crates. The first change here
was to update that clause to key off the `rename` field rather than the `name`
field.
Afterwards, though, this exposed an existing bug in the implementation. During
compilation we have a `Resolve` which is a graph of crates, but we don't know
*why* each edge in the dependency graph exists. In other words we don't know,
when looking at an edge of the graph, what `Dependency` caused that edge to be
drawn. We need to know this when passing `--extern` flags because the
`Dependency` is what lists what's being renamed.
This commit then primarily refactors `Resolve::deps` from an iterator of package
ids to an iterator of a tuples. The first element is the package id from before
and the second element is a list of `Dependency` directives which caused the
edge to ber driven.
This refactoring cleaned up a few places in the backend where we had to work
around the lack of this knowledge. Namely this also fixes the extra test added
here.
Alex Crichton [Wed, 25 Apr 2018 17:41:33 +0000 (10:41 -0700)]
Fix renaming crates as they come from 2 sources
Previously there was a verification in manifest parsing that the same dependency
must come from the same source, but this erroneously triggered an error to get
emitted when the `package` key was used to rename crates. The first change here
was to update that clause to key off the `rename` field rather than the `name`
field.
Afterwards, though, this exposed an existing bug in the implementation. During
compilation we have a `Resolve` which is a graph of crates, but we don't know
*why* each edge in the dependency graph exists. In other words we don't know,
when looking at an edge of the graph, what `Dependency` caused that edge to be
drawn. We need to know this when passing `--extern` flags because the
`Dependency` is what lists what's being renamed.
This commit then primarily refactors `Resolve::deps` from an iterator of package
ids to an iterator of a tuples. The first element is the package id from before
and the second element is a list of `Dependency` directives which caused the
edge to ber driven.
This refactoring cleaned up a few places in the backend where we had to work
around the lack of this knowledge. Namely this also fixes the extra test added
here.
Auto merge of #5394 - henriklaxhuber:master, r=alexcrichton
Pass linker path to build script
This change adds the environment variable LINKER to pass the path of the linker used by cargo to the build script. This complements the variable RUSTC (the rustc binary used) to give the build script full knowledge of its environment.
A specific usage example would be automatically generating bindings to system headers in cross compilation, e.g. by locating jni.h for android targets.
Adds a target directory parameter, that acts in the same manner as the environment variable `CARGO_TARGET_DIR`, to the following subcommands:
- `bench`
- `build`
- `check`
- `clean`
- `doc`
- `package`
- `publish`
- `run`
- `rustc`
- `rustdoc`
- `test`
Auto merge of #5398 - dwijnand:drop-legacy-paths, r=matklad
Drop legacy path support under Rust edition 2018 (or later)
builds on #5335
submitted for early feedback: wdyt @matklad? is this what you had in mind? what should change? what should be added? how should we test this? is the current (2015) messaging enough to drop it in 2018?
Henrik Laxhuber [Mon, 23 Apr 2018 15:11:10 +0000 (17:11 +0200)]
Added tests for RUSTC_LINKER buils script env var
Added tests for the environment variable RUSTC_LINKER that is
passed to the build script.
Also slightly improved readability of the code responsible for
passing the env var.
Henrik Laxhuber [Fri, 20 Apr 2018 15:04:14 +0000 (17:04 +0200)]
Pass linker path to build script
This change adds the environment variable LINKER to pass the path
of the linker used by cargo to the build script. This ammends the
variable RUSTC (the rustc binary used) to give the build script
full knowledge of its environment.
A specific usage example would be automatically generating bindings
to system headers in cross compilation, e.g. by locating jni.h for
android targets.
Auto merge of #5391 - varkor:check-message, r=alexcrichton
Print "Checking" for cargo check
I often alternate between `check` and `build` and I'd like to be able to tell which is currently happening (short-term memory being what it is, and "compiling" implying to me that codegen is occurring). Changing the message to "Checking" seemed to be reasonable (and there was precedent for `doc` with "Documenting").
Auto merge of #5390 - matklad:new-feature-behavior, r=alexcrichton
Enable new behavior of `--feature`
So far, the feedback on https://internals.rust-lang.org/t/help-us-test-the-breaking-bug-fix-to-cargo-features/7317 has been positive, so here's a PR to try this in nightly.
Note that the logic is slightly tweak for the case `cargo build -p not-a-workspace-member`: we want not only to resolve all ws members in this case, but to enable all of their features as well!
As a sanity check, this seems to be forward compatible with further improvements to features:
1) when we solve the grand bug of features being unified across the whole workspace, `cargo build -p not-a-member` would hopefully just work without additional contortions.
2) we might add a way to specify features per package, like `cargo build -p foo -p bar --features "foo/serde bar/serde"`
Auto merge of #5392 - ehuss:test-does-not-contain, r=alexcrichton
cargotest: Fix `with_*_does_not_contain` to support `[..]` and macro matching.
I changed it so that it is essentially the opposite of `with_*_contains` to keep it symmetric.
Any in-flight PRs using the old style will need to be updated (else they will incorrectly silently pass). Alternatively, we could rename the method to avoid that.
The following tests contained brackets, so they were not checking what they thought they were checking. I did a cursory look at them, but perhaps someone else could double-check that they make sense. Asserting what *doesn't* happen can be tricky since there is an infinite number of things that won't happen. Preferably a test would assert that it appears in one scenario and not another (like `incremental_profile` does), but some of them don't or can't.
BTW, would you be interested in a PR that adds some documentation to `cargotest`?
I've discovered things I didn't know where there. I think some docstrings on some of the methods, and a short guide for new contributors would be helpful.
Note that, due to the first commit, this still gives us all the benefits of #5249: if RUSTFLAGS is empty, we will run only a single rustc process, even if we can't cache it across different cargo invocations.
projects / cargo.git / log