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

Release 0.24.1 #475

Closed
tustvold opened this issue Sep 10, 2022 · 9 comments
Closed

Release 0.24.1 #475

tustvold opened this issue Sep 10, 2022 · 9 comments

Comments

@tustvold
Copy link

tustvold commented Sep 10, 2022

It would appear that 0.24.0 has just been yanked, and a new 0.25.0 release cut. Unfortunately this means released crates with a version pin of ^0.24.0 now fail to compile, as 0.25.0 is considered semver incompatible.

Reviewing the changelog for 0.25.0 I don't see anything obvious that would constitute a breaking change, and so I wonder if we might release a 0.24.1 with the change in #471 to avoid ecosystem churn? Nothing in that change appears to be breaking

FYI @Mingun

@Mingun
Copy link
Collaborator

Mingun commented Sep 10, 2022

Hm... Documentation for cargo yank stated, that crates that already uses yanked version still will able to use it. If that is not true, maybe this is a bug in cargo? It is surprise for me, that yanking breaks something. I tried to prevent using 0.23.0 and 0.24.0 in new projects.

@tustvold
Copy link
Author

tustvold commented Sep 10, 2022

Perhaps that is only if pinned to that specific version, as opposed to a semver constraint, I don't honestly know? I might file a PR to the docs repo to clarify that, and in so doing find out from the people who will know 🤔

In my experience I have frequently found yanked crates forcing me to drop everything and patch a version. It's certainly not transparent to downstreams in the way that would imply... Personally I wish crates.io didn't have the functionality, even in the case of CVEs, RUSTSEC is a better way to communicate that without forcing updates on downstreams.

For reference here is the failing CI job - https://github.com/apache/arrow-rs/runs/8286264184?check_suite_focus=true

@Mingun
Copy link
Collaborator

Mingun commented Sep 10, 2022

In your case you should update quick-xml in any case because of using quick_xml::de::from_reader, for example, here:
https://github.dev/tustvold/arrow-rs/blob/2721d253ecea84b06bc890a09a67dcb389bb7ad6/object_store/src/aws/client.rs#L388

It uses buffered reading and could break is S3 response will contain CDATA elements.

@adamreichold
Copy link
Contributor

adamreichold commented Sep 10, 2022

I think the relevant part of the Cargo documentation is

Note that existing crates locked to a yanked version will still be able to download the yanked version to use it. Cargo will, however, not allow any new crates to be locked to any yanked version.

meaning that only locked versions continue to work, meaning entries in Cargo.lock. However, resolving a yanked version from a constraint in Cargo.toml does not which is why one should generally only yank versions for which semver-compatible alternatives containing the fix exist.

@tustvold
Copy link
Author

You should update

Yes, agreed. All I'm asking is that we provide a way that people can update through semver, without needing new releases to be cut. Are you suggesting all downstreams should yank any releases using 0.24.0? Would it not be better to just cut a patch release that fixes the bug and let everyone get the fix for free?

@Mingun
Copy link
Collaborator

Mingun commented Sep 10, 2022

Because there are no significant changes yet, I thought that updating from 0.24 to 0.25 should not be a problem. It is possible to release a patch release also. Will release soon

meaning that only locked versions continue to work, meaning entries in Cargo.lock.

Maybe also it is just because last released version of object_store is 0.4.0 and Cargo.toml already specified 0.5.0 which is new version that should use new release. Maybe if there was 0.4.x, the error would not have arisen.

@tustvold
Copy link
Author

tustvold commented Sep 10, 2022

Cargo.toml already specified 0.5.0

This is because the 0.5.0 release is in its acceptance testing phase and has already been cut, this is part of why I would greatly appreciate a patch release

Maybe if there was 0.4.x, the error would not have arisen.

You can confirm this is not the case with a simple test

$ cargo new --lib foo
$ cd foo
$ echo 'quick-xml = "0.24.0"' >> Cargo.toml
$ cargo build
error: failed to select a version for the requirement `quick-xml = "^0.24.0"`
candidate versions found which didn't match: 0.25.0, 0.22.0, 0.21.0, ...
location searched: crates.io index
required by package `foo v0.1.0 (/home/raphael/foo)`

I thought that updating from 0.24 to 0.25 should not be a problem

It wouldn't be, the issue is yanking 0.24 and not providing a semver compatible replacement 😅

I've proposed rust-lang/cargo#11071 to address the documentation, as I agree it is definitely misleading

Will release soon

Thank you ❤️

@Mingun
Copy link
Collaborator

Mingun commented Sep 10, 2022

You can confirm this is not the case with a simple test

I don't think so. That crate is created locally and of course it is new. If it would be already released...


Released 0.24.1

@Mingun Mingun closed this as completed Sep 10, 2022
@tustvold
Copy link
Author

tustvold commented Sep 10, 2022

Thank you, especially for handling this so quickly 🎉

That crate is created locally and of course it is new. If it would be already released...

We can test this using cargo-tarpaulin which depends on 0.23 which has also been yanked

$ cargo new --lib foo
$ cd foo
$ echo 'cargo-tarpaulin = "0.21.0"' >> Cargo.toml
$ cargo build
error: failed to select a version for the requirement `quick-xml = "^0.23"`
candidate versions found which didn't match: 0.25.0, 0.24.1, 0.22.0, ...
location searched: crates.io index
required by package `cargo-tarpaulin v0.21.0`
    ... which satisfies dependency `cargo-tarpaulin = "^0.21.0"` of package `foo v0.1.0 (/home/raphael/foo)`

Similarly attempting to install inferno, also fails

cargo install inferno
    Updating crates.io index
  Installing inferno v0.11.7
error: failed to compile `inferno v0.11.7`, intermediate artifacts can be found at `/tmp/cargo-installUvTt2O`

Caused by:
  failed to select a version for the requirement `quick-xml = "^0.23"`
  candidate versions found which didn't match: 0.25.0, 0.24.1, 0.22.0, ...
  location searched: crates.io index
  required by package `inferno v0.11.7`

You might want to consider releasing a patch version of 0.23 as well...

bors added a commit to rust-lang/cargo that referenced this issue Mar 23, 2023
doc: clarify implications of `cargo-yank`

### What does this PR try to resolve?

I found the documentation for `cargo yank` was not especially clear on the implications of yanking a crate, and I have seen this causing confusion within the community - tafia/quick-xml#475.

On a somewhat related note, I have been observing lots more crates getting yanked recently and this is resulting in a fair amount of dependency upgrade busywork. I think/hope part of this is a documentation issue.
bors added a commit to rust-lang/cargo that referenced this issue May 3, 2023
doc: clarify implications of `cargo-yank`

### What does this PR try to resolve?

I found the documentation for `cargo yank` was not especially clear on the implications of yanking a crate, and I have seen this causing confusion within the community - tafia/quick-xml#475.

On a somewhat related note, I have been observing lots more crates getting yanked recently and this is resulting in a fair amount of dependency upgrade busywork. I think/hope part of this is a documentation issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants