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

Allow version-type: strict + otp-version: master for Elixir -based builds #144

Merged
merged 16 commits into from
Oct 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion .github/workflows/ubuntu.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ jobs:
fail-fast: false
matrix:
combo:
- otp-version: 'master'
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added as part of the "was failing, but shouldn't!" testbed.

elixir-version: '1.14.0'
os: 'ubuntu-latest'
- otp-version: 'master'
elixir-version: '1.14.0'
os: 'ubuntu-latest'
version-type: 'strict'
- otp-version: '25'
elixir-version: '1'
rebar3-version: '3'
Expand Down Expand Up @@ -113,12 +120,13 @@ jobs:
- elixir-version: 'master'
otp-version: '23.1'
os: 'ubuntu-20.04'
- elixir-version: 'master'
- elixir-version: 'main'
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was actually a fix. Since OTP 25, Elixir no longer has master, but main.

otp-version: '25'
os: 'ubuntu-20.04'
version-type: 'strict'
- gleam-version: 'nightly'
otp-version: '24'
os: 'ubuntu-latest'
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should've been broken, but can't understand how we didn't see it before. In any case, on the README, the Ubuntu build is broken and there were recently issues with running actions on GitHub actions like so, which was hiding bugs.

- gleam-version: '0.23'
otp-version: '24'
os: 'ubuntu-latest'
Expand Down
12 changes: 10 additions & 2 deletions __tests__/setup-beam.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,14 @@ async function testElixirVersions() {
got = await setupBeam.getElixirVersion(spec, otpVersion)
assert.deepStrictEqual(got, expected)

simulateInput('version-type', 'strict')
spec = '1.14.0'
otpVersion = 'master'
expected = 'v1.14.0'
got = await setupBeam.getElixirVersion(spec, otpVersion)
assert.deepStrictEqual(got, expected)
simulateInput('version-type', 'loose')

simulateInput('version-type', 'strict')
spec = 'v1.11.0-rc.0'
otpVersion = 'OTP-23'
Expand All @@ -205,9 +213,9 @@ async function testElixirVersions() {
simulateInput('version-type', 'loose')

simulateInput('version-type', 'strict')
spec = 'master'
spec = 'main'
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explained above, though probably only meaningful for OTP 25+

otpVersion = '23.1'
expected = 'master-otp-23'
expected = 'main-otp-23'
got = await setupBeam.getElixirVersion(spec, otpVersion)
assert.deepStrictEqual(got, expected)
simulateInput('version-type', 'loose')
Expand Down
111 changes: 49 additions & 62 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7189,8 +7189,8 @@ async function main() {
const rebar3Spec = core.getInput('rebar3-version', { required: false })

if (otpSpec !== 'false') {
const otpVersion = await installOTP(otpSpec, osVersion)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We install Elixir based on the direct OTP input, and not the output of this function (easier to reason about)...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one!

const elixirInstalled = await maybeInstallElixir(elixirSpec, otpVersion)
await installOTP(otpSpec, osVersion)
const elixirInstalled = await maybeInstallElixir(elixirSpec, otpSpec)

if (elixirInstalled === true) {
const shouldMixRebar = core.getInput('install-rebar', {
Expand Down Expand Up @@ -7300,19 +7300,13 @@ async function maybeInstallRebar3(rebar3Spec) {

async function getOTPVersion(otpSpec0, osVersion) {
const otpVersions = await getOTPVersions(osVersion)
const otpSpec = otpSpec0.match(/^(OTP-|maint-)?([^ ]+)/)
let otpVersion
if (otpSpec[1] && !isStrictVersion()) {
throw new Error(
`Requested Erlang/OTP version (${otpSpec0}) ` +
"should not contain 'OTP-, or maint-'",
)
}
if (otpSpec) {
otpVersion = getVersionFromSpec(
otpSpec[2],
Array.from(otpVersions.keys()).sort(),
)
let otpSpec = otpSpec0 // might be a branch (?)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greatly simplified. We drop OTP-, maint- and whatnot to favour "branch over version," as suggested by @ericmj.

const otpVersion = getVersionFromSpec(
otpSpec,
Array.from(otpVersions.keys()).sort(),
)
if (isVersion(otpSpec0)) {
otpSpec = `OTP-${otpSpec0}` // ... it's a version!
}
if (otpVersion === null) {
throw new Error(
Expand All @@ -7324,60 +7318,46 @@ async function getOTPVersion(otpSpec0, osVersion) {
return otpVersions.get(otpVersion) // from the reference, for download
}

async function getElixirVersion(exSpec0, otpVersion) {
async function getElixirVersion(exSpec0, otpVersion0) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also greatly simplified as per previous explanation.

const otpVersion = otpVersion0.match(/^([^-]+-)?(.+)$/)[2]
const otpVersionMajor = otpVersion.match(/^([^.]+).*$/)[1]
const elixirVersions = await getElixirVersions()
const semverVersions = Array.from(elixirVersions.keys()).sort()

const exSpec = exSpec0.match(/^v?(.+)(-otp-.+)/) || exSpec0.match(/^v?(.+)/)
let elixirVersion
if (exSpec[2] && !isStrictVersion()) {
throw new Error(
`Requested Elixir / Erlang/OTP version (${exSpec0} / ${otpVersion}) ` +
"should not contain '-otp-...'",
)
const exSpec = exSpec0.replace(/-otp-.*$/, '')
const elixirVersionFromSpec = getVersionFromSpec(exSpec, semverVersions, true)
let elixirVersionForDownload = elixirVersionFromSpec
if (isVersion(otpVersionMajor)) {
elixirVersionForDownload = `${elixirVersionFromSpec}-otp-${otpVersionMajor}`
}
if (exSpec) {
elixirVersion = getVersionFromSpec(exSpec[1], semverVersions)
}
if (!exSpec || elixirVersion === null) {
if (elixirVersionFromSpec === null) {
throw new Error(
`Requested Elixir version (${exSpec0}) not found in version list ` +
"(should you be using option 'version-type': 'strict'?)",
)
}
const otpMatch = otpVersion.match(/^(?:OTP-)?([^.]+)/)
let elixirVersionWithOTP

if (elixirVersions.get(elixirVersion)) {
const otpVersionMajor = otpMatch[1]
// We try for a version like `v1.4.5-otp-20`...
if (elixirVersions.get(elixirVersion).includes(otpMatch[1])) {
// ... and it's available: use it!
elixirVersionWithOTP = `${elixirVersion}-otp-${otpVersionMajor}`
core.info(
`Using Elixir ${elixirVersion} (built for OTP ${otpVersionMajor})`,
)
} else {
// ... and it's not available: exit with exception
throw new Error(
`Requested Elixir / Erlang/OTP version (${exSpec0} / ${otpVersion}) not ` +
'found in version list (did you check Compatibility between Elixir and Erlang/OTP?)',
)
}

const elixirVersionComp = elixirVersions.get(elixirVersionFromSpec)
if (
(elixirVersionComp && elixirVersionComp.includes(otpVersionMajor)) ||
!isVersion(otpVersionMajor)
) {
core.info(
`Using Elixir ${elixirVersionFromSpec} (built for Erlang/OTP ${otpVersionMajor})`,
)
} else {
throw new Error(
`Requested Elixir / Erlang/OTP version (${exSpec0} / ${otpVersion}) not ` +
"found in version list (should you be using option 'version-type': 'strict'?)",
`Requested Elixir / Erlang/OTP version (${exSpec0} / ${otpVersion0}) not ` +
'found in version list (did you check Compatibility between Elixir and Erlang/OTP?)',
)
}

return maybePrependWithV(elixirVersionWithOTP, elixirVersion)
return maybePrependWithV(elixirVersionForDownload)
}

async function getGleamVersion(gleamSpec0) {
const gleamSpec = gleamSpec0.match(/^v?(.+)/)
const gleamSpec = gleamSpec0.match(/^v?(.+)$/)
const gleamVersions = await getGleamVersions()
const gleamVersion = getVersionFromSpec(gleamSpec[1], gleamVersions)
const gleamVersion = getVersionFromSpec(gleamSpec[1], gleamVersions, true)
if (gleamVersion === null) {
throw new Error(
`Requested Gleam version (${gleamSpec0}) not found in version list ` +
Expand Down Expand Up @@ -7421,7 +7401,9 @@ async function getOTPVersions(osVersion) {
.trim()
.split('\n')
.forEach((line) => {
const otpMatch = line.match(/^(OTP-|maint-)?([^ ]+)/)
const otpMatch = line
.match(/^([^ ]+)?( .+)/)[1]
.match(/^([^-]+-)?(.+)$/)
const otpVersion = otpMatch[2]
otpVersions.set(otpVersion, otpMatch[0]) // we keep the original for later reference
})
Expand All @@ -7438,7 +7420,6 @@ async function getOTPVersions(osVersion) {
})
})
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unintended...

return otpVersions
}

Expand All @@ -7455,7 +7436,7 @@ async function getElixirVersions() {
.forEach((line) => {
const elixirMatch =
line.match(/^v?(.+)-otp-([^ ]+)/) || line.match(/^v?([^ ]+)/)
const elixirVersion = elixirMatch[1]
const elixirVersion = maybePrependWithV(elixirMatch[1])
const otpVersion = elixirMatch[2]
const otpVersions = otpVersionsForElixirMap.get(elixirVersion) || []
if (otpVersion) {
Expand Down Expand Up @@ -7502,7 +7483,7 @@ function isStrictVersion() {
return core.getInput('version-type', { required: false }) === 'strict'
}

function getVersionFromSpec(spec, versions) {
function getVersionFromSpec(spec, versions, maybePrependWithV0) {
let version = null

if (spec.match(/rc/) || isStrictVersion()) {
Expand All @@ -7527,7 +7508,11 @@ function getVersionFromSpec(spec, versions) {
}
}

return version === null || version === undefined ? null : version
let v = version === null || version === undefined ? null : version
if (maybePrependWithV0 && v != null) {
v = maybePrependWithV(v)
}
return v
}

function maybeCoerced(v) {
Expand Down Expand Up @@ -7634,15 +7619,17 @@ async function get(url0, pageIdxs) {
return ret
}

function maybePrependWithV(versionToPrepend, specVersion) {
const digitStart = /^\d+/
let v = versionToPrepend
if (digitStart.test(specVersion)) {
v = `v${versionToPrepend}`
function maybePrependWithV(v) {
if (isVersion(v)) {
return `v${v.replace('v', '')}`
}
return v
}

function isVersion(v) {
return /^v?\d+/.test(v)
}

module.exports = {
getOTPVersion,
getElixirVersion,
Expand Down
Loading