intersystems-community / zpm-registry

registry server for zpm
MIT License
1 stars 7 forks source link

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

Open isc-tleavitt opened 1 year ago

isc-tleavitt commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

isc-shuliu commented 2 months ago

@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.

https://github.com/intersystems-community/zpm-registry/blob/f7e4087269338a97a5cfe5197e74f310d137e817/src/cls/ZPM/Package.cls#L52-L60

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

isc-tleavitt commented 2 months ago

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

isc-tleavitt commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

@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 commented 2 months ago

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

isc-shuliu commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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: https://github.com/npm/node-semver/commit/059a5adec5aefaa764bf4fdb717ae8b42cbbefcd.

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

isc-shuliu commented 2 months ago

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

isc-tleavitt commented 2 months ago

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