-
Notifications
You must be signed in to change notification settings - Fork 89
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
ci: add the possitility to use other build generators than ninja inside the ci #3276
ci: add the possitility to use other build generators than ninja inside the ci #3276
Conversation
b41f205
to
66ed037
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3276 +/- ##
========================================
Coverage 56.60% 56.60%
========================================
Files 1064 1064
Lines 89765 89765
========================================
Hits 50807 50807
Misses 38958 38958 ☔ View full report in Codecov by Sentry. |
… processes are not modified by this PR and still use Ninja generator.
…rrent ci processes are not modified by this PR and still use Ninja generator." This reverts commit 66ed037.
Modification of argument names to use those expected by the config-build script. It allows to avoid issues raised by the whitespace in the "Unix Makefiles" value previously used (the parsing done inside the github action weirdly splits the keyword on the whitespace, ignoring the use of quotes or escape (while it runs without any error in a local bash) .
87c7fe9
to
077a1c0
Compare
This reverts commit 077a1c0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see a use for Xcode and eclipse in this context.
--eclipse | ||
Use "Eclipse CDT4 - Unix Makefiles" as build system generator. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would we include this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
--xcode | ||
Use "Xcode" as build system generator. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also...why include this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was for genericity purpose. The script can be executed outside the ci by a local docker run
call so I was guessing that we may want to use one of the generators supported by the project.
But I have not tested it so ok to remove it and to focus on the ci only.
…de the ci (#3276) * Add possibility to use other build generators than ninja in ci. --------- Co-authored-by: Gaetan <gaetan.fuss.fr@gmail.com> Co-authored-by: Randolph Settgast <settgast1@llnl.gov>
…de the ci (#3276) * Add possibility to use other build generators than ninja in ci. --------- Co-authored-by: Gaetan <gaetan.fuss.fr@gmail.com> Co-authored-by: Randolph Settgast <settgast1@llnl.gov>
PR objectives
Ninja
tool in the ci (smaller docker images, lighter setting)unix makefiles
, notNinja
ones) since errors may differ depending on the generator used (see pangea3 PR where ninja fails while make succeed)Implementation
Add the possibility to pass one of the
--ninja
,--makefile
,--xcode
or--eclipse
command line arguments to theci_build_and_test_in_container.sh
script. Variable names have been chosen in agreement with theconfig-build.py
script. The--makefile
option is added to build unix makefiles.Add a
BUILD_GENERATOR
input for thebuild_and_test.yml
script.--ninja
option is used by default to enable the Ninja generator (as it was the initial ci behaviour). The default value is used for CPU builds.Use of the
BUILD_GENERATOR
value of the GPU builds as input for thebuild_and_test.yml
call (because I will link this PR to another one where one of the GPU build will use the unix makefile generator).Set the
BUILD_GENERATOR
variable to--ninja
in the matrix of GPU builds. If not explicitely provided in the matrix definition, an empty string (leading to unix makefile generation) is passed asBUILD_GENERATOR
input to the build_and_test.yml scriptValidation
Commit
077a1c08bdbf5
has been used to validate that for now all jobs that were usingninja
still use it and that the PR doesn't modify the ci.Fixes PR #3159 for which ninja build is broken since commit 4bef600a.