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

Fix windows executable name #41

Merged
merged 9 commits into from
Oct 5, 2024
2 changes: 2 additions & 0 deletions .github/linters/.eslintrc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ rules:
'i18n-text/no-en': 'off',
'import/no-namespace': 'off',
'no-console': 'off',
'no-shadow': 'off',
'no-unused-vars': 'off',
'prettier/prettier': 'error',
'semi': 'off',
Expand All @@ -63,6 +64,7 @@ rules:
'@typescript-eslint/no-namespace': 'error',
'@typescript-eslint/no-non-null-assertion': 'warn',
'@typescript-eslint/no-require-imports': 'error',
'@typescript-eslint/no-shadow': 'error',
'@typescript-eslint/no-unnecessary-qualifier': 'error',
'@typescript-eslint/no-unnecessary-type-assertion': 'error',
'@typescript-eslint/no-unused-vars': 'error',
Expand Down
12 changes: 12 additions & 0 deletions .github/workflows/Check-Version.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
if($args.count -ne 1) {
Write-Host "This command requires 1 arg with the version to check"
exit 1
}

$result = pkl.exe --version | Select-String -Pattern $args[0] -Quiet

if($result -eq "True") {
exit 0
} else {
exit 1
}
14 changes: 11 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ on:
permissions:
contents: read

env:
CHECK_VERSION: 0.26.0

jobs:
test-typescript:
name: TypeScript Tests
Expand Down Expand Up @@ -60,7 +63,12 @@ jobs:
id: test-action
uses: ./
with:
pkl-version: 0.26.0
pkl-version: ${{ env.CHECK_VERSION }}

- name: Confirm download (unix)
run: pkl --version | grep "Pkl ${{ env.CHECK_VERSION }}"
if: matrix.os != 'windows-latest'

- name: Confirm download
run: pkl --version
- name: Confirm download (windows)
run: .github/workflows/Check-Version.ps1 "Pkl ${{ env.CHECK_VERSION }}"
if: matrix.os == 'windows-latest'
116 changes: 96 additions & 20 deletions dist/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 9 additions & 27 deletions src/main.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as core from '@actions/core'
import * as tc from '@actions/tool-cache'
import os from 'node:os'
import { chmod } from 'fs/promises'
import { determinePlatformInfo } from './platform'

/**
* The main function for the action.
Expand All @@ -17,8 +17,8 @@ export async function run(): Promise<void> {
if (cachedPath) {
core.debug(`Found cached Pkl version at ${cachedPath}`)
} else {
const assetName = findAssetName()
const downloadUrl = `https://github.com/apple/pkl/releases/download/${pklVersion}/${assetName}`
const platformInfo = determinePlatformInfo()
const downloadUrl = `https://github.com/apple/pkl/releases/download/${pklVersion}/${platformInfo.githubSourceAssetName}`

core.debug(`Download URL: ${downloadUrl}`)

Expand All @@ -32,7 +32,12 @@ export async function run(): Promise<void> {
core.debug(`Set executable permissions on: ${pklBinaryPath}`)

// Cache the downloaded file
cachedPath = await tc.cacheFile(pklBinaryPath, 'pkl', 'pkl', pklVersion)
cachedPath = await tc.cacheFile(
pklBinaryPath,
platformInfo.targetFileName,
'pkl',
pklVersion
)
core.debug(`Cached PKL binary to: ${cachedPath}`)
}

Expand All @@ -43,26 +48,3 @@ export async function run(): Promise<void> {
if (error instanceof Error) core.setFailed(error.message)
}
}

function findAssetName(): string {
const op = os.platform()
const arch = os.arch()

core.info(`Try to find asset name for: ${op}-${arch}`)
switch (op) {
case 'linux':
return 'pkl-linux-amd64'
case 'darwin':
switch (arch) {
case 'x64':
return 'pkl-macos-amd64'
case 'arm64':
return 'pkl-macos-aarch64'
}
break
case 'win32':
return 'pkl-windows-amd64.exe'
}

throw new Error(`Couldn't find asset name for ${op}-${arch}`)
}
98 changes: 98 additions & 0 deletions src/platform.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import os from 'node:os'

enum Platform {
Linux,
MacOS,
Windows
}

enum Architecture {
arm64,
x64
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I generally prefer as const objects or arrays instead of enums - enums are one of the few TypeScript features which are transpiled into entirely different runtime code (whereas most TypeScript type-related syntax is simply erased).


type PlatformInfo = {
plat: Platform
arch: Architecture
githubSourceAssetName: string
targetFileName: string
}

export function determinePlatformInfo(): PlatformInfo {
const plat = determineOS()
const arch = determineArch()

return {
plat,
arch,
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't look like these two are used by the caller

githubSourceAssetName: determineGithubAsset(plat, arch),
targetFileName: determineTargetFileName(plat)
}
}

function determineOS(): Platform {
switch (os.platform()) {
case 'linux':
return Platform.Linux
case 'darwin':
return Platform.MacOS
case 'win32':
return Platform.Windows
default:
throw new Error('Unsupported platform')
}
}

function determineArch(): Architecture {
switch (os.arch()) {
case 'arm64':
return Architecture.arm64
case 'x64':
return Architecture.x64
default:
throw new Error('Unsupported architecture')
}
}

function determineGithubAsset(plat: Platform, arch: Architecture): string {
switch (plat) {
case Platform.Linux:
switch (arch) {
case Architecture.arm64:
return 'pkl-linux-aarch64'
case Architecture.x64:
return 'pkl-linux-amd64'
default:
throw new Error('Unsupported architecture')
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this can happen here, right? Since arch is of type Architecture, and determineArch() can only return one of the two supported values

}
case Platform.MacOS:
switch (arch) {
case Architecture.arm64:
return 'pkl-macos-aarch64'
case Architecture.x64:
return 'pkl-macos-amd64'
default:
throw new Error('Unsupported architecture')
}
case Platform.Windows:
switch (arch) {
case Architecture.arm64:
throw new Error('Windows arm not yet supported')
case Architecture.x64:
return 'pkl-windows-amd64.exe'
default:
throw new Error('Unsupported architecture')
}
default:
throw new Error('Unsupported platform')
}
}

function determineTargetFileName(plat: Platform): string {
switch (plat) {
case Platform.Windows:
return 'pkl.exe'
default:
return 'pkl'
}
}
Loading