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

Re-enable LCMS2 for Windows #89

Merged
merged 3 commits into from
Feb 8, 2021
Merged

Re-enable LCMS2 for Windows #89

merged 3 commits into from
Feb 8, 2021

Conversation

kmilos
Copy link
Contributor

@kmilos kmilos commented Feb 1, 2021

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • 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.

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

@kmilos kmilos mentioned this pull request Feb 1, 2021
4 tasks
@kmilos
Copy link
Contributor Author

kmilos commented Feb 1, 2021

Any particular reason one is still checking libtiff 4.0.x on Windows, can we drop it?

AFAIK #69 is resolved and both libtiff 4.1 and 4.2 are fine on Windows...

@pkgw
Copy link
Contributor

pkgw commented Feb 1, 2021

Do you know if conda-forge/libtiff-feedstock#62 has been solved?

@kmilos
Copy link
Contributor Author

kmilos commented Feb 2, 2021

No idea to be honest... It doesn't mention a specific version problem...

#69 was fixed by making libtiff 4.1.0 and 4.2.0 behave just like 4.0.10 (as recommended by libtiff maintainers here), so I don't think there is a libtiff feedstock issue blocking this one.

poppler should probably use libtiff w/ Unix I/O just like it did for 4.0.10, no? A case statement like this is maybe needed in poppler code? I.e. patch this line the same way and add && defined(USE_WIN32_FILEIO): https://gitlab.freedesktop.org/poppler/poppler/-/blob/master/goo/TiffWriter.cc#L165

@pkgw
Copy link
Contributor

pkgw commented Feb 2, 2021

OK, I'm inclined to agree that we should drop the libtiff 4.0 here, but I'm not super tuned into the ecosystem implications. @conda-forge/core anyone have any opinions about this?

@kmilos
Copy link
Contributor Author

kmilos commented Feb 2, 2021

Yes, definitely needs to be coordinated across the whole of conda-forge: 1) stick with Unix I/O from libtiff 4.0 days and keep patching clients to match that, or 2) make a repo-wide switch to libtiff Win32 I/O

LCMS2 support is fundamentally not related to this decision (apart from misc version conflicts).

@ocefpaf
Copy link
Member

ocefpaf commented Feb 3, 2021

OK, I'm inclined to agree that we should drop the libtiff 4.0 here, but I'm not super tuned into the ecosystem implications. @conda-forge/core anyone have any opinions about this?

Let's give it a try. As far as I recall we have tests in place to catch if problems occur. Hopefully we'll notice before merging.

@pkgw
Copy link
Contributor

pkgw commented Feb 3, 2021

@ocefpaf You mind looking this over as a final check?

@ocefpaf
Copy link
Member

ocefpaf commented Feb 8, 2021

@ocefpaf You mind looking this over as a final check?

Thanks so much for looking into this. Everything seems fine IMO, merge?

@pkgw pkgw merged commit e01f509 into conda-forge:master Feb 8, 2021
@kmilos kmilos deleted the lcms2_win branch March 12, 2021 10:27
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

4 participants