Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[FIXED JENKINS-48535] Allow a BranchBuildStrategy to not build PR-mer…
…ge revisions where the only change is the target revision
  • Loading branch information
stephenc committed Dec 13, 2017
1 parent 9fd74e8 commit 6189c75
Show file tree
Hide file tree
Showing 4 changed files with 232 additions and 29 deletions.
10 changes: 10 additions & 0 deletions pom.xml
Expand Up @@ -85,6 +85,16 @@
</pluginRepositories>

<dependencies>
<dependency>
<groupId>org.kohsuke</groupId>
<artifactId>access-modifier-annotation</artifactId>
<version>1.11</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci</groupId>
<artifactId>annotation-indexer</artifactId>
<version>1.9</version>
</dependency>
<!-- plugin dependencies -->
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
Expand Down
97 changes: 89 additions & 8 deletions src/main/java/jenkins/branch/BranchBuildStrategy.java
Expand Up @@ -23,44 +23,125 @@
*/
package jenkins.branch;

import edu.umd.cs.findbugs.annotations.CheckForNull;
import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.ExtensionPoint;
import hudson.Util;
import hudson.model.AbstractDescribableImpl;
import jenkins.scm.api.SCMHead;
import jenkins.scm.api.SCMRevision;
import jenkins.scm.api.SCMSource;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.ProtectedExternally;

/**
* An extension point that allows controlling whether a specific {@link SCMHead} should be automatically built when
* discovered.
* <p>
* Methods marked as {@code SPI:} are intended to be implemented by implementers of {@link BranchBuildStrategy}.
* Methods marked as {@code API:} are intended to be invoked consumers of {@link BranchBuildStrategy}.
* A consumer invoking a {@code SPI:} method may get a {@link UnsupportedOperationException}.
*
* @since 2.0.0
*/
public abstract class BranchBuildStrategy extends AbstractDescribableImpl<BranchBuildStrategy>
implements ExtensionPoint {
/**
* Should the specified {@link SCMHead} for the specified {@link SCMSource} be built whenever it is detected as
* SPI: Should the specified {@link SCMHead} for the specified {@link SCMSource} be built whenever it is detected as
* created / modified?
*
* @param source the {@link SCMSource}
* @param head the {@link SCMHead}
* @return {@code true} if and only if the {@link SCMHead} should be automatically built when detected as created
* / modified.
* @deprecated use {@link #automaticBuild(SCMSource, SCMHead, SCMRevision, SCMRevision)}
*/
public abstract boolean isAutomaticBuild(SCMSource source, SCMHead head);
@Deprecated
@Restricted(ProtectedExternally.class)
public boolean isAutomaticBuild(@NonNull SCMSource source, @NonNull SCMHead head) {
throw new UnsupportedOperationException("Modern implementation accessed using legacy API method");
}

/**
* Should the specified {@link SCMRevision} of the {@link SCMHead} for the specified {@link SCMSource} be triggered
* when the {@link SCMHead} has been detected as created / modified?
* SPI: Should the specified {@link SCMRevision} of the {@link SCMHead} for the specified {@link SCMSource} be
* triggered when the {@link SCMHead} has been detected as created / modified?
*
* @param source the {@link SCMSource}
* @param head the {@link SCMHead}
* @param source the {@link SCMSource}
* @param head the {@link SCMHead}
* @param revision the {@link SCMRevision}
* @return {@code true} if and only if the {@link SCMRevision} should be automatically built when the
* {@link SCMHead} has been detected as created / modified.
* @since 2.0.12
* @deprecated use {@link #automaticBuild(SCMSource, SCMHead, SCMRevision, SCMRevision)}
*/
@Deprecated
@SuppressWarnings("deprecation")
@Restricted(ProtectedExternally.class)
public boolean isAutomaticBuild(@NonNull SCMSource source, @NonNull SCMHead head, @NonNull SCMRevision revision) {
throw new UnsupportedOperationException("Modern implementation accessed using legacy API method");
}

/**
* SPI: Should the specified {@link SCMRevision} of the {@link SCMHead} for the specified {@link SCMSource} be
* triggered when the {@link SCMHead} has been detected as created / modified?
*
* @param source the {@link SCMSource}
* @param head the {@link SCMHead}
* @param currRevision the {@link SCMRevision} that the head is now at
* @param prevRevision the {@link SCMRevision} that the head was last seen at or {@code null} if this is a newly
* discovered head. Care should be taken to consider the case of non
* {@link SCMRevision#isDeterministic()} previous revisions as polling for changes will have
* confirmed that there is a change between this and {@code currRevision} even if the two
* are equal.
* @return {@code true} if and only if the {@link SCMRevision} should be automatically built when the
* {@link SCMHead} has been detected as created / modified.
* @since 2.0.17
*/
@Restricted(ProtectedExternally.class)
public abstract boolean isAutomaticBuild(@NonNull SCMSource source,
@NonNull SCMHead head,
@NonNull SCMRevision currRevision,
@CheckForNull SCMRevision prevRevision);

/**
* API: Should the specified {@link SCMRevision} of the {@link SCMHead} for the specified {@link SCMSource} be
* triggered when the {@link SCMHead} has been detected as created / modified?
*
* @param source the {@link SCMSource}
* @param head the {@link SCMHead}
* @param currRevision the {@link SCMRevision} that the head is now at
* @param prevRevision the {@link SCMRevision} that the head was last seen at or {@code null} if this is a newly
* discovered head. Care should be taken to consider the case of non
* {@link SCMRevision#isDeterministic()} previous revisions as polling for changes will have
* confirmed that there is a change between this and {@code currRevision} even if the two
* are equal.
* @return {@code true} if and only if the {@link SCMRevision} should be automatically built when the
* {@link SCMHead} has been detected as created / modified.
* @since 2.0.17
*/
public boolean isAutomaticBuild(SCMSource source, SCMHead head, SCMRevision revision) {
return isAutomaticBuild(source, head);
@SuppressWarnings("deprecation")
public final boolean automaticBuild(@NonNull SCMSource source,
@NonNull SCMHead head,
@NonNull SCMRevision currRevision,
@CheckForNull SCMRevision prevRevision) {
if (Util.isOverridden(BranchBuildStrategy.class, getClass(), "isAutomaticBuild", SCMSource.class,
SCMHead.class, SCMRevision.class, SCMRevision.class)) {
// modern implementation written to the 2.0.17+ spec
return isAutomaticBuild(source, head, currRevision, prevRevision);
}
if (Util.isOverridden(BranchBuildStrategy.class, getClass(), "isAutomaticBuild", SCMSource.class,
SCMHead.class, SCMRevision.class)) {
// legacy implementation written to the 2.0.12-2.0.16 spec
return isAutomaticBuild(source, head, currRevision);
}
if (Util.isOverridden(BranchBuildStrategy.class, getClass(), "isAutomaticBuild", SCMSource.class,
SCMHead.class)) {
// legacy implementation written to the 2.0.0-2.0.11 spec
return isAutomaticBuild(source, head);
}
// this is going to throw an abstract method exception, but we should never get here as all implementations
// have to at least have overridden one of the methods above.
return isAutomaticBuild(source, head, currRevision, prevRevision);
}

/**
Expand Down
36 changes: 26 additions & 10 deletions src/main/java/jenkins/branch/MultiBranchProject.java
Expand Up @@ -2041,7 +2041,10 @@ public void observe(@NonNull SCMHead head, @NonNull SCMRevision revision) throws
revision
);
needSave = true;
if (isAutomaticBuild(source, head, revision)) {
// the previous "revision" for this head is not a revision for the current source
// either because the head was removed and then recreated, or because the head
// was taken over by a different source, thus the previous revision is null
if (isAutomaticBuild(source, head, revision, null)) {
scheduleBuild(
_factory,
project,
Expand All @@ -2055,12 +2058,12 @@ public void observe(@NonNull SCMHead head, @NonNull SCMRevision revision) throws
listener.getLogger().format("No automatic builds for %s%n", rawName);
}
} else if (revision.isDeterministic()) {
SCMRevision lastBuild = _factory.getRevision(project);
if (!revision.equals(lastBuild)) {
SCMRevision prevRevision = _factory.getRevision(project);
if (!revision.equals(prevRevision)) {
listener.getLogger()
.format("Changes detected: %s (%s → %s)%n", rawName, lastBuild, revision);
.format("Changes detected: %s (%s → %s)%n", rawName, prevRevision, revision);
needSave = true;
if (isAutomaticBuild(source, head, revision)) {
if (isAutomaticBuild(source, head, revision, prevRevision)) {
scheduleBuild(
_factory,
project,
Expand All @@ -2077,14 +2080,24 @@ public void observe(@NonNull SCMHead head, @NonNull SCMRevision revision) throws
listener.getLogger().format("No changes detected: %s (still at %s)%n", rawName, revision);
}
} else {
// TODO check if we should compare the revisions anyway...
// the revisions may not be deterministic, as in two checkouts of the same revision
// may result in additional changes (looking at you CVS) but that doesn't mean
// that the revisions are not capable of checking for differences, e.g. if
// rev1 == 2017-12-13 and rev2 == 2017-12-12 these could show as non-equal while both
// being non-deterministic (because they just specify the day not the timestamp within
// the day.

// fall back to polling when we have a non-deterministic revision/hash.
SCMTriggerItem scmProject = SCMTriggerItem.SCMTriggerItems.asSCMTriggerItem(project);
if (scmProject != null) {
PollingResult pollingResult = scmProject.poll(listener);
if (pollingResult.hasChanges()) {
listener.getLogger().format("Changes detected: %s%n", rawName);
needSave = true;
if (isAutomaticBuild(source, head, revision)) {
// get the previous revision
SCMRevision prevRevision = _factory.getRevision(project);
if (isAutomaticBuild(source, head, revision, prevRevision)) {
scheduleBuild(
_factory,
project,
Expand Down Expand Up @@ -2148,7 +2161,7 @@ public void observe(@NonNull SCMHead head, @NonNull SCMRevision revision) throws
_factory.decorate(project);
// ok it is now up to the observer to ensure it does the actual save.
observer.created(project);
if (isAutomaticBuild(source, head, revision)) {
if (isAutomaticBuild(source, head, revision, null)) {
scheduleBuild(
_factory,
project,
Expand All @@ -2171,10 +2184,13 @@ public void observe(@NonNull SCMHead head, @NonNull SCMRevision revision) throws
* Tests if the specified {@link SCMHead} should be automatically built when discovered / modified.
* @param source the source.
* @param head the head.
* @param revision the revision.
* @param currRevision the revision.
* @return {@code true} if the head should be automatically built when discovered / modified.
*/
private boolean isAutomaticBuild(SCMSource source, SCMHead head, SCMRevision revision) {
private boolean isAutomaticBuild(@NonNull SCMSource source,
@NonNull SCMHead head,
@NonNull SCMRevision currRevision,
@CheckForNull SCMRevision prevRevision) {
BranchSource branchSource = null;
for (BranchSource s: sources) {
if (s.getSource().getId().equals(source.getId())) {
Expand All @@ -2192,7 +2208,7 @@ private boolean isAutomaticBuild(SCMSource source, SCMHead head, SCMRevision rev
return !(head instanceof TagSCMHead);
} else {
for (BranchBuildStrategy s: buildStrategies) {
if (s.isAutomaticBuild(source, head, revision)) {
if (s.automaticBuild(source, head, currRevision, prevRevision)) {
return true;
}
}
Expand Down

0 comments on commit 6189c75

Please sign in to comment.