Adopt "vers" version range spec by pombredanne · Pull Request #12 · aboutcode-org/univers

pombredanne · 2021-11-15T14:54:14Z

This PR add support for the new vers draft specification for version ranges
The primary usage is going to be used in VulnerableCode to support
aboutcode-org/vulnerablecode#525

This a major refactoring of the library

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Also apply minor formatting Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Use namespaces rather than import renaming Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Based on the same code I wrote for Package URL. Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

This tests if a version is valid and is best closest to the gentoo code Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Using the join/split idiom is more powerful than just replacing spaces. It can handle any whitespace. Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Major refactoring: - rename version_range.py to version_constraints.py - then rename version_specifier.py to version_range.py The model is now working this way: - VersionRange is a collection of VersionConstraint with a specific Version class implementation to compare versions. - There is one VersionRange for each version scheme - A VersionRange can be built from a "vers" string or from a native, scheme specific syntax. - A VersionConstraint contains a comparator (such as =) and a Version instance - There are many Version implementations as before - Not all schemes are implemented, and a scheme is commonly the same string as a Package URL type. Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

sbs2001 · 2021-11-22T04:48:03Z

+- It can parse native version ranges notation into the common "vers" notation
+  and can return back native version ranges from a "vers".


Just to be clear.

"parse native version ranges notation into the common "vers" notation" means converting ">1.2.3" to "vers:pypi/>1.2.3"

"can return back native version ranges from a 'vers' " . What is this "native version ranges" ? Is it a string representation of the range or is it an object ?

is it a string representation of the range or is it an object ?

This is a string, for instance the one seen in an npm package.json for dependencies or in a setup.py install_requires

sbs2001 · 2021-11-22T05:13:34Z

+        >>> assert v24 in VersionConstraint(comparator="<=", version=v24)
+        >>> assert v24 not in VersionConstraint(comparator="<", version=v24)
+        """
+        if not version:


But for "*" comparator, version is supposed to be None. Hence it will always return false which is opposite of what we're expecting.

The version argument here is the version that we want to check. This is not the same as the version attribute of the constraint. You would never pass a constraint as an argument. The if not version: is just a guard against calling the method using None and other falsish values... but to your point this can be confusing and would be caught by the type check done below... so I am removing this line.

sbs2001 · 2021-11-22T05:14:22Z

+        if not version:
+            return False
+
+        # TODO: consider if we should accept any type and return False?


This is debatable, and I have no clear answer.

Raising a ValueError is best for now, removed comment in latest commit

sbs2001 · 2021-11-22T05:16:06Z

+        return constraints
+
+    @classmethod
+    def to_constraints_string(cls, constraints):


Are doctest possible for this ? Maybe add some examples.

Agreed and fixed in latest commit

sbs2001 · 2021-11-22T05:19:41Z

+        Return a VersionRange built from an npm "node-semver" range ``string``.
+        """
+        # FIXME: code is entirely duplicated with the GemVersionRange
+        from semantic_version import NpmSpec


Why are the imports lazy ?

I moved these to top level.

sbs2001 · 2021-11-22T05:20:35Z

+        Return a VersionRange built from a Rubygem version range ``string``.
+        """
+        # TODO: Gem version semantics are different from semver:
+        # there can be commonly more than 3 segments


This could be added in the README

@sbs2001

- from_vers() -> from_string() suggested by @sbs2001 - from_native_range() -> from_native() - to_native_range() -> to_native() Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

@sbs2001

Based on multiple review feedback from @sbs2001 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

@sbs2001

None in VersionConstraint will now raise a ValueError. Reported-by: @sbs2001 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

@sbs2001

Suggested by @sbs2001 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

@sbs2001

Suggested by: @sbs2001 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

@sbs2001

VersionConstraint.to_constraints_string is not a trivial function, soe some example help a lot Suggested by: @sbs2001 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

pombredanne · 2021-11-22T11:02:08Z

@sbs2001 Thank you for your detailed review!
I addressed all of your comments.

This adds univer from the current branch that support "vers". See aboutcode-org/univers#12 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Create NginxVersionRange.from_native method Contributed-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

@sbs2001

Suugested by: @sbs2001 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

sbs2001

LGTM. Thanks

pombredanne · 2021-11-30T13:46:45Z

Merging now!

This adds univer from the current branch that support "vers". See aboutcode-org/univers#12 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

pombredanne added 11 commits October 30, 2021 10:43

Rename test_data to data

83975a4

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Fix typo and use correct project URL

b024d4b

Also apply minor formatting Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Format code

b53a888

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Improve error handling

bec43f8

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Reorganize imports

9dfc73a

Use namespaces rather than import renaming Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Format code

3101bc2

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Add skeleton for "vers" tests

ccc4fbf

Based on the same code I wrote for Package URL. Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Add new gentoo.is_valid function

0053dc4

This tests if a version is valid and is best closest to the gentoo code Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Remove object subclassing, not need in Python 3

9f4cb8b

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Improve remove_spaces

bd9bf76

Using the join/split idiom is more powerful than just replacing spaces. It can handle any whitespace. Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

pombredanne requested review from Hritik14 and sbs2001 November 17, 2021 23:21

sbs2001 requested changes Nov 22, 2021

View reviewed changes

pombredanne added 6 commits November 22, 2021 11:24

Rename VersionRange factory functions

3de2aba

- from_vers() -> from_string() suggested by @sbs2001 - from_native_range() -> from_native() - to_native_range() -> to_native() Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Improve documentation

7b0ceab

Based on multiple review feedback from @sbs2001 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Clarify None in VersionConstraint behaviour

7ba6fba

None in VersionConstraint will now raise a ValueError. Reported-by: @sbs2001 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Move import to top level

0bc88f4

Suggested by @sbs2001 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Explain that Rubygems is not semver

2806876

Suggested by: @sbs2001 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Add doctests to to_constraints_string()

8894b9e

VersionConstraint.to_constraints_string is not a trivial function, soe some example help a lot Suggested by: @sbs2001 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

pombredanne added a commit to Hritik14/vulnerablecode that referenced this pull request Nov 23, 2021

Use latest univers branch

ba9e8be

This adds univer from the current branch that support "vers". See aboutcode-org/univers#12 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

pombredanne added 4 commits November 23, 2021 12:43

Fix typo coma->comma

36580a9

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Implement NginxVersionRange

1491f12

Create NginxVersionRange.from_native method Contributed-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Add reminder to handle PEP440 ===

7bcce9d

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Clarify that native range conversion is to string

588ef39

Suugested by: @sbs2001 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

pombredanne force-pushed the adopt-vers-spec branch from c6331a9 to 588ef39 Compare November 23, 2021 11:43

sbs2001 approved these changes Nov 30, 2021

View reviewed changes

pombredanne merged commit 7a99ab9 into main Nov 30, 2021

pombredanne deleted the adopt-vers-spec branch November 30, 2021 13:46

This was referenced Nov 30, 2021

Cannot convert VersionSpecifier to canonical representation and back #7

Closed

Naming: VersionSpecifier and VersionRange #8

Closed

Hritik14 pushed a commit to Hritik14/vulnerablecode that referenced this pull request Jan 23, 2022

Use latest univers branch

07b6b5c

This adds univer from the current branch that support "vers". See aboutcode-org/univers#12 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Hritik14 pushed a commit to Hritik14/vulnerablecode that referenced this pull request Jan 23, 2022

Use latest univers branch

bdd1aec

This adds univer from the current branch that support "vers". See aboutcode-org/univers#12 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Hritik14 pushed a commit to Hritik14/vulnerablecode that referenced this pull request Jan 23, 2022

Use latest univers branch

558c334

This adds univer from the current branch that support "vers". See aboutcode-org/univers#12 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Hritik14 pushed a commit to Hritik14/vulnerablecode that referenced this pull request Jan 25, 2022

Use latest univers branch

be31a0e

This adds univer from the current branch that support "vers". See aboutcode-org/univers#12 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

		- It can parse native version ranges notation into the common "vers" notation
		and can return back native version ranges from a "vers".

Uh oh!

Uh oh!

Conversation

pombredanne commented Nov 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pombredanne Nov 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pombredanne Nov 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pombredanne commented Nov 22, 2021

Uh oh!

sbs2001 left a comment

Choose a reason for hiding this comment

Uh oh!

pombredanne commented Nov 30, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pombredanne commented Nov 15, 2021 •

edited

Loading

pombredanne Nov 22, 2021 •

edited

Loading

pombredanne Nov 22, 2021 •

edited

Loading