bug: build directory should always be ignored

[andrewprock]

After a couple of days of wrangling I was finally able to get my wheel built and working properly. After everything was tested, I noticed that the custom local _build directory was showing up when I typed git status so I added that directory to .gitignore. Once it was in there, the wheel build broke. I was able to fix broken build by removing the directory from .gitingore. I verified this multiple times both directions.

For reference, this commit has the broken build:

https://github.com/andrewprock/pokerstove/commit/d67da05c79bf421d6151668b976fa0d35b61f852

[andrewprock]

Thinking on this more, this could be a bug with pipx. Not sure how to confirm this.

[henryiii]

This is incorrect:

build-dir = "_build"
wheel.packages = ["_build/python/pokerstove"]

You should not set the wheel.packages to the build dir. It should either point to the source directly (better), or you should have it empty and install everything via CMake.

[henryiii]

Is there a pure Python component? If not, just use CMake's install commands for the binaries and leave this empty.

[henryiii]

(Also, I highly recommend including the {wheel_tag} in the build directory, otherwise strange things will happen with caching if you build with two different versions of Python)

[andrewprock]

These all sound like constructive suggestions. Do you have any insight into the bug? Reviewing the source code, I believe this may in fact be a bug within scikit-build-core not pipx.

[henryiii]

The bug is that wheel.packages cannot contain the build directory. It was designed to copy pure Python packages into the wheel. Not your build directory. You need to use CMake's install mechanism to populate the wheel. CMake doesn't intend for the build directory to be relocatable.

[andrewprock]

I was referring to the bug where updating gitignore breaks the build. The contents of gitignore should not affect the build at all. I understand that the organization and the use of the tool could be improved. That's a separate issue independent of the bug.

[LecrisUT]

I was referring to the bug where updating gitignore breaks the build.

It is simply something that cannot be supported, e.g. trying to pip install -e would be chaotic. wheel.packages is the wrong interface for what you're trying to do, you need to use install(TARGETS)

[andrewprock]

I suppose in that case, the documentation should be updated to illuminate this unexpected tooling dependency. Having the build depend on the contents of gitignore is not an expected or typical limitation.

We appear to have run directly into Hyrum's Law with this use case.

[LecrisUT]

It is rather that the combination you tried to do would break in more ways than 3, and it is better to catch these issues early and instruct the developers on good practices rather than have them try to work their way to more and more unstandard CMake builds. E.g. you will notice that your build will break as soon as someone tried to pip install from pypi because you are using build RPATH. There is so much to comment on the build system in general.

[henryiii]

We use the .gitignore by default to filter the contents of the SDist, and the documentation states this. It is a natural and useful default - hatchling also does this. You can manually include files and exclude files to override the .gitignore, but that's how we filter by default.

You could technically add _build to your sdist.include - but don't do that! You should not put the build directory into wheel.packages. We probably should have an explicit check for this and a nicer error message?

[henryiii]

By the way, this is also how a lot of modern tooling works; Meson, Cargo, etc all integrate with git to help with packaging.

[andrewprock]

So yeah, that's a bad design decision. The documentation, which I've read and re-read multiple times over the last week is certainly not clear on this point. The closest it gets is a discussion about assembling the SDist. There is no mention of gitignore impacting the building the wheel.

Regardless, this should provide sufficient content for someone else running into the same issue to find it via a search engine.

There is nothing quite like doing a recursive diff across two directories and finding out that it's gitignore that's breaking the build. This reminds me of the time I filed a bug about go fmt breaking the build. I was told that was a feature as well 😄

Cheers!

[henryiii]

Documentation can always be improved! Where were you looking and expecting to find it? It's currently at https://scikit-build-core.readthedocs.io/en/latest/configuration.html#configuring-source-file-inclusion.

[andrewprock]

Yes, those are the docs I've read multiple times. They got me to where I am today. I suspect there must be some other documentation, possibly in scikit-build, that contains other useful information.

[andrewprock]

I'm trying to integrate the feedback in this thread, but I'm wrestling with some pragmatics.

It's been mentioned that I should use the cmake install feature, however the primary goal of creating a wheel is to use pip to install the package. Using install to assemble artifacts for an installer seems antithetical to the goal. I would like to avoid having to install -> build wheel -> uninstall -> install wheel.

It's also been mentioned that the native cmake file management tools should be used to assemble the artifacts. This is doable, but it still leads to the same problem. There will be an assembly directory which will be ignored in gitignore. I suppose a temporary directory could be used. Unfortunately, there has been a lot of difficulty in triaging broken builds when temp directories are removed. Is there an option to preserve the temporary directories used in the build process?

[henryiii]

Using install to assemble artifacts for an installer

The "installer" in pip is literally just a zip extractor. Wheel installs do nothing more than extract a few directories in a zip file (with a .whl extension) into specific locations. That's it.

If you want to make site-packages/example.<...>.so, then you should do:

python_add_library(example MODULE example.c WITH_SOABI)
install(TARGETS example LIBRARY DESTINATION .)

And if you want to make to make site-packages/example/extension.<...>.so, then you should do:

python_add_library(extension MODULE extension.c WITH_SOABI)
install(TARGETS extension LIBRARY DESTINATION example)

You are "installing" into the main wheel directory (there are a few other directories that you can manually target, as well, like SKBUILD_SCRIPTS_DIR). Then these directors are zipped up into a zip file with a .whl extension. Pip builds this wheel file, then installs it exactly the same way as if it download the .whl directly.

[henryiii]

This patch fixes it for me:

diff --git a/pyproject.toml b/pyproject.toml
index a7c63bb..697f8ba 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -7,16 +7,12 @@ name = "pokerstove"
 version = "1.2"

 [tool.scikit-build]
-build-dir = "_build"
-wheel.packages = ["_build/python/pokerstove"]
-cmake.version = ">=3.5"
-cmake.build-type = "Release"
+build-dir = "_build/{wheel_tag}"
+wheel.packages = []
+cmake.version = ">=3.15"
 build.targets = ["pokerstove"]
-install.components = ["pokerstove"]
+install.components = ["python"]
 sdist.exclude = ["src/programs", "android", "win32", "src/ext", "doc", ".*"]
-# build.verbose = true
-# logging.level = "DEBUG"
-
 [tool.scikit-build.cmake.define]
 BUILD_PYTHON = "ON"

diff --git a/src/lib/python/CMakeLists.txt b/src/lib/python/CMakeLists.txt
index 4c42a44..e8a0dcb 100644
--- a/src/lib/python/CMakeLists.txt
+++ b/src/lib/python/CMakeLists.txt
@@ -2,6 +2,7 @@ cmake_minimum_required(VERSION 3.18)

 # find swig and python and configure them
 set(CMAKE_SWIG_FLAGS)
+set(UseSWIG_MODULE_VERSION 2)
 find_package(SWIG REQUIRED)
 include(UseSWIG)

@@ -26,10 +27,13 @@ set_property(SOURCE pokerstove.i PROPERTY SWIG_MODULE_NAME ${PS_TARGET})
 swig_add_library(${PS_TARGET}
   TYPE MODULE
   LANGUAGE python
-  OUTPUT_DIR ${PYTHON_PROJECT_DIR}
   SOURCES pokerstove.i)
 add_library(${PROJECT_NAMESPACE}::${PS_TARGET} ALIAS ${PS_TARGET})

+install(TARGETS ${PS_TARGET} DESTINATION . COMPONENT python)
+get_property(_support_files TARGET ${PS_TARGET} PROPERTY SWIG_SUPPORT_FILES)
+install(FILES "${_support_files}" DESTINATION . COMPONENT python)
+
 # note: macOS is APPLE and also UNIX !
 if(APPLE)
   set_property(TARGET ${PS_TARGET} APPEND PROPERTY

Checked via:

$ uv build
$ tar -zvtf dist/*.tar.gz
$ unzip -l dist/*.whl
$ uv venv
$ uv pip install dist/*.whl
$ ./.venv/bin/python src/lib/python/test-python
-- rank --
2 6 4
-- suits --
c h c
-- cards --
2c 4c As
-- card sets --
empty card set:
3c4c5c9c 8cAc8dAh2s 8hAh
-- poker evals --
null eval:
one pair:      J5
three str8:    T
-- poker hand evals --
h1:
h2:  one pair:      J5
h3:  three str8:    T
-- poker hand --
ph1:
ph2:  8cAc8dAh2s
ph3:  AcAd3c4s5h6d7c
-- holdem hand evaluation --
high card:     A8
two pair:      A8K
-- holdem facts --
holdem board size:  5
num holdem rounds:  4
-- simple deck --
deck:  2c3c4c5c6c7c8c9cTcJcQcKcAc2d3d4d5d6d7d8d9dTdJdQdKdAd2h3h4h5h6h7h8h9hThJhQhKhAh2s3s4s5s6s7s8s9sTsJsQsKsAs/
deck:  Qc4cTdKsTc4dJs8c9sAhJcAd2d8s7sAc3cAs9cKdQd9hJdKh5h5s3d7d7hKc2h8d8h2c6c7cJh9d6h6dTh4hQs6sTs3sQh3h5c4s5d2s/
peek bottom:  4cQc
peek top:  2s
burn:  2s
holdem:  5d4s 5c3hQh
dealt (dead):  5c5d3hQh2s4s
deck:  Qc4cTdKsTc4dJs8c9sAhJcAd2d8s7sAc3cAs9cKdQd9hJdKh5h5s3d7d7hKc2h8d8h2c6c7cJh9d6h6dTh4hQs6sTs3s/Qh3h5c4s5d2s
-- card distribution --

[andrewprock]

Thank you for all the help @henryiii, testing your patch now.