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

Switch Linux and macOS CI runners to use Ninja build #2264

Merged
merged 7 commits into from
Oct 26, 2023
Merged

Switch Linux and macOS CI runners to use Ninja build #2264

merged 7 commits into from
Oct 26, 2023

Conversation

mewim
Copy link
Member

@mewim mewim commented Oct 25, 2023

No description provided.

@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (314781c) 89.61% compared to head (ed40a0f) 89.48%.
Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2264      +/-   ##
==========================================
- Coverage   89.61%   89.48%   -0.14%     
==========================================
  Files        1016     1021       +5     
  Lines       35806    36003     +197     
==========================================
+ Hits        32089    32218     +129     
- Misses       3717     3785      +68     

see 56 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mewim mewim changed the title Ninja ci Switch Linux and macOS CI runners to use Ninja build Oct 25, 2023
@mewim mewim requested a review from Riolku October 25, 2023 12:08
@Riolku
Copy link
Contributor

Riolku commented Oct 25, 2023

We should probably use gmake in at least one of the jobs. I don't think it particularly matters which, but we should test that gmake still works.

@Riolku
Copy link
Contributor

Riolku commented Oct 25, 2023

Benefit of doing it in GCC is build with ASAN and benchmark runs sooner. Benefit of doing it in clang means any clang tidy errors are shown earlier.

@mewim
Copy link
Member Author

mewim commented Oct 25, 2023

I did not switch the benchmark to use ninja so gmake is being tested in benchmark. Is it sufficient or do you prefer to use gmake in a test?

@Riolku
Copy link
Contributor

Riolku commented Oct 25, 2023

I think I'd rather have the benchmark use ninja and clang use gmake, because that should give us the fastest overall CI speeds.

@mewim
Copy link
Member Author

mewim commented Oct 25, 2023

OK I will redeploy the benchmark runners some time and update this...
The benchmark uses a different Dockerfile and runs on a different machine.

@mewim
Copy link
Member Author

mewim commented Oct 26, 2023

@Riolku I have updated and redeployed the benchmark runners to use Ninja build and switched clang build job back to using gmake. I also removed several unneeded dependencies from the benchmark runner.

@@ -255,19 +257,13 @@ jobs:
benchmark:
name: benchmark
needs: [gcc-build-test, clang-build-test]
env:
NUM_THREADS: 30
Copy link
Contributor

Choose a reason for hiding this comment

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

If you use an env var, remove the parameter to make on the later line.

benchmark/Dockerfile Show resolved Hide resolved
@mewim mewim merged commit 26b2ed5 into master Oct 26, 2023
12 checks passed
@mewim mewim deleted the ninja-ci branch October 26, 2023 15:08
Ashleyhx pushed a commit that referenced this pull request Oct 27, 2023
* Add ninja to Linux runners

* Use Ninja in CI workflows

* Update runner version

* Update benchmark Dockerfile

* Upgrade GitHub runner for benchmark

* Add missing ninja-build

* Revert CI benchmark dependencies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants