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

WIP: pure poetry translator #397

Merged
merged 14 commits into from
Nov 27, 2022
Merged

Conversation

phaer
Copy link
Member

@phaer phaer commented Nov 22, 2022

No description provided.

@phaer phaer force-pushed the poetry-translator branch from 37e86fa to 3b0953b Compare November 22, 2022 18:22
@@ -132,7 +133,22 @@
"properties": {
"hash": { "type": "string" },
"type": { "type": "string" },
"dir": { "type": "string" }
"pname": { "type": "string" },
"version": { "type": "string" }
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to store the version. The version is already contained in the keys of the attrset. The framework will still pass it to the fetcher despite not being explicitly present as a field.

decompressed = true;
# generic fields
_generic = {
# TODO: specify the default package name
Copy link
Member

Choose a reason for hiding this comment

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

Those TODOs are already resolved and can be removed

@@ -40,40 +40,40 @@
package = produceDerivation defaultPackageName (buildFunc {
name = defaultPackageName;
src = getSource defaultPackageName defaultPackageVersion;
format = "setuptools";
format = "other";
Copy link
Member

Choose a reason for hiding this comment

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

@chaoflow Maybe the format other could be the right choice for your branch as well. Maybe you guys could review each others builder changes. It would be good if we can have a generic builder that works for both poetry and requirements based projects projects.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, with pleasure! Maybe it would be useful to do a call or at least open up a pad to collect requirements in a table or so and try to see where those overlap? @chaoflow

I quickly experimented with just building a wheel for poetry projects a well and install that one in an extra step. That currently fails if the project being packaged has a dependency which is also (transitively) depended on by buildInputs, because that it tries to uninstall the former one first. I think going that route should allow us to ship most of the code? But not yet sure how to solve that

Copy link
Member

@DavHau DavHau Nov 28, 2022

Choose a reason for hiding this comment

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

Sure, let's have a call. How about we use the dream2nix weekly tomorrow (Tuesday) at 14:00 UTC for this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@phaer @DavHau I'm in as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah right, you do weekly meetings for this project :) Sounds good, will be there!

@DavHau
Copy link
Member

DavHau commented Nov 27, 2022

@phaer I added wheel to the nativeBuildInputs. All tests are green now. I will merge this already, so we have something that works and not having the python branches diverge too much. For further changes please open a new branch.

Thanks for your amazing work so far!

@DavHau DavHau marked this pull request as ready for review November 27, 2022 12:30
@DavHau DavHau merged commit 7925ea3 into nix-community:main Nov 27, 2022
@phaer
Copy link
Member Author

phaer commented Nov 27, 2022

@DavHau Thanks! Not sure whether its of much use right now, but I guess there's not much harm done as long as we don't really promote it in the docs.

Definitely plan to continue working on this, but can't really give a schedule

impl added a commit to impl/dream2nix that referenced this pull request Dec 13, 2022
This arg was inadvertently added to pip install in nix-community#397, which breaks
transitive dependencies on the packages we custom-install, like
setuptools and wheel.

Per @phaer, it wasn't supposed to be there and has already been removed
in the next set of larger changes, but this should fix those transitive
dependencies in the meantime.
impl added a commit to impl/dream2nix that referenced this pull request Dec 13, 2022
This arg was inadvertently added to pip install in nix-community#397; it breaks
transitive dependencies on the packages we custom-install, like
setuptools and wheel.

Per @phaer, it wasn't supposed to be there and has already been removed
in the next set of larger changes, but this should fix those transitive
dependencies in the meantime.
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.

3 participants