Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #2122 from kzantow/2.0
[JENKINS-33296] - plugin dependency issues during install
  • Loading branch information
daniel-beck committed Mar 16, 2016
2 parents f481741 + e7ad694 commit 449719c
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 55 deletions.
58 changes: 37 additions & 21 deletions core/src/main/java/hudson/ClassicPluginStrategy.java
Expand Up @@ -241,8 +241,8 @@ private static Manifest loadLinkedManifest(File archive) throws IOException {
}
}
}
for (DetachedPlugin detached : DETACHED_LIST)
detached.fix(atts,optionalDependencies);

fix(atts,optionalDependencies);

// Register global classpath mask. This is useful for hiding JavaEE APIs that you might see from the container,
// such as database plugin for JPA support. The Mask-Classes attribute is insufficient because those classes
Expand All @@ -260,6 +260,41 @@ private static Manifest loadLinkedManifest(File archive) throws IOException {
createClassLoader(paths, dependencyLoader, atts), disableFile, dependencies, optionalDependencies);
}

private static void fix(Attributes atts, List<PluginWrapper.Dependency> optionalDependencies) {
String pluginName = atts.getValue("Short-Name");

String jenkinsVersion = atts.getValue("Jenkins-Version");
if (jenkinsVersion==null)
jenkinsVersion = atts.getValue("Hudson-Version");

optionalDependencies.addAll(getImpliedDependencies(pluginName, jenkinsVersion));
}

/**
* Returns all the plugin dependencies that are implicit based on a particular Jenkins version
* @since 2.0
*/
@Nonnull
public static List<PluginWrapper.Dependency> getImpliedDependencies(String pluginName, String jenkinsVersion) {
List<PluginWrapper.Dependency> out = new ArrayList<>();
for (DetachedPlugin detached : DETACHED_LIST) {
// don't fix the dependency for itself, or else we'll have a cycle
if (detached.shortName.equals(pluginName)) {
continue;
}
if (BREAK_CYCLES.contains(pluginName + '/' + detached.shortName)) {
LOGGER.log(Level.FINE, "skipping implicit dependency {0} → {1}", new Object[] {pluginName, detached.shortName});
continue;
}
// some earlier versions of maven-hpi-plugin apparently puts "null" as a literal in Hudson-Version. watch out for them.
if (jenkinsVersion == null || jenkinsVersion.equals("null") || new VersionNumber(jenkinsVersion).compareTo(detached.splitWhen) <= 0) {
out.add(new PluginWrapper.Dependency(detached.shortName + ':' + detached.requireVersion));
LOGGER.log(Level.FINE, "adding implicit dependency {0} → {1} because of {2}", new Object[] {pluginName, detached.shortName, jenkinsVersion});
}
}
return out;
}

@Deprecated
protected ClassLoader createClassLoader(List<File> paths, ClassLoader parent) throws IOException {
return createClassLoader( paths, parent, null );
Expand Down Expand Up @@ -377,25 +412,6 @@ public String getShortName() {
public VersionNumber getSplitWhen() {
return splitWhen;
}

private void fix(Attributes atts, List<PluginWrapper.Dependency> optionalDependencies) {
// don't fix the dependency for yourself, or else we'll have a cycle
String yourName = atts.getValue("Short-Name");
if (shortName.equals(yourName)) return;
if (BREAK_CYCLES.contains(yourName + '/' + shortName)) {
LOGGER.log(Level.FINE, "skipping implicit dependency {0} → {1}", new Object[] {yourName, shortName});
return;
}

// some earlier versions of maven-hpi-plugin apparently puts "null" as a literal in Hudson-Version. watch out for them.
String jenkinsVersion = atts.getValue("Jenkins-Version");
if (jenkinsVersion==null)
jenkinsVersion = atts.getValue("Hudson-Version");
if (jenkinsVersion == null || jenkinsVersion.equals("null") || new VersionNumber(jenkinsVersion).compareTo(splitWhen) <= 0) {
optionalDependencies.add(new PluginWrapper.Dependency(shortName + ':' + requireVersion));
LOGGER.log(Level.FINE, "adding implicit dependency {0} → {1} because of {2}", new Object[] {yourName, shortName, jenkinsVersion});
}
}
}

private static final List<DetachedPlugin> DETACHED_LIST = Collections.unmodifiableList(Arrays.asList(
Expand Down
5 changes: 4 additions & 1 deletion core/src/main/java/hudson/PluginManager.java
Expand Up @@ -1140,7 +1140,9 @@ private List<Future<UpdateCenter.UpdateCenterJob>> install(@Nonnull Collection<S
String pluginName = n.substring(0, index);
String siteName = n.substring(index + 1);
UpdateSite.Plugin plugin = getPlugin(pluginName, siteName);
// TODO: Someone that understands what the following logic is about, please add a comment.
// There could be cases like:
// 'plugin.ambiguous.updatesite' where both
// 'plugin' @ 'ambigiuous.updatesite' and 'plugin.ambiguous' @ 'updatesite' resolve to valid plugins
if (plugin != null) {
if (p != null) {
throw new Failure("Ambiguous plugin: " + n);
Expand All @@ -1150,6 +1152,7 @@ private List<Future<UpdateCenter.UpdateCenterJob>> install(@Nonnull Collection<S
index = n.indexOf('.', index + 1);
}
}

if (p == null) {
throw new Failure("No such plugin: " + n);
}
Expand Down
21 changes: 15 additions & 6 deletions core/src/main/java/hudson/model/UpdateSite.java
Expand Up @@ -25,6 +25,7 @@

package hudson.model;

import hudson.ClassicPluginStrategy;
import hudson.PluginManager;
import hudson.PluginWrapper;
import hudson.Util;
Expand Down Expand Up @@ -467,7 +468,17 @@ public final class Data {
core = null;
}
for(Map.Entry<String,JSONObject> e : (Set<Map.Entry<String,JSONObject>>)o.getJSONObject("plugins").entrySet()) {
plugins.put(e.getKey(),new Plugin(sourceId, e.getValue()));
Plugin p = new Plugin(sourceId, e.getValue());
// JENKINS-33308 - include implied dependencies for older plugins that may need them
List<PluginWrapper.Dependency> implicitDeps = ClassicPluginStrategy.getImpliedDependencies(p.name, p.requiredCore);
if(!implicitDeps.isEmpty()) {
for(PluginWrapper.Dependency dep : implicitDeps) {
if(!p.dependencies.containsKey(dep.shortName)) {
p.dependencies.put(dep.shortName, dep.version);
}
}
}
plugins.put(e.getKey(), p);
}

connectionCheckUrl = (String)o.get("connectionCheckUrl");
Expand Down Expand Up @@ -612,7 +623,7 @@ public final class Plugin extends Entry {
public final String[] categories;

/**
* Dependencies of this plugin.
* Dependencies of this plugin, a name -&gt; version mapping.
*/
@Exported
public final Map<String,String> dependencies = new HashMap<String,String>();
Expand All @@ -634,10 +645,8 @@ public Plugin(String sourceId, JSONObject o) {
this.categories = o.has("labels") ? (String[])o.getJSONArray("labels").toArray(new String[0]) : null;
for(Object jo : o.getJSONArray("dependencies")) {
JSONObject depObj = (JSONObject) jo;
// Make sure there's a name attribute, that that name isn't maven-plugin - we ignore that one -
// and that the optional value isn't true.
if (get(depObj,"name")!=null
&& !get(depObj,"name").equals("maven-plugin")) {
// Make sure there's a name attribute and that the optional value isn't true.
if (get(depObj,"name")!=null) {
if (get(depObj, "optional").equals("false")) {
dependencies.put(get(depObj, "name"), get(depObj, "version"));
} else {
Expand Down
32 changes: 21 additions & 11 deletions core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java
Expand Up @@ -81,7 +81,6 @@
import java.util.List;
import java.util.MissingResourceException;
import java.util.ResourceBundle;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
Expand Down Expand Up @@ -358,7 +357,7 @@ private User createAccount(StaplerRequest req, StaplerResponse rsp, boolean self
if(si.fullname==null || si.fullname.length()==0)
si.fullname = si.username;

if(si.email==null || !si.email.contains("@"))
if(isMailerPluginPresent() && (si.email==null || !si.email.contains("@")))
si.errorMessage = Messages.HudsonPrivateSecurityRealm_CreateAccount_InvalidEmailAddress();

if (! User.isIdOrFullnameAllowed(si.username)) {
Expand All @@ -379,19 +378,30 @@ private User createAccount(StaplerRequest req, StaplerResponse rsp, boolean self
// register the user
User user = createAccount(si.username,si.password1);
user.setFullName(si.fullname);
try {
// legacy hack. mail support has moved out to a separate plugin
Class<?> up = Jenkins.getInstance().pluginManager.uberClassLoader.loadClass("hudson.tasks.Mailer$UserProperty");
Constructor<?> c = up.getDeclaredConstructor(String.class);
user.addProperty((UserProperty)c.newInstance(si.email));
} catch (RuntimeException e) {
throw e;
} catch (Exception e) {
LOGGER.log(Level.WARNING, "Failed to set the e-mail address",e);
if(isMailerPluginPresent()) {
try {
// legacy hack. mail support has moved out to a separate plugin
Class<?> up = Jenkins.getInstance().pluginManager.uberClassLoader.loadClass("hudson.tasks.Mailer$UserProperty");
Constructor<?> c = up.getDeclaredConstructor(String.class);
user.addProperty((UserProperty)c.newInstance(si.email));
} catch (ReflectiveOperationException e) {
throw new RuntimeException(e);
}
}
user.save();
return user;
}

@Restricted(NoExternalUse.class)
public boolean isMailerPluginPresent() {
try {
// mail support has moved to a separate plugin
return null != Jenkins.getInstance().pluginManager.uberClassLoader.loadClass("hudson.tasks.Mailer$UserProperty");
} catch (ClassNotFoundException e) {
LOGGER.finer("Mailer plugin not present");
}
return false;
}

/**
* Creates a new user account by registering a password to the user.
Expand Down
Expand Up @@ -22,7 +22,7 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
THE SOFTWARE.
-->

<!-- tag file sed by both signup.jelly and addUser.jelly -->
<!-- tag file used by both signup.jelly and addUser.jelly -->
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
<h1>${title}</h1>
Expand All @@ -49,10 +49,12 @@ THE SOFTWARE.
<td>${%Full name}:</td>
<td><input type="text" name="fullname" value="${data.fullname}" /></td>
</tr>
<j:if test="${it.mailerPluginPresent}">
<tr>
<td>${%E-mail address}:</td>
<td><input type="text" name="email" value="${data.email}" /></td>
</tr>
</j:if>
<j:if test="${captcha}">
<tr>
<td>${%Enter text as shown}:</td>
Expand Down
28 changes: 14 additions & 14 deletions war/src/main/js/api/plugins.js
Expand Up @@ -19,7 +19,7 @@ exports.recommendedPlugins = [
"gradle",
"ldap",
"mailer",
// "matrix-auth",
"matrix-auth",
"pam-auth",
"pipeline-stage-view",
"ssh-slaves",
Expand All @@ -38,7 +38,7 @@ exports.availablePlugins = [
{
"category":"Organization and Administration",
"plugins": [
// { "name": "dashboard-view" },
{ "name": "dashboard-view" },
{ "name": "build-monitor-plugin" },
{ "name": "cloudbees-folder" },
{ "name": "antisamy-markup-formatter" }
Expand All @@ -49,15 +49,15 @@ exports.availablePlugins = [
"description":"Add general purpose features to your jobs",
"plugins": [
{ "name": "ansicolor" },
// { "name": "build-name-setter" },
{ "name": "build-name-setter" },
{ "name": "build-timeout" },
{ "name": "config-file-provider" },
{ "name": "credentials-binding" },
{ "name": "rebuild" },
{ "name": "ssh-agent" },
// { "name": "throttle-concurrents" },
{ "name": "timestamper" }
// { "name": "ws-cleanup" }
{ "name": "throttle-concurrents" },
{ "name": "timestamper" },
{ "name": "ws-cleanup" }
]
},
{
Expand All @@ -72,12 +72,12 @@ exports.availablePlugins = [
{
"category":"Build Analysis and Reporting",
"plugins": [
// { "name": "checkstyle" },
// { "name": "cobertura" },
{ "name": "checkstyle" },
{ "name": "cobertura" },
{ "name": "htmlpublisher" },
{ "name": "junit" },
// { "name": "sonar" },
// { "name": "warnings" },
{ "name": "sonar" },
{ "name": "warnings" },
{ "name": "xunit" }
]
},
Expand All @@ -88,8 +88,8 @@ exports.availablePlugins = [
{ "name": "github-organization-folder" },
{ "name": "pipeline-stage-view" },
{ "name": "build-pipeline-plugin" },
// { "name": "conditional-buildstep" },
// { "name": "jenkins-multijob-plugin" },
{ "name": "conditional-buildstep" },
{ "name": "jenkins-multijob-plugin" },
{ "name": "parameterized-trigger" },
{ "name": "copyartifact" }
]
Expand Down Expand Up @@ -122,10 +122,10 @@ exports.availablePlugins = [
{
"category":"User Management and Security",
"plugins": [
// { "name": "matrix-auth" },
{ "name": "matrix-auth" },
{ "name": "pam-auth" },
{ "name": "ldap" },
// { "name": "role-strategy" },
{ "name": "role-strategy" },
{ "name": "active-directory" }
]
},
Expand Down
8 changes: 7 additions & 1 deletion war/src/main/js/pluginSetupWizardGui.js
Expand Up @@ -599,7 +599,13 @@ var createPluginSetupWizard = function(appendTarget) {
$('.plugin-list .selected').addClass('match');
}
else {
var matches = findElementsWithText($pl[0], text.toLowerCase(), function(d) { return d.toLowerCase(); });
var matches = [];
$containers.find('.title,.description').each(function() {
var localMatches = findElementsWithText(this, text.toLowerCase(), function(d) { return d.toLowerCase(); });
if(localMatches.length > 0) {
matches = matches.concat(localMatches);
}
});
$(matches).parents('.plugin').addClass('match');
if(lastSearch !== text && scroll) {
scrollPlugin($pl, matches, text);
Expand Down

0 comments on commit 449719c

Please sign in to comment.