CI: Preinstall packages needed for testing and benchmark.#1894
Conversation
Avoids having to compile gtest and google benchmark in every CI job. To make sure we also test downloading and building ourself, don't install for jobs that specify any matrix.packages.
WalkthroughThe changes update a GitHub Actions workflow in the cmake configuration. The conditions for the "Add ubuntu mirrors" and "Install packages (Ubuntu)" steps are simplified to check only for Linux runners that are not on the 'z15' platform, removing the requirement for matrix packages. Additionally, the installation command in the "Install packages (Ubuntu)" step now uses default packages ("libgtest-dev libbenchmark-dev") if Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as R
participant Workflow as W
participant MirrorsStep as MS
participant InstallStep as IS
R->>W: Trigger workflow (Linux runner, not z15)
W->>MS: Execute "Add ubuntu mirrors" step
MS-->>W: Mirrors step executed
W->>IS: Execute "Install packages (Ubuntu)" step
alt matrix.packages is defined
IS-->>W: Install packages from matrix
else matrix.packages not defined
IS-->>W: Install 'libgtest-dev libbenchmark-dev'
end
Suggested labels
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
| sudo apt-get update | ||
| sudo apt-get install -y --allow-downgrades --no-install-recommends ${{ matrix.packages }} | ||
| sudo apt-get install -y --allow-downgrades --no-install-recommends \ | ||
| ${{ matrix.packages || 'libgtest-dev libbenchmark-dev' }} |
There was a problem hiding this comment.
sudo apt-get install -y --allow-downgrades --no-install-recommends \
libgtest-dev libbenchmark-dev ${{ matrix.packages }}No need for || ?
There was a problem hiding this comment.
It is if we still want to also test the download-and-compile-ourselves code.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/cmake.yml (2)
656-662: Simplify Ubuntu Mirrors Addition Step
The updated condition now checks that the runner is Linux and that the job is not on a platform whosematrix.oscontains'z15'. This streamlined check aligns with the PR objective by excluding the preinstallation on designated platforms without adding extraneous conditions. Please verify that this simple exclusion based on'z15'satisfies all intended scenarios.
664-669: Default Package Fallback in the Installation Step
The installation command now smartly uses the expression
${{ matrix.packages || 'libgtest-dev libbenchmark-dev' }}
to preinstall a default set of packages when no specific packages are provided via the matrix. This change fulfills the preinstallation objective by ensuring essential packages are always available and can reduce CI time. One small note: please confirm that cases wherematrix.packagesmight be an empty string are handled as intended, and that overriding defaults works in all scenarios.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1894 +/- ##
===========================================
+ Coverage 82.85% 83.43% +0.57%
===========================================
Files 144 144
Lines 12946 12950 +4
Branches 2855 2858 +3
===========================================
+ Hits 10727 10805 +78
+ Misses 1248 1203 -45
+ Partials 971 942 -29 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Avoids having to compile gtest and google benchmark in every CI job. To make sure we also test downloading and building ourself, don't install for jobs that specify any matrix.packages.
In my spotchecks, it typically spends ~7-8sec extra to run apt install, while the compile is ~22-40sec faster.
Summary by CodeRabbit