hpyproject / hpy

HPy: a better API for Python
https://hpyproject.org
MIT License
1.02k stars 52 forks source link

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

Open f-cozzocrea opened 1 year ago

f-cozzocrea commented 1 year ago

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

fangerer commented 1 year ago

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?

f-cozzocrea commented 1 year ago

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: #310

hodgestar commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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.