C bindings are required to build Python package

[jonathanhogg]

Fixes #882

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.08%. Comparing base (d437097) to head (653cc25).
Report is 68 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #883      +/-   ##
==========================================
- Coverage   91.84%   90.08%   -1.77%     
==========================================
  Files          37       64      +27     
  Lines        4976     9184    +4208     
  Branches        0      956     +956     
==========================================
+ Hits         4570     8273    +3703     
- Misses        406      911     +505     

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

[pca006132]

Interesting, it should not require the C binding, and I thought the CI did test for the source distribution. I will look into it later tmr.

[jonathanhogg]

Ah. I see the problem, the C bindings are ON by default:

option(MANIFOLD_CBIND "Build C (FFI) bindings" ON)

and so need to be explicitly turned OFF in the cmake defines.

[jonathanhogg]

Probably worth adding a test step to make sure that the sdist can be built. Just needs to be something like:

pipx run build --sdist
pip wheel --wheel-dir dist dist/*.tar.gz

[pca006132]

probably this?

manifold/.github/workflows/build_wheels.yml

Line 37 in 3bf7f4b

run: pipx run build --sdist

[jonathanhogg]

probably this?

manifold/.github/workflows/build_wheels.yml

Line 37 in 3bf7f4b

run: pipx run build --sdist

Yes, I mean, that's where you'd probably want to add the step to build a wheel from the sdist to make sure it works.

[pca006132]

maybe add it in this PR?

[pca006132]

Thanks!

[jonathanhogg]

Fingers crossed I've not broken your deployment process 😉