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

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

Merged
merged 1 commit into from Aug 17, 2023

Conversation

dotlambda
Copy link
Member

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.

@dotlambda
Copy link
Member Author

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

@mweinelt
Copy link
Member

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

@dotlambda
Copy link
Member Author

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 changed the base branch from python-updates to staging July 28, 2023 19:46
@tjni
Copy link
Contributor

tjni commented Aug 16, 2023

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.

@dotlambda
Copy link
Member Author

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.

Comment on lines -42 to -45
patches = [ ];
nativeCheckInputs = old.nativeCheckInputs ++ [
self.tomli-w
];
Copy link
Contributor

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
Member Author

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.

Python batch upgrade automation moved this from In progress to Reviewer approved Aug 16, 2023
Copy link
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
21 of 22 checks passed
Python batch upgrade automation moved this from Reviewer approved to Done 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
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants