Repository navigation
Derive Windows cache paths and make the package switches exhaustive - #1140
Merged
Merged
Conversation
The MSYS2 and Cygwin forms of the _opam path were hard-coded as /d/a/<repo>/<repo>/_opam, the layout of GitHub-hosted runners only. Convert the actual workspace path instead, with the conversion the GnuPG call already used. On GitHub-hosted runners the strings are unchanged, so existing cache entries still match.
installUnixSystemPackages and updateUnixPackageIndexFiles switched on PLATFORM with no Windows case and no default, so a platform added to the union would compile and silently skip package installation. Take the platform as a parameter narrowed to Linux and macOS, and end each switch with a never check. The optional apt package lookup is only reachable on Linux, so drop its platform switch and the guard for an empty list, which the hard-coded call never passes.
smorimoto
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The MSYS2 and Cygwin forms of the
_opamcache path were hard-coded as/d/a/<repo>/<repo>/_opam, which matches GitHub-hosted runners only. The Unix package switches also had no case for a new platform, so one would have compiled and silently skipped package installation.What changed
toMsysPath()andtoCygwinPath()derive both paths fromGITHUB_WORKSPACE, with the conversion that the GnuPG call already used.installUnixSystemPackages()andupdateUnixPackageIndexFiles()take a Linux or macOS platform and end eachswitchwith anevercheck.Scope
Blast radius
On GitHub-hosted runners the cache paths are the same strings as before, so existing Windows cache entries still match.
Verification
D:\a\setup-ocaml\setup-ocaml\_opam, and the action tests pass unchanged.