Re: [PATCH] Commands requiring a work tree must not run in GIT_DIR
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jan 24, 2007, 14:14 UTC
- Message-ID
- <Pine.LNX.4.63.0701241508510.22628@wbgn013.biozentrum.uni-wuerzburg.de>
- In-Reply-To
- <7vodoohcol.fsf@assigned-by-dhcp.cox.net>
Hi,
On Wed, 24 Jan 2007, Junio C Hamano wrote:
Show 8 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > This patch helps when you accidentally run something like git-clean > > in the git directory instead of the work tree. > > I think "require_work_tree" reflects what we are trying to do > much better than NOT_BARE. So maybe we should rename NOT_BARE > to REQUIRE_WORK_TREE.
Hm. Might make sense.
But there is a subtle trap here: if a repo is not bare, it does have a work tree. But what we want here actually is NOT_INSIDE_GIT_DIR:
It is perfectly sensible to run git-pull from inside the git dir, since it has to cd to the top _anyway_. So, git-pull needs a work tree, but does not forbid running from within GIT_DIR.
Show 31 quoted lines
> Existing check function is_bare_repository() is sometimes used
> to see if it is a bare repository regardless of where you are
> (e.g. refs.c::log_ref_write()), so that function can stay as is,
> but the combined check below (you seem to have a few instances
> in your patch) can be made into a function require_work_tree().
>
> > diff --git a/git.c b/git.c
> > index 5133a07..2027d1c 100644
> > --- a/git.c
> > +++ b/git.c
> > @@ -302,8 +302,9 @@ static void handle_internal_command(int argc, const char **argv, char **envp)
> > prefix = setup_git_directory();
> > if (p->option & USE_PAGER)
> > setup_pager();
> > - if ((p->option & NOT_BARE) && is_bare_repository())
> > - die("%s cannot be used in a bare git directory", cmd);
> > + if ((p->option & NOT_BARE) &&
> > + (is_bare_repository() || is_inside_git_dir()))
> > + die("%s must be run in a work tree", cmd);
> > trace_argv_printf(argv, argc, "trace: built-in: git");
> >
> > exit(p->fn(argc, argv, prefix));
>
> Similar to the "conditionally require working tree" you did to
> ls-files, "apply --index" and perhaps "apply --cached" (but this
> is "perhaps" --- you _could_ have an index in a bare repository,
> although it is debatable if there is a valid use case for it),
> grep (grep_cache() but perhaps !cached for the same reason),
> "read-tree -u", "rerere", "update-index" (except --index-info
> and friends that feed object names directly without using
> working tree), should require working tree.As I said, maybe we have to introduce DISALLOW_INSIDE_GIT_DIR (which is ugly, since it is so long, but I cannot think of anything saner), and likewise "disallow_inside_GIT_DIR" for scripts.
> On the script front, bisect should require working tree, but I do not > think anybody is stupid enough to start bisecting in a bare repository > ;-)
OTOH it is really easy:
diff --git a/git-bisect.sh b/git-bisect.sh index 6da31e8..f8a2b64 100755 --- a/git-bisect.sh +++ b/git-bisect.sh @@ -11,6 +11,7 @@ git bisect replay <logfile> replay bisection log git bisect log show bisect log.' . git-sh-setup +require_work_tree sq() { @@PERL@@ -e ' Ciao, Dscho