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

Adopt PEP 517 #1683

Closed
3 tasks done
vytas7 opened this issue Feb 23, 2020 · 11 comments
Closed
3 tasks done

Adopt PEP 517 #1683

vytas7 opened this issue Feb 23, 2020 · 11 comments

Comments

@vytas7
Copy link
Member

vytas7 commented Feb 23, 2020

It is getting increasingly annoying to install Falcon with Cython support using anything but the official stable release wheels.
At some point we'll need to switch the Falcon package build system to use PEP 517.

  • Pick a PEP 517 compliant build tool
  • Support Cython by default on the relevant platforms
  • Make it easy to skip cythonization

A non-exhaustive list of build tools that could be used

  • Setuptools -- most incremental change; check if it fits the requirements above though
  • Flit -- a bit spartan for our use?
  • Poetry -- very solid machinery for dependency management, but we don't have any runtime deps 😜
  • Enscons -- interesting but best suited if one already is a scons user?
  • Anything else?
@vytas7 vytas7 added this to the Version 3.1 milestone Feb 23, 2020
@CaselIT
Copy link
Member

CaselIT commented Mar 24, 2020

Anything else?

@vytas7
Copy link
Member Author

vytas7 commented Mar 25, 2020

Yeah, thanks @CaselIT ! I was actually about to write that too, but it never happened. Fixing now.

@CaselIT
Copy link
Member

CaselIT commented Mar 25, 2020

Btw, I've tried setuptools pep517 mode for sqlalchemy, and it seems to execute the setup.py as when running in non-pep 517 mode, so it respects the env arguments and/or fallback if the compiler or cython is not found.

@vytas7
Copy link
Member Author

vytas7 commented Mar 31, 2020

I meant that while at it, it would be great if we could encapsulate cythonization as part of the PEP-517 build system, i.e. the user would only have to pip install, even from a source checkout.

Thoughts?

@CaselIT
Copy link
Member

CaselIT commented Mar 31, 2020

I think you can do something like this with setuptools that works with or without pep517. Take a look at the setup.py of sqlalchemy. It first tries to build the c extension and fallback it that fails

Using pep517 allows a package to specify a list of requirements to install your package, cython could be one of them, I guess. I have not tried it so I may be wrong.
Example pyproject.toml with the requirements:
https://github.com/pypa/setuptools/blob/3aeec3f0e989516e9229d9a75f5a038929dee6a6/pyproject.toml#L2-L7

@CaselIT
Copy link
Member

CaselIT commented Mar 31, 2020

I've tried to adapt the sqlalchemy setup to falcon here:
CaselIT@bedc2fb
The tests seen to pass https://travis-ci.org/github/CaselIT/falcon/builds/669415425

Should I open a draft pr so we can discuss / experiment it there?

@kgriffs
Copy link
Member

kgriffs commented Mar 31, 2020

If we could get this to work, then hopefully we won't need the --no-build-isolation hack

@CaselIT
Copy link
Member

CaselIT commented Mar 31, 2020

You can add the pyproject.toml to the ignore list when distributing to pypi.
Its absence makes pip not use pep517

@CaselIT
Copy link
Member

CaselIT commented Mar 31, 2020

Sqlalchemy currently is doing that, since pep517 seems still broken in some cases. Like in debian unstable the system pip has a broken pep517 module.
pypa/pip#7874 (comment)

@CaselIT
Copy link
Member

CaselIT commented Mar 31, 2020

An alternative to this may be to use a ci to build wheels for the various oses.
I've just experimented with using github actions to build wheels for sqlalchemy that has similar requirements, (has optional c modules).
I think I could adapt that github action workflow fairly easily for falcon.

For reference this is the current version of the action https://github.com/CaselIT/sqlalchemy/blob/36b3100b07e83866d8d5e6fa211c05c0aaacdf9c/.github/workflows/create-wheels.yaml

@vytas7
Copy link
Member Author

vytas7 commented May 26, 2021

Resolved fixed in #1709

@vytas7 vytas7 closed this as completed May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants