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

Add Python feature to Maturin configuration in pyproject.toml #347

Merged

Conversation

Jazzinghen
Copy link
Contributor

@Jazzinghen Jazzinghen commented Nov 29, 2024

What problem is this trying to solve

While developing an application in Python that uses Hifitime I moved from Python 3.12 to 3.13.

This resulted in errors at update of the dependencies:

$ poetry install --with=test,docs
Installing dependencies from lock file

Package operations: 1 install, 0 updates, 0 removals

  - Installing hifitime (4.0.0): Failed

  ChefBuildError

  Backend subprocess exited when trying to invoke build_wheel
  
  Running `maturin pep517 build-wheel -i /home/jazzinghen/.local/share/pyenv/versions/3.13.0/bin/python3.13 --compatibility off`
  📦 Including license file "/tmp/tmp2koz9epo/hifitime-4.0.0/LICENSE.txt"
  🔗 Found cffi bindings
  🐍 Using CPython 3.13 at /home/jazzinghen/.local/share/pyenv/versions/3.13.0/bin/python3.13 to generate the cffi bindings
     Compiling proc-macro2 v1.0.88
     Compiling unicode-ident v1.0.13
     Compiling static_assertions v1.1.0
     Compiling libc v0.2.161
     Compiling memchr v2.7.4
     Compiling libm v0.2.8
     Compiling adler2 v2.0.0
     Compiling autocfg v1.4.0
     Compiling gimli v0.31.1
     Compiling serde v1.0.210
     Compiling cfg-if v1.0.0
     Compiling heck v0.5.0
     Compiling rustc-demangle v0.1.24
     Compiling web-time v1.1.0
     Compiling lexical-util v1.0.3
     Compiling miniz_oxide v0.8.0
     Compiling num-traits v0.2.19
     Compiling lexical-parse-integer v1.0.2
     Compiling lexical-parse-float v1.0.2
     Compiling object v0.36.5
     Compiling quote v1.0.37
     Compiling syn v2.0.79
     Compiling lexical-core v1.0.2
     Compiling addr2line v0.24.2
     Compiling snafu-derive v0.8.5
     Compiling serde_derive v1.0.210
     Compiling backtrace v0.3.74
     Compiling snafu v0.8.5
     Compiling hifitime v4.0.0 (/tmp/tmp2koz9epo/hifitime-4.0.0)
      Finished `release` profile [optimized] target(s) in 3.72s
  💥 maturin failed
    Caused by: Failed to generate cffi declarations using /home/jazzinghen/.local/share/pyenv/versions/3.13.0/bin/python3.13: exit status: 1
  --- Stdout:
  
  --- Stderr:
  Traceback (most recent call last):
    File "<string>", line 2, in <module>
      import cffi
  ModuleNotFoundError: No module named 'cffi'
  
  Error: command ['maturin', 'pep517', 'build-wheel', '-i', '/home/jazzinghen/.local/share/pyenv/versions/3.13.0/bin/python3.13', '--compatibility', 'off'] returned non-zero exit status 1
  

  at ~/.local/pipx/venvs/poetry/lib/python3.13/site-packages/poetry/installation/chef.py:164 in _prepare
      160│ 
      161│                 error = ChefBuildError("\n\n".join(message_parts))
      162│ 
      163│             if error is not None:
    → 164│                 raise error from None
      165│ 
      166│             return path
      167│ 
      168│     def _prepare_sdist(self, archive: Path, destination: Path | None = None) -> Path:

Note: This error originates from the build backend, and is likely not a problem with poetry but with hifitime (4.0.0) not supporting PEP 517 builds. You can verify this by running 'pip wheel --no-cache-dir --use-pep517 "hifitime (==4.0.0)"'.

Even after adding cffi to my pyenv-based Python I would get other errors:

$ poetry install --with=test,docs
Installing dependencies from lock file

Package operations: 1 install, 0 updates, 0 removals

  - Installing hifitime (4.0.0): Failed

  ChefBuildError

  Backend subprocess exited when trying to invoke build_wheel
  
  Running `maturin pep517 build-wheel -i /home/jazzinghen/.local/share/pyenv/versions/3.13.0/bin/python3.13 --compatibility off`
  📦 Including license file "/tmp/tmp88c3m7n0/hifitime-4.0.0/LICENSE.txt"
  🔗 Found cffi bindings
  🐍 Using CPython 3.13 at /home/jazzinghen/.local/share/pyenv/versions/3.13.0/bin/python3.13 to generate the cffi bindings
     Compiling proc-macro2 v1.0.88
     Compiling unicode-ident v1.0.13
     Compiling static_assertions v1.1.0
     Compiling libc v0.2.161
     Compiling adler2 v2.0.0
     Compiling libm v0.2.8
     Compiling memchr v2.7.4
     Compiling gimli v0.31.1
     Compiling autocfg v1.4.0
     Compiling cfg-if v1.0.0
     Compiling serde v1.0.210
     Compiling heck v0.5.0
     Compiling rustc-demangle v0.1.24
     Compiling web-time v1.1.0
     Compiling lexical-util v1.0.3
     Compiling miniz_oxide v0.8.0
     Compiling num-traits v0.2.19
     Compiling lexical-parse-integer v1.0.2
     Compiling object v0.36.5
     Compiling lexical-parse-float v1.0.2
     Compiling quote v1.0.37
     Compiling syn v2.0.79
     Compiling lexical-core v1.0.2
     Compiling addr2line v0.24.2
     Compiling backtrace v0.3.74
     Compiling snafu-derive v0.8.5
     Compiling serde_derive v1.0.210
     Compiling snafu v0.8.5
     Compiling hifitime v4.0.0 (/tmp/tmp88c3m7n0/hifitime-4.0.0)
      Finished `release` profile [optimized] target(s) in 4.09s
  💥 maturin failed
    Caused by: Failed to generate cffi declarations using /home/jazzinghen/.local/share/pyenv/versions/3.13.0/bin/python3.13: exit status: 1
  --- Stdout:
  
  --- Stderr:
  Traceback (most recent call last):
    File "<string>", line 7, in <module>
      ffi.cdef(header.read())
      ~~~~~~~~^^^^^^^^^^^^^^^
    File "/home/jazzinghen/.local/share/pyenv/versions/3.13.0/lib/python3.13/site-packages/cffi/api.py", line 112, in cdef
      self._cdef(csource, override=override, packed=packed, pack=pack)
      ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/home/jazzinghen/.local/share/pyenv/versions/3.13.0/lib/python3.13/site-packages/cffi/api.py", line 126, in _cdef
      self._parser.parse(csource, override=override, **options)
      ~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/home/jazzinghen/.local/share/pyenv/versions/3.13.0/lib/python3.13/site-packages/cffi/cparser.py", line 390, in parse
      self._internal_parse(csource)
      ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^
    File "/home/jazzinghen/.local/share/pyenv/versions/3.13.0/lib/python3.13/site-packages/cffi/cparser.py", line 397, in _internal_parse
      self._process_macros(macros)
      ~~~~~~~~~~~~~~~~~~~~^^^^^^^^
    File "/home/jazzinghen/.local/share/pyenv/versions/3.13.0/lib/python3.13/site-packages/cffi/cparser.py", line 488, in _process_macros
      raise CDefError(
      ...<6 lines>...
          % (key, key, key, value))
  cffi.CDefError: only supports one of the following syntax:
    #define JD_J1900 ...     (literally dot-dot-dot)
    #define JD_J1900 NUMBER  (with NUMBER an integer constant, decimal/hex/octal)
  got:
    #define JD_J1900 2415020.0
  
  Error: command ['maturin', 'pep517', 'build-wheel', '-i', '/home/jazzinghen/.local/share/pyenv/versions/3.13.0/bin/python3.13', '--compatibility', 'off'] returned non-zero exit status 1
  

  at ~/.local/pipx/venvs/poetry/lib/python3.13/site-packages/poetry/installation/chef.py:164 in _prepare
      160│ 
      161│                 error = ChefBuildError("\n\n".join(message_parts))
      162│ 
      163│             if error is not None:
    → 164│                 raise error from None
      165│ 
      166│             return path
      167│ 
      168│     def _prepare_sdist(self, archive: Path, destination: Path | None = None) -> Path:

Note: This error originates from the build backend, and is likely not a problem with poetry but with hifitime (4.0.0) not supporting PEP 517 builds. You can verify this by running 'pip wheel --no-cache-dir --use-pep517 "hifitime (==4.0.0)"'.

Exploration

To solve this issue I tried multiple things, to the point of cloning the Hifitime repository and build the project there using maturin. When using the instructions in the repository's readme everything would build fine, however, when using the ones that were executed by Poetry they would fail with the same errors.

The README's instructions:

maturin build -F python --out dist

Poetry's instructions:

maturin pep517 build-wheel -i /home/jazzinghen/.local/share/pyenv/versions/3.13.0/bin/python3.13 --compatibility off

Poetry's instructions seemed a bit off, especially given what the README warns the users about in the first few lines:

If building from source, note that the Python package is only built if the python feature is enabled.

This, in conjunction with the fact that Poetry's lock file reported built packages only up till 3.12 made me think that there was something wrong with the way Hifitime's package was built.

Proposed solution

Add the required Python package to maturin's configuration inside pyproject.toml as that is read by the pep517 build process.

This is allowing me to build the packages by just running

maturin pep517 build-wheel --out dist

@Jazzinghen
Copy link
Contributor Author

I can confirm this solution works. When using my branch as source for hifitime in Poetry I get the following result:

Updating dependencies
Resolving dependencies... (17.6s)

Package operations: 1 install, 0 updates, 0 removals

  - Installing hifitime (4.0.0 f1ee3f6)

Writing lock file

Copy link

codecov bot commented Nov 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.11%. Comparing base (6ff7257) to head (f1ee3f6).
Report is 5 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #347   +/-   ##
=======================================
  Coverage   84.11%   84.11%           
=======================================
  Files          23       23           
  Lines        3677     3677           
=======================================
  Hits         3093     3093           
  Misses        584      584           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ChristopherRabotin
Copy link
Member

Superb! Thank you for the bug report and for the immediate fix.

@ChristopherRabotin ChristopherRabotin merged commit eda1f19 into nyx-space:master Nov 29, 2024
30 checks passed
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

Successfully merging this pull request may close these issues.

2 participants