bors [Wed, 7 Aug 2019 20:41:07 +0000 (20:41 +0000)]
Auto merge of #7214 - alexcrichton:json-diagnostics, r=ehuss
Add support for customizing JSON diagnostics from Cargo
Cargo has of #7143 enabled pipelined compilation by default which
affects how the compiler is invoked, especially with respect to JSON
messages. This, in some testing, has proven to cause quite a few issues
with rustbuild's current integration with Cargo. This commit is aimed at
adding features to Cargo to solve this issue.
This commit adds the ability to customize the stream of JSON messages
coming from Cargo. The new feature for Cargo is that it now also mirrors
rustc in how you can configure the JSON stream. Multiple
`--message-format` arguments are now supported and the value specified
is a comma-separated list of directives. In addition to the existing
`human`, `short`, and `json` directives these new directives have been
added:
* `json-render-diagnostics` - instructs Cargo to render rustc
diagnostics and only print out JSON messages for artifacts and Cargo
things.
* `json-diagnostic-short` - indicates that the `rendered` field of rustc
diagnostics should use the "short" rendering.
* `json-diagnostic-rendered-ansi` - indicates that the `rendered` field of rustc
diagnostics should embed ansi color codes.
The first option here, `json-render-diagnostics`, will be used by
rustbuild unconditionally. Additionally `json-diagnostic-short` will be
conditionally used based on the input to rustbuild itself.
This should be enough for external tools to customize how Cargo is
invoked and how all kinds of JSON diagnostics get printed, and it's
thought that we can relatively easily tweak this as necessary to extend
it and such.
bors [Tue, 6 Aug 2019 16:22:52 +0000 (16:22 +0000)]
Auto merge of #7219 - ehuss:remap-path-prefix-fix, r=alexcrichton
Fix remap-path-prefix from failing.
rustc currently remaps the source paths in the dep-info file, causing them to potentially point to a non-existent path. This causes the call to `canonicalize` added in #7137 to fail. Just ignore the error.
bors [Tue, 6 Aug 2019 14:23:42 +0000 (14:23 +0000)]
Auto merge of #7215 - ehuss:build-script-cleanup, r=alexcrichton
Clean up build script stuff and documentation.
I often get lost trying to understand some of this build script stuff, so this is an attempt to make it a little easier to follow. Overview:
- Remove `Context::build_script_overridden`.
- Add `BuildContext::script_override` to query if something is overridden.
- Do not bother to compute unit dependencies for overridden build scripts. This is the most risky change, since there are some subtle assumptions when connecting up the various build script maps. However, as best as I can reason through, it seems correct. The intent here is to avoid all the situations where it attempts to filter out these extra deps.
- Rename `Context::build_state` to `Context::build_script_outputs` to try to be clearer about what it is. Similarly rename `BuildState` to `BuildScriptOutputs`.
- Remove trivial 1-line function `load_build_deps`.
- Add some documentation.
Alex Crichton [Mon, 5 Aug 2019 16:42:28 +0000 (09:42 -0700)]
Add support for customizing JSON diagnostics from Cargo
Cargo has of #7143 enabled pipelined compilation by default which
affects how the compiler is invoked, especially with respect to JSON
messages. This, in some testing, has proven to cause quite a few issues
with rustbuild's current integration with Cargo. This commit is aimed at
adding features to Cargo to solve this issue.
This commit adds the ability to customize the stream of JSON messages
coming from Cargo. The new feature for Cargo is that it now also mirrors
rustc in how you can configure the JSON stream. Multiple
`--message-format` arguments are now supported and the value specified
is a comma-separated list of directives. In addition to the existing
`human`, `short`, and `json` directives these new directives have been
added:
* `json-render-diagnostics` - instructs Cargo to render rustc
diagnostics and only print out JSON messages for artifacts and Cargo
things.
* `json-diagnostic-short` - indicates that the `rendered` field of rustc
diagnostics should use the "short" rendering.
* `json-diagnostic-rendered-ansi` - indicates that the `rendered` field of rustc
diagnostics should embed ansi color codes.
The first option here, `json-render-diagnostics`, will be used by
rustbuild unconditionally. Additionally `json-diagnostic-short` will be
conditionally used based on the input to rustbuild itself.
This should be enough for external tools to customize how Cargo is
invoked and how all kinds of JSON diagnostics get printed, and it's
thought that we can relatively easily tweak this as necessary to extend
it and such.
bors [Mon, 5 Aug 2019 15:07:46 +0000 (15:07 +0000)]
Auto merge of #7208 - ehuss:fix-old-build-script-test, r=alexcrichton
Fix an old test.
This test was added in f888b4b7802d4c4699490af3810ce7dadf387915, part of #792, in a commented state. Might as well make it work. It is a bit surprising that passing `-l nonexistinglib` doesn't cause an error, but seems to be fine.
Discovered in #7200 this is just a straight up recipe for deadlock. One Cargo has a jobserver token and wants a file lock. Another Cargo has the file lock and wants a jobserver token. If we had a way to acquire a jobserver token in a nonblocking fashion we could perhaps solve this, but for now I think it's best to revert to the previous behavior.
bors [Thu, 1 Aug 2019 15:23:48 +0000 (15:23 +0000)]
Auto merge of #7191 - debris:prerelease_error_message, r=Eh2406
improve error message for unmatched prerelease dependencies
fixes #7007
error message before:
```
error: no matching package named `a` found
location searched: [..]
perhaps you meant: a
required by package `b v0.1.0 ([..])`
```
error message now
```
error: no matching package named `a` found
location searched: [..]
prerelease package needs to be specified explicitly
a = { version = "0.1.1-alpha.0" }
required by package `b v0.1.0 ([..])`
```
Auto merge of #7192 - alexcrichton:fix-backups, r=ehuss
Fix excluding target dirs from backups on OSX
This fixes an accidental regression from #6880 identified in #7189 by
moving where the configuration of backup preferences happens since it
was accidentally never happening due to the folder always having been
created.
Auto merge of #6817 - thomwiggers:fix-2748, r=ehuss
Handle symlinks to directories
When cargo encounters a link, check if it's a directory when considering it for further processing.
I'm not sure how this interacts with things such as submodules – it allowed me to publish the crate [pqcrypto-kyber768](https://github.com/rustpq/pqcrypto) which uses a link to a submodule.
Fixes #2748.
This PR:
* Add new tests that demonstrate that links to dirs can't be packaged
* Fixes those tests
* Enabled symlink tests to run on Windows
* Fix a bug in the handling of symlinks
* Add new test runner behaviour on Windows (it will ignore symlink-related tests and instead run them if you specify --ignored.)
Auto merge of #7143 - alexcrichton:stabilize-pipelined-compilation, r=ehuss
Enable pipelined compilation by default
This commit enables pipelined compilation by default in Cargo now that
the requisite support has been stablized in rust-lang/rust#62766. This
involved minor updates in a number of locations here and there, but
nothing of meat has changed from the original implementation (just
tweaks to how rustc is called).
Alex Crichton [Wed, 17 Jul 2019 19:41:27 +0000 (12:41 -0700)]
Enable pipelined compilation by default
This commit enables pipelined compilation by default in Cargo now that
the requisite support has been stablized in rust-lang/rust#62766. This
involved minor updates in a number of locations here and there, but
nothing of meat has changed from the original implementation (just
tweaks to how rustc is called).
Alex Crichton [Tue, 30 Jul 2019 15:55:57 +0000 (08:55 -0700)]
Fix excluding target dirs from backups on OSX
This fixes an accidental regression from #6880 identified in #7189 by
moving where the configuration of backup preferences happens since it
was accidentally never happening due to the folder always having been
created.
Auto merge of #7186 - ehuss:resolve-method, r=alexcrichton
Refactor resolve `Method`
This changes the enum `Method` to a struct called `ResolveOpts`. The intent is to try to make it a little easier to read and understand.
If this doesn't actually look better to anyone, I'm fine with discarding this (I can resubmit just with documentation). It only seems marginally better, but I have had difficulty with understanding the subtleties of `Method` for a while. I wasn't able to measure any performance difference.
This also has a few other changes:
- Adds some documentation.
- Removes `resolve_ws_precisely`, cutting down the number of resolve functions from 4 to 3.
Auto merge of #7185 - ehuss:targetinfo-error, r=alexcrichton
Clean up TargetInfo
- The main motivation here is to provide a better error message when collecting information from rustc fails (it now shows the command and the output).
- Remove `has_cfg_and_sysroot`. I think this dates back to when it was introduced in #2328, as a guard for older versions of rustc that did not know about `--print=cfg`. Unless I'm missing something, I don't think we need to retain this backwards compatibility.
- Add some documentation.
- Demote the rustc cache log messages to `debug` level. I haven't really seen any caching issues, so I don't think it needs to be info level.
- Some other misc cleanup (remove unused function, etc.).
Auto merge of #7137 - ehuss:dep-info-absolute-paths, r=alexcrichton
Fix some issues with absolute paths in dep-info files.
There were some issues with how #7030 was handling translating paths in dep-info files. The main consequence is that when coupled with https://github.com/rust-lang/rust/pull/61727 (which has not yet merged), the fingerprint would fail and be considered dirty when it should be fresh.
It was joining [`target_root`](https://github.com/rust-lang/cargo/blame/1140c527c4c3b3e2533b9771d67f88509ef7fc16/src/cargo/core/compiler/fingerprint.rs#L1352-L1360) which had 3 different values, but stripping [only one](https://github.com/rust-lang/cargo/blame/1140c527c4c3b3e2533b9771d67f88509ef7fc16/src/cargo/core/compiler/mod.rs#L323). This means for different scenarios (like using `--target`), it was creating incorrect paths in some cases. For example a target having a proc-macro dependency which would be in the host directory.
The solution here is to always use CARGO_TARGET_DIR as the base that all relative paths are checked against. This should handle all host/target differences.
The tests are marked with `#[ignore]` because 61727 has not yet merged.
This includes a second commit (which I can open as a separate PR if needed) which is an alternate solution to #7034. It adds dep-info tracking for registry dependencies. However, it tries to limit which kinds of paths it tracks. It will not track package-relative paths (like source files). It also adds an mtime cache to significantly reduce the number of stat calls (because every dependency was stating every file in sysroot).
Finally, I've run some tests using this PR with 61727 in rust-lang/rust. I can confirm that a second build is fresh (it doesn't erroneously rebuild). I can also confirm that the problem in https://github.com/rust-lang/rust/issues/59105 has *finally* been fixed!
My confidence in all this is pretty low, but I've been unable to think of any specific ways to make it fail. If anyone has ideas on how to better test this, let me know. Also, feel free to ask questions since I've been thinking about this a lot for the past few weeks, and there is quite a bit of subtle stuff going on.
Eric Huss [Thu, 25 Jul 2019 02:35:27 +0000 (19:35 -0700)]
Use canonical paths when parsing dep-info.
Instead of treating Windows differently, this just always uses canonical paths
on all platforms. This fixes a problem where symlinks were not treated
correctly on all platforms.
Switching rm_rf to the remove_dir_all crate because deleting symbolic links on
Windows is difficult.
Eric Huss [Mon, 15 Jul 2019 03:20:27 +0000 (20:20 -0700)]
Track dep-info for all dependencies.
This adds dep-info tracking for non-path dependencies. This avoids tracking
package-relative paths (like source files) in non-path dependencies, since we
assume they are static. It also adds an mtime cache to improve performance since
when rustc starts tracking sysroot files (in the future), it can cause a large
number of stat calls.
Auto merge of #7179 - alexcrichton:fix-tests, r=ehuss
Fix a deadlocking test with master libgit2
This commit fixes a test in Cargo to work around a seeming regression in
behavior in libgit2 around HTTP authentication. The expected flow for
HTTP authentication with git is that git sends an HTTP request and
receives an "unauthorized" response. It then sends another request with
authorization information and that's what we're testing is received in
the our test.
Previously libgit2 would issue a new HTTP connection if the previous one
was closed, but it looks like changes in libgit2 now require that the
same HTTP connection is used for the initial request and the subsequent
request with authorization information. This broke our test since it's
not using an HTTP compliant server at all and is just some handwritten
TCP reads/writes. The fix here is to basically stay with handwritten TCP
reads/writes but tweak how it happens so it's all on the same HTTP/TCP
connection to match what libgit2 is expecting.
Some extra assertions have also been added to try to prevent deadlocks
from happening in the future and instead make the test fail fast if this
situation comes up again.
Alex Crichton [Thu, 25 Jul 2019 16:07:25 +0000 (09:07 -0700)]
Fix a deadlocking test with master libgit2
This commit fixes a test in Cargo to work around a seeming regression in
behavior in libgit2 around HTTP authentication. The expected flow for
HTTP authentication with git is that git sends an HTTP request and
receives an "unauthorized" response. It then sends another request with
authorization information and that's what we're testing is received in
the our test.
Previously libgit2 would issue a new HTTP connection if the previous one
was closed, but it looks like changes in libgit2 now require that the
same HTTP connection is used for the initial request and the subsequent
request with authorization information. This broke our test since it's
not using an HTTP compliant server at all and is just some handwritten
TCP reads/writes. The fix here is to basically stay with handwritten TCP
reads/writes but tweak how it happens so it's all on the same HTTP/TCP
connection to match what libgit2 is expecting.
Some extra assertions have also been added to try to prevent deadlocks
from happening in the future and instead make the test fail fast if this
situation comes up again.
Auto merge of #7174 - alexcrichton:fix-cycle-check, r=Eh2406
Fix detection of cyclic dependencies through `[patch]`
This commit fixes detection of cyclic dependencies through the use of
`[patch]` by ensuring that `matches_id` isn't used because it returns a
false negative for registry dependencies when the dependency
specifications don't match but the resolve edges are still correct.
Alex Crichton [Wed, 24 Jul 2019 14:31:42 +0000 (07:31 -0700)]
Fix detection of cyclic dependencies through `[patch]`
This commit fixes detection of cyclic dependencies through the use of
`[patch]` by ensuring that `matches_id` isn't used because it returns a
false negative for registry dependencies when the dependency
specifications don't match but the resolve edges are still correct.
Auto merge of #7170 - ehuss:no-more-glob, r=alexcrichton
Remove include/exclude glob warning.
This removes the warning when a package include/exclude pattern produces different results from glob vs gitignore.
The pre-warnings were added in 1.21 (released 2017-10-12) #4270.
The migration was done in 1.36 (released 2019-07-04) #6924.
This will remove the warnings in 1.38 (to be released 2019-09-26).
Auto merge of #7070 - alexcrichton:smaller-cargo-lock, r=ehuss
Optimize lock file format for git merge conflicts
This commit is an attempt to refine Cargo's lock file format to generate
less git merge conflicts for lock file updates as well as make it easier
to manage lock file updates. The new lock file format has a few major changes:
* The `[metadata]` table is no longer used to track checksums. The
packages themselves now list `checksum` fields directly.
* The entries in the `dependencies` key no longer unconditionally
mention the version/source of the dependency. When unambiguous only
the name or only the name/version are mentioned.
As mentioned before the goal here is to reduce git merge conflict
likelihood between two cargo updates to a lock file. By not using
`[metadata]` the updates to package checksums should only happen on the
package itself, not in a shared metadata table where it's easy to
conflict with other updates. Additionally by making `dependencies`
entries shorter it's hoped that updating a crate will only either add
entries to a lock file or update lines related to just that package.
It's theorized that the amount of updates necessary to a lock file are
far less than today where the version has to be updated in many
locations.
As with all lock file format changes this new format is not turned on by
default. Support is added so Cargo will preserve it if it sees it (and
tests are added too), and then some time down the road we can flip the
switch and turn this on by default.
Auto merge of #7157 - ehuss:force-clippy, r=alexcrichton
Force clippy to run.
This causes `cargo clippy-preview` to always run, instead of possibly emitting no output if it is run a second time.
This is just a personal preference of mine, but I think would be better behavior which we have talked about before. I don't think the arguments that it should be "fast" like `cargo check` apply here. Once [cache-messages](https://github.com/rust-lang/cargo/issues/6986) is stabilized, this can be removed.
Alex Crichton [Wed, 19 Jun 2019 20:50:17 +0000 (13:50 -0700)]
Optimize lock file format for git merge conflicts
This commit is an attempt to refine Cargo's lock file format to generate
less git merge conflicts for lock file updates as well as make it easier
to manage lock file updates. The new lock file format has a few major changes:
* The `[metadata]` table is no longer used to track checksums. The
packages themselves now list `checksum` fields directly.
* The entries in the `dependencies` key no longer unconditionally
mention the version/source of the dependency. When unambiguous only
the name or only the name/version are mentioned.
As mentioned before the goal here is to reduce git merge conflict
likelihood between two cargo updates to a lock file. By not using
`[metadata]` the updates to package checksums should only happen on the
package itself, not in a shared metadata table where it's easy to
conflict with other updates. Additionally by making `dependencies`
entries shorter it's hoped that updating a crate will only either add
entries to a lock file or update lines related to just that package.
It's theorized that the amount of updates necessary to a lock file are
far less than today where the version has to be updated in many
locations.
As with all lock file format changes this new format is not turned on by
default. Support is added so Cargo will preserve it if it sees it (and
tests are added too), and then some time down the road we can flip the
switch and turn this on by default.
Auto merge of #7146 - alexcrichton:faster-proc-macro, r=Eh2406
Optimize runtime of `#[cargo_test_macro]`
I've noticed recently that the incremental compile time for our test
suite has felt like it's increased quite a bit. I think one reason is
that everything has to go through `#[cargo_test_macro]` unconditionally
on all incremental builds, and wow do we have a lot of tests being
pumped through that macro.
Instrumenting the macro a little bit shows that we spend nearly 2.5
seconds on each compilation simply executing this macro (note that it's
in debug mode as well, not release since we typically don't execute
tests in release mode.
This commit instead drops the usage of `syn` and `quote` in favor of a
"raw procedural macro" which is much more optimized for just our use
case, even in debug mode. This drops the collective time spent in the
macro to 0.2 seconds, even in debug mode!
Alex Crichton [Thu, 18 Jul 2019 14:45:44 +0000 (07:45 -0700)]
Optimize runtime of `#[cargo_test_macro]`
I've noticed recently that the incremental compile time for our test
suite has felt like it's increased quite a bit. I think one reason is
that everything has to go through `#[cargo_test_macro]` unconditionally
on all incremental builds, and wow do we have a lot of tests being
pumped through that macro.
Instrumenting the macro a little bit shows that we spend nearly 2.5
seconds on each compilation simply executing this macro (note that it's
in debug mode as well, not release since we typically don't execute
tests in release mode.
This commit instead drops the usage of `syn` and `quote` in favor of a
"raw procedural macro" which is much more optimized for just our use
case, even in debug mode. This drops the collective time spent in the
macro to 0.2 seconds, even in debug mode!
Auto merge of #7149 - alexcrichton:maybe-no-lock, r=ehuss
Don't fail if we can't acquire readonly lock
This commit updates support from #6940 to not only gracefully handle
situations where the lock can be acquired in readonly but not read/write
mode but also handle situations where even a readonly lock can't be
acquired. If a readonly lock can't be acquired (and the read/write
failed) then we likely can't touch anything in the directory, so there's
no value gained from locking anyway.
Alex Crichton [Fri, 19 Jul 2019 14:56:33 +0000 (07:56 -0700)]
Don't fail if we can't acquire readonly lock
This commit updates support from #6940 to not only gracefully handle
situations where the lock can be acquired in readonly but not read/write
mode but also handle situations where even a readonly lock can't be
acquired. If a readonly lock can't be acquired (and the read/write
failed) then we likely can't touch anything in the directory, so there's
no value gained from locking anyway.
Auto merge of #7142 - alexcrichton:disable-nightly, r=Eh2406
Add a way to disable all nightly tests
One thing I'm realizing now is that we test a number of nightly features
of Cargo/rustc, but if they change in rustc then because
rust-lang/rust's CI run's Cargo's tests it could prevent those changes
from landing! The purpose of running Cargo's tests in CI in
rust-lang/rust is to ensure that Cargo generally works, but there's no
need to test the nightly features as that's Cargo's job.
As a result this adds an environment variable that will be set from
rust-lang/rust's CI which will disable these tests.
Alex Crichton [Wed, 17 Jul 2019 20:58:36 +0000 (13:58 -0700)]
Add a way to disable all nightly tests
One thing I'm realizing now is that we test a number of nightly features
of Cargo/rustc, but if they change in rustc then because
rust-lang/rust's CI run's Cargo's tests it could prevent those changes
from landing! The purpose of running Cargo's tests in CI in
rust-lang/rust is to ensure that Cargo generally works, but there's no
need to test the nightly features as that's Cargo's job.
As a result this adds an environment variable that will be set from
rust-lang/rust's CI which will disable these tests.
Auto merge of #7131 - ehuss:remove-unused-opt-feature, r=alexcrichton
Remove unused feature filter.
NOTE: Do not merge this lightly. This upended my understanding of how the resolver handles features, and I'm still not sure about it.
Remove an unused check that an optional dependency is activated.
This was originally added 4 years ago in #1812, during a time when this code iterated over the actual dependencies from `Package.dependencies()`. In #5415 this was refactored so that it gets the `deps` list directly from the Resolver, which AFAIK has already filtered out the features. IIUC, this filtering is done [here](https://github.com/rust-lang/cargo/blame/705009eb3828123cc9dbcf5b28988cc63f60b03b/src/cargo/core/resolver/dep_cache.rs#L270-L272).
Auto merge of #7118 - alexcrichton:patch-bug, r=Eh2406
Handle activation conflicts for `[patch]` sources
This commit updates the resolver to ensure that it recognizes conflicts
when `[patch]` is used to augment an older version of what's already in
a source, for example. Previously the deduplication based on
semver-compatible versions didn't actually work when `[patch]` was used.
This meant that when you used `[patch]` it might not transitively affect
the entire crate graph, instead just giving you a version of a
dependency and everyone else. This violates the intention of `[patch]`!
The fix here is to catch this use case happening, when a `Dependency`
source specification mismatches an activated package we need to list a
second activation in the resolver to prevent major versions from being
selected from both the original source as well as the source of the id.