-
Notifications
You must be signed in to change notification settings - Fork 64
Editable installs with paths involving symlinks may include incorrect directories in __path__ #647
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
Comments
I've put together an example in #648. Happy to work on a solution with a bit of guidance on preferred approaches, but also maintainers should feel free to push to my branch if there's an easy solution that you'd like to see. Thanks in advance either way! |
We don't support subdirectories all that well currently either, so it's something I'd also like to see improved. We might need a setting to tell scikit-build-core that it's N levels deep in a repo. Symlinks are a bit tricky, as they can't be passed through a zip file, either for wheels or editable wheels (due to a old CPython bug) - I'm guessing this means they will need special handing? Do you have any ideas currently? |
For the most cases if we are using editable installs, we have a local git repo where the sym links are already setup, so the zip issue shouldn't be that important. |
The problem is that editable installs in PEP 660 are set up as a transient wheel, which means they pass through a zip file, even if the zip file is not kept around. This only affects things that are "installed", though. |
Sorry I lost this notification in the scramble. Yeah, the fact that we pass through a zip file is an issue I wasn't sure how to overcome completely either. I think it would be a reasonable for such files to not be truly editable for an editable install but to instead be copied over. That would be a slightly unfaithful representation of the editable install, but it would avoid what is probably a worse problem in setting attributes of the installed package (the |
That would be fine, I think. We could also add more modes to choose from, https://tttapa.github.io/py-build-cmake/Editable-install.html has some interesting ideas. There is a "symlink" idea using a |
Hmm yeah that solution does seem like a reasonable out. The "hook" approach is basically scikit-build-core's "redirect". The "symlink" approach could be added analogously. Then you would presumably categorize files by type to determine whether they are symlinked or copied in. |
The VERSION file is meant to be a single source of truth for the package version that we read everywhere. As a result, we also symlink it in a handful of places that need it, in particular in places where we build a Python package and want to read the version to embed it into the package metadata. Unfortunately, due to scikit-build/scikit-build-core#647, this results in editable installs of our packages having an incorrect `__path__` attribute that includes the root directory of the repository (where the real VERSION file lives) in addition to the root directory of the package (where the symlink lives). In most cases this causes no real problems, but it is problematic for cudf.pandas in particular because [we use the `__path__` to set the denylist](https://github.com/rapidsai/cudf/blob/35b18f408d75e30b40e9ff83d72df1904f46f18f/python/cudf/cudf/pandas/module_accelerator.py#L410). Since the root directory of the repository gets added to the denylist and that is where the tests live, tests using cudf.pandas do not work when run with editable installations. Note that the same problem does not arise when a pytest tst is converted to a normal Python module and run with `python -m cudf.pandas ${TEST_FILE}` because in that case [the `calling_module` we get from the frame](https://github.com/rapidsai/cudf/blob/35b18f408d75e30b40e9ff83d72df1904f46f18f/python/cudf/cudf/pandas/module_accelerator.py#L558) does not contain the full path to the file, only the raw filename. That is normally what you would expect as a user running an arbitrary script where the frames would only include full path to things outside of your working tree. The problem is that the main program when you run tests is pytest, and pytest has to then launch runs using the full paths to the test files. We should revert this change whenever scikit-build/scikit-build-core#647 is resolved. Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Matthew Murray (https://github.com/Matt711) - Bradley Dice (https://github.com/bdice) - GALI PREM SAGAR (https://github.com/galipremsagar) URL: #18198
For use cases like those supported by #362 (monorepos containing subdirectories with Python packages) it is not uncommon for the package name directories to be identical to the monorepo directories. If any files are symlinked from outside the Python package source, this causes problems for how scikit-build-core generates the paths used for the redirecting finder when building editable packages. The modules paths that are passed to
_make_editable
are absolutized, which means that is how they are embedded into theinstall
call in the_${pkg}_editable.py
file. The end result is that the__path__
produced when the package is imported may erroneously contain both the Python package directory and the higher-level directory in the monorepo.The text was updated successfully, but these errors were encountered: