Skip to content

gimp: condition python2 behind new withPython argument#203395

Merged
dotlambda merged 1 commit intoNixOS:masterfrom
LibreCybernetics:gimp-remove-python2-dep
Nov 30, 2022
Merged

gimp: condition python2 behind new withPython argument#203395
dotlambda merged 1 commit intoNixOS:masterfrom
LibreCybernetics:gimp-remove-python2-dep

Conversation

@fabianhjr
Copy link
Member

@fabianhjr fabianhjr commented Nov 28, 2022

Description of changes

Conditionally Python2 dep (EOLed 2020-01-01) as per #201859

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/)
  • 22.11 Release Notes (or backporting 22.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Nov 28, 2022
@vcunat
Copy link
Member

vcunat commented Nov 28, 2022

I wonder how important the python stuff is in gimp. The security issues in python2 will get fixed in nixpkgs (probably), so it might be OK to keep it here (by default).

@thiagokokada
Copy link
Contributor

thiagokokada commented Nov 28, 2022

I wonder how important the python stuff is in gimp. The security issues in python2 will get fixed in nixpkgs (probably), so it might be OK to keep it here (by default).

I think we need to add it to the release notes for 23.05 at least.

Looking at https://bugzilla.redhat.com/show_bug.cgi?id=1737933, Python looks like necessary for a good part of the core functionality of Gimp, so yeah, disabling by default may be bad.

However, I would still go with this as-is. While I am trying to get Python 2 in a good state here in nixpkgs (see #203362 and #203428) for the use cases that I think it matters (mainly for old packages that will never be updated to work with Python 3), if we can reduce the users of Python 2, it is better.

IMO, we should work in adding Gimp 2.99.x as an option for those that want it. Gimp 2.99.x will bring both GTK3 and Python 3 support, so this would fix two of our main deprecation targets in one package. Of course, this should be done in another PR, so I am giving this one a go.

@fabianhjr
Copy link
Member Author

Afaik it is used for plugins and not as common.

@jtobin might have better information.

@thiagokokada
Copy link
Contributor

Afaik it is used for plugins and not as common.

@jtobin might have better information.

Arch Linux seems to ship it without support for Python, so this looks good: https://github.com/archlinux/svntogit-packages/blob/d25b5ac3e107623411b25ffde09cd74efc37c457/trunk/PKGBUILD#L50

@fabianhjr fabianhjr added the 12.approvals: 1 This PR was reviewed and approved by one person. label Nov 29, 2022
@fabianhjr fabianhjr force-pushed the gimp-remove-python2-dep branch from 4c0a349 to d3b71d5 Compare November 30, 2022 01:39
@fabianhjr fabianhjr added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Nov 30, 2022
@dotlambda
Copy link
Member

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

1 package marked as broken and skipped:
  • gimpPlugins.exposureBlend
14 packages built:
  • gimp (gimpPlugins.gimp)
  • gimp-with-plugins
  • gimpPlugins.bimp
  • gimpPlugins.farbfeld
  • gimpPlugins.fourier
  • gimpPlugins.gap
  • gimpPlugins.gimplensfun
  • gimpPlugins.gmic
  • gimpPlugins.lightning
  • gimpPlugins.lqrPlugin
  • gimpPlugins.resynthesizer
  • gimpPlugins.texturize
  • gimpPlugins.waveletSharpen
  • toppler

@dotlambda dotlambda merged commit 194fd9b into NixOS:master Nov 30, 2022
@fabianhjr fabianhjr deleted the gimp-remove-python2-dep branch November 30, 2022 02:23
@jtojnar
Copy link
Member

jtojnar commented Nov 30, 2022

Afaik it is used for plugins and not as common.

The following built-in plug-ins use Python:

  • lib/gimp/2.0/plug-ins/colorxhtml/colorxhtml.py
  • lib/gimp/2.0/plug-ins/file-openraster/file-openraster.py
  • lib/gimp/2.0/plug-ins/foggify/foggify.py
  • lib/gimp/2.0/plug-ins/gradients-save-as-css/gradients-save-as-css.py
  • lib/gimp/2.0/plug-ins/histogram-export/histogram-export.py
  • lib/gimp/2.0/plug-ins/palette-offset/palette-offset.py
  • lib/gimp/2.0/plug-ins/palette-sort/palette-sort.py
  • lib/gimp/2.0/plug-ins/palette-to-gradient/palette-to-gradient.py
  • lib/gimp/2.0/plug-ins/py-slice/py-slice.py
  • lib/gimp/2.0/plug-ins/python-console/python-console.py
  • lib/gimp/2.0/plug-ins/python-eval/python-eval.py
  • lib/gimp/2.0/plug-ins/spyro_plus/spyro_plus.py

I do not think we have any third-party Python plug-ins packaged in gimpPlugins.

IMO, we should work in adding Gimp 2.99.x as an option for those that want it. Gimp 2.99.x will bring both GTK3 and Python 3 support, so this would fix two of our main deprecation targets in one package. Of course, this should be done in another PR, so I am giving this one a go.

The unstable version is packaged in #67576

I did not want to merge it to avoid increasing maintenance burden but perhaps we have no other choice if we want to preserve those plug-ins.

, Cocoa
, gtk-mac-integration-gtk2
, withPython ? false
, python2 ? null
Copy link
Member

@SuperSandro2000 SuperSandro2000 Nov 30, 2022

Choose a reason for hiding this comment

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

Why do we need the ? null here? We are not going to drop the python2 package from the tree.

#203752

Copy link
Member

Choose a reason for hiding this comment

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

We might in the future.

Copy link
Member

Choose a reason for hiding this comment

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

Then we can still do this in the future or just remove the dead code.

@adisbladis
Copy link
Member

This is a breaking change and should at least come with a release note.

@Sigmanificient
Copy link
Member

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

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants