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

Don't use the versions file for tools installed via NPM #23

Open
thomashoneyman opened this issue Aug 8, 2021 · 2 comments · May be fixed by #24
Open

Don't use the versions file for tools installed via NPM #23

thomashoneyman opened this issue Aug 8, 2021 · 2 comments · May be fixed by #24
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@thomashoneyman
Copy link
Collaborator

thomashoneyman commented Aug 8, 2021

The versions file exists to avoid rate limits imposed by GitHub when getting the latest release for various tools via the GitHub API. However, if a user specifies they want the latest tool and the tool is installed via NPM, then we should skip the versions file and just NPM install the latest -- we don't get rate limited on that.

@thomashoneyman thomashoneyman added enhancement New feature or request good first issue Good for newcomers labels Aug 8, 2021
@thomashoneyman
Copy link
Collaborator Author

This function decides whether to fetch an exact version of a tool (because the user specified an exact version) or to look up the latest version in the versions file:

-- | Resolve the exact version to provide for a tool in the environment, based
-- | on the action.yml file.
resolve :: Json -> Tool -> ExceptT Error Effect (Maybe { tool :: Tool, version :: Version })
resolve versionsContents tool = do
let key = Key.fromTool tool
field <- getVersionField key
case field of
Nothing -> pure Nothing
Just (Exact v) -> liftEffect do
Core.info "Found exact version"
pure (pure { tool, version: v })
Just Latest -> liftEffect do
Core.info $ fold [ "Fetching latest tag for ", Tool.name tool ]
let
version = lmap printJsonDecodeError $ (_ .: Tool.name tool) =<< decodeJson versionsContents
parse = lmap parseErrorMessage <<< Version.parseVersion
case parse =<< version of
Left e -> do
Core.setFailed $ fold [ "Unable to parse version: ", e ]
throwError $ error "Unable to complete fetching version."
Right v -> do
pure (pure { tool, version: v })

However, if the InstallMethod for a tool is NPM:

-- | How a tool will be installed: either a tarball from a URL, or an NPM package
-- | at a particular version.
data InstallMethod = Tarball TarballOpts | NPM NPMPackage

...then we should skip the versions file altogether and just NPM install the tool at its latest version according to NPM:

installMethod :: Tool -> Version -> InstallMethod
installMethod tool version = do
let
toolName = name tool
toolRepo = repository tool
formatArgs = { repo: toolRepo, tag: formatTag, tarball: _ }
formatGitHub' = formatGitHub <<< formatArgs
unsafeVersion str = fromRight' (\_ -> unsafeCrashWith "Unexpected Left") $ parseVersion str
executableName = case platform of
Windows -> toolName <> ".exe"
_ -> toolName
case tool of
PureScript -> Tarball
{ source: formatGitHub' case platform of
Windows -> "win64"
Mac -> "macos"
Linux -> "linux64"
, getExecutablePath: \p -> Path.concat [ p, "purescript", executableName ]
}
Spago -> Tarball
{ source: formatGitHub'
-- Spago has changed naming conventions from version to version
if version >= unsafeVersion "0.18.1" then case platform of
Windows -> "Windows"
Mac -> "macOS"
Linux -> "Linux"
else if version == unsafeVersion "0.18.0" then case platform of
Windows -> "windows-latest"
Mac -> "macOS-latest"
Linux -> "linux-latest"
else case platform of
Windows -> "windows"
Mac -> "osx"
Linux -> "linux"
, getExecutablePath: \p -> Path.concat [ p, executableName ]
}
Psa ->
NPM ("purescript-psa@" <> Version.showVersion version)
PursTidy ->
NPM ("purs-tidy@" <> Version.showVersion version)
Zephyr -> Tarball
{ source: formatGitHub' $ case platform of
Windows -> "Windows"
Mac -> "macOS"
Linux -> "Linux"
, getExecutablePath: \p -> Path.concat [ p, "zephyr", executableName ]
}

Currently that only covers psa and purs-tidy.

@pete-murphy pete-murphy linked a pull request Aug 10, 2021 that will close this issue
@pete-murphy
Copy link

Does something like this (#24) make sense as a possible fix?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Development

Successfully merging a pull request may close this issue.

2 participants