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

Add default manifest entries with Maven #318

Merged
merged 3 commits into from
Mar 28, 2022

Conversation

basil
Copy link
Member

@basil basil commented Mar 27, 2022

Cleans up a small amount of technical debt introduced in #251 during the transition to Maven 3. Supersedes #284. With this PR, we rely on Maven to add the default specification entries and default implementation entries to the manifest. This allows us to remove some ugly copypasta.

This change is not 100% backward-compatible, but it should be safe for the reasons given in the comments.

@basil basil added the chore label Mar 27, 2022
@basil basil requested review from jglick and Vlatombe March 27, 2022 23:45
Manifest manifest = new Manifest(is)
assert !manifest.getMainAttributes().getValue('Build-Jdk-Spec').isEmpty()
assert manifest.getMainAttributes().getValue('Created-By').startsWith('Maven Archiver')
assert manifest.getMainAttributes().getValue('Extension-Name') == null // was provided by Maven 2, but core prefers Short-Name
Copy link
Member Author

Choose a reason for hiding this comment

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

Maven 2 set this to the short name, but Maven 3 does not. Core always checks Short-Name first and only falls back to Extension-Name if Short-Name is not present. And we always set Short-Name in this plugin. So this was always redundant and can be safely removed.

assert manifest.getMainAttributes().getValue('Extension-Name') == null // was provided by Maven 2, but core prefers Short-Name
assert manifest.getMainAttributes().getValue('Group-Id').equals('org.jenkins-ci.tools.hpi.its')
assert manifest.getMainAttributes().getValue('Hudson-Version').equals('2.249.1')
assert manifest.getMainAttributes().getValue('Implementation-Title').equals('MyNewPlugin') // was project.artifactId in previous versions, now project.name
Copy link
Member Author

Choose a reason for hiding this comment

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

Maven 2 set this to the artifact ID, but Maven 3 sets this to the project name. I couldn't find any code in the ecosystem that relied on this being set to any particular value.

assert manifest.getMainAttributes().getValue('Plugin-ScmUrl').equals('https://github.com/jenkinsci/maven-hpi-plugin')
assert manifest.getMainAttributes().getValue('Plugin-Version').startsWith('1.0-SNAPSHOT')
assert manifest.getMainAttributes().getValue('Short-Name').equals('verify-it')
assert manifest.getMainAttributes().getValue('Specification-Title').equals('MyNewPlugin') // was project.description in previous versions, now project.name
Copy link
Member Author

Choose a reason for hiding this comment

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

Core does consume this to display on the installed plugins page, but only if src/main/resources/index.jelly is not present. As of #302 we enforce the existence of src/main/resources/index.jelly, so changing the semantics of Specification-Title will not result in any change in behavior.

src/it/verify-it/pom.xml Outdated Show resolved Hide resolved
src/it/verify-it/pom.xml Outdated Show resolved Hide resolved
src/it/verify-it/verify.groovy Outdated Show resolved Hide resolved
@jglick
Copy link
Member

jglick commented Mar 28, 2022

I am not sure this is a chore since it does have (minor) effects on behavior.

@basil basil added breaking and removed chore labels Mar 28, 2022
@basil
Copy link
Member Author

basil commented Mar 28, 2022

I am not sure this is a chore since it does have (minor) effects on behavior.

Changed to breaking

@basil basil merged commit c95309a into jenkinsci:master Mar 28, 2022
@basil basil deleted the default-entries branch March 28, 2022 17:00
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.

3 participants