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.

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

[Draft] Present HEAD commit from source branch instead of merged commit in PreBuildMerge plugin #1210

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 7 additions & 1 deletion src/main/java/hudson/plugins/git/GitSCM.java
Original file line number Diff line number Diff line change
Expand Up @@ -1163,7 +1163,13 @@ public EnvVars getEnvironment() {
for (GitSCMExtension ext : extensions) {
rev = ext.decorateRevisionToBuild(this,build,git,listener,marked,rev);
}
Build revToBuild = new Build(marked, rev, build.getNumber(), null);

Revision revToDisplay = rev;
for (GitSCMExtension ext : extensions) {
revToDisplay = ext.decorateRevisionToDisplay(revToDisplay, marked);
}

Build revToBuild = new Build(marked, rev, build.getNumber(), null, revToDisplay);
buildData.saveBuild(revToBuild);

if (buildData.getBuildsByBranchName().size() >= 100) {
Expand Down
37 changes: 34 additions & 3 deletions src/main/java/hudson/plugins/git/UserMergeOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public class UserMergeOptions extends AbstractDescribableImpl<UserMergeOptions>
private final String mergeTarget;
private String mergeStrategy;
private MergeCommand.GitPluginFastForwardMode fastForwardMode;
private DisplayRevision displayRevision;

/**
* @deprecated use the new constructor that allows to set the fast forward mode.
Expand All @@ -40,18 +41,24 @@ public UserMergeOptions(String mergeRemote, String mergeTarget, String mergeStra
this(mergeRemote, mergeTarget, mergeStrategy, MergeCommand.GitPluginFastForwardMode.FF);
}

public UserMergeOptions(String mergeRemote, String mergeTarget, String mergeStrategy,
MergeCommand.GitPluginFastForwardMode fastForwardMode) {
this(mergeRemote, mergeTarget, mergeStrategy, MergeCommand.GitPluginFastForwardMode.FF, DisplayRevision.MERGED);
}

/**
* @param mergeRemote remote name used for merge
* @param mergeTarget remote branch to be merged into current branch
* @param mergeStrategy merge strategy
* @param fastForwardMode fast forward mode
*/
public UserMergeOptions(String mergeRemote, String mergeTarget, String mergeStrategy,
MergeCommand.GitPluginFastForwardMode fastForwardMode) {
MergeCommand.GitPluginFastForwardMode fastForwardMode, DisplayRevision displayRevision) {
this.mergeRemote = mergeRemote;
this.mergeTarget = mergeTarget;
this.mergeStrategy = mergeStrategy;
this.fastForwardMode = fastForwardMode;
this.displayRevision = displayRevision;
}

@DataBoundConstructor
Expand Down Expand Up @@ -124,13 +131,26 @@ public void setFastForwardMode(MergeCommand.GitPluginFastForwardMode fastForward
this.fastForwardMode = fastForwardMode;
}

public DisplayRevision getDisplayRevision() {
for (DisplayRevision revision : DisplayRevision.values())
if (revision.equals(displayRevision))
return revision;
return DisplayRevision.MERGED;
}

@DataBoundSetter
public void setDisplayRevision(DisplayRevision displayRevision) {
this.displayRevision = displayRevision;
}

@Override
public String toString() {
return "UserMergeOptions{" +
"mergeRemote='" + mergeRemote + '\'' +
", mergeTarget='" + mergeTarget + '\'' +
", mergeStrategy='" + getMergeStrategy().name() + '\'' +
", fastForwardMode='" + getFastForwardMode().name() + '\'' +
", displayRevision='" + getDisplayRevision().name() + '\'' +
'}';
}

Expand All @@ -148,12 +168,13 @@ public boolean equals(Object o) {
return Objects.equals(mergeRemote, that.mergeRemote)
&& Objects.equals(mergeTarget, that.mergeTarget)
&& Objects.equals(mergeStrategy, that.mergeStrategy)
&& Objects.equals(fastForwardMode, that.fastForwardMode);
&& Objects.equals(fastForwardMode, that.fastForwardMode)
&& Objects.equals(displayRevision, that.displayRevision);
}

@Override
public int hashCode() {
return Objects.hash(mergeRemote, mergeTarget, mergeStrategy, fastForwardMode);
return Objects.hash(mergeRemote, mergeTarget, mergeStrategy, fastForwardMode, displayRevision);
}

@Extension
Expand All @@ -175,5 +196,15 @@ public Map<String, Object> customInstantiate(Map<String, Object> arguments) {
}

}
public static enum DisplayRevision {
MERGED,
SOURCE_BRANCH;

private DisplayRevision() {
}

public String toString() {
return this.name().toLowerCase(Locale.ENGLISH);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -372,4 +372,9 @@ public boolean enableMultipleRevisionDetection() {
public GitSCMExtensionDescriptor getDescriptor() {
return (GitSCMExtensionDescriptor) super.getDescriptor();
}

//What should be API of this method?
public Revision decorateRevisionToDisplay(Revision revToDisplay, Revision marked) {
return revToDisplay;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ public Revision decorateRevisionToBuild(GitSCM scm, Run<?, ?> build, GitClient g
return mergeRevision;
}

@Override
//What should be API of this method?
public Revision decorateRevisionToDisplay(Revision revToDisplay, Revision marked) {
return options.getDisplayRevision() == UserMergeOptions.DisplayRevision.SOURCE_BRANCH ? marked : revToDisplay;
}

@Override
public void decorateMergeCommand(GitSCM scm, Run<?, ?> build, GitClient git, TaskListener listener, MergeCommand cmd) throws IOException, InterruptedException, GitException {
if (options.getMergeStrategy() != null) {
Expand Down
16 changes: 16 additions & 0 deletions src/main/java/hudson/plugins/git/opt/PreBuildMergeOptions.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package hudson.plugins.git.opt;

import hudson.plugins.git.UserMergeOptions;
import org.eclipse.jgit.transport.RemoteConfig;
import org.jenkinsci.plugins.gitclient.MergeCommand;
import org.kohsuke.stapler.DataBoundSetter;
import org.kohsuke.stapler.export.Exported;
import org.kohsuke.stapler.export.ExportedBean;

Expand Down Expand Up @@ -33,6 +35,8 @@ public class PreBuildMergeOptions implements Serializable {

public MergeCommand.GitPluginFastForwardMode fastForwardMode = MergeCommand.GitPluginFastForwardMode.FF;

public UserMergeOptions.DisplayRevision displayRevision = UserMergeOptions.DisplayRevision.MERGED;

public RemoteConfig getMergeRemote() {
return mergeRemote;
}
Expand Down Expand Up @@ -74,6 +78,18 @@ public void setFastForwardMode(MergeCommand.GitPluginFastForwardMode fastForward
this.fastForwardMode = fastForwardMode;
}

@Exported
public UserMergeOptions.DisplayRevision getDisplayRevision() {
for (UserMergeOptions.DisplayRevision revision : UserMergeOptions.DisplayRevision.values())
if (revision.equals(displayRevision))
return revision;
return UserMergeOptions.DisplayRevision.MERGED;
}

public void setDisplayRevision(UserMergeOptions.DisplayRevision displayRevision) {
this.displayRevision = displayRevision;
}

@Exported
public String getRemoteBranchName() {
return (mergeRemote == null) ? null : mergeRemote.getName() + "/" + mergeTarget;
Expand Down
16 changes: 16 additions & 0 deletions src/main/java/hudson/plugins/git/util/Build.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ public class Build implements Serializable, Cloneable {
*/
public Revision revision;

public Revision displayRevision;

public int hudsonBuildNumber;
public Result hudsonBuildResult;

Expand All @@ -56,6 +58,15 @@ public class Build implements Serializable, Cloneable {
public Build(Revision marked, Revision revision, int buildNumber, Result result) {
this.marked = marked;
this.revision = revision;
this.displayRevision = revision;
this.hudsonBuildNumber = buildNumber;
this.hudsonBuildResult = result;
}

public Build(Revision marked, Revision revision, int buildNumber, Result result, Revision displayRevision) {
this.marked = marked;
this.revision = revision;
this.displayRevision = displayRevision;
this.hudsonBuildNumber = buildNumber;
this.hudsonBuildResult = result;
}
Expand Down Expand Up @@ -139,4 +150,9 @@ public Object readResolve() throws IOException {
marked = revision;
return this;
}

@Exported
public Revision getDisplayRevision() {
return displayRevision;
}
}
10 changes: 10 additions & 0 deletions src/main/java/hudson/plugins/git/util/BuildData.java
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,16 @@ public Build getLastBuildOfBranch(String branch) {
return lastBuild==null?null:lastBuild.revision;
}

/**
* Gets revision of the previous build to display.
* @return display revision of the last build.
* May be null will be returned if nothing has been checked out (e.g. due to wrong repository or branch)
*/
@Exported
public @CheckForNull Revision getLastBuiltDisplayRevision() {
return lastBuild==null?null:lastBuild.getDisplayRevision();
}

@Exported
public Map<String,Build> getBuildsByBranchName() {
return buildsByBranchName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,9 @@
${it.toString()}
</f:enum>
</f:entry>
<f:entry title="${%Revision to display}" field="displayRevision">
<f:enum>
${it.toString()}
</f:enum>
</f:entry>
</j:jelly>
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
<l:main-panel>
<h1>${%Git Build Data}</h1>

<strong>${%Revision}</strong>: ${it.lastBuild.SHA1.name()}
<strong>${%Revision}</strong>: ${it.lastBuiltDisplayRevision.sha1.name()}
<j:if test="${!empty(it.scmName)}"><br/><strong>${%SCM}</strong>: ${it.scmName}</j:if>
<j:if test="${!empty(it.remoteUrls)}">
<j:forEach var="remoteUrl" items="${it.remoteUrls}">
Expand All @@ -21,7 +21,7 @@
</j:forEach>
</j:if>
<ul>
<j:forEach var="branch" items="${it.lastBuild.revision.branches}">
<j:forEach var="branch" items="${it.lastBuiltDisplayRevision.branches}">
<li>${branch.name}</li>
</j:forEach>
</ul>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
xmlns:f="/lib/form" xmlns:i="jelly:fmt">
<t:summary icon="/plugin/git/icons/git-logo.svg">

<strong>${%Revision}</strong>: ${it.lastBuiltRevision.sha1.name()}
<strong>${%Revision}</strong>: ${it.lastBuiltDisplayRevision.sha1.name()}
<j:if test="${!empty(it.scmName)}"><br/><strong>${%SCM}</strong>: ${it.scmName}</j:if>
<j:if test="${!empty(it.remoteUrls)}">
<j:forEach var="remoteUrl" items="${it.remoteUrls}">
Expand All @@ -17,7 +17,7 @@
</j:forEach>
</j:if>
<ul>
<j:forEach var="branch" items="${it.lastBuiltRevision.branches}">
<j:forEach var="branch" items="${it.lastBuiltDisplayRevision.branches}">
<j:if test="${branch!=''}">
<li>${branch.name}</li>
</j:if>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,17 @@
import hudson.plugins.git.extensions.GitSCMExtensionTest;
import hudson.plugins.git.util.BuildData;
import nl.jqno.equalsverifier.EqualsVerifier;
import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.ObjectId;
import org.jenkinsci.plugins.gitclient.MergeCommand;
import org.junit.Test;

import java.io.File;
import java.io.IOException;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;

import static org.junit.Assert.*;

/**
Expand All @@ -26,21 +34,101 @@ public class PreBuildMergeTest extends GitSCMExtensionTest
private TestGitRepo repo;

private String MASTER_FILE = "commitFileBase";
private String INTEGRATION_FILE = "branchFile";

private String initCommit;

public void before() throws Exception {
repo = new TestGitRepo("repo", tmp.newFolder(), listener);
project = setupBasicProject(repo);
// make an initial commit to master
repo.commit(MASTER_FILE, repo.johnDoe, "Initial Commit");
initCommit = repo.commit(MASTER_FILE, repo.johnDoe, "Initial Commit");
// create integration branch
repo.git.branch("integration");
}

@Test
public void testBasicPreMerge() throws Exception {
// already existing test
FreeStyleBuild firstBuild = build(project, Result.SUCCESS);
}

@Test
public void testDisplayMergedRevision() throws Exception {
// add some commits to source and target branch
repo.git.checkoutBranch("integration", initCommit);
String integrationCommit = repo.commit(INTEGRATION_FILE, repo.johnDoe, "Integration Commit");
repo.git.checkoutBranch("master", initCommit);
String masterCommit = repo.commit("Master2", repo.johnDoe, "Master2 Commit");

FreeStyleBuild firstBuild = build(project, Result.SUCCESS);

// git repository placed in build's workspace
TestGitRepo repoInWorkspace = getRepoInWorkspace(firstBuild);

List<ObjectId> commitsInWorkspace = getCommitsOnHead(repoInWorkspace);

ObjectId mergeCommit = commitsInWorkspace.remove(0);

// verify state of workspace fit repo
// is it possible to verify merge commit (checking log message?)
assertEquals(Arrays.asList(integrationCommit, masterCommit, initCommit),
commitsInWorkspace.stream().map(AnyObjectId::name).collect(Collectors.toList()));

// verify revision displayed
assertEquals(GitSCM.class, project.getScm().getClass());
GitSCM gitSCM = (GitSCM)project.getScm();
BuildData buildData = gitSCM.getBuildData(firstBuild);
Revision buildRevision = buildData.getLastBuiltDisplayRevision();

assertEquals(mergeCommit, buildRevision.getSha1());
assertEquals(1, buildRevision.getBranches().size());
assertTrue(buildRevision.containsBranchName("origin/integration"));
}

@Test
public void testDisplaySourceRevision() throws Exception {
// change UserMergeOption
GitSCM gitSCM = (GitSCM)project.getScm();
gitSCM.getExtensions().get(PreBuildMerge.class).getOptions().setDisplayRevision(UserMergeOptions.DisplayRevision.SOURCE_BRANCH);

// add some commits to source and target branch
repo.git.checkoutBranch("integration", initCommit);
String integrationCommit = repo.commit(INTEGRATION_FILE, repo.johnDoe, "Integration Commit");
repo.git.checkoutBranch("master", initCommit);
String masterCommit = repo.commit("Master2", repo.johnDoe, "Master2 Commit");

FreeStyleBuild firstBuild = build(project, Result.SUCCESS);

// git repository placed in build's workspace
TestGitRepo repoInWorkspace = getRepoInWorkspace(firstBuild);
List<ObjectId> commitsInWorkspace = getCommitsOnHead(repoInWorkspace);

ObjectId mergeCommit = commitsInWorkspace.remove(0);

// verify state of workspace fit repo
// is it possible to verify merge commit (checking log message?)
assertEquals(Arrays.asList(integrationCommit, masterCommit, initCommit),
commitsInWorkspace.stream().map(AnyObjectId::name).collect(Collectors.toList()));
assertEquals(GitSCM.class, project.getScm().getClass());

// verify revision displayed
BuildData buildData = gitSCM.getBuildData(firstBuild);
Revision buildRevision = buildData.getLastBuiltDisplayRevision();

assertEquals(masterCommit, buildRevision.getSha1().name());
assertEquals(1, buildRevision.getBranches().size());
assertTrue(buildRevision.containsBranchName("origin/master"));
}

private List<ObjectId> getCommitsOnHead(TestGitRepo repoInWorkspace) throws InterruptedException {
return repoInWorkspace.git.revList("HEAD");
}

private TestGitRepo getRepoInWorkspace(FreeStyleBuild firstBuild) throws IOException, InterruptedException {
return new TestGitRepo("workspace", new File(firstBuild.getWorkspace().toString()), listener);
}

@Test
public void testFailedMerge() throws Exception {
FreeStyleBuild firstBuild = build(project, Result.SUCCESS);
Expand Down