Navigation Menu

Skip to content

Commit

Permalink
Merge pull request #2278 from daniel-beck/1.x-JENKINS-34073
Browse files Browse the repository at this point in the history 
Revert "[FIXED JENKINS-21486] Merged #2172: enforce plugin dependenci…
  • Loading branch information
@kohsuke
kohsuke committed Apr 20, 2016
2 parents c51c7dc + e0828dd commit b396ded
Show file tree
Hide file tree
Showing 9 changed files with 13 additions and 202 deletions.
7 changes: 2 additions & 5 deletions changelog.html
View file
Expand Up @@ -59,12 +59,9 @@
<li class=>
</ul>
</div><!--=TRUNK-END=-->
<h3><a name=v1.657>What's new in 1.657</a> (2016/04/19)</h3>
<h3><a name=v1.657>What's new in 1.657</a> (2016/04/20)</h3>
<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>
<h3><a name=v1.656>What's new in 1.656</a> (2016/04/03)</h3>
<ul class=image>
Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/hudson/PluginManager.java
View file
Expand Up @@ -489,8 +489,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();
}
Expand Down
98 changes: 10 additions & 88 deletions core/src/main/java/hudson/PluginWrapper.java
View file
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -223,27 +217,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 + ")";
}
}

Expand Down Expand Up @@ -408,21 +397,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
*/
Expand Down Expand Up @@ -549,71 +523,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);
}
}

Expand Down
74 changes: 0 additions & 74 deletions test/src/test/java/hudson/PluginManagerTest.java
View file
Expand Up @@ -45,7 +45,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;
Expand Down Expand Up @@ -338,75 +337,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 {
Expand All @@ -428,8 +358,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);
}
}
7 changes: 0 additions & 7 deletions test/src/test/java/hudson/PluginManagerUtil.java
View file
Expand Up @@ -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);
}
}
26 changes: 0 additions & 26 deletions test/src/test/java/hudson/PluginWrapperTest.java
View file

This file was deleted.

Binary file removed test/src/test/resources/plugins/dependee-0.0.2.hpi
View file
Binary file not shown.
Binary file removed test/src/test/resources/plugins/depender-0.0.2.hpi
View file
Binary file not shown.
Binary file removed test/src/test/resources/plugins/mandatory-depender-0.0.2.hpi
View file
Binary file not shown.

0 comments on commit b396ded

Please sign in to comment.