git ate my home directory :-(

git ate my home directory :-(

[Richard Weinberger]

Hi!

Today I've discovered that on the build server my home directory was empty.
A post-mortem analysis showed that the git-clean command I've added to my kernel build script
is the evil doer.
In my scripts I'm setting GIT_DIR to use git-fetch and git-reset without changing the
current working directory all the time.
But calling git-clean with GIT_DIR acts basically like a "rm -Rf .".

Here a small demo:

test@linux:~> git --version
git version 1.8.1.4
test@linux:~> ls
test@linux:~> touch a b c d e
test@linux:~> mkdir x
test@linux:~> cd x
test@linux:~/x> git init
Initialized empty Git repository in /home/test/x/.git/
test@linux:~/x> cd ..
test@linux:~> ls
a  b  c  d  e  x
test@linux:~> export GIT_DIR=/home/test/x/.git/
test@linux:~> git clean -d -f
Removing a
Removing b
Removing c
Removing d
Removing e
Removing x/
test@linux:~> ls
test@linux:~>
test@linux:~> # :-(

Is this behavior intended?

Thanks,
//richard

Re: git ate my home directory :-(

[Jonathan Nieder]

Hi,

Richard Weinberger wrote:

> In my scripts I'm setting GIT_DIR to use git-fetch and git-reset without changing the
> current working directory all the time.

Yeah, for historical reasons GIT_WORK_TREE defaults to $(pwd) when
GIT_DIR is explicitly set.

In git versions including the patch 2cd83d10bb6b (setup: suppress
implicit "." work-tree for bare repos, 2013-03-08, currently in "next"
but not "master"), you can set GIT_IMPLICIT_WORK_TREE=0 to avoid this
behavior.

Thanks for a useful example, and sorry for the trouble.

Sincerely,
Jonathan

Re: git ate my home directory :-(

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> In git versions including the patch 2cd83d10bb6b (setup: suppress
> implicit "." work-tree for bare repos, 2013-03-08, currently in "next"
> but not "master"), you can set GIT_IMPLICIT_WORK_TREE=0 to avoid this
> behavior.

WAT?

Re: git ate my home directory :-(

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Richard Weinberger wrote:
>
>> In my scripts I'm setting GIT_DIR to use git-fetch and git-reset without changing the
>> current working directory all the time.
>
> Yeah, for historical reasons GIT_WORK_TREE defaults to $(pwd) when
> GIT_DIR is explicitly set.

And it *WILL* be that way til the end of time.  Unless you are at
the top level of your working tree, you are supposed to tell where
the top level is with GIT_WORK_TREE when you use GIT_DIR.  Always.

And that is the answer you should be giving here, not implicit
stuff, which is an implementation detail to help aliases.  I do not
know how things will break when the end user sets and exports it to
the environment, and I do not think we would want to make any
promise on how it works.

Re: git ate my home directory :-(

[Jonathan Nieder]

Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:

>> In git versions including the patch 2cd83d10bb6b (setup: suppress
>> implicit "." work-tree for bare repos, 2013-03-08, currently in "next"
>> but not "master"), you can set GIT_IMPLICIT_WORK_TREE=0 to avoid this
>> behavior.
>
> WAT?

Is that false?

If I understand the history correctly, the ability to set the GIT_DIR
envvar was meant to allow a person to keep their .git directory outside
the worktree.  So you can do:

	git init my-favorite-repo
	cd my-favorite-repo
	...work as usual...

	# cleaning time!
	mv .git ~/my-favorite-repo-metadata.git
	GIT_DIR=$HOME/my-favorite-repo-metadata.git; export GIT_DIR
	... work as usual...

If you want to set GIT_DIR and treat it as a bare repository, the
sane way to do that is simply

	cd ~/my-favorite-bare-repository.git
	... use git as usual ...

But if something (for example relative paths used by your script)
ties your cwd somewhere else, you might really want to do

	GIT_DIR=~/my-favorite-bare-repository.git; export GIT_DIR
	... work as usual ...

and as a side effect of Jeff's patch there is now a mechanism to do
that:

	GIT_IMPLICIT_WORK_TREE=0; export GIT_IMPLICIT_WORK_TREE
	GIT_DIR=~/my-favorite-bare-repository.git; export GIT_DIR
	... work as usual ...

This is of course unsafe because it ties your usage to a specific
version of git.  And the variable is not advertised in the
documentation.

Re: git ate my home directory :-(

[Richard Weinberger]

Am 25.03.2013 23:06, schrieb Junio C Hamano:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Richard Weinberger wrote:
>>
>>> In my scripts I'm setting GIT_DIR to use git-fetch and git-reset without changing the
>>> current working directory all the time.
>>
>> Yeah, for historical reasons GIT_WORK_TREE defaults to $(pwd) when
>> GIT_DIR is explicitly set.
>
> And it *WILL* be that way til the end of time.  Unless you are at
> the top level of your working tree, you are supposed to tell where
> the top level is with GIT_WORK_TREE when you use GIT_DIR.  Always.
>
> And that is the answer you should be giving here, not implicit
> stuff, which is an implementation detail to help aliases.  I do not
> know how things will break when the end user sets and exports it to
> the environment, and I do not think we would want to make any
> promise on how it works.
>

Okay, I have to set GIT_DIR _and_ GIT_WORK_TREE to make my scripts safe again?
I've always set only GIT_DIR because it just worked (till today...).

Thanks,
//richard

Re: git ate my home directory :-(

[Jonathan Nieder]

Junio C Hamano wrote:

>                                                            I do not
> know how things will break when the end user sets and exports it to
> the environment, and I do not think we would want to make any
> promise on how it works.

That's a reasonable desire, and it means it's a good thing we noticed
this before the envvar escaped to "master".  People *will* use such
exposed interfaces unless they are clearly marked as internal.  That's
just a fact of life.

Here's a rough patch to hopefully improve matters.

Longer term, it would be nice to have something like
GIT_IMPLICIT_WORK_TREE exposed to let scripts cache the result of the
search for .git.  Maybe something like "GIT_BARE=(arbitrary value)"
would be a good interface.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---

diff --git a/cache.h b/cache.h
index 59e5b53..8f92b6d 100644
--- a/cache.h
+++ b/cache.h
@@ -377,7 +377,7 @@ static inline enum object_type object_type(unsigned int mode)
  * of this, but we use it internally to communicate to sub-processes that we
  * are in a bare repo. If not set, defaults to true.
  */
-#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT "GIT_IMPLICIT_WORK_TREE"
+#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT "GIT_INTERNAL_IMPLICIT_WORK_TREE"
 
 /*
  * Repository-local GIT_* environment variables; these will be cleared

Re: git ate my home directory :-(

[Jonathan Nieder]

Richard Weinberger wrote:

> Okay, I have to set GIT_DIR _and_ GIT_WORK_TREE to make my scripts safe again?
> I've always set only GIT_DIR because it just worked (till today...).

chdir-ing into the git repo without setting any GIT_* vars is probably
the simplest way to go.

Re: git ate my home directory :-(

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Junio C Hamano wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>>> In git versions including the patch 2cd83d10bb6b (setup: suppress
>>> implicit "." work-tree for bare repos, 2013-03-08, currently in "next"
>>> but not "master"), you can set GIT_IMPLICIT_WORK_TREE=0 to avoid this
>>> behavior.
>>
>> WAT?
>
> Is that false?
>
> If I understand the history correctly, the ability to set the GIT_DIR
> envvar was meant to allow a person to keep their .git directory outside
> the worktree.  So you can do:
>
> 	git init my-favorite-repo
> 	cd my-favorite-repo
> 	...work as usual...
>
> 	# cleaning time!
> 	mv .git ~/my-favorite-repo-metadata.git
> 	GIT_DIR=$HOME/my-favorite-repo-metadata.git; export GIT_DIR
> 	... work as usual...

... as usual except that you have to be at the top.

And that is why GIT_WORK_TREE was invented, so that you can anchor
where the top of the tree is with that variable and then chdir
around into its subdirectories.

Also later we added core.worktree so that $GIT_DIR/config can say
where its associated working tree is.

> This is of course unsafe because it ties your usage to a specific
> version of git.  And the variable is not advertised in the
> documentation.

We decided not to advitise it exactly because we do not intend to
guarantee that will be the way that variable will work.  It is an
implementation detail of that "alias" stuff in the topic it
addresses.

The documented way is to point at the tip with GIT_WORK_TREE.

Re: git ate my home directory :-(

[Junio C Hamano]

Richard Weinberger <richard@nod.at> writes:

> Okay, I have to set GIT_DIR _and_ GIT_WORK_TREE to make my scripts safe again?
> I've always set only GIT_DIR because it just worked (till today...).

That means you never run your script inside a subdirectory ;-)

If your $GIT_DIR is tied to a single working tree, a simpler way
would be to add

	[core]
		worktree = /path/to/the/work/tree/

to $GIT_DIR/config.

Re: git ate my home directory :-(

[Brandon Casey]

On Mon, Mar 25, 2013 at 3:13 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Junio C Hamano wrote:
>
>>                                                            I do not
>> know how things will break when the end user sets and exports it to
>> the environment, and I do not think we would want to make any
>> promise on how it works.
>
> That's a reasonable desire, and it means it's a good thing we noticed
> this before the envvar escaped to "master".  People *will* use such
> exposed interfaces unless they are clearly marked as internal.  That's
> just a fact of life.
>
> Here's a rough patch to hopefully improve matters.
>
> Longer term, it would be nice to have something like
> GIT_IMPLICIT_WORK_TREE exposed to let scripts cache the result of the
> search for .git.  Maybe something like "GIT_BARE=(arbitrary value)"
> would be a good interface.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
>
> diff --git a/cache.h b/cache.h
> index 59e5b53..8f92b6d 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -377,7 +377,7 @@ static inline enum object_type object_type(unsigned int mode)
>   * of this, but we use it internally to communicate to sub-processes that we
>   * are in a bare repo. If not set, defaults to true.
>   */
> -#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT "GIT_IMPLICIT_WORK_TREE"
> +#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT "GIT_INTERNAL_IMPLICIT_WORK_TREE"

Maybe the environment variable for internal-use-only should be
prefixed with an underscore?

-Brandon

Re: git ate my home directory :-(

[Richard Weinberger]

Am 25.03.2013 23:20, schrieb Junio C Hamano:
> Richard Weinberger <richard@nod.at> writes:
>
>> Okay, I have to set GIT_DIR _and_ GIT_WORK_TREE to make my scripts safe again?
>> I've always set only GIT_DIR because it just worked (till today...).
>
> That means you never run your script inside a subdirectory ;-)
>
> If your $GIT_DIR is tied to a single working tree, a simpler way
> would be to add
>
> 	[core]
> 		worktree = /path/to/the/work/tree/

I've used GIT_DIR in my scripts because changing the current working directory
within bash scripts often causes problem with other commands.
That's why I've used patters like:
export GIT_DIR=/path/to/repo/.git
git fetch ...
do_this
do_that
git reset --hard FETCH_HEAD
do_foo
git whatever

export GIT_DIR=/path/to/another/repo/.git
git fetch ...
...

But from now on I'll simply cd into the git repo...

Thanks,
//richard

Re: git ate my home directory :-(

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>
Sent: Monday, March 25, 2013 10:06 PM
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Richard Weinberger wrote:
>>
>>> In my scripts I'm setting GIT_DIR to use git-fetch and git-reset 
>>> without changing the
>>> current working directory all the time.
>>
>> Yeah, for historical reasons GIT_WORK_TREE defaults to $(pwd) when
>> GIT_DIR is explicitly set.
>
> And it *WILL* be that way til the end of time.  Unless you are at
> the top level of your working tree, you are supposed to tell where
> the top level is with GIT_WORK_TREE when you use GIT_DIR.  Always.

Should this important warning be part of the git(1) documentation on the 
environment variables (and possibly other places) given the consequences 
of this case? It wasn't something I'd appreciated from a simple reading.

>
> And that is the answer you should be giving here, not implicit
> stuff, which is an implementation detail to help aliases.  I do not
> know how things will break when the end user sets and exports it to
> the environment, and I do not think we would want to make any
> promise on how it works.
> --
Philip 

Re: git ate my home directory :-(

[Duy Nguyen]

On Tue, Mar 26, 2013 at 08:02:30AM -0000, Philip Oakley wrote:
> >> Yeah, for historical reasons GIT_WORK_TREE defaults to $(pwd) when
> >> GIT_DIR is explicitly set.
> >
> > And it *WILL* be that way til the end of time.  Unless you are at
> > the top level of your working tree, you are supposed to tell where
> > the top level is with GIT_WORK_TREE when you use GIT_DIR.  Always.
> 
> Should this important warning be part of the git(1) documentation on the 
> environment variables (and possibly other places) given the consequences 
> of this case? It wasn't something I'd appreciated from a simple reading.

Something like this, maybe?

-- 8< --
Subject: [PATCH] git.txt: document the implicit working tree setting with GIT_DIR

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 7efaa59..ce55abf 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -671,6 +671,8 @@ Git so take care if using Cogito etc.
 	specifies a path to use instead of the default `.git`
 	for the base of the repository.
 	The '--git-dir' command-line option also sets this value.
+	If neither GIT_WORK_TREE nor '--work-tree' is set, the
+	current directory will become the working tree.
 
 'GIT_WORK_TREE'::
 	Set the path to the working tree.  The value will not be
-- 
1.8.2.82.gc24b958
-- 8< --

Re: git ate my home directory :-(

[Richard Weinberger]

Am 26.03.2013 09:02, schrieb Philip Oakley:
> From: "Junio C Hamano" <gitster@pobox.com>
> Sent: Monday, March 25, 2013 10:06 PM
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>>
>>> Richard Weinberger wrote:
>>>
>>>> In my scripts I'm setting GIT_DIR to use git-fetch and git-reset without changing the
>>>> current working directory all the time.
>>>
>>> Yeah, for historical reasons GIT_WORK_TREE defaults to $(pwd) when
>>> GIT_DIR is explicitly set.
>>
>> And it *WILL* be that way til the end of time.  Unless you are at
>> the top level of your working tree, you are supposed to tell where
>> the top level is with GIT_WORK_TREE when you use GIT_DIR.  Always.
>
> Should this important warning be part of the git(1) documentation on the environment variables (and possibly other places) given the consequences of this case? It wasn't something
> I'd appreciated from a simple reading.

BTW: Can't we change git-clean such that it will not delete any files if GIT_DIR is set and GIT_WORK_TREE is "."?

Thanks,
//richard

Re: git ate my home directory :-(

[Jeff King]

On Tue, Mar 26, 2013 at 02:07:44PM +0100, Richard Weinberger wrote:

> >Should this important warning be part of the git(1) documentation on
> >the environment variables (and possibly other places) given the
> >consequences of this case? It wasn't something
> >I'd appreciated from a simple reading.
> 
> BTW: Can't we change git-clean such that it will not delete any files
> if GIT_DIR is set and GIT_WORK_TREE is "."?s

We could, but that would break the existing behavior for other people
(and I assume you mean "when GIT_WORK_TREE is not set at all", as I
would think GIT_WORK_TREE=. is explicit enough).

I am sympathetic to your data loss, but I wonder how common a problem it
is in practice. Git-clean already does a dry-run by default; you have to
give it `-f`. This is the first such report we've had. This seems more
akin to "oops, I accidentally ran `rm -rf` in the wrong directory". Yes,
it's catastrophic, but at some point you have to accept that deleting
files is what rm (and git-clean) does; you can only put so many safety
hoops in place.

I don't know. It's an uncommon enough case that we could deprecate
"GIT_WORK_TREE is implicitly `.`" entirely, but I think it would need a
deprecation period, and a way to get the same behavior (e.g., allowing
"GIT_WORK_TREE=.").

-Peff

Re: git ate my home directory :-(

[Jeff King]

On Tue, Mar 26, 2013 at 04:48:44PM +0700, Nguyen Thai Ngoc Duy wrote:

> Something like this, maybe?
> 
> -- 8< --
> Subject: [PATCH] git.txt: document the implicit working tree setting with GIT_DIR
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  Documentation/git.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 7efaa59..ce55abf 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -671,6 +671,8 @@ Git so take care if using Cogito etc.
>  	specifies a path to use instead of the default `.git`
>  	for the base of the repository.
>  	The '--git-dir' command-line option also sets this value.
> +	If neither GIT_WORK_TREE nor '--work-tree' is set, the
> +	current directory will become the working tree.

I think this is a good thing to mention, but a few nits:

  1. core.worktree is another way of setting it

  2. This can also be overridden by --bare (at least in "next").

  3. I think having core.bare set will also override this

-Peff

Re: git ate my home directory :-(

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Mar 26, 2013 at 04:48:44PM +0700, Nguyen Thai Ngoc Duy wrote:
>
>> Something like this, maybe?
>> 
>> -- 8< --
>> Subject: [PATCH] git.txt: document the implicit working tree setting with GIT_DIR
>> 
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>>  Documentation/git.txt | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/Documentation/git.txt b/Documentation/git.txt
>> index 7efaa59..ce55abf 100644
>> --- a/Documentation/git.txt
>> +++ b/Documentation/git.txt
>> @@ -671,6 +671,8 @@ Git so take care if using Cogito etc.
>>  	specifies a path to use instead of the default `.git`
>>  	for the base of the repository.
>>  	The '--git-dir' command-line option also sets this value.
>> +	If neither GIT_WORK_TREE nor '--work-tree' is set, the
>> +	current directory will become the working tree.
>
> I think this is a good thing to mention, but a few nits:
>
>   1. core.worktree is another way of setting it
>
>   2. This can also be overridden by --bare (at least in "next").
>
>   3. I think having core.bare set will also override this

Yeah.  And sorry I kept typing alias. I obviously meant "bare"; the
user visible symptom is closely linked to the use of alias, but the
most important aspect of the change in 2cd83d10bb6b (setup: suppress
implicit "." work-tree for bare repos, 2013-03-08) is about being in
a bare repository where by definition working tree does not exist.

Re: git ate my home directory :-(

[Richard Weinberger]

Am 26.03.2013 15:56, schrieb Jeff King:
> On Tue, Mar 26, 2013 at 02:07:44PM +0100, Richard Weinberger wrote:
>
>>> Should this important warning be part of the git(1) documentation on
>>> the environment variables (and possibly other places) given the
>>> consequences of this case? It wasn't something
>>> I'd appreciated from a simple reading.
>>
>> BTW: Can't we change git-clean such that it will not delete any files
>> if GIT_DIR is set and GIT_WORK_TREE is "."?s
>
> We could, but that would break the existing behavior for other people
> (and I assume you mean "when GIT_WORK_TREE is not set at all", as I
> would think GIT_WORK_TREE=. is explicit enough).

Is there a valid use case to call git-clean with GIT_DIR set but GIT_WORK_TREE
not (or to ."")?
It will delete "." ;)

> I am sympathetic to your data loss, but I wonder how common a problem it
> is in practice. Git-clean already does a dry-run by default; you have to
> give it `-f`. This is the first such report we've had. This seems more
> akin to "oops, I accidentally ran `rm -rf` in the wrong directory". Yes,
> it's catastrophic, but at some point you have to accept that deleting
> files is what rm (and git-clean) does; you can only put so many safety
> hoops in place.

The data loss was not too bad. I was able to restore anything within 2 hours.
But was kinda shocked that git-clean deletes files outside my git tree.
I'm aware of -d. But in my case it happened within a fully automated script.
I simply thought GIT_DIR=.. git-clean -f -d does the right thing...

> I don't know. It's an uncommon enough case that we could deprecate
> "GIT_WORK_TREE is implicitly `.`" entirely, but I think it would need a
> deprecation period, and a way to get the same behavior (e.g., allowing
> "GIT_WORK_TREE=.").

Yeah, this sounds sane.

Thanks,
//richard

P.s: I've told this story to some friends and co-workers which use git like me very day.
All of them were shocked about the behavior of git-clean and GIT_DIR.

Re: git ate my home directory :-(

[demerphq]

On 26 March 2013 18:06, Richard Weinberger <richard@nod.at> wrote:
> P.s: I've told this story to some friends and co-workers which use git like
> me very day.
> All of them were shocked about the behavior of git-clean and GIT_DIR.

Seconded. At $work lots of people started asking anxious questions
about this. It was suggested it is a potential security hole, although
I am not sure I agree, but the general idea being that if you could
manage to set this var in someones environment then they might use git
to do real damage to a system. (The counterargument being that if you
can set that in someones environment you can do worse already... But
im a not a security type so I cant say)

As a knee-jerk response we will be armoring various scripts we have
that use git automatically to refuse to run if GIT_DIR is set. I
suspect a lot of people will be doing the same.

cheers,
Yves



-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

Re: git ate my home directory :-(

[Jeff King]

On Tue, Mar 26, 2013 at 06:06:17PM +0100, Richard Weinberger wrote:

> >We could, but that would break the existing behavior for other people
> >(and I assume you mean "when GIT_WORK_TREE is not set at all", as I
> >would think GIT_WORK_TREE=. is explicit enough).
> 
> Is there a valid use case to call git-clean with GIT_DIR set but GIT_WORK_TREE
> not (or to ."")?
> It will delete "." ;)

Yes, setting GIT_DIR but not GIT_WORK_TREE has always been a valid way
to work on a repository where you do not want the working tree polluted
with your .git file. It's not a common setup, but people do use it.
E.g., you might keep ~/mail as a git repo, but do not want the presence
of ~/mail/.git to confuse your mail tools. You can keep ~/git/mail.git
as just a repository, and do "cd ~/mail && GIT_DIR=~/git/mail.git git
foo" (or "git --git-dir=~git/mail.git foo").

Later, we introduced GIT_WORK_TREE (and core.worktree), which provided
another way of doing the same thing (instead of the "cd", you could set
GIT_WORK_TREE). For the most part, I'd expect setting core.worktree to
be the simplest for such setups, as once it is set, you can just do "cd
~/git/mail.git git foo", and everything should just work.

We never deprecated the original "GIT_DIR without GIT_WORK_TREE means
'.' is the implicit worktree" behavior, as nobody ever complained, and
it would break existing users of the feature.

We could do so now, as long as we provide an escape hatch (and I think
spelling that hatch as GIT_WORK_TREE=. is probably sane, but I am open
to other suggestions). And in general we try to avoid such breakage
without a deprecation period to give people time to fix their scripts
and workflows.

> But was kinda shocked that git-clean deletes files outside my git tree.
> I'm aware of -d. But in my case it happened within a fully automated script.
> I simply thought GIT_DIR=.. git-clean -f -d does the right thing...

It did do the right thing; just not the one you expected. :)

The problem is not with "clean", which just happens to be a destructive
command, but rather with the notion of what the git tree is when you
provide GIT_DIR. Though clean is an obvious problematic command,
something like "git reset --hard" could also be destructive. I don't
think it makes sense to do any fix that is specific to clean. If there
is a fix to be done, it should be about making the working tree lookup
algorithm more obvious.

-Peff

Re: git ate my home directory :-(

[Jeff King]

On Tue, Mar 26, 2013 at 06:20:09PM +0100, demerphq wrote:

> Seconded. At $work lots of people started asking anxious questions
> about this. It was suggested it is a potential security hole, although
> I am not sure I agree, but the general idea being that if you could
> manage to set this var in someones environment then they might use git
> to do real damage to a system. (The counterargument being that if you
> can set that in someones environment you can do worse already... But
> im a not a security type so I cant say)

IMHO, that is just silly. Setting GIT_WORK_TREE=/ would be just as
destructive. Or GIT_EXTERNAL_DIFF="rm -rf /" (or GIT_PAGER, etc).
If there is a danger to the implicit-workdir behavior, it is due to
accidental usage, not from a malicious attack.

-Peff

Re: git ate my home directory :-(

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Yes, setting GIT_DIR but not GIT_WORK_TREE has always been a valid way
> to work on a repository where you do not want the working tree polluted
> with your .git file. It's not a common setup, but people do use it.
> E.g., you might keep ~/mail as a git repo, but do not want the presence
> of ~/mail/.git to confuse your mail tools. You can keep ~/git/mail.git
> as just a repository, and do "cd ~/mail && GIT_DIR=~/git/mail.git git
> foo" (or "git --git-dir=~git/mail.git foo").
>
> Later, we introduced GIT_WORK_TREE (and core.worktree), which provided
> another way of doing the same thing (instead of the "cd", you could set
> GIT_WORK_TREE).

A small correction is necessary as the above invites confusion.

When you are in ~/mail/subdir, because GIT_DIR alone does not give
you to specify where the root-level of the working tree is, you had
to "cd .." before running "GIT_DIR=~/git/mail.git git ...".  By
setting GIT_WORK_TREE to point at ~/mail once, you can freely chdir
around inside subdirectories of ~/mail without losing sight of where
the root-level is, and if your ~/git/mail.git is tied to a single
working tree (and that is true in this example, it is always ~/mail),
you can even set core.worktree in ~/git/mail.git/config.

These two mechanisms are *not* about allowing you to run git from
any random place, e.g. "/tmp".

> For the most part, I'd expect setting core.worktree to be the
> simplest for such setups, as once it is set, you can just do "cd
> ~/git/mail.git git foo", and everything should just work.

Yes.

> We could do so now, as long as we provide an escape hatch (and I think
> spelling that hatch as GIT_WORK_TREE=. is probably sane, but I am open
> to other suggestions).

If we were to do so, GIT_WORK_TREE=. would be the most sensible, but
I do not think it is worth breaking.  Why do these people set GIT_DIR
without setting GIT_WORK_TREE in the first place?

That is the source of the confusion.  Perhaps some random but
popular websites are spreading bad pieces of advice?

> The problem is not with "clean", which just happens to be a destructive
> command, but rather with the notion of what the git tree is when you
> provide GIT_DIR.

Yes, "git add ." would happily add random cruft to your index, which
is equally bad.

Re: git ate my home directory :-(

[demerphq]

On 26 March 2013 18:48, Jeff King <peff@peff.net> wrote:
> On Tue, Mar 26, 2013 at 06:20:09PM +0100, demerphq wrote:
>
>> Seconded. At $work lots of people started asking anxious questions
>> about this. It was suggested it is a potential security hole, although
>> I am not sure I agree, but the general idea being that if you could
>> manage to set this var in someones environment then they might use git
>> to do real damage to a system. (The counterargument being that if you
>> can set that in someones environment you can do worse already... But
>> im a not a security type so I cant say)
>
> IMHO, that is just silly. Setting GIT_WORK_TREE=/ would be just as
> destructive. Or GIT_EXTERNAL_DIFF="rm -rf /" (or GIT_PAGER, etc).
> If there is a danger to the implicit-workdir behavior, it is due to
> accidental usage, not from a malicious attack.

Yeah, that was my line of reasoning too. I'm glad to hear you agree.

cheers
Yves

-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

Re: git ate my home directory :-(

[Jeff King]

On Tue, Mar 26, 2013 at 11:20:58AM -0700, Junio C Hamano wrote:

> When you are in ~/mail/subdir, because GIT_DIR alone does not give
> you to specify where the root-level of the working tree is, you had
> to "cd .." before running "GIT_DIR=~/git/mail.git git ...".  By
> setting GIT_WORK_TREE to point at ~/mail once, you can freely chdir
> around inside subdirectories of ~/mail without losing sight of where
> the root-level is, and if your ~/git/mail.git is tied to a single
> working tree (and that is true in this example, it is always ~/mail),
> you can even set core.worktree in ~/git/mail.git/config.

Yeah, I did not talk about moving around to multiple working trees with
the same GIT_DIR. I have done that, but I do not know of a workflow
where it is a good practice, and not just a one-off hack.

> > We could do so now, as long as we provide an escape hatch (and I think
> > spelling that hatch as GIT_WORK_TREE=. is probably sane, but I am open
> > to other suggestions).
> 
> If we were to do so, GIT_WORK_TREE=. would be the most sensible, but
> I do not think it is worth breaking.  Why do these people set GIT_DIR
> without setting GIT_WORK_TREE in the first place?

I don't think there is a good reason. The argument, as I see it, is
mainly that doing so can be confusing and destructive, and there is not
a big benefit to allowing it.

I am not sure I am convinced it is worth the breakage, either. Curious
as to what the code would look like, I made a straw-man series, which
will follow. Note that I am not suggesting we do this, but still merely
thinking about the idea.

Notably, at the end of the series a number of tests fail. A few of them
are testing the GIT_DIR behavior explicitly (I fixed up t1510, but did
not hunt down all of the spots), but a few of them are legitimate
breakages in scripts. For example, difftool is broken because it sets
GIT_DIR. That gives us an indication of what kinds of breakages we would
see in real-world third-party scripts.

> > The problem is not with "clean", which just happens to be a destructive
> > command, but rather with the notion of what the git tree is when you
> > provide GIT_DIR.
> 
> Yes, "git add ." would happily add random cruft to your index, which
> is equally bad.

Eh, I would say it is bad, but not equally bad to removing your entire
home directory. ;)

-Peff

[DONOTAPPLY PATCH 1/3] environment: set GIT_WORK_TREE when we figure out work tree

[Jeff King]

If we end up in a sub-process where GIT_WORK_TREE is not set
but GIT_DIR is, we assume the current directory is the root
of the working tree. Since future patches will change that
assumption, let's defensively start setting GIT_WORK_TREE
explicitly.

Signed-off-by: Jeff King <peff@peff.net>
---
I didn't test this very well (in fact, I only noticed the issue after
having written the other two patches, as without it, those ones break
every external command which wants to run in a working tree). I would
not be surprised if there is some code path which sets GIT_DIR but not
this, or if there is some other weird fallout from having GIT_WORK_TREE
set explicitly.

 environment.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/environment.c b/environment.c
index 92c5dff..be2e509 100644
--- a/environment.c
+++ b/environment.c
@@ -194,6 +194,7 @@ void set_git_work_tree(const char *new_work_tree)
 	}
 	git_work_tree_initialized = 1;
 	work_tree = xstrdup(real_path(new_work_tree));
+	setenv(GIT_WORK_TREE_ENVIRONMENT, work_tree, 1);
 }
 
 const char *get_git_work_tree(void)
-- 
1.8.2.13.g0f18d3c

[DONOTAPPLY PATCH 2/3] setup: warn about implicit worktree with $GIT_DIR

[Jeff King]

It can be surprising to some users that pointing GIT_DIR to
a ".git" directory does not use the working tree that
surrounds the .git directory, but rather uses the current
working directory as the working tree.

Git has always worked this way, and for the most part it has
not been a big problem.  However, given that one way of the
user finding this out is by having a destructive git command
impact an unexpected area of the filesystem, it would be
nice to default to something less surprising and likely to
cause problems (namely, having no working directory).

This breaks existing users of the feature, of course; they
can adapt by setting GIT_WORK_TREE explicitly to ".", but
they need to be told to do so. Therefore we'll start with a
deprecation period and a warning to give them time to fix
their scripts and workflows.

Signed-off-by: Jeff King <peff@peff.net>
---
 setup.c               | 21 ++++++++++++++++++++-
 t/t1510-repo-setup.sh |  8 ++++++--
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/setup.c b/setup.c
index 01c5476..afc245f 100644
--- a/setup.c
+++ b/setup.c
@@ -437,6 +437,23 @@ const char *read_gitfile(const char *path)
 	return path;
 }
 
+static const char warn_implicit_work_tree_msg[] =
+N_("You have set GIT_DIR (or used --git-dir) without specifying\n"
+   "a working tree. In Git 2.0, the behavior will change from using current\n"
+   "working directory as the working tree to having no working tree at all.\n"
+   "If you wish to continue the current behavior, please set GIT_WORK_TREE\n"
+   "or core.worktree explicitly. See `git help git` for more details.");
+
+static void warn_implicit_work_tree(void)
+{
+	static int warn_once;
+
+	if (warn_once++)
+		return;
+
+	warning("%s", _(warn_implicit_work_tree_msg));
+}
+
 static const char *setup_explicit_git_dir(const char *gitdirenv,
 					  char *cwd, int len,
 					  int *nongit_ok)
@@ -503,8 +520,10 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,
 		free(gitfile);
 		return NULL;
 	}
-	else /* #2, #10 */
+	else { /* #2, #10 */
+		warn_implicit_work_tree();
 		set_git_work_tree(".");
+	}
 
 	/* set_git_work_tree() must have been called by now */
 	worktree = get_git_work_tree();
diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh
index cf2ee78..0910de1 100755
--- a/t/t1510-repo-setup.sh
+++ b/t/t1510-repo-setup.sh
@@ -243,7 +243,9 @@ test_expect_success '#2: worktree defaults to cwd with explicit GIT_DIR' '
 test_expect_success '#2: worktree defaults to cwd with explicit GIT_DIR' '
 	try_repo 2 unset "$here/2/.git" unset "" unset \
 		"$here/2/.git" "$here/2" "$here/2" "(null)" \
-		"$here/2/.git" "$here/2/sub" "$here/2/sub" "(null)"
+		"$here/2/.git" "$here/2/sub" "$here/2/sub" "(null)" \
+		2>message &&
+	test_i18ngrep "warning:.*GIT_DIR" message
 '
 
 test_expect_success '#2b: relative GIT_DIR' '
@@ -378,7 +380,9 @@ test_expect_success '#10: GIT_DIR can point to gitfile' '
 test_expect_success '#10: GIT_DIR can point to gitfile' '
 	try_repo 10 unset "$here/10/.git" unset gitfile unset \
 		"$here/10.git" "$here/10" "$here/10" "(null)" \
-		"$here/10.git" "$here/10/sub" "$here/10/sub" "(null)"
+		"$here/10.git" "$here/10/sub" "$here/10/sub" "(null)" \
+		2>message &&
+	test_i18ngrep "warning:.*GIT_DIR" message
 '
 
 test_expect_success '#10b: relative GIT_DIR can point to gitfile' '
-- 
1.8.2.13.g0f18d3c

[DONOTAPPLY PATCH 3/3] setup: treat GIT_DIR without GIT_WORK_TREE as a bare repo

[Jeff King]

Follow-through on the deprecation warning added by the last
commit.

We can drop all of the IMPLICIT_WORK_TREE code now, since
we default to that case.

Signed-off-by: Jeff King <peff@peff.net>
---
This would obviously come much later than patch 2, in Git 2.0 or
whatever.

But in case anyone did not read the discussion leading up to this
series, this breaks many tests. It's not meant for application, but
merely to look at what kinds of breakage we could see if we followed
this path.

 cache.h               | 12 ------------
 environment.c         |  1 -
 git.c                 |  1 -
 setup.c               | 26 +-------------------------
 t/t1510-repo-setup.sh | 24 ++++++++++--------------
 5 files changed, 11 insertions(+), 53 deletions(-)

diff --git a/cache.h b/cache.h
index c1fe67f..3c6b677 100644
--- a/cache.h
+++ b/cache.h
@@ -366,18 +366,6 @@ static inline enum object_type object_type(unsigned int mode)
 #define GIT_NOTES_REWRITE_MODE_ENVIRONMENT "GIT_NOTES_REWRITE_MODE"
 
 /*
- * This environment variable is expected to contain a boolean indicating
- * whether we should or should not treat:
- *
- *   GIT_DIR=foo.git git ...
- *
- * as if GIT_WORK_TREE=. was given. It's not expected that users will make use
- * of this, but we use it internally to communicate to sub-processes that we
- * are in a bare repo. If not set, defaults to true.
- */
-#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT "GIT_IMPLICIT_WORK_TREE"
-
-/*
  * Repository-local GIT_* environment variables; these will be cleared
  * when git spawns a sub-process that runs inside another repository.
  * The array is NULL-terminated, which makes it easy to pass in the "env"
diff --git a/environment.c b/environment.c
index be2e509..255e277 100644
--- a/environment.c
+++ b/environment.c
@@ -85,7 +85,6 @@ const char * const local_repo_env[] = {
 	DB_ENVIRONMENT,
 	GIT_DIR_ENVIRONMENT,
 	GIT_WORK_TREE_ENVIRONMENT,
-	GIT_IMPLICIT_WORK_TREE_ENVIRONMENT,
 	GRAFT_ENVIRONMENT,
 	INDEX_ENVIRONMENT,
 	NO_REPLACE_OBJECTS_ENVIRONMENT,
diff --git a/git.c b/git.c
index 0ffea57..d33f9b3 100644
--- a/git.c
+++ b/git.c
@@ -125,7 +125,6 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			static char git_dir[PATH_MAX+1];
 			is_bare_repository_cfg = 1;
 			setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir, sizeof(git_dir)), 0);
-			setenv(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, "0", 1);
 			if (envchanged)
 				*envchanged = 1;
 		} else if (!strcmp(cmd, "-c")) {
diff --git a/setup.c b/setup.c
index afc245f..319dbb5 100644
--- a/setup.c
+++ b/setup.c
@@ -437,23 +437,6 @@ const char *read_gitfile(const char *path)
 	return path;
 }
 
-static const char warn_implicit_work_tree_msg[] =
-N_("You have set GIT_DIR (or used --git-dir) without specifying\n"
-   "a working tree. In Git 2.0, the behavior will change from using current\n"
-   "working directory as the working tree to having no working tree at all.\n"
-   "If you wish to continue the current behavior, please set GIT_WORK_TREE\n"
-   "or core.worktree explicitly. See `git help git` for more details.");
-
-static void warn_implicit_work_tree(void)
-{
-	static int warn_once;
-
-	if (warn_once++)
-		return;
-
-	warning("%s", _(warn_implicit_work_tree_msg));
-}
-
 static const char *setup_explicit_git_dir(const char *gitdirenv,
 					  char *cwd, int len,
 					  int *nongit_ok)
@@ -514,16 +497,11 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,
 			set_git_work_tree(core_worktree);
 		}
 	}
-	else if (!git_env_bool(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, 1)) {
-		/* #16d */
+	else { /* #2, #10, #16d */
 		set_git_dir(gitdirenv);
 		free(gitfile);
 		return NULL;
 	}
-	else { /* #2, #10 */
-		warn_implicit_work_tree();
-		set_git_work_tree(".");
-	}
 
 	/* set_git_work_tree() must have been called by now */
 	worktree = get_git_work_tree();
@@ -600,8 +578,6 @@ static const char *setup_bare_git_dir(char *cwd, int offset, int len, int *nongi
 	if (check_repository_format_gently(".", nongit_ok))
 		return NULL;
 
-	setenv(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, "0", 1);
-
 	/* --work-tree is set without --git-dir; use discovered one */
 	if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {
 		const char *gitdir;
diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh
index 0910de1..7db4b3f 100755
--- a/t/t1510-repo-setup.sh
+++ b/t/t1510-repo-setup.sh
@@ -28,7 +28,7 @@ A few rules for repo setup:
 7. Effective core.worktree conflicts with core.bare
 
 8. If GIT_DIR is set but neither worktree nor bare setting is given,
-   original cwd becomes worktree.
+   we treat the repository as bare.
 
 9. If .git discovery is done inside a repo, the repo becomes a bare
    repo. .git discovery is performed if GIT_DIR is not set.
@@ -240,18 +240,16 @@ test_expect_success '#2b: relative GIT_DIR' '
 	! test -s message
 '
 
-test_expect_success '#2: worktree defaults to cwd with explicit GIT_DIR' '
+test_expect_success '#2: worktree defaults to bare with explicit GIT_DIR' '
 	try_repo 2 unset "$here/2/.git" unset "" unset \
-		"$here/2/.git" "$here/2" "$here/2" "(null)" \
-		"$here/2/.git" "$here/2/sub" "$here/2/sub" "(null)" \
-		2>message &&
-	test_i18ngrep "warning:.*GIT_DIR" message
+		"$here/2/.git" "(null)" "$here/2" "(null)" \
+		"$here/2/.git" "(null)" "$here/2/sub" "(null)"
 '
 
 test_expect_success '#2b: relative GIT_DIR' '
 	try_repo 2b unset ".git" unset "" unset \
-		".git" "$here/2b" "$here/2b" "(null)" \
-		"../.git" "$here/2b/sub" "$here/2b/sub" "(null)"
+		".git" "(null)" "$here/2b" "(null)" \
+		"../.git" "(null)" "$here/2b/sub" "(null)"
 '
 
 test_expect_success '#3: setup' '
@@ -379,16 +377,14 @@ test_expect_success '#10b: relative GIT_DIR can point to gitfile' '
 
 test_expect_success '#10: GIT_DIR can point to gitfile' '
 	try_repo 10 unset "$here/10/.git" unset gitfile unset \
-		"$here/10.git" "$here/10" "$here/10" "(null)" \
-		"$here/10.git" "$here/10/sub" "$here/10/sub" "(null)" \
-		2>message &&
-	test_i18ngrep "warning:.*GIT_DIR" message
+		"$here/10.git" "(null)" "$here/10" "(null)" \
+		"$here/10.git" "(null)" "$here/10/sub" "(null)"
 '
 
 test_expect_success '#10b: relative GIT_DIR can point to gitfile' '
 	try_repo 10b unset .git unset gitfile unset \
-		"$here/10b.git" "$here/10b" "$here/10b" "(null)" \
-		"$here/10b.git" "$here/10b/sub" "$here/10b/sub" "(null)"
+		"$here/10b.git" "(null)" "$here/10b" "(null)" \
+		"$here/10b.git" "(null)" "$here/10b/sub" "(null)"
 '
 
 # case #11: GIT_WORK_TREE works, gitfile case.
-- 
1.8.2.13.g0f18d3c

Re: [DONOTAPPLY PATCH 1/3] environment: set GIT_WORK_TREE when we figure out work tree

[Jonathan Nieder]

Jeff King wrote:

> --- a/environment.c
> +++ b/environment.c
> @@ -194,6 +194,7 @@ void set_git_work_tree(const char *new_work_tree)
>  	}
>  	git_work_tree_initialized = 1;
>  	work_tree = xstrdup(real_path(new_work_tree));
> +	setenv(GIT_WORK_TREE_ENVIRONMENT, work_tree, 1);
>  }

There's no rush, but I think this is a good change.  It makes the rest
of the codebase more resilient to running commands from a subdir of
the top level of the worktree.

Re: [DONOTAPPLY PATCH 2/3] setup: warn about implicit worktree with $GIT_DIR

[Jonathan Nieder]

Jeff King wrote:

> --- a/setup.c
> +++ b/setup.c
> @@ -437,6 +437,23 @@ const char *read_gitfile(const char *path)
>  	return path;
>  }
>  
> +static const char warn_implicit_work_tree_msg[] =
> +N_("You have set GIT_DIR (or used --git-dir) without specifying\n"
> +   "a working tree. In Git 2.0, the behavior will change

Please no.  I don't want git 2.0 to be delayed forever.

If we want this warning, would something like the following do?

	warning: You have set GIT_DIR without setting GIT_WORK_TREE
	hint: In this case, GIT_WORK_TREE defaults to '.'
	hint: To suppress this message, set GIT_WORK_TREE='.'

Thanks,
Jonathan

Re: [DONOTAPPLY PATCH 2/3] setup: warn about implicit worktree with $GIT_DIR

[Jeff King]

On Tue, Mar 26, 2013 at 01:21:42PM -0700, Jonathan Nieder wrote:

> Jeff King wrote:
> 
> > --- a/setup.c
> > +++ b/setup.c
> > @@ -437,6 +437,23 @@ const char *read_gitfile(const char *path)
> >  	return path;
> >  }
> >  
> > +static const char warn_implicit_work_tree_msg[] =
> > +N_("You have set GIT_DIR (or used --git-dir) without specifying\n"
> > +   "a working tree. In Git 2.0, the behavior will change
> 
> Please no.  I don't want git 2.0 to be delayed forever.

Please replace "2.0" with some future version, then. I just made up the
number. But...

> If we want this warning, would something like the following do?
> 
> 	warning: You have set GIT_DIR without setting GIT_WORK_TREE
> 	hint: In this case, GIT_WORK_TREE defaults to '.'
> 	hint: To suppress this message, set GIT_WORK_TREE='.'

That can help by teaching people how GIT_DIR behaves in general. But the
warning and hint will be small consolation to somebody who runs
"GIT_DIR=foo.git git clean -f" and sees it for the first time.

If you want to argue that people would see the warning in earlier runs
of git, I can kind of buy that. Although the incident that triggered
this discussion probably wouldn't have (I would usually start a
git-clean session with "git clean" without "-f" or "git status", either
of which would have done equally well as this warning to notify the user
what was going on).

Like I said earlier, though, I'm not really sure this is the direction
we want to go. This series is more about seeing what the fallouts are. I
probably shouldn't have included this middle patch at all, because the
interesting thing is what happens when we do turn it off.

-Peff

Re: [DONOTAPPLY PATCH 2/3] setup: warn about implicit worktree with $GIT_DIR

[Jonathan Nieder]

Jeff King wrote:
> On Tue, Mar 26, 2013 at 01:21:42PM -0700, Jonathan Nieder wrote:

>> If we want this warning, would something like the following do?
>> 
>> 	warning: You have set GIT_DIR without setting GIT_WORK_TREE
>> 	hint: In this case, GIT_WORK_TREE defaults to '.'
>> 	hint: To suppress this message, set GIT_WORK_TREE='.'
>
> That can help by teaching people how GIT_DIR behaves in general.

Yes, I think it would have helped in this case.  If I understand
correctly then for a while Richard was habitually setting GIT_DIR to
mean "act on this repository" and thought the worktree was
automatically being set to the containing directory.

I think patch 3 is a bad direction to go because there will always be
old scripts that follow what used to be the recommended way to use
GIT_DIR.  In the long term a warning like this that doesn't break them
(or a fatal error that at least doesn't confuse them) might be a good
way to go.

Thanks for your thoughtful work, as always.

Jonathan

Re: git ate my home directory :-(

[Philip Oakley]

From: "Duy Nguyen" <pclouds@gmail.com>
Sent: Tuesday, March 26, 2013 9:48 AM
> On Tue, Mar 26, 2013 at 08:02:30AM -0000, Philip Oakley wrote:
>> >> Yeah, for historical reasons GIT_WORK_TREE defaults to $(pwd) when
>> >> GIT_DIR is explicitly set.
>> >
>> > And it *WILL* be that way til the end of time.  Unless you are at
>> > the top level of your working tree, you are supposed to tell where
>> > the top level is with GIT_WORK_TREE when you use GIT_DIR.  Always.
>>
>> Should this important warning be part of the git(1) documentation on
>> the
>> environment variables (and possibly other places) given the
>> consequences
>> of this case? It wasn't something I'd appreciated from a simple
>> reading.
>
> Something like this, maybe?
>
> -- 8< --
> Subject: [PATCH] git.txt: document the implicit working tree setting
> with GIT_DIR
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> Documentation/git.txt | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 7efaa59..ce55abf 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -671,6 +671,8 @@ Git so take care if using Cogito etc.
>  specifies a path to use instead of the default `.git`
>  for the base of the repository.
>  The '--git-dir' command-line option also sets this value.
> + If neither GIT_WORK_TREE nor '--work-tree' is set, the
> + current directory will become the working tree.

I didn't feel this conveyed the Dire Warning effect that would be needed
to avoid the original misunderstanding.

It is easy to miss some of the potential consequences when other
priorities are pressing.

As Junio wondered, perhaps rhetorically, in a later message  "Why do
these people set GIT_DIR without setting GIT_WORK_TREE in the first
place?"

Perhaps
"If the GIT_DIR environment variable is set then it specifies a path to
use instead of the default `.git` for the base of the repository. Note
that the current directory `.` will be used as the working
GIT_WORK_TREE, if not set elsewhere. The --git-dir command-line
option also sets the GIT_DIR environment variable."


>
> 'GIT_WORK_TREE'::
>  Set the path to the working tree.  The value will not be
> -- 
> 1.8.2.82.gc24b958
> -- 8< --
> --
Philip 

Re: [DONOTAPPLY PATCH 2/3] setup: warn about implicit worktree with $GIT_DIR

[Matthieu Moy]

Jeff King <peff@peff.net> writes:

> I probably shouldn't have included this middle patch at all, because
> the interesting thing is what happens when we do turn it off.

Actually, I think the warning is the most important part. With the
warning enabled, people should notice they are doing something
potentially wrong and dangerous, so the warning essentially fixes the
issue in the short term.

I also think that changing the default behavior later makes sense (but I
agree that replacing "in Git 2.0" with "in a future version of Git" is
better, there's no urgency for this change and people start looking
forward 2.0).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: git ate my home directory :-(

[Duy Nguyen]

On Tue, Mar 26, 2013 at 10:04 PM, Jeff King <peff@peff.net> wrote:
>>       specifies a path to use instead of the default `.git`
>>       for the base of the repository.
>>       The '--git-dir' command-line option also sets this value.
>> +     If neither GIT_WORK_TREE nor '--work-tree' is set, the
>> +     current directory will become the working tree.
>
> I think this is a good thing to mention, but a few nits:
>
>   1. core.worktree is another way of setting it
>
>   2. This can also be overridden by --bare (at least in "next").
>
>   3. I think having core.bare set will also override this

Yeah, I looked back at t1510 and gave up. I think it's still true:

-- 8< --
A few rules for repo setup:

1. GIT_DIR is relative to user's cwd. --git-dir is equivalent to
   GIT_DIR.

2. .git file is relative to parent directory. .git file is basically
   symlink in disguise. The directory where .git file points to will
   become new git_dir.

3. core.worktree is relative to git_dir.

4. GIT_WORK_TREE is relative to user's cwd. --work-tree is
   equivalent to GIT_WORK_TREE.

5. GIT_WORK_TREE/core.worktree was originally meant to work only if
   GIT_DIR is set, but earlier git didn't enforce it, and some scripts
   depend on the implementation that happened to first discover .git by
   going up from the users $cwd and then using the specified working tree
   that may or may not have any relation to where .git was found in.  This
   historical behaviour must be kept.

6. Effective GIT_WORK_TREE overrides core.worktree and core.bare

7. Effective core.worktree conflicts with core.bare

8. If GIT_DIR is set but neither worktree nor bare setting is given,
   original cwd becomes worktree.
-- 8< --
-- 
Duy