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

Python: change bootstrapping. #248866

Merged
merged 32 commits into from
Aug 20, 2023
Merged

Python: change bootstrapping. #248866

merged 32 commits into from
Aug 20, 2023

Conversation

FRidh
Copy link
Member

@FRidh FRidh commented Aug 13, 2023

Description of changes

This is #245509 but then on python-updates branch so we can use Hydra.

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.

@tjni
Copy link
Contributor

tjni commented Aug 16, 2023

I have rebased this on staging-next for now to make it a little easier to reason about for me. I propose:

  1. For commits that are merged into master, pick it up after it's been merged into staging-next.
  2. For commits that are merged into staging, cherry-pick over to here and when it comes time to rebase this onto staging, it should be de-duplicated.

@tjni tjni force-pushed the python-updates branch 7 times, most recently from 866cd47 to fbed5e9 Compare August 17, 2023 06:03
@FRidh
Copy link
Member Author

FRidh commented Aug 17, 2023

I'm going to start one more run. I'm pretty confident we can merge then. Please don't add any more unrelated updates, only (larger) fixes.

@FRidh
Copy link
Member Author

FRidh commented Aug 17, 2023

jedi has one failing test on all platforms which has to be resolved

@tjni
Copy link
Contributor

tjni commented Aug 17, 2023

jedi needed to be updated to 0.19.0 to be compatible with attrs 23.1.0

@tjni
Copy link
Contributor

tjni commented Aug 18, 2023

The last 3 commits are PRs that were either merged or are targeting master but they should unblock testing of a few hundred dependent packages all together.

@FRidh
Copy link
Member Author

FRidh commented Aug 18, 2023

pg8000 will need to be fixed

adding license file 'LICENSE'
writing manifest file 'pg8000.egg-info/SOURCES.txt'

ERROR Missing dependencies:
        versioningit>=2.1.0

After that we can trigger hydra again.

@doronbehar
Copy link
Contributor

Small scipy update at #249921 .

@FRidh
Copy link
Member Author

FRidh commented Aug 19, 2023

About 4000 failures on this branch.

@tjni
Copy link
Contributor

tjni commented Aug 19, 2023

I looked through a good number of the failures and listed them at the top of #247287. The biggest improvement will be from fixing home-assistant packages (#250124). I'm not entirely sure where else we can get big improvements. I'd like to merge the outstanding PRs, merge those changes in master into staging-next, and then rebase on top again to get an up to date list of broken packages.

installShellCompletion --cmd pip \
--bash <($out/bin/pip completion --bash) \
--fish <($out/bin/pip completion --fish) \
--zsh <($out/bin/pip completion --zsh)
Copy link
Contributor

Choose a reason for hiding this comment

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

<($out/bin/pip completion --zsh --no-cache-dir) will properly clear the warning about .cache/pip not being writable. zsh may not work until pypa/pip#12166 is closed. (given pip is at the bottom of ~40000 packages leaving it in seems fine)

Copy link
Contributor

Choose a reason for hiding this comment

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

Also fixes #224610

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for figuring that out. This was merged without that change, but we should add this in a separate commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please do. I'm working on another change atm and don't need the two things coupled.

Copy link
Contributor

Choose a reason for hiding this comment

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

Tacked it on to this change https:/NixOS/nixpkgs/pull/250398/files

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you, I appreciate you creating that PR. I'm sorry I hadn't found the time yet today.

Comment on lines +45 to 47
sed -i "/versioningit >=/d" pyproject.toml
sed -i '/^name =.*/a version = "${version}"' pyproject.toml
sed -i "/dynamic =/d" pyproject.toml
Copy link
Member

Choose a reason for hiding this comment

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

nit: those could have been combined into one command

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, you're right. At the same time, even though it's less efficient, I feel that keeping separate commands here is more readable. But this motivates me to look into whether we can handle versioningit better in general, thank you!

Copy link
Member

Choose a reason for hiding this comment

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

I always find less code overhault more readable unless you are using some special character shortcuts. Like here, eg deduplicating pyproject.toml but as said it is not that super important and focusing on getting a setup-hook for verioningit is a better solution anyway.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/psa-poetry2nix-is-currently-broken-with-nixpkgs-unstable/32281/1

@jqqqqqqqqqq
Copy link
Contributor

Should we backport this to 23.05?

I'm seeing errors on 23.05 because some inconsistency happened:

[ERROR]   stderr) error: anonymous function at /nix/store/r5b4xjc4gvraxdslr98f1qpn62vp3gw8-source/pkgs/development/python-modules/wheel/default.nix:1:1 called with unexpected argument 'flit-core'
[ERROR]   stderr)
[ERROR]   stderr)        at /nix/store/r5b4xjc4gvraxdslr98f1qpn62vp3gw8-source/lib/customisation.nix:80:16:
[ERROR]   stderr)
[ERROR]   stderr)            79|     let
[ERROR]   stderr)            80|       result = f origArgs;
[ERROR]   stderr)              |                ^
[ERROR]   stderr)            81|
[ERROR]   stderr) (use '--show-trace' to show detailed location information)
[ERROR]  failure) Child process exited with error code: 1

And 23.05 is still using the old wheel package:

cat /nix/store/r5b4xjc4gvraxdslr98f1qpn62vp3gw8-source/pkgs/development/python-modules/wheel/default.nix
{ lib
, buildPythonPackage
, fetchFromGitHub
, bootstrapped-pip
, setuptools
}:

buildPythonPackage rec {
  pname = "wheel";
  version = "0.38.4";
  format = "other";

  src = fetchFromGitHub {
    owner = "pypa";
    repo = pname;
    rev = version;
    hash = "sha256-yZLU0t/nz6kfnnoLL15bybOxN4+SJUaTJsCpGffl1QU=";
    name = "${pname}-${version}-source";
    postFetch = ''
...

@kirillrdy
Copy link
Member

Should we backport this to 23.05?

I'm seeing errors on 23.05 because some inconsistency happened:

[ERROR]   stderr) error: anonymous function at /nix/store/r5b4xjc4gvraxdslr98f1qpn62vp3gw8-source/pkgs/development/python-modules/wheel/default.nix:1:1 called with unexpected argument 'flit-core'
[ERROR]   stderr)
[ERROR]   stderr)        at /nix/store/r5b4xjc4gvraxdslr98f1qpn62vp3gw8-source/lib/customisation.nix:80:16:
[ERROR]   stderr)
[ERROR]   stderr)            79|     let
[ERROR]   stderr)            80|       result = f origArgs;
[ERROR]   stderr)              |                ^
[ERROR]   stderr)            81|
[ERROR]   stderr) (use '--show-trace' to show detailed location information)
[ERROR]  failure) Child process exited with error code: 1

And 23.05 is still using the old wheel package:

cat /nix/store/r5b4xjc4gvraxdslr98f1qpn62vp3gw8-source/pkgs/development/python-modules/wheel/default.nix
{ lib
, buildPythonPackage
, fetchFromGitHub
, bootstrapped-pip
, setuptools
}:

buildPythonPackage rec {
  pname = "wheel";
  version = "0.38.4";
  format = "other";

  src = fetchFromGitHub {
    owner = "pypa";
    repo = pname;
    rev = version;
    hash = "sha256-yZLU0t/nz6kfnnoLL15bybOxN4+SJUaTJsCpGffl1QU=";
    name = "${pname}-${version}-source";
    postFetch = ''
...

I don't think it fits backporting criteria, also 23.11 is imminent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants