feat: build.tool-args #733
Conversation
henryiii
force-pushed
the
henryiii/feat/buildtoolargs
branch
from
fd2f807
to
5d1bf4a
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #733 +/- ##
==========================================
+ Coverage 81.97% 82.00% +0.03%
==========================================
Files 68 68
Lines 3888 3896 +8
==========================================
+ Hits 3187 3195 +8
Misses 701 701 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
henryiii
force-pushed
the
henryiii/feat/buildtoolargs
branch
from
3d40da7
to
2efb8d2
Compare
|
Thanks! |
|
I'm going to slip this in the next patch release, since a minor release won't be for a bit, and I want to get a patch release out now that we test free-threading support. |
| def build(self, build_args: Sequence[str]) -> None: | ||
| build_tool_args = self.settings.build.tool_args | ||
| if build_tool_args: | ||
| build_args = [*build_args, "--", *build_tool_args] |
There was a problem hiding this comment.
@henryiii Is build_args here the CMake args list? I'm seeing strange behavior where the ninja args are wrapped in a string:
*** Building project with Default Generator...
2024-05-14 22:28:14,653 - scikit_build_core - INFO - RUN: .local/share/venvs/rapids/lib/python3.10/site-packages/cmake/data/bin/cmake --build cudf/python/cudf/build/pip/cuda-12.2/release -v -- -v -j8 -l8
Change Dir: 'cudf/python/cudf/build/pip/cuda-12.2/release'
Run Build Command(s): .local/share/venvs/rapids/lib/python3.10/site-packages/ninja/data/bin/ninja -v -j 8 "-v -j8 -l8"
.local/share/venvs/rapids/lib/python3.10/site-packages/ninja/data/bin/ninja: invalid option -- ' '
It's possible there's multiple things going on. I have both CMAKE_BUILD_PARALLEL_LEVEL=8 and SKBUILD_BUILD_TOOL_ARGS="-v -j8 -l8", so it looks like -j8 is duplicated.
I don't believe -v -j8 -l8 should be a single string arg, but I'm not sure why cmake --build <dir> -- -v -j8 -l8 would be passing it as one unless the CMake args after the -- is a single string arg?
Perhaps the subprocess.call() for CMake is missing shell=True?
There was a problem hiding this comment.
I think it's SKBUILD_BUILD_TOOL_ARGS parsing. config_settings seem to be working just fine:
$ python -m gpep517 build-wheel --output-fd 1 --wheel-dir dist --config-json '{"build.tool-args": ["-j12", "-l12"]}'
There was a problem hiding this comment.
Env vars are ; separated.
There was a problem hiding this comment.
You only need the -l8, the others can be set from CMake or scikit-build-core.
There was a problem hiding this comment.
At a computer now. All SKBUILD_* vars are ; separated, like cmake. So it's SKBUILD_BUILD_TOOL_ARGS="-v;-j8;-l8". But you don't need the first two; you can do CMAKE_BUILD_PARALLEL_LEVEL=8 SKBUILD_CMAKE_VERBOSE=1 SKBUILD_BUILD_TOOL_ARGS=-l8. Or, pass via config settings with cmake.verbose and build.tool-args. For pip and build, you make a list by passing multiple times, so pip install . -Cbuild.tool-args="-j12" -Cbuild.tool-args="-l12" -Cbuild.tool-args="-v" would be one way to do it. Or pip install . -Ccmake.vebose="true" -Cbuild.tool-args="-l12" -Dcmake.define.CMAKE_BUILD_PARALLEL_LEVEL="8".
I'll probably rename cmake.verbose to build.verbose now that we have a build section. The old one will work though for back-compat.
|
Oh, I think I forgot to say that: it seems to work fine, and Gentoo is using it now. Thanks a lot! |
Close #722.