droid icon indicating copy to clipboard operation
droid copied to clipboard

Build strangely broken on latest master

Open nishihatapalmer opened this issue 3 years ago • 20 comments

Trying to do a maven build of the most recent master, and the maven dependency analyzer fails with an "Unsupported API" illegal argument exception:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-dependency-plugin:3.1.2:analyze-only (analyze) on project droid-core-interfaces: Execution analyze of goal org.apache.maven.plugins:maven-dependency-plugin:3.1.2:analyze-only failed: Unsupported api 524288 -> [Help 1] org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-dependency-plugin:3.1.2:analyze-only (analyze) on project droid-core-interfaces: Execution analyze of goal org.apache.maven.plugins:maven-dependency-plugin:3.1.2:analyze-only failed: Unsupported api 524288

I can disable the dependency analyzer locally so I can keep building and testing, but this will need to be resolved. No idea what causes this or how to resolve it myself.

nishihatapalmer avatar Jun 02 '21 15:06 nishihatapalmer

Not sure the fault is the dependency analyzer after all. I disabled that, then the license check plugin said there was invalid license files on files which were completely valid. Disabled that... and now nothing builds properly, the code says there are dependencies it can't find when they're a class in the same package! Strange things are breaking all over the place when building the code.

So, I don't know what's going on exactly, but something in the last few commits to master seems to have broken building. Builds still work on branches I haven't merged with latest master.

nishihatapalmer avatar Jun 03 '21 09:06 nishihatapalmer

Reverting the latest dependabot changes didn't fix the issue. Doing a manual bisect on commits to master to try to see where it breaks for me.

nishihatapalmer avatar Jun 03 '21 10:06 nishihatapalmer

PR600 (9b4f58c3) on 29 April 2021 breaks with the same dependency api error I have on latest master: Caused by: org.apache.maven.plugin.PluginExecutionException: Execution analyze of goal org.apache.maven.plugins:maven-dependency-plugin:3.1.2:analyze-only failed: Unsupported api 524288

PR555 (618f1eee) on 4 Jan 2021 builds cleanly. PR557 (5d3bbc74) on 20 Jan 2021 builds cleanly.

PR575 (ab042990) on 8 Feb 2021 breaks with unused annotations error in droid-export:

[WARNING]    jakarta.annotation:jakarta.annotation-api:jar:1.3.5:compile
[WARNING] Unused declared dependencies found:
[WARNING]    javax.annotation:javax.annotation-api:jar:1.3.2:compile

PR592 (63df4a09) on 29 April 2021 also breaks with the above annotations error. PR554 (2e0e1a5d) on 20 Jan 2021 also breaks with the above annotation error.

Reverting the change to cxf version back to 3.3.6 from 3.4.2 causes the build to work again, for the annotations error anyway. Now to see if it helps with the other error.

nishihatapalmer avatar Jun 03 '21 11:06 nishihatapalmer

Just reverting the cxf change on the current master does not fix the original error. So there's other things going on. Have to look at more granular commits between PR555 and PR600 (between 20 Jan 2021 and 29 April 2021).

nishihatapalmer avatar Jun 03 '21 12:06 nishihatapalmer

PR590 (5471aa94) on 29 April 2021 has dependency error (even with cxf version reversion). PR592 (63df4a09) on 29 April 2021 with cxf version reversion fails when assembling due to missing license header in .github/dependabot.yml.

nishihatapalmer avatar Jun 03 '21 12:06 nishihatapalmer

PR592 (63df4a0), excluding yml files from the licence check header, and reverting cxf version to 3.3.6 builds successfully.

PR515 (1d2f6469), excluding yml files from the licence check header, and reverting cxf version to 3.3.6 fails with the dependency error.

nishihatapalmer avatar Jun 03 '21 12:06 nishihatapalmer

I guess unsurprisingly, reverting the changes in PR515 fixes the problem (although the licence check plugin also produced a failure, saying files with valid licenses were invalid - so I disabled that). Still not sure what I missed given I think I made all reversions.

Now to figure out which change causes the issue.

nishihatapalmer avatar Jun 03 '21 13:06 nishihatapalmer

OK, change that breaks seems to be the upgrade of the dependency analyzer plugin, which makes sense:

                        <dependency>
                            <groupId>org.apache.maven.shared</groupId>
                            <artifactId>maven-dependency-analyzer</artifactId>
                            <version>1.11.1</version>
                        </dependency>
                    </dependencies>

Reverting from 1.11.3 to 1.11.1. allows master to build with cxf also reverted to 3.3.6.

However, making the same changes to my branch that merged master still has an error.

nishihatapalmer avatar Jun 03 '21 14:06 nishihatapalmer

Seem to have it. In my branch (which has made changes to export) from master:

  1. Revert CXF to 3.3.6 in droid-parent pom.xml (will check if needed really again)
  2. Revert dependency-analyzer to 1.11.1 in droid-parent pom.xml
  3. Remove the univocity dependency in droid-export pom.xml, which gives an error that it is an unused dependency.

Code builds again. The univocity part is to do with my local branch only.

nishihatapalmer avatar Jun 03 '21 14:06 nishihatapalmer

Note: I also disabled the owasp check (as it was flagging a vulnerability) and the license header check (as it was also behaving strangely). Will have to figure out what's going on there.

nishihatapalmer avatar Jun 03 '21 14:06 nishihatapalmer

The CXF change is still required to avoid the unused dependency error in droid-export on annotatations (jakarta/javax).

Don't know what causes that (is there an unused dependency? Is it a bug in the dependency checker?)

nishihatapalmer avatar Jun 03 '21 14:06 nishihatapalmer

Well, I can build master and my branch again with those changes. I'm building on PopOs 20.04 (ubuntu derivative), using OpenJDK 11.0.11.

Would be good to know if anyone else is having these issues, or if it's something weird to do with my local setup. Unless there is some bug in both CXF and dependency analyzer, it seems strange both need to be reverted to avoid dependency checking issues. Makes me suspect there is some odd dependency issue in the code that is being exposed by the updates.

nishihatapalmer avatar Jun 03 '21 14:06 nishihatapalmer

I checked out and saw the "Unsupported API" error on CentOS 8 building with Java 8, so its unlikely to be just your machine

sparkhi avatar Jun 03 '21 14:06 sparkhi

I checked out and saw the "Unsupported API" error on CentOS 8 building with Java 8, so its unlikely to be just your machine

Good to know it's not just my machine, thanks for confirming the issue.

nishihatapalmer avatar Jun 03 '21 15:06 nishihatapalmer

So - I don't intend to do any more investigation on this issue.

My primary concern is to submit a PR for the command line project that resolves many outstanding issues. I'm extremely close to that now, so all I need to do is build and test.

I will leave it to others to figure out why changes to cxf and dependency analyzer plugin are causing these issues (or exposing dependency issues that actually exist in the code), and what decision DROID needs to make to deal with it.

nishihatapalmer avatar Jun 03 '21 15:06 nishihatapalmer

I've done a deeper dive into the header not matching issue. It looks like the header mismatch started happening since the move of "license maven plugin" from v3.0 to v4.1 (by dependabot) I went through the source code of the license check plugin and it looks like the newer version of the plugin identifies the header as SLASHSTAR_STYLE instead of JAVADOC_STYLE

    JAVADOC_STYLE("/**", " * ", " */", "", null, "(\\s|\\t)*/\\*.*$", ".*\\*/(\\s|\\t)*$", false, true, false),
   SLASHSTAR_STYLE("/*", " * ", " */", "", null, "(\\s|\\t)*/\\*.*$", ".*\\*/(\\s|\\t)*$", false, true, false),

I do not know why that is the case though but looking at the above definitions, there is a possibility of mismatch depending on what is picked first, anyways, if we change the headers to delete one * from the first line, the error would go away.

Right now, my preference is to rollback the versions of the 2 dependencies (license-maven-plugin and maven-dependency-analyzer) to get the master being sane again before anything else

sparkhi avatar Jun 04 '21 13:06 sparkhi

I've just merged a fix that passed on all versions of java. Hopefully the master will be building successfully soon. Thanks

sparkhi avatar Jun 07 '21 11:06 sparkhi

like the newer version of the plugin identifies the header as SLASHSTAR_STYLE instead of JAVADOC_STYLE

@sparkhi Yes, that is intentional. Previously JAVADOC_STYLE was used for Java files. Unfortunately newer versions of the Javadoc tool complain about that, and so it was switched to SLASHSTAR_STYLE for Java files. The solution is to run mvn license:format and allow it to reformat the licenses as SLASHSTAR_STYLE.

adamretter avatar Jun 08 '21 13:06 adamretter

@sparkhi @nishihatapalmer May I suggest before anything else is committed that we should restore CI. I sent a PR to fix CI, but I don't know if anyone saw it - https://github.com/digital-preservation/droid/pull/620

adamretter avatar Jun 08 '21 13:06 adamretter

I can confirm master builds for me with those changes.

nishihatapalmer avatar Jun 11 '21 15:06 nishihatapalmer