Skip to content

HDDS-5238. Disable animal-sniffer maven plugin#2253

Merged
adoroszlai merged 3 commits intoapache:masterfrom
elek:nozoo
May 17, 2021
Merged

HDDS-5238. Disable animal-sniffer maven plugin#2253
adoroszlai merged 3 commits intoapache:masterfrom
elek:nozoo

Conversation

@elek
Copy link
Member

@elek elek commented May 17, 2021

JIRA: https://issues.apache.org/jira/browse/HDDS-5238

What changes were proposed in this pull request?

animal-sniffer maven plugin can check the API compatibility of the current code with a specific java version.

It's useful when the build uses a JDK (eg. Java 8) which is different from the target JDK (eg. Java 1.6), as it can compare the used Java API signatures with the used ones.

We inherited the plugin execution from the original parent Hadoop pom.xml, but it's not required as we build with Java 8 (and Java 11, but it's not important here) and our target minimum JDK is Java 8.

In the meantime, it's also removed from Hadoop by https://issues.apache.org/jira/browse/HADOOP-15938, as the same functionality (if required) can be achieved by standard javac parameters (see discussion from the Hadoop Jira).

On the other hand, the animal sniffer is slow. Removing it makes the build significant faster:

With master:

mvn clean install -DskipTests -Dskip.npx -DskipShade 
...
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  01:44 min
[INFO] Finished at: 2021-05-17T12:40:15+02:00
[INFO] ------------------------------------------------------------------------

With this patch:

mvn clean install -DskipTests -Dskip.npx -DskipShade 
...
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  58.603 s
[INFO] Finished at: 2021-05-17T12:42:05+02:00
[INFO] ------------------------------------------------------------------------

It seems to be 46 / 104 -> 44% build time improvement.

How was this patch tested?

Full CI test (+ local builds)

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, pending CI.

@adoroszlai adoroszlai merged commit ea1ed7c into apache:master May 17, 2021
@adoroszlai
Copy link
Contributor

Thanks @elek for the improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants