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.
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
pythonPackages.pillow: 6.2.1 -> 7.1.2 #77714
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please also squash the commits
@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. |
Anybody have some insight on why some of the image tests are now failing? Going to run |
As @cole-h mentioned in IRC, tests ofborg won't run tests by default (oops). Here is the setup summary from pillow 7.1.2 as of 716cf2a:
|
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
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)python -m PIL
confirms7.0.0
This feature was introduced in python-pillow/Pillow#3870, so running
python -m PIL
before7.0.0
will print:python3.7: No module named PIL.__main__; 'PIL' is a package and cannot be directly executed
nix-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.