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-58771] Consider plugin dependencies with test scope to consider whether bundling a dependency is needed #140

Merged
merged 2 commits into from
Sep 10, 2019

Conversation

Vlatombe
Copy link
Member

@Vlatombe Vlatombe commented Sep 9, 2019

JENKINS-58771

Plugin artifacts are determined only from project artifacts, which captures only dependencies with compile/runtime scopes.

In some cases, it can happen that transitive dependencies are resolved via a test-scoped trail, causing unexpected dependencies to be packaged in the resulting hpi.

@jglick
Copy link
Member

jglick commented Sep 10, 2019

Inspired by jenkinsci/kubernetes-plugin#592 I suppose.

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

src/it/JENKINS-58771-packaged-plugins/invoker.properties Outdated Show resolved Hide resolved
src/it/JENKINS-58771-packaged-plugins/pom.xml Outdated Show resolved Hide resolved
src/it/JENKINS-58771-packaged-plugins/verify.groovy Outdated Show resolved Hide resolved
@@ -487,13 +488,16 @@ public void buildWebapp(MavenProject project, File webappDirectory)

Set<MavenArtifact> artifacts = getProjectArtfacts();

// Also capture test dependencies
Set<MavenArtifact> dependencyArtifacts = getDirectDependencyArtfacts();
Copy link
Member

Choose a reason for hiding this comment

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

This will certainly help, though I doubt it will cover every possible case, since this gets only direct and not transitive dependencies. There is a different method for getting resolved transitive dependencies at test scope, but I am afraid it requires to mojo to be configured to resolve to test scope (not just compile), which might cause other unwanted effects. Probably this is good enough for now, until a more exotic situation is discovered.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I went for the simplest solution for now as I think it will think the most common cases.

@Vlatombe
Copy link
Member Author

Already had tried against jenkinsci/kubernetes-plugin#592, also gave a try with jenkinsci/workflow-cps-global-lib-plugin#75 and it works fine, no longer need to use the exclusion.

@jglick jglick added the bug label Sep 10, 2019
@jglick jglick merged commit be16de6 into jenkinsci:master Sep 10, 2019
@Vlatombe Vlatombe deleted the JENKINS-58771 branch September 10, 2019 13:58
jglick added a commit to jglick/workflow-cps-global-lib-plugin that referenced this pull request Sep 16, 2019
Vlatombe added a commit to Vlatombe/kubernetes-plugin that referenced this pull request Mar 4, 2020
* ssh-credentials 1.18
* fix bundled dependencies that shouldn't be packaged
(jenkinsci/maven-hpi-plugin#140 doesn't cover
all use cases)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants