Support getting files in the build tree from editable installs

[vyasr]

Resolves #807

[vyasr]

@LecrisUT here's the example for #807

[LecrisUT]

What is the type you get with importlib.resources.files(example)? It should be MultiplexedPath

Trying to refresh my reading of the workflow

[vyasr]

No, it is currently producing a PosixPath. But that is a good pointer, that does seem like the way that this should work. This seems similar to what I reported in #647.

[vyasr]

The fact that this is specifying a single directory (the redir) is perhaps part of where we could improve things. (Sub)packages are found in the known source files list by their __init__.py. As a result, that path is used as the root, which means that we cannot find files nested in the package but present in the build directory because the package does not exist in known_wheel_files.

[LecrisUT]

I remember submodule_search_locations was an important aspect that we converged to. Check around the execution of:

scikit-build-core/src/scikit_build_core/resources/_editable_redirect.py

Lines 83 to 84 in 9d49270

	if fullname in self.submodule_search_locations:
	submodule_search_locations = list(self.submodule_search_locations[fullname])

If self.submodule_search_locations['example'] has indeed a list of 2 then I think we have it wrong in the assumption of:

scikit-build-core/src/scikit_build_core/resources/_editable_redirect.py

Lines 91 to 95 in 9d49270

	return importlib.util.spec_from_file_location(
	fullname,
	os.path.join(self.dir, redir),
	submodule_search_locations=submodule_search_locations,
	)

(fullname == "example" should be called there)

[vyasr]

Indeed, that seems to be it. I added

        if fullname in self.submodule_search_locations:
            print(
                f"The submodule_search_locations for {fullname} "
                f"are {self.submodule_search_locations[fullname]}"
             )
             submodule_search_locations: list[str]  = list(self.submodule_search_locations[fullname])

and I see

(skbuild_dev) vyasr@vyasr-mlt test % python -c "import example"
The submodule_search_locations for example are {'/Users/vyasr/miniconda3/envs/skbuild_dev/lib/python3.11/site-packages/example', '/Users/vyasr/local/testing/skbc_importlib/example'}

What would you recommend that we do?

[LecrisUT]

Is this with the default loader or the new one. If it's the same on the default, then we need to rethink it. One reference would be what do setuptools and hatchling do for namespace packages. I remember one of them didn't support it at that time. Meson and cargo could be additional references, but those would be harder to navigate and might not be as minimal.

Worst case is we go for the method of meson defining full Loaders, but it's not ideal. The difficulty iirc was with getting everything else other than files interface to work correctly, e.g. reading the source code.

[vyasr]

The output is the same both with and without the loader (commenting out # loader=ScikitBuildRedirectingLoader(fullname, redir),).

[vyasr]

@LecrisUT any other thoughts on how we ought to proceed here? I've committed the test to the repo so hopefully it's easy enough to reproduce the issue if you need to.

[LecrisUT]

@LecrisUT any other thoughts on how we ought to proceed here? I've committed the test to the repo so hopefully it's easy enough to reproduce the issue if you need to.

It is a difficult one and I think we need to separate what we are testing here:

• import for python modules .py and .so with editable and without.
  This is the minimal that should be tested and work. This is already in the test suite, but I think it will help if it is easier to find
• Navigate packages with import (same as previous point but focus on covering project structures). I was thinking of covering this with test: editable install cover package navigation #535 but I don't like how hard it is to follow that one and track what navigation we are covering. It would help a lot if at least this part can be revisited.
• Navigate packages using relative path, i.e. within the source code
• Navigating using importlib.resources. If we cover the previous point, this part would be easier to follow. This part has too many moving pieces. What about covering the following way:
  • Go from testcase and work backwards to implementation
  • Test for the leaf (pkgA.pkgB.moduleC) and make sure it is always a pathlib.Path pointing to the resource in all cases
  • Test for the branches (pkgA, pkgA.pkgB) and make sure it is a pathlib.Path when not editable and a MultiplexedPath when editable
  • Naked modules (moduleD) always points to pathlib.Path
    Normally should be discouraged, but for prototyping I think this should be covered as well
  • Find how to fix the implementation
    • We should try to use as much vanilla functionality as possible, otherwise, make sure to cover execution, read source and relative navigation
    • Construct a virtual tree of pathlib.Path or MultiplexedPath at ScikitBuildRedirectingFinder constructor
    • Inject the virtual tree nodes in find_spec (get ready for all tests to breakdown from here)
    • Worst case scenario is we create our own Loaders as you made here, but if that's the case it is important to cover separately pkg vs python-module vs c-module (are there any c-package to worry about as well?), Extension/Source/Sourceless file loaders (there might be more) [^1] . We don't need to go as far as creating our own Traversable in this case

[^1] : https://github.com/mesonbuild/meson-python/blob/main/mesonpy/_editable.py

[vyasr]

That sounds like a solid plan. I don't follow the difference between your second and third top-level bullets, but in general the approach of adding tests for each of the possible cases and working backwards from there sounds right. This PR adds a test for one specific case from that list. I can work on slowly building this up. JFYI it will take me a few weeks to get back to this PR since I'll be traveling for the next two weeks.

@henryiii would you be open to me creating a PR that is just a bunch of xfailed tests indicating functionality that we would like to work and getting that merged then working backwards to fixing tests? I fear that the number of different cases alone will make this kind of work hard to keep track of, so getting test cases merged and consistently tested would already be a good starting point to indicate what we aspirationally want working.

[LecrisUT]

About 2nd and 3rd point, it is subtle but basically it involves how the Loader, submodule_search_locations, etc. interact. You remember the last PR on this, it basically addressed that.

[vyasr]

@henryiii friendly ping. Does the above approach of adding xfailed tests first sound OK?

[LecrisUT]

I think you should go ahead with the xfail approach. If needed I will open a branch and you can move the target to there.

[henryiii]

Ah, sorry, yes the xfail approach is preferred.

[vyasr]

Cool. I head out on vacation on Thursday and have a number of things to wrap up, so I may not get to this by then. If not, then I'll revisit in about 3 weeks (I'm off for 2, will realistically need a week to get caught up on everything else again).

[vyasr]

Working on the tests in #906.