Skip to content
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

Having from with multiple packages will use last from for all of them #1811

Closed
3 tasks done
daaain opened this issue Dec 31, 2019 · 11 comments · Fixed by python-poetry/poetry-core#108
Closed
3 tasks done
Labels
kind/bug Something isn't working as expected

Comments

@daaain
Copy link

daaain commented Dec 31, 2019

  • I am on the latest Poetry version.

  • I have searched the issues of this repo and believe that this is not a duplicate.

  • If an exception occurs when executing a command, I executed it again in debug mode (-vvv option).

  • OS version and name: python:3.7 Docker image

  • Poetry version: 1.0.0

Issue

If there is more than one package in packages with from set, the last one will be used for all. Even worse, packages without from will also be attempted to be resolved inside the last from directory.

So with:

packages = [
	{ include = "apple" },
    { include = "pear", from="pear_tree" },
    { include = "orange", from="orange_tree" },
]

Poetry will try to resolve both apple and pear from inside orange_tree as far as I observed from debug output.

If it's a known limitation that using from only one package should be defined then the docs (at https://python-poetry.org/docs/pyproject/#packages) should definitely mention it as I was stumped by it for hours.

@daaain daaain added the kind/bug Something isn't working as expected label Dec 31, 2019
@finswimmer
Copy link
Member

Hello @daaain ,

could you please provide a full minimal example of the folder structure and the pyproject.toml with which one can reproduce the issue? That's makes it easier to debug.

Thanks a lot 👍

fin swimmer

@abadger
Copy link

abadger commented May 21, 2020

EDIT: I think this is a different bug but would probably be masked if this bug didn't exist. I'll open a different bug report for this but also generate a different minimal reproducer that applies to this bug.

I'm not the original reporter but I believe I have the same issue. Minimal reproducer below:

$ wget https://toshio.fedorapeople.org/trees.tar.gz                                  (01:08:06)
$ tar -xzf trees.tar.gz                                                             (01:08:12)
$ cd trees                                                                           (01:08:16)
$ poetry install                                                               (01:08:20)
Installing dependencies from lock file

No dependencies to install or update

  - Installing trees (0.1.0)

[EnvCommandError]
Command ['/home/badger/.cache/pypoetry/virtualenvs/trees-qCG1GaiB-py3.7/bin/pip', 'install', '-e', '/var/tmp/trees'] errored with the following return code 1, and output: 
Obtaining file:///var/tmp/trees
    ERROR: Complete output from command python setup.py egg_info:
    ERROR: running egg_info
    writing src/trees.egg-info/PKG-INFO
    writing dependency_links to src/trees.egg-info/dependency_links.txt
    writing top-level names to src/trees.egg-info/top_level.txt
    error: package directory 'src/test' does not exist
    ----------------------------------------
ERROR: Command "python setup.py egg_info" failed with error code 1 in /var/tmp/trees/
WARNING: You are using pip version 19.1.1, however version 20.1.1 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
```</strike>

@abadger
Copy link

abadger commented May 21, 2020

Perhaps also of help:

$ poetry --version                                                             (01:14:22)
Poetry version 1.0.5

@abadger
Copy link

abadger commented May 21, 2020

Looks like the temporary setup.py that is generated has:

package_dir = \
{'': 'src'}

packages = \
['apple', 'test']

So that is wrong, at least.

@abadger
Copy link

abadger commented May 21, 2020

I started using poetry so I could forget everything I ever knew about setuptools and distutils ;-) but I think that the package_dir should look more like this:

package_dir = \
{'': 'src',       
 'test': '.'}

It's a bit late here so I may not be thinking clearly (and I know there are subtle ways to get package_dir wrong) so take that with a grain of salt. I did try forcing poetry to pause after writing setup.py, modifying the package_dir to be the above, and then continue and install did work if I intervened like that. So it's closer to correct, at least.

@abadger
Copy link

abadger commented May 21, 2020

Okay it's morning and I wonder if there's two bugs here and my example is running across both of them while the original poster is only running across one of them. In my example, one of the two things listed in include is not a package (test is only supposed to be included in the sdist, not as a python package for install).

So I think the bug in my case is that the generated setup.py puts test into the packages list:

packages = \
['apple', 'test']

The original reporter, however, did not use format="sdist", so for them, it's package_dir which is going to be wrong (I can tell that by looking at the code.... every include is being saved into the same key in package_dir: the one for empty string"". So the code is overwriting the from= instead of adding to it, exactly the symptoms in the original report.)

@abadger
Copy link

abadger commented May 21, 2020

Okay, minimal reproducer for this bug:

[pts/7@peru /var/tmp]$ wget https://toshio.fedorapeople.org/fruit.tar.gz
[pts/7@peru /var/tmp]$ tar -xzf fruit.tar.gz
[pts/7@peru /var/tmp]$ cd fruit
[pts/7@peru /var/tmp/fruit]$ poetry install
Installing dependencies from lock file

No dependencies to install or update

  - Installing fruit (0.1.0)

[EnvCommandError]
Command ['/home/badger/.cache/pypoetry/virtualenvs/trees-qCG1GaiB-py3.7/bin/pip', 'install', '-e', '/var/tmp/fruit'] errored with the following return code 1, and output: 
Obtaining file:///var/tmp/fruit
    ERROR: Complete output from command python setup.py egg_info:
    ERROR: running egg_info
    writing lib/fruit.egg-info/PKG-INFO
    writing dependency_links to lib/fruit.egg-info/dependency_links.txt
    writing top-level names to lib/fruit.egg-info/top_level.txt
    error: package directory 'lib/apple' does not exist
    ----------------------------------------
ERROR: Command "python setup.py egg_info" failed with error code 1 in /var/tmp/fruit/
WARNING: You are using pip version 19.1.1, however version 20.1.1 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.

here's the relevant section from the setup.py that poetry generates for the install case:

package_dir = \
{'': 'lib'}

packages = \
['apple', 'pear']

I believe for this bug, package_dir needs to include both lib and src. I can't find good documentation for package_dir that goes into detail about how to use it so I'm not sure of when empty string is appropriate and when it is not. In another project that I work on, we have:

    package_dir={'': 'lib',
                 'ansible_test': 'test/lib/ansible_test'}

where the python package that matches the sdist name is what is present in lib. I don't know if there's any difference between that and this, though:

    package_dir={'ansible': 'lib',
                 'ansible_test': 'test/lib/ansible_test'}

(I'll take the other case to a different bug report.

@abadger
Copy link

abadger commented May 24, 2020

Oops, I see that when I initially posted the simple reproducer for this, I left off the URL for the tarball containing the reproducer. That's fixed now.

I saw that in develop branch there's been some changes to install which could be relevant. I tested the reproducer against that. The good news is that install now completes successfully. The bad news is that it didn't do the right thing.

poetry install for this test case generates a fruit.pth that points to the toplevel dir:

$ cat /home/badger/.cache/pypoetry/virtualenvs/trees-qCG1GaiB-py3.8/lib/python3.8/site-packages/fruit.pth 
/var/tmp/fruit

But neither of the packages in the test case reside in the toplevel dir. They reside in the subdirs, /var/tmp/fruit/src and /var/tmp/fruit/lib. So there needs to be two new pth entries, one for each of those.

Poetry hash on the develop branch which I tested: abee30f

@JeppeKlitgaard
Copy link

I am running into the same problem on a project. Admittedly it is a rare use case to have a split project structure with multiple packages, but it would be super nice if this was supported.

@dmohns
Copy link

dmohns commented Sep 17, 2020

If I understand correctly the code that is causing this issue is https://github.com/python-poetry/poetry-core/blob/master/poetry/core/masonry/builders/sdist.py#L134-L135

I was tinkering around with something like this

if pkg_dir is not None:
    package_dir[include.package] = os.path.join(
        os.path.relpath(pkg_dir, str(self._path)),
        include.package
    )

but this is also not doing the right thing. I have to admit I know to little about setuptools to understand what is the correct way to go here.

jaharkes added a commit to jaharkes/poetry-core that referenced this issue Nov 16, 2020
When we define multiple packages from different source locations,
Poetry currently only uses the last specified from=. This patch adds
explicit paths to package_dir for any additional packages.

This fixes python-poetry/poetry#1811, python-poetry/poetry#2354

And possibly python-poetry/poetry#2450
jaharkes added a commit to jaharkes/poetry-core that referenced this issue Nov 16, 2020
When we define multiple packages from different source locations,
Poetry currently only uses the last specified from=. This patch adds
explicit paths to package_dir for any additional packages.

This fixes python-poetry/poetry#1811 python-poetry/poetry#2354
And possibly python-poetry/poetry#2450
jaharkes added a commit to jaharkes/poetry-core that referenced this issue Nov 16, 2020
When we define multiple packages from different source locations,
Poetry currently only uses the last specified from= location.
This patch adds explicit paths to package_dir for additional packages.

This fixes python-poetry/poetry#1811, fixes python-poetry/poetry#2354,
and possibly even python-poetry/poetry#2450.
jaharkes added a commit to jaharkes/poetry-core that referenced this issue Jun 3, 2021
When we define multiple packages from different source locations,
Poetry currently only uses the last specified from= location.
This patch adds explicit paths to package_dir for additional packages.

This fixes python-poetry/poetry#1811, fixes python-poetry/poetry#2354,
and possibly even python-poetry/poetry#2450.
jaharkes added a commit to jaharkes/poetry-core that referenced this issue Nov 10, 2021
When we define multiple packages from different source locations,
Poetry currently only uses the last specified from= location.
This patch adds explicit paths to package_dir for additional packages.

This fixes python-poetry/poetry#1811, fixes python-poetry/poetry#2354,
and possibly even python-poetry/poetry#2450.
neersighted pushed a commit to python-poetry/poetry-core that referenced this issue Nov 10, 2021
* Fix for including modules from different locations

When we define multiple packages from different source locations,
Poetry currently only uses the last specified from= location.
This patch adds explicit paths to package_dir for additional packages.

This fixes python-poetry/poetry#1811, fixes python-poetry/poetry#2354,
and possibly even python-poetry/poetry#2450.

* Test the fix for including modules from different locations

When we try to include moduleA from libA and moduleB from libB then
package_dir in the generated setup.py must to contain either one or
both `"moduleA": "libA/moduleA"` or `"moduleB": "libB/moduleB"`
so we are able to find both modules when building the source dist.

`ns["package_dir"].keys() == {"", "module_b"}`
should always be true, so we don't have to test for module_a in
`ns["package_dir"]`.
Copy link

github-actions bot commented Mar 2, 2024

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Something isn't working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants