Skip to content
Permalink
Browse files

Merge pull request #2236 from daniel-beck/JENKINS-34073

[JENKINS-34073] Revert fix for JENKINS-21486
  • Loading branch information...
daniel-beck committed Apr 6, 2016
2 parents 25340e0 + 70ee201 commit 1e338d3565c8c58d2556f344a7c34a3b3ab18ca3
@@ -56,10 +56,7 @@
<!-- Record your changes in the trunk here. -->
<div id="trunk" style="display:none"><!--=TRUNK-BEGIN=-->
<ul class=image>
<li class="major RFE">
Plugin dependencies are now strictly enforced during Jenkins startup, to avoid cryptic errors later.
You may use <code>-Dhudson.PluginWrapper.dependenciesVersionCheck.enabled=false</code> to revert to the old behavior.
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-21486">issue 21486</a>)
<li class=>
</ul>
</div><!--=TRUNK-END=-->
<h3><a name=v1.656>What's new in 1.656</a> (2016/04/03)</h3>
@@ -679,8 +679,7 @@ public void dynamicLoad(File arc) throws IOException, InterruptedException, Rest
// so existing plugins can't be depending on this newly deployed one.

plugins.add(p);
if (p.isActive())
activePlugins.add(p);
activePlugins.add(p);
synchronized (((UberClassLoader) uberClassLoader).loaded) {
((UberClassLoader) uberClassLoader).loaded.clear();
}
@@ -87,12 +87,6 @@
*/
@ExportedBean
public class PluginWrapper implements Comparable<PluginWrapper>, ModelObject {
/**
* A plugin won't be loaded unless his declared dependencies are present and match the required minimal version.
* This can be set to false to disable the version check (legacy behaviour)
*/
private static final boolean ENABLE_PLUGIN_DEPENDENCIES_VERSION_CHECK = Boolean.parseBoolean(System.getProperty(PluginWrapper.class.getName()+"." + "dependenciesVersionCheck.enabled", "true"));

/**
* {@link PluginManager} to which this belongs to.
*/
@@ -214,27 +208,22 @@ public Dependency(String s) {
if(idx==-1)
throw new IllegalArgumentException("Illegal dependency specifier "+s);
this.shortName = s.substring(0,idx);
String version = s.substring(idx+1);

this.version = s.substring(idx+1);
boolean isOptional = false;
String[] osgiProperties = version.split("[;]");
String[] osgiProperties = s.split(";");
for (int i = 1; i < osgiProperties.length; i++) {
String osgiProperty = osgiProperties[i].trim();
if (osgiProperty.equalsIgnoreCase("resolution:=optional")) {
isOptional = true;
}
}
this.optional = isOptional;
if (isOptional) {
this.version = osgiProperties[0];
} else {
this.version = version;
}
}

@Override
public String toString() {
return shortName + " (" + version + ") " + (optional ? "optional" : "");
return shortName + " (" + version + ")";
}
}

@@ -398,21 +387,6 @@ private String getVersionOf(Manifest manifest) {
return "???";
}

/**
* Returns the required Jenkins core version of this plugin.
* @return the required Jenkins core version of this plugin.
* @since 1.657
*/
@Exported
public @CheckForNull String getRequiredCoreVersion() {
String v = manifest.getMainAttributes().getValue("Jenkins-Version");
if (v!= null) return v;

v = manifest.getMainAttributes().getValue("Hudson-Version");
if (v!= null) return v;
return null;
}

/**
* Returns the version number of this plugin
*/
@@ -539,71 +513,19 @@ public boolean hasLicensesXml() {
* thrown if one or several mandatory dependencies doesn't exists.
*/
/*package*/ void resolvePluginDependencies() throws IOException {
if (ENABLE_PLUGIN_DEPENDENCIES_VERSION_CHECK) {
String requiredCoreVersion = getRequiredCoreVersion();
if (requiredCoreVersion == null) {
LOGGER.warning(shortName + " doesn't declare required core version.");
} else {
if (Jenkins.getVersion().isOlderThan(new VersionNumber(requiredCoreVersion))) {
throw new IOException(shortName + " requires a more recent core version (" + requiredCoreVersion + ") than the current (" + Jenkins.getVersion() + ").");
}
}
}
List<String> missingDependencies = new ArrayList<String>();
List<String> obsoleteDependencies = new ArrayList<String>();
List<String> disabledDependencies = new ArrayList<String>();
// make sure dependencies exist
for (Dependency d : dependencies) {
PluginWrapper dependency = parent.getPlugin(d.shortName);
if (dependency == null) {
if (parent.getPlugin(d.shortName) == null)
missingDependencies.add(d.toString());
} else {
if (dependency.isActive()) {
if (ENABLE_PLUGIN_DEPENDENCIES_VERSION_CHECK && dependency.getVersionNumber().isOlderThan(new VersionNumber(d.version))) {
obsoleteDependencies.add(dependency.getShortName() + "(" + dependency.getVersion() + " < " + d.version + ")");
}
} else {
disabledDependencies.add(d.toString());
}

}
}
if (!missingDependencies.isEmpty())
throw new IOException("Dependency "+Util.join(missingDependencies, ", ")+" doesn't exist");

// add the optional dependencies that exists
for (Dependency d : optionalDependencies) {
PluginWrapper dependency = parent.getPlugin(d.shortName);
if (dependency != null && dependency.isActive()) {
if (ENABLE_PLUGIN_DEPENDENCIES_VERSION_CHECK && dependency.getVersionNumber().isOlderThan(new VersionNumber(d.version))) {
obsoleteDependencies.add(dependency.getShortName() + "(" + dependency.getVersion() + " < " + d.version + ")");
} else {
dependencies.add(d);
}
}
}
StringBuilder messageBuilder = new StringBuilder();
if (!missingDependencies.isEmpty()) {
boolean plural = missingDependencies.size() > 1;
messageBuilder.append(plural ? "Dependencies " : "Dependency ")
.append(Util.join(missingDependencies, ", "))
.append(" ").append(plural ? "don't" : "doesn't")
.append(" exist. ");
}
if (!disabledDependencies.isEmpty()) {
boolean plural = disabledDependencies.size() > 1;
messageBuilder.append(plural ? "Dependencies " : "Dependency ")
.append(Util.join(disabledDependencies, ", "))
.append(" ").append(plural ? "are" : "is")
.append(" disabled. ");
}
if (!obsoleteDependencies.isEmpty()) {
boolean plural = obsoleteDependencies.size() > 1;
messageBuilder.append(plural ? "Dependencies " : "Dependency ")
.append(Util.join(obsoleteDependencies, ", "))
.append(" ").append(plural ? "are" : "is")
.append(" older than required.");
}
String message = messageBuilder.toString();
if (!message.isEmpty()) {
throw new IOException(message);
if (parent.getPlugin(d.shortName) != null)
dependencies.add(d);
}
}

@@ -47,7 +47,6 @@
import org.apache.commons.io.FileUtils;
import org.apache.tools.ant.filters.StringInputStream;
import static org.junit.Assert.*;

import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
@@ -340,75 +339,6 @@ private String callDependerValue() throws Exception {
assertTrue(r.jenkins.getExtensionList("org.jenkinsci.plugins.dependencytest.dependee.DependeeExtensionPoint").isEmpty());
}

@Issue("JENKINS-21486")
@Test public void installPluginWithObsoleteDependencyFails() throws Exception {
// Load dependee 0.0.1.
{
dynamicLoad("dependee.hpi");
}

// Load mandatory-depender 0.0.2, depending on dependee 0.0.2
try {
dynamicLoad("mandatory-depender-0.0.2.hpi");
fail("Should not have worked");
} catch (IOException e) {
// Expected
}
}

@Issue("JENKINS-21486")
@Test public void installPluginWithDisabledOptionalDependencySucceeds() throws Exception {
// Load dependee 0.0.2.
{
dynamicLoadAndDisable("dependee-0.0.2.hpi");
}

// Load depender 0.0.2, depending optionally on dependee 0.0.2
{
dynamicLoad("depender-0.0.2.hpi");
}

// dependee is not loaded so we cannot list any extension for it.
try {
r.jenkins.getExtensionList("org.jenkinsci.plugins.dependencytest.dependee.DependeeExtensionPoint");
fail();
} catch( ClassNotFoundException _ ){
}
}

@Issue("JENKINS-21486")
@Test public void installPluginWithDisabledDependencyFails() throws Exception {
// Load dependee 0.0.2.
{
dynamicLoadAndDisable("dependee-0.0.2.hpi");
}

// Load mandatory-depender 0.0.2, depending on dependee 0.0.2
try {
dynamicLoad("mandatory-depender-0.0.2.hpi");
fail("Should not have worked");
} catch (IOException e) {
// Expected
}
}


@Issue("JENKINS-21486")
@Test public void installPluginWithObsoleteOptionalDependencyFails() throws Exception {
// Load dependee 0.0.1.
{
dynamicLoad("dependee.hpi");
}

// Load depender 0.0.2, depending optionally on dependee 0.0.2
try {
dynamicLoad("depender-0.0.2.hpi");
fail("Should not have worked");
} catch (IOException e) {
// Expected
}
}

@Issue("JENKINS-12753")
@WithPlugin("tasks.jpi")
@Test public void dynamicLoadRestartRequiredException() throws Exception {
@@ -447,8 +377,4 @@ private String callDependerValue() throws Exception {
private void dynamicLoad(String plugin) throws IOException, InterruptedException, RestartRequiredException {
PluginManagerUtil.dynamicLoad(plugin, r.jenkins);
}

private void dynamicLoadAndDisable(String plugin) throws IOException, InterruptedException, RestartRequiredException {
PluginManagerUtil.dynamicLoad(plugin, r.jenkins, true);
}
}
@@ -48,16 +48,9 @@ public void before() throws Throwable {
}

public static void dynamicLoad(String plugin, Jenkins jenkins) throws IOException, InterruptedException, RestartRequiredException {
dynamicLoad(plugin, jenkins, false);
}

public static void dynamicLoad(String plugin, Jenkins jenkins, boolean disable) throws IOException, InterruptedException, RestartRequiredException {
URL src = PluginManagerTest.class.getClassLoader().getResource("plugins/" + plugin);
File dest = new File(jenkins.getRootDir(), "plugins/" + plugin);
FileUtils.copyURLToFile(src, dest);
if (disable) {
new File(dest.getPath() + ".disabled").createNewFile();
}
jenkins.pluginManager.dynamicLoad(dest);
}
}

This file was deleted.

Binary file not shown.
Binary file not shown.
Binary file not shown.

0 comments on commit 1e338d3

Please sign in to comment.
You can’t perform that action at this time.