googlefonts / compreffor

A CFF table subroutinizer for FontTools
Apache License 2.0
23 stars 22 forks source link

Only depend on setuptools-git-ls-files for sdist #153

Closed tjni closed 1 year ago

tjni commented 1 year ago

Some PEP 517 build frontends, like PyPA build, have a feature to check that build dependencies are present prior to starting the build.

The setuptools-git-ls-files dependency is only needed when constructing the sdist but not when building a wheel. Some package managers, such as nixpkgs, only build wheels but still need to bundle and depend on this to pass validation.

This commit adds some complexity to specify that setuptools-git-ls-files is only needed when building an sdist by extending setuptools.

This is the same solution implemented in https://github.com/adobe-type-tools/cffsubr/pull/23.

google-cla[bot] commented 1 year ago

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.

tjni commented 1 year ago

Oh, I'm sorry, I didn't notice that this requires signing the Google CLA. I'm not interested in reading and signing it. Please feel free to do whatever you'd like with the code, however.

anthrotype commented 1 year ago

@behdad I'm not against the changes but the author stated they do not wish to sign the CLA so am not sure how to proceed here. Also there's new CI build failures which I think are related to latest cibuildwheel attempting to build wheels for Python 3.12 (now release candidate so ABI stable) but compreffor somehow failing for that (we may need to regenerate the cython .cpp files with a more up-to-date cython to support 3.12).

davelab6 commented 1 year ago

I guess we will reject this and the next nix user who will agree to the CLA will make a similar PR that we are allowed to merge.