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.

Sign up for GitHub

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

Jump to bottom

build: remove bootstrap dependency on poetry #466

Merged
merged 2 commits into from
Jan 18, 2022
Merged

build: remove bootstrap dependency on poetry #466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
261d1d5
fix: remove buildInput dependency on poetry
cpcloud Dec 19, 2021
22adc78
style: nixpkgs-fmt
cpcloud Dec 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion mk-poetry-dep.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ pythonPackages.callPackage
++ lib.optional (stdenv.buildPlatform != stdenv.hostPlatform) pythonPackages.setuptools
++ lib.optional (!isSource) (getManyLinuxDeps fileInfo.name).pkg
++ lib.optional isDirectory buildSystemPkgs
++ lib.optional (!__isBootstrap) pythonPackages.poetry
Copy link
Member

Choose a reason for hiding this comment

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

This will break some downstreams (not all of the cases where this is required is covered by tests).
I would be open to something like !__isBootstrap && !isCross so that we can solve your issue while not breaking various projects using this code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can you write down such a scenario? I'm happy to write one or more test cases, so that folks don't spend a ton of time tracking down the problem to this (like I did) without a test breaking.

Copy link
Member

Choose a reason for hiding this comment

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

This is really the same as any other build-system missing caused by python-poetry/poetry#2789.

Adding poetry to each build was just a hack to circumvent that breakage for stuff depending on Poetry which I figured would be a fairly cheap workaround considering you are very likely to have poetry in your dependency graph anyway if you use this tool.

Maybe as a minimal reproduction case you could try an environment with https://github.com/tweag/pynixutil in it?

);

propagatedBuildInputs =
Expand Down
1 change: 1 addition & 0 deletions overrides.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2103,6 +2103,7 @@ self: super:
);

pendulum = super.pendulum.overridePythonAttrs (old: {
buildInputs = (old.buildInputs or [ ]) ++ [ self.poetry ];
# Technically incorrect, but fixes the build error..
preInstall = lib.optionalString stdenv.isLinux ''
mv --no-clobber ./dist/*.whl $(echo ./dist/*.whl | sed s/'manylinux_[0-9]*_[0-9]*'/'manylinux1'/)
Expand Down