Create initial julia-version implementation
#3
Conversation
|
@kleinschmidt could you do an unofficial review? |
| ```bnf | ||
| <specifier> ::= <tilde> | <caret> | <partial> | <nightly> | <alias> | ||
| <tilde> ::= "~" <partial> | ||
| <caret> ::= "^" <partial> | ||
| <partial> ::= <n> | <n> "." <n> | <n> "." <n> "." <n> | ||
| <nightly> ::= <n> "." <n> "-nightly" | "nightly" | ||
| <alias> ::= "lts" | "min" | ||
| <n> ::= "0" | <positive> <digits> | ||
| <digits> ::= <digit> | <digit> <digits> | ||
| <digit> ::= "0" | <positive> | ||
| <positive> ::= "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9" | ||
| ``` |
There was a problem hiding this comment.
Originally I used a Backus-Naur form grammar similar to what NPM semver used. However, that grammar isn't even internally consistent so I instead adopted the grammar syntax used by semver itself.
| }) | ||
|
|
||
| it("Avoid YAML number parsing", async () => { | ||
| expect(parseVersionSpecifiers("1.10")).toEqual(["1.10"]) |
There was a problem hiding this comment.
The README makes a reference to,
When passing in a scalar prefer using a string instead of a numeric value to avoid unwanted YAML decimal conversions (e.g.
1.10will be interpreted as1.1).
Which we should probably test
There was a problem hiding this comment.
Testing that requires an integration test from within GHA (which I can do). The YAML parsing done from within this package doesn't have that issue. Let me elaborate as this is subtle.
In a GitHub Actions workflow when passing in parameters to an action via with you can pass in any YAML data type. If you pass in say version: 1.10 the GitHub Actions YAML parser recognizes 1.10 a number which gets simplified to 1.1. When the julia-version action receives the input it reads the string "1.1". The test here is for the situation where a user sets the parameter:
version: |
- 1.10In that scenario GitHub Actions just passes in the string "- 1.10" which is then YAML parsed by julia-version. Due to the use of the "failsafe" YAML schema we read all scalars as strings which ensures we read "1.10" which is what we are testing for here.
There was a problem hiding this comment.
Ah gotcha, thanks for clearing that up!
| // Set outputs for other workflow steps to use | ||
| core.setOutput("time", new Date().toTimeString()) | ||
| const uniqueVersions = versionSort( | ||
| uniqueArray(resolvedVersions.filter<string>((value) => value !== null)) |
There was a problem hiding this comment.
Not quite following the scenarios where the version would be null?
There was a problem hiding this comment.
For example ^2 would currently resolve to null. Additionally, we do a HTTP HEAD request to confirm that a nightly version exists. In a scenario where a nightly doesn't exit a null would be emitted.
| range.startsWith("<=") || | ||
| range === "*" | ||
| ) { | ||
| return null |
There was a problem hiding this comment.
Instead of returning null here, would it not be better to fail earlier? Then we can simplify the resolveVersion() and resolveVersions().
There was a problem hiding this comment.
I'll consider that. I do think though that missing a julia compat entry doesn't always have to result in a failure.
There was a problem hiding this comment.
Oh that's a good call! I was just looking at the diff here and noticed it seems to just get called in resolveVersion() where if !juliaCompatRange it would throw an error.
When confirming `1.12-nightly` I saw 9s on the initial check and 1s on follow up checks.
This reverts commit 86b16cb.
Creates the action which essentially is the version resolution portion from
julia-actions/setup-julia. The motivation for creating this action is that it allows GitHub Action users to resolve a set of actions like["lts", "1.10", "1.11", "1"]and generate a unique set of resolved versions["1.10.8", "1.11.3"]and avoiding redundant executions in job matrices.I eventually want to have
setup-juliadepend on this as a package to avoid code duplication. Probably will save that for a follow up PR.TODO:
Add(Punted: Add support formanifestaliasmanifestalias #6)Alias "pre" should only return a version on prereleases and not official versions(Punted: Prereleases unsupported #5)Drop(Punted: Prereleases unsupported #5)include-all-prereleasesinput in favor of Julia style trailing-Depends on PR: