git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Fix new-workdir (again) to work on bare repositories
@ 2007-08-22  1:50 Shawn O. Pearce
  2007-08-22  3:25 ` Junio C Hamano
  0 siblings, 1 reply; 6+ messages in thread
From: Shawn O. Pearce @ 2007-08-22  1:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix new-workdir (again) to work on bare repositories
  2007-08-22  1:50 [PATCH] Fix new-workdir (again) to work on bare repositories Shawn O. Pearce
@ 2007-08-22  3:25 ` Junio C Hamano
  2007-08-22  3:36   ` Shawn O. Pearce
  2007-08-22  3:38   ` Junio C Hamano
  0 siblings, 2 replies; 6+ messages in thread
From: Junio C Hamano @ 2007-08-22  3:25 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git

"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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix new-workdir (again) to work on bare repositories
  2007-08-22  3:25 ` Junio C Hamano
@ 2007-08-22  3:36   ` Shawn O. Pearce
  2007-08-22  4:33     ` Junio C Hamano
  2007-08-22  3:38   ` Junio C Hamano
  1 sibling, 1 reply; 6+ messages in thread
From: Shawn O. Pearce @ 2007-08-22  3:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix new-workdir (again) to work on bare repositories
  2007-08-22  3:25 ` Junio C Hamano
  2007-08-22  3:36   ` Shawn O. Pearce
@ 2007-08-22  3:38   ` Junio C Hamano
  1 sibling, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2007-08-22  3:38 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix new-workdir (again) to work on bare repositories
  2007-08-22  3:36   ` Shawn O. Pearce
@ 2007-08-22  4:33     ` Junio C Hamano
  2007-08-22  5:33       ` Shawn O. Pearce
  0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2007-08-22  4:33 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git

"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?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix new-workdir (again) to work on bare repositories
  2007-08-22  4:33     ` Junio C Hamano
@ 2007-08-22  5:33       ` Shawn O. Pearce
  0 siblings, 0 replies; 6+ messages in thread
From: Shawn O. Pearce @ 2007-08-22  5:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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.

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2007-08-22  5:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-08-22  1:50 [PATCH] Fix new-workdir (again) to work on bare repositories Shawn O. Pearce
2007-08-22  3:25 ` Junio C Hamano
2007-08-22  3:36   ` Shawn O. Pearce
2007-08-22  4:33     ` Junio C Hamano
2007-08-22  5:33       ` Shawn O. Pearce
2007-08-22  3:38   ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).