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

python312Packages.pony: fix python3.12 compat, fix flask-mailman, fix flask-security-too #325181

Merged
merged 5 commits into from
Jul 11, 2024

Conversation

gador
Copy link
Member

@gador gador commented Jul 7, 2024

Description of changes

  • Pull upstream PR to fix python3.12 compatibility.
  • Update flask-mailman to fix compatibility with python3.12 (found during nixpkgs-review)
  • Fix flask-security-too by relaxing a dependency (failure found during nixpkgs-review)

Since these new build failures are only apparent when pony is fixed and are downstream, they are included in this PR

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.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.

closes #326096


Add a 👍 reaction to pull requests you find important.

@gador gador requested review from xvapx and d-goldin July 7, 2024 05:27
@gador gador changed the title python312Packages.pony: fix python3.12 compat python312Packages.pony: fix python3.12 compat, fix flask-mailman, fix flask-security-too Jul 7, 2024
@gador
Copy link
Member Author

gador commented Jul 7, 2024

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).

Result of nixpkgs-review pr 325181 run on x86_64-linux 1

3 packages failed to build and are new build failures:
22 packages built:
  • mutmut
  • mutmut.dist
  • pgadmin4
  • pgadmin4-desktopmode
  • pgadmin4-desktopmode.dist
  • pgadmin4.dist
  • python311Packages.flask-mailman
  • python311Packages.flask-mailman.dist
  • python311Packages.flask-security-too
  • python311Packages.flask-security-too.dist
  • python311Packages.pony
  • python311Packages.pony.dist
  • python311Packages.ponywhoosh
  • python311Packages.ponywhoosh.dist
  • python312Packages.flask-mailman
  • python312Packages.flask-mailman.dist
  • python312Packages.flask-security-too
  • python312Packages.flask-security-too.dist
  • python312Packages.pony
  • python312Packages.pony.dist
  • python312Packages.ponywhoosh
  • python312Packages.ponywhoosh.dist

The following issues got detected with the above build packages.
Please fix at least the ones listed with your changed packages:

python311Packages.ponywhoosh.dist:

got build log for '/nix/store/h91ma0rjsc3kbxkdw6cnvvrn5nszn1dq-python3.11-ponywhoosh-1.7.8-dist' from 'daemon'
When evaluating attribute ‘python311Packages.ponywhoosh.dist’:
warning: no-python-tests
Test runner could not discover any test cases: ‘Ran 0 tests in 0.000s’
Near pkgs/development/python-modules/ponywhoosh/default.nix:26:

   |
26 |     description = "Make your database over PonyORM searchable";
   | ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/no-python-tests.md

python311Packages.ponywhoosh:

got build log for '/nix/store/s7x8vzwnidih2pyddiv3nsyc9waflq87-python3.11-ponywhoosh-1.7.8' from 'daemon'
When evaluating attribute ‘python311Packages.ponywhoosh’:
warning: no-python-tests
Test runner could not discover any test cases: ‘Ran 0 tests in 0.000s’
Near pkgs/development/python-modules/ponywhoosh/default.nix:26:

   |
26 |     description = "Make your database over PonyORM searchable";
   | ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/no-python-tests.md

python312Packages.ponywhoosh.dist:

got build log for '/nix/store/p0grs3dhqsyjvqkgn5vljqwdl3kndrj9-python3.12-ponywhoosh-1.7.8-dist' from 'daemon'
When evaluating attribute ‘python312Packages.ponywhoosh.dist’:
warning: no-python-tests
Test runner could not discover any test cases: ‘Ran 0 tests in 0.000s’
Near pkgs/development/python-modules/ponywhoosh/default.nix:26:

   |
26 |     description = "Make your database over PonyORM searchable";
   | ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/no-python-tests.md

python312Packages.ponywhoosh:

got build log for '/nix/store/jkirpmb8vkv9g8di671pbmcivig0fbj1-python3.12-ponywhoosh-1.7.8' from 'daemon'
When evaluating attribute ‘python312Packages.ponywhoosh’:
warning: no-python-tests
Test runner could not discover any test cases: ‘Ran 0 tests in 0.000s’
Near pkgs/development/python-modules/ponywhoosh/default.nix:26:

   |
26 |     description = "Make your database over PonyORM searchable";
   | ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/no-python-tests.md

@gador
Copy link
Member Author

gador commented Jul 7, 2024

3 packages failed to build and are new build failures:

* agda-pkg: [plain log](https://termbin.com/zvul)

* agda-pkg.dist: [plain log](https://termbin.com/vztb)

Python3.12 related (ModuleNotFoundError: No module named 'distutils')

* tribler: log was empty

Probably also Python3.12 related

Copy link
Contributor

@bendlas bendlas left a comment

Choose a reason for hiding this comment

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

This fixes pgadmin for me

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jul 10, 2024
@gador gador mentioned this pull request Jul 10, 2024
13 tasks
@gador
Copy link
Member Author

gador commented Jul 10, 2024

@dotlambda I integrated your changes from #326096
Could you have a look? Thanks

gador and others added 3 commits July 10, 2024 16:11
Signed-off-by: Florian Brandes <[email protected]>
Co-authored-by: Robert Schütz <[email protected]>
fix build for python3.12

Signed-off-by: Florian Brandes <[email protected]>
Co-authored-by: Robert Schütz <[email protected]>
relax dependency which is actually newer

Signed-off-by: Florian Brandes <[email protected]>
Co-authored-by: Robert Schütz <[email protected]>
@superherointj
Copy link
Contributor

Git commit body is missing release notes for:

python312.flask-mailman: 1.1.0 -> 1.1.1

@gador
Copy link
Member Author

gador commented Jul 10, 2024

Result of nixpkgs-review pr 325181 run on aarch64-darwin 1

2 packages failed to build:
  • agda-pkg
  • agda-pkg.dist
22 packages built:
  • mutmut
  • mutmut.dist
  • pgadmin4
  • pgadmin4-desktopmode
  • pgadmin4-desktopmode.dist
  • pgadmin4.dist
  • python311Packages.flask-mailman
  • python311Packages.flask-mailman.dist
  • python311Packages.flask-security-too
  • python311Packages.flask-security-too.dist
  • python311Packages.pony
  • python311Packages.pony.dist
  • python311Packages.ponywhoosh
  • python311Packages.ponywhoosh.dist
  • python312Packages.flask-mailman
  • python312Packages.flask-mailman.dist
  • python312Packages.flask-security-too
  • python312Packages.flask-security-too.dist
  • python312Packages.pony
  • python312Packages.pony.dist
  • python312Packages.ponywhoosh
  • python312Packages.ponywhoosh.dist

@gador
Copy link
Member Author

gador commented Jul 10, 2024

@superherointj thanks for the build and review! I was unable to reproduce your build failure for some reason?

@superherointj
Copy link
Contributor

agda-pkg is just:

ModuleNotFoundError: No module named 'distutils'

@superherointj
Copy link
Contributor

superherointj commented Jul 10, 2024

@superherointj thanks for the build and review! I was unable to reproduce your build failure for some reason?

My host may be unreliable. It's the shared community darwin builder. And I'm a bit too locked there to try to sort this out.

I will trust your host then.

Can you just fix agda-pkg and tribler? Seems straightforward. Example (not exactly):

  • tribler, fails at libtorrent-rasterbar, needs to have at pkgs/top-level/all-packages.nix:

    python = python3Packages.python.withPackages (p: [ p.setuptools ]);
    libtorrent-rasterbar-2_0_x works.
    libtorrent-rasterbar-1_2_x errors with: ‘PyUnicode_AS_DATA’ was not declared in this scope (https://termbin.com/bn5n)

I'd prefer zeroing the build failures.

last commit was 05/2021 and no pkgs in nixpkgs depend on it

Signed-off-by: Florian Brandes <[email protected]>
@gador
Copy link
Member Author

gador commented Jul 10, 2024

Actually, it seems agda-pkg is being unmaintained upstream (agda/agda-pkg#49). Since no pkgs inside nixpkgs seem to depend on it and this does not appear to be a simple fix, I'll vote to remove this package. @alexarice you are listed as maintainer of this package: What do you recommend?

@superherointj superherointj requested a review from alexarice July 10, 2024 21:25
@gador
Copy link
Member Author

gador commented Jul 10, 2024

tribler fails due to a dependency failure of libtorrent-rasterbar-1.2.11 (also a distutils python3.12 issue). I'd rather keep the scope of the PR down (as it fixes packages with some dependencies, alongside pgadmin4). Maybe some of the maintainers of tribler can have a look into it? (libtorrent-rasterbar is unmaintained)

  • tribler, fails at libtorrent-rasterbar, needs to have at pkgs/top-level/all-packages.nix:
    > python = python3Packages.python.withPackages (p: [ p.setuptools ]);

This propably will not suffice. Python3.12 has removed distutils and since we upgraded to 3.12 we see all those distutils failures. There is no simple fix. Upstream has to move to different packages (depending on the functionality they need)

See: https://peps.python.org/pep-0632/

@alexarice
Copy link
Contributor

I think its safe to remove agda-pkg if it isn't maintained upstream. I believe it isn't widely used and is not used in any of the agda infrastructure

@superherointj
Copy link
Contributor

superherointj commented Jul 10, 2024

I found a fix for libtorrent-rasterbar-2_0_x. Add to pkgs/top-level/all-packages.nix:

python = python3Packages.python.withPackages (p: [ p.setuptools ]);

I also don't want to expand the scope of PR.
We can mark broken libtorrent-rasterbar-1_2_x broken. (Avoids unnecessary builds.)

@gador
Copy link
Member Author

gador commented Jul 10, 2024

python = python3Packages.python.withPackages (p: [ p.setuptools ]);

Cool. But doesn't tribler use a special libtorrent-rasterbar-1_2_x version?

@superherointj
Copy link
Contributor

superherointj commented Jul 10, 2024

python = python3Packages.python.withPackages (p: [ p.setuptools ]);

Cool. But doesn't tribler use a special libtorrent-rasterbar-1_2_x version?

Yeah. I can mark it broken if you don't want.

ModuleNotFoundError: No module named 'distutils'

Signed-off-by: Florian Brandes <[email protected]>
@gador
Copy link
Member Author

gador commented Jul 10, 2024

python = python3Packages.python.withPackages (p: [ p.setuptools ]);

Cool. But doesn't tribler use a special libtorrent-rasterbar-1_2_x version?

Yeah. I can mark it broken if you don't want.

Done

@gador
Copy link
Member Author

gador commented Jul 10, 2024

Just FYI: I did try your suggestion with the 1.2 version and it got passed the initial distutils error, but later failed to build with

libtorrent-rasterbar> Making all in python
libtorrent-rasterbar> make[2]: Entering directory '/build/source/bindings/python'
libtorrent-rasterbar> /nix/store/m22r7gsvm4w3ib3y2jcsnx0l7mis050q-python3-3.12.4-env/bin/python ./setup.py build
libtorrent-rasterbar> /build/source/bindings/python/src/converters.cpp: In static member function ‘static void to_string_view::construct(PyObject*, boost::python::converter::rvalue_from_python_stage1_data*)’:
libtorrent-rasterbar> /build/source/bindings/python/src/converters.cpp:159:63: error: ‘PyUnicode_AS_DATA’ was not declared in this scope; did you mean ‘PyUnicode_DATA’?
libtorrent-rasterbar>   159 |             data->convertible = new (storage) lt::string_view(PyUnicode_AS_DATA(x)libtorrent-rasterbar>       |                                                               PyUnicode_DATA
libtorrent-rasterbar> /build/source/bindings/python/src/converters.cpp:159:85: error: ‘PyUnicode_GET_DATA_SIZE’ was not declared in this scope
libtorrent-rasterbar>   159 |             data->convertible = new (storage) lt::string_view(PyUnicode_AS_DATA(x), PyUnicode_GET_DATA_SIZE(x));
libtorrent-rasterbar>       |                                                                                     ^~~~~~~~~~~~~~~~~~~~~~~
libtorrent-rasterbar> error: command '/nix/store/kp2j7yn0wzwq5piy494r54dafrh83s6s-gcc-wrapper-13.3.0/bin/g++' failed with exit code 1

So, yeah, seems broken either way

@superherointj
Copy link
Contributor

Result of nixpkgs-review pr 325181 run on x86_64-linux 1

1 package marked as broken and skipped:
  • tribler
22 packages built:
  • mutmut
  • mutmut.dist
  • pgadmin4
  • pgadmin4-desktopmode
  • pgadmin4-desktopmode.dist
  • pgadmin4.dist
  • python311Packages.flask-mailman
  • python311Packages.flask-mailman.dist
  • python311Packages.flask-security-too
  • python311Packages.flask-security-too.dist
  • python311Packages.pony
  • python311Packages.pony.dist
  • python311Packages.ponywhoosh
  • python311Packages.ponywhoosh.dist
  • python312Packages.flask-mailman
  • python312Packages.flask-mailman.dist
  • python312Packages.flask-security-too
  • python312Packages.flask-security-too.dist
  • python312Packages.pony
  • python312Packages.pony.dist
  • python312Packages.ponywhoosh
  • python312Packages.ponywhoosh.dist

@gador
Copy link
Member Author

gador commented Jul 10, 2024

Result of nixpkgs-review pr 325181 run on aarch64-darwin 1

22 packages built:
  • mutmut
  • mutmut.dist
  • pgadmin4
  • pgadmin4-desktopmode
  • pgadmin4-desktopmode.dist
  • pgadmin4.dist
  • python311Packages.flask-mailman
  • python311Packages.flask-mailman.dist
  • python311Packages.flask-security-too
  • python311Packages.flask-security-too.dist
  • python311Packages.pony
  • python311Packages.pony.dist
  • python311Packages.ponywhoosh
  • python311Packages.ponywhoosh.dist
  • python312Packages.flask-mailman
  • python312Packages.flask-mailman.dist
  • python312Packages.flask-security-too
  • python312Packages.flask-security-too.dist
  • python312Packages.pony
  • python312Packages.pony.dist
  • python312Packages.ponywhoosh
  • python312Packages.ponywhoosh.dist

@wegank wegank added 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people and removed 12.approvals: 1 This PR was reviewed and approved by one reputable person labels Jul 11, 2024
@superherointj superherointj merged commit f93bab4 into NixOS:master Jul 11, 2024
25 of 27 checks passed
@superherointj
Copy link
Contributor

We should have seen this #325297

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: python 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants