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

Cyclic dependency in the generated plugin when using testFixtures #175

Closed
roguexz opened this issue Dec 28, 2020 · 4 comments
Closed

Cyclic dependency in the generated plugin when using testFixtures #175

roguexz opened this issue Dec 28, 2020 · 4 comments
Milestone

Comments

@roguexz
Copy link

roguexz commented Dec 28, 2020

Problem

If you have a project that also consumes the java-test-fixtures plugin, then generated plugin manifest has a cyclic dependency to itself.

Test Case

build.gradle

plugins {
    id 'java-test-fixtures'
    id 'org.jenkins-ci.jpi' version '0.40.0'
}

group = 'sample-group'
version = '1.0.0'

jenkinsPlugin {
    coreVersion = '2.263.1'
}

dependencies {
    testImplementation 'org.jenkins-ci.plugins:git:4.0.1'
}

Build & inspect the plugin manifest file.

gradle clean build
cat build/tmp/jpi/MANIFEST.MF
Manifest-Version: 1.0
Group-Id: sample-group
Short-Name: sample-jpi
Long-Name: sample-jpi
Extension-Name: sample-jpi
Plugin-Version: 1.0.0
Jenkins-Version: 2.263.1
Minimum-Java-Version: 11
Plugin-Dependencies: sample-jpi:1.0.0;resolution:=optional
Support-Dynamic-Loading: true

Notice how the main plugin (sample-jpi) is also listed as a dependency.

@sghill sghill closed this as completed in f6abf2c Dec 29, 2020
@sghill sghill added this to the 0.41.0 milestone Dec 29, 2020
@sghill
Copy link

sghill commented Dec 29, 2020

Thanks for reporting this with a test case @roguexz!

The problem is from the way we set up configurations. Basically we had an assumption that any runtime configurations were intended to be optional features. In the case of testFixtures*, that's not true, so now it will be excluded.

This has been published as 0.41.0-rc.1. Feel free to try it out and if you could report back, I'd appreciate it.

@roguexz
Copy link
Author

roguexz commented Dec 29, 2020

Will try it out shortly and confirm

@roguexz
Copy link
Author

roguexz commented Dec 29, 2020

Works like a charm! Thank you for fixing it right away!! 😃

The checkAccessModifier task caught me by surprise. I understand its importance and will make necessary changes, none-the-less it might be worthwhile to consider it as a permissible option that just logs warnings before making it mandatory in the next release. The alternative is to just disable the task temporarily. So, I am good with this RC.

@sghill
Copy link

sghill commented Dec 29, 2020

Thanks for this feedback. I think it makes a lot of sense and will get it into the next rc.

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

No branches or pull requests

2 participants