When Building Wheels, Resolve Relative Path Dependencies Correctly #2114
Conversation
Building wheels unpacks the sdist which includes a copy of the
`pyproject.toml`. If dependencies are delcared using relative paths
(such as `mymodule = { path = '../mymodule' }`), these relative paths
are not available from the temporary directory created to unpack and
build the wheel. This fix retains the original path to the project for
those relative dependencies.
Fixes python-poetry#266.
kevinastone
force-pushed
the
build-path-dependencies
branch
from
6ecf519
to
01e9fba
Compare
|
Is there any chance this will be approved? It seems the same problem affects checking these projects using "tox". |
There was a problem hiding this comment.
While the fix will make the builds succeed, it does changes the current behaviour since the source files used for the package now relies on the original root instead of the isolated source copy. The resolution of relative paths outside of project's source is probably not something we want to encourage as this can cause all sorts of breakages.
For cases where the source is within the same direcotry, another approach to this might be the inclusion of the path explicitly once python-poetry/poetry-core#6 is merged and available.
Another approach could be to use the original root only for path resoultion if the path encountered cannot be found.
| @@ -30,7 +30,7 @@ class Factory: | |||
| """ | |||
|
|
|||
| def create_poetry( | |||
| self, cwd=None, io=None | |||
| self, cwd=None, io=None, original_root=None, | |||
| @@ -57,7 +57,7 @@ def create_poetry( | |||
| name = local_config["name"] | |||
| version = local_config["version"] | |||
| package = ProjectPackage(name, version, version) | |||
| package.root_dir = poetry_file.parent | |||
| package.root_dir = original_root or poetry_file.parent | |||
There was a problem hiding this comment.
This will sort of defeat the purpose, since we really want the build to use the source in cwd. We might want to consider another approach to this one.
There was a problem hiding this comment.
So after reading #2046 and especially the fact that these relative paths work fine if you build ONLY the wheel or ONLY the sdist... I have to ask, where is the need for this "isolated source copy" coming from? Building only the sdist doesn't seem to need it, building only the wheel doesn't seem to need it. So why is the CompleteBuilder trying to do something more than simply running both individual builders?
|
It sounds like paths outside of the project source are not supported then. Are there use-cases where relative paths work correctly? I've been trying to build more monorepo repositories, but they all fail on this wheel unpacking step. I'd recommend removing support for relative paths given all the footguns in the current implementation. |
|
Well, there are use cases where projects are not built/distributed where it would work. Like an application project. So, I'd be reluctant to support the removal of support relative paths. As for your particular use case, I recently raised #2270 as a means to enable cases like this. This should provide a cleaner approach to managing python mono-repos. It will require a bit of discussion before finalising how it would work exactly. So, feedback is welcome there. |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Building wheels unpacks the sdist which includes a copy of the
pyproject.toml. If dependencies are declared using relative paths(such as
mymodule = { path = '../mymodule' }), these relative pathsare not available from the temporary directory created to unpack and
build the wheel. This fix retains the original path to the project for
those relative dependencies.
Fixes #266.
Fixes #2046
Pull Request Check List
This is just a reminder about the most common mistakes. Please make sure that you tick all appropriate boxes. But please read our contribution guide at least once, it will save you unnecessary review cycles!
Note: If your Pull Request introduces a new feature or changes the current behavior, it should be based
on the
developbranch. If it's a bug fix or only a documentation update, it should be based on themasterbranch.If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!