feat: PVP Version implementation for the Stack build tool (Haskell) #13224

psilospore · 2021-12-21T22:00:28Z

Updates

I asked for clarifications on handling ranges on the renovate slack however I believe @rarkins is on vacation so this might be paused for a while.

Changes:

Added a pvp implementation in lib/versioning/

Context:

This adds a PVP implementation that can be used to integrate Stack support for Renovate.
@rarkins suggested approaching the implementation in 4 parts.
See this issue: #8187

Caribou uses Renovate and it's been great for our typescript repos, and it's been great. So thanks for creating this product! We would love to have support for our Haskell repos, and it would be great for the Haskell community in general.

TODOs

In the original issue they mention we only need to handle specific versions and not ranges, and that would also make the implementation easier. However, given that we were unsure how to implement some of the range-related functions.

How would do we run the tests?
I attempted npm run test but I get the following error:

> [email protected] git-check /Users/psilospore/develop/renovate
> node tools/check-git-version.mjs

(node:47255) ExperimentalWarning: The ESM module loader is experimental.
ERROR: Minimum Git version 2.33.0 is required
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] git-check: `node tools/check-git-version.mjs`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] git-check script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/psilospore/.npm/_logs/2021-12-21T21_54_31_380Z-debug.log
ERROR: "git-check" exited with 1.

Documentation (please check one with an [x])

I have updated the documentation, or
No documentation update is required
I'm not sure if I need to update documentation yet until we've done the rest of the parts

How I've tested my work (please tick one)

I have verified these changes via:

Code inspection only, or
Newly added/modified unit tests, or
No unit tests but ran on a real repository, or
Both unit tests + ran on a real repository

Co-authored-by: JasonMFry <[email protected]>

CLAassistant · 2021-12-21T22:00:35Z

All committers have signed the CLA.

viceice · 2021-12-21T22:07:04Z

Your git version is outdated, you need to update or use one of our docker images to run tests

viceice · 2021-12-21T22:08:41Z

lib/versioning/pvp/index.ts

+export const supportsRanges = true;
+
+// https://pvp.haskell.org/#version-numbers
+type PVP = {


You should use our generic versioning class as base for this, as it will provide most of your implemention.

Hi, thanks for reviewing! Could you point out the generic versioning class? When I look at the other implementations I don't see any of them using a generic versioning class.

Ruby for instance takes a similar approach:

interface RubyVersion { major: number; minor: number; patch: number; prerelease: string[] | null; }

Other ones seem to use the npm implementation if they use semver or pep440.

helm:

export const api: VersioningApi = { ...npm, }

rez:

function getMajor(version: string): number { try { return npm.getMajor(padZeroes(version)); } catch (err) /* istanbul ignore next */ { return pep440.getMajor(version); }

Note those other implementations would be incompatible with PVP.

Maybe @viceice meant using this https://github.com/renovatebot/renovate/blob/ee79cf7975a8cdda4952e38dc4d7d5676e350ec2/lib/versioning/loose/generic.ts ?

yes, this:

renovate/lib/versioning/loose/generic.ts

Line 131 in 68dfc27

export abstract class GenericVersioningApi<

the stuff before is obsolete and will be removed soon

Thanks for pointing that out. I'll try to refactor with that class now. That file also seems to calify our questions on ranges:

renovate/lib/versioning/loose/generic.ts

Lines 18 to 20 in ee79cf7

// since this file was meant for no range support, a range = version

// parse should return null if version not valid

// parse should return an object with property release, an array of version sections major.minor.patch

I have updated my PR to use the Generic versioning class now.

lib/versioning/pvp/index.ts

lib/versioning/api.ts

Co-authored-by: HonkingGoose <[email protected]>

…askell-stack

lib/versioning/pvp/readme.md

HonkingGoose · 2022-01-26T09:05:49Z

lib/versioning/pvp/readme.md

+The one unusual difference between PVP and semver is that there are 2 major versions, and that
+there could be additional version numbers past patch.


Suggested change

The one unusual difference between PVP and semver is that there are 2 major versions, and that

there could be additional version numbers past patch.

The one unusual difference between PVP and SemVer is that there are two major versions, and that there could be additional version numbers past _patch_.

This should go on one line (one sentence per line pattern).
SemVer is a specific term/name, so should be capitalized. 😉

Are you sure you that patch is the correct term? According to the Haskell Package Versioning Policy, c is called minor.

SemVer is a specific term/name, so should be capitalized. 😉

Sorry I have terrible grammar. Thanks for the suggestions!

Are you sure you that patch is the correct term? According to the Haskell Package Versioning Policy, c is called minor.

I've updated the instructions. Let me know if that's better.

lib/versioning/pvp/readme.md

Co-authored-by: HonkingGoose <[email protected]>

lib/versioning/pvp/readme.md

Co-authored-by: HonkingGoose <[email protected]>

lib/versioning/pvp/readme.md

lib/versioning/pvp/index.ts

Co-authored-by: Michael Kriese <[email protected]> Co-authored-by: HonkingGoose <[email protected]>

…askell-stack

lib/versioning/pvp/index.spec.ts

viceice · 2022-02-17T17:09:24Z

lib/versioning/pvp/index.spec.ts

+    ${'3.0.0.beta'}              | ${false}
+    ${'5.1.2-+'}                 | ${false}
+  `('isVersion("$input") === $expected', ({ input, expected }) => {
+    const res = !!pvp.isVersion(input);


lib/versioning/pvp/index.ts

viceice · 2022-02-17T17:13:15Z

lib/versioning/pvp/index.ts

+
+  /**
+   * Compare similar to GenericVersioningApi._compare implementation
+   * except 2.1.1.0 and 2.1.1 are not equivalent instead 2.1.1.0 > 2.1.1


Isn't this now fixed in base class, so we can remove this overload?

HonkingGoose

We can probably use h2's here, this also creates "anchors/links", that you can use to link to a specific section of this document.

lib/versioning/pvp/readme.md

Co-authored-by: Michael Kriese <[email protected]> Co-authored-by: HonkingGoose <[email protected]>

HonkingGoose · 2022-02-22T11:12:13Z

lib/versioning/pvp/readme.md

+Quotes from the [Haskell PVP Specification](https://pvp.haskell.org/):
+
+> A package version number **SHOULD** have the form A.B.C, and **MAY** optionally have any number of additional components, for example 2.1.0.4 (in this case, A=2, B=1, C=0).
+> A.B is known as the _major_ version number, and C the _minor_ version number.


Suggested change

> A.B is known as the _major_ version number, and C the _minor_ version number.

> A.B is known as the _major_ version number, and C the _minor_ version number.

These are separate quotes in the upstream docs, so we should put a newline between them.

HonkingGoose · 2022-02-22T11:14:51Z

lib/versioning/pvp/readme.md

+> A package version number **SHOULD** have the form A.B.C, and **MAY** optionally have any number of additional components, for example 2.1.0.4 (in this case, A=2, B=1, C=0).
+> A.B is known as the _major_ version number, and C the _minor_ version number.
+
+The one unusual difference between PVP and SemVer is that there are two major versions, and that there can be an optional number of additional version numbers past _minor_.


Suggested change

The one unusual difference between PVP and SemVer is that there are two major versions, and that there can be an optional number of additional version numbers past _minor_.

The main differences between PVP and SemVer:

- PVP allows _two_ major versions

- PVP allows an optional number of extra version numbers past _minor_

HonkingGoose · 2022-02-22T11:17:55Z

lib/versioning/pvp/readme.md

+> A.B is known as the _major_ version number, and C the _minor_ version number.
+
+The one unusual difference between PVP and SemVer is that there are two major versions, and that there can be an optional number of additional version numbers past _minor_.
+For example `1.2.3` (note there's no _patch_ version here), `1.2.3.4` (`4` is the _patch_ version), or `1.2.3.4.5.6.7` are all valid version numbers.


Suggested change

For example `1.2.3` (note there's no _patch_ version here), `1.2.3.4` (`4` is the _patch_ version), or `1.2.3.4.5.6.7` are all valid version numbers.

| PVP version | Note |

| --- | --- |

| `1.2.3` | No _patch_ version |

| `1.2.3.4` | `4` is the _patch_ version |

| `1.2.3.4.5.6.7` | This is also a valid version |

We can try a table here. You'll need to run yarn prettier-fix after applying the suggestion because it's not formatted the way Prettier wants.

HonkingGoose · 2022-02-22T11:19:46Z

lib/versioning/pvp/readme.md

+
+## Are ranges supported?
+
+Currently this is not supported but you can have ranges.


The h2 asks: Are ranges supported?
And our answer is: this is not supported, but right after: but you can have ranges.

This is really confusing me. I think there's some kind of difference between Cabal and Stack?

HonkingGoose · 2022-02-22T11:23:06Z

lib/versioning/pvp/readme.md

+## Are ranges supported?
+
+Currently this is not supported but you can have ranges.
+This implementation just supports updating extra-deps in the Stack build tool which does not support ranges.


Suggested change

This implementation just supports updating extra-deps in the Stack build tool which does not support ranges.

Our implementation supports updating extra-deps in the Stack build tool which does not support ranges.

We can probably say "our implementation" and get rid of the word "just". 😉

HonkingGoose · 2022-02-22T11:23:47Z

lib/versioning/pvp/readme.md

+
+Currently this is not supported but you can have ranges.
+This implementation just supports updating extra-deps in the Stack build tool which does not support ranges.
+If this is used for Cabal then it would be useful to support ranges.


This sentence doesn't really explain if I can use ranges with Cabal or not currently?

viceice · 2022-04-09T06:12:14Z

lib/versioning/pvp/index.spec.ts

+    ${'renovatebot/renovate#main'}                   | ${false}
+    ${'https://github.com/renovatebot/renovate.git'} | ${false}
+  `('isValid("$version") === $isValid', ({ version, isValid }) => {
+    const res = pvp.isValid(version);


please inline

viceice · 2022-04-09T06:12:26Z

lib/versioning/pvp/index.spec.ts

+    ${'3.0.0.beta'}              | ${false}
+    ${'5.1.2-+'}                 | ${false}
+  `('isVersion("$input") === $expected', ({ input, expected }) => {
+    const res = !!pvp.isVersion(input);


viceice · 2022-04-09T06:14:03Z

lib/versioning/pvp/index.ts

+    return { release: [major, ...rest] };
+  }
+
+  override getMajor(version: string): number | null {


why you need to override this?

viceice · 2022-04-09T06:14:26Z

lib/versioning/pvp/index.ts

+
+  /**
+   * Compare similar to GenericVersioningApi._compare implementation
+   * except 2.1.1.0 and 2.1.1 are not equivalent instead 2.1.1.0 > 2.1.1


rarkins · 2022-10-19T04:51:17Z

Closing due to staleness

psilospore and others added 2 commits December 21, 2021 16:54

feat: Initial attempt at PVP implementation

3f66759

Co-authored-by: JasonMFry <[email protected]>

Initial sub at test

ee79cf7

Co-authored-by: JasonMFry <[email protected]>

viceice reviewed Dec 21, 2021

View reviewed changes

HonkingGoose reviewed Dec 21, 2021

View reviewed changes

lib/versioning/pvp/index.ts Outdated Show resolved Hide resolved

tillydray mentioned this pull request Dec 22, 2021

Call for Participation: help add Haskell support to Renovate haskellweekly/haskellweekly#112

Closed

psilospore added 4 commits January 16, 2022 12:28

Merge remote-tracking branch 'upstream/main' into haskell-stack

e0ca9a3

Refactored to use GenericVersioningApi

50f5f46

Update pvp to use regex

ca70458

Update test case

f587061

psilospore requested review from viceice and HonkingGoose January 20, 2022 01:26

psilospore changed the title ~~feat: PVP implementation for the Stack build tool (Haskell)~~ feat: PVP Version implementation for the Stack build tool (Haskell) Jan 20, 2022

Merge branch 'main' into haskell-stack

cc7d212

HonkingGoose reviewed Jan 20, 2022

View reviewed changes

psilospore and others added 3 commits January 20, 2022 18:18

Apply suggestions from code review

8f68b38

Co-authored-by: HonkingGoose <[email protected]>

Add documentation for PVP

0d5147e

Merge branch 'haskell-stack' of github.com:psilospore/renovate into h…

d4f4f4b

…askell-stack

HonkingGoose suggested changes Jan 26, 2022

View reviewed changes

psilospore and others added 2 commits January 26, 2022 15:19

Apply suggestions from code review

f16c946

Co-authored-by: HonkingGoose <[email protected]>

Clarify optional additional components

f9cd49a

HonkingGoose suggested changes Jan 27, 2022

View reviewed changes

lib/versioning/pvp/readme.md Outdated Show resolved Hide resolved

Update lib/versioning/pvp/readme.md

d99e888

Co-authored-by: HonkingGoose <[email protected]>

HonkingGoose reviewed Jan 28, 2022

View reviewed changes

lib/versioning/pvp/readme.md Outdated Show resolved Hide resolved

Merge branch 'main' into haskell-stack

a913697

viceice reviewed Jan 28, 2022

View reviewed changes

lib/versioning/pvp/index.ts Outdated Show resolved Hide resolved

psilospore and others added 3 commits January 28, 2022 11:26

Apply suggestions from code review

a582f6f

Co-authored-by: Michael Kriese <[email protected]> Co-authored-by: HonkingGoose <[email protected]>

fix: Added additional test coverage, return correct major version

fbd784d

Merge branch 'main' into haskell-stack

53e4336

psilospore marked this pull request as ready for review February 10, 2022 00:17

rarkins and others added 3 commits February 15, 2022 17:17

Merge branch 'main' into haskell-stack

41131a2

fix: lint markdown and use import type

f4ec4f3

Merge branch 'haskell-stack' of github.com:psilospore/renovate into h…

83d46c6

…askell-stack

psilospore requested review from HonkingGoose and viceice February 17, 2022 13:57

Merge branch 'main' into haskell-stack

971a3f8

viceice reviewed Feb 17, 2022

View reviewed changes

HonkingGoose suggested changes Feb 19, 2022

View reviewed changes

lib/versioning/pvp/readme.md Outdated Show resolved Hide resolved

lib/versioning/pvp/readme.md Outdated Show resolved Hide resolved

lib/versioning/pvp/readme.md Outdated Show resolved Hide resolved

psilospore and others added 2 commits February 21, 2022 13:01

Apply suggestions from code review

db52252

Co-authored-by: Michael Kriese <[email protected]> Co-authored-by: HonkingGoose <[email protected]>

Merge branch 'main' into haskell-stack

d432f06

psilospore requested review from viceice and HonkingGoose February 21, 2022 18:06

Merge branch 'main' into haskell-stack

5401d90

HonkingGoose reviewed Feb 22, 2022

View reviewed changes

Merge branch 'main' into haskell-stack

78636d4

rarkins marked this pull request as draft March 25, 2022 10:23

viceice requested changes Apr 9, 2022

View reviewed changes

rarkins closed this Oct 19, 2022

github-actions bot locked as resolved and limited conversation to collaborators Nov 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: PVP Version implementation for the Stack build tool (Haskell) #13224

feat: PVP Version implementation for the Stack build tool (Haskell) #13224

psilospore commented Dec 21, 2021 •

edited

Loading

CLAassistant commented Dec 21, 2021 •

edited

Loading

viceice commented Dec 21, 2021

viceice Dec 21, 2021

psilospore Dec 27, 2021

jmatsushita Jan 13, 2022

viceice Jan 14, 2022

psilospore Jan 16, 2022

psilospore Jan 20, 2022

HonkingGoose Jan 26, 2022

psilospore Jan 26, 2022 •

edited

Loading

viceice Feb 17, 2022

viceice Apr 9, 2022

viceice Feb 17, 2022

viceice Apr 9, 2022

HonkingGoose left a comment

HonkingGoose Feb 22, 2022

HonkingGoose Feb 22, 2022

HonkingGoose Feb 22, 2022

HonkingGoose Feb 22, 2022

HonkingGoose Feb 22, 2022

HonkingGoose Feb 22, 2022

viceice Apr 9, 2022

viceice Apr 9, 2022

viceice Apr 9, 2022

viceice Apr 9, 2022

rarkins commented Oct 19, 2022

	// since this file was meant for no range support, a range = version
	// parse should return null if version not valid
	// parse should return an object with property release, an array of version sections major.minor.patch

		The one unusual difference between PVP and semver is that there are 2 major versions, and that
		there could be additional version numbers past patch.

	> A.B is known as the _major_ version number, and C the _minor_ version number.

	> A.B is known as the _major_ version number, and C the _minor_ version number.

-The one unusual difference between PVP and SemVer is that there are two major versions, and that there can be an optional number of additional version numbers past _minor_.
+The main differences between PVP and SemVer:
+- PVP allows _two_ major versions
+- PVP allows an optional number of extra version numbers past _minor_

-For example `1.2.3` (note there's no _patch_ version here), `1.2.3.4` (`4` is the _patch_ version), or `1.2.3.4.5.6.7` are all valid version numbers.
+| PVP version | Note |
+| --- | --- |
+| `1.2.3` | No _patch_ version |
+| `1.2.3.4` | `4` is the _patch_ version |
+| `1.2.3.4.5.6.7` | This is also a valid version |


		## Are ranges supported?

		Currently this is not supported but you can have ranges.

	This implementation just supports updating extra-deps in the Stack build tool which does not support ranges.
	Our implementation supports updating extra-deps in the Stack build tool which does not support ranges.

feat: PVP Version implementation for the Stack build tool (Haskell) #13224

feat: PVP Version implementation for the Stack build tool (Haskell) #13224

Conversation

psilospore commented Dec 21, 2021 • edited Loading

Updates

Changes:

Context:

TODOs

Documentation (please check one with an [x])

How I've tested my work (please tick one)

CLAassistant commented Dec 21, 2021 • edited Loading

viceice commented Dec 21, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

psilospore Jan 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HonkingGoose left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rarkins commented Oct 19, 2022

psilospore commented Dec 21, 2021 •

edited

Loading

CLAassistant commented Dec 21, 2021 •

edited

Loading

psilospore Jan 26, 2022 •

edited

Loading