Auto merge of #7311 - ehuss:pipeline-timing, r=alexcrichton
Experiment: Create timing report.
This is just an experiment, so I'm not sure if we'll want to merge it.
This adds an HTML report which gets saved to disk when the build is finished. It is primarily geared for identifying slow dependencies, and for visualizing how pipelining affects the build.
Here's an example: https://ehuss.github.io/cargo-timing.html
You can mouse over the blocks to highlight the reverse-dependencies that are released when a unit finishes. `syn` is a really good example.
It does a few other things, like displaying a message after each unit is finished. See the docs for more information.
Auto merge of #7368 - alexcrichton:canonical-urls-omg, r=ehuss
Work with canonical URLs in `[patch]`
This commit addresses an issue with how the resolver processes `[patch]`
annotations in manifests and lock files. Previously the resolver would
use the raw `Url` coming out of a manifest, but the rest of resolution,
when comparing `SourceId`, uses a canonical form of a `Url` rather than
the actual raw `Url`. This ended up causing discrepancies like those
found in #7282.
To fix the issue all `patch` intermediate storage in the resolver uses a
newly-added `CanonicalUrl` type instead of a `Url`. This
`CanonicalUrl` is then also used throughout the codebase, and all
lookups in the resolver as switched to using `CanonicalUrl` instead of
`Url`, which...
Alex Crichton [Mon, 16 Sep 2019 19:35:03 +0000 (12:35 -0700)]
Work with canonical URLs in `[patch]`
This commit addresses an issue with how the resolver processes `[patch]`
annotations in manifests and lock files. Previously the resolver would
use the raw `Url` coming out of a manifest, but the rest of resolution,
when comparing `SourceId`, uses a canonical form of a `Url` rather than
the actual raw `Url`. This ended up causing discrepancies like those
found in #7282.
To fix the issue all `patch` intermediate storage in the resolver uses a
newly-added `CanonicalUrl` type instead of a `Url`. This
`CanonicalUrl` is then also used throughout the codebase, and all
lookups in the resolver as switched to using `CanonicalUrl` instead of
`Url`, which...
Auto merge of #7373 - alexcrichton:clear-memos, r=ehuss
Clear out memoized hashes before building crates
Build script updates during execution can change the memoized hash of a
`Fingerprint`, and while previously we cleared out a single build
script's memoized hash we forgot to clear out everything that depended
on it as well. This commit pessimistically clears out all `Fingerprint`
memoized hashes just before building to ensure that during the build
everything has the most up-to-date view of the world, and when build
scripts change fingerprints everything that depends on them won't have
run yet.
Alex Crichton [Tue, 17 Sep 2019 14:04:28 +0000 (07:04 -0700)]
Clear out memoized hashes before building crates
Build script updates during execution can change the memoized hash of a
`Fingerprint`, and while previously we cleared out a single build
script's memoized hash we forgot to clear out everything that depended
on it as well. This commit pessimistically clears out all `Fingerprint`
memoized hashes just before building to ensure that during the build
everything has the most up-to-date view of the world, and when build
scripts change fingerprints everything that depends on them won't have
run yet.
Auto merge of #7350 - alexcrichton:mock-std, r=ehuss
Improve test suite for `-Zbuild-std`
This commit is aimed directly at rust-lang/wg-cargo-std-aware#33 and in
general making the `-Zbuild-std` tests more robust. The main change here
is that a new source tree is checked in, `tests/testsuite/mock-std`,
which mirrors rust-lang/rust's own tree for libstd. This mock tree is as
empty as it can be, ideally duplicating almost nothing but for not
requiring duplication of Cargo metadata about patches and such.
The end result here looks like:
* All `-Zbuild-std` tests are now run in parallel
* All tests run much more quickly since they're compiling tiny crates
instead of actually compiling libstd/libcore
* No tests require network access
* We verify that crates have access to the "custom" libraries
that we build
Coverage of tests is not currently expanded, but it's hoped that we
could add that shortly afterwards. Coverage has actually gone down
slightly since the custom target test was commented out temporarily and
the full integration test of running `-Zbuild-std` isn't run on CI any
more.
Alex Crichton [Thu, 12 Sep 2019 17:37:14 +0000 (10:37 -0700)]
Add back a full integration test for `-Zbuild-std`
Only run these tests on one CI builder (not all platforms) though. This
is extremely resource intensive since it rebuilds libstd. Currently this
does not share a build directly like before because the number of tests
are supposed to be small, but if necessary we can add that in later too.
Alex Crichton [Tue, 10 Sep 2019 18:01:31 +0000 (11:01 -0700)]
Improve test suite for `-Zbuild-std`
This commit is aimed directly at rust-lang/wg-cargo-std-aware#33 and in
general making the `-Zbuild-std` tests more robust. The main change here
is that a new source tree is checked in, `tests/testsuite/mock-std`,
which mirrors rust-lang/rust's own tree for libstd. This mock tree is as
empty as it can be, ideally duplicating almost nothing but for not
requiring duplication of Cargo metadata about patches and such.
The end result here looks like:
* All `-Zbuild-std` tests are now run in parallel
* All tests run much more quickly since they're compiling tiny crates
instead of actually compiling libstd/libcore
* No tests require network access
* We verify that crates have access to the "custom" libraries
that we build
Coverage of tests is not currently expanded, but it's hoped that we
could add that shortly afterwards. Coverage has actually gone down
slightly since the custom target test was commented out temporarily and
the full integration test of running `-Zbuild-std` isn't run on CI any
more.
Auto merge of #7159 - Aaron1011:feature/rustdoc-proc-macro-final, r=alexcrichton
Pass --crate-type to rustdoc
This supports the [corresponding rustc PR](https://github.com/rust-lang/rust/pull/62855). To enable rustdoc to properly
document macros, we pass a new flag '--proc-macro-crate' when
documenting a proc-macro crate. This causes rustdoc to enable the
proc-macro compiler logic that runs when rustc is building a proc-macro
crate.
This flag is essentially a more restricted version of
'--crate-type=proc-macro'. I didn't think it was necessary to pass the
full '--crate-type' flag to rustdoc, when only two options would ever be
used (proc-macro vs anything else).
Auto merge of #7360 - phil-opp:zbuild-std-custom-test-frameworks, r=alexcrichton
[-Zbuild-std] Only build libtest when libstd is built
Currently `libtest` is always compiled when a compilation unit uses a test harness. This implicitly adds builds the standard library too because `libtest` depends on it. This breaks the use of custom test frameworks in `no_std` crates as reported in https://github.com/rust-lang/cargo/pull/7216#issuecomment-529433594.
This pull request fixes the issue by only building `libtest` if `libstd` is built. This makes sense in my opinion because when the user explicitly specified `-Zbuild-std=core`, they probably don't want to build the full standard library and rather get a compilation error when they accidentally use `libtest`.
Dan Aloni [Thu, 12 Sep 2019 13:27:15 +0000 (16:27 +0300)]
testsuite: introduce profile_custom
This suite of tests verifies various cases around the 'inherits'
keyword, and also verifies the relationship between profile overrides
and custom profiles.
Dan Aloni [Thu, 12 Sep 2019 05:22:19 +0000 (08:22 +0300)]
Reinstate warning regarding 'debug' profile
The previous warning was detected at the decoding level, with the test
removed in an earlier commit. Here it is brought back, in the custom
profile processing level.
Keeping this warning will serve to prevent confusion, when people expect
to affect the 'debug' directory via the 'debug' profile to no effect,
where in fact the 'dev' profile is the profile that they opted to
change.
Auto merge of #7353 - alexcrichton:libstd-no-dylib, r=ehuss
Don't build libstd as a `dylib`
This commit forcibly prevents Cargo from building the `std` crate as a
`dylib`, even though libstd upstream lists a `dylib` crate type. We
ideally want a first-class feature for doing this one day, but for now
we can just hack around with the manifests to ensure that the `dylib`
crate type never shows up. Note that this is only supported for libstd,
and it's also all part of the unstable details of building std.
Aaron Hill [Sun, 21 Jul 2019 02:59:55 +0000 (22:59 -0400)]
Pass --crate-type to rustdoc
This supports the corresponding rustc PR. To enable rustdoc to properly
document macros, we mirror the '--crate-type' flag used by rustc.
Currently, all crate types other than 'proc-macro' are ignored by
rustdoc.
Auto merge of #7344 - alexcrichton:jobserver, r=ehuss
Create a jobserver with N tokens, not N-1
I recently added `jobserver` support to the `cc` crate and ended up
running afoul of a `jobserver` quirk on Windows. Due to how it's
implemented, on Windows you can't actually add more than the intial
number of tokens to the jobserver (it uses an IPC semaphore). On Unix,
however, you can since you're just writing bytes into a pipe.
In `cc`, however, I found it convenient to control parallelism by simply
releasing a token before the parallel loop, then reacquiring the token
after the loop. That way the loop just has to acquire a token for each
job it wants to spawn and then release it when the job finishes. This is
a bit simpler than trying to juggle the "implicit token" all over the
place as well as coordinating its use. It's technically invalid because
it allows a brief moment of `N+1` parallelism since we release a token
and then do a bit of work to acquire a new token, but that's hopefully
not really the end of the world.
In any case this commit updates Cargo's creation of a jobserver to create
it with `N` tokens instead of `N-1`. The same semantics are preserved
where Cargo then immediately acquires one of the tokens, but the
difference is that this "implicit token" can be released back to the
jobserver pool, unlike before.
Alex Crichton [Wed, 11 Sep 2019 17:13:59 +0000 (10:13 -0700)]
Don't build libstd as a `dylib`
This commit forcibly prevents Cargo from building the `std` crate as a
`dylib`, even though libstd upstream lists a `dylib` crate type. We
ideally want a first-class feature for doing this one day, but for now
we can just hack around with the manifests to ensure that the `dylib`
crate type never shows up. Note that this is only supported for libstd,
and it's also all part of the unstable details of building std.
Auto merge of #7340 - varkor:ignore-must_use-results, r=alexcrichton
Explicitly ignore some results
Use `let _ = ` to ignore some values that are `#[must_use]` when checking nested data types. This is necessary to compile cargo without warnings under https://github.com/rust-lang/rust/pull/62262.
Alex Crichton [Mon, 9 Sep 2019 15:46:43 +0000 (08:46 -0700)]
Create a jobserver with N tokens, not N-1
I recently added `jobserver` support to the `cc` crate and ended up
running afoul of a `jobserver` quirk on Windows. Due to how it's
implemented, on Windows you can't actually add more than the intial
number of tokens to the jobserver (it uses an IPC semaphore). On Unix,
however, you can since you're just writing bytes into a pipe.
In `cc`, however, I found it convenient to control parallelism by simply
releasing a token before the parallel loop, then reacquiring the token
after the loop. That way the loop just has to acquire a token for each
job it wants to spawn and then release it when the job finishes. This is
a bit simpler than trying to juggle the "implicit token" all over the
place as well as coordinating its use. It's technically invalid because
it allows a brief moment of `N+1` parallelism since we release a token
and then do a bit of work to acquire a new token, but that's hopefully
not really the end of the world.
In any case this commit updates Cargo's creation of a jobserver to create
it with `N` tokens instead of `N-1`. The same semantics are preserved
where Cargo then immediately acquires one of the tokens, but the
difference is that this "implicit token" can be released back to the
jobserver pool, unlike before.
Auto merge of #7337 - alexcrichton:less-optional, r=ehuss
Don't resolve std's optional dependencies
Use the `set_require_optional_deps(false)` escape hatch to avoid
resolving optional dependencies for libstd. While it doesn't really
matter a huge amount either way there's no need for us to generate
resolution nodes for things like `rand` just to throw them away because
they're never used.
Alex Crichton [Fri, 6 Sep 2019 22:36:43 +0000 (15:36 -0700)]
Don't resolve std's optional dependencies
Use the `set_require_optional_deps(false)` escape hatch to avoid
resolving optional dependencies for libstd. While it doesn't really
matter a huge amount either way there's no need for us to generate
resolution nodes for things like `rand` just to throw them away because
they're never used.
Auto merge of #7326 - Eh2406:map_dependencies, r=ehuss
`map_dependencies` is doing a deep clone, so lets make it cheaper
This removes a `FeatureMap::clone` that I noticed when profiling no-op builds of cargo, benchmarks show a ~5% improvement. Looks like #6880 means that there is a ref to every `Summery` so the `Rc::make_mut` dose a deep clone.
Auto merge of #7324 - Eh2406:two-copys-of-hash, r=alexcrichton
don't need to copy this string
This removes a `String::clone` that I noticed when profiling no-op builds of cargo, benchmarks show a barely visible improvement. Looks like it was added in #6880, but I am not sure why.
Auto merge of #7216 - ehuss:build-std, r=alexcrichton
Basic standard library support.
This is not intended to be useful to anyone. If people want to try it, that's great, but do not rely on this. This is only for experimenting and setting up for future work.
This adds a flag `-Zbuild-std` to build the standard library with a project. The flag can also take optional comma-separated crate names, like `-Zbuild-std=core`. Default is `std,core,panic_unwind,compiler_builtins`.
Closes rust-lang/wg-cargo-std-aware#10.
Note: I can probably break some of the refactoring into smaller PRs if necessary.
## Overview
The general concept here is to use two resolvers, and to combine everything in the Unit graph. There are a number of changes to support this:
- A synthetic workspace for the standard library is created to set up the patches and members correctly.
- Decouple `unit_dependencies` from `Context` to make it easier to manage.
- Add `features` to `Unit` to keep it unique and to remove the need to query a resolver.
- Add a `UnitDep` struct which encodes the edges between `Unit`s. This removes the need to query a resolver for `extern_crate_name` and `public`.
- Remove `Resolver` from `BuildContext` to avoid any confusion and to keep the complexity focused in `unit_dependencies`.
- Remove `Links` from `Context` since it used the resolver. Adjusted so that instead of checking links at runtime, they are all checked at once in the beginning. Note that it does not check links for the standard lib, but it should be safe? I think `compiler-rt` is the only `links`?
I currently went with a strategy of linking the standard library dependencies using `--extern` (instead of `--sysroot` or `-L`). This has some benefits but some significant drawbacks. See below for some questions.
## For future PRs
- Add Cargo.toml support. See https://github.com/rust-lang/wg-cargo-std-aware/issues/5
- Source is not downloaded. It assumes you have run `rustup component add rust-src`. See https://github.com/rust-lang/wg-cargo-std-aware/issues/11
- `cargo metadata` does not include any information about std. I don't know how this should work.
- `cargo clean` is not std-aware.
- `cargo fetch` does not fetch std dependencies.
- `cargo vendor` does not vendor std dependencies.
- `cargo pkgid` is not std-aware.
- `--target` is required on the command-line. This should default to host-as-target.
- `-p` is not std aware.
- A synthetic `Cargo.toml` workspace is created which has to know about things like `rustc-std-workspace-core`. Perhaps rust-lang/rust should publish the source with this `Cargo.toml` already created?
- `compiler_builtins` uses default features (pure Rust implementation, etc.). See https://github.com/rust-lang/wg-cargo-std-aware/issues/15
- `compiler_builtins` may need to be built without debug assertions, see [this](https://github.com/rust-lang/rust/blob/8e917f48382c6afaf50568263b89d35fba5d98e4/src/bootstrap/bin/rustc.rs#L210-L214). Could maybe use profile overrides.
- Panic issues:
- `panic_abort` is not yet supported, though it should probably be easy. It could maybe look at the profile to determine which panic implementation to use? This requires more hard-coding in Cargo to know about rustc implementation details.
- [This](https://github.com/rust-lang/rust/blob/8e917f48382c6afaf50568263b89d35fba5d98e4/src/bootstrap/bin/rustc.rs#L186-L201) should probably be handled where `panic` is set for `panic_abort` and `compiler_builtins`. I would like to get a test case for it. This can maybe be done with profile overrides?
- Using two resolvers is quite messy and causes a lot of complications. It would be ideal if it could only use one, though that may not be possible for the foreseeable future. See https://github.com/rust-lang/wg-cargo-std-aware/issues/12
- Features are hard-coded. See https://github.com/rust-lang/wg-cargo-std-aware/issues/13
- Lots of various platform-specific support is not included (musl, wasi, windows-gnu, etc.).
- Default `backtrace` is used with C compiler. See https://github.com/rust-lang/wg-cargo-std-aware/issues/16
- Sanitizers are not built. See https://github.com/rust-lang/wg-cargo-std-aware/issues/17
- proc_macro has some hacky code to synthesize its dependencies. See https://github.com/rust-lang/wg-cargo-std-aware/issues/18. This may not be necessary if this uses `--sysroot` instead.
- Profile overrides cause weird linker errors.
That is:
```toml
[profile.dev.overrides.std]
opt-level = 2
```
Using `[profile.dev.overrides."*"]` works. I tried fiddling with it, but couldn't figure it out.
We may also want to consider altering the syntax for profile overrides. Having to repeat the same profile for `std` and `core` and `alloc` and everything else would not be ideal.
- ~~`Context::unit_deps` does not handle build overrides, see #7215.~~ FIXED
## Questions for this PR
- I went with the strategy of using `--extern` to link the standard lib. This seems to work, and I haven't found any problems, but it seems risky. It also forces Cargo to know about certain implicit dependencies like `compiler_builtins` and `panic_*`. The alternative is to create a sysroot and copy all the crates to that directory and pass `--sysroot`. However, this is complicated by pipelining, which would require special support to copy `.rmeta` files when they are generated. Let me know if you think I should use a different strategy. I'm on the fence here, and I think using `--sysroot` may be safer, but adds more complexity.
- As an aside, if rustc ever tries to grab a crate from sysroot that was not passed in via `--extern`, then it results in duplicate lang items. For example, saying `extern crate proc_macro;` without specifying `proc_macro` as a dependency. We could prevent rustc from ever trying by passing `--sysroot=/nonexistent` to prevent it from trying. Or add an equivalent flag to rustc.
- How should this be tested? I added a janky integration test, but it has some drawbacks. It requires internet access. It is slow. Since it is slow, it reuses the same target directory for multiple tests which makes it awkward to work with.
- What interesting things are there to test?
- We may want to disable the test before merging if it seems too annoying to make it the default. It requires rust-src to be downloaded, and takes several minutes to run, and are somewhat platform-dependent.
- How to test that it is actually linking the correct standard library? I did tests locally with a modified libcore, but I can't think of a good way to do that in the test suite.
- I did not add `__CARGO_DEFAULT_LIB_METADATA` to the hash. I had a hard time coming up with a test case where it would matter.
- My only thought is that it is a problem because libstd includes a dylib, which prevents the hash from being added to the filename. It does cause recompiles when switching between compilers, for example, when it normally wouldn't.
- Very dumb question: Why exactly does libstd include a dylib? This can cause issues (see https://github.com/rust-lang/rust/issues/56443).
- This should probably change, but I want to better understand it first.
- The `bin_nostd` test needs to link libc on linux, and I'm not sure I understand why. I'm concerned there is something wrong there. libstd does not do that AFAIK.
projects / cargo.git / log