[MNG-6825] Cleaning Up dependencies plexus-utils/maven-shared-utils/commons-lang3/commons-io

[jira-importer]

Karl Heinz Marbaise opened MNG-6825 and commented

Experiment to see if we can get rid of commons-lang3 dependency.

• Identified code duplication between:
  • org.apache.commons.lang3.StringUtils from commons-lang3 (501879 Bytes commons-lang3-3.8.1.jar)
  • org.codehaus.plexus.util.StringUtil from plexus-utils (261801 Bytes plexus-utils-3.2.1.jar)
  • org.apache.maven.shared.utils from maven-shared-utils (166562 Bytes maven-shared-utils-3.2.1.jar)
  • We should replace the self implemented with org.codehaus.plexus.util.StringUtil. or with maven-shared-utils
• commons-io usage by maven-shared-utils (208700 Bytes commons-io-2.5.jar)

---

Affects: 3.6.3

Sub-tasks:

• MNG-6829 Remove commons-lang3 dependency

Issue Links:

• MNG-7423 Remove Dependency to org.apache.commons:commons-lang3
  ("is duplicated by")

• MNG-3819 [regression] Plugins that don't declare dependency on plexus-utils no longer get plexus-utils:1.1

• MNG-7423 Remove Dependency to org.apache.commons:commons-lang3

• MRESOLVER-350 Get rid of commons-lang dependency

• MNG-5378 Use maven-shared-utils in Maven core to replace plexus-utils

• MRESOLVER-350 Get rid of commons-lang dependency

Remote Links:

• GitHub Pull Request #111
  
• GitHub Pull Request #299
  
• GitHub Pull Request #45
  
• GitHub Pull Request #156
  
• GitHub Pull Request #158
  
• GitHub Pull Request #267
  
• GitHub Pull Request #255
  
• GitHub Pull Request #185
  
• GitHub Pull Request #183
  
• GitHub Pull Request #199
  
• GitHub Pull Request #7
  
• GitHub Pull Request #6
  
• GitHub Pull Request #95
  
• GitHub Pull Request #832
  
• GitHub Pull Request #833
  
• GitHub Pull Request #120
  
• GitHub Pull Request #275
  
• GitHub Pull Request #168
  
• GitHub Pull Request #139
  
• GitHub Pull Request #1066
  
• GitHub Pull Request #1067
  
• GitHub Pull Request #1078
  
• GitHub Pull Request #1080
  

Backported to: 4.0.0-alpha-7

3 votes, 9 watchers

[jira-importer]

Michael Osipov commented

Our approach should be to get rid of selfwritten stuff and use Commons Lang. Plexus Utils does way too many things at once. The current situation is for historical reasons.

[jira-importer]

Karl Heinz Marbaise commented

Using a 500 KB jar file for three classes: StringUtils, Validate and SystemUtils.

• StringUtils can be replaced by usage of either plexus-utils or maven-shared-utils or as I tested with self implementation
• SystemUtils is only used in some tests which can simply replaced by using JUnit Jupiter with all the support it has.
• Validate is a precondition class which checks for parameters etc. can be implemented very easily (done already to see how it works). Later this could be made part of maven-shared-utils.

Currently from maven-shared-utils we are using:

• org.apache.maven.shared.utils.io.FileUtils (Test Only)
• org.apache.maven.shared.utils.logging.MessageBuilder
• org.apache.maven.shared.utils.logging.MessageUtils

The real issue I have found (I was already aware of that) is that plexus-utils as well as maven-shared-utils having so many code in common (duplicate it more or less) and both of them are used.

Furthermore I found that currently the need for commons-io in Maven Core (or better it lands in the distribution lib directory) is only based on usage of commons-io in maven-shared-utils where only a single class org.apache.commons.io.input.XmlStreamReader is used of commons-io.

If I summarize this:

• If we would move to maven-shared-utils only we could get rid of
  • 501879 Byte commons-lang3 (/)
  • 261801 Byte plexus-utils (x)
  • 208700 Byte commons-io (x)

[jira-importer]

Michael Osipov commented

I have taked a look at it. There is still a lot of duplicate code which I think we need to reduce. Have as much as possible at Commons and reduce the rest. Another, interim, option is to shade a minimum set of classes.

[jira-importer]

Tim te Beek commented

Thought this could be a fun one to dive into for me, with my involvement in OpenRewrite & Moderne for some potential automated fixes (if appreciated!). For those not familiar: through public.moderne.io you can create a repository group containing some 80+ (but likely not yet {}all{}) Apache Maven projects, and not only find all occurrences of types and methods, but also replace those with alternatives where available.

So as a quick example; when I search for the type org.codehaus.plexus.util.StringUtils (emphasis on the final s), I see some 387 results across 44 out of 85 repositories, with the top three having 71 (maven-scm), 49 (maven) and 24 (maven-doxia) results each, with a long tail in the other 41 repositories. We can also zoom in on particular methods, and then only on projects that also depend on commons-lang3 or maven-shared-utils for instance, to swap those plexus implementations out for their alternatives, where desired.

All I would need to apply these fixes is some guidelines on what replacements are desired, and how you'd like me to approach such replacements.

• Should we add dependencies where they don't currently exist for instance? Or should we only replace where alternative are already available on the classpath?
• And would you want different replacements to be applied all together at once, or isolated for an easier review (but more PRs)?
• And would anyone want to pair on this such that it's not just me creating these, and potentially skip the asynchronous review work?

Any and all feedback appreciated, also if this work is not desired at this time. :) 

[jira-importer]

Elliotte Rusty Harold commented

Both org.codehaus.plexus.util.StringUtils and maven-shared-utils should go away. In many cases we can simply use the JDK. Otherwise the better choice is org.apache.commons.lang3.StringUtils as it's actively supported, which is more than we can say for plexus utils and maven-shared-utils. Generally we need to get away from the idea that generic utilities should be implemented in maven itself. They are better handled by Apache Commons, which focuses on those things and is used and tested far beyond Maven.

[jira-importer]

Tamas Cservenak commented

I agree here with Elliotte, those projects (commons, plexus-utils, maven-shared-utils) share common roots, have code copy pasted from each to other... IMHO, maven-shared-utils https://github.com/apache/maven-shared-utils should simply die off, plexus-utils should deprecate all methods that are present in other commons project, and at one point just drop them. This last step was on-hold almost forever, as maven was injecting "currently used" plexus-utils into plugins, which is not the case anymore, since 3.9 all plugins MUST declare and use their own explicit plexus-utils version, so the danger of incompatibility is gone.

[jira-importer]

Tim te Beek commented

Thank you both for weighing in here; helps to have the goal so clearly stated. As said this could be fun to dive into for me, but I don't want to burden anyone with review work, especially when these changes are applied at scale. So my proposal would be that I start small, but ideally do apply multiple changes per pull request, rather than have lots of small pull requests per repository.

My takeaways from above are:

• remove org.codehaus.plexus.util.StringUtils
• remove maven-shared-utils
• prefer JDK methods where possible
• fall back onto commons-* where there's no equivalent in JDK

Are those takeaways correct?

And could you indicate for which of these repositories you'd like me to create a first few pull requests?

apache/maven-acr-plugin @ master
apache/maven-antrun-plugin @ master
apache/maven-archetype @ master
apache/maven-archetypes @ master
apache/maven-archiver @ master
apache/maven-artifact-plugin @ master
apache/maven-artifact-resolver @ master
apache/maven-artifact-transfer @ master
apache/maven-assembly-plugin @ master
apache/maven-build-cache-extension @ master
apache/maven-changelog-plugin @ master
apache/maven-changes-plugin @ master
apache/maven-checkstyle-plugin @ master
apache/maven-clean-plugin @ master
apache/maven-common-artifact-filters @ master
apache/maven-compiler-plugin @ master
apache/maven-dependency-analyzer @ master
apache/maven-dependency-plugin @ master
apache/maven-dependency-tree @ master
apache/maven-deploy-plugin @ master
apache/maven-doap-plugin @ master
apache/maven-docck-plugin @ master
apache/maven-doxia-book-maven-plugin @ master
apache/maven-doxia-book-renderer @ master
apache/maven-doxia-converter @ master
apache/maven-doxia-sitetools @ master
apache/maven-doxia @ master
apache/maven-ear-plugin @ master
apache/maven-ejb-plugin @ master
apache/maven-enforcer @ master
apache/maven-file-management @ master
apache/maven-filtering @ master
apache/maven-gpg-plugin @ master
apache/maven-help-plugin @ master
apache/maven-indexer @ master
apache/maven-install-plugin @ master
apache/maven-integration-testing @ master
apache/maven-invoker-plugin @ master
apache/maven-invoker @ master
apache/maven-jar-plugin @ master
apache/maven-jarsigner-plugin @ master
apache/maven-jarsigner @ master
apache/maven-javadoc-plugin @ master
apache/maven-jdeprscan-plugin @ master
apache/maven-jdeps-plugin @ master
apache/maven-jlink-plugin @ master
apache/maven-jmod-plugin @ master
apache/maven-mapping @ master
apache/maven-patch-plugin @ master
apache/maven-pdf-plugin @ master
apache/maven-plugin-testing @ master
apache/maven-plugin-tools @ master
apache/maven-pmd-plugin @ master
apache/maven-project-info-reports-plugin @ master
apache/maven-project-utils @ master
apache/maven-rar-plugin @ master
apache/maven-release @ master
apache/maven-remote-resources-plugin @ master
apache/maven-reporting-api @ master
apache/maven-reporting-exec @ master
apache/maven-reporting-impl @ master
apache/maven-resolver-ant-tasks @ master
apache/maven-resolver @ master
apache/maven-resources-plugin @ master
apache/maven-runtime @ master
apache/maven-scm-publish-plugin @ master
apache/maven-scm @ master
apache/maven-script-interpreter @ master
apache/maven-scripting-plugin @ master
apache/maven-shade-plugin @ master
apache/maven-shared-incremental @ master
apache/maven-shared-io @ master
apache/maven-shared-jar @ master
apache/maven-shared-utils @ master
apache/maven-site-plugin @ master
apache/maven-source-plugin @ master
apache/maven-stage-plugin @ master
apache/maven-surefire @ master
apache/maven-toolchains-plugin @ master
apache/maven-verifier-plugin @ master
apache/maven-verifier @ master
apache/maven-wagon @ master
apache/maven-war-plugin @ master
apache/maven-wrapper @ master
apache/maven @ master

I'd thought to document any work I do here, and build up a repeatable set of recipes that can then later be applied to all repositories if so desired.

[jira-importer]

Michael Osipov commented

Same opinion here.

[jira-importer]

Michael Osipov commented

Tim te Beek, I'd say core, then plugins, later the rest.

[jira-importer]

Elliotte Rusty Harold commented

Feel free to throw any reviews my way. I do prefer smaller reviews. They tend not to be drop in replacements since null checking particular can vary from one line of code to the next. E.g. sometimes a StringUtils.isEmpty replacement needs to include a null check and sometime it doesn't if the string has already been dereferenced previously. We don't really have tooling to perform a lot of these modifications at scale, so if you do what you can when you can, eventually we'll get it done.

https://google.github.io/eng-practices/review/developer/small-cls.html

[jira-importer]

Tim te Beek commented

Opened another quick few pull requests to replace StringUtils, only there where commons-lang3 was already a dependency:
https://github.com/pulls?q=is%3Aopen+is%3Apr+author%3Atimtebeek+archived%3Afalse+org%3Aapache
Using these recipes: https://github.com/timtebeek/maven-rewrite/blob/main/string-utils.yml

Wanted to discuss here how we want to approach updating projects that do not already depend on commons-lang3: there's a total of 55 repositories using either Plexus StringUtils or Maven shared StringUtils in the 89 Apache Maven repositories scanned; details on this temporary link: https://public.moderne.io/results/lJnRe

I fear it'll be too noisy and not go over well with individual project maintainers if I were to create pull requests without some prior coordination here.  Can someone tell me what I should or should not do with regards to further upgrades for this issue? Technically there's no limitations to create more PRs, but we can't deny the human aspect around review & acceptance either. :) 

So far I'd created separate pull requests for StringUtils versus FileUtils & IOUtils to make each small review easier, as requested previously; do we want to continue with that approach also when that leads to dozens of pull requests per use case?

Only when all three use cases have been replaced with commons-lang3 / commons-io / commons-text can we evaluate if we can remove any of the plexus / maven-shared dependencies, for another potential round of PRs to remove the dependencies.

[jira-importer]

Elliotte Rusty Harold commented

You can send the reviews to me. I think it's useful to continue to separate pull requests for StringUtils versus FileUtils & IOUtils in case there's a bug git bisect needs to find.

Long term I think most (all?) of the StringUtils calls and many of the IO calls can be replaced with JDK methods exclusively, but for now adding commons-lang dependencies is a reasonable stepping stone.

[jira-importer]

Elliotte Rusty Harold commented

However the "co-authored by Moderne" in the commits might be a problem unless they've signed the CLA. This might require input from Apache license lawyers or equivalent.

[jira-importer]

Slawomir Jaranowski commented

We have in many project changes connected with this . 
But when we want release a specific project we don't have it on release notes for project ... 

Yes I know it can make a little more issue but will be clear.
Now I see a mess.

Simply each change in project/component should be connected with own issue  for project not with general one issue for Maven.

[jira-importer]

Tim te Beek commented

Thanks for the continued offer to review Elliotte Rusty Harold ! I've passed on your note about the "co-authored by Moderne" to see how we can best alleviate those concerns; we specifically setup https://public.moderne.io for OSS projects to use the platform. The co-authored is merely a way to attribute those changes such that it's clear automation was involved. It should not be a hindrance to see changes adopted, and have asked if this can  be confirmed here, or by signing the CCLA.

As for sjaranowski : I understand your concerns about the release notes for individual projects, but want to make it clear exactly what you're asking, and how that might affect the effort involved here. At present the code changes are automated through the platform UI, but that automation does not extend to cover Jira ticket creation. Looking at Jira there are some 72 projects defined, with a unknown mapping to the 89 repositories available in the platform; here's the full list of projects I see in Jira right now, to give sense of scale:

ARCHETYPE, DOXIA, DOXIASITETOOLS, DOXIATOOLS, JXR, MACR, MANT, MANTRUN, MANTTASKS, MARCHETYPES, MARTIFACT, MASFRES, MASSEMBLY, MBUILDCACHE, MCHANGELOG, MCHANGES, MCHECKSTYLE, MCLEAN, MCOMPILER, MDEP, MDEPLOY, MDOAP, MDOCCK, MEAR, MECLIPSE, MEJB, MENFORCER, MGPG, MINDEXER, MINSTALL, MINVOKER, MJAR, MJARSIGNER, MJAVADOC, MJDEPRSCAN, MJDEPS, MJLINK, MJMOD, MLINKCHECK, MMETRIC, MNG, MNGSITE, MPATCH, MPDF, MPH, MPIR, MPLUGIN, MPLUGINTESTING, MPMD, MPOM, MRAR, MRELEASE, MREPOSITORY, MRESOLVER, MRESOURCES, MRRESOURCES, MSCMPUB, MSCRIPTING, MSHADE, MSHARED, MSITE, MSKINS, MSOURCES, MSTAGE, MTOMCAT, MTOOLCHAINS, MVERIFIER, MWAR, MWRAPPER, SCM, SUREFIRE, WAGON

If you would like to see individual tickets per change per project we would have to:

1. define a mapping from repository to project, needed to attribute repository code changes to Jira projects
2. ditch the platform UI and switch to a GraphQL API client to detect applicable code changes
3. automated the creation of a ticket per project, likely linking back here for a full description
4. create the the code change PR per repository using the appropriate commit message with ticket reference
5. close the individual project tickets as completed when the PRs are merged (but only on full transformations)

We'll likely have to do multiple rounds of 2-5 for additional changes, for the initial migration of StringUtils to commons-lang3, and then later the subsequent migration to JDK internals when language levels allow. This would require a substantial time investment to setup that automation; which  while not impossible, would be harder to fit into my schedule as opposed to using the platform UI. The upside would be that it'd be easier to apply more such changes in the future while reflecting all such changes in the release notes.

Given that I'll be traveling for much of April through July for conferences I fear the required API setup and integration with Jira will then only happen after the summer, and any code changes would have to wait for that, unless anyone else wants to step up to help out.