Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
JENKINS-33950 - dependencies may be installed multiple times unnecess…
…arily, resulting in erroneous restart required indicators
  • Loading branch information
kzantow committed Apr 13, 2016
1 parent b17e04c commit bfeeec1
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 1 deletion.
51 changes: 51 additions & 0 deletions core/src/main/java/hudson/model/UpdateCenter.java
Expand Up @@ -1414,6 +1414,15 @@ public abstract class DownloadJob extends UpdateCenterJob {
@Exported
public abstract String getName();

/**
* Any additional detail about the plugin which might be displayed to the user,
* e.g. a version or URL
*/
@CheckForNull
public String getDetail() {
return null;
}

/**
* Called when the whole thing went successfully.
*/
Expand Down Expand Up @@ -1560,6 +1569,15 @@ public class Success extends InstallationStatus {
}
}

/**
* Indicates that the plugin was successfully installed.
*/
public class Skipped extends InstallationStatus {
@Override public boolean isSuccess() {
return true;
}
}

/**
* Indicates that the plugin is waiting for its turn for installation.
*/
Expand Down Expand Up @@ -1649,6 +1667,11 @@ private File getLegacyDestination() {
public String getName() {
return plugin.getDisplayName();
}

@Override
public String getDetail() {
return plugin.version;
}

@Override
public void _run() throws IOException, InstallationStatus {
Expand All @@ -1663,6 +1686,11 @@ public void _run() throws IOException, InstallationStatus {
} finally {
SecurityContextHolder.setContext(oldContext);
}
} else if (isAlreadyInstalling()) {
status = new UpdateCenter.DownloadJob.Skipped();
// check to see if the plugin is already installed at the same version and skip it
LOGGER.warning("Skipping duplicate install of: " + plugin.getDisplayName() + "@" + plugin.version);
return;
}

if (dynamicLoad) {
Expand All @@ -1678,6 +1706,29 @@ public void _run() throws IOException, InstallationStatus {
}
}

/**
* Indicates there is another installation job for this plugin
* @since TODO
*/
protected boolean isAlreadyInstalling() {
synchronized(UpdateCenter.this) {
for (UpdateCenterJob job : getJobs()) {
if (job == this) {
// oldest entries first, if we reach this instance,
// we need it to continue installing
return false;
}
if (job instanceof InstallationJob) {
InstallationJob ij = (InstallationJob)job;
if (ij.plugin.equals(plugin) && ij.plugin.version.equals(plugin.version)) {
return true;
}
}
}
return false;
}
}

protected void onSuccess() {
pm.pluginUploaded = true;
}
Expand Down
Expand Up @@ -25,7 +25,7 @@ THE SOFTWARE.
<?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">
<tr id="row${it.id}">
<td style="vertical-align: top; padding-right:1em">${it.name}</td>
<td style="vertical-align: top; padding-right:1em">${it.name} <j:if test="${it.detail != null}">(${it.detail})</j:if></td>
<j:set var="status" value="${it.status}" /><!-- so that two reference to this variable resolve to the same value. -->
<td id="status${status.id}" style="vertical-align:middle">
<st:include it="${status}" page="status.jelly" />
Expand Down

0 comments on commit bfeeec1

Please sign in to comment.