Skip to content

Regression in rendering of dependency version requirements #3047

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

Closed
dtolnay opened this issue Nov 27, 2020 · 2 comments · Fixed by #3049
Closed

Regression in rendering of dependency version requirements #3047

dtolnay opened this issue Nov 27, 2020 · 2 comments · Fixed by #3049
Labels
C-bug 🐞 Category: unintended, undesired behavior

Comments

@dtolnay
Copy link
Member

dtolnay commented Nov 27, 2020

Before After
(screenshot from a google image search)

To Reproduce
Steps to reproduce the behavior:

  1. Go to https://crates.io/crates/tide/0.8.1

Expected behavior
The screenshot on the left is much easier for me to read off the information that I care about. The >=A.B.C, <D.E.F ranges are just unnecessarily verbose/cluttered and disguises when there are real >=/< dependencies that are not equivalent to a ^ dependency (as in >=0.2, <0.4). 😕

Additional context
If this was not made as an intentional change, it's possible it was introduced inadvertently in the course of a dependency update such as #2990.

@dtolnay dtolnay added the C-bug 🐞 Category: unintended, undesired behavior label Nov 27, 2020
@Turbo87
Copy link
Member

Turbo87 commented Nov 27, 2020

I looked into this a little bit, and what happens is that on upload we parse the requirement string into a VersionReq and when we save it into the database we convert it back to a string. Similarly when we read it from the database we parse it into a VersionReq and then convert it back to a string when serializing as JSON.

Unfortunately, the new semver release changed their VersionReq::to_string implementation, which means that we're now converting to these ranges instead of using the ^ prefix in both cases.

From what I can tell we don't use the VersionReq for anything inside our code, so we could probably just save the original strings instead, and add a validation check in the upload route handler.

@Turbo87
Copy link
Member

Turbo87 commented Dec 3, 2020

#3051 reverted the semver upgrade for now, so I guess we can close this issue. #3049 will follow up on this by adding regression tests and saving the original version requirement in the database.

@Turbo87 Turbo87 closed this as completed Dec 3, 2020
bors added a commit that referenced this issue Dec 14, 2020
Save original dependency requirement string in the database when publishing

This PR should resolve #3047 by saving the original dependency requirement string in the database when publishing, and returning the original string too when calling the `GET /crates/:crate_id/:version/dependencies` endpoint.

Before #2990 we were turning `1.2.3` into `^1.2.3` automatically. After that PR were turning it into `>=1.2.3, <2.0.0`, and now, with this PR, we would keep it at `1.2.3` and it would be up to the API client to prefix the `^`, if necessary.

r? `@jtgeibel`

/cc `@dtolnay`
jtgeibel added a commit to jtgeibel/crates.io that referenced this issue Feb 26, 2021
This reverts commit 0bcb724.

This reverts the revert (rust-lang#3051) of rust-lang#2990. Now that rust-lang#3047 is resolved by
rust-lang#3094, it should be fine to bump this again.
jtgeibel added a commit to jtgeibel/crates.io that referenced this issue Feb 26, 2021
This reverts commit 0bcb724.

This reverts the revert (rust-lang#3051) of rust-lang#2990. Now that rust-lang#3047 is resolved by
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug 🐞 Category: unintended, undesired behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants