Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wait between releases of workspace crates/Retry on failure #224

Closed
sebschmi opened this issue Aug 4, 2020 · 46 comments · Fixed by frewsxcv/rust-crates-index#53, #322 or #342
Closed

Wait between releases of workspace crates/Retry on failure #224

sebschmi opened this issue Aug 4, 2020 · 46 comments · Fixed by frewsxcv/rust-crates-index#53, #322 or #342
Milestone

Comments

@sebschmi
Copy link
Contributor

sebschmi commented Aug 4, 2020

I just released a workspace, and cargo release failed to verify a tarball. The tarball was valid though, it was just that one of its dependencies was uploaded to crates.io only seconds before, which was apparently not enough for crates.io to process the dependency. Running publish on the same crate again after a few more seconds worked totally fine.

The same effect occurred when doing a release completely manually without cargo release. Again, waiting a few seconds between releasing the two crates that depend on each other fixed the problem.

Therefore I propose to not abort if a tarball fails to verify, but instead to wait a few seconds and try again.

@epage
Copy link
Collaborator

epage commented Aug 4, 2020

Could you clarify what you mean by "tarball failed to verify"? There isn't a step related to that in cargo-release.

There is an issue where crates.io asynchronously publishes crates, so you can't cargo publish two crates in a row but instead need to ensure the first crate is fully published. To workaround this, we poll crates.io until the crate is published or 300 seconds have passed.

@sebschmi
Copy link
Contributor Author

sebschmi commented Aug 5, 2020 via email

@nickbabcock
Copy link
Contributor

nickbabcock commented Aug 7, 2020

I'm trying to work through this issue as well. The crate in question.

command:

cargo release patch --workspace

output (fails with "error: failed to prepare local package for uploading")

Release
  jomini_derive 0.1.2
  jomini 0.1.2
? [y/N]
y                                                                                                                                                                                                                [2020-08-06T23:45:11Z INFO ] Update jomini_derive to version 0.1.2                                                                                                                                               [master 3f13cbf] Release 0.1.2
 1 file changed, 1 insertion(+), 1 deletion(-)
[2020-08-06T23:45:11Z INFO ] Update jomini to version 0.1.2
[master 00e7c0a] Release 0.1.2
 1 file changed, 1 insertion(+), 1 deletion(-)
[2020-08-06T23:45:11Z INFO ] Running cargo publish on jomini_derive
    Updating crates.io index                                                                                                                                                                                        Packaging jomini_derive v0.1.2 (/home/nick/projects/jomini/jomini_derive)
   Verifying jomini_derive v0.1.2 (/home/nick/projects/jomini/jomini_derive)
   Compiling proc-macro2 v1.0.19
   Compiling unicode-xid v0.2.1
   Compiling syn v1.0.38
   Compiling quote v1.0.7                                                                                                                                                                                           Compiling jomini_derive v0.1.2 (/home/nick/projects/jomini/target/package/jomini_derive-0.1.2)
    Finished dev [unoptimized + debuginfo] target(s) in 7.93s
   Uploading jomini_derive v0.1.2 (/home/nick/projects/jomini/jomini_derive)
[2020-08-06T23:45:24Z INFO ] Waiting for publish to complete...
[2020-08-06T23:45:26Z INFO ] Running cargo publish on jomini
    Updating crates.io index
   Packaging jomini v0.1.2 (/home/nick/projects/jomini)
error: failed to prepare local package for uploading

Caused by:
  failed to select a version for the requirement `jomini_derive = "^0.1.2-pre"`                                                                                                                                    candidate versions found which didn't match: 0.1.1, 0.1.0                                                                                                                                                        location searched: crates.io index
required by package `jomini v0.1.2 (/home/nick/projects/jomini)`

This is release.toml

tag-message = "Release {{version}}"
dev-version-ext = "pre"
pre-release-commit-message = "Release {{version}}"
post-release-commit-message = "Bump to {{next_version}}"

And this is how the dependent crate is referenced in Cargo.toml

# ...

[workspace]
members = ["jomini_derive"]

[dependencies]
serde = { version = "1", optional = true }
jomini_derive = { path = "jomini_derive", version = "^0.1.2-pre", optional = true }

[features]
default = ["derive"]
derive = ["serde", "jomini_derive"]

# ...

I'll attempt to use an (undocumented?) config field that I spotted in sv-parser and see if that works next:

dependent-version          = "upgrade"

Whenever I receive the "error: failed to prepare local package for uploading" failure, I execute the following to finish up.

cargo release --prev-tag-name 0.1.2-pre

It all works, just seems like there's now a couple steps instead of one :D

@clux
Copy link
Contributor

clux commented Sep 10, 2020

Having the same issue on kube-rs's release process pretty much every time i try to do a release this way:

Release
  kube-derive 0.42.0
  kube 0.42.0
  kube-runtime 0.42.0
? [y/N] 
y
[2020-09-10T07:57:16Z INFO ] Update kube-derive to version 0.42.0
[2020-09-10T07:57:16Z INFO ] Fixing kube's dependency on kube-derive to `^0.42.0` (from `^0.41.0`)
[2020-09-10T07:57:16Z INFO ] Fixing kube-runtime's dependency on kube-derive to `^0.42.0` (from `^0.41.0`)
[master 369aee0c] (cargo-release) version 0.42.0
 3 files changed, 3 insertions(+), 3 deletions(-)
[2020-09-10T07:57:16Z INFO ] Update kube to version 0.42.0
[2020-09-10T07:57:16Z INFO ] Fixing examples's dependency on kube to `^0.42.0` (from `^0.41.0`)
[2020-09-10T07:57:16Z INFO ] Fixing tests's dependency on kube to `^0.42.0` (from `^0.41.0`)
[2020-09-10T07:57:16Z INFO ] Fixing kube-runtime's dependency on kube to `^0.42.0` (from `^0.41.0`)
[master 74f58f54] (cargo-release) version 0.42.0
 4 files changed, 4 insertions(+), 4 deletions(-)
[2020-09-10T07:57:16Z INFO ] Update kube-runtime to version 0.42.0
[2020-09-10T07:57:16Z INFO ] Fixing examples's dependency on kube-runtime to `^0.42.0` (from `^0.41.0`)
[master 8772b2e9] (cargo-release) version 0.42.0
 2 files changed, 2 insertions(+), 2 deletions(-)
[2020-09-10T07:57:16Z INFO ] Running cargo publish on kube-derive
    Updating crates.io index
   Packaging kube-derive v0.42.0 (/home/clux/repos/kube-rs/kube-derive)
   Verifying kube-derive v0.42.0 (/home/clux/repos/kube-rs/kube-derive)
   Compiling proc-macro2 v1.0.21
   Compiling unicode-xid v0.2.1
   Compiling syn v1.0.40
   Compiling memchr v2.3.3
   Compiling serde_derive v1.0.115
   Compiling serde v1.0.115
   Compiling lazy_static v1.4.0
   Compiling ryu v1.0.5
   Compiling regex-syntax v0.6.18
   Compiling serde_json v1.0.57
   Compiling itoa v0.4.6
   Compiling thread_local v1.0.1
   Compiling aho-corasick v0.7.13
   Compiling quote v1.0.7
   Compiling regex v1.3.9
   Compiling Inflector v0.11.4
   Compiling kube-derive v0.42.0 (/home/clux/repos/kube-rs/target/package/kube-derive-0.42.0)
    Finished dev [unoptimized + debuginfo] target(s) in 15.02s
   Uploading kube-derive v0.42.0 (/home/clux/repos/kube-rs/kube-derive)
[2020-09-10T07:57:34Z INFO ] Waiting for publish to complete...
[2020-09-10T07:57:40Z INFO ] Running cargo publish on kube
    Updating crates.io index
   Packaging kube v0.42.0 (/home/clux/repos/kube-rs/kube)
   Verifying kube v0.42.0 (/home/clux/repos/kube-rs/kube)
error: failed to verify package tarball

Caused by:
  failed to select a version for the requirement `kube-derive = "^0.42.0"`
  candidate versions found which didn't match: 0.40.0, 0.39.0, 0.38.0, ...
  location searched: crates.io index
  required by package `kube v0.42.0 (/home/clux/repos/kube-rs/target/package/kube-0.42.0)`

usually this is fixed by me going into the failing subdirectory and running cargo publish manually in that dir.

The problem can actually be seen manually if you just cargo publish fast enough locally, so this isn't a problem with cargo-release (just something that would be nice to account for with a wait/retry).

example: here after having just published kube, and trying to publish the dependent kube-runtime:

> repos > kube-rs > kube-runtime $ cargo publish
    Updating crates.io index
   Packaging kube-runtime v0.42.0 (/home/clux/repos/kube-rs/kube-runtime)
   Verifying kube-runtime v0.42.0 (/home/clux/repos/kube-rs/kube-runtime)
error: failed to verify package tarball

Caused by:
  failed to select a version for the requirement `kube = "^0.42.0"`
  candidate versions found which didn't match: 0.40.0, 0.39.0, 0.38.0, ...
  location searched: crates.io index
  required by package `kube-runtime v0.42.0 (/home/clux/repos/kube-rs/target/package/kube-runtime-0.42.0)`

but a few seconds later it suceeds:

> repos > kube-rs > kube-runtime $ cargo publish
    Updating crates.io index
   Packaging kube v0.42.0 (/home/clux/repos/kube-rs/kube)
   Verifying kube v0.42.0 (/home/clux/repos/kube-rs/kube)
   ...

@kpcyrd
Copy link

kpcyrd commented Nov 8, 2020

I ran into the same problem with my old tooling and had a sleep in my script because of that:

#!/bin/sh
set -xe
(cd common; cargo publish)
sleep 30 # wait for crates.io to do it's thing
(cd daemon && cargo publish)
(cd tools && cargo publish)
(cd worker && cargo publish)
git push origin master release --tags

Edit: I tried a regular cargo release and didn't have any issues, maybe this issue can be closed?

@epage
Copy link
Collaborator

epage commented Dec 26, 2020

From #247

(it's probably possible to write a more advanced solution with a curl to crates.io to check that the previous upload has completed, but I'm not even sure that's perfect if all that's happening under the hood is that the write operation (the publish) propagates to all of crates' replicas.)

In theory, that is what wait_for_publish is doing but for some reason, it isn't. Looking at people's logs in #224, we are waiting, for example

[2020-09-10T07:57:34Z INFO ] Waiting for publish to complete...
[2020-09-10T07:57:40Z INFO ] Running cargo publish on kube

Here we waited for 6s. Yet somehow, it wasn't good enough. Apparently, the part of the index we are checking gets updated but not everything needed to publishing. We are then racing for that to be updated.

crates.io didn't list where discussions should happen, so I went ahead and used their email link. Here was my message:

Greetings!
Not sure what the right support forum is for this, so went with the email link on the page.
`cargo-release` supports publishing an entire workspace.  When asynchronous index updates were added, we had to start polling the index for the crate to be ready for the next publish to happen.  Unfortunately, we seem to still see races.  My only guess is that the index isn't updated last or atomically with other parts of an update.
See https://github.com/sunng87/cargo-release/issues/224
Any suggestions for how we can ensure that publishing a dependency and dependent back-to-back can work?
Ed Page

@jtgeibel
Copy link

jtgeibel commented Dec 26, 2020

Hi @epage, in general it's best to post publicly on the issue tracker to get broader feedback, unless the inquiry includes private information that can't be shared publicly. A much smaller group of people have access to see and respond to support emails.

However, I'm not really sure that this is caused by crates.io. It looks like the crates-index dependency accesses the index maintained by cargo on the local filesystem. It sounds like the publish is failing while cargo is locally building the package, not as part of the upload to crates.io. The error message includes error: failed to prepare local package for uploading which indicates that the package isn't yet being uploaded to crates.io. Assuming cargo and crates-index are accessing the same local index (based on my quick look at crates-index), I don't understand why crates-index sees an updated index, but cargo is failing to build the package locally due to a missing dependency.

I'm guessing that there is some subtlety between how cargo and crates-index view the local git repo. It looks like crates-index fetches master and then does a hard reset to FETCH_HEAD. Maybe cargo accesses the index in a slightly different way, such that it believes it's index is fresh and doesn't update it's own local ref. (I'm not really sure why crates-index does a hard reset. My understanding is that this is a bare repository, so there isn't a local working copy to update.)

My guess is that this would best be reported to cargo-index to see why their view of the index is sometimes fresher than cargo's own view of the index.

epage pushed a commit that referenced this issue Dec 26, 2020
Reduce the chance of publish failing, see #224
@epage
Copy link
Collaborator

epage commented Dec 26, 2020

@jtgeibel

However, I'm not really sure that this is caused by crates.io. It looks like the crates-index dependency accesses the index maintained by cargo on the local filesystem. It sounds like the publish is failing while cargo is locally building the package, not as part of the upload to crates.io

Yes, it is during the test-build of the package before uploading but it happens when we are publishing back-to-back. Let's step through the interaction to be clear

  • Publish A
    • Update index
    • Test build A
    • Upload A
  • Wait for index to include new A
  • Publish B (which depends on A)
    • Update index
    • Test build B Fails to find new A

The only information we have to go off of is cargo not being able to find the dependency

  failed to select a version for the requirement `jomini_derive = "^0.1.2-pre"`                                                                                                                                    candidate versions found which didn't match: 0.1.1, 0.1.0                                                                                                                                                        location searched: crates.io index
required by package `jomini v0.1.2 (/home/nick/projects/jomini)`

The git repo should be time invariant though. Me running the same operation again 5s later should have no bearing unless the remote has changed.

This leaves us in a bit of a weird case. Why would cargo say it couldn't resolve a dependency when the dependency can be found in the index? My only wild speculation I can come up with is cargo is stripping out entries from the index that it couldn't get a .crate file for and the .crate wasn't published yet even though the index was. I did a quick search through cargo but didn't dig through all of the resolve layers to see what all is influencing it.

@jtgeibel
Copy link

Running git branch -a in ~/.cargo/registry/index/github.com-1ecc6299db9ec823 shows:

  remotes/origin/HEAD
  remotes/origin/master

My guess is that whatever method crates-index is using to access the index is slightly different than how cargo reads the index. It looks like crates-index is fetching master, which I assume will update remotes/origin/master. Maybe cargo is using remotes/origin/HEAD instead.

I'm not too familiar with the cargo side of things, but I assume that cargo doesn't fetch the git repo on every single publish. My theory is that crates-index fetches the latest commit and sees that the crate is published, but that it does so in such a way that cargo doesn't see the same commit and that cargo assumes its HEAD commit is fresh enough to not require an additional fetch.

My 2nd theory is that if cargo uses remotes/origin/master, then maybe crates-index is fetching master but not updating remotes/origin/master. The code appears to do the equivalent of git reset --hard ??? (I'm not sure what exactly would go in ??? here). This operation doesn't really make sense to me on a bare repo (where there is no local working copy to update). Additionally, running git show says fatal: your current branch 'master' does not have any commits yet. This is because .git/HEAD contains ref: refs/heads/master, but there are no local branches at all in this repo at all, only remote refs. (Another interesting note, there are no branches or remotes defined at all in the .git/config file in the bare repo.)

In summary, the configuration of this local git repo is a bit weird. If it is one of the cases above, then I think crates-index needs updated to match the logic in cargo just a bit more closely to ensure it is leaving the expected branch pointing to the newest commit.

Given a fresh index commit, dependency resolution in cargo should be completely offline. I'm fairly certain cargo will only attempt to download .crate files once it has resolved an appropriate version meeting the semver requirements.

@kpcyrd
Copy link

kpcyrd commented Dec 27, 2020

I just ran into this problem too, and it seems there's no way to recover without finishing the release manually after that happened and some crates are already published while others aren't. Maybe a hard-coded sleep could be used as a workaround until a proper solution is found?

[...]
   Compiling rebuilderd-common v0.9.1 (/<redacted>/rebuilderd/target/package/rebuilderd-common-0.9.1)
    Finished dev [unoptimized + debuginfo] target(s) in 1m 51s
   Uploading rebuilderd-common v0.9.1 (/<redacted>/rebuilderd/common)
[2020-12-27T16:40:08Z INFO ] Waiting for publish to complete...
[2020-12-27T16:40:12Z INFO ] Running cargo publish on rebuilderd
    Updating crates.io index
   Packaging rebuilderd v0.9.1 (/<redacted>/rebuilderd/daemon)
error: failed to prepare local package for uploading

Caused by:
  failed to select a version for the requirement `rebuilderd-common = "=0.9.1"`
  candidate versions found which didn't match: 0.9.0, 0.8.0, 0.7.0, ...
  location searched: crates.io index
  required by package `rebuilderd v0.9.1 (/<redacted>/rebuilderd/daemon)`
% cargo release 0.9.1
Release
  rebuilderd-common 0.9.1
  rebuilderd 0.9.1
  rebuilderd-tests 0.9.1
  rebuildctl 0.9.1
  rebuilderd-worker 0.9.1
? [y/N] 
y
[2020-12-27T16:40:21Z INFO ] Running cargo publish on rebuilderd-common
    Updating crates.io index
   Packaging rebuilderd-common v0.9.1 (/<redacted>/rebuilderd/common)
   Verifying rebuilderd-common v0.9.1 (/<redacted>/rebuilderd/common)
   Compiling autocfg v1.0.1
   Compiling libc v0.2.81
   Compiling proc-macro2 v1.0.24
[...]
   Compiling reqwest v0.10.10
   Compiling rebuilderd-common v0.9.1 (/<redacted>/rebuilderd/target/package/rebuilderd-common-0.9.1)
    Finished dev [unoptimized + debuginfo] target(s) in 1m 50s
   Uploading rebuilderd-common v0.9.1 (/<redacted>/rebuilderd/common)
error: api errors (status 200 OK): crate version `0.9.1` is already uploaded
%
% cargo release --version
cargo-release-release 0.13.8
%

@epage
Copy link
Collaborator

epage commented Dec 28, 2020

I just ran into this problem too, and it seems there's no way to recover without finishing the release manually after that happened and some crates are already published while others aren't. Maybe a hard-coded sleep could be used as a workaround until a proper solution is found?

We just merged one, see #247 . Hasn't been released yet though.

@sunng87
Copy link
Collaborator

sunng87 commented Dec 28, 2020 via email

@clux
Copy link
Contributor

clux commented Jan 2, 2021

Tested it today with 3 coupled crate releases (same cmd as my report earlier), and it still failed 😭

Resumed manually and did a couple of quick up-enter retries on cargo publish in my terminal and found that it doesn't always work the second try either, managed to retry 5 times before it was picked up (around 12-14s after publish). So a) it doesn't look like it's an issue with the local cache, and b) my hard-coded timeout was too short :(

EDIT: Had another go today with the same 3 crates 4 days later, and it managed all three! So the sleep must be making it slightly better at least.

@MathiasKoch
Copy link

I am seeing this exact issue with my workspace of three inter-dependant crates as well.
It's a bit sad, as it makes an otherwise awesome crate kinda useless for these workspace setups, which is where i personally think this crate would provide the highest benefit of all places.

Is there anything we can do? Perhaps we can query the crates.io api until the new version of the crates are live, rather than just arbitrary sleep?

jtgeibel added a commit to jtgeibel/rust-crates-index that referenced this issue Mar 2, 2021
In cargo, `HEAD` is updated instead of `master`. This mismatch means
that the two refs can drift, depending on when cargo and this library
last fetched changes.

I believe this change will fix crate-ci/cargo-release#224.

https://github.com/rust-lang/cargo/blob/0b2059e9811caa238a603f0300d9c5fb485000a6/src/cargo/sources/registry/remote.rs#L56
@jtgeibel
Copy link

jtgeibel commented Mar 2, 2021

I've opened PR frewsxcv/rust-crates-index#53 which I believe will resolve this. If you've been running into this issue, it would be great if you would test a build of cargo-release based on that branch and report back.

@epage
Copy link
Collaborator

epage commented Mar 3, 2021

@MathiasKoch

Perhaps we can query the crates.io api until the new version of the crates are live, rather than just arbitrary sleep?

We already do this and for some reason it doesn't work :(

@jtgeibel

I've opened PR frewsxcv/rust-crates-index#53 which I believe will resolve this. If you've been running into this issue, it would be great if you would test a build of cargo-release based on that branch and report back.

Thanks for finding this!

I'm a bit surprised at it though. I would assume HEAD/master are "the same". Each tool will update the local repo from refs that should be the same, so the repos should update to the same things. The only difference I can think of is the remote updates master before HEAD and we are hitting a race of checking on master before HEAD is updated. I would have assumed HEAD would be updated before master but who knows. I would have also thought they got updated much faster than the time it takes for us to fetch master, update the working tree, and then run cargo publish up to the point where it fails.

@jtgeibel
Copy link

jtgeibel commented Mar 3, 2021

@epage here's what I'm seeing locally under ~/.cargo/registry/index/github.com-1ecc6299db9ec823/.git:

$ cat refs/heads/master 
44906f5939ce65e82ce7361061eab8b87097929e
$ cat refs/remotes/origin/HEAD 
ff486005fb156362806c5f8346c3b95980aaa003
$ cat refs/remotes/origin/master 
dcc34372567a4f0ef08a8f244415898b06f40848

These commits have a range of timestamps as well: Tue Mar 2 23:26:05 2021 +0000, Tue Mar 2 23:00:24 2021 +0000, Sun Feb 28 00:30:36 2021 +0000. So these branches aren't kept in sync if they are fetched separately. I think that git fetch on the command line will fetch all references from a remote, but the code in cargo and crates-index fetch only individual refs.

@MathiasKoch
Copy link

@jtgeibel
I just tried a patched cargo-release, with the newly released crates-index v0.16.3, that i think should include your PR?

Same result:

Release
  serde_at 0.9.0
  atat_derive 0.9.0
  atat 0.9.0
? [y/N] 
y
[2021-03-04T07:40:36Z INFO ] Update serde_at to version 0.9.0
[2021-03-04T07:40:36Z INFO ] Fixing atat's dependency on serde_at to `^0.9.0` (from `^0.8.4`)
[2021-03-04T07:40:36Z INFO ] Fixing atat_derive's dependency on serde_at to `^0.9.0` (from `^0.8.4`)
[master 07d1875] (cargo-release) version 0.9.0
 3 files changed, 3 insertions(+), 3 deletions(-)
[2021-03-04T07:40:36Z INFO ] Update atat_derive to version 0.9.0
[2021-03-04T07:40:36Z INFO ] Fixing atat's dependency on atat_derive to `^0.9.0` (from `^0.8.4`)
[master 4f1d8ee] (cargo-release) version 0.9.0
 2 files changed, 2 insertions(+), 2 deletions(-)
[2021-03-04T07:40:36Z INFO ] Update atat to version 0.9.0
[master 390cd17] (cargo-release) version 0.9.0
 1 file changed, 1 insertion(+), 1 deletion(-)
[2021-03-04T07:40:36Z INFO ] Running cargo publish on serde_at
    Updating crates.io index
   Packaging serde_at v0.9.0 (/home/mathias/git/blackbird/atat/serde_at)
   Verifying serde_at v0.9.0 (/home/mathias/git/blackbird/atat/serde_at)
   Compiling typenum v1.12.0
   Compiling version_check v0.9.2
   Compiling serde v1.0.123
   Compiling byteorder v1.4.2
   Compiling heapless v0.5.6
   Compiling stable_deref_trait v1.2.0
   Compiling hash32 v0.1.1
   Compiling generic-array v0.14.4
   Compiling generic-array v0.13.3
   Compiling generic-array v0.12.4
   Compiling as-slice v0.1.5
   Compiling serde_at v0.9.0 (/home/mathias/git/blackbird/atat/target/package/serde_at-0.9.0)
    Finished dev [unoptimized + debuginfo] target(s) in 2.77s
   Uploading serde_at v0.9.0 (/home/mathias/git/blackbird/atat/serde_at)
[2021-03-04T07:40:52Z INFO ] Running cargo publish on atat_derive
    Updating crates.io index
   Packaging atat_derive v0.9.0 (/home/mathias/git/blackbird/atat/atat_derive)
   Verifying atat_derive v0.9.0 (/home/mathias/git/blackbird/atat/atat_derive)
  Downloaded serde_at v0.9.0
  Downloaded 1 crate (10.4 KB) in 6.40s
   Compiling typenum v1.12.0
   Compiling version_check v0.9.2
   Compiling serde v1.0.123
   Compiling proc-macro2 v1.0.24
   Compiling heapless v0.5.6
   Compiling stable_deref_trait v1.2.0
   Compiling unicode-xid v0.2.1
   Compiling byteorder v1.4.2
   Compiling syn v1.0.60
   Compiling generic-array v0.14.4
   Compiling hash32 v0.1.1
   Compiling quote v1.0.9
   Compiling generic-array v0.13.3
   Compiling generic-array v0.12.4
   Compiling as-slice v0.1.5
   Compiling serde_at v0.9.0
   Compiling atat_derive v0.9.0 (/home/mathias/git/blackbird/atat/target/package/atat_derive-0.9.0)
    Finished dev [unoptimized + debuginfo] target(s) in 10.63s
   Uploading atat_derive v0.9.0 (/home/mathias/git/blackbird/atat/atat_derive)
[2021-03-04T07:41:05Z INFO ] Waiting for publish to complete...
[2021-03-04T07:41:10Z INFO ] Running cargo publish on atat
    Updating crates.io index
   Packaging atat v0.9.0 (/home/mathias/git/blackbird/atat/atat)
error: failed to prepare local package for uploading

Caused by:
  failed to select a version for the requirement `atat_derive = "^0.9.0"`
  candidate versions found which didn't match: 0.8.4, 0.7.1, 0.7.0, ...
  location searched: crates.io index
  required by package `atat v0.9.0 (/home/mathias/git/blackbird/atat/atat)`

@Byron
Copy link

Byron commented Aug 12, 2021

I am experimenting with a release tool which is able to handle workspace releases in the way I would imagine it. Thus far it seems to work according to my expectations:

  • given a one or more workspace members for release, bump their versions and release them in the correct order
  • fix changed version numbers in affected workspace member manifests
  • handle cyclic dependencies and create 'stable' releases nonetheless
  • handle crate-index being out of date by retrying the publish (primitive, I know) - more experimentation is needed to know how well all this works.
  • run in dry-run mode by default unless --execute is specified

As I will keep using it for gitoxide I am sure that one day it will do its job admirably, which is also when I would be glad to turn it into a cargo subcommand as well.

@epage
Copy link
Collaborator

epage commented Aug 12, 2021

Is there a reason you re-implemented rather than contributing the improvements to cargo-release?

@pksunkara
Copy link

  • given a one or more workspace members for release, bump their versions and release them in the correct order
  • fix changed version numbers in affected workspace member manifests
  • handle cyclic dependencies and create 'stable' releases nonetheless
  • handle crate-index being out of date by retrying the publish (primitive, I know) - more experimentation is needed to know how well all this works.

All of this is exactly what cargo-workspaces does. It is almost a rewrite of lerna which is popular and battle tested workspace releaser.

@epage
Copy link
Collaborator

epage commented Aug 12, 2021

To be clear, cargo-release covers

  • given a one or more workspace members for release, bump their versions and release them in the correct order
  • fix changed version numbers in affected workspace member manifests

And has remaining:

  • handle cyclic dependencies and create 'stable' releases nonetheless
    • This is the first time someone has brought this up
    • @pksunkara what approach are you taking for cycles?
  • handle crate-index being out of date by retrying the publish (primitive, I know) - more experimentation is needed to know how well all this works.
    • We usually handle this well but there seems to be some kind of race that causes things to fail.
    • @pksunkara looks like our waiting logic is the same exact you use BareIndex while we use Index. Is there a reason you went with BareIndex? Have you seen a failure where the index said you were good but the next publish still failed? To be clear, its rare, so the absence of it doesn't mean success.

@pksunkara
Copy link

handle cyclic dependencies and create 'stable' releases nonetheless

Actually my bad. I didn't read the word cyclic correctly. Imagine foo and bar depending on each other, I am not sure this can be done in single command. If you are using pure cargo, you bump foo first, release it and then bump bar and release it. Using the same methodology, you would need to run two commands in cargo-workspaces or cargo-release.

looks like our waiting logic is the same exact you use BareIndex while we use Index. Is there a reason you went with BareIndex? Have you seen a failure where the index said you were good but the next publish still failed? To be clear, its rare, so the absence of it doesn't mean success.

BareIndex always had the correct implementation until frewsxcv/rust-crates-index#53 landed.

@pksunkara
Copy link

what approach are you taking for cycles?

Looking at #288, did you actually mean DAG instead of a cycle? Because clap doesn't have a dep cycle.

@epage
Copy link
Collaborator

epage commented Aug 12, 2021

Looking at #288, did you actually mean DAG instead of a cycle? Because clap doesn't have a dep cycle.

clap depends on clap_derive and clap_derive dev-depends on clap.

@pksunkara
Copy link

pksunkara commented Aug 12, 2021

Ah, dev-dependencies does not need a version specified, they can be pure path dependencies. And release management only cares about the dependency line if it has a version. So, from cargo-workspaces perspective, clap_derive does not depend on clap since it does not specify a version and this constraint does not needed to be taken into account, thus making it a DAG.

@epage
Copy link
Collaborator

epage commented Aug 12, 2021

If frewsxcv/rust-crates-index#53 is what was blocking this issue being resolved, it looks like that was released in 0.16.3. It looks like we didn't upgrade past 0.16.2 until 7fcbd72 which was released as, coincidentally cargo-release 0.16.3 on July 31st.

Hopefully that means this issue is completely resolved. I am a bit hesitant to close this and remove the sleep because we kept thinking this was resolved before.

@Byron
Copy link

Byron commented Aug 13, 2021

Please try it out: https://crates.io/crates/cargo-smart-release

I have bootstrapped it by using itself to create the first release. Interestingly it did fail to publish the first time around due to old/invalid versions of dependent crates being used.

However, it retries the publish up to three times and the second time around it worked.

There is definitely something weird going on. Investigating.

@epage
Copy link
Collaborator

epage commented Aug 13, 2021

@Byron If you have any thoughts on splitting out the version editing code into a crate for both of us to reuse, let's play with that! I eventually want a bare bones version of that also in cargo-edit so I can bump major on a breaking change before the next release, while its still top of mind.

If crates-index is fully working now, I'd recommend that over retry, or to reduce the chance of needing to retry.

We are starting talks (see #298) on release-only-changed. How do you handle or plan to handle a changed lock file? In theory, a changed lock file could mean nothing needs to release or everything needs to release.

How do you plan to handle breaking-change detection? I've been considering adding support for conventional-commit for this, though it depends on how well you can detect which crate the breaking change is intended for if multiple were touched by the commit.

Be sure to check out clap-cargo to have a more cargo-like CLI.

@Byron
Copy link

Byron commented Aug 15, 2021

Let me start by thanking you for moving this forward and fostering collaboration! This will without any doubt lead to the 'release' issue to be solved once and for all, with all tools being equally strong while catering to different schools (and I am saying that without ever having the chance to try cargo workspaces. Maybe one day we can even enjoy a more capable cargo publish or cargo release being bundled with cargo.

@Byron If you have any thoughts on splitting out the version editing code into a crate for both of us to reuse, let's play with that! I eventually want a bare bones version of that also in cargo-edit so I can bump major on a breaking change before the next release, while its still top of mind.

This sounds like a good idea and I would love to use such a crate in cargo-smart-release.

If crates-index is fully working now, I'd recommend that over retry, or to reduce the chance of needing to retry.

With crates-index I am having trust issues, it's a fuzzy feeling that may be entirely invalid, maybe it's cargo itself that has some strange issues right after having updated the index. I think it's best to have cargo-smart-release use cargo publish only and see if the issue stems from cargo itself without another crate fiddling with the index beforehand. My hunch is that cargo itself updates the index but then has issues with crate resolution somehow, but I need more experience to know for sure.

We are starting talks (see #298) on release-only-changed. How do you handle or plan to handle a changed lock file? In theory, a changed lock file could mean nothing needs to release or everything needs to release.

A great point, I didn't think of that (or build scripts that alter what's compiled, etc, environment variables, its a nightmare ;)). For now cargo-smart-release doesn't handle it but I am now subscribed to the issue and would certainly implement at least a way to override its 'dirty' checking. For now, one can't even do that and it won't release a crate if it thinks it didn't change.

How do you plan to handle breaking-change detection? I've been considering adding support for conventional-commit for this, though it depends on how well you can detect which crate the breaking change is intended for if multiple were touched by the commit.

A rough thought was to use git workspaces to quickly create a sandbox in which one can reset a workspace crate the depends on the (workspace) crate to be published to the previous release and see if it still compiles with the latest version of the crate to be published. If there are no workspace crates depending on the publishee then one could probably find crates that do on crates-io. In short, compiling actual code to see if there's breakage. This could be a time-consuming process but I would consider it the exception as well. Most of the time authors will know if they caused breaking changes.
My guess is that I will never actually implement it as my current workflow works well enough (i.e. bump the version after a breaking change, worst case one bumps too often and leaves out a version number).

Be sure to check out clap-cargo to have a more cargo-like CLI.

That looks great, and I'd probably use it right away if I wouldn't use argh for this one.

@Byron
Copy link

Byron commented Aug 16, 2021

Regarding breaking changes, I just came across the aptly named crate cargo breaking, which has the potential to be the smart and fast way of determining breaking changes. I didn't try it yet, but will try it soon.

@epage epage added this to the 1.0 milestone Aug 23, 2021
epage added a commit to epage/cargo-release that referenced this issue Aug 25, 2021
We are assuming crate-ci#224 is now resolved, so removing the sleep.  In case
people run into problems, they can always set the env var to re-enable
it.

Closes crate-ci#224
@clux
Copy link
Contributor

clux commented Oct 9, 2021

Tried this out today using cargo-release at 0.18.0 on kube-rs without the workaround to see if it's truly fixed. Unfortunately, I still hit the bug when not setting PUBLISH_GRACE_SLEEP=20 so possibly still worth recommending that people set it.

full build log

cargo release minor --execute
Release
  kube-core 0.61.0
  kube-derive 0.61.0
  kube 0.61.0
  kube-runtime 0.61.0
? [y/N] 
y
[2021-10-09T21:20:21Z INFO  cargo_release] Update kube-core to version 0.61.0
[2021-10-09T21:20:21Z INFO  cargo_release] Fixing examples's dependency on kube-core to `^0.61.0` (from `^0.60.0`)
[2021-10-09T21:20:21Z INFO  cargo_release] Fixing kube's dependency on kube-core to `^0.61.0` (from `^0.60.0`)
[2021-10-09T21:20:21Z INFO  cargo_release] Fixing kube-runtime's dependency on kube-core to `^0.61.0` (from `^0.60.0`)
prerelease: kube-core bumping docs from 0.60.0 -> 0.61.0
[2021-10-09T21:20:21Z INFO  cargo_release] Update kube-derive to version 0.61.0
[2021-10-09T21:20:21Z INFO  cargo_release] Fixing examples's dependency on kube-derive to `^0.61.0` (from `^0.60.0`)
[2021-10-09T21:20:21Z INFO  cargo_release] Fixing kube's dependency on kube-derive to `^0.61.0` (from `^0.60.0`)
[2021-10-09T21:20:21Z INFO  cargo_release] Fixing kube-runtime's dependency on kube-derive to `^0.61.0` (from `^0.60.0`)
prerelease: kube-derive nothing to do
[2021-10-09T21:20:21Z INFO  cargo_release] Update kube to version 0.61.0
[2021-10-09T21:20:21Z INFO  cargo_release] Fixing examples's dependency on kube to `^0.61.0` (from `^0.60.0`)
[2021-10-09T21:20:21Z INFO  cargo_release] Fixing kube-runtime's dependency on kube to `^0.61.0` (from `^0.60.0`)
[2021-10-09T21:20:21Z INFO  cargo_release] Fixing tests's dependency on kube to `^0.61.0` (from `^0.60.0`)
prerelease: kube nothing to do
[2021-10-09T21:20:21Z INFO  cargo_release] Update kube-runtime to version 0.61.0
[2021-10-09T21:20:21Z INFO  cargo_release] Fixing examples's dependency on kube-runtime to `^0.61.0` (from `^0.60.0`)
prerelease: kube-runtime nothing to do
[master ccbe1cfb] release 0.61.0
 9 files changed, 27 insertions(+), 24 deletions(-)
[2021-10-09T21:20:21Z INFO  cargo_release] Running cargo publish on kube-core
    Updating crates.io index
   Packaging kube-core v0.61.0 (~/repos/kube-rs/kube-core)
   Verifying kube-core v0.61.0 (~/repos/kube-rs/kube-core)
   Compiling autocfg v1.0.1
   Compiling proc-macro2 v1.0.29
   Compiling unicode-xid v0.2.2
   Compiling syn v1.0.80
   Compiling serde_derive v1.0.130
   Compiling serde v1.0.130
   Compiling libc v0.2.103
   Compiling ryu v1.0.5
   Compiling itoa v0.4.8
   Compiling hashbrown v0.11.2
   Compiling serde_json v1.0.68
   Compiling k8s-openapi v0.13.1
   Compiling bytes v1.1.0
   Compiling percent-encoding v2.1.0
   Compiling base64 v0.13.0
   Compiling matches v0.1.9
   Compiling fnv v1.0.7
   Compiling once_cell v1.8.0
   Compiling num-traits v0.2.14
   Compiling num-integer v0.1.44
   Compiling indexmap v1.7.0
   Compiling form_urlencoded v1.0.1
   Compiling http v0.2.5
   Compiling quote v1.0.10
   Compiling time v0.1.44
   Compiling ordered-float v2.8.0
   Compiling thiserror-impl v1.0.30
   Compiling thiserror v1.0.30
   Compiling serde-value v0.7.0
   Compiling chrono v0.4.19
   Compiling kube-core v0.61.0 (~/repos/kube-rs/target/package/kube-core-0.61.0)
    Finished dev [unoptimized + debuginfo] target(s) in 22.31s
   Uploading kube-core v0.61.0 (~/repos/kube-rs/kube-core)
[2021-10-09T21:20:53Z INFO  cargo_release::cargo] Waiting for publish to complete...
[2021-10-09T21:20:55Z INFO  cargo_release] Running cargo publish on kube-derive
    Updating crates.io index
   Packaging kube-derive v0.61.0 (~/repos/kube-rs/kube-derive)
   Verifying kube-derive v0.61.0 (~/repos/kube-rs/kube-derive)
   Compiling proc-macro2 v1.0.29
   Compiling unicode-xid v0.2.2
   Compiling syn v1.0.80
   Compiling serde_derive v1.0.130
   Compiling strsim v0.10.0
   Compiling fnv v1.0.7
   Compiling ident_case v1.0.1
   Compiling serde v1.0.130
   Compiling ryu v1.0.5
   Compiling serde_json v1.0.68
   Compiling itoa v0.4.8
   Compiling quote v1.0.10
   Compiling darling_core v0.13.0
   Compiling darling_macro v0.13.0
   Compiling darling v0.13.0
   Compiling kube-derive v0.61.0 (~/repos/kube-rs/target/package/kube-derive-0.61.0)
    Finished dev [unoptimized + debuginfo] target(s) in 13.39s
   Uploading kube-derive v0.61.0 (~/repos/kube-rs/kube-derive)
[2021-10-09T21:21:10Z INFO  cargo_release::cargo] Waiting for publish to complete...
[2021-10-09T21:21:14Z INFO  cargo_release] Running cargo publish on kube
    Updating crates.io index
   Packaging kube v0.61.0 (~/repos/kube-rs/kube)
   Verifying kube v0.61.0 (~/repos/kube-rs/kube)
error: failed to verify package tarball

Caused by:
  failed to select a version for the requirement `kube-derive = "^0.61.0"`
  candidate versions found which didn't match: 0.60.0, 0.59.0, 0.58.1, ...
  location searched: crates.io index
  required by package `kube v0.61.0 (~/repos/kube-rs/target/package/kube-0.61.0)`

EDIT: added full logs.

@pksunkara
Copy link

It should have definitely been fixed. What's your local cargo version?

@clux
Copy link
Contributor

clux commented Oct 9, 2021

cargo --version gives cargo 1.55.0 (32da73ab1 2021-08-23)

@pksunkara
Copy link

I just double checked the logic in cargo and crates-index (which is used in this crate). Looks the same as before and everything is correct.

I would wait for @epage to respond with some workarounds. If it is still not working, you could try https://github.com/pksunkara/cargo-workspaces

@clux
Copy link
Contributor

clux commented Oct 9, 2021

Thanks. It's ultimately not a problem due to the evar workaround, just thought I'd share that we still hit it.

@epage
Copy link
Collaborator

epage commented Oct 9, 2021

After reading through cargo's code, I no longer trust crates-index :). On the positive side, next week I plan to extract the logic from cargo so it can be reused.

You can export PUBLISH_GRACE_SLEEP=5 to sleep for 5s between calls.

@pksunkara
Copy link

After reading through cargo's code, I no longer trust crates-index

Do you have any examples? I remember doing this and crates-index was pretty good except for the bug I found. I would ask you to try going with the BareIndex first though (this is actually what cargo does) before doing what you planned.

@epage
Copy link
Collaborator

epage commented Oct 10, 2021

sigh I thought I had already switched to bare-index, that was my whole reason for removing the sleep.

As for "why", its not because of this but problems we ran into with cargo-edit which has much broader exposure to use cases, like frewsxcv/rust-crates-index#65. In general, I would rather we work with the cargo team on this so we can stay up to date with them, rather than chasing them. This splitting out would be in coordination with them.

epage added a commit to epage/cargo-release that referenced this issue Oct 10, 2021
I meant to implement this before reovign the sleep but it fell through
the cracks.

Fixes crate-ci#224
@epage
Copy link
Collaborator

epage commented Oct 10, 2021

0.18.1 is released using BareIndex, which we hope fixes this.

@Byron
Copy link

Byron commented Oct 10, 2021

sigh I thought I had already switched to bare-index, that was my whole reason for removing the sleep.

What does working on the bare index actually mean? Does it mean it won't affect the work tree/checkout? The reason I am asking is that I noticed that cargo itself may fail a few times to see newly published crates even after apparently trying to fetch changes beforehand. As using a non-bare crates index does all the bare version does, it should make no difference either way.

cargo definitely has some logic to help deciding when to expensively update the crates index, and maybe encountering a crate version that is referred to but doesn't exist in the index is one of them. As such an update will be triggered the next time cargo runs, it might set some flag or clear a flag to know what to do next time it is invoked. As sometimes having cargo update the crates index still won't show the newly published crate, and when disabling paranoia towards git2, that must mean the newly published crate version isn't yet in the index. I vaguely seem to remember that the crates index does have a queue for creating commits in the index, maybe that sometimes is a bit slow to process to cause many seconds of delay.

Lastly, cargo smart-release brutally retries to run cargo publish up to three times, and sometimes it does take until the last attempt for a publish to go through as the desired dependent crate version took a while to appear.

With that in mind, it should be possible to use crates-index to update the crates index repeatedly until the previously published crate versions are present, which is an improvement over brute-forcing it, waiting a fixed amount of time, or over updating the index just once (as it doesn't seem to work).

@epage
Copy link
Collaborator

epage commented Oct 10, 2021

The reason I am asking is that I noticed that cargo itself may fail a few times to see newly published crates even after apparently trying to fetch changes beforehand.

Depending on what you were seeing, this might be because cargo publish was switched a while ago to put crates into a queue and it only blocks until it is in the queue. In theory, once it is in the index repo, it should be fully accessible.

We do loop over checking the index, with a timeout. The issue is more our index being updated but cargo still not seeing it. @pksunkara dug more into the details but there were discrepancies between how the older Index logic pulled compared to BareIndex now. In theory, a bare index should just be an optimization.

@epage
Copy link
Collaborator

epage commented Dec 23, 2021

@Byron

Regarding breaking changes, I just came across the aptly named crate cargo breaking, which has the potential to be the smart and fast way of determining breaking changes. I didn't try it yet, but will try it soon.

Thought I'd report back on how cargo-breaking worked out. I have some open issues on programmatic use but I also gave it a try on clap to try to identify missed changes to report in the CHANGELOG. Unfortunately, its pretty immature. They are effectively doing cargo expand + syn to manually re-implement rust's pub logic to determine what the API is which has problems. They are working on a rustc backend which makes install harder because the API is fairly brittle which would also be a blocker for programmatic use because you'd have to use the same nightly version in your crate. In talking with them I've created a prototype using rustdoc. rustdoc has its own issues and there is a lot more to go. Hopefully the cargo-breaking folks (and anyone else interested) will be able to help get the community something easier to use (CLI or programmatically) than semverver.

@Byron
Copy link

Byron commented Dec 24, 2021

Thanks for sharing, it's an exciting area of research, and by the looks of it, there are three approaches at least.

  • cargo breaking (using syn)
  • cargo api using rustdoc (and maybe other backends in future)
  • semverver using rmeta + LLVM, but it's probably best to read the implementation notes

I wonder what the outcome would be - automatic documentation of API changes at the very least, and maybe a more fine-grained system to determine compatibility than semver that might even affect how cargo does dependency resolution one day?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment