Skip to content

python310Packages.poetry-core: 1.5.1 -> 1.6.1#244535

Merged
tjni merged 1 commit intoNixOS:stagingfrom
dotlambda:poetry-core-1.6.1
Aug 17, 2023
Merged

python310Packages.poetry-core: 1.5.1 -> 1.6.1#244535
tjni merged 1 commit intoNixOS:stagingfrom
dotlambda:poetry-core-1.6.1

Conversation

@dotlambda
Copy link
Copy Markdown
Member

@dotlambda dotlambda commented Jul 20, 2023

Description of changes

Diff: python-poetry/poetry-core@1.5.1...1.6.1

Changelog: https://github.com/python-poetry/poetry-core/blob/1.6.1/CHANGELOG.md

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Jul 20, 2023
@dotlambda dotlambda mentioned this pull request Jul 20, 2023
Merged
@ofborg ofborg bot requested a review from jonringer July 20, 2023 20:14
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Jul 20, 2023
@dotlambda
Copy link
Copy Markdown
Member Author

dotlambda commented Jul 21, 2023

I get

ERROR: zeroconf-0.70.0-cp310-cp310-macosx_13_0_x86_64.whl is not a supported wheel on this platform.

on x86_64-darwin. That's the same issue as before: #220252

@dotlambda dotlambda marked this pull request as draft July 21, 2023 00:31
@mweinelt mweinelt force-pushed the python-updates branch 3 times, most recently from cceedbd to 3f1211a Compare July 26, 2023 16:23
@mweinelt
Copy link
Copy Markdown
Member

mweinelt commented Jul 27, 2023

So two derivations, and we divert for x86_64-darwin? This is annoying.

@dotlambda
Copy link
Copy Markdown
Member Author

dotlambda commented Jul 28, 2023

So two derivations, and we divert for x86_64-darwin?

Not necessarily. We could add even more patches to keep the vendored dependencies at their current versions.

@dotlambda dotlambda force-pushed the poetry-core-1.6.1 branch from 2ded0af to b052103 Compare July 28, 2023 06:43
@dotlambda dotlambda marked this pull request as ready for review July 28, 2023 06:44
@mweinelt mweinelt requested review from FRidh and infinisil July 28, 2023 10:15
@dotlambda dotlambda force-pushed the poetry-core-1.6.1 branch from b052103 to df047df Compare July 28, 2023 15:46
@dotlambda dotlambda changed the base branch from python-updates to staging July 28, 2023 19:46
@tjni
Copy link
Copy Markdown
Contributor

tjni commented Aug 16, 2023 •
edited
Loading

How does everyone feel about merging this?

Starting in packaging version 22 (pypa/packaging#513), the platform tag used in the wheel name on x86_64-darwin is now the real OS version number (e.g. 11.0, 12.0, 13.0) instead of 10.16, which is an old version number that is still around for compatibility reasons after Apple changed their versioning scheme in Big Sur from 10.X to 1X.Y. This is not ideal both before and after the change because it's not hermetic: the tag in the wheel name depends on the OS of the building machine, but it's been OK because we build the wheel and immediately install it in the same derivation on the same machine, so both steps should see the same tag.

Except, it's broken now because pip has not updated its vendored version of packaging while poetry-core has, and the tags disagree between the two tools even on the same machine.

So I see the following options:

  1. Commit this change, which downgrades packaging inside of poetry-core.
  2. Patch the vendored packaging inside of pip with pypa/packaging@1ec97e2.
  3. Run pip with SYSTEM_VERSION_COMPAT=0 in pipInstallHook to mimic the new packaging behavior.
  4. Do not use pip to install (switching to installer is in progress but still needs time to fix other issues).

I don't have a strong opinion which option we decide, but I'm hoping we can come up with a path forward.

@tjni tjni mentioned this pull request Aug 16, 2023
Closed
@dotlambda
Copy link
Copy Markdown
Member Author

dotlambda commented Aug 16, 2023

I think we can merge this PR and then decide whether to use a cleaner solution in the future. The packaging downgrade also happened before this PR, so this only changes the poetry-core version.

tjni
tjni reviewed Aug 16, 2023
View reviewed changes
pkgs/tools/package-management/poetry/default.nix
Comment on lines -42 to -45
patches = [ ];
nativeCheckInputs = old.nativeCheckInputs ++ [
self.tomli-w
];
Copy link
Copy Markdown
Contributor

@tjni tjni Aug 16, 2023

Choose a reason for hiding this comment

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

Can this override go away completely?

Copy link
Copy Markdown
Member Author

@dotlambda dotlambda Aug 16, 2023

Choose a reason for hiding this comment

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

Yes. This was only needed because poetry used a different version of poetry-core than what's in pythonPackages.

tjni
tjni approved these changes Aug 16, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@tjni tjni left a comment

Choose a reason for hiding this comment

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

I agree we should merge this and follow up with a new PR to clean up these vendor patches.

@tjni tjni merged commit c0ebafd into NixOS:staging Aug 17, 2023
@dotlambda dotlambda deleted the poetry-core-1.6.1 branch August 17, 2023 02:21
@dotlambda dotlambda mentioned this pull request Sep 9, 2023
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jonringer jonringer Awaiting requested review from jonringer

@mweinelt mweinelt Awaiting requested review from mweinelt

@SuperSandro2000 SuperSandro2000 Awaiting requested review from SuperSandro2000

@FRidh FRidh Awaiting requested review from FRidh

@infinisil infinisil Awaiting requested review from infinisil

1 more reviewer

@tjni tjni tjni approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches.

Projects

Python batch upgrade
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dotlambda @mweinelt @tjni