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

qt5.qtbase: python2 -> python3 #75916

Merged
merged 1 commit into from Dec 19, 2019
Merged

qt5.qtbase: python2 -> python3 #75916

merged 1 commit into from Dec 19, 2019

Conversation

grahamc
Copy link
Member

@grahamc grahamc commented Dec 19, 2019 •
edited

The only python reference in qtbase's dev output is a python script
which imports print from future (mkspecs/features/uikit/devices.py)
so I guess it is good enough.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

@worldofpeace
Copy link
Contributor

Does this not mean this is broken? Future won't be included, and this is native.

@ofborg ofborg bot requested review from qknight, periklis and bkchr December 19, 2019 02:21
@grahamc
Copy link
Member Author

grahamc commented Dec 19, 2019

No, python2 added a __future__ module to bridge thegap to py3, and it continues to work in py3: https://docs.python.org/3.8/library/__future__.html https://docs.python.org/2/library/__future__.html Seeing an import __future__ statement means the author is intentionally supporting 2 and 3.

@worldofpeace
Copy link
Contributor

No, python2 added a __future__ module to bridge thegap to py3, and it continues to work in py3: https://docs.python.org/3.8/library/__future__.html https://docs.python.org/2/library/__future__.html Seeing an import __future__ statement means the author is intentionally supporting 2 and 3.

Oh, future != __future__ 😄 But this should still be a buildInputs?

@grahamc
Copy link
Member Author

grahamc commented Dec 19, 2019

That is above my pay grade, I don't know! :o

@grahamc @worldofpeace
qt5.qtbase: python2 -> python3
ef96a4a 
The only python reference in qtbase's dev output is a python script
which imports print from __future__ (mkspecs/features/uikit/devices.py).
It's not clear whether it depends on python or it just provides a python
script. But I believe it's still good to have in buildInputs so it has
a proper shebang.

Co-authored-by: worldofpeace <worldofpeace@protonmail.ch>
@worldofpeace worldofpeace changed the base branch from master to staging December 19, 2019 02:47
@worldofpeace
Copy link
Contributor

Ahh nixpkgs, we're really blessed in needless pings sometimes.

@grahamc
Copy link
Member Author

grahamc commented Dec 19, 2019

We should surely wait for ofborg to test this one now :)

@ofborg ofborg bot added 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: nixos labels Dec 19, 2019
@ofborg ofborg bot requested a review from bkchr December 19, 2019 03:01
@jonringer
Copy link
Contributor

jonringer commented Dec 19, 2019 •
edited

in python3, import __future__ essentially does nothing. You should only need the python3 interpreter if that's the case

EDIT: what i said isn't technically true, but the most common use case it to allow for python2 to run python3 code.

@grahamc grahamc merged commit 5782c39 into NixOS:staging Dec 19, 2019
@grahamc grahamc deleted the python3/qtbase branch December 19, 2019 03:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ttuegel ttuegel Awaiting requested review from ttuegel ttuegel is a code owner

@qknight qknight Awaiting requested review from qknight

@periklis periklis Awaiting requested review from periklis

@bkchr bkchr Awaiting requested review from bkchr

Assignees
No one assigned
Labels
6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: nixos 6.topic: python 6.topic: qt/kde 6.topic: rust 8.has: documentation 8.has: module (update) 8.has: package (new) 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-darwin-stdenv 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@grahamc @worldofpeace @jonringer