Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test project compilation fixes for Eclipse #1317

Merged
merged 4 commits into from Oct 6, 2020
Merged

Conversation

iloveeclipse
Copy link
Member

This fixes issues #1316 and #1315.

Make sure these boxes are checked before submitting your PR -- thank you!

  • Added an entry into CHANGELOG.md if you have changed SpotBugs code

This partly fixes issue #1315 compilation error on test projects, so
that Eclipse can access spotbugs project too.

The remaining errors are related to the issue #1316, which is not that
trivial, so is coming in the next commit.

Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
This fixes issue #1316 and finally fixes issue #1315.

This change switches from Jaxen 1.1.6 to 1.2.0 to allow compilation of
the projects that rely on SpotBugs project with Java 11.

Jaxen is a transitive optional dependency of dom4j that we use in
SpotBugs.

Jaxen 1.1.6 ships org.w3c.dom package, which is also now shipped by JDK
in java.xml module. Java 9+ comiler disallow compilation if modules on
module path ship same package. So compilation of spotbugsTestCases with
Java 11 fails if the target release is 11 with the error: "The package
org.w3c.dom is accessible from more than one module: <unnamed>,
java.xml".

Solution is to update Jaxen to 1.2.0 version, that does not ship
org.w3c.dom package anymore.

Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
@@ -4,7 +4,7 @@ apply from: "$rootDir/gradle/maven.gradle"

dependencies {
compileOnly 'junit:junit:4.13'
compileOnly project(':spotbugs')
testImplementation project(':spotbugs')
Copy link
Member

Choose a reason for hiding this comment

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

Could you consider to add explanations in CHANGELOG for this change?
I think users of test-harness and test-harness-jupiter needs to this change (from compile-only scope to compile scope).

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure how the changed compilation setting here can affect users. Dependencies itself aren't changed. That should be fully transparent for clients.

Copy link
Member Author

Choose a reason for hiding this comment

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

@KengoTODA : anything else is missing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ping...

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @KengoTODA ,

to work on #1254, I had to cherry-pick the changes here. Can the changes get merged so I can work with actual master?

Best regards and thanks,
Simeon

Copy link
Member

Choose a reason for hiding this comment

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

@iloveeclipse
I think changes in test-harness affect its dependency.
That dependency will be changed.
api configuration makes test-harness dependent on that.

Copy link
Member Author

Choose a reason for hiding this comment

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

@wreulicke : not sure what do you mean by that. How this change can affect clients of test-harness? I'm not the gradle user, I have no idea why there are multiple dependencies chains invented by gradle, and how that, once a module is compiled, can affect its clients - I really can't understand that. And btw, let assume test-harness clients would be now also depend on spotbugs - what's wrong by that? How that would affect them, they had spotbugs on classpath already anyway?

My problem is that current state on master is not usable with Eclipse, so contributing is a pain, nothing compiles. That is way more severe issue as a theoretical possibility to somehlw break one or two test-harness clients that may exists in the world.

@iloveeclipse iloveeclipse force-pushed the eclipse_fixes branch 2 times, most recently from 94324ed to c28885f Compare September 30, 2020 07:23
This fixes #1314

Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
This fixes issue #893

Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
@sonarcloud
Copy link

sonarcloud bot commented Sep 30, 2020

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 1 Code Smell

30.0% 30.0% Coverage
0.0% 0.0% Duplication

@KengoTODA
Copy link
Member

Sorry for my late reaction, recently it's a long national holiday in China.

I'll submit another PR to make CHANGELOG better.

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.

None yet

4 participants