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

Fixed shading for javax.annotation.CheckForNull #563

Merged
merged 1 commit into from
Jan 28, 2018

Conversation

dimafeng
Copy link
Contributor

This should fix the compilation problems described testcontainers/testcontainers-scala#11

@rnorth
Copy link
Member

rnorth commented Jan 28, 2018

This sounds fine if it resolves the problem - thank you for your detective work.

Can I just check my understanding:

  • This will add Findbugs' jsr305 as a shaded dependency
  • However, we don't relocate this shaded dep, unlike most other deps we shade
  • Also, it looks like the javax.annotations relocation is still present in the POM.

Can we relocate this dep, to avoid conflicts? Or is leaving it un-relocated an important part of this change?

@dimafeng
Copy link
Contributor Author

As I understood, the reason why it didn't work before is that:

  1. There wasn't any implementation of jsr305 (specifically CheckForNull which is used in InspectContainerResponse)
  2. artifactSet include pattern should contain groupId/artifactId (not the classes' package).

My changes just fix relocation. com.google.code.findbugs:jsr305 is a source of classes for relocation and the following classes are added to the resulted jar:

5410 org/testcontainers/shaded/javax/annotation/CheckForNull.class 
 5411 org/testcontainers/shaded/javax/annotation/CheckForSigned.class 
 5412 org/testcontainers/shaded/javax/annotation/CheckReturnValue.class 
 5413 org/testcontainers/shaded/javax/annotation/Detainted.class 
 5414 org/testcontainers/shaded/javax/annotation/MatchesPattern$Checker.class 
 5415 org/testcontainers/shaded/javax/annotation/MatchesPattern.class 
 5416 org/testcontainers/shaded/javax/annotation/Nonnegative$Checker.class 
 5417 org/testcontainers/shaded/javax/annotation/Nonnegative.class 
 5418 org/testcontainers/shaded/javax/annotation/Nonnull$Checker.class 
 5419 org/testcontainers/shaded/javax/annotation/Nonnull.class 
 5420 org/testcontainers/shaded/javax/annotation/Nullable.class 
 5421 org/testcontainers/shaded/javax/annotation/OverridingMethodsMustInvokeSuper.class 
 5422 org/testcontainers/shaded/javax/annotation/ParametersAreNonnullByDefault.class 
 5423 org/testcontainers/shaded/javax/annotation/ParametersAreNullableByDefault.class 
 5424 org/testcontainers/shaded/javax/annotation/PropertyKey.class 
 5425 org/testcontainers/shaded/javax/annotation/RegEx$Checker.class 
 5426 org/testcontainers/shaded/javax/annotation/RegEx.class 
 5427 org/testcontainers/shaded/javax/annotation/Signed.class 
 5428 org/testcontainers/shaded/javax/annotation/Syntax.class 
 5429 org/testcontainers/shaded/javax/annotation/Tainted.class 
 5430 org/testcontainers/shaded/javax/annotation/Untainted.class 
 5431 org/testcontainers/shaded/javax/annotation/WillClose.class 
 5432 org/testcontainers/shaded/javax/annotation/WillCloseWhenClosed.class 
 5433 org/testcontainers/shaded/javax/annotation/WillNotClose.class 
 5434 org/testcontainers/shaded/javax/annotation/concurrent/GuardedBy.class 
 5435 org/testcontainers/shaded/javax/annotation/concurrent/Immutable.class 
 5436 org/testcontainers/shaded/javax/annotation/concurrent/NotThreadSafe.class 
 5437 org/testcontainers/shaded/javax/annotation/concurrent/ThreadSafe.class 
 5438 org/testcontainers/shaded/javax/annotation/meta/Exclusive.class 
 5439 org/testcontainers/shaded/javax/annotation/meta/Exhaustive.class 
 5440 org/testcontainers/shaded/javax/annotation/meta/TypeQualifier.class 
 5441 org/testcontainers/shaded/javax/annotation/meta/TypeQualifierDefault.class 
 5442 org/testcontainers/shaded/javax/annotation/meta/TypeQualifierNickname.class 
 5443 org/testcontainers/shaded/javax/annotation/meta/TypeQualifierValidator.class 
 5444 org/testcontainers/shaded/javax/annotation/meta/When.class

Does it answer the questions?
Sorry for not including these details in the PR description.

@rnorth
Copy link
Member

rnorth commented Jan 28, 2018

Ah of course - makes perfect sense! Thank you.

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

Successfully merging this pull request may close these issues.

3 participants