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

Add LittleCMS2 support. #80

Merged
merged 6 commits into from
Jul 8, 2020
Merged

Conversation

pkgw
Copy link
Contributor

@pkgw pkgw commented Jul 3, 2020

Checklist

  • Used a fork of the feedstock to propose changes
  • Bumped the build number (the version is unchanged)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

Closes #58. AFAICT, this hasn't happened simply just because no one has gotten around to it?

@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.

I do have some suggestions for making it better though...

For recipe:

  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

pillow uses the custom "PIL" license.
@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.

@pkgw
Copy link
Contributor Author

pkgw commented Jul 3, 2020

@conda-forge/core Many errors about gcc not being found on platforms like stock Linux. Looking at the builds that did succeed, I think setuptools 48 is breaking this? The macOS build is succeeding with setuptools 48, and the Windows errors are different, so I think it might only be Linux. Linux/ppc64le succeeded and those builds got setuptools 47.

@ocefpaf
Copy link
Member

ocefpaf commented Jul 3, 2020

Yep. Older setuptools fixes it on Linux. The problem on Windows is b/c we are using an older libtiff there due to #69. Not sure if we solved that or not.

@pkgw
Copy link
Contributor Author

pkgw commented Jul 4, 2020

The changelog for setuptools 49.1.0 (cf. conda-forge/setuptools-feedstock#153) says that it reverts the distutils bundling that looks like the likely cause of the Linux issues, so we might have to wait a bit longer for that to shake out. No rush here, though.

@patricksnape
Copy link
Contributor

@conda-forge-admin please rerender

@patricksnape
Copy link
Contributor

Looks like we have package conflicts for libtiff 4.0 on Windows?

On the libtiff 4.0 variant, I get package compatibility errors:

```
Package jpeg conflicts for:
lcms2 -> jpeg[version='>=9b,<10a|>=9c,<10a|>=9d,<10a']
jpeg=9
libtiff=4.0 -> jpeg[version='9.*|>=9b,<10a|>=9c,<10a']

Package libtiff conflicts for:
libtiff=4.0
lcms2 -> libtiff[version='>=4.1.0,<5.0a0']
```

On libtiff 4.1, I get a separate issue during test (that is not obviously
related to LCMS?):

```
(%PREFIX%) %SRC_DIR%>"%PREFIX%\python.exe" -s "%SRC_DIR%\run_test.py"
import: 'PIL'
import: 'PIL.Image'
D:\bld\pillow_1593795315516\_test_env\lib\site-packages\PIL\Image.py:115: RuntimeWarning: The _imaging extension was built for another version of Pillow or PIL:
Core version: "7.2.0"
Pillow version: 7.2.0
  warnings.warn(str(v), RuntimeWarning)
Traceback (most recent call last):
  File "D:\bld\pillow_1593795315516\test_tmp\run_test.py", line 5, in <module>
    import PIL.Image
  File "D:\bld\pillow_1593795315516\_test_env\lib\site-packages\PIL\Image.py", line 100, in <module>
    "Pillow version: %s" % (getattr(core, "PILLOW_VERSION", None), __version__)
ImportError: The _imaging extension was built for another version of Pillow or PIL:
Core version: "7.2.0"
Pillow version: 7.2.0
```
@pkgw
Copy link
Contributor Author

pkgw commented Jul 6, 2020

Yeah, and I ran into other issues with the libtiff 4.1 branch of the Windows builds. (See commit message.) Given that this feature seemingly isn't urgent, I just didn't enable LCMS for Windows, maintaining the status quo.

With that taken care of, shall we go ahead and merge?

@patricksnape patricksnape merged commit 81c957c into conda-forge:master Jul 8, 2020
@kmilos
Copy link
Contributor

kmilos commented Jul 8, 2020

#69 was addressed by conda-forge/libtiff-feedstock#51 AFAIK

@pkgw pkgw deleted the add-littlecms branch July 9, 2020 16:30
@kmilos
Copy link
Contributor

kmilos commented Feb 1, 2021

Can we try to bring back lcms2 support in Pillow please?

@pkgw
Copy link
Contributor Author

pkgw commented Feb 1, 2021

@kmilos Sure. Would you feel comfortable preparing a pull request yourself?

@kmilos
Copy link
Contributor

kmilos commented Feb 1, 2021

Sure, will try in the coming days.

@kmilos
Copy link
Contributor

kmilos commented Feb 1, 2021

Ok, was quicker than I thought: #89

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.

Require the LittleCMS library?
5 participants