rvesse / airline

Java annotation-based framework for parsing Git like command line structures with deep extensibility
https://rvesse.github.io/airline/
Apache License 2.0
128 stars 20 forks source link

[enchancement] Remove commons-lang and commons-collections dependencies #64

Closed agentgt closed 6 years ago

agentgt commented 7 years ago

It would be nice if airline had less dependencies for a variety of reasons but the most notable one being resulting uber/single/shaded jar size (assuming you are making a command line application).

I know there are probably some tree shaking like tools but I have never trusted those (or maybe there are not. I can't remember if proguard removes unneeded classes)..

Commons lang would be particularly easy to remove as StringUtils and Pair are the only things used. I could do a PR for that dependency if you like?

rvesse commented 7 years ago

Could you provide more information as to why you see this as a problem?

The two dependencies in question are about 1MB total which is pretty tiny in this day and age. Other than internal dependencies between modules and test scoped dependencies these are the only dependencies that Airline has.

In most production cases where I have used airline the actual dependencies of the command line tool being created have vastly outweighed Airline's dependencies. So in terms of size stripping these out would have minimal effect on the final artefact size

agentgt commented 7 years ago

Here is the use case:

We have a command line application that we created for our builds. Sort of a build tool. We would like to check this build tool into source control (right now we build the build tool).

Ideally this build tool would be small even if we don't check it in as we have to download it on a new build.

Otherwise we might do some caching similar to the gradle/maven wrappers.

agentgt commented 7 years ago

BTW I'm not the only one that wants this. Some incidental googling revealed this:

https://gitlab.citius.usc.es/github/jflap-lib/commit/e8fe41c13fdfa0a9bed9be0ce9b42b32eb8248b6 Notice the commit comment.

In most production cases where I have used airline the actual dependencies of the command line tool being created have vastly outweighed Airline's dependencies. So in terms of size stripping these out would have minimal effect on the final artefact size

Not really true. Our command line apps classes ~ 70k. I would imagine most command line applications are not massive WAR like applications.

And if they are big boy applications than dependencies (collisions) are even more of an issue.

rvesse commented 7 years ago

That particular commit comment is in relation to removing the original library from which this library was forked. The original had much larger dependencies in the form of Google Guava which also has a lots of issues with binary incompatibility between versions. Note Guava is also much larger at 5 MB

I switched over to Apache commons because of the strong guarantees of binary compatibility that avoids dependency collision and conflict problems.

agentgt commented 7 years ago

Do you need more use cases or more reasons?

But I have a feeling your mind might already be made on this. And I can understand why. More work and more risk. I can just continue with my fork for the time being.

agentgt commented 7 years ago

Just a few coding things I recommend if you don't already know about them (and I promise I'll eventually close this issue and leave you alone):

There are an enormous amount of places where arrays are converted back and forth with both AirlineUtils and Commons collections:

Arrays.asList

Most of these could have been replaced with Arrays.asList(...). Once an array is a list you an then just rely on the Iterable contracts.

That is instead of:

convertToSomeCollectionOrIterable(new String[] { "a", "b" ,"c"})

You could write:

Arrays.asList("a","b","c");

I would imagine in large part this was probably due to the conversion away from Guava.

IMO It appears the greatest value that the external libraries provided was split.

Split in plain java can be written as:

"someString".split(Pattern.quote(separator));

I believe split was slow at one time but the builtin split even though uses regex is now not that slow.

join and the other methods were/are trivial to write and in many cases could have been inlined or god forbid just use a for loop 😄 .

Hopefully you don't mind the criticism (in general my code I'm sure is worse).

Honestly I appreciate the library and the work to maintain it. 😄 .