Skip to content

Conversation

awvwgk
Copy link
Member

@awvwgk awvwgk commented Sep 21, 2020

This is probably not immediately useful, therefore I'm opening it as draft for discussion.

This PR implements a version type, somewhat similar to Python's StrictVersion, which allows parsing version numbers and comparing them, if they follow a strict format. Version numbers can become complicated, the most involved versioning scheme I have seen so far is the Arch Linux PKGBUILD version, featuring a version epoch, a version number (probably semantic, but not necessarily) and a build number: [<int>:]<int>[.<int>...]-<int>.

Here support for a version of the form of <int>[.<int>...] is implemented, where the number of subversions is limited to an arbitrarily chosen 10 (probably way too much) 3 (for semantic versioning <major>[.<minor>[.<patch>]]). Have a look at the tests to see what is possible.

@everythingfunctional
Copy link
Member

I'm a big fan of semantic versioning. This means 3 parts to a version number, <major>.<minor>.<patch>. I think we should at least strongly encourage this for any published fpm packages.

The requirements are basically:

  • Any additions to public interfaces require a minor version bump
  • Any backward incompatible change requires a major version bump

If semantic versioning is followed, users know that patch and minor version updates are safe (won't break anything), and major version upgrades may require some rework.

@certik
Copy link
Member

certik commented Sep 22, 2020

You can probably already guess my opinion: I suggest we require semantic versioning, just like Cargo.

@awvwgk
Copy link
Member Author

awvwgk commented Sep 22, 2020

I'm all for encouraging semantic versioning. It won't hurt to give fpm at least the ability to make sense from version numbers not strictly following this scheme to make it a more robust.

A good example is the conda-forge-pinning feedstock, not because it uses a timestamp as version, but because it has a long list of important packages not following semantic versioning.

@LKedward
Copy link
Member

This looks good @awvwgk. I agree that understanding versions will be important in fpm for dependencies.

I agree with everything that @everythingfunctional has said; semantic versioning is easy and intuitive to understand.
Since we're starting from scratch here, I also think it makes sense to stick to one version format for all packages - unless there are advantages/requirements for other formats?

However many user packages won't be destined for publishing so I don't think fpm should require a version string for packages or attempt to enforce the rules of semantic versioning beyond the format of the field.

Enforcement of semantic versioning rules should happen at package registry level (fpm-registry) with fpm simply having helper options to automatically set and bump the version as is required. fpm-registry already requires a version field to be present in package manifests during its CI checks. On PR, fpm-registry can check interfaces against any prior registry versions and throw an error if the rules aren't met. I don't believe this will be difficult to implement in our current checks.

@awvwgk
Copy link
Member Author

awvwgk commented Sep 22, 2020

Semantic versioning is fine, up to the point where the developer applying it fails to follow its convention, on purpose or by accident. Also, API breakage is not always a good measure for versioning, not every compatibility breakage is defined by an API or ABI change.

I'm not advocating against semantic versioning. Just keep in mind that versioning can be complicated and even requiring semantic versioning might not be enough. It would be thoughtful of fpm to allow for some flexibility in this regard, even if it is just to make an already painful job of sorting out dependency versions not more painful.

@everythingfunctional
Copy link
Member

I very much agree with what @LKedward said. True, semantic versioning isn't bullet proof - despite our best efforts, users can (intentionally or not) end up depending on internal implementation details - but it's still helpful and worth utilizing.

I could envision allowing some additional info in the version string, but it would not be used for ordering of versions, and for any packages that supply it, that would be a necessary piece for the users of the library to include when specifying a version. The most obvious use case that comes to mind would be different versions for different operating systems, and still allow users use wildcard version specification (i.e. some_lib = '2.*-linux') would match the latest version 2.0 <= x < 3.0 that ends in -linux. Thus, a version string would have the format <major>.<minor>.<patch>[-whatever_you_want] where semantic versioning is still implied between versions with matching -whatever_you_want and there is no ordering implied between different optional endings. Seem reasonable?

@awvwgk
Copy link
Member Author

awvwgk commented Sep 22, 2020

Maybe this is just some misunderstanding. Would this mean the version string must match the regex \d+\.\d+\.\d+?

The most obvious use case that comes to mind would be different versions for different operating systems, and still allow users use wildcard version specification (i.e. some_lib = '2.*-linux') would match the latest version 2.0 <= x < 3.0 that ends in -linux.

Sorry for derailing this conversation, the better solution than mangling this into the version string would be allowing the registry to specify variants and fpm use them with

some_lib.version = "2"
some_lib.variant = "linux"

@everythingfunctional
Copy link
Member

I think we could allow the absence of the minor or patch versions to simply imply that they are zero.

The variant route might be an interesting approach instead of complicating the version.

@awvwgk awvwgk force-pushed the version branch 2 times, most recently from 272b6e6 to 38edbc6 Compare September 22, 2020 18:24
@awvwgk
Copy link
Member Author

awvwgk commented Sep 22, 2020

I set the maximum subversion limit to three, resulting effectively in “semantic versioning,” with optional minor and patch version.
Also implemented a “semantic version” matching for the version date type as well, while trying to keep it as flexible as possible.

The match operator between two versions would work like this:

some_dep1 = "2"  # >=2 and <3
some_dep2 = "0.7" # >=0.7 and <0.8
some_dep3 = "1.3" # >=1.3 and <1.4
some_dep4 = "1.0.7" # >=1.0.7 and <1.0.8 <=> ==1.0.7
# to discuss: version numbers ending on zero
some_dep5 = "2.0" # currently >=2 and <3, not >=2.1 and <3.1

This might not be the best choice to determine the version number to match against:

do ii = size(rhs%num), 1, -1
if (rhs%num(ii) /= 0) then
tmp%num = rhs%num(:ii)
exit
end if
end do

Probably, we should not compare against a version data type at all, but have a separate version-constraint data type for this purpose.

@arjenmarkus
Copy link
Member

arjenmarkus commented Sep 23, 2020 via email

@awvwgk
Copy link
Member Author

awvwgk commented Sep 23, 2020

Agreed, I'm just using the number of subversions to create the matching now.

@everythingfunctional
Copy link
Member

I believe most package managers use a different type for the package version versus the specified version of a dependency. They then use some sort of constraint solver for selecting the appropriate version from the repository/registry.

I agree with @arjenmarkus that most users would probably expect some_dep = "2.0" to mean >= 2.0.0 and < 2.1.0, whereas some_dep = "2" would mean >=2.0.0 and <3.0.0.

@awvwgk
Copy link
Member Author

awvwgk commented Sep 25, 2020

Thanks for the input and discussion so far. I guess I will just mark it as ready for review now.

The .match. operator should now work as expected without surprises:

some_dep1 = "2"  # >=2 and <3
some_dep2 = "0.7" # >=0.7 and <0.8
some_dep3 = "1.3" # >=1.3 and <1.4
some_dep4 = "1.0.7" # >=1.0.7 and <1.0.8 <=> ==1.0.7
some_dep5 = "2.0" # >=2.0 and <2.1

All comparison operators are implemented as elemental, so they should play nicely together with array operations and any or all reductions.

@awvwgk awvwgk marked this pull request as ready for review September 25, 2020 19:30
@everythingfunctional
Copy link
Member

Didn't check the code for it, but I think your example

some_dep5 = "2.0" # >=2.1 and <3.1

should be

some_dep5 = "2.0" # >=2.0 and <2.1

Copy link
Member

@LKedward LKedward left a comment

Choose a reason for hiding this comment

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

This is a nice clean and general solution which will be very useful.
Thanks @awvwgk. 👍

@certik
Copy link
Member

certik commented Sep 28, 2020

@awvwgk, can you please address @everythingfunctional's question, and then resolve conflicts? After that this is ready to merge.

- allow semantic version matching
@awvwgk
Copy link
Member Author

awvwgk commented Sep 28, 2020

Rebased against 90ddc6f

@milancurcic
Copy link
Member

@awvwgk Please merge when ready.

@awvwgk awvwgk merged commit 0c35749 into fortran-lang:master Sep 29, 2020
@awvwgk awvwgk deleted the version branch September 29, 2020 14:46
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.

6 participants