-
Notifications
You must be signed in to change notification settings - Fork 8
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
Better support fo module version format #54
Comments
Alternatively we could tighten the semantics on BCR itself to reject versions that don't follow the semver spec. |
Ahh, dang. Yeah, it should have a |
https://bazel.build/rules/lib/bazel_module#version doesn't say it has to be semver btw (though I did intend for it to be semver). Also no mention of semver here: https://bazel.build/external/overview#bzlmod. |
According to this comment, the format is specifed in-code like this: https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Version.java That leads me to believe that there is no other place where this is formalized right now.
With that, |
When I release 1.0.0, will it be "newer" than 1.0.0rc1? If not, how can I fix this now? |
I'd just run https://www.npmjs.com/package/semver-compare-cli via npx to answer that experimentally. Do we think Bazel is correct here? Feels to me like we'll hit lots of other places where people's semver parsers will break and so BCR/bzlmod should be stricter about what it accepts. |
The version format is specified here: https://bazel.build/external/module#version_format "1.0.0rc1" is newer than "1.0.0" because "0rc1" > "0" per https://semver.org/#spec-item-11, sub-item 4 |
Good to know. I think this might catch others as well. I'll work around my own mistake though. |
This is quite an unfortunate situation :S https://bazel.build/rules/lib/bazel_module#version is the documentation for the The actual documentation for the |
Just to confirm, |
There it also doesn't say anything about version format. |
No; the comparison is lexicographical regarding dot-delimited sections, so EDIT: This is (partially) wrong: |
Indeed, I'll fix that ASAP. |
It just occurred to me that we could mitigate the confusion by enforcing that the version list in metadata.json is sorted. That way, if a PR adds a new version and it's not at the end, you'll immediately see something is wrong. |
It looks like @cgrindel has implemented proper version support in renovatebot/renovate#21606 if I'm not mistaken. Will have to see if/how we could also use that here (and maybe whether we can extract that in a separate repo, so that all JS/TS projects that need Bazel module version support can take advandtage of it)? |
Got bored and wanted to explore Typescript. Tested on https://typescriptlang.org/play: > console.log(sortVersions(['1.2.3', '1.2.3-d', '1.2.3.bcr.1', '1.2.3rc3', '1', 'k'])) ["k", "1.2.3rc3", "1.2.3.bcr.1", "1.2.3", "1.2.3-d", "1"] Addresses #54. Fixes #142.
* Implement proper version sorting Got bored and wanted to explore Typescript. Tested on https://typescriptlang.org/play: > console.log(sortVersions(['1.2.3', '1.2.3-d', '1.2.3.bcr.1', '1.2.3rc3', '1', 'k'])) ["k", "1.2.3rc3", "1.2.3.bcr.1", "1.2.3", "1.2.3-d", "1"] Addresses #54. Fixes #142. * fix formatting * more formatting * fiiiiiinnnne i'll install pnpm --------- Co-authored-by: Yun Peng <pcloudy@google.com>
bazelbuild/bazel-central-registry#404 broke the registry.bazel.build site, because our deploy action has been broken: https://github.com/bazel-contrib/bcr-ui/actions/runs/4055858424
I guess we have to patch this dependency to understand more semver formats, since rules_xcodeproj isn't following the spec https://semver.org/#semantic-versioning-specification-semver
/cc @brentleyjones
The text was updated successfully, but these errors were encountered: