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

Ignore missing setup.py until BEFORE_BUILD is run #1139

Closed
abitrolly opened this issue Jun 14, 2022 · 14 comments
Closed

Ignore missing setup.py until BEFORE_BUILD is run #1139

abitrolly opened this issue Jun 14, 2022 · 14 comments

Comments

@abitrolly
Copy link
Contributor

Description

In https://github.com/savoirfairelinux/opendht/tree/master/python setup.py is generated from setup.py.in after the build is run. But cibuildwheel fails too early, without runnin CIBW_BEFORE_ALL and CIBW_BEFORE_BUILD commands.

cibuildwheel: Could not find any of {setup.py, setup.cfg, pyproject.toml} at root of package

Build log

https://github.com/abitrolly/opendht/runs/6872632272?check_suite_focus=true

CI config

https://github.com/abitrolly/opendht/actions/runs/2492318864/workflow

abitrolly added a commit to abitrolly/opendht that referenced this issue Jun 14, 2022
There is only `setup.py.in` template
pypa/cibuildwheel#1139
@henryiii
Copy link
Contributor

henryiii commented Jun 15, 2022

We could see if it's movable, but couldn't you add a pyproject.toml, which will give better build reproducibility anyway? Also, you could probably touch setup.py before running.

I think it's advantageous to quit quickly if cibuildwheel is misconfigured, so if the above suggestion works, that might be best.

@abitrolly
Copy link
Contributor Author

@henryiii CMake recommends doing out of tree builds, which means it generates all files for the build in a new directory, including setup.py. I placed a stub in abitrolly/opendht@766170c but after reading CMake docs it looks I also need to place it inside pregenerated CMake structure.

I am not sure it is possible to replace the dynamic parts of setup.py with static pyproject.toml.

      version="@PACKAGE_VERSION@",
...
      ext_modules = cythonize(Extension(
          "opendht",
          ["@CURRENT_SOURCE_DIR@/opendht.pyx"],
          include_dirs = ['@PROJECT_SOURCE_DIR@/include'],
          language="c++",
          extra_compile_args=["-std=c++17"],
          extra_link_args=["-std=c++17"],
          libraries=["opendht"],
          library_dirs = ['@CURRENT_BINARY_DIR@', '@PROJECT_BINARY_DIR@']
      ))

@abitrolly
Copy link
Contributor Author

I think it's advantageous to quit quickly if cibuildwheel is misconfigured, so if the above suggestion works, that might be best.

Having an env flag like CIBW_BEFORE_SKIP_CHECK would make it look less like a hack.

@henryiii
Copy link
Contributor

You wouldn't replace anything in setup.py (unless you had a deprecated keyword like setup_requires in your setup call). You'd just have a "basic" pyproject.toml like this:

[build-system]
requires = ["setuptools>=42"]
build-backend = "setuptools.build_meta"

Every package should have that. It defines the base build system. No changes needed to setup.py.

I'd recommend something like scikit-build where you talk to CMake from an existing setup.py, rather than inject it like this. (Pybind11 examples https://github.com/pybind/scikit_build_example and by-hand https://github.com/pybind/cmake_example) Your structure will not work from source directly - you'd have to do some hacks to make a working SDist. If you are generating valid SDists, maybe use the SDist with cibuildwheel instead?

Also, why are you using cythonize? Cython has had native setuptools support for something like six years. In fact, I think since you are putting the cythonize around the Extension, I think it's not doing anything - Extension natively supports .pyx.

@henryiii
Copy link
Contributor

henryiii commented Jun 16, 2022

I think it's fair to require a hack for cibuildwheel if you require a hack to build with pip / build and don't provide pyproject.toml, which every modern package should provide. I'm not fond of adding another flag to CIBW for this, especially a flag that can't be configured via TOML too since the point of the flag is the TOML is missing!

@henryiii
Copy link
Contributor

henryiii commented Jun 16, 2022

Actually, you can improve your build too!

# python/pyproject.toml
[build-system]
requires = ["setuptools>=42", "cython", "cmake"]
build-backend = "setuptools.build_meta"

[tool.cibuildwheel]
before-all = "cmake ."

Now you need no configuration in your CI file, and you can run it locally, and it might even work on macOS or Windows. (This is using your same in-tree build in the current source. If you wanted to do out-of-tree, you'd put the pyproject.toml in the build dir, and then point cmake back at the source dir.)

@abitrolly
Copy link
Contributor Author

Also, why are you using cythonize? Cython has had native setuptools support for something like six years.

I can't answer this, because is the project of mine. Just trying to use it, but I guess it is historical abitrolly/opendht@42ba16d and it from seven years ago.

@joerick
Copy link
Contributor

joerick commented Jun 19, 2022

I think it's fair to require a hack for cibuildwheel if you require a hack to build with pip / build and don't provide pyproject.toml, which every modern package should provide. I'm not fond of adding another flag to CIBW for this, especially a flag that can't be configured via TOML too since the point of the flag is the TOML is missing!

Agreed.

@joerick joerick closed this as not planned Won't fix, can't repro, duplicate, stale Jun 19, 2022
@abitrolly
Copy link
Contributor Author

So how to provide pyproject.toml that won't need CMake substitutions?

@henryiii
Copy link
Contributor

henryiii commented Jun 19, 2022

What’s wrong with the one I show above? This one:

# python/pyproject.toml
[build-system]
requires = ["setuptools>=42", "cython", "cmake"]
build-backend = "setuptools.build_meta"

[tool.cibuildwheel]
before-all = "cmake ."

@abitrolly
Copy link
Contributor Author

A few problems.

  1. This doesn't replace setup.py and CMake substitutions
  2. Adds another layer to the CI cake
  3. cmake . command needs to be at project root one level up (https://discourse.cmake.org/t/how-to-set-project-root-when-no-project-command-is-present/5895)

@henryiii
Copy link
Contributor

henryiii commented Jun 21, 2022

This doesn't replace setup.py and CMake substitutions

It's not supposed to "replace it". pyproject.toml tells your builder (pip or build) how to make your wheel. It tells you what is required - for you, that's Cython, CMake, and setuptools. That's why you are having to manually install a Python package (Cython) from your distro package manager! You currently can't control CMake or Cython version!

setup.py is the Python configuration file for setuptools. Some other tools have (more limited) configuration files too. .trampolim.py for Trampolim. build.py for something (I forget what). Etc.

All well behaved Python packages should have a pyproject.toml, regardless of if they have setup.py or some other configuration file too. It's not a replacement. It's a requirement - you can't control the system used to read the setup.py file (the build environment, setuptools) from setup.py! The default assumptions may change in the future; specifying the build backend exactly is best.

Adds another layer to the CI cake

A very useful layer that lets you run cibuldwheel locally or on any other CI system too! Only CIBW_BUILD should be in your CI file, unless you have very special needs (and overrides fix most of those needs).

command needs to be at project root one level up

Rewrite if needed, but I think this is in the right place and does run at cibuldwheel root, not project root. All your config for cibuldwheel except build should go here, then you can run locally for testing as long as you have docker.

@abitrolly
Copy link
Contributor Author

Rewrite if needed, but I think this is in the right place and does run at cibuldwheel root, not project root.

I don't see how that is going to work. CMake generates project files from the root level and the passes down variables, such as libs and include paths that are necessary to build the extension. Right now the cake is this.

  1. install system deps
  2. compile newer versions of some system deps from source
  3. generate project with cmake
  4. build main lib and python wheels

I don't see how that 's possible to do this in pyproject.toml. Although running cibuildwheel locally could be useful, of course.

@henryiii
Copy link
Contributor

I've done it in abitrolly/opendht#2

abitrolly added a commit to abitrolly/opendht that referenced this issue Dec 13, 2023
There is only `setup.py.in` template
pypa/cibuildwheel#1139
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants