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

grass: pin to python 3.11 #325731

Merged
merged 3 commits into from
Jul 10, 2024
Merged

grass: pin to python 3.11 #325731

merged 3 commits into from
Jul 10, 2024

Conversation

fpletz
Copy link
Member

@fpletz fpletz commented Jul 9, 2024 •
edited
Loading

Description of changes

Pin to Python 3.11 since wxpython is not yet compatible to Python 3.12.

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.

Add a 👍 reaction to pull requests you find important.

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Jul 9, 2024
@ofborg ofborg bot requested review from sikmir, nh2, imincik, l0b0, nialov and willcohen July 9, 2024 04:55
@ofborg ofborg bot requested a review from lsix July 9, 2024 07:24
mjoerg
mjoerg reviewed Jul 9, 2024
View reviewed changes
Copy link
Contributor

@mjoerg mjoerg left a comment

Choose a reason for hiding this comment

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

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

1 package failed to build:
  • qgis-ltr
2 packages built:
  • grass
  • qgis

@fpletz
Copy link
Member Author

fpletz commented Jul 9, 2024

Thanks, fixed qgis-ltr.

@fpletz
Copy link
Member Author

fpletz commented Jul 9, 2024 •
edited
Loading

qgis-ltr still fails. But this is not due to the update of the python version but the update of gqgis-ltr to 3.34.8: https://hydra.nixos.org/job/nixpkgs/trunk/qgis-ltr.x86_64-linux. Thus unrelated to this PR.

@imincik
Copy link
Contributor

imincik commented Jul 9, 2024 •
edited
Loading

QGIS is building successfully even without this PR on Hydra.

@imincik
Copy link
Contributor

imincik commented Jul 9, 2024

I am not sure why QGIS-LTR fails. It was building successfully at the moment of 3.34.8 update.

imincik
imincik previously requested changes Jul 9, 2024
View reviewed changes
pkgs/applications/gis/grass/default.nix Show resolved Hide resolved
pkgs/applications/gis/qgis/unwrapped-ltr.nix Show resolved Hide resolved
pkgs/applications/gis/qgis/unwrapped.nix Show resolved Hide resolved
@fpletz
Copy link
Member Author

fpletz commented Jul 9, 2024

QGIS is building successfully even without this PR on Hydra.

Then it maybe was the gdal update? Not sure judging by hydra.

fpletz and others added 3 commits July 9, 2024 23:06
@fpletz @imincik
grass: pin to python 3.11
1e0e33c 
Pin to Python 3.11 since wxpython is not yet compatible to Python 3.12.

Co-authored-by: Ivan Mincik <[email protected]>
@fpletz
qgis: pin to python 3.11
bf6402f 
Grass depends on Python 3.11 as wxwidgets doesn't support Python 3.12.
@fpletz
qgis-ltr: pin to python 3.11
5bdcce6 
Grass depends on Python 3.11 as wxwidgets doesn't support Python 3.12.
@fpletz fpletz force-pushed the pkgs/grass-python-311 branch from 6dbe31b to 5bdcce6 Compare July 9, 2024 21:07
@ofborg ofborg bot requested a review from imincik July 9, 2024 22:18
@imincik
Copy link
Contributor

imincik commented Jul 10, 2024 •
edited
Loading

@fpletz , thanks for taking my suggestion for GRASS.

What about removing QGIS changes from this PR so we can merge GRASS now. I need to investigate more what's going on with QGIS (I'm traveling right now and will have a look in next few days).

@imincik imincik mentioned this pull request Jul 10, 2024
Merged
@fpletz
Copy link
Member Author

fpletz commented Jul 10, 2024

@imincik I disagree because right now QGIS doesn't work at all. With this PR at least qgis works. I don't know why qgis-ltr fails but this PR improves the situation.

@fpletz
Copy link
Member Author

fpletz commented Jul 10, 2024 •
edited
Loading

The be more precise: Without this PR, qgis fails to evaluate:

error: wxpython-4.2.1 not supported for interpreter python3.12

With the grass changes qgis builds but fails to start.

With the grass and qgis changes from this PR, qgis works. qgis-ltr fails in the same way with or without the changes in this PR but the changes are necessary until wxpython supports python 3.12.

@imincik
Copy link
Contributor

imincik commented Jul 10, 2024

but the changes are necessary until wxpython supports python 3.12.

I am not able to investigate now, but we have python312Packages.wxpython.

Will have a look ASAP.

@fpletz
Copy link
Member Author

fpletz commented Jul 10, 2024

I am not able to investigate now, but we have python312Packages.wxpython.

Which fails to evaluate. 😸

globin
globin approved these changes Jul 10, 2024
View reviewed changes
Copy link
Member

@globin globin left a comment

Choose a reason for hiding this comment

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

The pins can be removed again when wxpython supports python 3.12. This is the logical solution for the time being.

@globin globin dismissed imincik’s stale review July 10, 2024 14:03

See review

@globin globin merged commit 4fe6a12 into NixOS:master Jul 10, 2024
26 of 28 checks passed
@fpletz fpletz deleted the pkgs/grass-python-311 branch July 10, 2024 14:13
@imincik
Copy link
Contributor

imincik commented Jul 12, 2024

@fpletz , @globin qgis-ltr should be fixed in #326550 . Please review if you can.

imincik added a commit to imincik/geospatial-nix that referenced this pull request Jul 18, 2024
@imincik
grass: pin to python 3.11
ae20dfd 
Nixpkgs PR:
NixOS/nixpkgs#325731
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mjoerg mjoerg mjoerg left review comments

@globin globin globin approved these changes

@sikmir sikmir Awaiting requested review from sikmir

@nh2 nh2 Awaiting requested review from nh2

@l0b0 l0b0 Awaiting requested review from l0b0

@nialov nialov Awaiting requested review from nialov

@willcohen willcohen Awaiting requested review from willcohen

@lsix lsix Awaiting requested review from lsix

@imincik imincik Awaiting requested review from imincik

Assignees
No one assigned
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fpletz @imincik @mjoerg @globin