"Bare" Repository should not return working directory.

If a repository is "bare", it currently still returns a working directory.
This conflicts with the specification of "bare"-ness.

Bug: 311902

Change-Id: Ib54b31ddc80b9032e6e7bf013948bb83e12cfd88
Signed-off-by: Mathias Kinzler <mathias.kinzler@sap.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositorySetupWorkDirTest.java

@@ -0,0 +1,208 @@
+/*
+ * Copyright (C) 2010, Mathias Kinzler <mathias.kinzler@sap.com>
+ *
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Distribution License v1.0 which
+ * accompanies this distribution, is reproduced below, and is
+ * available at http://www.eclipse.org/org/documents/edl-v10.php
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Eclipse Foundation, Inc. nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.eclipse.jgit.lib;
+
+import java.io.File;
+import java.io.IOException;
+
+import org.eclipse.jgit.junit.LocalDiskRepositoryTestCase;
+
+/**
+ * Tests for setting up the working directory when creating a Repository
+ */
+public class RepositorySetupWorkDirTest extends LocalDiskRepositoryTestCase {
+
+	public void testIsBare_CreateRepositoryFromArbitraryGitDir()
+			throws Exception {
+		File gitDir = getFile("workdir");
+		assertTrue(new Repository(gitDir).isBare());
+	}
+
+	public void testNotBare_CreateRepositoryFromDotGitGitDir() throws Exception {
+		File gitDir = getFile("workdir", Constants.DOT_GIT);
+		Repository repo = new Repository(gitDir);
+		assertFalse(repo.isBare());
+		assertWorkdirPath(repo, "workdir");
+		assertGitdirPath(repo, "workdir", Constants.DOT_GIT);
+	}
+
+	public void testWorkdirIsParentDir_CreateRepositoryFromDotGitGitDir()
+			throws Exception {
+		File gitDir = getFile("workdir", Constants.DOT_GIT);
+		Repository repo = new Repository(gitDir);
+		String workdir = repo.getWorkDir().getName();
+		assertEquals(workdir, "workdir");
+	}
+
+	public void testNotBare_CreateRepositoryFromWorkDirOnly() throws Exception {
+		File workdir = getFile("workdir", "repo");
+		Repository repo = new Repository(null, workdir);
+		assertFalse(repo.isBare());
+		assertWorkdirPath(repo, "workdir", "repo");
+		assertGitdirPath(repo, "workdir", "repo", Constants.DOT_GIT);
+	}
+
+	public void testWorkdirIsDotGit_CreateRepositoryFromWorkDirOnly()
+			throws Exception {
+		File workdir = getFile("workdir", "repo");
+		Repository repo = new Repository(null, workdir);
+		assertGitdirPath(repo, "workdir", "repo", Constants.DOT_GIT);
+	}
+
+	public void testNotBare_CreateRepositoryFromGitDirOnlyWithWorktreeConfig()
+			throws Exception {
+		File gitDir = getFile("workdir", "repoWithConfig");
+		File workTree = getFile("workdir", "treeRoot");
+		setWorkTree(gitDir, workTree);
+		Repository repo = new Repository(gitDir, null);
+		assertFalse(repo.isBare());
+		assertWorkdirPath(repo, "workdir", "treeRoot");
+		assertGitdirPath(repo, "workdir", "repoWithConfig");
+	}
+
+	public void testBare_CreateRepositoryFromGitDirOnlyWithBareConfigTrue()
+			throws Exception {
+		File gitDir = getFile("workdir", "repoWithConfig");
+		setBare(gitDir, true);
+		Repository repo = new Repository(gitDir, null);
+		assertTrue(repo.isBare());
+	}
+
+	public void testWorkdirIsParent_CreateRepositoryFromGitDirOnlyWithBareConfigFalse()
+			throws Exception {
+		File gitDir = getFile("workdir", "repoWithBareConfigTrue", "child");
+		setBare(gitDir, false);
+		Repository repo = new Repository(gitDir, null);
+		assertWorkdirPath(repo, "workdir", "repoWithBareConfigTrue");
+	}
+
+	public void testNotBare_CreateRepositoryFromGitDirOnlyWithBareConfigFalse()
+			throws Exception {
+		File gitDir = getFile("workdir", "repoWithBareConfigFalse", "child");
+		setBare(gitDir, false);
+		Repository repo = new Repository(gitDir, null);
+		assertFalse(repo.isBare());
+		assertWorkdirPath(repo, "workdir", "repoWithBareConfigFalse");
+		assertGitdirPath(repo, "workdir", "repoWithBareConfigFalse", "child");
+	}
+
+	public void testNotBare_MakeBareUnbareBySetWorkdir() throws Exception {
+		File gitDir = getFile("gitDir");
+		Repository repo = new Repository(gitDir);
+		repo.setWorkDir(getFile("workingDir"));
+		assertFalse(repo.isBare());
+		assertWorkdirPath(repo, "workingDir");
+		assertGitdirPath(repo, "gitDir");
+	}
+
+	public void testExceptionThrown_BareRepoGetWorkDir() throws Exception {
+		File gitDir = getFile("workdir");
+		try {
+			new Repository(gitDir).getWorkDir();
+			fail("Expected IllegalStateException missing");
+		} catch (IllegalStateException e) {
+			// expected
+		}
+	}
+
+	public void testExceptionThrown_BareRepoGetIndex() throws Exception {
+		File gitDir = getFile("workdir");
+		try {
+			new Repository(gitDir).getIndex();
+			fail("Expected IllegalStateException missing");
+		} catch (IllegalStateException e) {
+			// expected
+		}
+	}
+
+	public void testExceptionThrown_BareRepoGetIndexFile() throws Exception {
+		File gitDir = getFile("workdir");
+		try {
+			new Repository(gitDir).getIndexFile();
+			fail("Expected Exception missing");
+		} catch (IllegalStateException e) {
+			// expected
+		}
+	}
+
+	private File getFile(String... pathComponents) {
+		String rootPath = new File(new File("target"), "trash").getPath();
+		for (String pathComponent : pathComponents)
+			rootPath = rootPath + File.separatorChar + pathComponent;
+		File result = new File(rootPath);
+		result.mkdir();
+		return result;
+	}
+
+	private void setBare(File gitDir, boolean bare) throws IOException {
+		Repository repo = new Repository(gitDir, null);
+		repo.getConfig().setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null,
+				ConfigConstants.CONFIG_KEY_BARE, bare);
+		repo.getConfig().save();
+	}
+
+	private void setWorkTree(File gitDir, File workTree) throws IOException {
+		Repository repo = new Repository(gitDir, null);
+		repo.getConfig()
+				.setString(ConfigConstants.CONFIG_CORE_SECTION, null,
+						ConfigConstants.CONFIG_KEY_WORKTREE,
+						workTree.getAbsolutePath());
+		repo.getConfig().save();
+	}
+
+	private void assertGitdirPath(Repository repo, String... expected)
+			throws IOException {
+		File exp = getFile(expected).getCanonicalFile();
+		File act = repo.getDirectory().getCanonicalFile();
+		assertEquals("Wrong Git Directory", exp, act);
+	}
+
+	private void assertWorkdirPath(Repository repo, String... expected)
+			throws IOException {
+		File exp = getFile(expected).getCanonicalFile();
+		File act = repo.getWorkDir().getCanonicalFile();
+		assertEquals("Wrong working Directory", exp, act);
+	}
+}

org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties

@@ -23,6 +23,7 @@ badObjectType=Bad object type: {0}
 badSectionEntry=Bad section entry: {0}
 base64InputNotProperlyPadded=Base64 input not properly padded.
 baseLengthIncorrect=base length incorrect
+bareRepositoryNoWorkdirAndIndex=Bare Repository has neither a working tree, nor an index
 blobNotFound=Blob not found: {0}
 blobNotFoundForPath=Blob not found: {0} for path: {1}
 cannotBeCombined=Cannot be combined.

org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java

@@ -83,6 +83,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String badSectionEntry;
 	/***/ public String base64InputNotProperlyPadded;
 	/***/ public String baseLengthIncorrect;
+	/***/ public String bareRepositoryNoWorkdirAndIndex;
 	/***/ public String blobNotFound;
 	/***/ public String blobNotFoundForPath;
 	/***/ public String cannotBeCombined;

org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright (C) 2010, Mathias Kinzler <mathias.kinzler@sap.com>
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Distribution License v1.0 which
+ * accompanies this distribution, is reproduced below, and is
+ * available at http://www.eclipse.org/org/documents/edl-v10.php
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Eclipse Foundation, Inc. nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.eclipse.jgit.lib;
+
+/**
+ * Constants for use with the Configuration classes: section names,
+ * configuration keys
+ */
+public class ConfigConstants {
+	/** The "core" section */
+	public static final String CONFIG_CORE_SECTION = "core";
+
+	/** The "autocrlf" key */
+	public static final String CONFIG_KEY_AUTOCRLF = "autocrlf";
+
+	/** The "bare" key */
+	public static final String CONFIG_KEY_BARE = "bare";
+
+	/** The "filemode" key */
+	public static final String CONFIG_KEY_FILEMODE = "filemode";
+
+	/** The "logallrefupdates" key */
+	public static final String CONFIG_KEY_LOGALLREFUPDATES = "logallrefupdates";
+
+	/** The "repositoryformatversion" key */
+	public static final String CONFIG_KEY_REPO_FORMAT_VERSION = "repositoryformatversion";
+
+	/** The "worktree" key */
+	public static final String CONFIG_KEY_WORKTREE = "worktree";
+}

org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java

@@ -168,15 +168,15 @@ public class Repository {
 	 *            for default value in which case it depends on GIT_WORK_TREE.
 	 * @param workTree
 	 *            GIT_WORK_TREE (the root of the checkout). May be null for
-	 *            default value if GIT_DIR is
+	 *            default value if GIT_DIR is provided.
 	 * @param objectDir
 	 *            GIT_OBJECT_DIRECTORY (where objects and are stored). May be
 	 *            null for default value. Relative names ares resolved against
-	 *            GIT_WORK_TREE
+	 *            GIT_WORK_TREE.
 	 * @param alternateObjectDir
 	 *            GIT_ALTERNATE_OBJECT_DIRECTORIES (where more objects are read
 	 *            from). May be null for default value. Relative names ares
-	 *            resolved against GIT_WORK_TREE
+	 *            resolved against GIT_WORK_TREE.
 	 * @param indexFile
 	 *            GIT_INDEX_FILE (the location of the index file). May be null
 	 *            for default value. Relative names ares resolved against
@@ -199,15 +199,15 @@ public class Repository {
 	 *            for default value in which case it depends on GIT_WORK_TREE.
 	 * @param workTree
 	 *            GIT_WORK_TREE (the root of the checkout). May be null for
-	 *            default value if GIT_DIR is
+	 *            default value if GIT_DIR is provided.
 	 * @param objectDir
 	 *            GIT_OBJECT_DIRECTORY (where objects and are stored). May be
 	 *            null for default value. Relative names ares resolved against
-	 *            GIT_WORK_TREE
+	 *            GIT_WORK_TREE.
 	 * @param alternateObjectDir
 	 *            GIT_ALTERNATE_OBJECT_DIRECTORIES (where more objects are read
 	 *            from). May be null for default value. Relative names ares
-	 *            resolved against GIT_WORK_TREE
+	 *            resolved against GIT_WORK_TREE.
 	 * @param indexFile
 	 *            GIT_INDEX_FILE (the location of the index file). May be null
 	 *            for default value. Relative names ares resolved against
@@ -220,8 +220,8 @@ public class Repository {
 	 *             accessed.
 	 */
 	public Repository(final File d, final File workTree, final File objectDir,
-			final File[] alternateObjectDir, final File indexFile,
-			FS fs) throws IOException {
+			final File[] alternateObjectDir, final File indexFile, FS fs)
+			throws IOException {
 
 		if (workTree != null) {
 			workDir = workTree;
@@ -233,7 +233,8 @@ public class Repository {
 			if (d != null)
 				gitDir = d;
 			else
-				throw new IllegalArgumentException(JGitText.get().eitherGIT_DIRorGIT_WORK_TREEmustBePassed);
+				throw new IllegalArgumentException(
+						JGitText.get().eitherGIT_DIRorGIT_WORK_TREEmustBePassed);
 		}
 
 		this.fs = fs;
@@ -245,11 +246,33 @@ public class Repository {
 		loadConfig();
 
 		if (workDir == null) {
-			String workTreeConfig = getConfig().getString("core", null, "worktree");
+			// if the working directory was not provided explicitly,
+			// we need to decide if this is a "bare" repository or not
+			// first, we check the working tree configuration
+			String workTreeConfig = getConfig().getString(
+					ConfigConstants.CONFIG_CORE_SECTION, null,
+					ConfigConstants.CONFIG_KEY_WORKTREE);
 			if (workTreeConfig != null) {
+				// the working tree configuration wins
 				workDir = fs.resolve(d, workTreeConfig);
-			} else {
+			} else if (getConfig().getString(
+					ConfigConstants.CONFIG_CORE_SECTION, null,
+					ConfigConstants.CONFIG_KEY_BARE) != null) {
+				// we have asserted that a value for the "bare" flag was set
+				if (!getConfig().getBoolean(ConfigConstants.CONFIG_CORE_SECTION,
+						ConfigConstants.CONFIG_KEY_BARE, true))
+					// the "bare" flag is false -> use the parent of the
+					// meta data directory
+					workDir = gitDir.getParentFile();
+				else
+					// the "bare" flag is true
+					workDir = null;
+			} else if (Constants.DOT_GIT.equals(gitDir.getName())) {
+				// no value for the "bare" flag, but the meta data directory
+				// is named ".git" -> use the parent of the meta data directory
 				workDir = gitDir.getParentFile();
+			} else {
+				workDir = null;
 			}
 		}
 
@@ -268,10 +291,12 @@ public class Repository {
 
 		if (objectDatabase.exists()) {
 			final String repositoryFormatVersion = getConfig().getString(
-					"core", null, "repositoryFormatVersion");
+					ConfigConstants.CONFIG_CORE_SECTION, null,
+					ConfigConstants.CONFIG_KEY_REPO_FORMAT_VERSION);
 			if (!"0".equals(repositoryFormatVersion)) {
 				throw new IOException(MessageFormat.format(
-						JGitText.get().unknownRepositoryFormat2, repositoryFormatVersion));
+						JGitText.get().unknownRepositoryFormat2,
+						repositoryFormatVersion));
 			}
 		}
 	}
@@ -280,8 +305,9 @@ public class Repository {
 		try {
 			userConfig.load();
 		} catch (ConfigInvalidException e1) {
-			IOException e2 = new IOException(MessageFormat.format(
-					JGitText.get().userConfigFileInvalid, userConfig.getFile().getAbsolutePath(), e1));
+			IOException e2 = new IOException(MessageFormat.format(JGitText
+					.get().userConfigFileInvalid, userConfig.getFile()
+					.getAbsolutePath(), e1));
 			e2.initCause(e1);
 			throw e2;
 		}
@@ -322,7 +348,8 @@ public class Repository {
 	public void create(boolean bare) throws IOException {
 		final RepositoryConfig cfg = getConfig();
 		if (cfg.getFile().exists()) {
-			throw new IllegalStateException(MessageFormat.format(JGitText.get().repositoryAlreadyExists, gitDir));
+			throw new IllegalStateException(MessageFormat.format(
+					JGitText.get().repositoryAlreadyExists, gitDir));
 		}
 		gitDir.mkdirs();
 		refs.create();
@@ -334,12 +361,17 @@ public class Repository {
 		head.disableRefLog();
 		head.link(Constants.R_HEADS + Constants.MASTER);
 
-		cfg.setInt("core", null, "repositoryformatversion", 0);
-		cfg.setBoolean("core", null, "filemode", true);
+		cfg.setInt(ConfigConstants.CONFIG_CORE_SECTION, null,
+				ConfigConstants.CONFIG_KEY_REPO_FORMAT_VERSION, 0);
+		cfg.setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null,
+				ConfigConstants.CONFIG_KEY_FILEMODE, true);
 		if (bare)
-			cfg.setBoolean("core", null, "bare", true);
-		cfg.setBoolean("core", null, "logallrefupdates", !bare);
-		cfg.setBoolean("core", null, "autocrlf", false);
+			cfg.setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null,
+					ConfigConstants.CONFIG_KEY_BARE, true);
+		cfg.setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null,
+				ConfigConstants.CONFIG_KEY_LOGALLREFUPDATES, !bare);
+		cfg.setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null,
+				ConfigConstants.CONFIG_KEY_AUTOCRLF, false);
 		cfg.save();
 	}
 
@@ -1114,10 +1146,17 @@ public class Repository {
 	}
 
 	/**
-	 * @return a representation of the index associated with this repo
+	 * @return a representation of the index associated with this
+	 *         {@link Repository}
 	 * @throws IOException
+	 *             if the index can not be read
+	 * @throws IllegalStateException
+	 *             if this is bare (see {@link #isBare()})
 	 */
-	public GitIndex getIndex() throws IOException {
+	public GitIndex getIndex() throws IOException, IllegalStateException {
+		if (isBare())
+			throw new IllegalStateException(
+					JGitText.get().bareRepositoryNoWorkdirAndIndex);
 		if (index == null) {
 			index = new GitIndex(this);
 			index.read();
@@ -1129,8 +1168,13 @@ public class Repository {
 
 	/**
 	 * @return the index file location
+	 * @throws IllegalStateException
+	 *             if this is bare (see {@link #isBare()})
 	 */
-	public File getIndexFile() {
+	public File getIndexFile() throws IllegalStateException {
+		if (isBare())
+			throw new IllegalStateException(
+					JGitText.get().bareRepositoryNoWorkdirAndIndex);
 		return indexFile;
 	}
 
@@ -1269,10 +1313,22 @@ public class Repository {
 		return relName;
 	}
 
+	/**
+	 * @return the "bare"-ness of this Repository
+	 */
+	public boolean isBare() {
+		return workDir == null;
+	}
+
 	/**
 	 * @return the workdir file, i.e. where the files are checked out
+	 * @throws IllegalStateException
+	 *             if the repository is "bare"
 	 */
-	public File getWorkDir() {
+	public File getWorkDir() throws IllegalStateException {
+		if (isBare())
+			throw new IllegalStateException(
+					JGitText.get().bareRepositoryNoWorkdirAndIndex);
 		return workDir;
 	}
 
@@ -1357,7 +1413,8 @@ public class Repository {
 	 */
 	public void scanForRepoChanges() throws IOException {
 		getAllRefs(); // This will look for changes to refs
-		getIndex(); // This will detect changes in the index
+		if (!isBare())
+			getIndex(); // This will detect changes in the index
 	}
 
 	/**