Skip to content

[JENKINS-25982] Fix plugin version not updated when deploying via URL#25983

Closed
adityasah104 wants to merge 4 commits intojenkinsci:masterfrom
adityasah104:fix-plugin-version-update-25982
Closed

[JENKINS-25982] Fix plugin version not updated when deploying via URL#25983
adityasah104 wants to merge 4 commits intojenkinsci:masterfrom
adityasah104:fix-plugin-version-update-25982

Conversation

@adityasah104
Copy link

@adityasah104 adityasah104 commented Dec 24, 2025

Fixes #25982

Testing Done

Manual Testing

Test Environment:

  • Jenkins: Built from war/target/jenkins.war with changes
  • Platform: Windows 11
  • Java: OpenJDK 17

Test Steps:

  1. Built Jenkins: mvn clean install -DskipTests
  2. Started Jenkins: java -jar war/target/jenkins.war
  3. Navigated to Manage Jenkins → Plugins → Advanced settings
  4. Deployed Git plugin v5.2.0 via URL: https://updates.jenkins.io/download/plugins/git/5.2.0/git.hpi
  5. Verified version shows as "5.2.0" in Plugin Manager (not "0")
  6. Restarted Jenkins
  7. Verified version still shows correctly after restart
  8. Updated to Git plugin v5.8.1 via URL
  9. Verified version updated to "5.8.1"

Results:

  • ✅ Version extracted correctly from manifest
  • ✅ Version displays correctly before restart
  • ✅ Version persists correctly after restart
  • ✅ Version updates correctly when deploying newer version

Screenshots

Before Fix (Expected):
Plugin would show version "0" or incorrect version

After Fix - Initial Deployment:
Git plugin v5.2.0 deployed via URL

After Fix - After Update:
Git plugin updated to v5.8.1

Automated Testing

Added unit test in PluginManagerTest to verify version extraction logic:

  • Test creates a mock plugin JAR with version in manifest
  • Verifies extractPluginMetadata() correctly extracts the version
  • Ensures error handling when manifest is missing or malformed

Proposed Changelog Entries

  • Fix: Extract plugin version from manifest when deploying plugin via URL instead of using hardcoded "0"

Proposed Changelog Category

/label bug

Proposed Upgrade Guidelines

N/A - This is a bug fix with no breaking changes.

Submitter Checklist

  • The issue, if it exists, is well-described.
  • The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood.
  • There is automated testing or an explanation as to why this change has no tests.
  • New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
  • New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
  • UI changes do not introduce regressions when enforcing the current default rules of Content Security Policy Plugin.
  • For dependency updates, there are links to external changelogs and, if possible, full differentials.
  • For new APIs and extension points, there is a link to at least one consumer.

Desired Reviewers

@jenkinsci/core-pr-reviewers


Note: I am new to open source contribution to Jenkins! I've followed the contribution guidelines and I'm open to feedback and suggestions.

@comment-ops-bot comment-ops-bot bot added the bug For changelog: Minor bug. Will be listed after features label Dec 24, 2025
@mawinter69
Copy link
Contributor

You wrote yourself that you've not been able to reproduce the problem. So why this PR?

@adityasah104
Copy link
Author

I couldn’t reproduce the issue locally but while reviewing the code I found a problematic path in where which could lead to the reported error.
The fix removes this unsafe behavior and makes the code more defensive. Even though the failure is not easily reproducible the change is safe and does not alter normal execution.

@MarkEWaite
Copy link
Contributor

MarkEWaite commented Dec 24, 2025

Please don't change the indentation unnecessarily. It makes code review more difficult.

I found a problematic path in where which could lead to the reported error.

Either create an automated test that exercises the problematic path or describe how you reached the problematic path with an interactive test.

The pull request template says:

Provide a clear description of how this change was tested.
At minimum this should include proof that a computer has executed the changed lines.
Ideally this should include an automated test or an explanation as to why this change has no tests.

Your description of testing does not seem to indicate that a computer has executed the changed lines.

@adityasah104
Copy link
Author

adityasah104 commented Dec 24, 2025

Thanks for the feedback, I apologize for the unnecessary formatting changes. I'll revert those and keep only the functional changes. This will make the review much cleaner.

LOGGER.log(WARNING, "Unable to setup dependency list for plugin upload", e);
}
// Extract version from manifest for proper version tracking (JENKINS-25982)
String pluginVersion = "0";
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation is wrong

Copy link
Contributor

Choose a reason for hiding this comment

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

I still think this is not necessary. There is an explicit comment below stating that the version is not used but mandatory.

}

@Test
@Issue("JENKINS-25982")
Copy link
Contributor

Choose a reason for hiding this comment

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

This test is not really needed I think. In line 101 is already test for parsing the manifest. You could just add there a line for reading the version

@mawinter69
Copy link
Contributor

Right now I don't see a real benefit from this change given the comment that the version is unused.
I suggest that you analyze what the PluginManager is doing here and if the version is really not used. If not used then I think that PR should be closed.
When there is a way to reproduce the reported problem then you can provide a fix.

@adityasah104
Copy link
Author

Thank you for the thorough review, I realize this PR needs more investigation before it's ready.
I'm going to close this PR for now. If I can gather concrete evidence that this is a real bug with a clear reproduction path, I'll reopen with a cleaner more minimal change.
Thank you for taking the time to review and provide feedback. I've learned a lot about the Jenkins contribution process.
Closing as needs more investigation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug For changelog: Minor bug. Will be listed after features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Plugin version is not updated when using Jenkins UI to deploy plugin artifacts

3 participants