You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I have felt for quite a long time that semver ranges like ^x.y.z or ~x.y.z are an antipattern, because they obscure what the actual bounds are, and they encourage you to tighten lower bounds prematurely. For instance, lots of JS tools push you towards using a caret range with whatever version you happened to be using when you were writing the code. Of course, it's a reasonable default at first if you're adding a dependency at version x.y.z to declare the bounds as >=x.y.z <(x+1).0.0 (note: this is what ^x.y.z means as long as x >= 1), but if you later make a change to support newer versions of that dependency, you often won't need to tighten the lower bound at the same time. That is, if I've written ^x.y.z and I want to allow my package to be used with a new version (x+1).0.0 of the same dependency, caret ranges encourage me to just write ^(x+1).0.0, which means that my package can no longer be used with any x.* version. Instead, if I was forced to write >=x.y.z <(x+1).0.0 to begin with, it's easy to see how to relax the upper bound without tightening the lower bound.
To be explicit, my proposal is that the only acceptable version range operators are >, >=, <, <=, and ==.
Note that the caret or tilde ranges aren't part of the semver spec; as far as I'm aware, they were first introduced by the node semver package. Elm does this already (and in fact goes a little further), by requiring that all version ranges are of the form a.b.c <= v < d.e.f.
Reacted by Thomas Honeyman, John Watson, Fabrizio Ferrai, Alex and Pete Murphy
Great point. I would totally go as far as Elm does. I have only one question about this: how should we import packages from Bower? Should we convert the caret range to the explicit range?
I think so, yes. I guess the alternative is allowing existing packages to use carets and at some point disallowing new uploads from using them? I don’t have a strong preference but I feel like since we are generating the manifest files for existing versions in an automated manner anyway, we might as well convert them during the initial import?
Yes, I agree that in this case we have the option of converting everything, so we should just go for that.
However there is always the questions of: do we remove packages/versions if we cannot convert them? (in this initial import)
E.g. while trying to mass-convert all Bower packages the first time I found out that ~20 packages were just unreachable (I assume because the owners just took the repos down). In this case taking the decision to just remove them from the index is straightforward, but there are trickier cases, e.g. when it comes to the name-constraints (I didn't check yet if any package would fail the constraints we defined in #3)
Intuitively I'd be fine just removing non-compliant stuff that we cannot convert in any way, but I guess it's good to keep somewhere a list of "things that were removed due to $reason"
I may be wrong, but I think it should always be possible to rewrite any version range which bower understands in a format which would be acceptable according to this proposal.
What about attempting to adjust a package to the simple bounds, and dropping it / opening a PR to update it if that can’t be done? I’m happy to take on opening PRs to packages that have to be updated so their version bounds work.
I may be wrong, but I think it should always be possible to rewrite any version range which bower understands in a format which would be acceptable according to this proposal.
@hdgarrood I agree with that, in my previous comment I am asking what we should do in the general case.
@thomashoneyman the problem in this case is that published versions are considered immutable, so there's no "update" operation that can be performed, we have to say either "yes" or "no".
For future packages/versions we can just have CI reject packages if the ranges don't fit our constraints, so there shouldn't be the need for us to do updates to repos
Ah right, I see. I don't mind losing a few, but if we find we are rejecting lots of packages which are still in use, then we should probably reconsider. Perhaps if need be we could apply more relaxed rules to package versions which existed before the initial import?
we cannot get a "parse tree" or even answer the question "what kind of range is this?", so we'd have to have a custom parser to detect "simple ranges" or something like that
At this point I am not sure anymore if it's worth adding this constraint at all vs just issuing a recommendation client-side in the publishing flow. This is because in the latter case we don't have to be accurate since it's just a recommendation, while in the CI here we need to be 100% correct so that everything can be automated
Issuing a recommendation client-side sounds fine to me! However I don't think it's true that it's impossible to convert a caret range to a simple >=a.b.c <d.e.f range. The node-semver package readme does give some examples, even:
I suspect the question being asked in the issue you linked, which is impossible to answer, is "given a caret range, what's the maximum version that is within the range?" This is not defined because caret ranges always have a strict upper bound such as <2.0.0, i.e. an upper bound that uses < rather than <=. If you have a strict upper bound, say <2.0.0, you can always add one to the patch version, i.e. 1.x.y -> 1.x.(y+1), so there's no maximum version which satisfies that bound. That's fine though - we don't need to answer the question of "what is the maximum version within the range," because we can just sidestep the issue by providing a strict upper bound.
Currently we convert the ranges when importing from Bower, I'll leave this issue open to track the fact that we should error out if the ranges in the Manifest are not simple.
There is a widespread convention of depending on any version of a package, up to the next major version (or up to the next minor version).
I worry about these simple ranges being problematic in that folks who would usually write ^1.0.0 or ^1.3.4 will start to write >1.0.0 <2.0.0, which can opt them in to pre-releases (ie. that range will include 2.0.0-rc.1 -- Halogen's release candidates come to mind -- but not the actual 2.0.0 release).
They really need to write >1.0.0 <2.0.0-0, but this isn't obvious and I'm going to predict that most people won't do it. I suggest that we take one of these courses of action:
We support < <= == >= >, as @hdgarrood suggested, and we also support ^ because of how widely it's used
We only support simple ranges, but we implicitly treat the end version as the actual end version (ie. < 2.0.0 is considered to actually mean < 2.0.0-0).
We don't allow you to publish versions with build metadata / release candidate information to the registry. If you want to use one of those packages you'll have to point to it directly, not via the registry.
Elm inspired the original discussion here, and I'm fairly sure they don't even allow things like build or pre-release information to be in their version ranges, which means they can do < 2.0.0 with no worries -- it really will restrict to the 1.x.x version series.
I think option (3) is not viable in the first place, because we are importing many packages that have pre-release versions.
I don't have an opinion yet on what's the best course of action here, but I wonder: do we have to do anything in the first place about this, i.e. is it such a big issue?
There are two scenarios where this can happen:
a package with overly lax bounds is published
an application has overly lax bounds on a package
For case (1) we could simply restrict the bounds (either with authors publishing a new version, or with Trustees fixing the bounds for old packages), and for case (2) the application developers can just fix the bounds locally as soon as they notice the breakage.
I think option (3) is not viable in the first place, because we are importing many packages that have pre-release versions.
For option 3 to be viable we would drop packages with prerelease versions. Perhaps that's not acceptable! (You could still depend on those packages by pointing at their Git url, those specific versions just wouldn't be in the registry.)
I wonder: do we have to do anything in the first place about this, i.e. is it such a big issue?
Something feels wrong to me that the obvious bounds to write (ie. >=2.0.0 <3.0.0) are incorrect, and so the design of these version bounds will lead people to write incorrect bounds. Overly lax bounds can be fixed, but ideally we'd avoid the issue by not making it so easy to write subtly incorrect bounds in the first place.
...but perhaps in practice this is rare enough that we don't worry about it. I don't have good data on this.
Yeah, I meant to add a check to the API pipeline to only allow only simple ranges when adding new packages (but not when importing legacy ones) - this is the reason why we had Range originally defined as { original :: String, converted :: String }, setup that was removed in #229 because we forgot what it was for 😂
This check should just verify that original == converted (since the converted ranges are always simple)
Uh, I just realized that if we introduce the check mentioned in the comment above - i.e. verifying that all the ranges in a manifest are the same as the ranges that parseRange returns us, something like >=1.3.4 <2.0.0-0 - then there's no point in removing RC releases, since they don't fit in such range (as you noted above):
Indeed in all of this I am assuming that the only range we'd accept is in the form of >=A.B.C <X.Y.Z-0.
I'd say this is fine because:
we'd check this client side when publishing a package, and clearly communicate to users that their ranges need to be in that form
introducing a different acceptance criteria would require us to reimplement parseRange from scratch, instead of reusing the js implementation, which is something that I'm not really keen on
This is my proposed specification for accepted versions and ranges to start with. I can open a new issue if that would be better. If it generally looks good, then I'd like to open a PR enforcing this check in the registry and the legacy import tool.
Registry Versions & Version Ranges
The PureScript registry allows packages to specify a version for themselves and version ranges for their dependencies.
We use a restricted version of the SemVer spec which only allows versions with major, minor, and patch places (no build metadata or prerelease identifiers) and version ranges with the >= and < operators.
This decision keeps versions and version ranges easy to read, understand, and maintain over time.
Package Versions
Package versions always take the form X.Y.Z, representing major, minor, and patch places. All three places must be natural numbers. For example, in a manifest file:
{
"name": "my-package",
"version": "1.0.1"
}
If a package uses all three places (ie. it begins with a non-zero number, such as 1.0.0), then:
MAJOR means values have been changed or removed, and represents a breaking change to the package.
MINOR means values have been added, but existing values are unchanged.
PATCH means the API is unchanged and there is no risk of breaking code.
If a package only uses two places (ie. it begins with a zero, such as 0.1.0), then:
MAJOR is unused because it is zero
MINOR means values have been changed or removed and represents a breaking change to the package
PATCH means values may have been added, but existing values are unchanged
If a package uses only one place (ie. it begins with two zeros, such as 0.0.1), then all changes are potentially breaking changes.
Version Ranges
Version ranges are always of the form >=X.Y.Z <X.Y.Z, where both versions must be valid and the first version must be less than the second version.
When comparing versions, the major place takes precedence, then the minor place, and then the patch place. For example:
1.0.0 is greater than 0.12.0
0.1.0 is greater than 0.0.12
0.0.1 is greater than 0.0.0
All dependencies must take this form. For example, in a manifest file:
Just a thought, once a library hits 1.0.0, "release candidate" releases could be implemented on top of this if people followed a convention. If all "odd" major numbers represent actual releases, then "even" major numbers could indicate a release candidate for the upcoming release.
1.0.0 - actual release
2.0.0 - release candidate for 3.0.0 (i.e. the real v2)
3.0.0 - actual release (i.e. the real v2)
Not sure if this is worthwhile to enforce because it would get confusing pretty quickly.
@JordanMartinez while such a scheme might be appealing to some folks, I would personally not like this to be enforced on my version numbers.
SemVer is designed in such a way that makes it easiest to spot breaking changes, and I feel that's what we should keep as our first priority. The scheme above blurs this, as it would force a breaking release even if there are no actual breaking changes.
@f-f Yeah, I don't disagree with you. While the above idea can be implemented, I think it's too confusing.I also agree that banning these 'fancy' ranges will remove a lot of ambiguity. Hence why it was "just a thought." 😁
Activity
f-f commentedon Jul 11, 2020
Great point. I would totally go as far as Elm does. I have only one question about this: how should we import packages from Bower? Should we convert the caret range to the explicit range?
hdgarrood commentedon Jul 11, 2020
I think so, yes. I guess the alternative is allowing existing packages to use carets and at some point disallowing new uploads from using them? I don’t have a strong preference but I feel like since we are generating the manifest files for existing versions in an automated manner anyway, we might as well convert them during the initial import?
f-f commentedon Jul 11, 2020
Yes, I agree that in this case we have the option of converting everything, so we should just go for that.
However there is always the questions of: do we remove packages/versions if we cannot convert them? (in this initial import)
E.g. while trying to mass-convert all Bower packages the first time I found out that ~20 packages were just unreachable (I assume because the owners just took the repos down). In this case taking the decision to just remove them from the index is straightforward, but there are trickier cases, e.g. when it comes to the name-constraints (I didn't check yet if any package would fail the constraints we defined in #3)
Intuitively I'd be fine just removing non-compliant stuff that we cannot convert in any way, but I guess it's good to keep somewhere a list of "things that were removed due to $reason"
hdgarrood commentedon Jul 11, 2020
I may be wrong, but I think it should always be possible to rewrite any version range which bower understands in a format which would be acceptable according to this proposal.
thomashoneyman commentedon Jul 11, 2020
What about attempting to adjust a package to the simple bounds, and dropping it / opening a PR to update it if that can’t be done? I’m happy to take on opening PRs to packages that have to be updated so their version bounds work.
f-f commentedon Jul 11, 2020
@hdgarrood I agree with that, in my previous comment I am asking what we should do in the general case.
f-f commentedon Jul 11, 2020
@thomashoneyman the problem in this case is that published versions are considered immutable, so there's no "update" operation that can be performed, we have to say either "yes" or "no".
For future packages/versions we can just have CI reject packages if the ranges don't fit our constraints, so there shouldn't be the need for us to do updates to repos
hdgarrood commentedon Jul 12, 2020
Ah right, I see. I don't mind losing a few, but if we find we are rejecting lots of packages which are still in use, then we should probably reconsider. Perhaps if need be we could apply more relaxed rules to package versions which existed before the initial import?
f-f commentedon Mar 8, 2021
Update: I'm looking at this and trying to implement something using the semver package.
It looks like:
At this point I am not sure anymore if it's worth adding this constraint at all vs just issuing a recommendation client-side in the publishing flow. This is because in the latter case we don't have to be accurate since it's just a recommendation, while in the CI here we need to be 100% correct so that everything can be automated
hdgarrood commentedon Mar 9, 2021
Issuing a recommendation client-side sounds fine to me! However I don't think it's true that it's impossible to convert a caret range to a simple
>=a.b.c <d.e.frange. The node-semver package readme does give some examples, even:https://github.com/npm/node-semver#caret-ranges-123-025-004
I suspect the question being asked in the issue you linked, which is impossible to answer, is "given a caret range, what's the maximum version that is within the range?" This is not defined because caret ranges always have a strict upper bound such as
<2.0.0, i.e. an upper bound that uses<rather than<=. If you have a strict upper bound, say<2.0.0, you can always add one to the patch version, i.e.1.x.y->1.x.(y+1), so there's no maximum version which satisfies that bound. That's fine though - we don't need to answer the question of "what is the maximum version within the range," because we can just sidestep the issue by providing a strict upper bound.f-f commentedon Apr 7, 2021
In #140 we found out that indeed it's possible to convert to "simple ranges" without implementing it ourselves, e.g. see an example conversion from the tests:
https://github.com/purescript/registry/blob/3d091b10d63350e8d374612e0eabbf825394cb40/ci/test/Main.purs#L140
Currently we convert the ranges when importing from Bower, I'll leave this issue open to track the fact that we should error out if the ranges in the Manifest are not simple.
thomashoneyman commentedon Oct 18, 2021
There is a widespread convention of depending on any version of a package, up to the next major version (or up to the next minor version).
I worry about these simple ranges being problematic in that folks who would usually write
^1.0.0or^1.3.4will start to write>1.0.0 <2.0.0, which can opt them in to pre-releases (ie. that range will include2.0.0-rc.1-- Halogen's release candidates come to mind -- but not the actual 2.0.0 release).They really need to write
>1.0.0 <2.0.0-0, but this isn't obvious and I'm going to predict that most people won't do it. I suggest that we take one of these courses of action:< <= == >= >, as @hdgarrood suggested, and we also support^because of how widely it's used< 2.0.0is considered to actually mean< 2.0.0-0).Elm inspired the original discussion here, and I'm fairly sure they don't even allow things like build or pre-release information to be in their version ranges, which means they can do
< 2.0.0with no worries -- it really will restrict to the1.x.xversion series.f-f commentedon Oct 20, 2021
I think option (3) is not viable in the first place, because we are importing many packages that have pre-release versions.
I don't have an opinion yet on what's the best course of action here, but I wonder: do we have to do anything in the first place about this, i.e. is it such a big issue?
There are two scenarios where this can happen:
For case (1) we could simply restrict the bounds (either with authors publishing a new version, or with Trustees fixing the bounds for old packages), and for case (2) the application developers can just fix the bounds locally as soon as they notice the breakage.
thomashoneyman commentedon Oct 20, 2021
For option 3 to be viable we would drop packages with prerelease versions. Perhaps that's not acceptable! (You could still depend on those packages by pointing at their Git url, those specific versions just wouldn't be in the registry.)
Something feels wrong to me that the obvious bounds to write (ie. >=2.0.0 <3.0.0) are incorrect, and so the design of these version bounds will lead people to write incorrect bounds. Overly lax bounds can be fixed, but ideally we'd avoid the issue by not making it so easy to write subtly incorrect bounds in the first place.
...but perhaps in practice this is rare enough that we don't worry about it. I don't have good data on this.
thomashoneyman commentedon Oct 21, 2021
As a quick note, we do already ban build metadata:
https://github.com/purescript/registry/blob/b1f4b5e0052ab1f9c01c081e0d83a9f2ec6f92a7/ci/src/Registry/API.purs#L262-L264
f-f commentedon Dec 1, 2021
Is this something that we'll need to resolve before shipping the first version?
f-f commentedon Dec 1, 2021
TODO:
thomashoneyman commentedon Jan 6, 2022
Community poll:
https://discourse.purescript.org/t/rfc-disallow-build-metadata-and-prerelease-identifiers-in-the-registry
We also need to spec what ranges are allowed as input, which I realize we haven’t done. For example, what does <1.0.0 ==1.2.3 mean? Is it legal?
f-f commentedon Jan 6, 2022
Yeah, I meant to add a check to the API pipeline to only allow only simple ranges when adding new packages (but not when importing legacy ones) - this is the reason why we had
Rangeoriginally defined as{ original :: String, converted :: String }, setup that was removed in #229 because we forgot what it was for 😂This check should just verify that
original == converted(since theconvertedranges are always simple)f-f commentedon Jan 9, 2022
Uh, I just realized that if we introduce the check mentioned in the comment above - i.e. verifying that all the ranges in a manifest are the same as the ranges that
parseRangereturns us, something like>=1.3.4 <2.0.0-0- then there's no point in removing RC releases, since they don't fit in such range (as you noted above):Indeed in all of this I am assuming that the only range we'd accept is in the form of
>=A.B.C <X.Y.Z-0.I'd say this is fine because:
parseRangefrom scratch, instead of reusing the js implementation, which is something that I'm not really keen onthomashoneyman commentedon Jan 19, 2022
This is my proposed specification for accepted versions and ranges to start with. I can open a new issue if that would be better. If it generally looks good, then I'd like to open a PR enforcing this check in the registry and the legacy import tool.
Registry Versions & Version Ranges
The PureScript registry allows packages to specify a version for themselves and version ranges for their dependencies.
We use a restricted version of the SemVer spec which only allows versions with major, minor, and patch places (no build metadata or prerelease identifiers) and version ranges with the
>=and<operators.This decision keeps versions and version ranges easy to read, understand, and maintain over time.
Package Versions
Package versions always take the form
X.Y.Z, representing major, minor, and patch places. All three places must be natural numbers. For example, in a manifest file:{ "name": "my-package", "version": "1.0.1" }If a package uses all three places (ie. it begins with a non-zero number, such as
1.0.0), then:MAJORmeans values have been changed or removed, and represents a breaking change to the package.MINORmeans values have been added, but existing values are unchanged.PATCHmeans the API is unchanged and there is no risk of breaking code.If a package only uses two places (ie. it begins with a zero, such as
0.1.0), then:MAJORis unused because it is zeroMINORmeans values have been changed or removed and represents a breaking change to the packagePATCHmeans values may have been added, but existing values are unchangedIf a package uses only one place (ie. it begins with two zeros, such as
0.0.1), then all changes are potentially breaking changes.Version Ranges
Version ranges are always of the form
>=X.Y.Z <X.Y.Z, where both versions must be valid and the first version must be less than the second version.When comparing versions, the major place takes precedence, then the minor place, and then the patch place. For example:
1.0.0is greater than0.12.00.1.0is greater than0.0.120.0.1is greater than0.0.0All dependencies must take this form. For example, in a manifest file:
{ "name": "my-package", "version": "1.0.1", "targets": { "lib": { "my-dependency": ">=1.0.0 <2.0.0", "my-dependency-2": ">=2.1.5 <2.1.6", "my-dependency-3": ">=0.1.0 <12.19.124" }, } }JordanMartinez commentedon Jan 19, 2022
Just a thought, once a library hits 1.0.0, "release candidate" releases could be implemented on top of this if people followed a convention. If all "odd" major numbers represent actual releases, then "even" major numbers could indicate a release candidate for the upcoming release.
1.0.0- actual release2.0.0- release candidate for3.0.0(i.e. the realv2)3.0.0- actual release (i.e. the realv2)Not sure if this is worthwhile to enforce because it would get confusing pretty quickly.
f-f commentedon Jan 19, 2022
@JordanMartinez while such a scheme might be appealing to some folks, I would personally not like this to be enforced on my version numbers.
SemVer is designed in such a way that makes it easiest to spot breaking changes, and I feel that's what we should keep as our first priority. The scheme above blurs this, as it would force a breaking release even if there are no actual breaking changes.
JordanMartinez commentedon Jan 19, 2022
@f-f Yeah, I don't disagree with you. While the above idea can be implemented, I think it's too confusing.I also agree that banning these 'fancy' ranges will remove a lot of ambiguity. Hence why it was "just a thought." 😁