Bug 243665

Summary: graphics/py-pillow: Update to 7.0.0
Product: Ports & Packages Reporter: Kai Knoblich <kai>
Component: Individual Port(s)Assignee: Kai Knoblich <kai>
Status: Closed FIXED    
Severity: Affects Only Me CC: koobs, python, sunpoet
Priority: --- Flags: koobs: maintainer-feedback+
Version: Latest   
Hardware: Any   
OS: Any   
URL: https://pillow.readthedocs.io/en/stable/releasenotes/7.0.0.html
See Also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=243336
https://reviews.freebsd.org/D23713
Attachments:
Description Flags
py-pillow-7.0.0.patch
kai: maintainer-approval+
py-pillow-7.0.0-bump-consumers.patch
none
py-pillow-7.0.0-bump-consumers-v2.patch none

Description Kai Knoblich freebsd_committer freebsd_triage 2020-01-28 09:31:09 UTC
Created attachment 211124 [details]
py-pillow-7.0.0.patch

Attached is sunpoet's patch from bug #243336 with additional CONFLICTS_INSTALL entries.

Before applying the patch one needs to do "svn cp graphics/py-pillow graphics/py-pillow6".
Comment 1 Kai Knoblich freebsd_committer freebsd_triage 2020-01-28 09:41:09 UTC
Results of "make test" for each Python flavor:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

With py37, py38:

> ==== 1298 passed, 129 skipped in 32.14 seconds ====
> ==== 1298 passed, 129 skipped in 31.40 seconds ====

With py35:

> ==== 1296 passed, 131 skipped in 40.62 seconds ====

With py36:

> ==== 1297 passed, 130 skipped in 33.41 seconds ====

Version requirements in setup.py, etc.:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Except two ports no consumer has pinned requirements for graphics/py-pillow. The two exceptions are:

- net-mgmt/netbox: OK, patched out in Makefile
- www/py-wagtail: OK, defined in Makefile and setup.py as "<7.0".


More thorough QA results regarding the backwards incompatible changes that were introduced in the 7.0.0 release (see also the release notes in the URL field for further detail):


Removal of the PILLOW_VERSION constant:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- graphics/impressive: OK, still uses Python 2.7 only and already has compatible code to cope with different Pillow versions.
- graphics/py-blockdiag: OK, still uses Python 2.7 only. No compatible code for newer Pillow versions.
- graphics/py-mcomix: OK, still uses Python 2.7 only. No compatible code for newer Pillow versions.
- graphics/sk1: OK, still uses Python 2.7 only. No compatible code for newer Pillow versions.
- print/hplib: OK, already patched out in r503333 as preparation for the 6.0 release of graphics/py-pillow.
- x11/xpra: OK, still uses Python 2.7 only and already has compatible code to cope with different Pillow versions.

Conclusion: OK, as all those consumers (except print/hplib maybe, as it already runs with Python 3) will get graphics/py-pillow6 as a dependency and should work as usual.

Removal of the *ImagePlugin.__version__ constants:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- OK, no port makes use of these constants.


Removal of PyQt4 and PySide code:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- OK, not relevant as Qt4 and PySide are already gone from the Ports tree.


Removal of setting the image size for TIFF images by "im.size(x, y):
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- graphics/py-imageio: OK, uses "im.size = ( ... )" only in documentation context in "imageio/plugins/pillow_info.py".
- graphics/py-imageio24: OK, uses "im.size = ( ... )" only in documentation context in "imageio/plugins/pillow_info.py".
- net-p2p/deluge-cli: OK, uses Python 2.7 only and "im.size = ( ... )" is used for ICO files. In this context using "im.size = ( ... )" is allowed as it selects a subimage.


Changed return value of "Image.draft()":
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- graphics/py-mcomix: OK, uses Python 2.7 only.
- graphics/py-sane: OK, uses Python 2.7 only.

Conclusion: OK, as all those consumers will get graphics/py-pillow6 as a dependency and should work as usual.


Default resampling filter:
~~~~~~~~~~~~~~~~~~~~~~~~~~
The default resampling filter was changed from "IMAGE.NEAREST" to "IMAGE.BICUBIC" for the "fit()", "pad()", "resize()" and "scale()" functions. Those functions are used by several consumers for image processing/manipulation.

Conclusion: Should be OK, but we should keep in mind that the changes of the above mentioned functions will have definitely a small impact on the Pillow consumers. A bit more CPU cycles than usual are then required for image processing but yields in better image results/quality. 

If there are problems once Pillow has been updated to 7.0.0 we can still assign the problematic consumers (if any) to Pillow 6.2.2 and bump PORTREVISION.
Comment 2 Kai Knoblich freebsd_committer freebsd_triage 2020-01-28 09:42:50 UTC
Created attachment 211125 [details]
py-pillow-7.0.0-bump-consumers.patch

Attached a prelimary patch that changes the *_DEPENDS to "graphics/py-pillow6" and bumps the PORTREVISION of all consumers that can still be built for Python 2.7 or later.

Exceptions are ports, that have "USES=python:3.[0-9]+" and no conflicting dependencies that could break "bulk -a":

- astro/py-astLib
- audio/lollypop
- games/openage
- games/unknown-horizons
- graphics/py-cairosvg
- graphics/py-imageio
- graphics/py-pyocr
- graphics/variety
- math/py-plastex
- misc/py-tflearn
- net-im/py-matrix-synapse
- net-mgmt/netbox
- science/gramps
- www/py-weboob
- www/xist
Comment 3 Kai Knoblich freebsd_committer freebsd_triage 2020-01-28 09:45:04 UTC
Technically a few more ports could make of use of Pillow 7.0.0 if the Python 2.7 build is deactivated for them, e.g. by setting "USES=python:3.5+", to avoid "bulk -a" breakages. It would also save a few PORTREVISION bumps and would be a few more steps towards the deorbit of Python 2.7.

I left those ports out from the initial patch for the sake of brevity:

Ports have no consumers that depend upon them and no conflicting dependencies:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- devel/py-pydenticon
- games/py-hypatia_engine
- graphics/py-cartopy
- graphics/py-photocollage
- graphics/py-pypillowfight
- misc/py-gluoncv
- security/vinetto (unmaintained)
- www/py-instabot


Ports that doesn't have "USE_PYTHON=distutils" but can be also built for any Python version:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- cad/k40-whisperer
- comms/apitran
- math/asymptote
- misc/cs
- misc/mmdnn

Ports that have "USE_PYTHON=distutils noflavors":
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- games/pysolfc (I plan to drop the Python 2.7 support as part of this issue.)


If we want to go that route with dropping the Python 2.7 support for those ports we should do it before landing Pillow 7.0.0, IMHO.
Comment 4 Kubilay Kocak freebsd_committer freebsd_triage 2020-01-28 10:52:51 UTC
Thank you for this Kai, greatly appreciated, especially the extensive QA

Pillow 7 update looks good.

Couple of notes:

Regarding www/py-wagtail: OK, defined in Makefile and setup.py as "<7.0"...

We should probably its *_DEPENDS to ${PYTHON_PKGNAMEPREFIX}pillow<7.0:graphics-py-pillow6

More broadly (with the above as an example), as part of the QA, if we come across any ports that have depends either inconsistent or incomplete (especially ">X & <Y" cases) with their setup.py's, we should fix those and make them match.

Those changes are:

Approved by: portmgr (blanket(s): Fix dependencies, framework/python compliance)
Comment 5 Kai Knoblich freebsd_committer freebsd_triage 2020-02-13 13:04:41 UTC
Created attachment 211612 [details]
py-pillow-7.0.0-bump-consumers-v2.patch

Attached is an updated patch for the consumers of graphics/py-pillow.

(In reply to Kubilay Kocak from comment #4)

We've already discussed the situation about www/py-wagtail on IRC some days ago. The dependency for graphics/py-pillow with "< 7.0" is declared correctly in the port's Makefile and in the setup.py thus no particular action is required.

I also assume that I've your approval to land Pillow 7.0.0. ;-)
Comment 6 Kubilay Kocak freebsd_committer freebsd_triage 2020-02-14 01:17:41 UTC
(In reply to Kai Knoblich from comment #5)

You have the helm Kai, thank you
Comment 7 Po-Chuan Hsieh freebsd_committer freebsd_triage 2020-02-14 09:59:46 UTC
(In reply to Kai Knoblich from comment #5)

Please hold on. I assume we'll have pillow 7.0.0 landed with dependent ports having conditional dependency on pillow/pillow6. But the patches just moves all dependent ports to pillow6.
Comment 8 Kai Knoblich freebsd_committer freebsd_triage 2020-02-14 18:43:53 UTC
(In reply to Sunpoet Po-Chuan Hsieh from comment #7)

Not all ports are assigned to graphics/py-pillow6 as noted in comment #2. 

I assume you mean something like this for consumers that can also be built for Python 2.7:

> .if ${PYTHON_REL} < 3500
> RUN_DEPENDS+=   ${PYTHON_PKGNAMEPREFIX}pillow6>=0:devel/py-pillow6@${PY_FLAVOR}
> .else
> RUN_DEPENDS+=   ${PYTHON_PKGNAMEPREFIX}pillow>=0:devel/py-pillow@${PY_FLAVOR}
> .endif

I initially included the above in my considerations and I'm not entirely averse to it either, because more ports can be used with Pillow 7.0.0 then.

But I went away from that idea for now as it also makes the whole scenario more complex, IMHO. (I'd start a review on Phabricator then to get a better overall picture).

I'd just like to know if there are good reasons to extend the grace period of Python 2.7 by adding the conditionals from above?
Comment 9 Po-Chuan Hsieh freebsd_committer freebsd_triage 2020-02-14 20:30:57 UTC
(In reply to Kai Knoblich from comment #8)

> Not all ports are assigned to graphics/py-pillow6 as noted in comment #2. 

Oh, I overlooked that.

I think pillow has enough consumers (almost 100) to have PY_PILLOW in Mk/Uses/python.mk. That's also what I planned for math/py-scipy update.

> I'd just like to know if there are good reasons to extend the grace period of Python 2.7 by adding the conditionals from above?

According to EXPIRATION_DATE of python27, it has 1-year grace period. I still keep python 2.7 support temporarily.

In other words, if a port drops python 2.7 support, could we simply restrict all its dependent ports to use python 3 (if they all work fine)?
Comment 10 Kubilay Kocak freebsd_committer freebsd_triage 2020-02-15 00:03:19 UTC
(In reply to Sunpoet Po-Chuan Hsieh from comment #9)

I'd like to see less dependencies in python.mk than more. Every Python package that declares a dependency that includes a <version-spec> cant be accurately or correctly described using a shared dependency line

It doesn't even matter whether or not the Python package declares the <version-spec> correctly or not. Whatever is specified in setup.py *must* be 'exactly' satisfied, otherwise it results in a runtime dependency error (even when the dependency is installed).
Comment 11 Kai Knoblich freebsd_committer freebsd_triage 2020-02-16 09:33:38 UTC
(In reply to Sunpoet Po-Chuan Hsieh from comment #9)
(In reply to Kubilay Kocak from comment #10)

Ok, I created a review at Phabricator that contains conditional statements to use either Pillow 7 or Pillow 6 for consumers that can be built with Python 2 or newer.
Comment 12 commit-hook freebsd_committer freebsd_triage 2020-02-24 11:36:59 UTC
A commit references this bug:

Author: kai
Date: Mon Feb 24 11:36:24 UTC 2020
New revision: 526962
URL: https://svnweb.freebsd.org/changeset/ports/526962

Log:
  graphics/py-pillow: Update to 7.0.0

  * Repo-Copy the Pillow 6.2.2 release to graphics/py-pillow6 to retain
    backwards compatibility for Python 2 consumers as the Pillow 7.0.0 release
    dropped the support for Python 2.

  * Apply conditional statements to use either Pillow 7.x or Pillow 6.x for
    consumers that can be built for Python 2 or newer.

  * Exceptions are ports that can be built only for either Python 2 or
    Python 3.  For the first case, consumers are just assigned to the
    repo-copied graphics/py-pillow6.

  * Also remove Pillow from BUILD_DEPENDS of math/py-PyWavelets as it is not
    listed in setup.py as a build dependency [1] and relax the version
    requirements of Pillow for www/py-wagtail.

  Release Notes:

  * https://pillow.readthedocs.io/en/latest/releasenotes/index.html

  Backward Incompatible Changes (7.0.0):

  * https://pillow.readthedocs.io/en/latest/releasenotes/7.0.0.html#backwards-incompatible-changes

  Detailed Changelog:

  * https://github.com/python-pillow/Pillow/blob/7.0.0/CHANGES.rst

  PR:		243665
  Submitted by:	sunpoet (patch for 7.0.0 and repo-copied version)
  Reviewed by:	koobs [1], sunpoet
  Approved by:	koobs (maintainer)
  Differential Revision:	https://reviews.freebsd.org/D23713

Changes:
  head/cad/k40-whisperer/Makefile
  head/comms/apitran/Makefile
  head/databases/py-mongoengine/Makefile
  head/deskutils/calibre/Makefile
  head/devel/py-asciimatics/Makefile
  head/devel/py-factory-boy/Makefile
  head/devel/py-mwlib/Makefile
  head/devel/py-pydenticon/Makefile
  head/games/fretsonfire/Makefile
  head/games/hypatia_engine/Makefile
  head/games/pysolfc/Makefile
  head/games/pythonsudoku/Makefile
  head/graphics/Makefile
  head/graphics/caffe/Makefile
  head/graphics/impressive/Makefile
  head/graphics/py-PyOpenGL/Makefile
  head/graphics/py-actdiag/Makefile
  head/graphics/py-blockdiag/Makefile
  head/graphics/py-cartopy/Makefile
  head/graphics/py-django-easy-thumbnails/Makefile
  head/graphics/py-face_recognition/Makefile
  head/graphics/py-imageio24/Makefile
  head/graphics/py-img2pdf/Makefile
  head/graphics/py-mcomix/Makefile
  head/graphics/py-nwdiag/Makefile
  head/graphics/py-photocollage/Makefile
  head/graphics/py-pillow/Makefile
  head/graphics/py-pillow/distinfo
  head/graphics/py-pillow6/
  head/graphics/py-pillow6/Makefile
  head/graphics/py-pyinsane2/Makefile
  head/graphics/py-pypillowfight/Makefile
  head/graphics/py-sane/Makefile
  head/graphics/py-scikit-image/Makefile
  head/graphics/py-seqdiag/Makefile
  head/graphics/py-sorl-thumbnail/Makefile
  head/graphics/py-soya3d/Makefile
  head/graphics/sk1/Makefile
  head/lang/mono/Makefile
  head/math/asymptote/Makefile
  head/math/py-PyWavelets/Makefile
  head/misc/cs/Makefile
  head/misc/mmdnn/Makefile
  head/misc/py-gluoncv/Makefile
  head/misc/wotsap/Makefile
  head/multimedia/freevo/Makefile
  head/net/py-rainbowstream/Makefile
  head/net-p2p/deluge-cli/Makefile
  head/print/hplip/Makefile
  head/print/py-reportlab/Makefile
  head/print/py-trml2pdf/Makefile
  head/science/rdkit/Makefile
  head/security/py-volatility/Makefile
  head/security/vinetto/Makefile
  head/textproc/py-qrcode/Makefile
  head/textproc/py-xhtml2pdf/Makefile
  head/www/go-appengine-sdk/Makefile
  head/www/google-appengine/Makefile
  head/www/py-django-filer/Makefile
  head/www/py-django-markdownx/Makefile
  head/www/py-django-mezzanine/Makefile
  head/www/py-django-photologue/Makefile
  head/www/py-django-simple-captcha/Makefile
  head/www/py-instabot/Makefile
  head/www/py-pywikibot/Makefile
  head/www/py-wagtail/Makefile
  head/www/py-wagtail/files/
  head/www/py-wagtail/files/patch-setup.py
  head/www/seahub/Makefile
  head/www/twms/Makefile
  head/x11/cinnamon/Makefile
  head/x11/py-pyscreenshot/Makefile
  head/x11/py-pyvirtualdisplay/Makefile
  head/x11/xpra/Makefile
  head/x11-toolkits/py-easygui/Makefile
  head/x11-toolkits/py-kivy/Makefile
Comment 13 Kai Knoblich freebsd_committer freebsd_triage 2020-03-28 10:44:44 UTC
Comment on attachment 211124 [details]
py-pillow-7.0.0.patch

^ Triage: Maintainer approval was given at the end of review D23713.