Remove Commons Lang dependency

[ppkarwasz]

Commons Compress version 1.26.0 depends on Commons Lang, which is a 650 KiB dependency. In reality, however, only 4 methods accounting for less than 1% of Commons Lang are used.

We replace the dependency on commons-lang3 with a local copy of these 4 methods.

[garydgregory]

As I've pointed out on the dev mailing list: This a horrible mess IMO. To make things even worse, this PR adds public code (the package name is irrelevant), with ZERO Javadoc and ZERO unit tests. This is not the example we should set for contributors and even less for ourselves. This seems to me like a reflection of short term hacking without long term care. I am -1, but it seems pointless as some people are in love with copy-paste over long term maintenance.

[ppkarwasz]

To make things even worse, this PR adds public code (the package name is irrelevant), with ZERO Javadoc and ZERO unit tests.

Sure, there is work to be done on this PR (I marked it as draft). Don't worry, if you are willing to accept this solution, I will make the package really private (in OSGi and JPMS) and add the relevant Javadoc and unit tests.

This is not the example we should set for contributors and even less for ourselves. This seems to me like a reflection of short term hacking without long term care. I am -1, but it seems pointless as some people are in love with copy-paste over long term maintenance.

Let's ignore FieldUtils for now. What kind of maintenance work do you think this code will require in the following years? As I see it:

• The usages of ArrayFill in Commons Compress are overkill: all the usages provide a non-null array parameter, so the method can be simplified to:

  public int[] fill(int[] a, int val) {
    Arrays.fill(a, val);
    return a;
  }

  Also note that Commons Lang does not provide a helper method for the most common usage of Arrays.fill, which is:

  public int[] createFilledArray(int size, int val) {
    int[] a = new int[size];
    Arrays.fill(a, val);
    return a;
  }

• The implementation of SystemProperties.getOsName() in Commons Lang delegates to a complex wrapper to System.getProperty(name) that handles security exceptions and an empty or null parameter name. None of these conditions apply to the os.name property, which must have a valuet (see Javadoc for System.getProperties() and should be allowed by SecurityManager.

FieldUtils

The code that relies on FieldUtils constitutes a maintenance problem itself, since it relies on the protected FilterInputStream.in field. The field is protected for a reason: users of FilterInputStream should not skip access the underlying stream directly, but use the wrapping FilterInputStream instead.

[Glavo]

I released Kala Compress 1.27.1-1. It is based on commons-compress, but does not depend on commons-io and commons-lang. Maybe this will be what you want.

[ppkarwasz]

I released Kala Compress 1.27.1-1. It is based on commons-compress, but does not depend on commons-io and commons-lang. Maybe this will be what you want.

I don't believe that forking Commons Compress is the right approach. Your changes in Kala Compress are aligned with what we plan for Commons Compress 2.x.

I have created a 2.x branch. Feel free to open PRs against that branch to split Commons Compress in separate modules. Probably the first module we need to split is a commons-compress2-api module that would include the interfaces to access the various compression algorithms.

[acasademont]

We would really appreciate this one, it came as a surprise to us that when we updated from 1.25.0 we had to add a new dependency on commons lang.

[vlsi]

The change looks good, and it does solve the true pain for the end-users.

Currently commons-compress brings 700KiB commons-lang3 dependency while it uses only a few methods from it. It does sound like a bloat that the PR addresses.

The point of "the code duplicates lang3" is somewhat valid, however, the only "complicated" code relates to FieldUtils which is used in a to-be-deprecated pack200, so I believe it is absolutely fine to move FieldUtils right into pack200 so the "duplicated getField" would die with pack200.

[vlsi]

This is used in a pack200 which has been deprecated for removal in Java 11.
Have you considered moving the implementation to pack200 so the other compressors won't accidentally depend on reflection-like approaches?