git clone regression

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

git clone regression

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:29

It appears that with 1.5.3-rc5

	$ git clone --bare $origin_url target.git

does not set "core.bare = true" in target.git/config.  We used
to, at least with 1.5.2.2.  I am strongly suspecting that this
is another fallout from the worktree series.

[PATCH] Fix initialization of a bare repository

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:29

The recent work-tree cleanup broke it; core.bare was set to false
erroneously when calling "git --bare init".  Coincidentally, this
fixes "git clone --bare", too.

Noticed by Junio Hamano.

Signed-off-by: Johannes Schindelin <redacted>
---

	On Wed, 15 Aug 2007, Junio C Hamano wrote:

	> It appears that with 1.5.3-rc5
	> 
	> 	$ git clone --bare $origin_url target.git
	> 
	> does not set "core.bare = true" in target.git/config.  We used
	> to, at least with 1.5.2.2.  I am strongly suspecting that this
	> is another fallout from the worktree series.

	Sorry.  Yes, this is another fallout.

	Maybe this is not enough, though.  Maybe we need to check if the 
	GIT_DIR=. too, since 7efeb8f0 would set the work tree to ".", too.

	But maybe this would merit a separate fix in set_work_tree(), like

		if (!strcmp(getenv(GIT_DIR_ENVIRONMENT), ".")) {
			inside_work_tree = 0;
			return NULL;
		}

	Hmm?

 builtin-init-db.c |   13 ++++++++-----
 git.c             |    1 +
 2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 75fb227..2e45a7e 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -302,11 +302,14 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 			usage(init_db_usage);
 	}
 
-	git_work_tree_cfg = xcalloc(PATH_MAX, 1);
-	if (!getcwd(git_work_tree_cfg, PATH_MAX))
-		die ("Cannot access current working directory.");
-	if (access(get_git_work_tree(), X_OK))
-		die ("Cannot access work tree '%s'", get_git_work_tree());
+	if (!is_bare_repository_cfg) {
+		git_work_tree_cfg = xcalloc(PATH_MAX, 1);
+		if (!getcwd(git_work_tree_cfg, PATH_MAX))
+			die ("Cannot access current working directory.");
+		if (access(get_git_work_tree(), X_OK))
+			die ("Cannot access work tree '%s'",
+					get_git_work_tree());
+	}
 
 	/*
 	 * Set up the default .git directory contents
diff --git a/git.c b/git.c
index 1bf2744..f0062a0 100644
--- a/git.c
+++ b/git.c
@@ -100,6 +100,7 @@ static int handle_options(const char*** argv, int* argc, int* envchanged)
 		} else if (!strcmp(cmd, "--bare")) {
 			static char git_dir[PATH_MAX+1];
 			setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir, sizeof(git_dir)), 1);
+			is_bare_repository_cfg = 1;
 		} else if (!strcmp(cmd, "-2") ||
 				!strcmp(cmd, "--redirect-stderr")) {
 			if (dup2(1, 2) < 0)
-- 
1.5.1.rc1.4887.ga4a43-dirty

Re: [PATCH] Fix initialization of a bare repository

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:29

Johannes Schindelin [off-list ref] writes:
	Maybe this is not enough, though.  Maybe we need to check if the 
	GIT_DIR=. too, since 7efeb8f0 would set the work tree to ".", too.

	But maybe this would merit a separate fix in set_work_tree(), like

		if (!strcmp(getenv(GIT_DIR_ENVIRONMENT), ".")) {
			inside_work_tree = 0;
			return NULL;
		}

	Hmm?
I do not know.  If you treat "." differently from a pathname
that is the same as what $(pwd) would give you, you will confuse
even more users.

Currently we have:

 - char *git_work_tree_cfg: the location of the work tree when
   $GIT_WORK_TREE nor --work-tree option is in use.  Contrary to
   the name of the variable, it does not necessarily come from
   core.worktree.  This comes from core.worktree if set,
   otherwise derived from the location of the repository
   (i.e. unless $GIT_DIR is set, above "/.git" of the found
   repository, otherwise ".").

 - char *work_tree: the location of the work tree, which comes
   from $GIT_WORK_TREE (or --work-tree option) if set, otherwise
   git_work_tree_cfg above.

 - int inside_work_tree: is $(cwd) inside of the work tree (if
   we have one)?

 - int is_bare_repository_cfg: are we treating this repository
   as bare?  Contrary to the name of the variable, it does not
   necessarily come from core.bare (get_git_work_tree()
   overwrites the value found in core.bare to false in if we
   have a work_tree).

 - int is_bare_repository(): are we treating this repository
   as bare?  This is the function the callers usually use.

I think tying the presense of work_tree and repository bareness
as is_bare_repository() does is almost always right for the
normal callers.  One valid use of GIT_WORK_TREE is to put a work
tree to a repository otherwise declared as a bare one.

But obviously init-db is rather a special case with a chicken
and egg problem.  What it wants to know is not if we are
currently treating the repository as a bare one, but if we would
want to mark its core.bare with "this should be normally treated
as bare".  Your patch treats this special case as such without
affecting the normal codepath, which I think is a sane thing to
do.

Thanks for a quick fix.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help