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

Including tests dir in sdist breaks install #2353

Closed
3 tasks done
TheErk opened this issue Jan 17, 2020 · 5 comments · Fixed by python-poetry/poetry-core#6 or #2268
Closed
3 tasks done

Including tests dir in sdist breaks install #2353

TheErk opened this issue Jan 17, 2020 · 5 comments · Fixed by python-poetry/poetry-core#6 or #2268
Assignees

Comments

@TheErk
Copy link

TheErk commented Jan 17, 2020

  • 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: Linux Debian Stretch

  • Poetry version: 1.0.2

  • Link of a Gist with the contents of your pyproject.toml file: https://gist.github.com/TheErk/21ebdae3b0a22a492c05bbacc495ebd0

Issue

I have a project whose structure is:

sdist-issue/
├── poetry.lock
├── pyproject.toml
├── README.rst
├── src
│   └── sdist_issue
│       └── __init__.py
└── tests
    ├── __init__.py
    └── test_sdist_issue.py

I want tp include the 'sdist-issue/tests' directory in my source distribution.
But when I use:

packages = [
    { include = "sdist_issue", from = "src" },
    { include = "tests", format = "sdist" }
]

poetry build works as expected while poetry install fails:

$ poetry install -vvv

[EnvCommandError]
Command ['/home/eric/VEnvs/py36/bin/pip', 'install', '-e', '/home/eric/Poetry/sdist-issue/sdist-issue'] errored with the following return code 1, and output: 
Obtaining file:///home/eric/Poetry/sdist-issue/sdist-issue
    ERROR: Command errored out with exit status 1:
     command: /home/eric/VEnvs/py36/bin/python3.6 -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'/home/eric/Poetry/sdist-issue/sdist-issue/setup.py'"'"'; __file__='"'"'/home/eric/Poetry/sdist-issue/sdist-issue/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' egg_info
         cwd: /home/eric/Poetry/sdist-issue/sdist-issue/
    Complete output (7 lines):
    running egg_info
    creating src/sdist_issue.egg-info
    writing src/sdist_issue.egg-info/PKG-INFO
    writing dependency_links to src/sdist_issue.egg-info/dependency_links.txt
    writing top-level names to src/sdist_issue.egg-info/top_level.txt
    writing manifest file 'src/sdist_issue.egg-info/SOURCES.txt'
    error: package directory 'src/tests' does not exist
    ----------------------------------------
ERROR: Command errored out with exit status 1: python setup.py egg_info Check the logs for full command output.


Traceback (most recent call last):
  File "/home/eric/.poetry/lib/poetry/_vendor/py3.6/clikit/console_application.py", line 131, in run
    status_code = command.handle(parsed_args, io)
  File "/home/eric/.poetry/lib/poetry/_vendor/py3.6/clikit/api/command/command.py", line 120, in handle
    status_code = self._do_handle(args, io)
  File "/home/eric/.poetry/lib/poetry/_vendor/py3.6/clikit/api/command/command.py", line 171, in _do_handle
    return getattr(handler, handler_method)(args, io, self)
  File "/home/eric/.poetry/lib/poetry/_vendor/py3.6/cleo/commands/command.py", line 92, in wrap_handle
    return self.handle()
  File "/home/eric/.poetry/lib/poetry/console/commands/install.py", line 88, in handle
    builder.build()
  File "/home/eric/.poetry/lib/poetry/masonry/builders/editable.py", line 17, in build
    return self._setup_build()
  File "/home/eric/.poetry/lib/poetry/masonry/builders/editable.py", line 41, in _setup_build
    self._env.run_pip('install', '-e', str(self._path))
  File "/home/eric/.poetry/lib/poetry/utils/env.py", line 843, in run_pip
    return self._run(cmd, **kwargs)
  File "/home/eric/.poetry/lib/poetry/utils/env.py", line 1071, in _run
    return super(VirtualEnv, self)._run(cmd, **kwargs)
  File "/home/eric/.poetry/lib/poetry/utils/env.py", line 875, in _run
    raise EnvCommandError(e, input=input_)

@TheErk
Copy link
Author

TheErk commented Jan 17, 2020

This have similarities with: #1009 but I think the problem here is simpler.

The functional need is: I want to be able to build a source tarball (sdist) of the package which includes any kind of file or directory in the source tree.

@DriverX
Copy link

DriverX commented Jan 25, 2020

+1 same problem
Workaround

include = [
    "tests/**/*",
]

But tests dir will be copied in wheel package too.

jtrakk referenced this issue in jtrakk/poetry Mar 3, 2020
If multiple distributions have `tests` as a top-level package, they'll conflict whenever both are installed. (Examples [here]((https://github.com/python-poetry/poetry/issues/1905) and [here](NixOS/nixpkgs#81482). Two common alternative strategies are:

1. not distributing tests (as [here](https://github.com/pypa/sampleproject)), or 
2. placing tests in a subdirectory of the main package, rather than adjacent (as [here](http://blog.habnab.it/blog/2013/07/21/python-packages-and-you/) and [here](http://as.ynchrono.us/2007/12/filesystem-structure-of-python-project_21.html)). Each of these strategies will avoid this issue. 

Users may fall into this trap because of the [package documentation](https://python-poetry.org/docs/pyproject/#packages) page, which gives an example:

```
packages = [
    { include = "my_package" },
    { include = "tests", format = "sdist" },
]
```

Having two top-level packages in a distribution is relatively unusual, but does have some use cases.  Using `"tests"` as a top-level package name in the example is likely to lead to conflicts, however. The alternate package in the documentation example could have a unique name like `"my_other_package"`, which would reduce the likelihood of this kind of overlap.
@kasteph kasteph self-assigned this Mar 6, 2020
@kasteph kasteph transferred this issue from python-poetry/poetry Mar 28, 2020
@decathorpe
Copy link
Contributor

I think adding tests to packages in pyproject.toml definitely does the wrong thing, and include should be used instead - because when installing from the sdist (e.g. from pypi), the tests package will get installed as well, which is not what users want or expect, and it will conflict with other tests packages that get installed.

I also need to patch out all tests packages and subpackages from setup.py files generated by poetry for fedora packaging because of this issue.

@kasteph
Copy link
Member

kasteph commented Mar 29, 2020

Hey @decathorpe I've made a PR (#6) to address this issue. It's awaiting review from the core team but if you would like, you can also leave a review on that PR. Thanks!

kasteph referenced this issue in python-poetry/poetry-core Apr 20, 2020
…ckage` directive.

Unify common logic between WheelBuilder and Builder. Move SDistBuilder logic from Builder to SDistBuilder.
Resolves: #8
kasteph referenced this issue in python-poetry/poetry-core Apr 20, 2020
…ckage` directive.

Unify common logic between WheelBuilder and Builder. Move SDistBuilder logic from Builder to SDistBuilder.
Resolves: #8
kasteph referenced this issue in python-poetry/poetry-core Apr 20, 2020
…ckage` directive.

Unify common logic between WheelBuilder and Builder. Move SDistBuilder logic from Builder to SDistBuilder.
Resolves: #8
kasteph referenced this issue in python-poetry/poetry-core Apr 20, 2020
…ckage` directive.

Unify common logic between WheelBuilder and Builder. Move SDistBuilder logic from Builder to SDistBuilder.
Resolves: #8
kasteph referenced this issue in python-poetry/poetry-core Apr 20, 2020
…ckage` directive.

Unify common logic between WheelBuilder and Builder. Move SDistBuilder logic from Builder to SDistBuilder.
Resolves: #8
kasteph referenced this issue in python-poetry/poetry-core Apr 20, 2020
…ckage` directive.

Unify common logic between WheelBuilder and Builder. Move SDistBuilder logic from Builder to SDistBuilder.
Resolves: #8
kasteph referenced this issue in python-poetry/poetry-core Apr 25, 2020
…ckage` directive.

Unify common logic between WheelBuilder and Builder. Move SDistBuilder logic from Builder to SDistBuilder.
Resolves: #8
kasteph referenced this issue in python-poetry/poetry-core Apr 25, 2020
…ckage` directive.

Unify common logic between WheelBuilder and Builder. Move SDistBuilder logic from Builder to SDistBuilder.
Resolves: #8
kasteph referenced this issue in python-poetry/poetry-core Apr 26, 2020
…ckage` directive.

Unify common logic between WheelBuilder and Builder. Move SDistBuilder logic from Builder to SDistBuilder.
Resolves: #8
abn referenced this issue in python-poetry/poetry-core Apr 26, 2020
- Unify common logic between WheelBuilder and Builder. 
- Move SDistBuilder logic from Builder to SDistBuilder.

Resolves: #8
@abn abn transferred this issue from python-poetry/poetry-core Apr 26, 2020
Copy link

github-actions bot commented Mar 3, 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 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants