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

Errors when using "setup.py bdist_wheel" with pyproject.toml #435

Open
f-cozzocrea opened this issue May 23, 2023 · 6 comments
Open

Errors when using "setup.py bdist_wheel" with pyproject.toml #435

f-cozzocrea opened this issue May 23, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@f-cozzocrea
Copy link

I'm seeing an issue when whenever I'm trying to install an extension with the setup.py bdist_wheel + pip install method with a very basic pyproject.toml file that looks like this:

[project]
name = "quickstart"
version = "0.0.1"

This is the error output:

error: Error: setup script specifies an absolute path:

    /home/f-cozzocrea/projects/hpy-pep517/venv/hpy-pep517/lib/python3.10/site-packages/hpy/devel/src/runtime/argparse.c

setup() arguments must *always* be /-separated paths relative to the
setup.py directory, *never* absolute paths.

I've created a basic repository here with steps to reproduce the errors: https://github.com/f-cozzocrea/hpy-pep517

--- System info ---
Hpy version: 0.9.0rc2
CPython version: 3.10
OS: Ubuntu 22.04

@f-cozzocrea f-cozzocrea changed the title Errors when using bdist_wheel and pip install with --use-pep517 or pyproject.toml Errors when using "setup.py bdist_wheel" with pyproject.toml May 23, 2023
@fangerer
Copy link
Contributor

Hi @f-cozzocrea! Thank you again for this issue.
@hodgestar: This is the problem I've mentioned in our last call. Newer setuptools requires to have relative paths in the sources list. Making relative paths to HPy runtime sources doesn't make a lot of sense to me. What do you think? Should we copy the sources to the extension's build dir?

@fangerer fangerer added the bug Something isn't working label May 24, 2023
@f-cozzocrea
Copy link
Author

Thanks @fangerer, that makes sense. I have a few thoughts about that.

  1. As far as I've seen, the Python community consensus is that modern packages should be self-contained and shouldn't interact with files outside of the package except to interact with libraries, or through utilities like importlib.resources. I would usually recommend using importlib.resources, but it doesn't support Python2.7, and it's more intended for data files. I also don't know how well it plays with setuptools. Relative paths to the runtime sources in the Hpy package doesn't make sense to me either. Copying the files to the extension build directory is likely the best option.

  2. Is there a best practice for this in the Python packaging community? Distributing C source files in a Python package for use in other projects is certainly rare, but surely Hpy isn't the first project to do so?

  3. Is statically linking to a library of these sources an option? Linking to libraries is certainly more supported by setuptools. It seems like there's been previous discussion about static linking for other reasons: Do not compile helpers with extension by default. #310

@hodgestar
Copy link
Contributor

I think point 1 above is overstated. hpy.devel is a build dependency. All of it is by definition outside the package being built, and I doubt "the Python community" (whoever they happen to be) intended to ban build dependencies.

We don't want to compile these for anyone -- they're intended to just be extra C utilities provided for convenience. Extensions should compile them themselves if they use them.

I would vote that we copy the extra sources into the build folder during the build, but also add an option to not copy them in. For this matches our intention most closely -- "here are some useful C utilities that you can include in your extension".

Perhaps the copying could use importlib.resources to do the copying.

@fangerer @f-cozzocrea Does my reasoning make sense to you? If there is some standard way to handle this, it would be good to know, but keeping what little simplicity we can in the hpy.devel setup utilities is also quite important.

@f-cozzocrea
Copy link
Author

@hodgestar I agree with your reasoning. When I say "Python community", I just mean the outcomes of packaging PEPs (like 517) which disallow absolute paths. I think copying by default in a simple and robust way makes the most sense.

@hodgestar
Copy link
Contributor

@f-cozzocrea Woot. I agree that the banning by default was probably worth it to avoid people accidentally depending on external files, even if it makes things a bit more complex for us.

@fangerer
Copy link
Contributor

Sounds good to me as well.

Adding a bit more background on #310: This issue exists because we have seen compilation problems with some package (I don't remember which one). Since we are adding the HPy helper sources, it might happen that the source code is not compatible with certain compiler options. The idea was to build a static lib with our configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants