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

Simplify setting cmake-preset #204

Open
LecrisUT opened this issue Feb 23, 2023 · 10 comments
Open

Simplify setting cmake-preset #204

LecrisUT opened this issue Feb 23, 2023 · 10 comments
Labels
enhancement New feature or request

Comments

@LecrisUT
Copy link
Collaborator

LecrisUT commented Feb 23, 2023

I'm thinking if we can pass conditional cmake flags based on the optional dependencies or other parameter that we can pass to pip install ..

This came up when thinking about how to code-coverage a python-C binding. If we build with the --coverage flags, then in principle we should be able to trigger those during pytest and resolve the information with lcov. But we also don't want to ship it with those flags on. So any advice on how to design this?

Also related, being able to configure via cmake-preset?

@henryiii
Copy link
Collaborator

You can do --config-settings=cmake.define.SOME_OPTION=ON to pass -DSOME_OPTION=ON. You can also pass arbitrary CMake args via cmake.args. You should be able to select a preset with those args as well - possibly we can streamline this in the future, similar to defines.

You can't detect extras, those are only for dependencies and are not passed through the built system.

@LecrisUT
Copy link
Collaborator Author

Holy smokes that worked. I've enabled the coverage build and changed the build directory and I managed to get test coverage across the python api. Thanks!

@LecrisUT LecrisUT changed the title Conditional cmake flags Simplify passing cmake flags Feb 28, 2023
@LecrisUT
Copy link
Collaborator Author

LecrisUT commented Feb 28, 2023

So to summarize on what would be nice to have here:

  • Ability to pass -DSOME_OPTION directly. Better with -Ccmake.define.SOME_OPTION in future pip release
  • Ability to pass cmake-preset

@henryiii
Copy link
Collaborator

henryiii commented Mar 1, 2023

What do you mean by "directly"? I'm not sure how it can be more "direct". The next version of Pip will support -Ccmake.define.SOME_OPTION=ON, which will be quite a bit nicer (no double equals and exactly the same as pypa/build).

CMake preset can currently only be passed via cmake.args, but we probably could easily add a shortcut for it similar to defines.

@LecrisUT
Copy link
Collaborator Author

LecrisUT commented Mar 1, 2023

"directly" as in just passing -DSOME_OPTION instead of --config-settings=... or -Ccmake.define.SOME_OPTION. Is there a potential clash or implementation issue on that?

@henryiii
Copy link
Collaborator

henryiii commented Mar 1, 2023

That would then be an option to Pip instead of us?

@LecrisUT
Copy link
Collaborator Author

LecrisUT commented Mar 1, 2023

Oh, I remember some implementation with setup.py that basically forwarded any -D[unknown_flag] to cmake directly. Is that not possible on the scikit-build-core side, only on the user side?

@henryiii
Copy link
Collaborator

henryiii commented Mar 1, 2023

The best I could properly do would be -CD.SOME_OPTION, which is different enough (CD.) that I didn't think it was worth the shortcut, and TOML version makes more sense:

[tool.scikit-build.cmake.define]
SOME_OPTION=true

# vs
[tool.scikit-build.D]
SOME_OPTION=true

@henryiii
Copy link
Collaborator

henryiii commented Mar 1, 2023

PEP 517 handles all configuration though the config-settings object that's passed to the build hook. You can't just pass arbitrary things around - and it never worked very well with setup.py, as calling that directly is discouraged and somewhat broken, and getting those things passed through pip was not pretty (I remember lots of -- and global-options and things like that). We could make a custom runner (a "scikit-build" CLI app) that would provide a custom interface, but that's still some ways off.

@LecrisUT
Copy link
Collaborator Author

LecrisUT commented Mar 1, 2023

Hmm, if it is not possible, I think the current method is preferred. Just having -CD. seems very confusing. I've edited my comment, maybe you can mark this part as resolved/hide?

@henryiii henryiii changed the title Simplify passing cmake flags Simplify setting cmake-preset Mar 1, 2023
@henryiii henryiii added the enhancement New feature or request label Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants