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

Initial implementation of python modules with poetry #343

Merged
merged 25 commits into from
Mar 14, 2023

Conversation

layus
Copy link
Collaborator

@layus layus commented Mar 8, 2023

This has been long in the making, and currently it only works in a very
restricted setup. Nonetheless, it comes with a working example, so comments are
welcome.

Copy link
Member

@aherrmann aherrmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for pushing this forward! This is a really cool feature, and I don't think much is missing to have this ready to merge. I've added a couple comments.

testing/python-poetry/MODULE.bazel Outdated Show resolved Hide resolved
testing/python-poetry/tests/BUILD.bazel Outdated Show resolved Hide resolved
testing/python-poetry/tests/nixpkgs_repositories.bzl Outdated Show resolved Hide resolved
toolchains/python/python.bzl Show resolved Hide resolved
toolchains/python/python.bzl Outdated Show resolved Hide resolved
toolchains/python/python.bzl Outdated Show resolved Hide resolved
toolchains/python/python.bzl Outdated Show resolved Hide resolved
toolchains/python/python.bzl Outdated Show resolved Hide resolved
toolchains/python/python.bzl Outdated Show resolved Hide resolved
toolchains/python/python.bzl Show resolved Hide resolved
@layus
Copy link
Collaborator Author

layus commented Mar 12, 2023

Stuck on some disk space limitation. Probably /run/usr/${id} AFAIR, because nix uses that as $TMP and mach-nix fills it by unpacking it's database of all the python packages it knows.

@aherrmann
Copy link
Member

This provides a solution to #346 for Python.

@layus layus marked this pull request as ready for review March 13, 2023 20:39
@layus layus requested a review from benradf as a code owner March 13, 2023 20:39
@layus
Copy link
Collaborator Author

layus commented Mar 13, 2023

Done, I think it is as ready as can be.

README.md Outdated Show resolved Hide resolved
Copy link
Member

@aherrmann aherrmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @layus , this is great!
I've added a few more comments, but all of them are small comments.
Please address the comments and then feel free to add the merge-queue label.

testing/python/tests/import_packages_test.py Outdated Show resolved Hide resolved
toolchains/python/package_set_to_json.nix Outdated Show resolved Hide resolved
toolchains/python/python_package.bzl Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@layus layus added the merge-queue merge on green CI label Mar 14, 2023
@layus
Copy link
Collaborator Author

layus commented Mar 14, 2023

Concerning mach-nix, it is way too difficult to make it work on recent nixpkgs, the project is currently unmaintained, and last update it received references a nixpkgs from one year ago. I remembered just in time that you can craft python packages sets manually in nixpkgs.
Once dream2nix support for python improves, we could also add it to the list. DavHau moved it's focus from mach-nix to dream2nix, so I am pretty confident dream2nix will make steady progress on python. I will never thank that person enough ✨

@layus
Copy link
Collaborator Author

layus commented Mar 14, 2023

@aherrmann Thanks for your thorough and fast reviews, working on this PR has been a real pleasure !

@mergify mergify bot merged commit ad61728 into master Mar 14, 2023
@mergify mergify bot deleted the poetry-python-modules branch March 14, 2023 17:01
@mergify mergify bot removed the merge-queue merge on green CI label Mar 14, 2023
@aherrmann
Copy link
Member

Once dream2nix support for python improves, we could also add it to the list. DavHau moved it's focus from mach-nix to dream2nix, so I am pretty confident dream2nix will make steady progress on python. I will never thank that person enough sparkles

That sounds very exciting! Yes, perhaps there is even a way to use dream2nix more broadly for rules_nixpkgs integration of different languages.

Thanks for your thorough and fast reviews, working on this PR has been a real pleasure !

Likewise, thank you for the great work!

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