Remove Commons Lang dependency
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.
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.
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
ArrayFillin Commons Compress are overkill: all the usages provide a non-null array parameter, so the method can be simplified to:
Also note that Commons Lang does not provide a helper method for the most common usage ofpublic int[] fill(int[] a, int val) { Arrays.fill(a, val); return a; }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 toSystem.getProperty(name)that handles security exceptions and an empty ornullparametername. None of these conditions apply to theos.nameproperty, which must have a valuet (see Javadoc for System.getProperties() and should be allowed bySecurityManager.
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.
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 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.
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.