Skip to content
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

pillow v9.0.0 #104

Merged
merged 27 commits into from Feb 3, 2022
Merged

Conversation

regro-cf-autotick-bot
Copy link
Contributor

It is very likely that the current package version for this feedstock is out of date.

Checklist before merging this PR:

  • Dependencies have been updated if changed: see upstream
  • Tests have passed
  • Updated license if changed and license_file is packaged

Information about this PR:

  1. Feel free to push to the bot's branch to update this PR if needed.
  2. The bot will almost always only open one PR per version.
  3. The bot will stop issuing PRs if more than 3 version bump PRs generated by the bot are open. If you don't want to package a particular version please close the PR.
  4. If you want these PRs to be merged automatically, make an issue with @conda-forge-admin,please add bot automerge in the title and merge the resulting PR. This command will add our bot automerge feature to your feedstock.
  5. If this PR was opened in error or needs to be updated please add the bot-rerun label to this PR. The bot will close this PR and schedule another one. If you do not have permissions to add this label, you can use the phrase @conda-forge-admin, please rerun bot in a PR comment to have the conda-forge-admin add it for you.

Dependency Analysis

We couldn't run dependency analysis due to an internal error in the bot. :( Help is very welcome!

This PR was created by the regro-cf-autotick-bot. The regro-cf-autotick-bot is a service to automatically track the dependency graph, migrate packages, and propose package version updates for conda-forge. Feel free to drop us a line if there are any issues! This PR was generated by https://github.com/regro/autotick-bot/actions/runs/1647490976, please use this URL for debugging.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@h-vetinari
Copy link
Member

@conda-forge/pillow

I was just wondering about the following:

  --------------------------------------------------------------------
  PIL SETUP SUMMARY
  --------------------------------------------------------------------
  version      Pillow 9.0.0
  platform     linux 3.9.9 | packaged by conda-forge | (main, Dec 20 2021, 02:40:17)
               [GCC 9.4.0]
  --------------------------------------------------------------------
  --- JPEG support available
  --- OPENJPEG (JPEG2000) support available (2.4)
  --- ZLIB (PNG/ZIP) support available
  *** LIBIMAGEQUANT support not available
  --- LIBTIFF support available
  --- FREETYPE2 support available
  *** RAQM (Text shaping) support not available
  --- LITTLECMS2 support available
  --- WEBP support available
  *** WEBPMUX support not available
  *** XCB (X protocol) support not available

should we not add the respective libraries to get support complete? Any reasons this wasn't done in the past (except perhaps unavailability of the libs in conda-forge)?

I'm also thinking to try running the upstream test suite here, though this might obviously uncover some sleeping bugs and could be decoupled from the bump here.

@h-vetinari h-vetinari mentioned this pull request Jan 4, 2022
5 tasks
Copy link
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

This should now be ready in principle, however, I want to make sure we pick up any necessary changes for supporting pillow-simd (see this comment ff.) into the first builds for 9.0.0, as pillow apparently has a 18 month cadence for major releases, and having such a change at a major version boundary makes a bunch of things easier on the infra & resolver level.

@ocefpaf
Copy link
Member

ocefpaf commented Jan 4, 2022

should we not add the respective libraries to get support complete? Any reasons this wasn't done in the past (except perhaps unavailability of the libs in conda-forge)?

Yeah. Probably. To be honest I don't remeber.

I'm also thinking to try running the upstream test suite here, though this might obviously uncover some sleeping bugs and could be decoupled from the bump here.

Sure. Let's go for it. I'm on a "break" from conda-forge to focus on the day job a bit so I won't work on this anytime soon but I will be happy to review/merge anything if you need me.

@h-vetinari
Copy link
Member

h-vetinari commented Jan 4, 2022

@radarhere @hugovk
Switching on the test suite here runs into some errors (in all three cases the test image is missing; I can also not find them upstream):

FAILED Tests/test_tiff_crashes.py::test_tiff_crashes[Tests/images/crash_1.tif]
FAILED Tests/test_tiff_crashes.py::test_tiff_crashes[Tests/images/crash_2.tif]
FAILED Tests/test_tiff_crashes.py::test_tiff_crashes[Tests/images/crash-81154a65438ba5aaeca73fd502fa4850fbde60f8.tif]

There's one other failure that is also a bit cryptic for me:

__________________________________ test_show ___________________________________

    @pytest.mark.skipif(
        not on_ci() or is_win32(),
        reason="Only run on CIs; hangs on Windows CIs",
    )
    def test_show():
        for mode in ("1", "I;16", "LA", "RGB", "RGBA"):
            im = hopper(mode)
>           assert ImageShow.show(im)
E           assert 0
E            +  where 0 = <function show at 0x7f5ac5a32550>(<PIL.Image.Image image mode=1 size=128x128 at 0x7F5AC443DAC0>)
E            +    where <function show at 0x7f5ac5a32550> = ImageShow.show

Tests/test_imageshow.py:51: AssertionError

Note, the test suite wasn't run here until now, so it's not strictly necessary and we can also skip however many tests we want (in principle). Still, I consider it best practice to run the full test suite to verify that the packaging has gone smoothly.

@radarhere
Copy link
Contributor

radarhere commented Jan 4, 2022

The images you mention are located at https://github.com/python-pillow/pillow-depends/tree/main/test_images

We removed them from our main repository because antivirus software on users' computers was flagging them as problematic - which of course is exactly why we were testing them, but the antivirus software did not realise that the installed version of Pillow had fixes for them.

In our CI, we copy these images in.
https://github.com/python-pillow/Pillow/blob/3f77466f11e04ddefdbbe79de6511ff8fb0b087b/.ci/install.sh#L54-L55
https://github.com/python-pillow/Pillow/blob/main/depends/install_extra_test_images.sh

on_ci is intended to avoid running these checks on users' local machines where the images have not been copied.

@radarhere
Copy link
Contributor

The 0 value being returned by ImageShow.show probably means that your environment does not have any image viewers available. If I'm reading your logs correctly, this is just occurring on Linux.

The image viewers we look for on Linux are xdg-open, display, gm, eog and xv.

@h-vetinari
Copy link
Member

Thanks a lot for the quick responses. I'll try to see if I can get the extra images to run here, but if it doesn't work, we'll just skip them.

For the viewers, I think the best is just to skip that one test, rather than try to shoehorn a viewer into the CI (might need yum_requirements, which I'd prefer to avoid as they could affect build time).

@h-vetinari h-vetinari force-pushed the 9.0.0_hc4b656 branch 2 times, most recently from 73a889b to 50f650b Compare January 5, 2022 00:02
@radarhere
Copy link
Contributor

The load() method that I'm patching is where the tag_v2 data is read from the file.
The y value is the number of tags in the frame.
When it initially loads the frame, there are 16 (b'\x10\x00') tags in the frame.
When it seeks forward and then back, there are 0 (b'\x00\x00') tags in the frame - this is the part that doesn't make sense to me.

Since tag_v2 is then empty, IMAGEWIDTH is not in it, leading to the error.

@h-vetinari
Copy link
Member

When it seeks forward and then back, there are 0 (b'\x00\x00') tags in the frame - this is the part that doesn't make sense to me.

I think it would be fine for now to skip this test, WDYT?

@h-vetinari
Copy link
Member

@radarhere, thanks a lot for the patch, the test is now passing! :)

While I think this would be good enough to release, I'm still trying to get raqm packaged for conda-forge, and was wondering if we you see a chance that we figure out the failing library detection on windows:

  --------------------------------------------------------------------
  --- JPEG support available
  --- OPENJPEG (JPEG2000) support available (2.4)
  --- ZLIB (PNG/ZIP) support available
  *** LIBIMAGEQUANT support not available
  --- LIBTIFF support available
  --- FREETYPE2 support available
  *** RAQM (Text shaping) support not available
  --- LITTLECMS2 support available
  --- WEBP support available
  *** WEBPMUX support not available
  *** XCB (X protocol) support not available
  --------------------------------------------------------------------

LIBIMAGEQUANT / WEBPMUX / XCB should all be there, but it's not being picked up.

@radarhere
Copy link
Contributor

If I run searches for imagequant.dll, when Pillow is about to be installed, it is absent. It appears in another search later on, but by then it is too late. Here is the full log.

This suggests to me that the order of operations needs to be rearranged so that libimagequant is installed before Pillow, rather than after.

@h-vetinari
Copy link
Member

h-vetinari commented Jan 14, 2022

@radarhere
So I checked in several ways with the last commit, and imagequant.dll definitely is around at bld.bat-time, in %LIBRARY_BIN% == %LIBRARY_PREFIX%\bin == %PREFIX%\Library\bin, where basically all the conda-forge DLLs get put (in contrast, %LIBRARY_LIB% == %PREFIX%\Library\lib contains static libs, for the most part)

"Search for imagequant.dll, part 5"
 Volume in drive D is Temporary Storage
 Volume Serial Number is 323C-989D

 Directory of %PREFIX%\Library\bin

01/14/2022  04:53 AM    <DIR>          .
01/14/2022  04:53 AM    <DIR>          ..
05/07/2021  06:31 PM            20,960 api-ms-win-core-console-l1-1-0.dll
[...]
01/07/2022  11:30 AM            10,240 imagequant.dll

@h-vetinari
Copy link
Member

If you look further up in bld.bat, you see that all other libraries use the same location (well, WEBP_ROOT & XCB_ROOT are my additions and not working yet either...)

set JPEG_ROOT=%LIBRARY_PREFIX%
set JPEG2K_ROOT=%LIBRARY_PREFIX%
set ZLIB_ROOT=%LIBRARY_PREFIX%
set IMAGEQUANT_ROOT=%LIBRARY_PREFIX%
set TIFF_ROOT=%LIBRARY_PREFIX%
set FREETYPE_ROOT=%LIBRARY_PREFIX%
set FRIBIDI_ROOT=%LIBRARY_PREFIX%
set LCMS_ROOT=%LIBRARY_PREFIX%
set WEBP_ROOT=%LIBRARY_PREFIX%
set XCB_ROOT=%LIBRARY_PREFIX%

@radarhere
Copy link
Contributor

I don't really think this is a problem with Pillow detecting files. I think this is a problem with files not being there in the first place.

  • imagequant.lib and demux.h not present anywhere, so Pillow can't find them. I don't think I'm able to help you with that, as you are more familiar with feedstock installation than I am.
  • I don't see anything that would be installing raqm at all.
  • I don't think that xcb even works on Windows.

@saraedum saraedum mentioned this pull request Jan 18, 2022
3 tasks
@h-vetinari
Copy link
Member

h-vetinari commented Jan 24, 2022

OK, I propose to merge this as is @conda-forge/pillow. The other bindings can be improved incrementally.

PS. If desired, I can clean up the commit-history.

@ocefpaf
Copy link
Member

ocefpaf commented Jan 24, 2022

OK, I propose to merge this as is @conda-forge/pillow. The other bindings can be improved incrementally.

Fine with me.

PS. If desired, I can clean up the commit-history.

That is fine. No need to clean it. Can you lift the "change request" so we cam merge?

PS: thanks for the awesome work you did here!

@bollwyvl
Copy link
Contributor

bollwyvl commented Feb 2, 2022

Hooray pillow 9.0.0! cleans up a number of CVEs, would be lovely to see this merged.

@bollwyvl
Copy link
Contributor

bollwyvl commented Feb 2, 2022

@conda-forge-admin please rerender

@bollwyvl
Copy link
Contributor

bollwyvl commented Feb 2, 2022

@conda-forge/pillow back up to ✔️

@regro-cf-autotick-bot regro-cf-autotick-bot mentioned this pull request Feb 3, 2022
3 tasks
@ocefpaf ocefpaf merged commit 3aecc20 into conda-forge:master Feb 3, 2022
@ocefpaf
Copy link
Member

ocefpaf commented Feb 3, 2022

I guess that the "changes requested" threw me off. I usually do these reviews on mobile and I skips those that have pending things. However, it was clear that @h-vetinari was done and I missed it. Thanks again for this awesome PR @h-vetinari !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants