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

Bug # 946966 #65

Closed
wants to merge 5 commits into from
Closed

Bug # 946966 #65

wants to merge 5 commits into from

Conversation

abhaychrungoo
Copy link

Puppetlabs recommends following the semver 1.0.0 specification for publishing modules

Current pulp implementation of comparing module versions ignores semver 1.0.0 specification, particularly

"A pre-release version number MAY be denoted by appending an arbitrary string immediately following the patch version and a dash. The string MUST be comprised of only alphanumerics plus dash [0-9A-Za-z-]. Pre-release versions satisfy but have a lower precedence than the associated normal version. Precedence SHOULD be determined by lexicographic ASCII sort order. For instance: 1.0.0-alpha1 < 1.0.0-beta1 < 1.0.0-beta2 < 1.0.0-rc1 < 1.0.0."

This patch fixes the issue.

Some tests:

PUBLISH puppet-test-0.0.2-rc1

[root@puppet-master2 tmp]# puppet module install puppet/test --target-dir=/tmp/mods
Notice: Preparing to install into /tmp/mods ...
Notice: Downloading from http://.:puppetforge@puppetrepo.server.com ...
Notice: Installing -- do not interrupt ...
/tmp/mods
âââ puppet-test (v0.0.2-rc1)

PUBLISH puppet-test-0.0.2-rc1-11

[root@puppet-master2 tmp]# rm -rf /tmp/mods
[root@puppet-master2 tmp]# rm -rf mods
[root@puppet-master2 tmp]# puppet module install puppet/test --target-dir=/tmp/mods
Notice: Preparing to install into /tmp/mods ...
Notice: Created target directory /tmp/mods
Notice: Downloading from http://.:puppetforge@puppetrepo.server.com ...
Notice: Installing -- do not interrupt ...
/tmp/mods
âââ puppet-test (v0.0.2-rc1-11)

PUBLISH puppet-test-0.0.2

[root@puppet-master2 tmp]# rm -rf /tmp/mods
[root@puppet-master2 tmp]# puppet module install puppet/test --target-dir=/tmp/mods
Notice: Preparing to install into /tmp/mods ...
Notice: Created target directory /tmp/mods
Notice: Downloading from http://.:puppetforge@puppetrepo.server.com ...
Notice: Installing -- do not interrupt ...
/tmp/mods
âââ puppet-test (v0.0.2)

@mhrivnak
Copy link
Contributor

mhrivnak commented Sep 6, 2013

Thank you for the submission! I ended up using the semantic_version python library to accomplish this, which resulted in a very clean and very small diff, and allows us to not be responsible for implementing and maintaining semver compatibility.

That said, looking at your PR definitely helped, and I hope we can find an opportunity in the future to merge some commits from you. :)

@mhrivnak mhrivnak closed this Sep 6, 2013
@abhaychrungoo
Copy link
Author

I think semantic_version is the right approach.
My dev machine which is el5 couldn't install semantic_version which is python 2.6+, and that's why I chose to write code.

Having realized that pulp server is also 2.6+, I think this is the right move. Having introduced the library now, have you considered making version a semver instead of a string.

There are similar issues(although less significant) like pulp forge does not accept a puppet module abc/xyz --version=v1.2.3 command because of the superfluous v.

You'll notice that puppetforge does, and many projects like librarian-puppet and r10k are building functionality around that assumption.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants