[build-script] Improvements to CMakeOptions. #23303
Conversation
|
This looks good to me, but, I think that I would defer to @Rostepher or @ddunbar here. |
drodriguez
force-pushed
the
cmake-options-improvements
branch
from
b2a5491 to
45d2137
Compare
drodriguez
force-pushed
the
cmake-options-improvements
branch
from
45d2137 to
ce2463d
Compare
|
Rebased on top of current master. @swift-ci please test |
|
Build failed |
|
Build failed |
drodriguez
force-pushed
the
cmake-options-improvements
branch
from
ce2463d to
9ef97f0
Compare
|
Fix a problem where the version parameters are neither bools, numbers or strings (they get parsed into specific types by the argparser), and cannot be passed directly to (The Linux problem seemed unrelated, though) @swift-ci please test |
|
Build failed |
|
Build failed |
drodriguez
force-pushed
the
cmake-options-improvements
branch
from
9ef97f0 to
632f936
Compare
|
Making the linter happy. @swift-ci please test |
|
Build failed |
|
Build failed |
drodriguez
force-pushed
the
cmake-options-improvements
branch
from
632f936 to
1170c57
Compare
|
Recovering the changes overwritten in the last forced push. @swift-ci please test |
|
Build failed |
|
Build failed |
CMakeOptions was briefly used in the cmake module, but mostly unused in the rest of the modules, however, several products were dealing with what eventually will end as CMake options, but not using the already existing code, and duplicating it. The changes tries to unify all usage around CMakeOptions which avoids code duplication. Additionally, it provides some more API surface in CMakeOptions to make it more useful. - The initializer can be passed a list of tuples or another CMakeOptions to copy into the newly created options. - Boolean Python values are treated automatically as boolean CMake options and transformed into `TRUE` and `FALSE` automatically. - Provides `extend`, which will add all the tuples from a list or all the options from another `CMakeOptions` into the receiving `CMakeOptions`. - Provides `__contains__`, which makes checking for existing options a little easier (however, being `CMakeOptions` basically a list, the checking has to include also the value, which is not that helpful). - Modify LLVM and Swift products to use `CMakeOptions`. It makes the boolean values clearer and avoid a lot of repetitive string formatting. - Modify the tests to make them pass and provide new test for newly introduced features.
drodriguez
force-pushed
the
cmake-options-improvements
branch
from
1170c57 to
6bf0826
Compare
|
Ready for review. Fixed the conflicts with current master, and checked it still works as intended. @swift-ci please test |
|
@Rostepher: I think this would be valuable going forward, and might simplify the later changes. If there's some feedback (positive or negative) I can also work on it. If we don’t want the changes at all, it would also be nice to know, to try to work around them, and not be hold by them. Thanks a lot. |
Rostepher
left a comment
Rostepher
left a comment
There was a problem hiding this comment.
These changes look good to me.
4 participants
Applies to SR-237
This code was extracted and improved from #23038.
CMakeOptions was briefly used in the cmake module, but mostly unused in
the rest of the modules, however, several products were dealing with
what eventually will end as CMake options, but not using the already
existing code, and duplicating it.
The changes tries to unify all usage around CMakeOptions which avoids
code duplication. Additionally, it provides some more API surface in
CMakeOptions to make it more useful.
to copy into the newly created options.
options and transformed into
TRUEandFALSEautomatically.extend, which will add all the tuples from a list or allthe options from another
CMakeOptionsinto the receivingCMakeOptions.__contains__, which makes checking for existing options alittle easier (however, being
CMakeOptionsbasically a list, thechecking has to include also the value, which is not that helpful).
CMakeOptions. It makes theboolean values clearer and avoid a lot of repetitive string
formatting.
introduced features.
/cc @Rostepher, @compnerd