search

crate-ci / cargo-release

Cargo subcommand `release`: everything about releasing a rust crate.
Apache License 2.0
1.33k stars 112 forks source link

Unconditionally query crates.io HTTP index for publish status #693

Closed Jake-Shadle closed 1 year ago

Jake-Shadle commented 1 year ago

This replaces the crates-index with tame-index, which adds built-in support for sparse registries, resolving a couple of issues, as well as using gix instead of git2 which has improved functionality for authentication, resolving another issue (or at least, I believe it should).

~This unfortunately drastically increases the number of dependencies for this crate, due to gix and reqwest being pulled in, but there are a couple of options to mitigate this.~

~1. Replace the usage of git2 with gix in a follow up PR, which would remove some crates completely and not have 2 implementations of git. There is at least one missing feature in gix that I am aware of that this crate uses (dirty checking of the repository), but since this crate already uses the git cli for some operations like fetch anyways, that would be easy enough to replicate with the cli until gix gets that functionality in the future.~ ~2. Just drop support for git registry indices altogether. This may not be possible if using a custom registry that only supports the git sparse protocol, but some alternative registry sources such as cloudsmith already support the sparse protocol, so maybe not that big of an issue. Only using the crates.io sparse registry, even if the user uses an old cargo that doesn't support it, is irrelevant for this crate's usage as it is only checking for the existence of the crate, or the published version.~

Resolves: #218 Resolves: #679 Resolves: #692

Jake-Shadle commented 1 year ago

So I refactored this to still use tame-index, but without the sparse or git features enabled, in favor of doing "manual" HTTP calls to the crates.io index and zero disk I/O, eliminating the concern around cargo's global package lock, and completely sidestepping all of the issues around the git index. This massively reduces the dependencies added by this PR by no longer using gix (though I would be willing to do a follow up PR to replace this crate's usage of git2 with gix since rust > not-rust IMO).

AFAICT there are no tests in this crate around index communication, which makes sense, so I ran a quick test by just releasing one of my own crates with the changes in this PR.

   Compiling cargo-fetcher v0.14.6 (/home/jake/code/cargo-fetcher/target/package/cargo-fetcher-0.14.6)
    Finished dev [unoptimized + debuginfo] target(s) in 54.48s
    Packaged 52 files, 415.4KiB (107.8KiB compressed)
   Uploading cargo-fetcher v0.14.6 (/home/jake/code/cargo-fetcher)
    Uploaded cargo-fetcher v0.14.6 to registry `crates-io`
note: Waiting for `cargo-fetcher v0.14.6` to be available at registry `crates-io`.
You may press ctrl-c to skip waiting; the crate should be available shortly.
   Published cargo-fetcher v0.14.6 at registry `crates-io`
     Pushing Pushing 0.14.6, main to origin

Obviously you can't tell from the output but the waiting was < 2s, and most importantly, I had blown away ~/.cargo/registry/index/github.com-1ecc6299db9ec823 to ensure that a missing/outdated crates.io git index didn't affect cargo-release, which was my initial reason for opening this PR, seeing that not having a crates.io git index (which will be more and more common due it no longer being the default) caused a massive stall in cargo-release.

Jake-Shadle commented 1 year ago

So I had to bump the MSRV, as tame-index requires 1.67.0 due to gix also having a 1.67.0 MSRV, if that's an issue I can close this PR.

Jake-Shadle commented 1 year ago

Actually I lied, bumped it to 1.67.1 to get https://github.com/rust-lang/rust-clippy/pull/10265 and avoid a bunch of churn that only applies to 1.67.0.

epage commented 11 months ago

Finally giving in to going with this approach. I made some tweaks in #719.

epage commented 11 months ago

So I refactored this to still use tame-index, but without the sparse or git features enabled, in favor of doing "manual" HTTP calls to the crates.io index and zero disk I/O, eliminating the concern around cargo's global package lock, and completely sidestepping all of the issues around the git index.

In doing the upgrades (#722), it drew attention that this wasn't correct and disk IO is done, even in 0.5.1 which this PR is based on.

Jake-Shadle commented 11 months ago

In doing the upgrades (https://github.com/crate-ci/cargo-release/pull/722), it drew attention that this wasn't correct and disk IO is done, even in 0.5.1 which this PR is based on.

No, it is only done if you don't set the etag, which this PR always did.