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

Stop using the "name" argument in the "install-plugin" CLI command #4123

Merged
merged 3 commits into from
Jul 27, 2019

Conversation

jglick
Copy link
Member

@jglick jglick commented Jul 19, 2019

Cleans up a longstanding UX regression from #2795: there is no good reason to require the -name option to the install-plugin CLI command. At best it is a nuisance, and at worst it could mean data corruption from typos. So instead of

java -jar …/jenkins-cli.jar -s http://jenkins/ install-plugin -name some-thing = < target/some-thing.hpi

you can just write

java -jar …/jenkins-cli.jar -s http://jenkins/ install-plugin = < target/some-thing.hpi

since the manifest header

Short-Name: some-thing

already conveys this information.

Ditto for URL-based installation; the former code tried to parse out the short name from the URL path, which was pretty fragile. Now you can just

java -jar …/jenkins-cli.jar -s http://jenkins/ install-plugin http://download-server/this-naming-is-irrelevant.hpi

Also the download is now to a temporary file followed by an atomic move, which should reduce the chance of corrupted files breaking the installation.

Changelog entry:

  • The -name option to the install-plugin command is no longer necessary nor honored.

@jglick jglick mentioned this pull request Jul 19, 2019
26 tasks
@daniel-beck
Copy link
Member

As a side effect, this will take care of JENKINS-29065 in that "installation" of an invalid file won't work anymore, possibly breaking Jenkins.

@jglick
Copy link
Member Author

jglick commented Jul 19, 2019

"installation" of an invalid file won't work anymore, possibly breaking Jenkins

Confirmed.

Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

Looks good to me

@jglick
Copy link
Member Author

jglick commented Jul 24, 2019

Test flake:

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:86)
	at org.junit.Assert.assertTrue(Assert.java:41)
	at org.junit.Assert.assertTrue(Assert.java:52)
	at hudson.util.RetrierTest.sleepWorksTest(RetrierTest.java:100)

@jglick jglick added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Jul 24, 2019
Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

Please add checks for Runtime exceptions when reading the manifest (or just for the entire try/catch block)

core/src/main/java/hudson/cli/InstallPluginCommand.java Outdated Show resolved Hide resolved
core/src/main/java/hudson/cli/InstallPluginCommand.java Outdated Show resolved Hide resolved
@oleg-nenashev oleg-nenashev removed the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Jul 26, 2019
@jglick jglick added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Jul 26, 2019
@oleg-nenashev oleg-nenashev changed the title InstallPluginCommand.name is unnecessary Stop using the "name" argument in the "install-plugin" CLI command Jul 27, 2019
@oleg-nenashev
Copy link
Member

It is rather a bug than a feature, but renaming PR retriggers the build more reliably than close/open

@oleg-nenashev oleg-nenashev merged commit aee0b50 into jenkinsci:master Jul 27, 2019
@jglick jglick deleted the InstallPluginCommand.name branch July 29, 2019 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants