Browse Source

WorkingTreeModifiedEvent: must be fired explicitly after merge

A merge may write files to the working tree. After a successful
merge one must fire a WorkingTreeModifiedEvent explicitly if
getModifiedFiles() is not empty.

Also, any touched files must be reported by the
WorkingTreeModifiedEvent fired by DirCacheCheckout.checkout().

Bug: 552636
Change-Id: I5fab8279ed8be8a4ae34cddfa726836b9277aea6
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
next
Thomas Wolf 5 years ago
parent
commit
64f2407f19
  1. 11
      org.eclipse.jgit.test/src/org/eclipse/jgit/events/ChangeRecorder.java
  2. 34
      org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CherryPickCommandTest.java
  3. 14
      org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java
  4. 4
      org.eclipse.jgit/src/org/eclipse/jgit/api/CherryPickCommand.java
  5. 5
      org.eclipse.jgit/src/org/eclipse/jgit/api/RevertCommand.java
  6. 30
      org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java

11
org.eclipse.jgit.test/src/org/eclipse/jgit/events/ChangeRecorder.java

@ -43,7 +43,6 @@
package org.eclipse.jgit.events;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import java.util.Arrays;
@ -99,10 +98,12 @@ public class ChangeRecorder implements WorkingTreeModifiedListener {
Arrays.sort(expectedModified);
Arrays.sort(actuallyDeleted);
Arrays.sort(expectedDeleted);
assertArrayEquals("Unexpected modifications reported", expectedModified,
actuallyModified);
assertArrayEquals("Unexpected deletions reported", expectedDeleted,
actuallyDeleted);
assertEquals("Unexpected modifications reported",
Arrays.toString(expectedModified),
Arrays.toString(actuallyModified));
assertEquals("Unexpected deletions reported",
Arrays.toString(expectedDeleted),
Arrays.toString(actuallyDeleted));
reset();
}
}

34
org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CherryPickCommandTest.java

@ -345,6 +345,40 @@ public class CherryPickCommandTest extends RepositoryTestCase {
}
}
@Test
public void testCherryPickNewFileFiresModifiedEvent() throws Exception {
ListenerHandle listener = null;
try (Git git = new Git(db)) {
writeTrashFile("test.txt", "a");
git.add().addFilepattern("test.txt").call();
git.commit().setMessage("commit1").call();
git.checkout().setCreateBranch(true).setName("a").call();
writeTrashFile("side.txt", "side");
git.add().addFilepattern("side.txt").call();
RevCommit side = git.commit().setMessage("side").call();
assertNotNull(side);
assertNotNull(git.checkout().setName(Constants.MASTER).call());
writeTrashFile("test.txt", "b");
assertNotNull(git.add().addFilepattern("test.txt").call());
assertNotNull(git.commit().setMessage("commit2").call());
ChangeRecorder recorder = new ChangeRecorder();
listener = db.getListenerList()
.addWorkingTreeModifiedListener(recorder);
CherryPickResult result = git.cherryPick()
.include(side.getId()).call();
assertEquals(CherryPickStatus.OK, result.getStatus());
recorder.assertEvent(new String[] { "side.txt" },
ChangeRecorder.EMPTY);
} finally {
if (listener != null) {
listener.remove();
}
}
}
@Test
public void testCherryPickOurCommitName() throws Exception {
try (Git git = new Git(db)) {

14
org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java

@ -57,12 +57,9 @@ import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStreamReader;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import org.eclipse.jgit.api.MergeResult.MergeStatus;
import org.eclipse.jgit.api.RebaseCommand.InteractiveHandler;
@ -78,6 +75,7 @@ import org.eclipse.jgit.errors.AmbiguousObjectException;
import org.eclipse.jgit.errors.IllegalTodoFileModification;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.events.ChangeRecorder;
import org.eclipse.jgit.events.ListenerHandle;
import org.eclipse.jgit.junit.RepositoryTestCase;
import org.eclipse.jgit.lib.AbbreviatedObjectId;
@ -2014,10 +2012,9 @@ public class RebaseCommandTest extends RepositoryTestCase {
checkoutBranch("refs/heads/topic");
writeTrashFile("sub/file0", "unstaged modified file0");
Set<String> modifiedFiles = new HashSet<>();
ChangeRecorder recorder = new ChangeRecorder();
ListenerHandle handle = db.getListenerList()
.addWorkingTreeModifiedListener(
event -> modifiedFiles.addAll(event.getModified()));
.addWorkingTreeModifiedListener(recorder);
try {
// rebase
assertEquals(Status.OK, git.rebase()
@ -2035,9 +2032,8 @@ public class RebaseCommandTest extends RepositoryTestCase {
+ "[sub/file0, mode:100644, content:file0]",
indexState(CONTENT));
assertEquals(RepositoryState.SAFE, db.getRepositoryState());
List<String> modified = new ArrayList<>(modifiedFiles);
Collections.sort(modified);
assertEquals("[file1, sub/file0]", modified.toString());
recorder.assertEvent(new String[] { "file1", "file2", "sub/file0" },
new String[0]);
}
@Test

4
org.eclipse.jgit/src/org/eclipse/jgit/api/CherryPickCommand.java

@ -160,6 +160,10 @@ public class CherryPickCommand extends GitCommand<CherryPickResult> {
merger.setCommitNames(new String[] { "BASE", ourName, //$NON-NLS-1$
cherryPickName });
if (merger.merge(newHead, srcCommit)) {
if (!merger.getModifiedFiles().isEmpty()) {
repo.fireEvent(new WorkingTreeModifiedEvent(
merger.getModifiedFiles(), null));
}
if (AnyObjectId.isEqual(newHead.getTree().getId(),
merger.getResultTreeId())) {
continue;

5
org.eclipse.jgit/src/org/eclipse/jgit/api/RevertCommand.java

@ -58,6 +58,7 @@ import org.eclipse.jgit.api.errors.NoMessageException;
import org.eclipse.jgit.api.errors.UnmergedPathsException;
import org.eclipse.jgit.api.errors.WrongRepositoryStateException;
import org.eclipse.jgit.dircache.DirCacheCheckout;
import org.eclipse.jgit.events.WorkingTreeModifiedEvent;
import org.eclipse.jgit.internal.JGitText;
import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.Constants;
@ -175,6 +176,10 @@ public class RevertCommand extends GitCommand<RevCommit> {
+ "This reverts commit " + srcCommit.getId().getName() //$NON-NLS-1$
+ ".\n"; //$NON-NLS-1$
if (merger.merge(headCommit, srcParent)) {
if (!merger.getModifiedFiles().isEmpty()) {
repo.fireEvent(new WorkingTreeModifiedEvent(
merger.getModifiedFiles(), null));
}
if (AnyObjectId.isEqual(headCommit.getTree().getId(),
merger.getResultTreeId()))
continue;

30
org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java vendored

@ -53,9 +53,11 @@ import java.text.MessageFormat;
import java.time.Instant;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.eclipse.jgit.api.errors.CanceledException;
import org.eclipse.jgit.api.errors.FilterFailedException;
@ -142,6 +144,8 @@ public class DirCacheCheckout {
private ArrayList<String> removed = new ArrayList<>();
private ArrayList<String> kept = new ArrayList<>();
private ObjectId mergeCommitTree;
private DirCache dc;
@ -432,11 +436,11 @@ public class DirCacheCheckout {
if (mtime == null || mtime.equals(Instant.EPOCH)) {
entry.setLastModified(f.getEntryLastModifiedInstant());
}
keep(entry, f);
keep(i.getEntryPathString(), entry, f);
}
} else
// The index contains a folder
keep(i.getDirCacheEntry(), f);
keep(i.getEntryPathString(), i.getDirCacheEntry(), f);
} else {
// There is no entry in the merge commit. Means: we want to delete
// what's currently in the index and working tree
@ -496,8 +500,11 @@ public class DirCacheCheckout {
dc.unlock();
} finally {
if (performingCheckout) {
Set<String> touched = new HashSet<>(conflicts);
touched.addAll(getUpdated().keySet());
touched.addAll(kept);
WorkingTreeModifiedEvent event = new WorkingTreeModifiedEvent(
getUpdated().keySet(), getRemoved());
touched, getRemoved());
if (!event.isEmpty()) {
repo.fireEvent(event);
}
@ -826,14 +833,14 @@ public class DirCacheCheckout {
break;
case 0xDFD: // 3 4
keep(dce, f);
keep(name, dce, f);
break;
case 0xF0D: // 18
remove(name);
break;
case 0xDFF: // 5 5b 6 6b
if (equalIdAndMode(iId, iMode, mId, mMode))
keep(dce, f); // 5 6
keep(name, dce, f); // 5 6
else
conflict(name, dce, h, m); // 5b 6b
break;
@ -863,7 +870,7 @@ public class DirCacheCheckout {
conflict(name, dce, h, m); // 9
break;
case 0xFD0: // keep without a rule
keep(dce, f);
keep(name, dce, f);
break;
case 0xFFD: // 12 13 14
if (equalIdAndMode(hId, hMode, iId, iMode))
@ -883,7 +890,7 @@ public class DirCacheCheckout {
conflict(name, dce, h, m);
break;
default:
keep(dce, f);
keep(name, dce, f);
}
return;
}
@ -968,7 +975,7 @@ public class DirCacheCheckout {
if (initialCheckout)
update(name, mId, mMode);
else
keep(dce, f);
keep(name, dce, f);
} else
conflict(name, dce, h, m);
}
@ -1031,7 +1038,7 @@ public class DirCacheCheckout {
// Nothing in Head
// Something in Index
// -> Merge contains nothing new. Keep the index.
keep(dce, f);
keep(name, dce, f);
} else
// Merge contains something and it is not the same as Index
// Nothing in Head
@ -1182,7 +1189,7 @@ public class DirCacheCheckout {
// to the other one.
// -> In all three cases we don't touch index and file.
keep(dce, f);
keep(name, dce, f);
}
}
}
@ -1231,12 +1238,13 @@ public class DirCacheCheckout {
}
}
private void keep(DirCacheEntry e, WorkingTreeIterator f)
private void keep(String path, DirCacheEntry e, WorkingTreeIterator f)
throws IOException {
if (e != null && !FileMode.TREE.equals(e.getFileMode()))
builder.add(e);
if (force) {
if (f.isModified(e, true, this.walk.getObjectReader())) {
kept.add(path);
checkoutEntry(repo, e, this.walk.getObjectReader());
}
}

Loading…
Cancel
Save