[PATCH] Fix new-workdir (again) to work on bare repositories

[PATCH] Fix new-workdir (again) to work on bare repositories

[Shawn O. Pearce]

My day-job workflow involves using multiple workdirs attached to a
bunch of bare repositories.  Such repositories are stored inside of
a directory called "foo.git", which means `git rev-parse --git-dir`
will return "." and not ".git".  Under such conditions new-workdir
was getting confused about where the Git repository it was supplied
is actually located.

If we get "." for the result of --git-dir query it means we should
use the user supplied path as-is, and not attempt to perform any
magic on it, as the path is directly to the repository.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 contrib/workdir/git-new-workdir |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/contrib/workdir/git-new-workdir b/contrib/workdir/git-new-workdir
index 3ff6bd1..119cff9 100755
--- a/contrib/workdir/git-new-workdir
+++ b/contrib/workdir/git-new-workdir
@@ -24,10 +24,14 @@ git_dir=$(cd "$orig_git" 2>/dev/null &&
   git rev-parse --git-dir 2>/dev/null) ||
   die "\"$orig_git\" is not a git repository!"
 
-if test "$git_dir" = ".git"
-then
+case "$git_dir" in
+.git)
 	git_dir="$orig_git/.git"
-fi
+	;;
+.)
+	git_dir=$orig_git
+	;;
+esac
 
 # don't link to a workdir
 if test -L "$git_dir/config"
-- 
1.5.3.rc6

Re: [PATCH] Fix new-workdir (again) to work on bare repositories

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

> My day-job workflow involves using multiple workdirs attached to a
> bunch of bare repositories.

Are you sure the patch would help?

For one thing, I do not think we supported such a layout,
officially or unofficially --- the thing is in contrib so it
could not be official, but that is besides the point ;-).  Older
git might have worked by accident, though.

You may have made the part to create the new directory and make
bunch of symbolic links to work with your patch, but as far as I
know, new-workdir is designed to share the .git/config file with
the borrowed repository, which means the configuration would say
"core.bare = yes" for a bare repository.  So I suspect that the
initial checkout after creating the new directory and populating
its .git would barf, although I haven't tested it.

Re: [PATCH] Fix new-workdir (again) to work on bare repositories

[Shawn O. Pearce]

Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> > My day-job workflow involves using multiple workdirs attached to a
> > bunch of bare repositories.
> 
> Are you sure the patch would help?

Yes.  I tested it in this use case.  It works with the patch, it
fails rather nicely without.  Fun errors about not being able to
cp HEAD from a directory that shouldn't have a HEAD...

> For one thing, I do not think we supported such a layout,
> officially or unofficially --- the thing is in contrib so it
> could not be official, but that is besides the point ;-).  Older
> git might have worked by accident, though.

True, but it works, and uh, we have this new fangled --work-tree
thing to go along with --git-dir, so why can't I symlink my entire
.git content over to somewhere else and pretend like the mess that
is --work-tree doesn't exist?
 
> You may have made the part to create the new directory and make
> bunch of symbolic links to work with your patch, but as far as I
> know, new-workdir is designed to share the .git/config file with
> the borrowed repository, which means the configuration would say
> "core.bare = yes" for a bare repository.  So I suspect that the
> initial checkout after creating the new directory and populating
> its .git would barf, although I haven't tested it.

Indeed.  I have a driver script that sets up my bare repos, it
removes core.bare from their configs.  So if you go into the bare
repo our auto-sensing bare thing gets activated and says "Hmm, it
ends in .git but isn't exactly .git so its bare!" (correct answer).
If you cd into a workdir created by git-new-workdir the auto-sensing
bare thing gets activated and says "Hmm, it is exactly .git so its
not-bare!" (correct answer).

So removing that core.bare thing makes the magic work.  But if you
leave core.bare in foo.git/config, yes, a workdir created from it
is mighty confused.

-- 
Shawn.

Re: [PATCH] Fix new-workdir (again) to work on bare repositories

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

>> You may have made the part to create the new directory and make
>> bunch of symbolic links to work with your patch, but as far as I
>> know, new-workdir is designed to share the .git/config file with
>> the borrowed repository, which means the configuration would say
>> "core.bare = yes" for a bare repository.  So I suspect that the
>> initial checkout after creating the new directory and populating
>> its .git would barf, although I haven't tested it.
>
> Indeed.  I have a driver script that sets up my bare repos, it
> removes core.bare from their configs.  So if you go into the bare
> repo our auto-sensing bare thing gets activated and says "Hmm, it
> ends in .git but isn't exactly .git so its bare!" (correct answer).
> If you cd into a workdir created by git-new-workdir the auto-sensing
> bare thing gets activated and says "Hmm, it is exactly .git so its
> not-bare!" (correct answer).
>
> So removing that core.bare thing makes the magic work.

Ok, that is better than the response I sent (our messages
crossed).  In that case, perhaps you would want a warning or a
suggestion at the end of new-workdir script, probably before
checkout, to teach the user about that magic?

Re: [PATCH] Fix new-workdir (again) to work on bare repositories

[Shawn O. Pearce]

Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> > So removing that core.bare thing makes the magic work.
> 
> Ok, that is better than the response I sent (our messages
> crossed).  In that case, perhaps you would want a warning or a
> suggestion at the end of new-workdir script, probably before
> checkout, to teach the user about that magic?

-->8--
Suggest unsetting core.bare when using new-workdir on a bare repository

If core.bare is set to true in the config file of a repository that
the user is trying to create a working directory from we should
abort and suggest to the user that they remove the option first.

If we leave the core.bare=true setting in the config file then
working tree operations will get confused when they attempt to
execute in the new workdir, as it shares its config file with the
bare repository.  The working tree operations will assume that the
workdir is bare and abort, which is not what the user wants.

If we changed core.bare to be false then working tree operations
will function in the workdir but other operations may fail in the
bare repository, as it claims to not be bare.

If we remove core.bare from the config then Git can fallback on
the legacy guessing behavior.  This allows operations in the bare
repository to work as though it were bare, while operations in the
workdirs to act as though they are not bare.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 contrib/workdir/git-new-workdir |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/contrib/workdir/git-new-workdir b/contrib/workdir/git-new-workdir
index 119cff9..c6e154a 100755
--- a/contrib/workdir/git-new-workdir
+++ b/contrib/workdir/git-new-workdir
@@ -33,6 +33,14 @@ case "$git_dir" in
 	;;
 esac
 
+# don't link to a configured bare repository
+isbare=$(git --git-dir="$git_dir" config --bool --get core.bare)
+if test ztrue = z$isbare
+then
+	die "\"$git_dir\" has core.bare set to true," \
+		" remove from \"$git_dir/config\" to use $0"
+fi
+
 # don't link to a workdir
 if test -L "$git_dir/config"
 then
-- 
1.5.3.rc6


-- 
Shawn.

Re: [PATCH] Fix new-workdir (again) to work on bare repositories

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
>> My day-job workflow involves using multiple workdirs attached to a
>> bunch of bare repositories.
>
> Are you sure the patch would help?
>
> For one thing, I do not think we supported such a layout,
> officially or unofficially --- the thing is in contrib so it
> could not be official, but that is besides the point ;-).  Older
> git might have worked by accident, though.
>
> You may have made the part to create the new directory and make
> bunch of symbolic links to work with your patch, but as far as I
> know, new-workdir is designed to share the .git/config file with
> the borrowed repository, which means the configuration would say
> "core.bare = yes" for a bare repository.  So I suspect that the
> initial checkout after creating the new directory and populating
> its .git would barf, although I haven't tested it.

Having said that, I am not saying that we should forbid such a
layout that uses a work tree or more attached to a bare
repository.  We need to figure out what the best practice should
be.

 * Maybe using such a layout needs GIT_WORK_TREE environment to
   be set?

 * Maybe using such a layout needs "core.bare = false" even
   though the true repository is a bare repository?  This has an
   obvious drawback that git-fetch cannot update the branch
   pointed by HEAD even though it is a bare repository.

 * Maybe ignore "core.bare = true" if $GIT_DIR ends with
   "/.git"?  I do not like relying on names too much, though.

 * Maybe ignore "core.bare = true" if $GIT_DIR/index exists?
   This is almost right, except that there is a chicken and egg
   problem -- the index does not exist until the initial
   checkout, and checkout wants to make sure you are not in a
   bare repository.