diff --git a/changelog.html b/changelog.html
index 34ab87dca1518ac3e978f2b2972ff797deff58c4..f0c9c5c3f25ac08b61e2dc25094ecc572ade000a 100644
--- a/changelog.html
+++ b/changelog.html
@@ -55,6 +55,9 @@ Upcoming changes
+ -
+ Misleading error message trying to dynamically update a plugin (which is impossible) on an NFS filesystem.
+ (issue 12753)
-
Updated component used by the swap space monitor to better support Debian and AIX.
-
diff --git a/core/src/main/java/hudson/ClassicPluginStrategy.java b/core/src/main/java/hudson/ClassicPluginStrategy.java
index 261a92892abf67ffc9d07dca50c6f34e79b7b895..10a52532c9f0a5e21a1c80c63433fafa15ae92b8 100644
--- a/core/src/main/java/hudson/ClassicPluginStrategy.java
+++ b/core/src/main/java/hudson/ClassicPluginStrategy.java
@@ -66,6 +66,7 @@ import java.util.HashSet;
import java.util.List;
import java.util.Vector;
import java.util.jar.Attributes;
+import java.util.jar.JarFile;
import java.util.jar.Manifest;
import java.util.logging.Level;
import java.util.logging.Logger;
@@ -92,15 +93,26 @@ public class ClassicPluginStrategy implements PluginStrategy {
this.pluginManager = pluginManager;
}
- public PluginWrapper createPluginWrapper(File archive) throws IOException {
- final Manifest manifest;
- URL baseResourceURL;
+ @Override public String getShortName(File archive) throws IOException {
+ Manifest manifest;
+ if (isLinked(archive)) {
+ manifest = loadLinkedManifest(archive);
+ } else {
+ JarFile jf = new JarFile(archive, false);
+ try {
+ manifest = jf.getManifest();
+ } finally {
+ jf.close();
+ }
+ }
+ return PluginWrapper.computeShortName(manifest, archive);
+ }
- File expandDir = null;
- // if .hpi, this is the directory where war is expanded
+ private static boolean isLinked(File archive) {
+ return archive.getName().endsWith(".hpl") || archive.getName().endsWith(".jpl");
+ }
- boolean isLinked = archive.getName().endsWith(".hpl") || archive.getName().endsWith(".jpl");
- if (isLinked) {
+ private static Manifest loadLinkedManifest(File archive) throws IOException {
// resolve the .hpl file to the location of the manifest file
final String firstLine = IOUtils.readFirstLine(new FileInputStream(archive), "UTF-8");
if (firstLine.startsWith("Manifest-Version:")) {
@@ -112,12 +124,24 @@ public class ClassicPluginStrategy implements PluginStrategy {
// then parse manifest
FileInputStream in = new FileInputStream(archive);
try {
- manifest = new Manifest(in);
+ return new Manifest(in);
} catch (IOException e) {
throw new IOException("Failed to load " + archive, e);
} finally {
in.close();
}
+ }
+
+ @Override public PluginWrapper createPluginWrapper(File archive) throws IOException {
+ final Manifest manifest;
+ URL baseResourceURL;
+
+ File expandDir = null;
+ // if .hpi, this is the directory where war is expanded
+
+ boolean isLinked = isLinked(archive);
+ if (isLinked) {
+ manifest = loadLinkedManifest(archive);
} else {
if (archive.isDirectory()) {// already expanded
expandDir = archive;
diff --git a/core/src/main/java/hudson/PluginManager.java b/core/src/main/java/hudson/PluginManager.java
index e1342b4ce14eb736dcd3c2802c1320656f264345..7d9e73c7775227195a9deee9e913bf5af7be2c91 100644
--- a/core/src/main/java/hudson/PluginManager.java
+++ b/core/src/main/java/hudson/PluginManager.java
@@ -412,11 +412,21 @@ public abstract class PluginManager extends AbstractModelObject implements OnMas
*/
public void dynamicLoad(File arc) throws IOException, InterruptedException, RestartRequiredException {
LOGGER.info("Attempting to dynamic load "+arc);
- final PluginWrapper p = strategy.createPluginWrapper(arc);
- String sn = p.getShortName();
+ PluginWrapper p = null;
+ String sn;
+ try {
+ sn = strategy.getShortName(arc);
+ } catch (AbstractMethodError x) {
+ LOGGER.log(WARNING, "JENKINS-12753 fix not active: {0}", x.getMessage());
+ p = strategy.createPluginWrapper(arc);
+ sn = p.getShortName();
+ }
if (getPlugin(sn)!=null)
throw new RestartRequiredException(Messages._PluginManager_PluginIsAlreadyInstalled_RestartRequired(sn));
+ if (p == null) {
+ p = strategy.createPluginWrapper(arc);
+ }
if (p.supportsDynamicLoad()== YesNoMaybe.NO)
throw new RestartRequiredException(Messages._PluginManager_PluginDoesntSupportDynamicLoad_RestartRequired(sn));
@@ -442,10 +452,11 @@ public abstract class PluginManager extends AbstractModelObject implements OnMas
// run initializers in the added plugin
Reactor r = new Reactor(InitMilestone.ordering());
- r.addAll(new InitializerFinder(p.classLoader) {
+ final ClassLoader loader = p.classLoader;
+ r.addAll(new InitializerFinder(loader) {
@Override
protected boolean filter(Method e) {
- return e.getDeclaringClass().getClassLoader()!=p.classLoader || super.filter(e);
+ return e.getDeclaringClass().getClassLoader() != loader || super.filter(e);
}
}.discoverTasks(r));
try {
diff --git a/core/src/main/java/hudson/PluginStrategy.java b/core/src/main/java/hudson/PluginStrategy.java
index e599812e4ab449b4ede1582779852e8d3cec1ec4..2acb1f706555ac5fed9f4ed2d2c0bd2c4ffef26a 100644
--- a/core/src/main/java/hudson/PluginStrategy.java
+++ b/core/src/main/java/hudson/PluginStrategy.java
@@ -24,11 +24,10 @@
package hudson;
import hudson.model.Hudson;
-import jenkins.model.Jenkins;
-
import java.io.File;
import java.io.IOException;
import java.util.List;
+import javax.annotation.Nonnull;
/**
* Pluggability point for how to create {@link PluginWrapper}.
@@ -50,6 +49,13 @@ public interface PluginStrategy extends ExtensionPoint {
PluginWrapper createPluginWrapper(File archive)
throws IOException;
+ /**
+ * Finds the plugin name without actually unpacking anything {@link #createPluginWrapper} would.
+ * Needed by {@link PluginManager#dynamicLoad} to decide whether such a plugin is already installed.
+ * @return the {@link PluginWrapper#getShortName}
+ */
+ @Nonnull String getShortName(File archive) throws IOException;
+
/**
* Loads the plugin and starts it.
*
diff --git a/core/src/main/java/hudson/PluginWrapper.java b/core/src/main/java/hudson/PluginWrapper.java
index 870930f7def60c0202d461eb4b42065b8a3de71a..b7364499a4b6ac4717b68b7b3870a81864020177 100644
--- a/core/src/main/java/hudson/PluginWrapper.java
+++ b/core/src/main/java/hudson/PluginWrapper.java
@@ -238,7 +238,7 @@ public class PluginWrapper implements Comparable, ModelObject {
return idx != null && idx.toString().contains(shortName) ? idx : null;
}
- private String computeShortName(Manifest manifest, File archive) {
+ static String computeShortName(Manifest manifest, File archive) {
// use the name captured in the manifest, as often plugins
// depend on the specific short name in its URLs.
String n = manifest.getMainAttributes().getValue("Short-Name");
diff --git a/test/src/test/java/hudson/PluginManagerTest.java b/test/src/test/java/hudson/PluginManagerTest.java
index a54f55e2c67eaa76057bbb28fdb42d7b89864061..ad7b6bd9fd943e97d48e76d1776092be191e284a 100644
--- a/test/src/test/java/hudson/PluginManagerTest.java
+++ b/test/src/test/java/hudson/PluginManagerTest.java
@@ -40,6 +40,7 @@ import java.net.URLClassLoader;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.Future;
+import jenkins.RestartRequiredException;
import org.apache.commons.io.FileUtils;
import org.apache.tools.ant.filters.StringInputStream;
import static org.junit.Assert.*;
@@ -356,4 +357,22 @@ public class PluginManagerTest {
assertTrue(r.jenkins.getExtensionList("org.jenkinsci.plugins.dependencytest.dependee.DependeeExtensionPoint").isEmpty());
}
+ @Bug(12753)
+ @WithPlugin("tasks.jpi")
+ @Test public void dynamicLoadRestartRequiredException() throws Exception {
+ File jpi = new File(r.jenkins.getRootDir(), "plugins/tasks.jpi");
+ assertTrue(jpi.isFile());
+ FileUtils.touch(jpi);
+ File timestamp = new File(r.jenkins.getRootDir(), "plugins/tasks/.timestamp2");
+ assertTrue(timestamp.isFile());
+ long lastMod = timestamp.lastModified();
+ try {
+ r.jenkins.getPluginManager().dynamicLoad(jpi);
+ fail("should not have worked");
+ } catch (RestartRequiredException x) {
+ // good
+ }
+ assertEquals("should not have tried to delete & unpack", lastMod, timestamp.lastModified());
+ }
+
}