Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

JENKINS-67725 - Extending BranchBuildStrategies to allow setting last revision built + access to SCMEvent #291

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

JENKINS-67725 - Extending BranchBuildStrategies to allow setting last revision built + access to SCMEvent #291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
cb76f85
Discussion point JENKINS-67725
Feb 3, 2022
7b58e9c
Initial version - slightly extended API.
Feb 3, 2022
975123f
Added Tests
Feb 3, 2022
8e32385
Remove whitespace
Feb 3, 2022
2e784f3
More whitespace and minor edits
Feb 3, 2022
4955abf
More cleanup
Feb 3, 2022
47e54a0
Add SCMEvent to BranchBuildStrategy Extension.
Feb 4, 2022
34ba5d2
Merge branch 'master' into feature/JENKINS-67725
carpnick Mar 1, 2022
8659b96
Update src/main/java/jenkins/branch/MultiBranchProject.java
carpnick Mar 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 94 additions & 3 deletions src/main/java/jenkins/branch/BranchBuildStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import jenkins.scm.api.SCMSource;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.ProtectedExternally;
import jenkins.scm.api.SCMEvent;

/**
* An extension point that allows controlling whether a specific {@link SCMHead} should be automatically built when
Expand Down Expand Up @@ -160,13 +161,46 @@ public boolean isAutomaticBuild(@NonNull SCMSource source,
* {@link SCMHead} has been detected as created / modified.
* @since 2.4.2
*/
@Deprecated
@SuppressWarnings("deprecation")
@Restricted(ProtectedExternally.class)
public boolean isAutomaticBuild(@NonNull SCMSource source,
@NonNull SCMHead head,
@NonNull SCMRevision currRevision,
@CheckForNull SCMRevision lastBuiltRevision,
@CheckForNull SCMRevision lastSeenRevision,
@NonNull TaskListener listener){
throw new UnsupportedOperationException("Modern implementation accessed using legacy API method");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems like a breaking change? if an Extension overrides this calss and updated to the modern way removing the over ride on this deprecated function - then any consumers that have not updated will not blow up at runtime?

If the extension can do something about it - then it would seem this method is just un-needed - or could be used to call the modern method using the
Util.isOverridden trick.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually followed the example of what the rest of the plugin extension did. IE - Line 111 was originally shipped with 2.0.17 while line 138 was shipped with 2.1.3. Basically everytime there is a new version, the previous version is set to UnsupportedOperationException.

So the way I understand it the SPI method finds out dynamically which method is overridden (original, V2, V3, etc) and then calls that specific override. So the SPI method uses Util.isOverridden to determine which API method to call.

}

/**
* 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 build head is now at
* @param lastBuiltRevision the {@link SCMRevision} that the build head was last seen at or {@code null} if this is a newly
* discovered head. It replaces prevRevision from the previous SPI version. 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.
* @param lastSeenRevision the {@link SCMRevision} that the head was last seen
* @param listener the {@link TaskListener} that can be used for outputting any rational for the decision
* @return {@code true} if and only if the {@link SCMRevision} should be automatically built when the
* {@link SCMHead} has been detected as created / modified.
* @param scmEvent the {@link SCMEvent} that started the build if it exists.
* @since 2.4.2
*/
@Restricted(ProtectedExternally.class)
public abstract boolean isAutomaticBuild(@NonNull SCMSource source,
@NonNull SCMHead head,
@NonNull SCMRevision currRevision,
@CheckForNull SCMRevision lastBuiltRevision,
@CheckForNull SCMRevision lastSeenRevision,
@NonNull TaskListener listener);
@NonNull TaskListener listener,
@CheckForNull SCMEvent scmEvent);



/**
* API: Should the specified {@link SCMRevision} of the {@link SCMHead} for the specified {@link SCMSource} be
Expand Down Expand Up @@ -243,10 +277,46 @@ public final boolean automaticBuild(@NonNull SCMSource source,
@NonNull SCMRevision currRevision,
@CheckForNull SCMRevision lastBuiltRevision,
@CheckForNull SCMRevision lastSeenRevision,
@NonNull TaskListener listener) {
@NonNull TaskListener listener ) {
return automaticBuild(source, head, currRevision, lastBuiltRevision, lastSeenRevision, listener, null);
}


/**
* 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 lastBuiltRevision the {@link SCMRevision} that the build 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.
* @param lastSeenRevision the {@link SCMRevision} that the head was last seen
* @param listener the TaskListener to be used
* @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.4.2+
*/
@SuppressWarnings("deprecation")
public final boolean automaticBuild(@NonNull SCMSource source,
@NonNull SCMHead head,
@NonNull SCMRevision currRevision,
@CheckForNull SCMRevision lastBuiltRevision,
@CheckForNull SCMRevision lastSeenRevision,
@NonNull TaskListener listener,
@CheckForNull SCMEvent scmEvent ) {

if (Util.isOverridden(BranchBuildStrategy.class, getClass(), "isAutomaticBuild", SCMSource.class,
SCMHead.class, SCMRevision.class, SCMRevision.class, SCMRevision.class, TaskListener.class, SCMEvent.class)) {
// modern implementation written to the 2.4.2+
return isAutomaticBuild(source, head, currRevision, lastBuiltRevision, lastSeenRevision, listener, scmEvent);
}
if (Util.isOverridden(BranchBuildStrategy.class, getClass(), "isAutomaticBuild", SCMSource.class,
SCMHead.class, SCMRevision.class, SCMRevision.class, SCMRevision.class, TaskListener.class)) {
// modern implementation written to the 2.4.2+ spec
// modern implementation written to the 2.4.2 spec
return isAutomaticBuild(source, head, currRevision, lastBuiltRevision, lastSeenRevision, listener);
}
if (Util.isOverridden(BranchBuildStrategy.class, getClass(), "isAutomaticBuild", SCMSource.class,
Expand Down Expand Up @@ -274,6 +344,27 @@ public final boolean automaticBuild(@NonNull SCMSource source,
return isAutomaticBuild(source, head, currRevision, lastBuiltRevision, lastSeenRevision, listener);
}

/**
* API:Should we update last built revision if we did not do a build?
*
* @return {@code true} if and only if we should consider whatever commit we received as built.
*/
@SuppressWarnings("deprecation")
public final boolean updatingLastBuiltRevisionWithNoBuild() {
return isUpdatingLastBuiltRevisionWithNoBuild(new LogTaskListener(Logger.getLogger(getClass().getName()), Level.INFO));
}

/**
* SPI: Should we update last built revision if we did not do a build?
*
* @param listener the TaskListener to be used
* @return {@code true} if and only if we should consider whatever commit we received as built.
*/
@Restricted(ProtectedExternally.class)
public boolean isUpdatingLastBuiltRevisionWithNoBuild(@NonNull TaskListener listener) {
return false;
}

/**
* {@inheritDoc}
*/
Expand Down
48 changes: 47 additions & 1 deletion src/main/java/jenkins/branch/MultiBranchProject.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2207,6 +2207,21 @@ private void doAutomaticBuilds(@NonNull SCMHead head, @NonNull SCMRevision revis
);
} else {
listener.getLogger().format("No automatic build triggered for %s%n", rawName);

//Should we update the last Build revision with no build performed?
if (isUpdatingLastBuiltRevisionWithNoBuild()) {
//We need to save the revision here so we dont get in a loop
//Imagine a buildstrategy said dont build - but changes detected always is true - it puts jenkins into a loop
//Either it was chosen not to be built or it did build - either way we want to gaurantee it doesnt build the same thing again right?
//At least let a plugin owner extend and change behavior even if the default is do not update the last built revision.
try {
listener.getLogger().format("Updating revision hash so it will not be built again %n");
_factory.setRevisionHash(project, revision);

} catch (IOException e) {
printStackTrace(e, listener.error("Could not update last revision hash doAutomaticBuilds"));
}
}
}
try {
_factory.setLastSeenRevisionHash(project, revision);
Expand All @@ -2215,6 +2230,37 @@ private void doAutomaticBuilds(@NonNull SCMHead head, @NonNull SCMRevision revis
}
}

/**
* Tests if the specified buildStrategies say to update the last built revision.
* @return {@code true} if we are updating using setRevisionHash.
*/
private boolean isUpdatingLastBuiltRevisionWithNoBuild() {
BranchSource branchSource = null;
for (BranchSource s: MultiBranchProject.this.sources) {
if (s.getSource().getId().equals(source.getId())) {
branchSource = s;
break;
}
}
if (branchSource == null) {
// no match, means default to False
return false;
}
List<BranchBuildStrategy> buildStrategies = branchSource.getBuildStrategies();
if (buildStrategies.isEmpty()) {
// we will use default behaviour, return False
return false;
} else {
for (BranchBuildStrategy s: buildStrategies) {
if (s.updatingLastBuiltRevisionWithNoBuild()) {
//IF it is ever true, return
return true;
}
}
return false;
}
}

/**
* Tests if the specified {@link SCMHead} should be automatically built when discovered / modified.
* @param head the head.
Expand Down Expand Up @@ -2244,7 +2290,7 @@ private boolean isAutomaticBuild(@NonNull SCMHead head,
return !(head instanceof TagSCMHead);
} else {
for (BranchBuildStrategy s: buildStrategies) {
if (s.automaticBuild(source, head, currRevision, lastBuiltRevision, lastSeenRevision, listener)) {
if (s.automaticBuild(source, head, currRevision, lastBuiltRevision, lastSeenRevision, listener, event)) {
return true;
}
}
Expand Down
91 changes: 84 additions & 7 deletions src/test/java/integration/EventsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ public static class BuildEverythingStrategyImpl extends BranchBuildStrategy {
public boolean isAutomaticBuild(@NonNull SCMSource source, @NonNull SCMHead head,
@NonNull SCMRevision currRevision,
SCMRevision lastBuiltRevision, SCMRevision lastSeenRevision,
TaskListener listener) {
TaskListener listener, @CheckForNull SCMEvent scmEvent) {
return true;
}

Expand Down Expand Up @@ -700,7 +700,7 @@ public static class BuildChangeRequestsStrategyImpl extends BranchBuildStrategy
public boolean isAutomaticBuild(@NonNull SCMSource source, @NonNull SCMHead head,
@NonNull SCMRevision currRevision,
SCMRevision lastBuiltRevision, SCMRevision lastSeenRevision,
@NonNull TaskListener listener) {
@NonNull TaskListener listener, @CheckForNull SCMEvent scmEvent) {
return head instanceof ChangeRequestSCMHead;
}

Expand All @@ -716,7 +716,7 @@ public static class BuildTrustedChangeRequestsStrategyImpl extends BranchBuildSt
public boolean isAutomaticBuild(@NonNull SCMSource source, @NonNull SCMHead head,
@NonNull SCMRevision currRevision,
SCMRevision lastBuiltRevision, SCMRevision lastSeenRevision,
@NonNull TaskListener listener) {
@NonNull TaskListener listener, @CheckForNull SCMEvent scmEvent) {
if (head instanceof ChangeRequestSCMHead) {
try {
return currRevision.equals(source.getTrustedRevision(currRevision, listener));
Expand Down Expand Up @@ -1083,7 +1083,7 @@ public BuildRevisionStrategyImpl(String... approved) {
public boolean isAutomaticBuild(@NonNull SCMSource source, @NonNull SCMHead head,
@NonNull SCMRevision currRevision,
SCMRevision lastBuiltRevision, SCMRevision lastSeenRevision,
@NonNull TaskListener listener) {
@NonNull TaskListener listener, @CheckForNull SCMEvent scmEvent) {
return currRevision instanceof MockSCMRevision
&& approved.contains(((MockSCMRevision) currRevision).getHash());
}
Expand Down Expand Up @@ -1149,7 +1149,7 @@ public IgnoreTargetChangesStrategyImpl() {
public boolean isAutomaticBuild(@NonNull SCMSource source, @NonNull SCMHead head,
@NonNull SCMRevision currRevision,
SCMRevision lastBuiltRevision, SCMRevision lastSeenRevision,
@NonNull TaskListener listener) {
@NonNull TaskListener listener, @CheckForNull SCMEvent scmEvent) {
if (currRevision instanceof ChangeRequestSCMRevision) {
ChangeRequestSCMRevision<?> currCR = (ChangeRequestSCMRevision<?>) currRevision;
if (lastBuiltRevision instanceof ChangeRequestSCMRevision) {
Expand Down Expand Up @@ -2866,7 +2866,7 @@ public static class SkipInitialBuildStrategyImpl extends BranchBuildStrategy {
public boolean isAutomaticBuild(@NonNull SCMSource source, @NonNull SCMHead head,
@NonNull SCMRevision currRevision,
SCMRevision lastBuiltRevision, SCMRevision lastSeenRevision,
TaskListener listener) {
TaskListener listener, @CheckForNull SCMEvent scmEvent) {
if (lastSeenRevision != null) {
return true;
}
Expand Down Expand Up @@ -2907,12 +2907,89 @@ public void given_multibranch_when__build_is_skipped_then_lastBuiltRevision_is_n
}
}

@Test
public void given_multibranch_when__build_is_skipped_then_lastBuiltRevision_is_updated() throws Exception {
try (MockSCMController c = MockSCMController.create()) {
c.createRepository("foo");
BasicMultiBranchProject prj = r.jenkins.createProject(BasicMultiBranchProject.class, "my-project");
prj.setCriteria(null);
BranchSource source = new BranchSource(new MockSCMSource(c, "foo", new MockSCMDiscoverBranches()));
source.setBuildStrategies(Collections.<BranchBuildStrategy>singletonList(new SkipAndUpdateRevisionHashBuildStrategyImpl()));
prj.getSourcesList().add(source);
fire(new MockSCMHeadEvent(SCMEvent.Type.CREATED, c, "foo", "master", c.getRevision("foo", "master")));
FreeStyleProject master = prj.getItem("master");
r.waitUntilNoActivity();
assertThat("The master branch was built", master.getLastBuild(), nullValue());
SCMRevision scmLastSeenRevision1 = prj.getProjectFactory().getLastSeenRevision(master);
assertNotNull(scmLastSeenRevision1);
assertNotNull(prj.getProjectFactory().getRevision(master));
}
}

@Test
public void given_multibranch_when_build_is_skipped_due_to_use_of_scmevent() throws Exception {
try (MockSCMController c = MockSCMController.create()) {
c.createRepository("foo");
BasicMultiBranchProject prj = r.jenkins.createProject(BasicMultiBranchProject.class, "my-project");
prj.setCriteria(null);
BranchSource source = new BranchSource(new MockSCMSource(c, "foo", new MockSCMDiscoverBranches()));
source.setBuildStrategies(Collections.<BranchBuildStrategy>singletonList(new UseSCMEventBuildStrategy()));
prj.getSourcesList().add(source);
c.addFile("foo", "master", "adding file", "file", new byte[0]);
fire(new MockSCMHeadEvent(SCMEvent.Type.UPDATED, c, "foo", "master", c.getRevision("foo", "master")));
FreeStyleProject master = prj.getItem("master");
r.waitUntilNoActivity();
assertThat("The master branch was not built", master.getLastBuild(), notNullValue());
assertNotNull(prj.getProjectFactory().getRevision(master));
}
}

public static class UseSCMEventBuildStrategy extends BranchBuildStrategy {
@Override
public boolean isAutomaticBuild(@NonNull SCMSource source, @NonNull SCMHead head,
@NonNull SCMRevision currRevision,
SCMRevision lastBuiltRevision, SCMRevision lastSeenRevision,
TaskListener listener, @CheckForNull SCMEvent scmEvent) {
//Assume not null
//Returns effective true
return scmEvent.getTimestamp() >= 0;
}


@TestExtension(
"given_multibranch_when_build_is_skipped_due_to_use_of_scmevent")
public static class DescriptorImpl extends BranchBuildStrategyDescriptor {

}
}

public static class SkipAndUpdateRevisionHashBuildStrategyImpl extends BranchBuildStrategy {
@Override
public boolean isAutomaticBuild(@NonNull SCMSource source, @NonNull SCMHead head,
@NonNull SCMRevision currRevision,
SCMRevision lastBuiltRevision, SCMRevision lastSeenRevision,
TaskListener listener, @CheckForNull SCMEvent scmEvent) {
return false;
}

@Override
public boolean isUpdatingLastBuiltRevisionWithNoBuild(TaskListener listener){
return true;
}

@TestExtension(
"given_multibranch_when__build_is_skipped_then_lastBuiltRevision_is_updated")
public static class DescriptorImpl extends BranchBuildStrategyDescriptor {

}
}

public static class SkipIAllBuildStrategyImpl extends BranchBuildStrategy {
@Override
public boolean isAutomaticBuild(@NonNull SCMSource source, @NonNull SCMHead head,
@NonNull SCMRevision currRevision,
SCMRevision lastBuiltRevision, SCMRevision lastSeenRevision,
TaskListener listener) {
TaskListener listener, @CheckForNull SCMEvent scmEvent) {
return false;
}

Expand Down