search

conda-forge / pillow-feedstock

A conda-smithy repository for pillow.
BSD 3-Clause "New" or "Revised" License
2 stars 30 forks source link

Bindings improvements #109

Open h-vetinari opened 2 years ago

h-vetinari commented 2 years ago

A lot of updates in #104, but some work outstanding:

h-vetinari commented 2 years ago

@conda-forge/pillow @conda-forge/core

104 added some more support for the bindings that pillow offers, among them libimagequant. Apparently pillow itself does not distribute that library due to a tighter license:

Libimagequant is licensed GPLv3, which is more restrictive than the Pillow license, therefore we will not be distributing binaries with libimagequant support enabled.

... which lead to https://github.com/python-pillow/Pillow/issues/6019 being raised upstream.

I see at least three options and wanted to ask for opinions:

  1. simply drop libimagequant support
  2. keep libimagequant as dependency and accept tighter license(-in-aggregate)
  3. turn into multi-output recipe
    • single output per binding: likely too painful considering the number of options
    • pillow (default) & pillow-no-<x> (where demand exists)
    • probably needs a meta-package (otherwise pillow-no-<x> can never replace pillow as a transitive dependency). This is further complicated by already occupying (most likely) a part of the build string with #103.

CC @radarhere

nulano commented 2 years ago

I did not see the Raqm discussion in #104 until now, and can't tell if it has been resolved. For reference, Pillow wheels are built with this pip command:

    pip wheel $(pip_opts) \
        --global-option build_ext --global-option --enable-raqm \
        --global-option --vendor-raqm --global-option --vendor-fribidi \
        -w $abs_wheelhouse --no-deps .

https://github.com/python-pillow/pillow-wheels/blob/a57b83c1b536d53d7c03d97200e81a9fd4bd123a/config.sh#L144-L147

nulano commented 2 years ago

use conda-forge packaged raqm, blocked on https://github.com/conda-forge/staged-recipes/pull/17457

I don't know if this is a good idea. The reason the vendored version of Raqm is used is due to FriBiDi's license, similar to the libimagequant issue. The patch allows Raqm to be enabled optionally when users don't mind the LGPL license requirements (by loading FriBiDi at runtime if it is found). Ideally it would be an optional dependency; I'm not sufficiently familiar with Conda to know what is the best way to achieve that.

ocefpaf commented 2 years ago

Sadly conda doesn't have an implementation for optional dependencies. @h-vetinari at this point you are the pillow conda pkg expert and I'll defer to what you think is the best course of action here.

h-vetinari commented 2 years ago

I haven't reviewed all the licenses, but #104 did include fribidi as well.

It seems to me that a 4th solution might be:

  1. pillow_<ver>_no_gpl (only core bindings, license-compatible), pillow_<ver>_all (also includes (L)GPL bindings)

That might also be easiest to implement. I'd appreciate if someone could tell me which bindings are compatible with the (non-standard) pillow license - for example, https://pillow.readthedocs.io/en/stable/installation.html#external-libraries doesn't mention the fribidi license implication of raqm.

I don't know if this is a good idea. The reason the vendored version of Raqm is used is due to FriBiDi's license, similar to the libimagequant issue.

I don't understand this - how is it different (functionally as well as from a license perspective) if raqm is vendored or comes in as a dependency? I had tried the installation invocation with -vendor-raqm in #104 without success, hence starting to package it directly.

nulano commented 2 years ago

AFAIK all dependencies have a license similar to Pillow (i.e. non-copyleft, most are BSD-style) except for libimagequant (GPL) and fribidi (LGPL). All dependencies except for these two are also included in wheels distributed by Pillow.

I don't understand this - how is it different (functionally as well as from a license perspective) if raqm is vendored or comes in as a dependency?

Raqm itself is MIT licensed. The patch in the vendored version is only to allow loading fribidi at runtime (without creating a hard dependency for the system loader/linker). This way Pillow doesn't actually distribute LGPL licensed fribidi, meaning it is not "infected" by its LGPL license. Quoting from the (L)GPL FAQ:

Does the LGPL have different requirements for statically vs dynamically linked modules with a covered work? (#LGPLStaticVsDynamic) For the purpose of complying with the LGPL (any extant version: v2, v2.1 or v3):

(1) If you statically link against an LGPLed library, you must also provide your application in an object (not necessarily source) format, so that a user has the opportunity to modify the library and relink the application.

(2) If you dynamically link against an LGPLed library already present on the user's computer, you need not convey the library's source. On the other hand, if you yourself convey the executable LGPLed library along with your application, whether linked with statically or dynamically, you must also convey the library's sources, in one of the ways for which the LGPL provides.

IIUC (IANAL) including the LGPLed fribidi library in Pillow wheels would count as static linking (even if it is done via an SO object). Either way, if Pillow wheel distributed fribidi, Pillow and anyone who further redistributes the wheels would have to abide by LGPL restrictions (by providing fribidi sources). By being loaded at runtime, the LGPL license no longer applies, as it is clearly being loaded from the user's computer. If someone develops an application that depends on Pillow with Raqm, they are also free to redistribute them both including fribidi, if they choose to abide by LGPL.

Now, IIUC conda deals with dependencies using dynamic linking by supplying each shared library object separatly. To be honest, I have no idea how this kind of dependency interacts with the LGPL, so I'm afraid I can't help with that, especially as I'm not sufficiently familiar with how conda is typically used.

However, I expect the Raqm patch should be able to pick up fribidi if it is installed among the other shared library objects even if it is not explicitly listed in the dependencies, which I would expect is good enough to avoid falling under the terms of LGPL.

h-vetinari commented 2 years ago

Yeah OK, I see how including it in the wheel or not makes a difference for pillow, but conda does not understand or accept the concept of a user-supplied library. So anyone wanting to use raqm through conda-forge will require fribidi from conda-forge, and therefore incur that license being present within their environment.

AFAIK all dependencies have a license similar to Pillow (i.e. non-copyleft, most are BSD-style) except for libimagequant (GPL) and fribidi (LGPL)

Thanks. That means a possible split could be:

h-vetinari commented 2 years ago

Could someone from @conda-forge/core chime in on the proposed split into pillow_<version>_no_gpl and pillow_<version>_all (based on the license of the libraries for which we include bindings), and making that split by means of a build-string?

I could implement that, but haven't heard back if this is considered a good idea in principle (alternative would be dropping a bunch of bindings or accepting the GPL-in-aggregate, both of which I consider inferior), or how it should be done technically (I think build string is the best/easiest, but there are others).

hmaarrfk commented 2 years ago

I think we should consolidate with: https://github.com/conda-forge/python-feedstock/issues/387

h-vetinari commented 2 years ago

Fair point, but then, that issue hasn't seen any activity in 1.5yrs, and the issue for pillow seems more urgent to several people. If there's ever something like a conda-wide metapackage to avoid the GPL, this could be easily incorporated with my proposal, but I think it's not necessary to couple them and IMO it would be unreasonable to require a conda-forge-wide solution just for this particular issue.

hmaarrfk commented 2 years ago

Honestly, I would just remove libimagequant and tell Pillow that without a better build system, they shouldn't add depeendecies like that.

They could make it a plugin that is used to compile a .so that is brought it upon request.

hmaarrfk commented 2 years ago

The truth is that dependencies have consequences, one consequence might be that a package isn't released as widely as it would otherwise be.

Other consequences could be that downstream patches out your system and you get inconsistent performance.

hmaarrfk commented 2 years ago

This would mostly get you time to discuss with conda-forge wide about how to go about gpl/non-gpl.

nulano commented 2 years ago

Honestly, I would just remove libimagequant and tell Pillow that without a better build system, they shouldn't add depeendecies like that.

They could make it a plugin that is used to compile a .so that is brought it upon request.

Pillow doesn't distribute binary wheels containing imagequant or Raqm. imagequant is only available to users who choose to build from source. I don't recall seeing any issues about missing imagequant support raised in the tracker in the past few years. If it is a big issue, I would expect it to be fine to drop imagequant at least until someone requests it.

OTOH Raqm is requested about once a month. There is a mechanism for loading it at runtime as I explained above, so usually those tickets get resolved by either installing FriBiDi (sometimes it is required to install Pillow from source, e.g. on Raspberry Pi where Pillow doesn't distribute official wheels).

hmaarrfk commented 2 years ago

Hmm ok. I think I understand.

@h-vetinari, are you suggesting we build ONCE, but then have two fiendly names for users to install the extra packages as needed?

h-vetinari commented 2 years ago

@h-vetinari, are you suggesting we build ONCE, but then have two fiendly names for users to install the extra packages as needed?

I think it'd be best to build two variants in the same feedstock (add no_gpl / all to cbc.yaml and then insert it in the buildstring resp. add an if-condition for the GPL bindings in the build scripts). That's because I don't want to do surgery on the bindings code that pillow generates. It would mean double the CI jobs, but I think that's OK.

hmaarrfk commented 2 years ago

Double the CI jobs here is ok. I'm just worried about blowing up the build matrices elsewhere.

Honestly, i would apologize to the one use and state that this was included erroneously. They can build Linux packages easily themselves either locally or on the cloud.

Then focus on getting a cohesive gpl plan

h-vetinari commented 2 years ago

I'm just worried about blowing up the build matrices elsewhere.

The point about the build strings is that there's no need to blow up any other matrices.

Every other recipe can keep using pillow, but if a user wants the no-gpl version (which I would weigh down; default being everything), then they will be able to specify pillow =*=*_no_gpl in their environment, and everything else will keep working.

hmaarrfk commented 2 years ago

I understand. However I really do think that some cencensus about how to move forward should be documented and discussed before having feedstocks with different naming conventions.

We already kinda broke torchvision build strings because we wanted them to be consistent with pytorch. They had CPU/GPU and python flipped.

hmaarrfk commented 2 years ago

thatbissue wqs stalr not due to lack of interest, but due to lack of time from any of the interested paryies to move things forward.

dopplershift commented 2 years ago

I would make no_gpl the default and have people opt into using GPL packages, but that's based on my personal opinions regarding the GPL and libraries.

h-vetinari commented 2 years ago

@dopplershift: I would make no_gpl the default and have people opt into using GPL packages, but that's based on my personal opinions regarding the GPL and libraries.

I think that would effectively be equivalent to not publishing the GPL-bindings at all. Not everyone cares about the GPL. I think those that do should opt into not having those bindings.

@hmaarrfk: that issue was stalled not due to lack of interest, but due to lack of time from any of the interested parties to move things forward.

I sympathize, really, it happens to a lot of threads/topics. Still, if it doesn't bubble up on peoples priority lists over the course of 1.5 years, that's still an indicator that the urgency/priority weren't that pressing.

jakirkham commented 2 years ago

As to GPL/non-GPL, would suggest looking at issue ( https://github.com/conda-forge/conda-forge.github.io/issues/209 ), which discusses this. Ideally users would be able to affect this with one command. The common use case is likely the user doesn't want GPL anywhere so one package that does this that affects how pillow, python, etc. install dependencies would be ideal

A subquestion here is how users handle other GPL variants in this decision (LGPL2, LGPL3, AGPL, etc.). Some users are ok with GPL and not AGPL. Some users are ok with LGPL, but not GPL and especially not AGPL. Some care about the version of LGPL used. Though this is a bit into the weeds just want to mention there is some attention to detail needed in the design

hmaarrfk commented 2 years ago

that's still an indicator that the urgency/priority weren't that pressing.

This is not true.

The community is always growing and evolving. Not every project is taken on immediately.

I am asking that you make an effort to reach out to the wider conda-forge community.

Many people there have been involved in larger migrations that could carve a way forward to getting your goal of a gpl-free distribution within conda-forge.

I really wish the problem ended with libimagequant. It doesn't, there are quite a few other packages like this. By leveraging expertise you will get a much more cohesive solution.

jakirkham commented 2 years ago

Yeah agree with Mark. We lacked the tooling to take on this problem in a scalable way in the past.

That said, after thinking about this, I think this may be more doable now. Wrote up a proposal here ( https://github.com/conda-forge/conda-forge.github.io/issues/1608 ). Feedback welcome 🙂

h-vetinari commented 2 years ago

that's still an indicator that the urgency/priority weren't that pressing.

This is not true.

Indicator: something that provides an indication [i.e. it's not a statement claiming absolute truth]

I am asking that you make an effort to reach out to the wider conda-forge community. [...]

I'd be happy to try advocating for an as-simple-as-possible no_gpl meta-package, but with no v2-vs-v3 "bells" or "AGPL/LGPL" whistles - those are exactly the kind of things that bog down proposals and implementation for years, because there are inevitably people that are extremely passionate about licensing, and any solution doesn't correctly reflect their pet peeve. Without hard constraints it's just a black hole to pour energy into.

I'll be happy to contribute in this space, but I continue to find that it's not a reasonable ask to solve a conda-forge wide problem (John opened https://github.com/conda-forge/conda-forge.github.io/issues/209 over 5 years ago) for situation like this one. In the case at hand, I'll just remove the GPL-bindings, pending outcome of these various discussions.

jakirkham commented 2 years ago

I don't think it is as hard to solve as you are suggesting. Certainly it was 5 years ago. Two reasons: We did not use SPDX (license metadata was the Wild West) and we lacked the ability to do repodata hot-fixing.

Anyways I think it is worth reading over the proposal ( https://github.com/conda-forge/conda-forge.github.io/issues/1608 ). Think it has the needed functionality without being overcomplicated or burdensome. Though if you spot anything that should be considered, that would be great feedback to have 🙂

h-vetinari commented 2 years ago

I don't think it is as hard to solve as you are suggesting.

All the better!

isuruf commented 2 years ago

Is there any potential user of conda-forge/pillow who can't use conda-forge because of this? If GPL is an issue, what about readline which is a dependency of python?

xhochy commented 2 years ago

Is there any potential user of conda-forge/pillow who can't use conda-forge because of this? If GPL is an issue, what about readline which is a dependency of python?

In business terms, it is much easier to argue that GPL is OK if the interpreter binds to it than when a library does. If pillow would link to a GPL library, it would be a hard to for me to use it a commercial setting, readline in Python is minor thing, corporate policies see in some kind of grey area that they accept. If not, I would have already spent the time to build a version to get rid of it.