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

Support pub package manager #131

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

shravankshenoy
Copy link

@shravankshenoy shravankshenoy commented Mar 2, 2024

Fixes #110

How Versioning works in Pub/Dart

Pub Version semantics very closely follow semver except for a few corner cases listed below (refer https://github.com/dart-lang/pub_semver/tree/master)

  1. Version ordering does take build suffixes into account (i.e. 1.2.3+1 is considered lower than 1.2.3+2)

  2. Pre-release versions of the maximum is excluded from range unless the max is itself a pre-release or the min is a pre-release of the same version. Other pre release versions belong to range, with the priority mentioned in point 3.

    For example, a <2.0.0 constraint will prohibit not just 2.0.0 but any pre-release of 2.0.0. However, <2.0.0-beta will exclude 2.0.0-beta but allow 2.0.0-alpha. Likewise, >2.0.0-alpha <2.0.0 will exclude 2.0.0-alpha but allow 2.0.0-beta. Note that <2.0.0 will allow prereleases of 1.x.x like 1.2.3-alpha or 1.5.6-dev

  3. Any stable version is considered higher priority than any unstable version. For example, the below versions in increasing order of priority are : 1.2.0-alpha < 1.3.0-experimental < 1.0.0 < 1.2.0.
    Note that 1.0.0 is considered higher priority than 1.3.0-experimental since it is a stable version

  4. Pub defines the "next breaking" version as the version which increments the major version if it's greater than zero, and the minor version otherwise, resets subsequent digits to zero, and strips any pre-release or build suffix.
    To make use of this, pub defines a "^" operator which yields a version constraint greater than or equal to a given version, but less than its next breaking one.

Implementation

Implementation of above 4 points is discussed below

  1. Implemented in univers by default
>>> from univers.versions import PubVersion
>>> PubVersion("1.2.3+1") < PubVersion("1.2.3+2")
True

  1. Currently VersionRange class which PubVersionRange inherits from considers pre-release version of max constraint as belonging to range
>>> from univers.version_range import PubVersionRange
>>> range = PubVersionRange.from_native(">=1.0.0 <2.0.0")
>>> str(range)
'vers:pub/>=1.0.0|<2.0.0'
>>> PubVersion("2.0.0-alpha") in range
True
 

NOTE: In line 151 of version_range_test.dart Pre-release versions of max are allowed, so I am not sure whether README is outdated or I missed something

  1. Not implemented currently. Should we implement this? Any thoughts?
>>> PubVersion("1.3.4-alpha") < PubVersion("1.2.3")
False
  1. Logic has been implemented in from_native method of PubVersionRange
>>> str(PubVersionRange.from_native("^1.3.4"))
'vers:pub/>=1.3.4|<2.0.0'
>>> str(PubVersionRange.from_native("^0.4.5"))
'vers:pub/>=0.4.5|<0.5.0'
>>> PubVersion("1.4.5") in PubVersionRange.from_native("^1.3.4")
True

Things to Consider

  1. Pub treats Integer and String builds separately, whereas univers considers all builds to be of type String. Refer lines 121 and 122 of version_test.dart
  2. I believe point 2 in Implementation is loosely related to Properly handle the pre-release versions in VersionRange #130 , although Pub does consider pre release versions to be part of range if it is not prerelease of max verison. There have been discussions regarding this behaviour in Introduce a pub upgrade --(no)-allow-prereleases flag. dart-lang/pub#2471
  3. Most of the tests have been ported from pub-semver repo.
    For example, in test_pub_version.py, test_equal is from lines 117 to 124 of version_test.dart, test_compare is from lines 69 to 100 of version_test.dart and so on. The pub-semver repo has many other tests which are specific to functions available in the pub-semver library, but not relevant to us like prioritize(), antiorioritize() etc. In case there are other tests from the pub-semver library you would like me to incorporate here, let me know.

Copy link
Contributor

@TG1999 TG1999 left a comment

Choose a reason for hiding this comment

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

Minor nits for your consideration, also please remove all unrelated changes.

@@ -385,6 +385,12 @@ def build_value(cls, string):
return super().build_value(string.lstrip("vV"))


class DartVersion(SemverVersion):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
class DartVersion(SemverVersion):
class PubVersion(SemverVersion):

@@ -702,4 +712,5 @@ def bump(self, index):
OpensslVersion,
LegacyOpensslVersion,
AlpineLinuxVersion,
DartVersion,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
DartVersion,
PubVersion,

@shravankshenoy
Copy link
Author

Minor nits for your consideration, also please remove all unrelated changes.

Thanks @TG1999 for the initial review, I will replace all instances of DartVersion with PubVersion.

Regarding the other un related changes, I believe they were caused by the black formatter. I have black=22.3.0 as mentioned in requirements-dev.txt, but it is still causing the changes. Infact when I ran black --check ., I got a bunch of files which seem to not follow black standards, as shown in image below

image

Could you check if you are getting a similar result when you run the command? If so I would be glad to create a separate PR which fixes all the formatting issues.

Moreover, even the tests which are failing in the Azure pipeline are due to Black formatting, although I have run black on all the files I have created in this PR. My hunch is the files listed above might be causing the problem, but let me know if that is not the case.

image

@TG1999 TG1999 self-requested a review July 9, 2024 15:50
@keshav-space keshav-space marked this pull request as draft July 25, 2024 09:18
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

Successfully merging this pull request may close these issues.

Support pub package manager for dart and flutter
2 participants