ICRAR / crc32c

A python package implementing the crc32c algorithm in hardware and software
GNU Lesser General Public License v2.1
41 stars 25 forks source link

`checkarm.h` missing in 2.7 sdist #52

Closed mgorny closed 2 months ago

mgorny commented 2 months ago

It seems that the sdist archive for 2.7 is incomplete:

$ pip install .
Processing /tmp/crc32c-2.7
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Building wheels for collected packages: crc32c
  Building wheel for crc32c (pyproject.toml) ... error
  error: subprocess-exited-with-error

  × Building wheel for crc32c (pyproject.toml) did not run successfully.
  │ exit code: 1
  ╰─> [13 lines of output]
      /tmp/pip-build-env-u57ncqey/overlay/lib/python3.12/site-packages/setuptools/_distutils/dist.py:268: UserWarning: Unknown distribution option: 'test_suite'
        warnings.warn(msg)
      running bdist_wheel
      running build
      running build_py
      running build_ext
      building 'crc32c._crc32c' extension
      x86_64-pc-linux-gnu-gcc -fno-strict-overflow -Wsign-compare -fPIC -DNDEBUG -Isrc/cc32c/ext/ -I/tmp/venv/include -I/usr/include/python3.12 -c src/crc32c/ext/_crc32c.c -o build/temp.linux-x86_64-cpython-312/src/crc32c/ext/_crc32c.o
      src/crc32c/ext/_crc32c.c:30:10: fatal error: checkarm.h: No such file or directory
         30 | #include "checkarm.h"
            |          ^~~~~~~~~~~~
      compilation terminated.
      error: command '/usr/bin/x86_64-pc-linux-gnu-gcc' failed with exit code 1
      [end of output]

  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed building wheel for crc32c
Failed to build crc32c
WARNING: Cache entry deserialization failed, entry ignored
ERROR: ERROR: Failed to build installable wheels for some pyproject.toml based projects (crc32c)

It seems that at least checkarm.h is missing in sdist.

rtobar commented 2 months ago

Thanks @mgorny for reporting this so quickly. Weirdly enough the sdist generation doesn't have the headers on CI, but it does locally, which led me to the incorrect sense of security when I introduced the breaking change (see last part of the commit message in 85efdc6).

I've pushed now the changes to fix this, together with a new step on our CI jobs to test that the sdist can be installed. After all is green I'll merge to master and release 2.7.post0.

mgorny commented 2 months ago

Yeah, it's weird. It gets included if I run python -m build -s locally here too. No clue why it'd be different on CI — except maybe old setuptools version?

rtobar commented 2 months ago

That was also my guess, but OTOH a cursory look into setuptool's documentation doesn't suggest headers are automatically included if they are listed in the depends option of an Extension (I didn't look into the changelog either). Anyway, better to list them explicitly for what it's worth.

I've now merged to master and tagged v2.7.post0, which should make its way up to PyPI in twenty minutes or so.

rtobar commented 2 months ago

@mgorny just had a look at setuptool's changelog, and indeed this was changed in 68.1.0, although the documentation doesn't seem to have been updated. Maybe I'll put forward a PR for that.