Skip to content

add some more documentation for non-inclusive version ranges#24025

Closed
cosmicexplorer wants to merge 2 commits intospack:developfrom
cosmicexplorer:spec-parse-inequalities
Closed

add some more documentation for non-inclusive version ranges#24025
cosmicexplorer wants to merge 2 commits intospack:developfrom
cosmicexplorer:spec-parse-inequalities

Conversation

@cosmicexplorer
Copy link
Copy Markdown
Contributor

@cosmicexplorer cosmicexplorer commented May 31, 2021 •
edited
Loading

Problem

#26422 removes the \.99+ suffix many packages append to version strings in order to support non-inclusive range bounds e.g. 1.2 <= v < 2: previously we had recommended the use of @1.2:1.2.999 for this, but we now recommend using @1.2:1. However, some packages still contain this \.99+ workaround, and our documentation doesn't explicitly describe the new recommendation.

Solution

  • Fix some \.99+ bounds that remained after Remove .99 when possible #26422.
  • Clarify in documentation how this works, and explicitly say that \.99+ is deprecated.

Result

We should be able to reserve .99 components for actual versions named .99 instead of using it as a workaround.

@cosmicexplorer
Copy link
Copy Markdown
Contributor Author

cosmicexplorer commented May 31, 2021 •
edited
Loading

As mentioned in the linked comment at #20258 (comment):

Still working on this, but no longer have access to push to spack/spack, so will open up a new draft PR! I've made great progress in reducing the implementation logic, but have run into another conundrum, where Version('1.6') < Version('1.6.5'), but Version('1.6') is not also greater than Version('1.6.5'). I now believe this is incorrect, since a Version('1.6') can resolve to e.g. Version('1.6.8'), which is clearly greater than Version('1.6.5'). I will attempt to succinctly describe why this is now a concern in the new PR.

Currently considering the ramifications of making Version('1.6') > Version('1.6.5') true -- I believe this is actually necessary to support strict inequalities at all, as well as being the right thing to do, and can be implemented somewhat succinctly -- but I have to verify that first.

@cosmicexplorer cosmicexplorer force-pushed the spec-parse-inequalities branch 2 times, most recently from 2cfbc82 to 1c229c0 Compare May 31, 2021 08:39
@michaelkuhn
Copy link
Copy Markdown
Member

michaelkuhn commented May 31, 2021

Currently considering the ramifications of making Version('1.6') > Version('1.6.5') true -- I believe this is actually necessary to support strict inequalities at all, as well as being the right thing to do, and can be implemented somewhat succinctly -- but I have to verify that first.

The problem with this are packages that do not follow semver. For instance, cube has releases for 4.4, 4.4.2 etc., where 4.4 < 4.4.2. I guess we could also map those to 4.4.0 even though that is not the correct upstream version.

@haampie
Copy link
Copy Markdown
Member

haampie commented Jun 1, 2021 •
edited
Loading

We may have to discriminate between 1.6 being the closed-open range [1.6.0..., 1.7.0...) and being the closed-closed "range" [1.6.0..., 1.6.0...] -- Version('1.6') is ambiguous, you'd need something like Version('1.6', singleton/exact=True/False) to ensure that the version is really a singleton or a range. (Maybe we should just almost always represent a version as a closed-closed, closed-open, open-closed or open-open range? 🤔)

@cosmicexplorer cosmicexplorer force-pushed the spec-parse-inequalities branch 3 times, most recently from eee2143 to 917215a Compare June 10, 2021 10:57
@cosmicexplorer cosmicexplorer force-pushed the spec-parse-inequalities branch from 917215a to 9c79fd1 Compare July 23, 2021 07:10
@cosmicexplorer cosmicexplorer mentioned this pull request Jul 23, 2021
Closed
@cosmicexplorer cosmicexplorer force-pushed the spec-parse-inequalities branch from 9c79fd1 to 6b00fb2 Compare July 25, 2021 08:22
@spackbot-app spackbot-app bot added the flake8 label Jul 25, 2021
@cosmicexplorer cosmicexplorer force-pushed the spec-parse-inequalities branch 9 times, most recently from 9c7e94d to b390944 Compare July 25, 2021 09:33
@cosmicexplorer cosmicexplorer force-pushed the spec-parse-inequalities branch 8 times, most recently from 7716b5e to 5de74d1 Compare April 3, 2022 20:15
This was referenced Sep 30, 2022
Open
Closed
@cosmicexplorer cosmicexplorer force-pushed the spec-parse-inequalities branch from 5de74d1 to 1d8acff Compare October 4, 2022 02:51
@spackbot-app spackbot-app bot added core PR affects Spack core functionality workflow labels Oct 4, 2022
@cosmicexplorer cosmicexplorer force-pushed the spec-parse-inequalities branch 2 times, most recently from d157a89 to 826610e Compare October 4, 2022 18:35
@cosmicexplorer cosmicexplorer force-pushed the spec-parse-inequalities branch from 826610e to d83afaa Compare October 4, 2022 18:53
@cosmicexplorer
Copy link
Copy Markdown
Contributor Author

cosmicexplorer commented Oct 4, 2022

Just redid this PR entirely to completely avoid modifying spec syntax or any logic in version.py!

@cosmicexplorer cosmicexplorer mentioned this pull request Oct 5, 2022
Open
2 tasks
@cosmicexplorer
Copy link
Copy Markdown
Contributor Author

cosmicexplorer commented Oct 28, 2022

Just moved a paragraph about version ranges from version-specifier to version-ranges. @haampie do you think this is good to go?

@cosmicexplorer
Copy link
Copy Markdown
Contributor Author

cosmicexplorer commented Jun 25, 2023

This is now largely obviated by the work to make e.g. spack install [email protected] work now. Thanks to whoever did that!

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

Reviewers

@haampie haampie Awaiting requested review from haampie

Assignees

No one assigned

Labels

binary-packages commands conflicts core PR affects Spack core functionality dependencies directives documentation Improvements or additions to documentation environments fetching flake8 new-variant patch python R tests General test capability(ies) update-package utilities vendored-dependencies versions virtual-dependencies workflow

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cosmicexplorer @michaelkuhn @haampie