Skip to content

writers.writePython2: remove / writers.writePyPy{2,3}: init#150132

Merged
Lassulus merged 2 commits intoNixOS:masterfrom
wamserma:remove-python2Writer
Dec 15, 2021
Merged

writers.writePython2: remove / writers.writePyPy{2,3}: init#150132
Lassulus merged 2 commits intoNixOS:masterfrom
wamserma:remove-python2Writer

Conversation

@wamserma
Copy link
Member

@wamserma wamserma commented Dec 10, 2021 •
edited
Loading

Motivation for this change

#148779
python2 is EOL and due for removal; functions unused in nixpkgs

a replacement using the PyPy interpreter is added to ease transition

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 pr 150132". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@wamserma wamserma requested a review from FRidh December 10, 2021 21:14
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Dec 10, 2021
rnhmjoj
rnhmjoj requested changes Dec 11, 2021
View reviewed changes
Copy link
Contributor

@rnhmjoj rnhmjoj left a comment

Choose a reason for hiding this comment

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

This change needs an entry in the release notes and maybe an alias.

@Lassulus
Copy link
Member

Lassulus commented Dec 11, 2021 •
edited
Loading

maybe we should inline the makePythonWriter function into writePython3?
not needed anymore since we have mutiple python writers again

@wamserma wamserma force-pushed the remove-python2Writer branch from fde5bf3 to 1a62fac Compare December 11, 2021 13:02
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation labels Dec 11, 2021
@wamserma wamserma changed the title writers.writePython2: remove writers.writePython2: remove / writers.writePyPy{2,3}: init Dec 11, 2021
@wamserma wamserma force-pushed the remove-python2Writer branch from 1a62fac to 757a0ea Compare December 11, 2021 13:06
@wamserma
Copy link
Member Author

wamserma commented Dec 11, 2021

This change needs an entry in the release notes and maybe an alias.

PR updated, also added writers for PyPy as an alternative for legacy scripts.

@wamserma wamserma requested a review from rnhmjoj December 11, 2021 13:08
@ofborg ofborg bot added 8.has: clean-up This PR removes packages or removes other cruft 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. and removed 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Dec 11, 2021
@rnhmjoj
Copy link
Contributor

rnhmjoj commented Dec 12, 2021

PR updated, also added writers for PyPy as an alternative for legacy scripts.

Thanks you. Unfortunately the aliases seem to be messing with pkgs.tests.writers somehow. It has probably to do with writers being an attrset, but I don't know how to make an alias in this case.

@wamserma
Copy link
Member Author

wamserma commented Dec 12, 2021

PR updated, also added writers for PyPy as an alternative for legacy scripts.

Thanks you. Unfortunately the aliases seem to be messing with pkgs.tests.writers somehow. It has probably to do with writers being an attrset, but I don't know how to make an alias in this case.

Ah yes, fixed.

@wamserma wamserma force-pushed the remove-python2Writer branch from 757a0ea to 31f8472 Compare December 12, 2021 14:05
@wamserma wamserma requested a review from Lassulus December 12, 2021 14:05
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. labels Dec 12, 2021
@piegamesde piegamesde mentioned this pull request Dec 13, 2021
Closed
@Lassulus
Copy link
Member

Lassulus commented Dec 15, 2021

sadly this has already a merge conflict.

@Lassulus Lassulus added the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 15, 2021
@wamserma wamserma force-pushed the remove-python2Writer branch from 31f8472 to b93e478 Compare December 15, 2021 09:01
@wamserma
Copy link
Member Author

wamserma commented Dec 15, 2021

sadly this has already a merge conflict.

Rebased the PR.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 15, 2021
@Lassulus
Copy link
Member

Lassulus commented Dec 15, 2021

hmm, seems like the pypy3 writer test doesn't work for me:
https://p.krebsco.de/0bww66n

@wamserma
Copy link
Member Author

wamserma commented Dec 15, 2021

hmm, seems like the pypy3 writer test doesn't work for me

That is due to pypy3Packages.bootstrapped-pip not building. Nothing caused by this PR.

@Lassulus Lassulus merged commit 65ca86b into NixOS:master Dec 15, 2021
@wamserma wamserma deleted the remove-python2Writer branch December 15, 2021 10:02
@Profpatsch
Copy link
Member

Profpatsch commented Jan 1, 2022

Why? Why remove it? Does that mean all programs written for python 2 have to be deleted now, because somebody declared it “EOL”?

@Profpatsch
Copy link
Member

Profpatsch commented Jan 1, 2022

I’d like to revert this until python2 is actually removed (iff that ever happens which I honestly hope it won’t).

@Profpatsch
Copy link
Member

Profpatsch commented Jan 1, 2022

To be clear, I am rather angry at this change, breaking an API like this without any good reason (and no, “EOL” is not a good reason!) is a total no-go.

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Jan 1, 2022

and no, “EOL” is not a good reason!

It totally is for me: the CPython 2 interpreter and its standard library almost surely have security vulnerabilities that will never be fixed.

@7c6f434c
Copy link
Member

7c6f434c commented Jan 1, 2022 via email

@wamserma
Copy link
Member Author

wamserma commented Jan 1, 2022

@Profpatsch This is just a helper to write a python script from a nix expression and have a given interpreter (Python2) set. To my knowledge this helper was not used in nixpkgs and most likely unused outside nixpkgs.
We want to retire Python2 from nixpkgs, because it is EOL and will not get any security fixes.
If you still need to write scripts with Python2 syntax from a nix expression, you can simply switch to writePyPy2, which will set PyPy as interpreter in the scripts shebang line. All other are urged to convert their scripts to Python3.
Further, we did not "break" the API, we rather deprecated it. You can always revive the python2-writer with an overlay without side-effects on the other script-writing helpers, if you have the need to keep it.

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

Reviewers

@Profpatsch Profpatsch Awaiting requested review from Profpatsch

@FRidh FRidh Awaiting requested review from FRidh

@rnhmjoj rnhmjoj Awaiting requested review from rnhmjoj

@Lassulus Lassulus Awaiting requested review from Lassulus

Assignees

No one assigned

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: clean-up This PR removes packages or removes other cruft 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@wamserma @Lassulus @rnhmjoj @Profpatsch @7c6f434c