### What does this PR try to resolve?
This PR relaxes the `bad_crate_type` test to have it only match the
prefix of the unknown crate type error message emitted by rustc.
This is so that the cargo test isn't sensitive to (future) suggestions
for known crate types that rustc may emit to help the user.
### How should we test and review this PR?
This test should already be run as part of cargo CI. (This is definitely
run as part of rust-lang/rust CI, lol.)
### Additional information
rust-lang/rust side PR that's trying to add a suffix to the bad crate
type error message to list all valid `--crate-type` values:
https://github.com/rust-lang/rust/pull/134720.
Without relaxing this test, the rust-lang/rust side PR [fails
with](https://github.com/rust-lang/rust/pull/134720#issuecomment-2561993224):
<details>
<summary>rust-lang/rust CI fail message</summary>
(Ignore the missing colon after `unknown crate type`)
```
---- bad_config::bad_crate_type stdout ----
running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/cargo build -v`
thread 'bad_config::bad_crate_type' panicked at src/tools/cargo/tests/testsuite/bad_config.rs:434:10:
---- expected: tests/testsuite/bad_config.rs:424:27
++++ actual: stderr
1 1 | [ERROR] failed to run `rustc` to learn about crate-type bad_type information
2 2 |
3 3 | Caused by:
4 4 | process didn't exit successfully: `rustc - --crate-name ___ --print=file-names --crate-type bad_type` ([EXIT_STATUS]: 1)
5 5 | --- stderr
6 - [ERROR] unknown crate type: `bad_type`
Error: 6 + [ERROR] unknown crate type `bad_type`, expected one of: `bin`, `cdylib`, `dylib`, `lib`, `proc-macro`, `rlib`
7 7 |
```
</details>
Discussed at
https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/Reblessing.20a.20cargo.20test.
### What does this PR try to resolve?
Do nothing but move code around
* `cargo_package.rs` -> `cargo_package/mod.rs`
* Extract vcs info helpers to `cargo_package/vcs.rs`
* Extract verification helpers to `cargo_package/verify.rs`
Doing so because I realized how big `cargo_package.rs` has grown.
Like, the vcs helpers will continue growing with #14981 and potentially
other optimizations.
### How should we test and review this PR?
All tests pass.
### What does this PR try to resolve?
This adds a special case for `package.{readme,license-file}`
to Git VCS status check.
If they were specified with paths outside the current package root,
but still under git workdir, Cargo checks git status of those files
to determine if they were dirty.
We don't need to take care of other fields with path values because
* `PathSource` only list files under the package root.
Things like `target.path` works for `cargo build`, but won't be
included in `.crate` file from `cargo publish`.
* The only exceptions are `package.readme`/`package.license-file`.
Cargo would copy files over if they are outside package root.
### How should we test and review this PR?
While this doesn't fix ever case listed in #14967,
it at least fixes one of them.
This adds a special case for `package.{readme,license-file}`
to Git VCS status check.
If they were specified with paths outside the current package root,
but still under git workdir, Cargo checks git status of those files
to determine if they were dirty.
We don't need to take care of other fields with path values because
* `PathSource` only list files under the package root.
Things like `target.path` works for `cargo build`, but won't be
included in `.crate` file from `cargo publish`.
* The only exceptions are `package.readme`/`package.license-file`.
Cargo would copy files over if they are outside package root.
This is a test showing corner cases that dirty files outside
the package being packaging actually made the `.crate` file dirty.
However, `cargo package` and `.cargo_vcs_info.json` didn't capture it.
### What does this PR try to resolve?
So we don't need to `p.to_str().unwrap()`
and are able to pass different types for each argument
### How should we test and review this PR?
No response.
<!--
Thanks for submitting a pull request 🎉! Here are some tips for you:
* If this is your first contribution, read "Cargo Contribution Guide"
first:
https://doc.crates.io/contrib/
* Run `cargo fmt --all` to format your code changes.
* Small commits and pull requests are always preferable and easy to
review.
* If your idea is large and needs feedback from the community, read how:
https://doc.crates.io/contrib/process/#working-on-large-features
* Cargo takes care of compatibility. Read our design principles:
https://doc.crates.io/contrib/design.html
* When changing help text of cargo commands, follow the steps to
generate docs:
https://github.com/rust-lang/cargo/tree/master/src/doc#building-the-man-pages
* If your PR is not finished, set it as "draft" PR or add "WIP" in its
title.
* It's ok to use the CI resources to test your PR, but please don't
abuse them.
### What does this PR try to resolve?
Explain the motivation behind this change.
A clear overview along with an in-depth explanation are helpful.
You can use `Fixes #<issue number>` to associate this PR to an existing
issue.
### How should we test and review this PR?
Demonstrate how you test this change and guide reviewers through your
PR.
With a smooth review process, a pull request usually gets reviewed
quicker.
If you don't know how to write and run your tests, please read the
guide:
https://doc.crates.io/contrib/tests
### Additional information
Other information you want to mention in this PR, such as prior arts,
future extensions, an unresolved problem, or a TODO list.
-->
### What does this PR try to resolve?
Fixes#14154
~~Added some of the missing package metadata to the fingerprint so that
rebuilds are triggered correctly.~~
~~Also updated the test to prevent a future regression.~~
Moved the manifest metadata tracking to use dep info in favor of
fingerprint so that rebuilds are only triggered if the metadata is
actually used.
This change moves the manifest metadata track to dep-info files
with the goal of reduce unneeded rebuilds when metadata is changed as
well fixing issues where builds are not retrigged due to metadata
changes when they should (ie. #14154)
Personally I liked that the test was only dependent on what really matters, the lack of presence of a particular filename. Now the test would fail if Cargo one day adds more (generated) files to the package.
Co-authored-by: Weihang Lo <weihanglo@users.noreply.github.com>
### What does this PR try to resolve?
Despite being interned `SourceId::Eq` is not a `ptr::eq`. Which in turn
is because `SourceId`s concept of identity is a complete mess. The code
relies on having to IDs that are `Eq` but do not have the same values
for their fields.
As one measure of this `SourceId` has an `impl Hash` which does
something different from `fn full_hash` and `fn stable_hash`. Separately
`SourceIdInner` has a different implementation. Similar levels of
complexity exist for `Eq`. Every one of these `impl`s was added due to a
real bug/issue we've had that needs to stay fixed. Not all of witch are
reproducible enough to have made it into our test suite.
I [have some
ideas](https://github.com/rust-lang/cargo/pull/14665#issuecomment-2412077472)
for how to reorganize the types so that this is easier to reason about
and faster. But given the history and the complexity I want to move
extremely carefully.
### How should we test and review this PR?
The test pass, and it's a one line change, but this still needs careful
review.
### Additional information
r? @ehuss I remember you and Alex working very hard to track down most
of these bugs.
The main benefit is that it won't facilitate hangs due to attempts to
read from untrackable directory entries, like names pipes or sockets.
Related to https://github.com/GitoxideLabs/gitoxide/pull/1629
### Tasks
* [x] upgrade
* [x] incorporate updated `gix` [once everything is
fixed](https://github.com/GitoxideLabs/gitoxide/pull/1740).
* [x] assure tests work
### Postponed
It turns out that the new `gix` version doesn't magically fix the FIFO
issue, so the following test still fails.
It's not super-trivial to fix apparently (I tried), so let's do it in a
separate PR.
Here is the patch I have so far in case anyone is interested to fix it
earlier or wants to share insights :).
```patch
commit dfef545eae215f0b9da9f3d4424b52cba7edaec3
Author: Sebastian Thiel <sebastian.thiel@icloud.com>
Date: Sun Dec 22 19:05:40 2024 +0100
fix: assure possibly blocking non-files (like FIFOs) won't be picked up for publishing.
This would otherwise cause the publish to hang.
diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs
index 776590697..c78463a32 100644
--- a/src/cargo/sources/path.rs
+++ b/src/cargo/sources/path.rs
@@ -626,8 +626,11 @@ fn list_files_gix(
.filter(|res| {
// Don't include Cargo.lock if it is untracked. Packaging will
// generate a new one as needed.
+ // Also don't include untrackable directory entries, like FIFOs.
res.as_ref().map_or(true, |item| {
- !(item.entry.status == Status::Untracked && item.entry.rela_path == "Cargo.lock")
+ item.entry.disk_kind != Some(gix::dir::entry::Kind::Untrackable)
+ && !(item.entry.status == Status::Untracked
+ && item.entry.rela_path == "Cargo.lock")
})
})
.map(|res| res.map(|item| (item.entry.rela_path, item.entry.disk_kind)))
diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs
index 1740de4ac..1c6b3db89 100644
--- a/tests/testsuite/package.rs
+++ b/tests/testsuite/package.rs
@@ -6873,3 +6873,29 @@ See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for
"#]])
.run();
}
+
+#[cargo_test]
+#[cfg(unix)]
+fn simple_with_fifo() {
+ let p = project()
+ .file(
+ "Cargo.toml",
+ r#"
+ [package]
+ name = "foo"
+ version = "0.1.0"
+ edition = "2015"
+ "#,
+ )
+ .file("src/main.rs", "fn main() {}")
+ .build();
+
+ std::process::Command::new("mkfifo")
+ .current_dir(p.root())
+ .arg(p.root().join("blocks-when-read"))
+ .status()
+ .expect("a FIFO can be created");
+
+ // If this hangs, Cargo tried to package a FIFO and is reading it forever.
+ p.cargo("package").run();
+}
```
The main benefit is that it won't facilitate hangs due to attempts
to read from untrackable directory entries, like names pipes or
sockets.
Related to https://github.com/GitoxideLabs/gitoxide/pull/1629
### What does this PR try to resolve?
Address
https://github.com/rust-lang/cargo/pull/14968#issuecomment-2555901072
> I think the ideal solution is to be relative to the current directory
but that takes more work and this incremental improvement is great!
Sorry I should have noticed your comment earlier.
This enables merge conflict notifications on GitHub. Homu was providing
this service for us, but it was shut down today. I find these
notifications useful, as they give an opportunity for the author to
proactively rebase and not require a round trip with the reviewer.
### What does this PR try to resolve?
This PR is resolving an
[issue](https://github.com/rust-lang/cargo/issues/14776#issuecomment-2515017963)
where `cargo doc --open --examples` does not give a clear message when
the examples file is given to it.
I changed the message from `no crates with documentation` in the
[cargo_doc.rs](58b2d609ec/src/cargo/ops/cargo_doc.rs (L62))
and
[doc.rs](58b2d609ec/tests/testsuite/doc.rs (L1504))
to `requested crate documentation is not available to open`. Now it
becomes easy for the user to understand what they're missing.
Fixes#14776
### How should we test and review this PR?
Here is the command through which you can test this change:
`cargo test SNAPSHOTS=overwrite -- doc::open_no_doc_crate`
<!--
Thanks for submitting a pull request 🎉! Here are some tips for you:
* If this is your first contribution, read "Cargo Contribution Guide"
first:
https://doc.crates.io/contrib/
* Run `cargo fmt --all` to format your code changes.
* Small commits and pull requests are always preferable and easy to
review.
* If your idea is large and needs feedback from the community, read how:
https://doc.crates.io/contrib/process/#working-on-large-features
* Cargo takes care of compatibility. Read our design principles:
https://doc.crates.io/contrib/design.html
* When changing help text of cargo commands, follow the steps to
generate docs:
https://github.com/rust-lang/cargo/tree/master/src/doc#building-the-man-pages
* If your PR is not finished, set it as "draft" PR or add "WIP" in its
title.
* It's ok to use the CI resources to test your PR, but please don't
abuse them.
### What does this PR try to resolve?
Explain the motivation behind this change.
A clear overview along with an in-depth explanation are helpful.
You can use `Fixes #<issue number>` to associate this PR to an existing
issue.
### How should we test and review this PR?
Demonstrate how you test this change and guide reviewers through your
PR.
With a smooth review process, a pull request usually gets reviewed
quicker.
If you don't know how to write and run your tests, please read the
guide:
https://doc.crates.io/contrib/tests
### Additional information
Other information you want to mention in this PR, such as prior arts,
future extensions, an unresolved problem, or a TODO list.
-->
### What does this PR try to resolve?
Dirty file paths in the original message were stripped relative to
package root.
User is not able to know the full path to find dirty files.
This PR makes it relative to git workdir.
### How should we test and review this PR?
This was found during #14962
### What does this PR try to resolve?
This PR adds the `test` cfg as a well known Cargo cfg as
https://github.com/rust-lang/compiler-team/issues/785 is now in FCP and
before the compiler change.
### How should we test and review this PR?
Look at the new argument passed and the test changes.
### Additional information
Detailed motivation at
https://github.com/rust-lang/compiler-team/issues/785, but summary
`test` is a weird well known cfg for `rustc` as it's mostly a convention
and build system like Cargo may to set it conditionally (`lib.test =
false` for Cargo, not done in this PR).
Pre-require to https://github.com/rust-lang/rust/pull/131729.
r? @epage
### What does this PR try to resolve?
Flatten code with let-else for easier reasoning of exit of
`check_repo_state` function.
### How should we test and review this PR?
This also adds some more debug logs.
Shouldn't affect any real production behavior.
### What does this PR try to resolve?
This helps debug <https://github.com/rust-lang/cargo/issues/14955>.
### How should we test and review this PR?
While `check_repo_state` is the culprit, let's add some traces for
future.
### Additional information
stdlib crates get a SourceId which has an absolute path pointing into
the sysroot. This makes the metadata hash change depending on where
you've installed Rust. This is causing problems because the different
hashes snowball into the optimizer making different decisions which ends
up changing the binary size.
(Some context: at work we're working with embedded devices with little
flash storage so it often happens that a binary builds locally and then
fails to fit in flash in CI, just because CI has installed rustc to a
different path. Improving binary size is *not* a goal of this PR, after
the fix the size will be whatever, but at least it won't change based on
the rustc path anymore)
Overview of the fix:
- For libstd crates, the metadata hash now contains the path relative to
the sysroot, instead of the absolute path.
- The absolute path is still hashed into the fingerprint (not the
metadata) so moving the rustc installation triggers a rebuild. This
ensures stdlib crates are rebuilt when upgrading nightly versions.
- The rustc version is still hashed into the metadata as usual, so
upgrading Rust releases (not nightly versions) does cause a metadata
change.
Repro of the bug:
```
$ git clone https://github.com/embassy-rs/embassy --branch cargo-nondet-repro
$ cd embassy/
$ cd examples/nrf52840
$ RUSTUP_HOME=~/.rustup1 cargo build --release --bin wifi_esp_hosted
....
Finished `release` profile [optimized + debuginfo] target(s) in 13.33s
$ llvm-size target/thumbv7em-none-eabi/release/wifi_esp_hosted
text data bss dec hex filename
114500 80 48116 162696 27b88 target/thumbv7em-none-eabi/release/wifi_esp_hosted
$ RUSTUP_HOME=~/.rustup2 cargo build --release --bin wifi_esp_hosted
....
Finished `release` profile [optimized + debuginfo] target(s) in 9.64s
$ llvm-size target/humbv7em-none-eabi/release/wifi_esp_hosted
text data bss dec hex filename
114272 80 48116 162468 27aa4 target/thumbv7em-none-eabi/release/wifi_esp_hosted
```
### What does this PR try to resolve?
This was always enabled on nightly since 1.83-nightly (2024-09).
We have no feedback since then, so assume it is a low-impact change.
This stabilization is targeted at 1.85 (2025-02-20)
Fixes#14346
### How should we test and review this PR?
Let's do an FCP.
This was always enabled on nightly since 1.83-nightly (2024-09).
We have no feedback since then, so assume it is a low-impact change.
This stabilization is targeted at 1.85 (2025-02-20)
stdlib crates get a SourceId which has an absolute path pointing into the sysroot. This
makes the metadata hash change depending on where you've installed Rust. This is causing
problems because the different hashes snowball into the optimizer making different
decisions which ends up changing the binary size.
(Some context: at work we're working with embedded devices with little flash storage
so it often happens that a binary builds locally and then fails to fit in flash in CI,
just because CI has installed rustc to a different path. Improving binary size is
*not* a goal of this PR, after the fix the size will be whatever, but at least it
won't change based on the rustc path anymore)
Overview of the fix:
- For libstd crates, the metadata hash now contains the path relative to the sysroot, instead of the absolute path.
- The absolute path is still hashed into the fingerprint (not the metadata) so moving the rustc installation triggers a rebuild. This ensures stdlib crates are rebuilt when upgrading nightly versions.
- The rustc version is still hashed into the metadata as usual, so upgrading Rust releases (not nightly versions) does cause a metadata change.
Repro of the bug:
```
$ git clone https://github.com/embassy-rs/embassy --branch cargo-nondet-repro
$ cd embassy/
$ cd examples/nrf52840
$ RUSTUP_HOME=~/.rustup1 cargo build --release --bin wifi_esp_hosted
....
Finished `release` profile [optimized + debuginfo] target(s) in 13.33s
$ llvm-size target/thumbv7em-none-eabi/release/wifi_esp_hosted
text data bss dec hex filename
114500 80 48116 162696 27b88 target/thumbv7em-none-eabi/release/wifi_esp_hosted
$ RUSTUP_HOME=~/.rustup2 cargo build --release --bin wifi_esp_hosted
....
Finished `release` profile [optimized + debuginfo] target(s) in 9.64s
$ llvm-size target/humbv7em-none-eabi/release/wifi_esp_hosted
text data bss dec hex filename
114272 80 48116 162468 27aa4 target/thumbv7em-none-eabi/release/wifi_esp_hosted
```
### What does this PR try to resolve?
Blocked on <https://github.com/rust-lang/cargo/pull/14943> (or can just
merge this one).
Fixes#14935#14935 failed because since 125e873dffc4b68b263c5decd88750ec10fd441e
[`std_resolve`][1] only includes `sysroot` as primary package.
When any custom Cargo feature is provided via `-Zbuild-std-feature`,
the default feature set `panic-unwind` would be gone, so no
`panic_unwind` crate presents in `std_resolve`.
When then calling [`std_resolve.query`][2] with the default set of
crates from [`std_crates`][3], which automatically includes
`panic_unwind` when `std` presents, it'll result in spec not found
because `panic_unwind` was not in `std_resolve` anyway.
[1]:
addcc8ca71/src/cargo/core/compiler/standard_lib.rs (L96)
[2]:
addcc8ca71/src/cargo/core/compiler/standard_lib.rs (L158)
[3]:
addcc8ca71/src/cargo/core/compiler/standard_lib.rs (L156)
### How should we test and review this PR?
This patch is kinda a revert of 125e873dffc4b68b263c5decd88750ec10fd441e
in terms of the behavior.
With this, now `std_resolve` is always resolved to the same set of
packages that Cargo will use to generate the unit graph, (technically
the same set of crates + `sysroot`), by sharing the same set of primary
packages via `std_crates` functions.
Note that when multiple `--target`s provided, if std is specified or
there
is one might support std, Cargo will always resolve std dep graph.
To test it manually, run
```
RUSTFLAGS="-C panic=abort" cargo +nightly-2024-12-15 b -Zbuild-std=std,panic_abort -Zbuild-std-features=panic_immediate_abort
```
change to this PR's cargo with the same nightly rustc, it would succeed.
I am a bit reluctant to add an new end-2end build-std test, but I still
did it.
A bit scared when mock-std gets out-of-sync of features in std in
rust-lang/rust.
