When running `cargo package` the verify the build cache
(target-dir/build-dir) will not be used and all dependencies will be
recompiled.
This is inconsistent as setting target dir (via `CARGO_TARGET_DIR` for
example) will cause `cargo package` to reuse the build cache.
This commit changes the default behavior to always use the build cache,
matching the behavior of having target-dir set.
This should also help fixing these spurious "cannot package because
some excluded file is untracked" issues.
Remove the respective `git2` implementation at the same time
as there seems to be no need for it.
A user will now be able to use flags like `--workspace` with `cargo
publish`.
`cargo package` will now also work with those flags without having to
pass `--no-verify --exclude-lockfile`.
Many release tools have come out that solve this problem.
They will still need a lot of the logic that went into that for other
parts of the release process.
However, a cargo-native solution allows for:
- Verification during dry-run
- Better strategies for waiting for the publish timeout
`cargo publish` is non-atomic at this time.
If there is a server side error, network error, or rate limit during the publish,
the workspace will be left in a partially published state.
Verification is done before any publishing so that won't affect things.
There are multiple strategies we can employ for improving this over time,
including
- atomic publish
- `--idempotent` (#13397)
- leave this to release tools to manage
This includes support for `--dry-run` verification.
As release tools didn't have a way to do this before,
users may be surprised at how slow this is because a `cargo build` is
done instead of a `cargo check`. This is being tracked in #14941.
This adds to `cargo package` the `--registry` and `--index` flags to
help with resolving dependencies when depending on a package being
packaged at that moment.
These flags are only needed when a `cargo package --workspace` operation
would have failed before due to inability to find a locally created
dependency.
Regarding the publish timeout, `cargo publish --workspace` publishes
packages in batches and we only timeout if nothing in the batch has
finished being published within the timeout, deferring the rest to the
next wait-for-publish. So for example, if you have packages `a`, `b`, `c` then
we'll wait up to 60 seconds and if only `a` and `b` were ready in that time,
we'll then wait another 60 seconds for `c`.
During testing, users ran into issues with `.crate` checksums that we've
not been able to reproduce since:
- https://github.com/rust-lang/cargo/issues/1169#issuecomment-2567995987
- #14396
By stabilizing this, Cargo's behavior becomes dependent on an overlay
registry.
When generating a lockfile or verifying a package, we overlay the
locally generated `.crate` files on top of the registry so the registry
appears as it would and everything works.
If there is a conflict with a version, the local version wins which is
important for the dry-run mode of release tools as they won't have
bumped the version yet.
Our concern for the overlay registry is dependency confusion attacks.
Considering this is not accessible for general user operations, this
should be fine.
Fixes#1169Fixes#10948
### What does this PR try to resolve?
This PR reworks `cargo-test-support` and `testsuite` to use Snapbox's
[`cargo_bin!()`](https://docs.rs/snapbox/latest/snapbox/cmd/macro.cargo_bin.html)
instead of
[`cargo_bin()`](https://docs.rs/snapbox/latest/snapbox/cmd/fn.cargo_bin.html)
which makes assumptions about the structure of Cargo's build directory.
`cargo_bin!()` uses `CARGO_BIN_EXE_*` for locating the `cargo` binary
which should be more resilient to directory/layout changes.
Linking a relevant Zulip discussion
[here](https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/cargo_bin_exe.20and.20tests/with/513638220)[#t-cargo
> cargo_bin_exe and
tests](https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/cargo_bin_exe.20and.20tests/with/513638220)
As shown in that link, we could make these variables available at
runtime and not need to do this. However, `cargo-test-support`, as an
API, is a bit weird in that it is baking in support for one specific
binary. This can be confusing for callers and makes it more annoying for
callers provide their own `fn cargo`, e.g. see crate-ci/cargo-fixit#7
### Implementation Notes
`cargo_bin!()` only works when being called from the `testsuite` as it's
only set when executing integration tests and `cargo-test-support` is a
regular crate.
To make this change, I introduced an extension trait `CargoProjectExt`
in `testsuite` for running `.cargo()` and implemented it on `Project`.
In `cargo-test-support` other functionality relies on `.cargo()` so
these also needed to be moved to `testsuite`
*
[`src/tools.rs`](https://github.com/rust-lang/cargo/blob/master/crates/cargo-test-support/src/tools.rs)
* Parts
[`src/cross_compile`](https://github.com/rust-lang/cargo/blob/master/crates/cargo-test-support/src/cross_compile.rs)
* I had to split this up unfortunately, as `disabled()` requires running
Cargo to check if we should disable cross compile tests.
* Other fns in `cross_compile` are used in `cargo-test-support` so
moving everything to `testsuite` would have ended up requiring moving
many things to test suite.
### How to test and review this PR?
I'd definitely recommend reviewing commit by commit.
There are a lot of diffs due to the nature of reorganizing things.
I did my best to split things things into smaller PRs but they still
contain a lot of `use` statement diffs.
r? @epage
### What does this PR try to resolve?
The `clippy::perf` lint group is fairly useful for catching bad
practices that might hurt performance marginally.
This PR fixes most of them except `clippy::large_enum_variant`, which
doesn't feel right at this moment and need more researches. And that is
why I didn't enable the lint group.
### How to test and review this PR?
The `clippy::perf` lint group is fairly useful for catching bad
practices that might hurt performance marginally.
This PR fixes most of them except `clippy::large_enum_variant`,
which doesn't feel right at this moment and need more researches.
Anyway, overall this PR should be good.
This reverts commit 71ea2e5c5fa285e8e0336d51fd03ba4a427154bf.
`Repository::discover` and `Repository::status_file` are too expenstive
to run inside a loop. And `cargo package` are doing a lot of duplicate
works for checking submodule VCS status.
The possible fix might look like
* Let `status_submodules` function returns a path entry set, so
Cargo can check whether a source file is dirty based on that.
* When listing files in `PathSource`, attach the VCS status of a
path entry assoicated with. Then subsequent operations can skip
status check entirely.
The above solutions are not trivial, and the dirtiness check is
informational only based on T-cargo conclusion, so we should be
good just reverting the change now.
Again, the caveat of this is that we can't really detect
dirty symlinks that links into a Git submodule.
If a there is a symlink into a git repository/submodule,
when checking its git status with the wrong outer repo,
we'll get an NotFound error,
as the object doesn't belong to the outer repository.
This kind of error blocked the entire `cargo package` operation.
This fix additionally discovers the nearest Git repository,
and then checks status with that,
assuming the repo is the parent of the source file of the symlink.
This is a best effort solution, so if the check fails we ignore.
Dirtiness check for symlinks is mostly informational.
And changes in submodule would fail git-status as well (see #15384).
To avoid adding complicated logic to handle that,
for now we ignore the status check failure.
Stop a few tests which aren't marked with `public_network_test` from
trying to access crates.io. While doing so, also make sure that other
`cargo_add` and `cargo_info` tests initialize the registry.
Lockfile might be gitignore'd,
So it never shows up in `git status`.
However, `cargo packag` vcs checks actually performs
`git status --untracked --ignored`.
It is a bit confusing that lockfile is ignored but still counts as dirty
from the report of `cargo package`.
There are some more nuances:
We check lockfile's VCS status if the lockfile is ouside the current
package root. That means a non-workspace Cargo package will not have
this VCS check. I don't think it is good that we have diverged behaviors
Hence this revert.
We can always re-evaluate later,
as we've reserved rooms for doing more dirty checks.
https://github.com/rust-lang/cargo/issues/14967#issuecomment-2651329783
### What does this PR try to resolve?
Fixes#15059Fixes#15159
This provides an escape hatch `--exclude-lockfile`for uncommon workflows
that don't verify (`--no-verify` is passed) the build with their
unpublished packages
In effect, this takes the heuristic removed in #14815 and replaces it
with a flag
When `--exclude-lockfile` is enabled,
`cargo package` will not verify the lock file if present,
nor will it generate a new one if absent.
Cargo.lock will not be included in the resulting tarball.
Together with `--no-verify`,
this flag decouples packaging from checking the registry index.
While this is useful for some non-normal workflows that requires
to assemble packages having unpublished dependencies.
It is recommended to use `-Zpackage-workspace` to package the entire
workspace, instead of opting out lockfile.
### How should we test and review this PR?
The first commit was stolen from
<1a104b5444>
(credit to @NoisyCoil!)
The second added two failing cases we observed in #15059.
### Additional information
Lockfile might not be under the package root, but its entries
may be outdated and get packaged into the `.crate` tarball.
We take a conservative action that if the lockfile is dirty,
then all workspace members are considered dirty.
Workspace manifest might not be under the package root, but
workspace inheritance may still affect what is packaged into
the `.crate.` tarball. We take a conservative action that if
the workspace manifest is dirty, then all workspace members
are considered dirty.
### What does this PR try to resolve?
Just found we can simplify `human_readable_bytes` a bit.
This also remove the `bytesize` dependency,
since we already have a hand-rolled function.
(It is a good crate, though)
### How should we test and review this PR?
Should have no real functional difference.
`cargo package` starts reporting IEC (KiB) instead of SI (KB).
So do some thresholds of `cargo package`.
Could also switch entirely to `bytesize` if that is preferred.
### Additional information
When `--exclude-lockfile` is enabled,
`cargo package` will not verify the lock file if present,
nor will it generate a new one if absent.
Cargo.lock will not be included in the resulting tarball.
Together with `--no-verify`,
this flag decouples packaging from checking the registry index.
While this is useful for some non-normal workflows that requires
to assemble packages having unpublished dependencies.
It is recommended to use `-Zpackage-workspace` to package the entire
workspace, instead of opting out lockfile.
After dd698ff0483bda065d1f78968e70ee86fb116a72,
`cargo package --no-verify` at least fails in three different cases:
* An unpublished package depending on itself as a dev-dependency (cyclic self-referential dev-dependencies).
* Can be resolved by removing the `version` field from the affected dev-dependency.
* `-Zpackage-workspace` doesn't help with it.
* Existing `cargo package` has `--package <pkg>` specifying certain unpublished packages.
* Can be resolved by specifying all unpublished packages in one `cargo` call.
* `-Zpackage-workspace` also requires all dependency versions available in the target registry when calling, so doesn't help.
* `cargo package --no-verify` has been used as a kind of “plumbing commands” to create tarballs without considering dependency orders. The use cases include:
* Preparing tarballs for other package managers.
* Integrating into custom develop workflows for unpublished/internal crates.
* Constructing custom/private registries.
This commit shows the former two cases.
metdata path fields may point to a dirty symlilnk and cause
duplicate report. This commit combines `dirty_metadata_paths`
and `dirty_symlinks` into one so is able to de-duplicate them.
This adds a special case for checking source files are symlinks
and have been modified when under a VCS control
This is required because those paths may link to a file outside the
current package root, but still under the git workdir, affecting the
final packaged `.crate` file.
This may have potential performance issue. If a package contains
thousands of symlinks, Cargo will fire `git status` for each of them.
`cargo package` will warn users when git `core.symlinks` is `false`
and some symlinks were checked out as plain files during packaging.
Git config [`core.symlinks`] defaults to true when unset.
In git-for-windows (and git as well),
the config should be set to false explicitly when the repo was created,
if symlink support wasn't detected [^1].
We assume the config was always set at creation time and never changed.
So, if it is true, we don't bother users with any warning.
[^1]: <f1241afcc7/setup.c (L2394-L2403)>
[`core.symlinks`]: https://git-scm.com/docs/git-config#Documentation/git-config.txt-coresymlinks
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.
