test(publish): Cover more wait-for-publish cases
These came from trying to guess what cases are causing problems in #11314. Unfortunately, can't reproduce it so far but figured it'd be good to keep these around.
Make cargo forward pre-existing CARGO if set
Currently, Cargo will always set `$CARGO` to point to what it detects its own path to be (using `std::env::current_exe`). Unfortunately, this runs into trouble when Cargo is used as a library, or when `current_exe` is not actually the binary itself (e.g., when invoked through Valgrind or `ld.so`), since `$CARGO` will not point at something that can be used as `cargo`. This, in turn, means that users can't currently rely on `$CARGO` to do the right thing, and will sometimes have to invoke `cargo` directly from `$PATH` instead, which may not reflect the `cargo` that's currently in use.
This patch makes Cargo re-use the existing value of `$CARGO` if it's already set in the environment. For Cargo subcommands, this will mean that the initial invocation of `cargo` in `cargo foo` will set `$CARGO`, and then Cargo-as-a-library inside of `cargo-foo` will inherit that (correct) value instead of overwriting it with the incorrect value `cargo-foo`. For other execution environments that do not have `cargo` in their call stack, it gives them the opportunity to set a working value for `$CARGO`.
One note about the implementation of this is that the test suite now needs to override `$CARGO` explicitly so that the _user's_ `$CARGO` does not interfere with the contents of the tests. It _could_ remove `$CARGO` instead, but overriding it seemed less error-prone.
Fixes#10119.
Fixes#10113.
Only remove fingerprints and build script artifacts of the requested package
Fixes#10069
This is my first PR to cargo. Let me know if you want me to add tests, or if there are any other changes you would like to see :)
This PR changes the globs used to remove fingerprints and build artifacts when running `cargo clean -p <pkid>`. The glob used was `<package_name>-*` which would match artifacts for packages that are prefixed by `<package_name>-` (e.g. `cargo clean -p sqlx` would also remove artifacts for `sqlx-{core,macros,etc.}`). This problem is not seen with other artifacts since those use the crate name instead of package name which normalize hyphens to underscores.
The changes follow the naive approach mentioned in #10069 where some basic string manipulation is done to strip off the last hyphen, hash, and potential extension to get the original package name which can be used to determine if the artifact is actually for the desired package. This means that this **does not** handle trying to resolve the package to determine the artifacts, so it still ignores the url and version that may be passed in the pkgid
Clean stale git temp files
### What does this PR try to resolve?
When cargo is interrupted while libgit2 is indexing the pack file, it will leave behind a temp file that never gets deleted. These files can be very large. This checks for these stale temp files and deletes them.
### How should we test and review this PR?
There is a simulated test here. To test with the actual behavior:
1. Run `CARGO_HOME=chome cargo fetch`
2. While it is "resolving deltas", hit Ctrl-C.
3. Notice that there is a 200MB file in `chome/registry/index/github.com-1ecc6299db9ec823/.git/objects/pack/`
4. Do that several times if you want, each time adds another 200MB file.
5. Build this PR: `cargo b -r`
6. Run `CARGO_HOME=chome CARGO_LOG=cargo::sources::utils=debug ./target/release/cargo fetch`
7. Notice that it deletes the `pack_git2_*` files.
add a note that some warnings (and/or errors) can be auto-fixed
This adds a note that some warnings (and/or errors) can be auto-fixed by running `cargo --fix`. This only supports `cargo check` as we can't show the same note for `cargo clippy` since it is an external subcommand.
The message for the note was taken from [#10976 (comment)](https://github.com/rust-lang/cargo/issues/10976#issuecomment-1212517765).
fix(publish): Block until it is in index
Originally, crates.io would block on publish requests until the publish
was complete, giving `cargo publish` this behavior by extension. When
crates.io switched to asynchronous publishing, this intermittently broke
people's workflows when publishing multiple crates. I say interittent
because it usually works until it doesn't and it is unclear why to the
end user because it will be published by the time they check. In the
end, callers tend to either put in timeouts (and pray), poll the
server's API, or use `crates-index` crate to poll the index.
This isn't sufficient because
- For any new interested party, this is a pit of failure they'll fall
into
- crates-index has re-implemented index support incorrectly in the past,
currently doesn't handle auth, doesn't support `git-cli`, etc.
- None of these previous options work if we were to implement
workspace-publish support (#1169)
- The new sparse registry might increase the publish times, making the
delay easier to hit manually
- The new sparse registry goes through CDNs so checking the server's API
might not be sufficient
- Once the sparse registry is available, crates-index users will find
out when the package is ready in git but it might not be ready through
the sparse registry because of CDNs
So now `cargo` will block until it sees the package in the index.
- This is checking via the index instead of server APIs in case there
are propagation delays. This has the side effect of being noisy
because of all of the "Updating index" messages.
- This blocks by default but there is an unstable `publish.timeout` config field that will disable blocking when set to 0. See #11222 for stablization
Blocking is opt-out as that is the less error prone case for casual users while those doing larger integrations are also likely to do the testing needed to make more complicated scenarios work where blocking is disabled.
Right now we block after the publish. An alternative would be to block until all dependencies are in the index which makes the blocking only happen when needed
- Blocking on dependencies can be imprecise to detect when to block vs propagate an error up
- This is the less error prone case for users. For example I recently publish a crate in one tab and immediately switched to another tab to use it and this only worked because `cargo-release` blocked until it was ready to use
In reviewing this change, be sure to look at the individual commits
- The first makes it possible to write the tests for this
- The second adds a test that shows the current behavior
- The third updates the test to the expected behavior, showing all of this works
In addition to the publish tests:
- We want to maximize the nightly-to-stable time to collect feedback
- We will put this in TWiR's testing section to raise visibility
Fixes#9507
Previously, `is_dep_activated` depends on `activated_dependencies`,
which is a map of `PackageFeaturesKey` to its own activated `DepFeature`s.
`PackageFeaturesKey` in feature resolver is always comprised of
* A `PackageId` of a given dependency, and
* A enum value that helps decoupling activated features for that dependency.
Currently depending on the type of given dependency.
Looking into issue 10526, it has an `activated_dependencies` of
```
{
(mycrate, NormalOrDevOrArtifactTarget(None)) -> [dep:mybindep]
}
```
However, this [line][1] accidentally use a parent's `pkgid`
and a dependency's `FeaturesFor` to compose a key:
```
(mycrate, NormalOrDevOrArtifactTarget("x86_64-unknown-linux-gnu"))
```
That is never a valid key to query features for artifacts dependency.
A valid key should be comprised of components both from the parent:
```
(mycrate, NormalOrDevOrArtifactTarget(None))
```
Or both from the dependency itself:
```
(mybindep, NormalOrDevOrArtifactTarget("x86_64-unknown-linux-gnu"))
```
As aforementioned `activated_dependencies` only stores parent packages
and their activated features. Those informations are included in
`activated_features` as well, so this commit goes with the route that
removes `activated_dependencies` and uses only dependency's infomation
to query if itself is activated.
[1]: 0b84a35c2c/src/cargo/core/compiler/unit_dependencies.rs (L1097-L1098)
Fix confusing error messages when using -Zsparse-registry
Built-in replacements of crates.io have confusing a description:
> crates.io index (which is replacing registry `crates-io`)
This adds a special case for built-in source replacements of `crates-io`. User-defined replacements of crates.io continue to use the existing description.
Also fixes the test framework `__CARGO_TEST_CRATES_IO_URL_DO_NOT_USE_THIS` variable to strip the `sparse+` prefix when checking if a url `is_crates_io`.
Fixes#11277
Currently, Cargo will always set `$CARGO` to point to what it detects
its own path to be (using `std::env::current_exe`). Unfortunately, this
runs into trouble when Cargo is used as a library, or when `current_exe`
is not actually the binary itself (e.g., when invoked through Valgrind
or `ld.so`), since `$CARGO` will not point at something that can be used
as `cargo`. This, in turn, means that users can't currently rely on
`$CARGO` to do the right thing, and will sometimes have to invoke
`cargo` directly from `$PATH` instead, which may not reflect the `cargo`
that's currently in use.
This patch makes Cargo re-use the existing value of `$CARGO` if it's
already set in the environment. For Cargo subcommands, this will mean
that the initial invocation of `cargo` in `cargo foo` will set `$CARGO`,
and then Cargo-as-a-library inside of `cargo-foo` will inherit that
(correct) value instead of overwriting it with the incorrect value
`cargo-foo`. For other execution environments that do not have `cargo`
in their call stack, it gives them the opportunity to set a working
value for `$CARGO`.
One note about the implementation of this is that the test suite now
needs to override `$CARGO` explicitly so that the _user's_ `$CARGO` does
not interfere with the contents of the tests. It _could_ remove `$CARGO`
instead, but overriding it seemed less error-prone.
Fixes#10119.
Fixes#10113.
Improve the error message if `publish` is `false` or empty list
### What does this PR try to resolve?
close https://github.com/rust-lang/cargo/issues/11262
Improve the error message if `publish` is the false or empty list.
Say `publish` is set to `false` or an empty list in Cargo.toml and prevents publishing.
### How should we test and review this PR?
Unit test
### Additional information
If there was a way we could tell `publish` to be empty or `false`, I think it would get better. If you know an easy way to implement it, please feel free to comment.
Add test for deleted index entry
This adds a test for when an entry is deleted from the index. This is done on crates.io occasionally for things like copyright takedown requests.
This behavior was tripping a debug assert which this removes. I'm not entirely certain why the debug assert is there, but I think it is not correct, since this is obviously a scenario where there might be a cached file, but the index responds with NotFound.
Originally, crates.io would block on publish requests until the publish
was complete, giving `cargo publish` this behavior by extension. When
crates.io switched to asynchronous publishing, this intermittently broke
people's workflows when publishing multiple crates. I say interittent
because it usually works until it doesn't and it is unclear why to the
end user because it will be published by the time they check. In the
end, callers tend to either put in timeouts (and pray), poll the
server's API, or use `crates-index` crate to poll the index.
This isn't sufficient because
- For any new interested party, this is a pit of failure they'll fall
into
- crates-index has re-implemented index support incorrectly in the past,
currently doesn't handle auth, doesn't support `git-cli`, etc.
- None of these previous options work if we were to implement
workspace-publish support (#1169)
- The new sparse registry might increase the publish times, making the
delay easier to hit manually
- The new sparse registry goes through CDNs so checking the server's API
might not be sufficient
- Once the sparse registry is available, crates-index users will find
out when the package is ready in git but it might not be ready through
the sparse registry because of CDNs
So now `cargo` will block until it sees the package in the index.
- This is checking via the index instead of server APIs in case there
are propagation delays. This has the side effect of being noisy
because of all of the "Updating index" messages.
- This is done unconditionally because cargo used to block and that
didn't seem to be a problem, blocking by default is the less error
prone case, and there doesn't seem to be enough justification for a
"don't block" flag.
The timeout was 5min but I dropped it to 1m. Unfortunately, I don't
have data from `cargo-release` to know what a reasonable timeout is, so
going ahead and dropping to 60s and assuming anything more is an outage.
Fixes#9507
Originally, crates.io would block on publish requests until the publish
was complete, giving `cargo publish` this behavior by extension. When
crates.io switched to asynchronous publishing, this intermittently broke
people's workflows when publishing multiple crates. I say interittent
because it usually works until it doesn't and it is unclear why to the
end user because it will be published by the time they check. In the
end, callers tend to either put in timeouts (and pray), poll the
server's API, or use `crates-index` crate to poll the index.
This isn't sufficient because
- For any new interested party, this is a pit of failure they'll fall
into
- crates-index has re-implemented index support incorrectly in the past,
currently doesn't handle auth, doesn't support `git-cli`, etc.
- None of these previous options work if we were to implement
workspace-publish support (#1169)
- The new sparse registry might increase the publish times, making the
delay easier to hit manually
- The new sparse registry goes through CDNs so checking the server's API
might not be sufficient
- Once the sparse registry is available, crates-index users will find
out when the package is ready in git but it might not be ready through
the sparse registry because of CDNs
This introduces unstable support for blocking by setting
`publish.timeout` to non-zero value.
A step towards #9507
refactor(tests): Prepare for wait-for-publish test changes
In #11062, we are updating `cargo publish` to wait until a package is published. The problem is a lot of our tests will block until the timeout. In finding the tests to update, I was originally relying on test failures from the extra output when timing out. The problem is not all tests verify the test output so they don't fail.
This tries to update the tests to make the introduction of a timeout more obvious.
- Adding `with_stderr` where it wasn't before
- Moving away from `with_stderr_contains` for publish tests
To help with that, I made the predicates on cargo commands more consistent.
I also moved descriptions of tests to be outside of the test so I can more easily document the `registry::init` calls with what we are doing.
