[Java] EPL Dependencies

[martin-traverse]

Describe the bug, including details regarding any error messages, version, and platform.

Hi,

Please could I ask when and why the Eclipse Collections dependencies were introduced? This puts EPL dependencies into the dependency tree. Our clients are in the financial sector and these kind of licensing issues often cause a lot more pain that you might think they should!

We are a FINOS project and use their license classification scheme which is available here:

https://community.finos.org/docs/governance/software-projects/license-categories/

I see Apache has a similar policy:

https://apache.org/legal/resolved.html

The EPL is category B for both FINOS and Apache. We picked this up because it flagged with our license checks in CI. Although we can add an exception and start reproducing the license in NOTICE and LICENSE files, including it in our distribution packages etc., this doesn't help when clients have their own license scanning and acceptance process for getting software into the enterprise. For this reason, we generally try to stick to "Category A" licenses and count anything that pulls in category B dependencies as being category B. (We do use category B for testing, compliance checks and other non-shipped components).

Please can you share some info on the reasoning around this decision? Is there any appetite to reverse if the touch points are small and/or there are alternatives available, perhaps something from Apache Commons? More generally, do you have a view on your policy towards category B licenses going forward?

I'll be very interested to hear your thoughts on this subject!

Component(s)

Java

[lidavidm]

09d6ca7

Sorry about this. We were trying to eliminate a dependency on Netty from the core. Looking around I think our only choice is to use stdlib map and accept any potential performance hit, or to vendor the Netty implementation of this collection.

[lidavidm]

We also need to set up similar CI to prevent issues like this in the future.

[martin-traverse]

Thanks for the quick feedback - it is actually good from our point of view to hear that this is not intentional! Is there any chance the fix will be in version 16 or is it too late for that now?

For license scans we use Gradle-License-Report for Java, pip-licenses for Python and license-checker-rseidelsohn for JavaScript - appreciate you have a lot more languages to worry about!

[lidavidm]

I'm going to try to get this in ASAP. Actually, I just labeled this as a critical fix. I put together a basic Java-only license scan already. My schedule is about to go pear-shaped but hopefully I can get this done tonight/tomorrow.

[jbonofre]

Thanks for the report. We have several Maven plugins that can check the license (rat doesn't check the license by its own). I will take a look to detect this (we should accept Cat A licenses by default, Cat B should be discussed, Cat X is not allowed).

[assignUser]

I explicitly added the blocker label because I'm not sure if our scripts treat critical fix as a blocker. I think that label is more for call outs in the release notes cc @raulcd

[jbonofre]

I explicitly added the blocker label because I'm not sure if our scripts treat critical fix as a blocker. I think that label is more for call outs in the release notes cc @raulcd

Thanks @assignUser ! I agree. I will take a look on this one as well.

[kou]

Issue resolved by pull request 40904
#40904

[martin-traverse]

Thank you so much for the quick response - I look forward to getting the fix in 16.0!

[alexanderankin]

You can remove the generated annotation now

[donraab]

@martin-traverse @lidavidm FYI, Eclipse Collections is dual licensed under EPL 1.0 and EDL 1.0 (Eclipse Distribution License), which is listed under the link of Category A licenses you submitted in this issue. One of the biggest users of Eclipse Collections is the Legend project which is also a FINOS project.

Note: I am the creator of Eclipse Collections. Thanks!

[lidavidm]

Aha, good to know. Thanks for chiming in - I saw the report and just wanted to make sure we avoided any potential problems even if it turned out to be a false alarm. (In the end, we only need 1 class so vendoring was probably preferable in any case.)

[donraab]

@lidavidm You're welcome. I was curious to understand why Apache Arrow had a dependency on Eclipse Collections which is how I found this issue, albeit a bit too late unfortunately to save the effort. Using one of the primitive collections like IntObjectHashMap is a fairly common reason some folks first introduce Eclipse Collections as a dependency.

I certainly appreciate every project's desire and challenge to reduce runtime dependencies. I found the PR that introduced the Eclipse Collections dependency six months ago and it simultaneously removed Netty Common as a dependency. There was a specific difference in the values() implementation in MapWithOrdinalImpl when the IntObjectHashMap from Netty was previously used. I don't know why the different implementation using StreamSupport with Apache Arrow Preconditions was needed to build the values collection. It might be worth contacting the developer who originally swapped the Netty IntObjectMap out for the Eclipse Collections IntObjectMap now that the code base includes a copy of the Netty IntObjectMap to see if the original values() code needs to be reverted back.

@martin-traverse I would appreciate if you would let me know if there is an issue with Gradle License Report picking up dual licenses, or if there is something we can address in Eclipse Collections to make sure this doesn't happen to others in the future thinking they have to use the EPL license when the EDL license is there to provide flexibility to consumers of the library. Thanks!

[jbonofre]

@donraab Hi. The EPL license is Cat B at ASF, meaning that it could be possible as binary, but avoided in source (https://www.apache.org/legal/resolved.html). As EDL is a kind of BSD-3 license, it's a Cat A, so no problem to be included/used in Apache project.
Generally speaking, I would avoid EPL licensed deps, and double check for EDL (but less problematic).

[donraab]

@jbonofre Thanks! Eclipse Collections is dual-licensed with EDL for this reason so folks shouldn't have issues including it as a dependency. I am a bit concerned now if automated scanners are picking up EPL and ignoring EDL as the more permissive option. Hoping if there is something we can do to address it in Eclipse Collections we'll create an issue and fix it.

[jbonofre]

@donraab yeah, agree some scanners are not smart enough to deal with dual licenses 😄

[martin-traverse]

@donraab apologies you are correct, EDL is available as an option and classed as category A by FINOS, we have just not configured it in our license scanning setup. I had not come across EDL before and was not aware it had different terms from EPL, until now.

Still I agree with @lidavidm - removing the dependency is a good idea anyway to keep the tree small. Managing version updates, conflicts and security patches in the dependency tree is a big issue for us, for this reason keeping the tree as lean as possible is one of our guiding design principles. Obviously there is a balance to be struck, we try to stick to self-contained package families where possible, so bringing in a foundational lib like this from a different family would go against the grain for us in this particular project. For a project like Arrow, where you don't know what stack the client apps will be using, a small, un-opinionated dependency tree is even more important IMO. It's one of the reasons we went with Arrow in the first place.

[donraab]

@martin-traverse @lidavidm Here's a link to the change that I explained in a previous comment may have been lost with this Netty / Eclipse Collections dependency shuffle / removals.

There was a values implementation in MapWithOrdinalImpl as follows before Netty IntObjectHashMap dependency was removed.

public Collection<V> values() {
      return StreamSupport.stream(secondary.entries().spliterator(), false)
          .map((IntObjectMap.PrimitiveEntry<V> t) -> Preconditions.checkNotNull(t).value())
          .collect(Collectors.toList());
    }

This was replaced with current implementation of values() when Eclipse Collections IntObjectHashMap was used.

Now that Netty IntObjectHashMap has returned as a copy in the same package, I suggest again making sure this code which was changed works as is currently implemented. I do not have any understanding of why the specialized values code was necessary before (or if it is not), but am simply noting a diff that occurred with these changes. If it works as is, then great!

Thanks!

[lidavidm]

Ah sorry, thanks for the reminder

@vibhatha do you think you could put in the fix sometime?

[vibhatha]

@lidavidm I followed the thread. I think I should be able to, but probably next week?

[lidavidm]

sure, there's no rush

[vibhatha]

Thanks!