From 3c51b35e03efc268dda087d9eec1596a35faee3a Mon Sep 17 00:00:00 2001 From: Mathias Kinzler Date: Wed, 16 Jun 2010 08:50:26 +0200 Subject: [PATCH] "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 --- .../jgit/lib/RepositorySetupWorkDirTest.java | 208 ++++++++++++++++++ .../org/eclipse/jgit/JGitText.properties | 1 + .../src/org/eclipse/jgit/JGitText.java | 1 + .../org/eclipse/jgit/lib/ConfigConstants.java | 70 ++++++ .../src/org/eclipse/jgit/lib/Repository.java | 109 ++++++--- 5 files changed, 363 insertions(+), 26 deletions(-) create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositorySetupWorkDirTest.java create mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositorySetupWorkDirTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositorySetupWorkDirTest.java new file mode 100644 index 000000000..6e5e0054b --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositorySetupWorkDirTest.java @@ -0,0 +1,208 @@ +/* + * Copyright (C) 2010, Mathias Kinzler + * + * 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); + } +} diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties index b769671c7..91b67daf8 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties +++ b/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. diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java index 49056c7b8..0c64b9edd 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java +++ b/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; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java new file mode 100644 index 000000000..7a7fc888f --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java @@ -0,0 +1,70 @@ +/* + * Copyright (C) 2010, Mathias Kinzler + * 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"; +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java index 98362af98..a71640460 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java +++ b/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 } /**