search

python-poetry / poetry

Python packaging and dependency management made easy
https://python-poetry.org
MIT License
31.61k stars 2.26k forks source link

Exclude does not work for binary artifacts #5441

Closed nylocx closed 3 days ago

nylocx commented 2 years ago

Issue

First thanks for this great project, can't be said too often.

This issue was partially fixed by the commit mentioned in https://github.com/python-poetry/poetry/issues/4711, but there is still a bug in poetry core (current master). https://github.com/python-poetry/poetry-core/blob/master/src/poetry/core/masonry/builders/wheel.py#L181-L184

This get the relative path after the is_excluded test but the is_excluded function expects a path relative to the source root. So even if you ignore a file that is later build into build/lib*/ it is then copied into the wheel. This can be easily fixed by changing the order and doing it like

rel_path = str(pkg.relative_to(lib))
if pkg.is_dir() or self.is_excluded(rel_path):
    continue

 if rel_path in wheel.namelist():
    continue

I hope I got it right, it is a bit hard without a debugger to get the application flow.

Kind regards, Alex

nylocx commented 2 years ago

Any chance someone could have a look at this? I currently manually edit my install to get it to work. I could try to create a PR for the change above, but I'm not sure if I am missing something important.

nylocx commented 2 years ago

This issue is still bothering us and the "patch" I provided above is no longer working for projects with a src layout. After a short discussion with @finswimmer in Discord I created a minimal example to reproduce this issue.

minimal_example.zip

The minimal example includes a Dockerfile that I used to verify the behavior. To reproduce run

docker build -t example .
docker run -it --rm example

This will put you in a bash shell where you can run

poetry build -vvv -f wheel
unzip -l dist/*.whl
poetry_cython_demo/cython_code.pyx
poetry_cython_demo/compiled/source.c

are included in the wheel and according to the excludes in the pyproject.toml they should't be part of the wheel.

exclude = ["src/poetry_cython_demo/compiled", "src/poetry_cython_demo/*.pyx", "poetry_cython_demo/compiled", "poetry_cython_demo/*.pyx"]

If you need more input please let me know.

tueda commented 2 years ago

I think I am hitting a similar issue in an example with scikit-build. The following commands (with Poetry version: 1.1.13, C++ compiler required)

git clone https://github.com/tueda/test-poetry-scikit-build.git
cd test-poetry-scikit-build
poetry install
poetry build -vvv

shows that the following files are included in both wheel and sdist:

  - Adding: /home/tueda/tmp/test-poetry-scikit-build/hello/_hello.cpython-310-x86_64-linux-gnu.so
  - Adding: /home/tueda/tmp/test-poetry-scikit-build/hello/_hello.cxx

while they are filtered out by tool.poetry.exclude but selectively specified in tool.poetry.include with the format option:

include = [
    { path = "CMakeLists.txt", format = "sdist" },
    { path = "hello/**/*.cxx", format = "sdist" },
    { path = "hello/**/*.so", format = "wheel" },
]
exclude = [
    "hello/**/*.cxx",
    "hello/**/*.so",
]
horta commented 1 year ago

Same here: the binaries are being included into sdist and wheel. Significant part of pyproject.toml:

packages = [{ include = "hmmer" }]
exclude = [{ path = "hmmer/data/bin/*", format = "sdist" }]
include = [
  { path = "hmmer/data/bin/*", format = "wheel" },
  { path = "build_hmmer.py", format = "sdist" },
  { path = "build_hmmer.sh", format = "sdist" },
]

Files inside hmmer/data/bin/*

Repo: https://github.com/EBI-Metagenomics/hmmer-py

finswimmer commented 3 days ago

I cannot reproduce the behavior anymore with the minimal example given by @nylocx in https://github.com/python-poetry/poetry/issues/5441#issuecomment-1156251608

So I assume we have fixed it at some point.

If anyone disagree, feel free to leave a comment and a new minimal example to reproduce the issue.

tueda commented 3 days ago

It now works for my case above. Thanks!