pythonPackages.pillow: 6.2.1 -> 7.1.2 #77714
Conversation
|
@jonringer anything else you can think of? |
|
no, LGTM. However, it takes forever to |
|
Could someone add the security label to this please? https://nvd.nist.gov/vuln/detail/CVE-2020-5310 |
|
@jonringer is the |
|
I think it kept freezing due to nipype halting on python3.8 tests (so the review would never complete). Now this is hitting an error with nix-review |
|
And that is blocking this PR? |
|
|
Yeesh, quite a few there. How is this sort of breakage typically addressed? |
|
Is anyone still working on this issue? |
|
I’m happy to do whatever but unsure what needs to be done at this point. |
|
generally try to mitigate breakages as much as possible some breakages will be unrelated to your changes, and they can be left broken |
|
I'm going to try bumping Pillow and running I suspect some packages might need to be locked to python3+, but I'll see what this initial run does, first. Might not have too much time until finals are over, though I do plan on getting this merged. |
evanjs
changed the title
|
Anybody have some insight on why some of the image tests are now failing? Going to run |
pythonPackages.pillow: drop support for python2.7 - Support for Python 2.7 was removed in Pillow 7.0.0 - https://github.com/python-pillow/Pillow/blob/master/CHANGES.rst#700-2020-01-02 - use test command from travis build configuration - https://github.com/python-pillow/Pillow/blob/1671f6bd71888c1454cd9f0e06dfae5976391984/.ci/test.sh - add support for jpeg2k, imagequant
add python_magic to propagatedBuildInputs to allow django-versatileimagefield package to build
apply patch from MIC-DKFZ/batchgenerators#59 to allow batchgenerators to build with pillow 7.1.2+
|
Updated, rebased, squashed, cleaned, etc. Please let me know if there's anything else you guys can think of! |
|
|
|
@GrahamcOfBorg build python3Packages.pillow python38Packages.pillow |
Yeah, I was experiencing issues with the CUDA-based packages, and my only CUDA machine is sorta low on disk space for a |
|
apparently 256G of RAM isn't enough to ward off the oom daemon. Had to add another 256G of swap :( |
|
|
Yeahhh that’s what I was seeing, too. For example, I was able to build When I went through the This is assuming that I’d be happy to go through the errors for the other packages, but am currently unable to do a full @jonringer any chance you could get me the logs folder for that review? Assuming none of the failing build logs are blank... I was seeing a lot of that in my end |
|
I'll make a gist when it's complete, but my quick overview was that no failures were specifically from pillow bump (but there's a lot of noise, so can't be 100% sure) |
|
https://gist.github.com/jonringer/f4acde3d40383d5c6633bb1cc2299ec6 not complete, but most of the failures are there |
|
@jonringer Results from building individual packagesFailers:
- ib-controller: N/A - uses requireFile
- python37Packages.aplpy - unrelated (?) test failures
[x] python37Packages.arviz
[x] python37Packages.baselines
[x] python37Packages.batchgenerators - Could not find a version that satisfies the requirement pillow<7.1 (from batchgenerators==0.20.1) (also 0.20.0) ## FIXED with 0c9c0d6feb3005686174d016b8341c2980061ca7
- python37Packages.blaze - broken
- python37Packages.caffe - Could not find OpenBLAS
- python37Packages.dask-jobqueue - failed tests (network-related?)
- python37Packages.datashader - unrelated test failures (e.g. `w ib-cTypeError: 'float' object is not iterable`)
[x] python37Packages.django-versatileimagefield
[x] python37Packages.dm-sonnet
[x] python37Packages.edward
[x] python37Packages.graph_nets
- python37Packages.imbalanced-learn - errors encountered in tests (e.g. socket.gaierror: [Errno -2] Name or service not known)
[x] python37Packages.mask-rcnn
- python37Packages.odo - broken
- python37Packages.optuna - Could not find OpenBLAS
- python37Packages.pelican - test errors - assert not out, out - test_custom_generation_works - pillow is a dep but pelican is up to date in nixpkgs so either the test failures are unrelated or it"'"s an issue upstream
- python37Packages.persim - various test errors (e.g. "ValueError: Non-string object detected for the array ordering. Please pass in 'C', 'F', 'A', or 'K' instead")
- python37Packages.psd-tools - Could not find a version that satisfies the requirement attrs>=19.2.0 (from psd-tools==1.8.35) (from versions: none)
- python37Packages.pygbm - test errors (e.g. "TypeError: Failed in nopython mode pipeline (step: convert to parfors)")
[x] python37Packages.pymc3
- python37Packages.pyro-ppl - broken - "Could not find a version that satisfies the requirement pyro-api>=0.1.1 (from pyro-ppl==1.1.0) (from versions: none)"
- python37Packages.pytorchWithCuda - failed to build (?)
- python37Packages.ripser - needs persim
- python37Packages.rl-coach - broken
- python37Packages.scikit-tda - needs persim
- python37Packages.streamz - errors during tests - "TypeError: __new__() got an unexpected keyword argument 'start'"
- python37Packages.stumpy - errors during tests - "ValueError: tuple is not allowed for map key"
- python37Packages.sunpy - errors during tests - "RuntimeError: unsupported data type"
[x] python37Packages.tensorflow (python37Packages.tensorflow-build ,python37Packages.tensorflow-build_1 ,python37Packages.tensorflowWithoutCuda ,python37Packages.tensorflow_1)
- python37Packages.tensorflow-bin (python37Packages.tensorflow-bin_1) - Could not find a version that satisfies the requirement tensorflow-estimator<1.15.0rc0,>=1.14.0rc0 (from tensorflow==1.14.0) (from versions: none)
[x] python37Packages.tensorflow_2 (python37Packages.tensorflow-build_2)
[x] python37Packages.tensorflow-probability
- python37Packages.tensorflowWithCuda
[x] python37Packages.tflearn
- python38Packages.apache-airflow - failed install tests - "sqlite3.OperationalError: no such table: log"
- python38Packages.aplpy - same errors as python37 package
[x] python38Packages.batchgenerators
- python38Packages.blaze - py37
- python38Packages.caffe - py37
- python38Packages.dask-jobqueue -py37
- python38Packages.datashader - py37
- python38Packages.django-raster - failing test - "celery.exceptions.InvalidTaskError: Task keyword arguments is not a mapping"
- python38Packages.django-versatileimagefield
"
- python38Packages.imbalanced-learn - py37
- python38Packages.odo - py37
- python38Packages.pelican - py37
- python38Packages.persim - py37
- python38Packages.psd-tools - py37
- python38Packages.pygbm - py37
- python38Packages.pyro-ppl - py37
- python38Packages.pytorchWithCuda - py37
- python38Packages.ripser - py37
- python38Packages.scikit-tda - py37
- python38Packages.streamz - py37
- vimiv - test failure - "ValueError: Namespace Gtk not available"The only packages I couldn't verify were those depending on tensorflow, specifically pytorch. If somebody can verify the errors I saw with the tensorflow packages, I think that should cover everything after considering the unrelated errors noted for other python packages. I'm not sure why these packages would have failed to build in @jonringer's |
I'm testing my new 3990X server a built. Some packages such as I think this PR is fine as is. We'll just deal with breakages on master |
|
Thanks @jonringer. This PR has been driving my anxiety with all the failures for the past few months. Happy to take a look at things that do end up breaking in master. It looks like the CVEs only affect <6.2.0, so this can probably wait until 20.09, unless there’s any other reason we should backport. Thanks guys! |
|
Looks like pytorch and batchgenerators build fine on master with Pillow 7.1.2. Thanks again for being so thorough, @jonringer! I do wonder why/if there’s a way we can improve nixpkgs-review in this regard, though. cc @Mic92 |
|
@jonringer Which part does take so much memory. The evaluation or building? What process do you see in htop when it eats all the RAM? |
It's when doing |
Yes, but can you check what nix command is causing this? |
|
I think this PR was merged prematurely. Of course I understand that we all make mistakes, but @evanjs even explicitly stated in the PR body that this broke Python 2.7 compatibility. |
|
I think they have been quite thorough here.
Yes, which is EOL. If users need Python 2.7 variants they need to put in the effort to ensure that keeps working. As I wrote on Discourse there is going to be a lot more breakage the coming weeks due to packages dropping Python 2 support. |
The PR had been going on for four months. There was ample time for others to review. It would have been nice to have some help as this is a major version bump, and I felt like it was disingenuous to @evanjs to draw this PR out any longer. He already put forward some effort to fix some issues in downstream packages, and verified that some of the failures I had with my review were able to build successfully if built individually. Although breakages are never ideal, you can see there is a lot of noise with broken packages currently. So it's hard to determine what was broken before. Even if the tooling improved with Mic92/nixpkgs-review#85 , it's an issue that it took ~4hrs on my 3990X to complete the review, even then, some packages were erroneously failing either due to too high of NIX_BUILD_CORE (E.g. bash-completion fails with NIX_BUILD_CORE=128) or oom issues even with 256GB of ram + 256GB swap. |
Motivation for this change
7.1.2 (2020-04-25)
7.1.1 (2020-04-02)
7.1.0 (2020-04-01)
7.0.0 (2020-01-02)
Support for Python 2.7 has been dropped
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)python -m PILconfirms7.0.0This feature was introduced in python-pillow/Pillow#3870, so running
python -m PILbefore7.0.0will print:python3.7: No module named PIL.__main__; 'PIL' is a package and cannot be directly executednix-shell -p nixpkgs-review --run "nixpkgs-review wip"passed, but is there anything more in depth I can do to test reverse dependencies or etc?The only thing I can think of is any libraries that might depend on this but don't require python2.7, etc.