🚸 Add support for CMake presets#363
Conversation
| - name: Configure CMake with cmake-presets | ||
| if: ${{ inputs.use-cmake-presets }} | ||
| env: | ||
| PRESET_NAME: ${{ inputs.preset-name }} | ||
| SETUP_MLIR: ${{ inputs.setup-mlir }} | ||
| run: | | ||
| LIT_ARG="" | ||
| if [ "$SETUP_MLIR" = "true" ]; then | ||
| LIT_ARG="-DLLVM_EXTERNAL_LIT=$(which lit)" | ||
| fi | ||
| cmake --preset "$PRESET_NAME" -DENABLE_COVERAGE=ON "$LIT_ARG" |
There was a problem hiding this comment.
@burgholzer, I have two questions here:
- Do we want a
coveragepreset for this? I'm leaning toward no. - Do we want to force
-DCMAKE_BUILD_TYPE=Debug, or do we want to allow a user to pass the "wrong" config here?
There was a problem hiding this comment.
Another question: Do you see a better way of dealing with lit? We could get rid of the conditional statement above if we enforced the usage of something like LIT_PATH. We could export LIT_PATH=$(which lit) in our workflows, and the preset in FullStaQD/compiler would have to be adjusted to say "LLVM_EXTERNAL_LIT": "$env(LIT_PATH)" here: https://github.com/FullStaQD/compiler/blob/8ccc24fbe49595abe9c34a938c3d4ffa2cdaab94/CMakePresets.json#L12
We already have to say "MLIR_DIR": "$env(MLIR_DIR)" and "LLVM_DIR": "$env(LLVM_DIR)" if we do not want to start overwriting the default preset values. I feel like we are starting to miss the point of presets if we need to call cmake --preset dev -DMLIR_DIR=path/to/mlir/installation/dir -DLLVM_DIR=/path/to/llvm/installation/dir -DLLVM_EXTERNAL_LIT=$(which lit) to overwrite the defaults.
| - name: Generate compilation database with cmake-presets | ||
| if: ${{ inputs.use-cmake-presets }} | ||
| env: | ||
| PRESET_NAME: ${{ inputs.preset-name }} | ||
| run: cmake --preset "$PRESET_NAME" |
There was a problem hiding this comment.
@burgholzer, also here: Do we want to force -DCMAKE_BUILD_TYPE=Debug, or do we want to allow a user to pass the "wrong" config here?
4869ed9 to
f0fdd1f
Compare
Description
This PR adds support for CMake presets.
Fixes #352
Checklist
I have added appropriate tests that cover the new/changed functionality.I have updated the documentation to reflect these changes.