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

vcs: support explicit fast-forward merging #731

Closed
wants to merge 1 commit into from
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ public Collection<WorkItem> run(Path scratchPath) {
var remoteBranch = new Branch(repo.upstreamFor(toBranch).orElseThrow(() ->
new IllegalStateException("Could not get remote branch name for " + toBranch.name())
));
repo.merge(remoteBranch); // should always be a fast-forward merge
repo.merge(remoteBranch, Repository.FastForward.ONLY);
if (!repo.isClean()) {
throw new RuntimeException("Local repository isn't clean after fast-forward merge - has the fork diverged unexpectedly?");
}
Expand Down
22 changes: 19 additions & 3 deletions vcs/src/main/java/org/openjdk/skara/vcs/Repository.java
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,25 @@ Hash amend(String message,
void prune(Branch branch, String remote) throws IOException;
void delete(Branch b) throws IOException;
void rebase(Hash hash, String committerName, String committerEmail) throws IOException;
void merge(Hash hash) throws IOException;
void merge(Branch branch) throws IOException;
void merge(Hash hash, String strategy) throws IOException;

enum FastForward {
ONLY,
DISABLE,
AUTO
}

default void merge(Hash hash) throws IOException {
merge(hash, FastForward.AUTO);
}
void merge(Hash hash, FastForward ff) throws IOException;
default void merge(Branch branch) throws IOException {
merge(branch, FastForward.AUTO);
}
void merge(Branch branch, FastForward ff) throws IOException;
default void merge(Hash hash, String strategy) throws IOException {
merge(hash, strategy, FastForward.AUTO);
}
void merge(Hash hash, String strategy, FastForward ff) throws IOException;
void abortMerge() throws IOException;
void addRemote(String name, String path) throws IOException;
void setPaths(String remote, String pullPath, String pushPath) throws IOException;
Expand Down
26 changes: 19 additions & 7 deletions vcs/src/main/java/org/openjdk/skara/vcs/git/GitRepository.java
Original file line number Diff line number Diff line change
Expand Up @@ -1093,28 +1093,40 @@ public Repository copyTo(Path destination) throws IOException {
}

@Override
public void merge(Hash h) throws IOException {
merge(h.hex(), null);
public void merge(Hash h, FastForward ff) throws IOException {
merge(h.hex(), null, ff);
}

@Override
public void merge(Branch b) throws IOException {
merge(b.name(), null);
public void merge(Branch b, FastForward ff) throws IOException {
merge(b.name(), null, ff);
}

@Override
public void merge(Hash h, String strategy) throws IOException {
merge(h.hex(), strategy);
public void merge(Hash h, String strategy, FastForward ff) throws IOException {
merge(h.hex(), strategy, ff);
}

private void merge(String ref, String strategy) throws IOException {
private void merge(String ref, String strategy, FastForward ff) throws IOException {
var cmd = new ArrayList<String>();
cmd.addAll(List.of("git", "-c", "user.name=unused", "-c", "user.email=unused",
"merge", "--no-commit"));

if (ff == FastForward.AUTO) {
cmd.add("--ff");
} else if (ff == FastForward.DISABLE) {
cmd.add("--no-ff");
} else if (ff == FastForward.ONLY) {
cmd.add("--ff-only");
} else {
throw new IllegalArgumentException("Unexpected fast forward value: " + ff);
}

if (strategy != null) {
cmd.add("-s");
cmd.add(strategy);
}

cmd.add(ref);
try (var p = capture(cmd)) {
await(p);
Expand Down
44 changes: 33 additions & 11 deletions vcs/src/main/java/org/openjdk/skara/vcs/hg/HgRepository.java
Original file line number Diff line number Diff line change
Expand Up @@ -948,26 +948,48 @@ public Repository copyTo(Path destination) throws IOException {
}

@Override
public void merge(Hash h) throws IOException {
merge(h.hex(), null);
public void merge(Hash h, FastForward ff) throws IOException {
merge(h, null, ff);
}

@Override
public void merge(Branch b) throws IOException {
merge(b.name(), null);
public void merge(Branch b, FastForward ff) throws IOException {
var hash = resolve(b).orElseThrow(() ->
new IOException("Can't lookup branch " + b.name())
);
merge(hash, null, ff);
}

@Override
public void merge(Hash h, String strategy) throws IOException {
merge(h.hex(), strategy);
}
public void merge(Hash other, String strategy, FastForward ff) throws IOException {
var head = head();
List<String> cmd = null;

private void merge(String ref, String strategy) throws IOException {
var cmd = new ArrayList<String>();
cmd.addAll(List.of("hg", "merge", "--rev=" + ref));
var update = List.of("hg", "update", "--rev", other.hex());
var debugsetparents = List.of("hg", "debugsetparents", head.hex(), other.hex());
var merge = new ArrayList<>(List.of("hg", "merge", "--rev=" + other.hex()));
if (strategy != null) {
cmd.add("--tool=" + strategy);
merge.add("--tool=" + strategy);
}

if (ff == FastForward.ONLY) {
cmd = update;
} else if (ff == FastForward.DISABLE) {
if (isAncestor(head, other)) {
cmd = debugsetparents;
} else {
cmd = merge;
}
} else if (ff == FastForward.AUTO) {
if (isAncestor(head, other)) {
cmd = update;
} else {
cmd = merge;
}
} else {
throw new IllegalArgumentException("Unexpected fast forward value: " + ff);
}

try (var p = capture(cmd)) {
await(p);
}
Expand Down
50 changes: 50 additions & 0 deletions vcs/src/test/java/org/openjdk/skara/vcs/RepositoryTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -2521,4 +2521,54 @@ void testMercurialTagWithoutEmail() throws IOException, InterruptedException {
assertNull(annotated.author().email());
}
}

@ParameterizedTest
@EnumSource(VCS.class)
void testNonFastForwardMerge(VCS vcs) throws IOException {
try (var dir = new TemporaryDirectory()) {
var r = Repository.init(dir.path(), vcs);

var readme = dir.path().resolve("README");
Files.write(readme, List.of("Hello, readme!"));

r.add(readme);
var hash1 = r.commit("Add README", "duke", "duke@openjdk.java.net");

Files.write(readme, List.of("Another line"), WRITE, APPEND);
r.add(readme);
var hash2 = r.commit("Modify README", "duke", "duke@openjdk.java.net");

r.checkout(hash1, false);
r.merge(hash2, Repository.FastForward.DISABLE);
var hash3 = r.commit("Non fast-forward merge", "duke", "duke@openjdk.java.net");
var mergeCommit = r.lookup(hash3).orElseThrow();
assertEquals(2, mergeCommit.parents().size());
}
}

@ParameterizedTest
@EnumSource(VCS.class)
void testFastForwardMerge(VCS vcs) throws IOException {
try (var dir = new TemporaryDirectory()) {
var r = Repository.init(dir.path(), vcs);

var readme = dir.path().resolve("README");
Files.write(readme, List.of("Hello, readme!"));

r.add(readme);
var hash1 = r.commit("Add README", "duke", "duke@openjdk.java.net");
var other = r.branch(hash1, "other");
r.checkout(other);

Files.write(readme, List.of("Another line"), WRITE, APPEND);
r.add(readme);
var hash2 = r.commit("Modify README", "duke", "duke@openjdk.java.net");

r.checkout(r.defaultBranch());
r.merge(hash2, Repository.FastForward.AUTO);
var diff = r.diff(r.head());
assertEquals(List.of(), diff.patches());
assertEquals(2, r.commits().asList().size());
}
}
}