PVP versioning requirements

[ysangkok]

@rarkins suggested in #31493 that I detail some possible requirements for a PVP versioning module (BTW, see the FAQ, which contains a comparison with semver). The PVP scheme is widely used, so it would make sense to configure it for use as the default with the Hackage datasource, that I can also contribute (see linked thread).

I have continued working on a PVP prototype which supports a subset of the Cabal version expression syntax: >=X.Y && <N.M. Here are some differences to the NPM semver flavour, that I found:

Major version has two components in PVP, but only one in NPM semver

Because the first two components in PVP are part of the major component, they both go into the major component, meaning that getMajor returns differing results:

      expect(pvp.getMajor("1.0.0")).toEqual(1.0);
      expect(pvp.getMajor("1.0.1")).toEqual(1.0);
      expect(pvp.getMajor("1.1.1")).toEqual(1.1);
      expect(npm.getMajor("1.0.0")).toEqual(1);
      expect(npm.getMajor("1.1.1")).toEqual(1); // Note how this differs in NPM compared to PVP

PVP accepts an arbitrary number of components

In PVP, there are versions that wouldn't even be valid NPM versions. Because getPatch returns a float, we can put all those digits into the float. We'd get precision issues if there are many patch digits, but I think that is an edge case that doesn't happen often.

      expect(pvp.getPatch("1.0.0.5.1")).toEqual(5.1);
      expect(pvp.getPatch("1.0.1.6")).toEqual(6);
      expect(pvp.getPatch("1.1.2.7")).toEqual(7);
      expect(npm.isValid("1.0.0.5.1")).toBeFalse(); // Note how this differs in NPM compared to PVP
      expect(npm.isValid("1.1.2.7")).toBeFalse(); // Note how this differs in NPM compared to PVP

Syntax differences

Because the versioning needs to produce the new version string with getNewVersion, I think it might be best to use the Cabal format consistently, instead of e.g. trying to use NPM syntax (i.e. without &&) in a PVP context.

This means having e.g. >=1.0 && <1.1 in range arguments instead of >=1.0 <1.1.

getSatisfyingVersion

In queries like these, you can see how PVP and NPM yield different results.

      expect(
        pvp.getSatisfyingVersion(
          ['1.0.0', '1.0.4', '1.3.0', '2.0.0'],
          '>=1.0 && <1.1',
        ),
      ).toBe('1.0.4');
      expect(
        npm.getSatisfyingVersion(
          ['1.0.0', '1.0.4', '1.3.0', '2.0.0'],
          '^1.0.0',
        ),
      ).toBe('1.3.0'); // Note how this differs in NPM compared to PVP

Why support just this Cabal expression syntax?

In the above example, we use NPM's caret syntax to try to select all compatible versions. In PVP, there is a syntax like this, called ^>=, but I propose not supporting this at first, to start out small. Similar functionality is achievable with >= && < and this particular syntax is popular since it allows for specifying compatibility with multiple major versions, instead of just one. Many libraries have frequent breaking changes in Haskell, e.g. the standard library, which changes in every version. It shouldn't be too difficult to support ^>= 4.17 || ^>= 4.18 || ... in the future too.

Supporting an arbitrary amount of components in the lower end of the range

Sometimes, release X.Y has a bug, which is fixed in X.Y.Z. A dependency bound will then be specified with the .Z even though PVP dictates that X.Y.W (where W < Z) is also API compatible. This shouldn't be hard to support, so I'd include this requirement. I just haven't mentioned it above because it's not controversial.

getNewVersion

I'd like to wait with implementing getNewVersion until we have the manager in place. Is that ok?

What's next?

I have a prototype that I'd like to extend with more tests. I'd like to then submit that in a PR. After we add versioning, I'd like to continue with the data source and manager (subject to discussion in #31493).

For now, my questions are: Do you have any concerns with adding PVP as a versioning scheme?

[mihaimaruseac]

From a Haskell point of view, this seems reasonable. Thank you for it!

[ysangkok]

@secustor Is there anything I can do to advance this?

[ysangkok]

Ok, I have come up with a draft that works all right. It is not quite ready for PR submission yet, since the signature of getMajor is causing me problems: It has to return a number.

As previously mentioned, in version A.B.C, both A and B are part of the major version. So I thought, why not use the decimal digits to store the B component? So the major version of 1.2.3 would be the IEEE double 1.2.

Now, that works fine for some versions, but it doesn't work well for versions like 1.1 and 1.10, because they are considered to have the same major version. This could be a problem, I imagine, so I am wondering how to address this. This is unfortunate, because I really I wouldn't have to touch any generic code...

I looked into the code, trying to figure out why it has to be number, and I found this:

lib/workers/repository/process/lookup/update-type.ts
17:  if (versioning.getMajor(newVersion)! > versioning.getMajor(currentVersion)!) {

So I am wondering, maybe the only reason it is a number, is such that this comparison can be made? That is, to detect whether it is major upgrade or not.

What is the right way forward here? I have a few suggestions:

1. Ignore the issue for now, and just live with 1.1 and 1.10 being treated as the same major version. Would cause e.g. confusing branch names.
2. I can think of a workaround: Always pad the B in A.B.C to e.g. 5 digits. So 1.1 becomes 1.00001 and 1.10 becomes 1.00010. That way, the major versions are distinct. But it still causes confusing branch names, and it only works as long as IEEE precision limits don't affect us...
3. Maybe TypeScript supports a version of polymorphism where getMajor for each versioning can choose it's own comparable type? Since versions don't need to be compared across versioning systems, this seems sound to me. One advantage with something like this, if it is possible, is also that number is already comparable, so all the versioning schemes that use this type would already be compatible with such an interface definition. This is my preferred solution, but before spending time on it, I want to hear whether such a change would be reasonable to you, @secustor ? (and anyone else reading along)

[secustor]

I would like to avoid introducing generics for versionings for such changes if it is for a single use case.

It would be hacky but as we use the getMajor only for comparison. It would also be possible to use something like this.
That solution assumes the second digit of the major will be <1000 at all times.

const [firstMajor, secondMajor] = '1.1'.split('.').map(Number)
const major = firstMajor * 1000 + secondMajor

@viceice What are you thoughts?
The only other alternative would be to move this behind the versionings APIs and e.g. give the compare function additional parameters to check only specific parts of the version.

[viceice]

I think I would add a new optional versioning API function

function isSame(type: 'major'|'minor'|'patch', a: string, b: string): boolean;

if it's available, use it otherwise fall back to existing comparison.

[secustor]

K, create a separate PR which implements the API and handles the optional comparison.

[ysangkok]

Ok, PR submitted: #32207

[ysangkok]

Submitted PR to add what was discussed here:

• feat(versioning): add PVP versioning scheme #32298