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

Respect "use for snapshots" and "use for prereleases" #91

Open
isc-tleavitt opened this issue Dec 9, 2022 · 13 comments · Fixed by #112
Open

Respect "use for snapshots" and "use for prereleases" #91

isc-tleavitt opened this issue Dec 9, 2022 · 13 comments · Fixed by #112

Comments

@isc-tleavitt
Copy link
Contributor

Not sure if this is here, zpm, or both, but I suspect it's here.

If I disable "Use for Snapshots" and/or "Use for Prereleases" - e.g.:

zpm "repo -n registry -r -s 0 -pre 0"

Snapshot and prerelease versions are still listed (zpm "repo -list-modules") and installed (via zpm "install").

@daimor
Copy link
Member

daimor commented Dec 9, 2022

I'm pretty sure it was not implemented, at all, for the registry. And I think, best way to implement it, on both sides

@isc-tleavitt
Copy link
Contributor Author

I'm hoping to carve out some more time this month for open source projects - can possibly contribute on both ends.

@isc-shuliu
Copy link
Collaborator

@isc-tleavitt
The code here seems to suggest a semantic versioning template of major.minor.patch+undefined-prerelease+metadata. The part between patch+ and -prerelease is undefined.

Property versionMajor As %Integer(%JSONINCLUDE = "NONE") [ SqlComputeCode = {Set {*} = +$Piece({version},".",1)}, SqlComputed, SqlComputeOnChange = %%INSERT ];
Property versionMinor As %Integer(%JSONINCLUDE = "NONE") [ SqlComputeCode = {Set {*} = +$Piece({version},".",2)}, SqlComputed, SqlComputeOnChange = %%INSERT ];
Property versionPatch As %Integer(%JSONINCLUDE = "NONE") [ SqlComputeCode = {Set {*} = +$Piece($Piece($Piece({version},".",3),"-"),"+")}, SqlComputed, SqlComputeOnChange = %%INSERT ];
Property versionPrerelease As %String(%JSONINCLUDE = "NONE") [ SqlComputeCode = {Set {*} = $Piece($Piece($Piece({version},".",3,*),"-",2,*),"+")}, SqlComputed, SqlComputeOnChange = %%INSERT ];
Property versionBuildmetadata As %String(%JSONINCLUDE = "NONE") [ SqlComputeCode = {Set {*} = $Piece({version},"+",2)}, SqlComputed, SqlComputeOnChange = %%INSERT ];

Any chance that we actually want this part to be SNAPSHOT and if so, should we add that as an additional property?

@isc-tleavitt
Copy link
Contributor Author

@isc-shuliu the Semantic Versioning spec places prerelease designations before build metadata, so a semver that has + before - is invalid.

@isc-tleavitt
Copy link
Contributor Author

I think this is time to define snapshots correctly: if versionPrerelease is SNAPSHOT, it's a snapshot. if versionPrerelease is something else non-null, it's a prerelease.

@isc-tleavitt
Copy link
Contributor Author

Previously we've had some use of build metadata "+snapshot" treated specially, but if we're adding new repo-level support around snapshots this is the way to go.

@isc-shuliu
Copy link
Collaborator

isc-shuliu commented Aug 27, 2024

@isc-tleavitt
Just to confirm, since we are using "SNAPSHOT" the same way as other prerelease tags, does that mean we consider version expression "1.x" to be satisfiable by "2.0.0-SNAPSHOT"?

Set version = ##class(%IPM.General.SemanticVersion).FromString("2.0.0-SNAPSHOT")
Do ##class(%IPM.General.SemanticVersionExpression).FromString("1.x", .expression)
Write version.Satisfies(expression)  
// Output: 1

@isc-tleavitt
Copy link
Contributor Author

@isc-shuliu that feels like a bug on the IPM end.

@isc-shuliu
Copy link
Collaborator

In IPM.General.SemanticVersionExpression.Comparator:Evaluate() it calls IPM.General.SemanticVersion:Follows, where it is documented that

1.0.0-alpha < 1.0.0-alpha.1 < ... < 1.0.0-rc.1 < 1.0.0

It appears this is expected behavior

@isc-shuliu
Copy link
Collaborator

In IPM.General.SemanticVersionExpression.Comparator:Evaluate() it calls IPM.General.SemanticVersion:Follows, where it is documented that

1.0.0-alpha < 1.0.0-alpha.1 < ... < 1.0.0-rc.1 < 1.0.0

It appears this is expected behavior

Assuming we consider 1.x to be equivalent to >=1.0.0 AND <2.0.0

@isc-tleavitt
Copy link
Contributor Author

That's what 1.x means, but this actually boils down to some meaningful changes in the node-semver spec we used as an original basis; see: npm/node-semver@059a5ad.

Need a fix in IPM for this - I'll create an issue there. It's a good catch.

@isc-shuliu
Copy link
Collaborator

@isc-tleavitt
Can you give me write access so that I can push and create a PR?

@isc-tleavitt
Copy link
Contributor Author

Just did but it looks like you figured it out faster than I could.

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 a pull request may close this issue.

3 participants