From 3c544647b7b3b291ad1f3661881f6d3bf9f90da4 Mon Sep 17 00:00:00 2001 From: Christian Halstrick Date: Fri, 9 Dec 2011 11:17:59 +0100 Subject: [PATCH 1/3] Fix ResolveMerger not to add paths with FileMode 0 When ResolveMerger finds a path where it has to do a content merge it will try the content merge and if that succeeds it'll add the newly produced content to the index. For the FileMode of this new index entry it blindly copies the FileMode it finds for that path in the common base tree. If by chance the common base tree does not contain this path it'll try to add FileMode 0 (MISSING) to the index. One could argue that this can't happen: how can the ResolveMerger successfully (with no conflicts) merge two contents if there is no common base? This was due to another bug in ResolveMerger. It failed to find out that for two files which differ only in the FileMode (e.g. 644 vs. 755) it should not try a content merge. Change-Id: I7a00fe1a6c610679be475cab8a3f8aa4c08811a1 Signed-off-by: Christian Halstrick Signed-off-by: Robin Rosenberg --- .../eclipse/jgit/api/MergeCommandTest.java | 75 +++++++++++++++++ .../org/eclipse/jgit/merge/ResolveMerger.java | 82 +++++++++++++++++-- 2 files changed, 150 insertions(+), 7 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java index bb9058891..d1aaa0a5f 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java @@ -1023,6 +1023,81 @@ public class MergeCommandTest extends RepositoryTestCase { assertFalse(folder2.exists()); } + @Test + public void testFileModeMerge() throws Exception { + Git git = new Git(db); + + writeTrashFile("mergeableMode", "a"); + setExecutable(git, "mergeableMode", false); + writeTrashFile("conflictingModeWithBase", "a"); + setExecutable(git, "conflictingModeWithBase", false); + RevCommit initialCommit = addAllAndCommit(git); + + // switch branch + createBranch(initialCommit, "refs/heads/side"); + checkoutBranch("refs/heads/side"); + setExecutable(git, "mergeableMode", true); + writeTrashFile("conflictingModeNoBase", "b"); + setExecutable(git, "conflictingModeNoBase", true); + RevCommit sideCommit = addAllAndCommit(git); + + // switch branch + createBranch(initialCommit, "refs/heads/side2"); + checkoutBranch("refs/heads/side2"); + setExecutable(git, "mergeableMode", false); + assertFalse(new File(git.getRepository().getWorkTree(), + "conflictingModeNoBase").exists()); + writeTrashFile("conflictingModeNoBase", "b"); + setExecutable(git, "conflictingModeNoBase", false); + addAllAndCommit(git); + + // merge + MergeResult result = git.merge().include(sideCommit.getId()) + .setStrategy(MergeStrategy.RESOLVE).call(); + assertEquals(MergeStatus.CONFLICTING, result.getMergeStatus()); + assertTrue(canExecute(git, "mergeableMode")); + assertFalse(canExecute(git, "conflictingModeNoBase")); + } + + @Test + public void testFileModeMergeWithDirtyWorkTree() throws Exception { + Git git = new Git(db); + + writeTrashFile("mergeableButDirty", "a"); + setExecutable(git, "mergeableButDirty", false); + RevCommit initialCommit = addAllAndCommit(git); + + // switch branch + createBranch(initialCommit, "refs/heads/side"); + checkoutBranch("refs/heads/side"); + setExecutable(git, "mergeableButDirty", true); + RevCommit sideCommit = addAllAndCommit(git); + + // switch branch + createBranch(initialCommit, "refs/heads/side2"); + checkoutBranch("refs/heads/side2"); + setExecutable(git, "mergeableButDirty", false); + addAllAndCommit(git); + + writeTrashFile("mergeableButDirty", "b"); + + // merge + MergeResult result = git.merge().include(sideCommit.getId()) + .setStrategy(MergeStrategy.RESOLVE).call(); + assertEquals(MergeStatus.FAILED, result.getMergeStatus()); + assertFalse(canExecute(git, "mergeableButDirty")); + } + + private void setExecutable(Git git, String path, boolean executable) { + new File(git.getRepository().getWorkTree(), path) + .setExecutable(executable); + assertEquals(executable, canExecute(git, path)); + } + + private boolean canExecute(Git git, String path) { + return (new File(git.getRepository().getWorkTree(), path).canExecute()); + } + private RevCommit addAllAndCommit(final Git git) throws Exception { git.add().addFilepattern(".").call(); return git.commit().setMessage("message").call(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java index b76356895..8211780ca 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java @@ -51,6 +51,7 @@ import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.LinkedList; @@ -378,12 +379,46 @@ public class ResolveMerger extends ThreeWayMerger { if (isIndexDirty()) return false; - if (nonTree(modeO) && modeO == modeT && tw.idEqual(T_OURS, T_THEIRS)) { - // OURS and THEIRS are equal: it doesn't matter which one we choose. - // OURS is chosen. - add(tw.getRawPath(), ours, DirCacheEntry.STAGE_0); - // no checkout needed! - return true; + if (nonTree(modeO) && nonTree(modeT) && tw.idEqual(T_OURS, T_THEIRS)) { + // OURS and THEIRS have equal content. Check the file mode + if (modeO == modeT) { + // content and mode of OURS and THEIRS are equal: it doesn't + // matter which one we choose. OURS is chosen. + add(tw.getRawPath(), ours, DirCacheEntry.STAGE_0); + // no checkout needed! + return true; + } else { + // same content but different mode on OURS and THEIRS. + // Try to merge the mode and report an error if this is + // not possible. + int newMode = mergeFileModes(modeB, modeO, modeT); + if (newMode != FileMode.MISSING.getBits()) { + if (newMode == modeO) + // ours version is preferred + add(tw.getRawPath(), ours, DirCacheEntry.STAGE_0); + else { + // the preferred version THEIRS has a different mode + // than ours. Check it out! + if (isWorktreeDirty()) + return false; + DirCacheEntry e = add(tw.getRawPath(), theirs, + DirCacheEntry.STAGE_0); + toBeCheckedOut.put(tw.getPathString(), e); + } + return true; + } else { + // FileModes are not mergeable. We found a conflict on modes + add(tw.getRawPath(), base, DirCacheEntry.STAGE_1); + add(tw.getRawPath(), ours, DirCacheEntry.STAGE_2); + add(tw.getRawPath(), theirs, DirCacheEntry.STAGE_3); + unmergedPaths.add(tw.getPathString()); + mergeResults.put( + tw.getPathString(), + new MergeResult(Collections + . emptyList())); + } + return true; + } } if (nonTree(modeO) && modeB == modeT && tw.idEqual(T_BASE, T_THEIRS)) { @@ -582,7 +617,12 @@ public class ResolveMerger extends ThreeWayMerger { // no conflict occurred, the file will contain fully merged content. // the index will be populated with the new merged version DirCacheEntry dce = new DirCacheEntry(tw.getPathString()); - dce.setFileMode(tw.getFileMode(0)); + int newMode = mergeFileModes(tw.getRawMode(0), tw.getRawMode(1), + tw.getRawMode(2)); + // set the mode for the new content. Fall back to REGULAR_FILE if + // you can't merge modes of OURS and THEIRS + dce.setFileMode((newMode == FileMode.MISSING.getBits()) ? FileMode.REGULAR_FILE + : FileMode.fromBits(newMode)); dce.setLastModified(of.lastModified()); dce.setLength((int) of.length()); InputStream is = new FileInputStream(of); @@ -599,6 +639,34 @@ public class ResolveMerger extends ThreeWayMerger { } } + /** + * Try to merge filemodes. If only ours or theirs have changed the mode + * (compared to base) we choose that one. If ours and theirs have equal + * modes return that one. If also that is not the case the modes are not + * mergeable. Return {@link FileMode#MISSING} int that case. + * + * @param modeB + * filemode found in BASE + * @param modeO + * filemode found in OURS + * @param modeT + * filemode found in THEIRS + * + * @return the merged filemode or {@link FileMode#MISSING} in case of a + * conflict + */ + private int mergeFileModes(int modeB, int modeO, int modeT) { + if (modeO == modeT) + return modeO; + if (modeB == modeO) + // Base equal to Ours -> chooses Theirs if that is not missing + return (modeT == FileMode.MISSING.getBits()) ? modeO : modeT; + if (modeB == modeT) + // Base equal to Theirs -> chooses Ours if that is not missing + return (modeO == FileMode.MISSING.getBits()) ? modeT : modeO; + return FileMode.MISSING.getBits(); + } + private static RawText getRawText(ObjectId id, Repository db) throws IOException { if (id.equals(ObjectId.zeroId())) From 720119744f6c93c2ef3983067e48eb8e133129d1 Mon Sep 17 00:00:00 2001 From: Robin Rosenberg Date: Thu, 15 Dec 2011 23:33:00 +0100 Subject: [PATCH 2/3] Fix MergeCommandTest to pass if File.executable is not supported Change-Id: If11080ed6e53d9df88a1ae42f48ee8914d54669b --- .../org/eclipse/jgit/api/MergeCommandTest.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java index d1aaa0a5f..30a94520d 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java @@ -60,6 +60,7 @@ import org.eclipse.jgit.lib.RepositoryTestCase; import org.eclipse.jgit.merge.MergeStrategy; import org.eclipse.jgit.merge.ResolveMerger.MergeFailureReason; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.FileUtils; import org.junit.Test; import org.junit.experimental.theories.DataPoints; @@ -1025,6 +1026,9 @@ public class MergeCommandTest extends RepositoryTestCase { @Test public void testFileModeMerge() throws Exception { + if (!FS.DETECTED.supportsExecute()) + return; + // Only Java6 Git git = new Git(db); writeTrashFile("mergeableMode", "a"); @@ -1061,6 +1065,10 @@ public class MergeCommandTest extends RepositoryTestCase { @Test public void testFileModeMergeWithDirtyWorkTree() throws Exception { + if (!FS.DETECTED.supportsExecute()) + return; + // Only Java6 (or set x bit in index) + Git git = new Git(db); writeTrashFile("mergeableButDirty", "a"); @@ -1089,13 +1097,13 @@ public class MergeCommandTest extends RepositoryTestCase { } private void setExecutable(Git git, String path, boolean executable) { - new File(git.getRepository().getWorkTree(), path) - .setExecutable(executable); - assertEquals(executable, canExecute(git, path)); + FS.DETECTED.setExecute( + new File(git.getRepository().getWorkTree(), path), executable); } private boolean canExecute(Git git, String path) { - return (new File(git.getRepository().getWorkTree(), path).canExecute()); + return FS.DETECTED.canExecute(new File(git.getRepository() + .getWorkTree(), path)); } private RevCommit addAllAndCommit(final Git git) throws Exception { From e178ba20d096e9b1572b2fdeb5b1e207ea55802c Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Thu, 15 Dec 2011 15:50:46 -0800 Subject: [PATCH 3/3] Add API checking using clirr In order to generate API reports run: mvn clirr:clirr The reports are generated to the folder target/site/clirr-report.html under the respective project. In order to check API compatibility and fail the build on incompatible changes run: mvn clirr:check For now we compare the API against the latest release 1.1.0.201109151100-r. Bug: 336849 Change-Id: I21baaf3a6883c5b4db263f712705cc7b8ab6d888 Signed-off-by: Matthias Sohn Signed-off-by: Kevin Sawicki --- org.eclipse.jgit.ant/pom.xml | 19 +++++++++++++++++++ org.eclipse.jgit.console/pom.xml | 19 +++++++++++++++++++ org.eclipse.jgit.http.server/pom.xml | 19 +++++++++++++++++++ org.eclipse.jgit.storage.dht/pom.xml | 19 +++++++++++++++++++ org.eclipse.jgit/pom.xml | 20 ++++++++++++++++++++ pom.xml | 19 +++++++++++++++++++ 6 files changed, 115 insertions(+) diff --git a/org.eclipse.jgit.ant/pom.xml b/org.eclipse.jgit.ant/pom.xml index 22bea9aa5..bbb7e59fd 100644 --- a/org.eclipse.jgit.ant/pom.xml +++ b/org.eclipse.jgit.ant/pom.xml @@ -110,6 +110,25 @@ UTF-8 + + + org.codehaus.mojo + clirr-maven-plugin + + + + + + org.codehaus.mojo + clirr-maven-plugin + ${clirr-version} + + ${jgit-last-release-version} + info + + + + diff --git a/org.eclipse.jgit.console/pom.xml b/org.eclipse.jgit.console/pom.xml index a21632dca..db9e7292b 100644 --- a/org.eclipse.jgit.console/pom.xml +++ b/org.eclipse.jgit.console/pom.xml @@ -108,6 +108,25 @@ UTF-8 + + + org.codehaus.mojo + clirr-maven-plugin + + + + + + org.codehaus.mojo + clirr-maven-plugin + ${clirr-version} + + ${jgit-last-release-version} + info + + + + diff --git a/org.eclipse.jgit.http.server/pom.xml b/org.eclipse.jgit.http.server/pom.xml index 433c6682f..413d5d93f 100644 --- a/org.eclipse.jgit.http.server/pom.xml +++ b/org.eclipse.jgit.http.server/pom.xml @@ -125,6 +125,25 @@ + + + org.codehaus.mojo + clirr-maven-plugin + + + + + + org.codehaus.mojo + clirr-maven-plugin + ${clirr-version} + + ${jgit-last-release-version} + info + + + + diff --git a/org.eclipse.jgit.storage.dht/pom.xml b/org.eclipse.jgit.storage.dht/pom.xml index bd1d2fd5a..2c2b37e98 100644 --- a/org.eclipse.jgit.storage.dht/pom.xml +++ b/org.eclipse.jgit.storage.dht/pom.xml @@ -157,6 +157,25 @@ + + + org.codehaus.mojo + clirr-maven-plugin + + + + + + org.codehaus.mojo + clirr-maven-plugin + ${clirr-version} + + ${jgit-last-release-version} + info + + + + diff --git a/org.eclipse.jgit/pom.xml b/org.eclipse.jgit/pom.xml index 97e08f6e7..33e972e37 100644 --- a/org.eclipse.jgit/pom.xml +++ b/org.eclipse.jgit/pom.xml @@ -142,7 +142,13 @@ + + + org.codehaus.mojo + clirr-maven-plugin + + @@ -155,4 +161,18 @@ + + + + + org.codehaus.mojo + clirr-maven-plugin + ${clirr-version} + + ${jgit-last-release-version} + info + + + + diff --git a/pom.xml b/pom.xml index 87953f796..8f8adc107 100644 --- a/pom.xml +++ b/pom.xml @@ -130,14 +130,23 @@ yyyyMMddHHmm ${project.build.directory}/META-INF/MANIFEST.MF + 1.1.0.201109151100-r 0.1.44-1 4.5 2.0.12 2.5 7.1.6.v20100715 2.4.0a + 2.3 + + + jgit-repository + http://download.eclipse.org/jgit/maven + + + @@ -246,6 +255,16 @@ + + + org.codehaus.mojo + clirr-maven-plugin + ${clirr-version} + + ${jgit-last-release-version} + info + +