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

Improve on the build process of this module #92

Merged
merged 1 commit into from
Nov 20, 2022

Conversation

leonardp
Copy link
Contributor

Adds a builder class for building mbedtls and nng.
Uses setup.cfg to specify the repo url and revision.

This should fix #1 and can be a step to solve #10 as well.

@leonardp
Copy link
Contributor Author

@codypiersall I saw the smoketest failing and updated the PR again.

Instead of subclassing build I now subclassed build_ext and tested it on a local Ubuntu VM with pip install -e ..

If you have the time I can recommend this blogpost on the current state of python packaging.
It left me quite a bit confused on what the right way doing things would be..

@leonardp leonardp force-pushed the build_improvements branch 3 times, most recently from 2c08960 to f614c47 Compare October 28, 2021 18:57
Adds a builder class for building mbedtls and nng.
Add custom build option to build nng/mbedtls (hardcoded: 'yes')
Uses setup.cfg to specify the repo url and revision
@leonardp
Copy link
Contributor Author

@codypiersall try again?
I think I was on the wrong branch my Ubuntu VM.
I have now tested it with a new clone on the correct branch with python setup.py develop and it should be fine 🤞

@codypiersall
Copy link
Owner

Hey @leonardp, I really appreciate the time you've taken on this and I do like the approach.

Unfortunately some stuff in my personal life recently blew up (thankfully not involving health or family) so I do not know when I will have time to look at this in depth.

@leonardp
Copy link
Contributor Author

Hello @codypiersall, I hope you are (doing) well.
I've looked at the state of python packaging and https://python-poetry.org/ looks like a future-proof candidate for packaging that
could be used today.
Would you be OK with moving to poetry? If so, I could give this a shot.

@wbadart
Copy link

wbadart commented May 17, 2022

@leonardp, I was a poetry user for a while (personally and professionally) and really like the project, but it has some shortcomings that ultimately pushed me back to setuptools. Namely, lack of support for editable installs and lack of support for PEP 621. It's possible that neither of these are deal-breakers for pynng, but I'd personally recommend a build system with a better track record of standards adherence (e.g. flit, pdm, or of course (as of very recently) setuptools.

@codypiersall
Copy link
Owner

Took me over a year from the initial PR, but I finally merged this. Your PR is way better than what I was doing before, so thank you!

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.

The way we're building the nng library is an abuse of setuptools
3 participants