Skip to content
This repository has been archived by the owner on Mar 30, 2021. It is now read-only.

[JENKINS-33095] Fix circular dependency issue #2

Merged
merged 1 commit into from Feb 23, 2016
Merged

Conversation

jtnord
Copy link
Member

@jtnord jtnord commented Feb 23, 2016

JENKINS-33095

The core version that icon-shim uses is old so when run on a newer core the plugin manager will add dependencies to plugins that have been split from core. This causes an unwanted circular dependency as matrix-auth was split from core post the original baseline and has a dependency on cloudbees-folder, which depends on credentials, which depends on icon-shim which causes all these plugins to not load causing massive headaches....

SEVERE: found cycle in plugin dependencies: (root=Plugin:credentials, deactivating all involved) Plugin:credentials -> Plugin:icon-shim -> Plugin:matrix-auth -> Plugin:cloudbees-folder -> Plugin:credentials

@reviewbybees

@daniel-beck
Copy link
Member

🐝

@ghost
Copy link

ghost commented Feb 23, 2016

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

@abayer
Copy link
Member

abayer commented Feb 23, 2016

🐝

@jtnord
Copy link
Member Author

jtnord commented Feb 23, 2016

@reviewbybees done (classing @daniel-beck's thumbs up as a bee)

@rsandell
Copy link
Member

I don't think this plugin is no longer needed in 1.609

The core dep is old because those old versions don't have a l:icon tag to use while newer core versions has.

@@ -28,7 +28,7 @@ THE SOFTWARE.
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>1.520</version>
<version>1.609</version>

Choose a reason for hiding this comment

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

This would be a good case for the parent pom (plugin-pom). Just specify the jenkins.version as 1.609.

Copy link
Member Author

Choose a reason for hiding this comment

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

this isn't actually a plugin - it's one of the submodules that is a plugin - so would be a bit more work, but...
given the severity of the issue (this causes credentials not to load which causes so much other stuff not to load that you can;t even start Jenkis) I wanted to do as small a change as possible (so it would be as uncontroversial as possible).

Choose a reason for hiding this comment

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

That works for me.

@jtnord
Copy link
Member Author

jtnord commented Feb 23, 2016

@rsandell if it is not needed then it should be replcaed with an empty plugin with no deps that targets 1.609+ @tfennelly?

@jenkinsadmin
Copy link
Member

Thank you for this pull request! Please check this document for how the Jenkins project handles pull requests.

@KostyaSha
Copy link
Member

Maybe plugin manager should break dependency cycle (exclude splitted plugin) in case of such cycles?

@jglick jglick changed the title Fix circular dependency issue [JENKINS-33095] Fix circular dependency issue Feb 23, 2016
@jglick
Copy link
Member

jglick commented Feb 23, 2016

🐝

@jglick
Copy link
Member

jglick commented Feb 23, 2016

Maybe plugin manager should …

See JENKINS-28942, but in the meantime this is an emergency.

@jglick
Copy link
Member

jglick commented Feb 23, 2016

Verified that the change fixes the issue.

jglick added a commit that referenced this pull request Feb 23, 2016
[JENKINS-33095] Fix circular dependency issue
@jglick jglick merged commit 205809b into master Feb 23, 2016
@jglick jglick deleted the jtnord-new-core branch February 23, 2016 23:04
jglick added a commit to jenkinsci/matrix-auth-plugin that referenced this pull request Feb 25, 2016
Also requiring icon-shim 2.0.3 to force the user to pick up jenkinsci/icon-shim-plugin#2 and avoid JENKINS-33095.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants