Windows CI workflow#29
Conversation
e002566 to
a29837d
Compare
a29837d to
43a0658
Compare
|
PSA: Do not try to use Issue: Using vim in windows-latest Doing so completely borks the session. Nothing I tried was able to exit or kill it. The only way out I found was to cancel the workflow from the web UI (and rescue the terminal using |
|
|
|
I tried building in a When I exited the session, however, the same exact commands were run via CI and got the wrong version. |
43a0658 to
a0fdb1d
Compare
|
BTW, I confirmed the obvious:
I just ran a new session, breaking after setup. I ran I expected that to report "Up do date" as well and do not know how the configuration changed. Trying again, manually running
Next steps:
|
8eaa39c to
aeeed3f
Compare
|
Regarding the non-informative "configuration changed" message, I found the following Cabal issue: "configuration changed" is way too vague It seems that mysterious "configuration changed" rebuilds are not uncommon on Windows. One comment suggests that |
c0a37c3 to
b5ae633
Compare
|
I confirmed that the BTW, the I am unable to pass commands via SSH so that they run without the interactive shell. I bet that this is deliberate, for security reasons, so I did not try to circumvent this restriction. Here is a side-by-side
I wrote a Bash script to check directories in the Job output: Shell output: Notes:
The |
b5ae633 to
e2636fb
Compare
|
The test executable can be checked with Shell: Job: What!?!? This indicates that the test executable is linking to the correct DLL. In the same test run, however, I see the following output: Looking at the test code, I confirmed that the value that is parsed is indeed acquired using The test executable also has the following link, which looks good: Surely
|
afe2a6b to
7dd4000
Compare
bfa2f7b to
02dccff
Compare
|
I (finally) figured out what was going wrong! We must only ever add Windows-style directories to the I had been adding a POSIX-style directory to the The directory that we add is computed, as we have different directories depending on if GHC was installed via Chocolatey or via GHCup, which is sometimes even determined by network conditions. I found that program With this fixed, the correct |
|
Commit message Test Windows using a separate workflow We only test supported versions of GHC using the LLVM/Clang that is installed with that version of GHC. Changes include:
|
|
What version of GHC should we test? Currently/tentatively:
Note that we cannot test using GHC 9.2 because it ships with GCC. Note that I cannot test manual dispatch until the new workflow is in |
|
A few more notes:
|
|
We still need to configure the required tests. Perhaps I should refactor the non-Windows tests firsts, to add support for manual dispatch if nothing else? |
6c6b0da to
a804c81
Compare
|
Note to self:
Since
In this PR, we only test with bundled clang, which is 14 for ghc 9.4 .. 9.12, and clang 20 for ghc 9.14; ghc 9.2 comes with gcc, rather than clang. For testing:
|
f8e0cd3 to
36ad2a9
Compare
|
With this PR, we test the following versions of GHC on Windows, using the LLVM/Clang that is installed with each version:
(Note: We do not test 9.2, which still uses GCC.) Do we need to also test on push? It does not seem to be the case, because we use The required checks also need to be updated. (I do not have permissions for that.) |
36ad2a9 to
61cadb2
Compare
jorisdral
left a comment
There was a problem hiding this comment.
With this PR, we test the following versions of GHC on Windows, using the LLVM/Clang that is installed with each version:
* PR: 9.4, 9.14 * MQ: 9.6, 9.8, 9.10, 9.12(Note: We do not test 9.2, which still uses GCC.)
I've left a review comment about which versions are tested for which triggers
Do we need to also test on push? It does not seem to be the case, because we use
mainas our development branch and all commits to that branch must first go through a PR and the MQ. Cache seems to work regardless. You had a comment about it, though, so I would like to confirm. If is is indeed required, it should test the union of the above sets (all versions tested in PR and MQ)?
Yes, we should run all jobs on push to main. Caches created for the default branch are available to all other branches. Caches created on a non-default branch are only available to that specific branch. So if jobs do not run on push to main, then you'd only get caching effects for re-runs on a specific branch. If main has caches then all PRs benefit from that cache. See https://docs.github.com/en/actions/reference/workflows-and-actions/dependency-caching#restrictions-for-accessing-a-cache
The required checks also need to be updated. (I do not have permissions for that.)
I will do that!
|
|
||
| jobs: | ||
| setup: | ||
| name: "Windows: Setup" |
There was a problem hiding this comment.
Minor: you can choose to not prefix job/step names with Windows because the workflow already has Windows in the name
There was a problem hiding this comment.
This is on purpose, because the workflow context is not always visible. The run page is used when debugging, and the workflow is only shown when you are scrolled to the very top of the page. When you are scrolled down and looking at logs, the browser title, the page title (top), and the job index (left) all just show the job name with no workflow context. When debugging, you may have a number of logs open. When you switch tabs, scrolling up to confirm the workflow can make you lose your place in the logs and is quite annoying. (Example)
Other jobs also duplicate the workflow in the name, so this is not inconsistent. IMHO, the duplication in some contexts is a small price to pay to make it easier when debugging.
| - name: "[build] Build dependencies" | ||
| run: cabal build all --only-dependencies | ||
|
|
||
| - name: "[build] Build all" | ||
| run: cabal build all |
There was a problem hiding this comment.
Optimisation: save the cache right after building dependencies. None of the later steps (like cabal build all) will add anything new to the store AFAICS
| - name: "[build] Build dependencies" | |
| run: cabal build all --only-dependencies | |
| - name: "[build] Build all" | |
| run: cabal build all | |
| - name: "[build] Build dependencies" | |
| run: cabal build all --only-dependencies | |
| # Only save a new cache if it does not exist yet | |
| - name: "[setup] Save cache" | |
| uses: actions/cache/save@v4 | |
| if: ${{ steps.restore-cache.outputs.cache-hit != 'true' && (github.event_name != 'workflow_dispatch' || inputs.enable_cache) }} | |
| with: | |
| path: ${{ steps.setup-haskell.outputs.cabal-store }} | |
| key: ${{ steps.restore-cache.outputs.cache-primary-key }} | |
| - name: "[build] Build all" | |
| run: cabal build all |
There was a problem hiding this comment.
For hs-bindgen we were saving the cache at the very end because there were multiple steps that could add things to the cabal store (hs-bindgen builds, manual builds). That's not the case here, so we can save the cache much earlier. Re-runs will then always have a cache available, even if later steps failed, like building or testing
There was a problem hiding this comment.
FYI: I don't think the !cancelled condition should be added here. We should only save the cache if dependencies were built successfully
There was a problem hiding this comment.
This was written for consistency with the haskell workflow, and I see that has now changed. Sounds good to me! I needed to change the logic to support manual dispatch options. (When dispatching a workflow, you can disable the cache if you think it might be causing issues.)
Speaking of cache, I am not a fan of the cache-reset-number design because one needs to commit to the repo to do an admin task. I prefer to use the CLI (or web page) when a developer decides to delete cache. To avoid bloat, I generally periodically "clear cache" automatically by putting a timestamp in the key. For example, one can do so monthly (date +%Y%m), yearly (date +%Y), weekly (date +%Yw%W), etc. I am of course using cache-reset-number for consistency, but I figured I should mention that there are better ways. 🙇
There was a problem hiding this comment.
Speaking of cache, I am not a fan of the
cache-reset-numberdesign because one needs to commit to the repo to do an admin task. I prefer to use the CLI (or web page) when a developer decides to delete cache. To avoid bloat, I generally periodically "clear cache" automatically by putting a timestamp in the key. For example, one can do so monthly (date +%Y%m), yearly (date +%Y), weekly (date +%Yw%W), etc. I am of course usingcache-reset-numberfor consistency, but I figured I should mention that there are better ways. 🙇
Right, I've used that in the past as well, and I think that's totally reasonable to use an automatic reset
We only test supported versions of GHC using the LLVM/Clang that is installed with that version of GHC. Changes include: * Paths are computed, so that they work when GHC is installed by Chocolatey as well as GHCup. We should no longer have failures due to Chocolatey servers being down. * Pre-installed LLVM/Clang installations are removed from the `PATH` to ensure that we never use them. * Support for manual dispatch is added, allowing developers to specify the GHC versions to test, enable/disable cache, and set a `tmate` breakpoint. * Additional debugging output is added. * Steps are not flattened because all steps are defined in a single file. * Tests are run sequentially, to make it easier to debug failures.
61cadb2 to
9b46bbb
Compare
|
The checkout step of the macOS job completely stalled. I requested to cancel the workflow, but even that did not work! Perhaps it will time out... |
Experimenting and debugging...