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

JENKINS-72441 Hide parent gson classes #447

Merged
merged 2 commits into from Dec 20, 2023

Conversation

jonesbusy
Copy link
Contributor

See https://issues.jenkins.io/browse/JENKINS-72441

jcloud bundle an old version of gson. Fixed on 2.6.0 but unreleased

Testing done

Automated tests

Submitter checklist

Edit tasklist title
Beta Give feedback Tasklist Submitter checklist, more options

Delete tasklist

Delete tasklist block?
Are you sure? All relationships in this tasklist will be removed.
  1. Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
    Options
  2. Ensure that the pull request title represents the desired changelog entry
    Options
  3. Please describe what you did
    Options
  4. Link to relevant issues in GitHub or Jira
    Options
  5. Link to relevant pull requests, esp. upstream and downstream changes
    Options
  6. Ensure you have provided tests - that demonstrates feature works or fixes the issue
    Options

Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I tested this as follows:

Reproduced the problem by creating a Pipeline job that uploaded an artifact to S3. Could no longer reproduce the problem after this PR. Also verified in the Script Console that loading com.google.gson.Gson from the Artifact Manager on S3 plugin classloader loaded the class from the GSON API plugin's GSON JAR before this PR and from the Artifact Manager on S3 plugin's GSON JAR after this PR.

pom.xml Show resolved Hide resolved
@basil basil merged commit 87eb541 into jenkinsci:master Dec 20, 2023
0 of 2 checks passed
@basil basil added the bug Something isn't working label Dec 20, 2023
@jonesbusy jonesbusy deleted the feature/JENKINS-72441-hide-gson branch December 20, 2023 16:40
@jglick
Copy link
Member

jglick commented Jan 17, 2024

Reproduced the problem by…

Via hpi:run, or using plugins from the UC? There is a RealJenkinsRule test here which apparently did not catch the problem; does anyone happen to know if adding this plugin to bom would have caught it?

@basil
Copy link
Member

basil commented Jan 17, 2024

Via hpi:run, or using plugins from the UC?

The latter.

@jglick
Copy link
Member

jglick commented Jan 17, 2024

So I checked the effect of reverting this PR on top of #450 and indeed MinioIntegrationTest.artifactArchive blows up with

NoSuchMethodError: 'void ConstructorConstructor.<init>(Map)'
	at GsonModule.provideGson(GsonModule.java:130)
	at GsonModule$$FastClassByGuice$$320f98e2.GUICE$TRAMPOLINE(<generated>)
	at GsonModule$$FastClassByGuice$$320f98e2.apply(<generated>)
	at ProviderMethod$FastClassProviderMethod.doProvision(ProviderMethod.java:260)
	at ProviderMethod.doProvision(ProviderMethod.java:171)
	at InternalProviderInstanceBindingImpl$CyclicFactory.provision(InternalProviderInstanceBindingImpl.java:185)
	at InternalProviderInstanceBindingImpl$CyclicFactory.get(InternalProviderInstanceBindingImpl.java:162)
	at ProviderToInternalFactoryAdapter.get(ProviderToInternalFactoryAdapter.java:40)
	at SingletonScope$1.get(SingletonScope.java:169)
	at InternalFactoryToProviderAdapter.get(InternalFactoryToProviderAdapter.java:45)
	at SingleParameterInjector.inject(SingleParameterInjector.java:40)
	at SingleParameterInjector.getAll(SingleParameterInjector.java:60)
	at ConstructorInjector.provision(ConstructorInjector.java:113)
	at ConstructorInjector.construct(ConstructorInjector.java:91)
	at ConstructorBindingImpl$Factory.get(ConstructorBindingImpl.java:300)
	at ProviderToInternalFactoryAdapter.get(ProviderToInternalFactoryAdapter.java:40)
	at SingletonScope$1.get(SingletonScope.java:169)
	at InternalFactoryToProviderAdapter.get(InternalFactoryToProviderAdapter.java:45)
	at FactoryProxy.get(FactoryProxy.java:60)
	at InternalInjectorCreator.loadEagerSingletons(InternalInjectorCreator.java:213)
	at InternalInjectorCreator.injectDynamically(InternalInjectorCreator.java:186)
	at InternalInjectorCreator.build(InternalInjectorCreator.java:113)
	at Guice.createInjector(Guice.java:87)
	at ContextBuilder.buildInjector(ContextBuilder.java:405)
	at ContextBuilder.buildInjector(ContextBuilder.java:328)
	at ContextBuilder.buildView(ContextBuilder.java:615)
	at ContextBuilder.buildView(ContextBuilder.java:595)
	at s3.S3BlobStore.getContext(S3BlobStore.java:136)
	at CustomBehaviorBlobStoreProvider.getContext(CustomBehaviorBlobStoreProvider.java:68)
	at JCloudsArtifactManager.getContext(JCloudsArtifactManager.java:384)
	at JCloudsArtifactManager.archive(JCloudsArtifactManager.java:127)
	at ArtifactArchiver.perform(ArtifactArchiver.java:258)
	at …

similar to the reported error. Unfortunately this test also requires Docker fixtures, and bom/Jenkinsfile does not use VMs, so adding this plugin to bom would not accomplish anything. At least we would find out about such errors eventually (in the absence of any user alarms) just by Dependabot updates to the bom dep.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants