Make RepositoryMapping use java.nio.file.Path

org.eclipse.resources.Path.fromOSString normalizes paths. However, the
FileRepositoryBuilder returns paths like
/some/path/container/../.git/modules/... . If that get's normalized, we
end up with /some/path/.git/modules/... , which defeats the subsequent
attempt to relativize the string to ../.git/modules/... .

Using java.nio.file.Path, we can avoid this premature normalization,
and moreover we can create relative paths in way more cases.

Change-Id: I19da4aed3a4a8476f6cb4b1466fa5ab75b095f66
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

2 files changed, 71 insertions, 34 deletions

diff --git a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/project/RepositoryMappingTest.java b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/project/RepositoryMappingTest.java
index ccc158573c..0d22fdfc57 100644
--- a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/project/RepositoryMappingTest.java
+++ b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/project/RepositoryMappingTest.java
@@ -16,6 +16,7 @@ import static org.junit.Assert.assertNull;
 import java.io.File;
 
 import org.eclipse.core.resources.IFile;
+import org.eclipse.core.resources.IProject;
 import org.eclipse.core.resources.IWorkspaceRoot;
 import org.eclipse.core.resources.ResourcesPlugin;
 import org.eclipse.core.runtime.IPath;
@@ -118,6 +119,51 @@ public class RepositoryMappingTest extends GitTestCase {
 				mapping.getGitDirAbsolutePath());
 	}
 
+	/**
+	 * Tests that a {@link RepositoryMapping} internally uses a relative path if
+	 * at all possible.
+	 */
+	@Test
+	public void shouldResolveAsRelativePath() {
+		IProject proj = project.getProject();
+		IPath projectPath = proj.getLocation()
+				.removeTrailingSeparator();
+		String gitHereTest = ".git";
+		String gitTest = "../../.git";
+		String gitSubdirTest = "foobar/.git";
+		String gitSubmoduleTest = "../../.git/modules/submodule";
+		// Construct an absolute path different from the project location:
+		// should be preserved. upToSegment ensures we don't loose the root
+		// component.
+		String gitAbsolute = projectPath.uptoSegment(0)
+				.append(projectPath.segment(0) + "fake").append(".git")
+				.toOSString();
+		String parents = "";
+		while (projectPath.segmentCount() > 2) {
+			String pathString = projectPath.toOSString();
+			assertRepoMappingPath(proj, pathString, gitHereTest, parents);
+			assertRepoMappingPath(proj, pathString, gitTest, parents);
+			assertRepoMappingPath(proj, pathString, gitSubdirTest, parents);
+			assertRepoMappingPath(proj, pathString, gitSubmoduleTest, parents);
+			assertRepoMappingPath(proj, pathString, gitAbsolute, "");
+			projectPath = projectPath.removeLastSegments(1);
+			parents += "../";
+		}
+	}
+
+	private void assertRepoMappingPath(IProject testProject, String path,
+			String testDir, String parents) {
+		String testDirOS = testDir.replace('/', File.separatorChar);
+		File testFile = new File(testDirOS);
+		if (!testFile.isAbsolute()) {
+			testFile = new File(new File(path), testDirOS);
+		}
+		RepositoryMapping mapping = RepositoryMapping.create(testProject,
+				testFile);
+		assertEquals(parents + testDir,
+				mapping.getGitDirPath().toPortableString());
+	}
+
 	private IPath getWorkTreePath() {
 		return new Path(repository.getWorkTree().getAbsolutePath());
 	}
diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/project/RepositoryMapping.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/project/RepositoryMapping.java
index 752697f679..562ca59159 100644
--- a/org.eclipse.egit.core/src/org/eclipse/egit/core/project/RepositoryMapping.java
+++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/project/RepositoryMapping.java
@@ -66,10 +66,14 @@ public class RepositoryMapping {
 			.getDefault().getRepositoryCache();
 
 	/**
-	 * Construct a {@link RepositoryMapping} for a previously connected project.
+	 * Construct a {@link RepositoryMapping} for a previously connected
+	 * container.
 	 *
-	 * @param p TODO
-	 * @param initialKey TODO
+	 * @param p
+	 *            {@link Properties} to read the git directory from. See also
+	 *            {@link GitProjectData}.
+	 * @param initialKey
+	 *            property key to use to read the git directory
 	 */
 	public RepositoryMapping(@NonNull final Properties p,
 			@NonNull final String initialKey) {
@@ -80,12 +84,14 @@ public class RepositoryMapping {
 	}
 
 	/**
-	 * Construct a {@link RepositoryMapping} for previously unknown project.
+	 * Construct a {@link RepositoryMapping} for previously unmapped container.
 	 *
 	 * @param mappedContainer
+	 *            to create the mapping for
 	 * @param gitDir
-	 * @return a new RepositoryMapping for given container. Returns <code>null
-	 *         <code> if container does not exists.
+	 *            for the new mapping
+	 * @return a new RepositoryMapping for given container. Returns {@code null}
+	 *         if container does not exist or is not local.
 	 */
 	@Nullable
 	public static RepositoryMapping create(@NonNull IContainer mappedContainer,
@@ -97,42 +103,27 @@ public class RepositoryMapping {
 		return new RepositoryMapping(mappedContainer, location, gitDir);
 	}
 
-	private RepositoryMapping(@NonNull
-	final IContainer mappedContainer, final @NonNull IPath location,
-			@NonNull final File gitDir) {
+	private RepositoryMapping(final @NonNull IContainer mappedContainer,
+			final @NonNull IPath location, final @NonNull File gitDir) {
 		container = mappedContainer;
 		containerPathString = container.getProjectRelativePath()
 				.toPortableString();
-		final IPath gLoc = Path.fromOSString(gitDir.getPath())
-				.removeTrailingSeparator();
 		if (!gitDir.isAbsolute()) {
 			// It is relative to location, so we can set it right away.
-			gitDirPathString = gLoc.toPortableString();
+			gitDirPathString = Path.fromOSString(gitDir.getPath())
+					.removeTrailingSeparator().toPortableString();
 			return;
 		}
-		final IPath cLoc = location.removeTrailingSeparator();
-		final IPath gLocParent = gLoc.removeLastSegments(1);
-
-		if (cLoc.isPrefixOf(gLoc)) {
-			int matchingSegments = gLoc.matchingFirstSegments(cLoc);
-			IPath remainder = gLoc.removeFirstSegments(matchingSegments);
-			String device = remainder.getDevice();
-			if (device == null)
-				gitDirPathString = remainder.toPortableString();
-			else
-				gitDirPathString = remainder.toPortableString().substring(
-						device.length());
-		} else if (gLocParent.isPrefixOf(cLoc)) {
-			int cnt = cLoc.segmentCount() - cLoc.matchingFirstSegments(gLocParent);
-			StringBuilder p = new StringBuilder("");  //$NON-NLS-1$
-			while (cnt-- > 0) {
-				p.append("../");  //$NON-NLS-1$
-			}
-			p.append(gLoc.segment(gLoc.segmentCount() - 1));
-			gitDirPathString = p.toString();
-		} else {
-			gitDirPathString = gLoc.toPortableString();
+		java.nio.file.Path gPath = gitDir.toPath();
+		java.nio.file.Path lPath = location.toFile().toPath();
+		// Require at least one common component for using a relative path
+		if (lPath.getNameCount() > 0 && gPath.getNameCount() > 0
+				&& gPath.getRoot().equals(lPath.getRoot())
+				&& gPath.getName(0).equals(lPath.getName(0))) {
+			gPath = lPath.relativize(gPath);
 		}
+		gitDirPathString = Path.fromOSString(gPath.toString())
+				.removeTrailingSeparator().toPortableString();
 	}
 
 	/**