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

poetry-core does not execute build function in build.py #7470

Closed
4 tasks done
cdgriffith opened this issue Feb 4, 2023 · 8 comments
Closed
4 tasks done

poetry-core does not execute build function in build.py #7470

cdgriffith opened this issue Feb 4, 2023 · 8 comments
Labels
kind/bug Something isn't working as expected status/triage This issue needs to be triaged

Comments

@cdgriffith
Copy link

  • Poetry version: 1.3.2
  • Python version: 3.10.9
  • OS version and name: Windows 11
  • pyproject.toml:
  • I am on the latest stable Poetry version, installed using a recommended method.
  • I have searched the issues of this repo and believe that this is not a duplicate.
  • I have consulted the FAQ and blog for any relevant entries or release notes.
  • If an exception occurs when executing a command, I executed it again in debug mode (-vvv option) and have included the output below.

Issue

Trying to switch to the lighter poetry-core for building wheels, but the build function is not executed inside the build.py file.

Using poetry.core as the build backend, I will only see Build File Imported (custom print statements in build.py) and neither build function is called.

However if I switch the build system back to the old method of just poetry:

[build-system]
requires = ["poetry", "Cython", "setuptools", "wheel"]
build-backend = "poetry.masonry.api"

Everything will work fine when I run pip wheel . -v and show both:

Build File Imported
Build with Cython

build.py

#!/usr/bin/env python
# -*- coding: utf-8 -*-
import os
from setuptools.command.build_ext import build_ext

from pathlib import Path
import shutil

root = Path(__file__).parent

print("Build File Imported")

def copy_files():
    # Why does poetry put include files at the root of site-packages? So bad! Copy them inside the package
    shutil.copy(root / "AUTHORS.rst", root / "box" / "AUTHORS.rst")
    shutil.copy(root / "CHANGES.rst", root / "box" / "CHANGES.rst")
    shutil.copy(root / "LICENSE", root / "box" / "LICENSE")
    shutil.copy(root / "box_logo.png", root / "box" / "box_logo.png")


try:
    from Cython.Build import cythonize
except ImportError:
    # Got to provide this function. Otherwise, poetry will fail
    def build(setup_kwargs):
        print("Build without Cython")
        copy_files()


# Cython is installed. Compile
else:
    # This function will be executed in setup.py:
    def build(setup_kwargs):
        print("Build with Cython")
        copy_files()
        # Build
        setup_kwargs.update(
            {
                "ext_modules": cythonize(
                    [
                        str(file.relative_to(root))
                        for file in Path(root, "box").glob("*.py")
                        if file.name != "__init__.py"
                    ],
                    compiler_directives={"language_level": 3},
                ),
            }
        )

Package itself is https://github.com/cdgriffith/Box/blob/master/pyproject.toml and issue brought up by cdgriffith/Box#244

@cdgriffith cdgriffith added kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Feb 4, 2023
@dimbleby
Copy link
Contributor

dimbleby commented Feb 4, 2023

probably broken by python-poetry/poetry-core#318, try

[tool.poetry.build]
script = "build.py"
generate-setup-file = true

@cdgriffith
Copy link
Author

That does allow it to work!

Does create a unwanted setup.py file. Is this simply new behavior, or will this be something fixed in later versions, that poetry-core will be able to call build?

@dimbleby
Copy link
Contributor

dimbleby commented Feb 4, 2023

setting this explicitly is simply restoring the old behaviour - the default changed

@cdgriffith
Copy link
Author

Got ya, thank you for the fast help @dimbleby !

arkamar added a commit to arkamar/gentoo that referenced this issue Feb 5, 2023
Synapse has frequent releases, we can limit poetry-core to <1.5.0 now
and wait for the next version where the issue will be solved.

Bug: https://bugs.gentoo.org/893244
See-also: matrix-org/synapse#14949
See-also: python-poetry/poetry#7470
Signed-off-by: Petr Vaněk <arkamar@atlas.cz>
arkamar added a commit to arkamar/gentoo that referenced this issue Feb 5, 2023
Synapse has frequent releases, we can limit poetry-core to <1.5.0 now
and wait for the next version where the issue will be solved.

Bug: https://bugs.gentoo.org/893244
See-also: matrix-org/synapse#14949
See-also: python-poetry/poetry#7470
Signed-off-by: Petr Vaněk <arkamar@atlas.cz>
gentoo-bot pushed a commit to gentoo/gentoo that referenced this issue Feb 5, 2023
Synapse has frequent releases, we can limit poetry-core to <1.5.0 now
and wait for the next version where the issue will be solved.

Bug: https://bugs.gentoo.org/893244
See-also: matrix-org/synapse#14949
See-also: python-poetry/poetry#7470
Signed-off-by: Petr Vaněk <arkamar@atlas.cz>
Closes: #29379
Signed-off-by: Sam James <sam@gentoo.org>
@JacobHayes
Copy link

JacobHayes commented Feb 8, 2023

mistaken OP

I think the old

[tool.poetry]
build = "build.py"

behaves differently than

[tool.poetry.build]
script = "build.py"
generate-setup-file = true

The old method would include generated shared objects in the built wheels, however a wheel built with [tool.poetry.build] will be missing shared objects. I confirmed that by running the following snippet with a package using Cython for both styles of config:

$ rm src/{**/,}*.cpython-*.so # Delete cython generated .cpython-*.so files
$ rm -rf build/ dist/
$ poetry build -f wheel
$ strings dist/*.whl | grep '\.cpython.*.so'  # Output for `build = `, no output for `script = `

Is this intentional or perhaps an accidental change? (it does seem to still occur even with poetry-core>=1.0.0,<1.5.0, so I don't think it's a 1.5.0 only issue) If this is not intentional, I'm happy to put together a full reproduction.

I totally got mixed up across tests of editable installs, wheel builds, and toggling the flag above. However, I think I now know what it does in each situation:

  • 🔴 poetry install --only-root with generate-setup-file = true: the build script is called without build isolation (leading to missing Cython / other build deps for me)
  • 🟡 poetry build -f wheel with generate-setup-file = false: the build script is called with build isolation, however:
    • the build function is not called (fair enough, not using setup, so setup_kwargs doesn't make sense), just ran as <build isolation venv>/bin/python3 <script>
    • the PATH does not seem to include the build isolation venv's bin dir, so subprocess calling a tool like cythonize do not work without doing something like str(Path(sys.executable).parent / "cythonize")
  • 🟢 poetry build -f wheel with generate-setup-file = true: the build script (and build func) is called with build isolation and the shared objects are bundled into the wheel
  • 🟢 poetry install --only-root with generate-setup-file = false: the build script is called with build isolation and the shared objects placed back in the correct spot in the source

Given those last two are mutually exclusive and setup.py will 🪓 eventually, I think setting generate-setup-file = false and working around the PATH issue (if this is not expected, I'd be happy to PR if anyone has a few pointers!) is the path I'll take. Here's the build script I'm using for cython now:

import sys
from pathlib import Path
from shutil import rmtree
from subprocess import check_call

SRC_DIR = Path(__file__).parent.parent.absolute() / "src". # NOTE: I have this in a `scripts/` dir next to `src/`, adjust if necessary
CYTHON_MODULES = sorted(SRC_DIR.rglob("*.pyx"))


if __name__ == "__main__":
    if CYTHON_MODULES:
        # NOTE: The PATH doesn't seem to always be set correctly when called via Poetry, so ensure we use the cythonize
        # relative to our python binary (which may be from the build isolation venv).
        check_call([str(Path(sys.executable).parent / "cythonize"), "-i", *CYTHON_MODULES])
        # If using namespace packages, cythonize will create build/ dirs above the nearest `__init__.py`, which may
        # incorrectly be bundled into the wheel.
        for build_dir in SRC_DIR.rglob("build/"):
            rmtree(build_dir)

NOTE: I also had to add include = ["dyno/**/*.so"] under [tool.poetry] to override a *.so entry in my .gitignore .

(also, hey @cdgriffith, small world 😄)

SokolovYaroslav pushed a commit to JetBrains-Research/YouTokenToMe that referenced this issue Feb 17, 2023
SokolovYaroslav pushed a commit to JetBrains-Research/YouTokenToMe that referenced this issue Feb 17, 2023
SokolovYaroslav pushed a commit to JetBrains-Research/YouTokenToMe that referenced this issue Feb 17, 2023
SokolovYaroslav pushed a commit to JetBrains-Research/YouTokenToMe that referenced this issue Feb 17, 2023
@jcampbell05
Copy link

@dimbleby is there a plan to make a more stable API around this ? I've started building a plugin to try to handle Cython building like this.

The fact it's an unstable API at the moment makes it unfortunately more work to rely on this behaviour as understandable change-logs won't mention behaviour changes like this.

@dimbleby
Copy link
Contributor

dimbleby commented Mar 8, 2023

#2740

Copy link

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 Feb 29, 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 status/triage This issue needs to be triaged
Projects
None yet
Development

No branches or pull requests

4 participants