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

support some pep517 build backends #64

Closed

Conversation

emmettbutler
Copy link

  • Adds support for the hatchling and flit_core PEP517 build backends
  • Fixes an infinite recursion bug by avoiding package name collisions in overlay
  • Adds a --py argument to the CLI allowing the user to specify the Python version to be used in the built shell
  • Pins nixpkgs to improve reproducibility

@cript0nauta
Copy link
Owner

Pins nixpkgs to improve reproducibility

I think this change shouldn't be necessary. If you use the --nixpkgs option, the expression will be instantiated with <nixpkgs> pinned to the desired version: https://github.com/cript0nauta/pynixify/blob/main/pynixify/nixpkgs_sources.py#L135

@cript0nauta
Copy link
Owner

Adds a --py argument to the CLI allowing the user to specify the Python version to be used in the built shell

This change looks good. Would you be able to create a new PR that only includes this fix, without any changes to formatting or import order?

Also, please make sure to use the escape_string function when including a string inside a Nix expression.

@cript0nauta
Copy link
Owner

Fixes an infinite recursion bug by avoiding package name collisions in overlay

Do you have an example of when this bug is triggered? Maybe we should create a test in the command test suite that reproduces it.

@cript0nauta
Copy link
Owner

Adds support for the hatchling and flit_core PEP517 build backends

This changeset looks interesting, though I'm not familiar with PEP517. I'll need some time to review the changes since they're larger than the other fixes.

Thanks again for your contributions!

@emmettbutler
Copy link
Author

emmettbutler commented May 20, 2023

@cript0nauta thanks for the quick feedback! i broke out the pep517 changes into this pull request for your review #65

here's a pull request implementing the new cli option #66

here's a pull request showing how to replicate the recursion bug i encountered emmettbutler#4

thanks again! i'll look forward to your feedback.

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.

2 participants