Skip to content

Specifying dependencies: add exact version section - #9961

Closed
marcospb19 wants to merge 8 commits into
rust-lang:masterfrom
marcospb19:patch-1
Closed

marcospb19 wants to merge 8 commits into
rust-lang:masterfrom
marcospb19:patch-1

Conversation

@marcospb19

@marcospb19 marcospb19 commented Oct 5, 2021 •

Copy link
Copy Markdown

Specifying exact versions is extremely useful, I gave it a dedicated section separating from the "Comparison requirements" one, even tho it can be mixed up with comparison requirements.

Specify an exact version together with other comparison requirements seems to be redundant:

time = "= 0.3.3, > 0.2.0"
rand = "= 0.8.3, >= 0.8.3, < 0.8.4"

It's redundant because it can only solve to "0.3.3" and "0.8.3".

Questions:

  1. Is it ok to separate these sections?
  2. Should I add a note in the Exact version requirement section saying that it can be chained with other Comparison requirements? (even if it's useless)

I would suggest separating it and adding a new warning for when this occurs, saying that it is redundant (I also would like to implement it).

In [dependencies] a "=" can be used to specify an exact version
@rust-highfive

Copy link
Copy Markdown

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 5, 2021
@alexcrichton

Copy link
Copy Markdown
Member

Thanks for the PR! I think it's fine to split this out to its own section, although strict version requirements like this are generally an anti-pattern on crates.io itself so we tend to advise against it, so could the section be placed at the end instead of the front? (if you're up for it calling out the hazards of publishing a crate with strict version requirements would also be great). Otherwise though I think it might be good to say that multiple requirements can be listed (I thought they needed to be separated with , but maybe not?), but I don't think there's any need to call out specifically that for =foo requirements specifying other things may not make much sense.

Rewrote sections about specifying dependencies versions in Cargo.toml
@marcospb19

Copy link
Copy Markdown
Author

I thought they needed to be separated with , but maybe not?

Oh yes, they do, sorry, I edited the original message

rand = "= 0.8.3, >= 0.8.3, < 0.8.4"

@marcospb19 marcospb19 closed this Oct 7, 2021
@marcospb19
marcospb19 deleted the patch-1 branch October 7, 2021 10:40
@marcospb19
marcospb19 restored the patch-1 branch October 7, 2021 10:40
@marcospb19 marcospb19 reopened this Oct 7, 2021
@marcospb19

marcospb19 commented Oct 7, 2021 •

Copy link
Copy Markdown
Author

Oops, I tried renaming the branch, that closed the PR unintentionally.

@marcospb19

Copy link
Copy Markdown
Author

@alexcrichton I tried to rewrite all the version requirements sections, they were untouched for more than 4 years.

I said that using strict version requirements would be redundant with other requirements, however, there is this counter-example:

time = "=0.3, < 0.3.5"
# which is equivalent to
time = ">=0.3, < 0.3.5"
# and
time = ">=0.3.0, < 0.3.5"

Comment on lines +163 to +165
> **Note**: Using strict requirements with exact versions are a bad practice and
> should be avoided, as it disables the ability of receiving important patches
> of security, soundness and bugfixes.

@marcospb19 marcospb19 Oct 7, 2021 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the advice you suggested, about the hazard of using strict requirements.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not word this so strongly. This goes against the advice given in https://doc.rust-lang.org/cargo/reference/resolver.html#recommendations where exact requirements do have some use cases (for example, serde has an exact semver requirement on serde_derive because the two are very tightly coupled and not intended to be used independently.

There are other use cases, such as a temporary workaround to lock to an exact version. I think we've done that with cargo itself before.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed it to:

> **Note**: When using strict requirements with exact versions, no updates can
> be applied, including bugfixes and security updates.

Any other recommendations?

Comment on lines +26 to +42
[semver]: https://github.com/steveklabnik/semver#requirements
[semver]: https://github.com/dtolnay/semver

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unresolved question: where was this pointing to? The previous repository now leads to this new one I inserted, but there is no section "#requirements".

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it used to summarize the rules for requirements, but that has been removed and instead points to this page (somewhat circular).

I notice this page doesn't link to or describe what SemVer is anywhere. Perhaps it could link to https://semver.org/ and/or https://doc.rust-lang.org/cargo/reference/resolver.html#semver-compatibility?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed it to https://doc.rust-lang.org/cargo/reference/resolver.html#semver-compatibility, because it already points to https://semver.org/ in the first line.

So it's like we're pointing to both.

@ehuss ehuss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

Lots of comments, but only because I think this text can be really important to be as clear as possible.

Comment on lines +54 to +55
If we specify the version string as `"1.0.4"`, compatible versions are, `1.0.x`
where `x` is greater than 4.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don' tthink this is correct. Specifying "1.0.4" means that it will pick any version 1.x.y greater than 1.0.4.

@marcospb19 marcospb19 Dec 17, 2021 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this to be a bit hard to explain without being confusing and verbose.

I tried organizing this in another way, here's what I came up with:

If we specify the version string as `"1.4.8"`, compatible versions are:
- `"1.4.x"` with `x ≥ 8`.
- `"1.x.y"` with `x > 2`, `y` can be anything.

What do you think?

separated with a comma, e.g., `>= 1.2, < 1.5`.
As shown in the examples above, it is possible to combine any requirements to
restrict the version even further, however, this is only necessary with
[comparison requirements](#Comparison-requirements).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Section fragments should be in lower case.

Suggested change
[comparison requirements](#Comparison-requirements).
[comparison requirements](#comparison-requirements).

Also, I would maybe include what it means to combine requirements, both the syntax and the semantics. That is, saying that you can combine multiple requirements with commas, and that it means that all requirements must be met.

@marcospb19 marcospb19 Dec 17, 2021 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the section upper case and added this text to the multiple requirements part:

The list of requirements should be separated by commas, spaces are optional.

A version is considered to be compatible with the list only if it is compatible
with all elements of the list.

Comment on lines +163 to +165
> **Note**: Using strict requirements with exact versions are a bad practice and
> should be avoided, as it disables the ability of receiving important patches
> of security, soundness and bugfixes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not word this so strongly. This goes against the advice given in https://doc.rust-lang.org/cargo/reference/resolver.html#recommendations where exact requirements do have some use cases (for example, serde has an exact semver requirement on serde_derive because the two are very tightly coupled and not intended to be used independently.

There are other use cases, such as a temporary workaround to lock to an exact version. I think we've done that with cargo itself before.

Comment on lines +10 to +11
You can specify different dependencies for:
- Development and release profiles.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what is meant is meant here, but it seems to imply you can independently define dependencies for different profiles (which can't be done). Can you say why this is mentioning profiles here?

@marcospb19 marcospb19 Dec 17, 2021 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was mistaken in this one, glad you got my mistake in the review.

Rewritten to:

Dependencies can be specified for [crate features](features.md) and for
different [architectures and operating systems](#platform-specific-dependencies).

Comment on lines +26 to +42
[semver]: https://github.com/steveklabnik/semver#requirements
[semver]: https://github.com/dtolnay/semver

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it used to summarize the rules for requirements, but that has been removed and instead points to this page (somewhat circular).

I notice this page doesn't link to or describe what SemVer is anywhere. Perhaps it could link to https://semver.org/ and/or https://doc.rust-lang.org/cargo/reference/resolver.html#semver-compatibility?

Comment on lines +46 to +48
**Caret requirements** allow SemVer compatible updates, it's the only
requirement type that uses specific rules for versions with and without the zero
digits.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused by this statement about the zero digits.

Usually I think it is best to have a definition up front when introducing a term, and I think it would be good to lead with the sentences that define its left-most non-zero behavior. I think the wording here is very important since this is introducing a potentially confusing topic, and this is one of the most important parts of cargo. If possible, it would be best to try to approach it assuming you don't know what semver is, or what a version requirement is and what it means. The original text tried to do that by introducing a topic by example from the perspective of what someone does with these dependencies. I'm not saying it has to use that style, just mentioning that is one way to introduce a new concept.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I rewrote this section and now the definition comes first, needs another review.


### Strict version requirements

**Strict version requirements** allow you to lock the numbers for major, minor

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little uneasy introducing this as a separately defined kind of requirement. For example =1 is not what I would characterize as "strict". Is there a particular reason for adding it? It seems like it fits with the comparison section, as = is a comparison operator.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My reasoning was that strict requirements only unique purpose is specifying exact versions without having to create a list of multiple requirements.

For example, =1.2.3 is a shorthand for >= 1.2.3, < 1.2.4.

And the other comparison operators all leave open ranges to be combined with more operators, = on the other hand, works very similarly to *, specifying a part of the version, and having the missing parts be "anything".

@ehuss ehuss added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 25, 2021
@bors

bors commented Dec 5, 2021

Copy link
Copy Markdown
Contributor

☔ The latest upstream changes (presumably #10158) made this pull request unmergeable. Please resolve the merge conflicts.

@ehuss

ehuss commented Dec 14, 2021

Copy link
Copy Markdown
Contributor

Ping @marcospb19 Just checking in to see if you are still interested in working on this, or if you had any questions.

@marcospb19

Copy link
Copy Markdown
Author

I'm still interested!

Sorry for the delay, I've been struggling finding time between multiple projects.

Now seems like a better time for trying finishing it, I'll see if I can take a look.

Thanks for the ping :D .

@marcospb19

Copy link
Copy Markdown
Author

Working on the conflict now.

@marcospb19

Copy link
Copy Markdown
Author

About the conflicts, some smaller work were done in the second section of this document in order to improve some of the explanations, I reused the new texts added there, but kept the new section organization that was made in this PR, containing the table of all operators and keeping the Caret Requirement explanation inside of it's dedicated section.


> **Note**: This is the only type of version requirement that has different
> rules for versions that start with `0`, and the version `"0.x.y"` is not
> compatible with any other versions.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand this last clause about 0.x.y not being compatible with anything else, isn't 0.x.(y+1) compatible with that?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if x > 0.

Before the review this was stating that 0.0.x was not compatible with other versions, @ehuss said that 0.0.x was not a common version, so it wouldn't be as useful in this explanation, so I changed to 0.x.y without noticing that it is now incorrect.

I'm not sure how to proceed here, is this information even useful? I do often get confused about this, maybe I should just leave the definition.

Or remove completely the "and" part?

, and the version "0.x.y" is not compatible with any other versions.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this note trying to convey? It's correct that 0.0.x is not that common but it's incorrect to say that 0.x.y is not compatible with any other versions.

Comment on lines +72 to +79
^1 := >=1.0.0, <2.0.0
^1.2 := >=1.2.0, <2.0.0
^1.2.3 := >=1.2.3, <2.0.0
^0 := >=0.0.0, <1.0.0
^0.0 := >=0.0.0, <0.1.0
^0.0.3 := >=0.0.3, <0.0.4 (exact)
^0.2 := >=0.2.0, <0.3.0
^0.2.3 := >=0.2.3, <0.3.0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC #10158 tried not to mention much about the ^ syntax on requirement to reduce confusion. Do we want to follow the rule here and throughout this PR?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How could I proceed here?

  1. Rename caret requirement to SemVer requirement.
  2. Inside of the caret requirements section, remove all carets, and add a note explaining that "1.4.8" == "^1.4.8".
  3. Try to move all caret requirements out of it's own section, and put it on the start of the file.

I tried approaching 3 like #10158 but I found it to be weird with the new table with all the operators.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... maybe keep calling it caret but only mention it as a alternative syntax at the end of the "Caret requirements" section? Such like

Note that Cargo favors specifying caret requirements without a real caret ^. You can still use the alternative syntax. For example, ^1.2.3 is exactly equivalent to 1.2.3.

Though it still seems a bit awkward...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like @weihanglo's idea here to not use ^ in this section where possible, and at the end there can be a small code-block that says that ^1.2.3 := 1.2.3 or similar

@alexcrichton alexcrichton removed their assignment Mar 8, 2022
@ehuss

ehuss commented Mar 22, 2022

Copy link
Copy Markdown
Contributor

I'm going to close due to inactivity. If you or anyone is interested in continuing this, feel free to open a new PR with the above comments addressed.

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

Labels

S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants