refactor: enable macos build workflow#217
Conversation
|
Merging to
|
6d9a903 to
fb464b0
Compare
There was a problem hiding this comment.
Bug: Missing CMake Configuration in macOS Job
The macOS job is missing the essential CMake configuration step. It attempts to build the project without first running cmake -B to initialize the build directory, which will cause the build to fail. This configuration step is correctly included in the ubuntu and selfhosted jobs.
.github/workflows/build.yml#L12-L27
libCacheSim/.github/workflows/build.yml
Lines 12 to 27 in 15f3241
Was this report helpful? Give feedback by reacting with 👍 or 👎
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the macOS build workflow and updates the checkout action to version 4 across all jobs.
- Uncomments and updates the macOS job with a new dependency installation script and switches from macos-10.15 to macos-latest.
- Upgrades the actions/checkout step from v2 to v4 in macOS, Ubuntu, and self-hosted jobs.
Comments suppressed due to low confidence (3)
.github/workflows/build.yml:17
- Upgrading to actions/checkout@v4 is a good update; please verify that any changes in default behaviors do not affect downstream steps.
- uses: actions/checkout@v4
.github/workflows/build.yml:32
- Upgrading to actions/checkout@v4 is a good update; please verify that any changes in default behaviors do not affect downstream steps.
- uses: actions/checkout@v4
.github/workflows/build.yml:52
- Upgrading to actions/checkout@v4 is a good update; please verify that any changes in default behaviors do not affect downstream steps.
- uses: actions/checkout@v4
| # run: make test | ||
| macos: | ||
| name: macos / clang | ||
| runs-on: macos-latest |
There was a problem hiding this comment.
Using 'macos-latest' may lead to varying build environments over time; consider pinning to a specific macOS version to ensure consistent builds if required.
| runs-on: macos-latest | |
| runs-on: macos-12 |
|
Not sure why the test results are not correct. |
|
Might the test be failing due to a different RNG implementation on macOS? |
I thought about this, but LRU does not use RNG, or is there a hidden one that I missed? |
It seems that the LRU test passes, and that Cacheus is the problematic test (we can see this from the |
While LeCaR uses RNG as well, it passed the test. |
|
Confirmed the difference is from RNG which makes dump |
Oh, I read it wrong. Yes, it looks like RNG is the reason. We have an internal fast and deterministic RNG, do you want to submit a PR to fix it? |
Thank you for confirming this! |
At first glance, CACHEUS uses the internal RNG, so probably need to dig deeper. |
|
Thank you! Does it only affect CACHEUS? |
Yes. Only CacheUs is affected by the initial state of RNG |
update macOS job configuration and upgrade checkout action to v4