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

…4123)

* InstallPluginCommand.name is unnecessary.

* Minor changes to error reporting requested by @oleg-nenashev.

core/src/main/java/hudson/cli/InstallPluginCommand.java

@@ -25,7 +25,6 @@
 
 import hudson.AbortException;
 import hudson.Extension;
-import hudson.FilePath;
 import hudson.PluginManager;
 import hudson.util.VersionNumber;
 import jenkins.model.Jenkins;
@@ -38,10 +37,14 @@
 import java.io.File;
 import java.net.URL;
 import java.net.MalformedURLException;
+import java.nio.file.Files;
+import java.nio.file.StandardCopyOption;
 import java.util.HashSet;
 import java.util.List;
 import java.util.ArrayList;
 import java.util.Set;
+import java.util.jar.JarFile;
+import java.util.jar.Manifest;
 import org.apache.commons.io.FileUtils;
 
 /**
@@ -52,45 +55,47 @@
  */
 @Extension
 public class InstallPluginCommand extends CLICommand {
+
+    @Override
     public String getShortDescription() {
         return Messages.InstallPluginCommand_ShortDescription();
     }
 
     @Argument(metaVar="SOURCE",required=true,usage=
             "If this is an URL, Jenkins downloads the URL and installs that as a plugin. " +
-            "If it is the string ‘=’, the file will be read from standard input of the command, and ‘-name’ must be specified. " +
+            "If it is the string ‘=’, the file will be read from standard input of the command. " +
             "Otherwise the name is assumed to be the short name of the plugin in the existing update center (like ‘findbugs’), " +
             "and the plugin will be installed from the update center. If the short name includes a minimum version number " +
             "(like ‘findbugs:1.4’), and there are multiple update centers publishing different versions, the update centers " +
             "will be searched in order for the first one publishing a version that is at least the specified version.")
     public List<String> sources = new ArrayList<>();
 
-    @Option(name="-name",usage="If specified, the plugin will be installed as this short name (whereas normally the name is inferred from the source name automatically).")
-    public String name; // TODO better to parse out Short-Name from the manifest and deprecate this option
+    @Deprecated
+    @Option(name = "-name", usage = "No longer used.")
+    public String name;
 
     @Option(name="-restart",usage="Restart Jenkins upon successful installation.")
     public boolean restart;
 
     @Option(name="-deploy",usage="Deploy plugins right away without postponing them until the reboot.")
     public boolean dynamicLoad;
 
+    @Override
     protected int run() throws Exception {
-        Jenkins h = Jenkins.getActiveInstance();
+        Jenkins h = Jenkins.get();
         h.checkPermission(PluginManager.UPLOAD_PLUGINS);
         PluginManager pm = h.getPluginManager();
 
-        if (sources.size() > 1 && name != null) {
-            throw new IllegalArgumentException("-name is incompatible with multiple sources");
+        if (name != null) {
+            stderr.println("-name is deprecated; it is no longer necessary nor honored.");
         }
 
         for (String source : sources) {
             if (source.equals("=")) {
-                if (name == null) {
-                    throw new IllegalArgumentException("-name required when using -source -");
-                }
                 stdout.println(Messages.InstallPluginCommand_InstallingPluginFromStdin());
-                File f = getTargetFile(name);
+                File f = getTmpFile();
                 FileUtils.copyInputStreamToFile(stdin, f);
+                f = moveToFinalLocation(f);
                 if (dynamicLoad) {
                     pm.dynamicLoad(f);
                 }
@@ -101,21 +106,12 @@ protected int run() throws Exception {
             try {
                 URL u = new URL(source);
                 stdout.println(Messages.InstallPluginCommand_InstallingPluginFromUrl(u));
-                String n;
-                if (name != null) {
-                    n = name;
-                } else {
-                    n = u.getPath();
-                    n = n.substring(n.lastIndexOf('/') + 1);
-                    n = n.substring(n.lastIndexOf('\\') + 1);
-                    int idx = n.lastIndexOf('.');
-                    if (idx > 0) {
-                        n = n.substring(0, idx);
-                    }
+                File f = getTmpFile();
+                FileUtils.copyURLToFile(u, f); // TODO JENKINS-58248 proxy
+                f = moveToFinalLocation(f);
+                if (dynamicLoad) {
+                    pm.dynamicLoad(f);
                 }
-                getTargetFilePath(n).copyFrom(u);
-                if (dynamicLoad)
-                    pm.dynamicLoad(getTargetFile(n));
                 continue;
             } catch (MalformedURLException e) {
                 // not an URL
@@ -172,11 +168,24 @@ protected int run() throws Exception {
         return 0; // all success
     }
 
-    private static FilePath getTargetFilePath(String name) {
-        return new FilePath(getTargetFile(name));
+    private static File getTmpFile() throws Exception {
+        return File.createTempFile("download", ".jpi.tmp", Jenkins.get().getPluginManager().rootDir);
     }
 
-    private static File getTargetFile(String name) {
-        return new File(Jenkins.getActiveInstance().getPluginManager().rootDir,name+".jpi");
+    private static File moveToFinalLocation(File tmpFile) throws Exception {
+        String pluginName;
+        try (JarFile jf = new JarFile(tmpFile)) {
+            Manifest mf = jf.getManifest();
+            if (mf == null) {
+                throw new IllegalArgumentException("JAR lacks a manifest");
+            }
+            pluginName = mf.getMainAttributes().getValue("Short-Name");
+        }
+        if (pluginName == null) {
+            throw new IllegalArgumentException("JAR manifest lacks a Short-Name attribute and so does not look like a plugin");
+        }
+        File target = new File(Jenkins.get().getPluginManager().rootDir, pluginName + ".jpi");
+        Files.move(tmpFile.toPath(), target.toPath(), StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE);
+        return target;
     }
 }

test/src/test/java/hudson/cli/InstallPluginCommandTest.java

@@ -41,7 +41,7 @@ public void fromStdin() throws Exception {
         assertNull(r.jenkins.getPluginManager().getPlugin("token-macro"));
         assertThat(new CLICommandInvoker(r, "install-plugin").
                 withStdin(InstallPluginCommandTest.class.getResourceAsStream("/plugins/token-macro.hpi")).
-                invokeWithArgs("-name", "token-macro", "-deploy", "="),
+                invokeWithArgs("-deploy", "="),
             CLICommandInvoker.Matcher.succeeded());
         assertNotNull(r.jenkins.getPluginManager().getPlugin("token-macro"));
     }