Test cases for different use cases of generated files #1193

eirrgang · 2025-12-11T21:01:51Z

Per conference calls, these are some test cases relevant to #808 and #1160.

eirrgang · 2025-12-11T21:03:23Z

I'm personally most interested in the use case of being able to open a compiled library with ctypes (7bdec1e), but I figured it was worth trying to explore the edge cases of whatever machinery is in place.

eirrgang · 2025-12-11T23:39:54Z

Should we mark.xfail the cases that don't work (to make automation happy) or should we discuss and poke at them a bit first?

LecrisUT · 2025-12-12T00:50:18Z

Awesome, will try to chevk them out saturday or late today.

Should we mark.xfail the cases that don't work (to make automation happy) or should we discuss and poke at them a bit first?

Let's first look at the mechanism of how it fails and then we should mark them xfail and revert it in #808.

LecrisUT

LGTM overall, just needs a few iterations to clean this up. I did not go through all the test failing cases, but at the first glance, it looks to fail as we are expecting it to fail currently.

tests/packages/cmake_generated/src/cmake_generated/__init__.py

LecrisUT · 2025-12-12T08:12:22Z

tests/packages/cmake_generated/src/cmake_generated/nested1/generated.py.in

+def nested2_check():
+    if nested2 is not None:
+        return "success"


Why not try ... import directly in here?

Good question. I'm not sure whether there is reason to consider how the importer machinery might behave differently during import as during execution, so I wanted at least one case that really exercised the redirections of modules that depended on each other at import time. If you are confident that this shouldn't be an issue, we probably don't need nested2 at all.

I'm not sure whether there is reason to consider how the importer machinery might behave differently during import as during execution

Not that I know of, other than resolving the circular dependency. But if you want for that to do the testing, could you also print the exception when it failed?

tests/test_editable_generated.py

LecrisUT · 2025-12-12T08:29:55Z

tests/packages/cmake_generated/src/cmake_generated/__init__.py

+
+
+def ctypes_function():
+    # Question: can anyone think of a clever way to embed the actual library name in some other package metadata?


Unless ctypes itself can search from the filename with no suffix, I don't really think there is. Most likely the user should generate the .py file if it is important.

Suggested change

# Question: can anyone think of a clever way to embed the actual library name in some other package metadata?

LecrisUT · 2025-12-12T08:40:01Z

tests/packages/cmake_generated/src/cmake_generated/__init__.py

+* one level above and below, and
+* from parallel subpackages.
+
+Question: Do we want to test both relative and absolute imports or just one or the other?


We did have issues with absolute and relative imports behaving differently. We should probably try to support it, but where do you have in mind to check this?

It would be relevant to interdependent submodules, if parts of the package were in the source tree and parts were in the build tree, so I would say between nested1.generated and nested2

Well, the only difference I can think of in there is breaking up the circular dependency and importing only the module and not any from ... import. I don't think we actually have any control on that thing though and would be independent of the mechanism (as long as it can resolve with or without navigating the potential circular dependencies)

LecrisUT · 2025-12-12T08:46:18Z

tests/packages/cmake_generated/CMakeLists.txt

+# (add_custom_command) Note that bundling a generated file with sdist is out of
+# scope for now. Note: cmake_generated/nested1/generated.py should try to open
+# both generated and static files.
+configure_file(src/cmake_generated/nested1/generated.py.in generated.py)


For inplace mode, it is run with cmake -S /path/to/src -B /path/to/src. So for it to "work" you would have to configure it in the appropriate src-like paths that would then be installed. Alternatively, and my preference, forget about supporting inplace since even getting that one to work by the user is a struggle. In that mode we don't do anything special either, we copy everything as if it were a pure python project.

Right... I don't remember from when we talked whether the long-term plan was to rely on running out of the build tree or only support an installed path. It makes sense to clarify one way or the other. It makes sense to me to discourage inplace and xfail it with an explanation (that inplace means the build tree must look like the package layout)

Build tree from inplace would be different from build tree with redirect (what #1170 would become). The latter would make sense to focus support more on, but the former is really just an ugly workaround.

My inclination is

Assume CMakeLists.txt authors are not concerning themselves with the layout of the build tree, make the default test scenario assume incompatible layouts, and be clear what the expected behavior is.

If there is an expectation of using the build tree in some situations and there are caveats on the build tree location or layout, be clear that it is a special situation, and test and document it.

It sounds like inplace is in flux and step 2. could be beyond the scope of this PR.

Yep, sounds good. We indeed need to document the different editable modes better, but that's for a separate PR.

eirrgang · 2025-12-12T16:44:00Z

tests/packages/cmake_generated/src/cmake_generated/__init__.py

+
+
+def ctypes_function():
+    # Question: can anyone think of a clever way to embed the actual library name in some other package metadata?


Suggested change

# Question: can anyone think of a clever way to embed the actual library name in some other package metadata?

eirrgang · 2025-12-12T16:51:58Z

tests/test_editable_generated.py

+    build_isolation_flags = []
+    if not build_isolation:
+        build_isolation_flags.append("--no-build-isolation")
+
+    # Use a context so that we only change into the directory up until the point where
+    # we run the editable install. We do not want to be in that directory when importing
+    # to avoid importing the source directory instead of the installed package.
+    with monkeypatch.context() as m:
+        package = PackageInfo("cmake_generated")
+        process_package(package, tmp_path, m)
+
+        assert isolated.wheelhouse
+
+        ninja = [
+            "ninja"
+            for f in isolated.wheelhouse.iterdir()
+            if f.name.startswith("ninja-")
+        ]
+        cmake = [
+            "cmake"
+            for f in isolated.wheelhouse.iterdir()
+            if f.name.startswith("cmake-")
+        ]
+
+        isolated.install("pip>23")
+        isolated.install("scikit-build-core", *ninja, *cmake)
+
+        isolated.install(
+            "-v",
+            *config_mode_flags,
+            *build_isolation_flags,
+            *editable_flag,
+            ".",
+        )


Suggested change

build_isolation_flags = []

if not build_isolation:

build_isolation_flags.append("--no-build-isolation")

# Use a context so that we only change into the directory up until the point where

# we run the editable install. We do not want to be in that directory when importing

# to avoid importing the source directory instead of the installed package.

with monkeypatch.context() as m:

package = PackageInfo("cmake_generated")

process_package(package, tmp_path, m)

assert isolated.wheelhouse

ninja = [

"ninja"

for f in isolated.wheelhouse.iterdir()

if f.name.startswith("ninja-")

]

cmake = [

"cmake"

for f in isolated.wheelhouse.iterdir()

if f.name.startswith("cmake-")

]

isolated.install("pip>23")

isolated.install("scikit-build-core", *ninja, *cmake)

isolated.install(

"-v",

*config_mode_flags,

*build_isolation_flags,

*editable_flag,

".",

)

# Use a context so that we only change into the directory up until the point where

# we run the editable install. We do not want to be in that directory when importing

# to avoid importing the source directory instead of the installed package.

with monkeypatch.context() as m:

package = PackageInfo("cmake_generated")

process_package(package, tmp_path, m)

assert isolated.wheelhouse

ninja = [

"ninja"

for f in isolated.wheelhouse.iterdir()

if f.name.startswith("ninja-")

]

cmake = [

"cmake"

for f in isolated.wheelhouse.iterdir()

if f.name.startswith("cmake-")

]

isolated.install("pip>23")

if not build_isolation:

isolated.install("scikit-build-core", *ninja, *cmake)

isolated.install(

"-v",

*config_mode_flags,

*editable_flag,

".",

isolated=build_isolation,

)

eirrgang and others added 4 commits December 10, 2025 17:01

Establish a new test structure for different resource layouts.

0426e9a

Check that a compiled binary can be used via ctypes

7bdec1e

Check other generated file use cases.

1ce8a55

style: pre-commit fixes

d280d76

LecrisUT reviewed Dec 12, 2025

View reviewed changes

eirrgang commented Dec 12, 2025

View reviewed changes

LecrisUT mentioned this pull request Dec 14, 2025

Consolidate editable and isolated fixtures #1194

Open



		def ctypes_function():
		# Question: can anyone think of a clever way to embed the actual library name in some other package metadata?

Test cases for different use cases of generated files #1193

Are you sure you want to change the base?

Test cases for different use cases of generated files #1193

Uh oh!

Conversation

eirrgang commented Dec 11, 2025

Uh oh!

eirrgang commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eirrgang commented Dec 11, 2025

Uh oh!

LecrisUT commented Dec 12, 2025

Uh oh!

LecrisUT left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

eirrgang commented Dec 11, 2025 •

edited

Loading