[PATCH] HOME must be set before calling git-init when creating test repositories

[PATCH] HOME must be set before calling git-init when creating test repositories

[Alex Riesen]

Otherwise the created test repositories will be affected by users ~/.gitconfig.
For example, setting core.logAllrefupdates in users config will make all
calls to "git config --unset core.logAllrefupdates" fail which will break
the first test which uses the statement and expects it to succeed.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---

The first test which fails is t2017, btw.
I still wonder if this should be moved even further up.

 t/test-lib.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 7cc9a52..4f394c3 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -984,14 +984,14 @@ rm -fr "$test" || {
 	exit 1
 }
 
+HOME=$(pwd)
+export HOME
+
 test_create_repo "$test"
 # Use -P to resolve symlinks in our working directory so that the cwd
 # in subprocesses like git equals our $PWD (for pathname comparisons).
 cd -P "$test" || exit 1
 
-HOME=$(pwd)
-export HOME
-
 this_test=${0##*/}
 this_test=${this_test%%-*}
 for skp in $GIT_SKIP_TESTS
-- 
1.7.4.1.271.g4540f

[PATCH, fixed] HOME must be set before calling git-init when creating test repositories

[Alex Riesen]

Otherwise the created test repositories will be affected by users ~/.gitconfig.
For example, setting core.logAllrefupdates in users config will make all
calls to "git config --unset core.logAllrefupdates" fail which will break
the first test which uses the statement and expects it to succeed.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---

Aah... Sorry. Missed the test's part of the new HOME.

 t/test-lib.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 7cc9a52..8792e4a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -984,14 +984,14 @@ rm -fr "$test" || {
 	exit 1
 }
 
+HOME=$(pwd)/"$test"
+export HOME
+
 test_create_repo "$test"
 # Use -P to resolve symlinks in our working directory so that the cwd
 # in subprocesses like git equals our $PWD (for pathname comparisons).
 cd -P "$test" || exit 1
 
-HOME=$(pwd)
-export HOME
-
 this_test=${0##*/}
 this_test=${this_test%%-*}
 for skp in $GIT_SKIP_TESTS
-- 
1.7.4.1.271.g4540f

Re: [PATCH] HOME must be set before calling git-init when creating test repositories

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

> Otherwise the created test repositories will be affected by users ~/.gitconfig.
> For example, setting core.logAllrefupdates in users config will make all
> calls to "git config --unset core.logAllrefupdates" fail which will break
> the first test which uses the statement and expects it to succeed.

Doesn't this change the location of HOME used during the test as well?

Re: [PATCH] HOME must be set before calling git-init when creating test repositories

[Alex Riesen]

On Fri, Mar 25, 2011 at 21:49, Junio C Hamano <gitster@pobox.com> wrote:
> Alex Riesen <raa.lkml@gmail.com> writes:
>
>> Otherwise the created test repositories will be affected by users ~/.gitconfig.
>> For example, setting core.logAllrefupdates in users config will make all
>> calls to "git config --unset core.logAllrefupdates" fail which will break
>> the first test which uses the statement and expects it to succeed.
>
> Doesn't this change the location of HOME used during the test as well?
>

As long as the test only includes test-lib.sh only once - it doesn't.
Why? Or rather, how?

Re: [PATCH] HOME must be set before calling git-init when creating test repositories

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

> On Fri, Mar 25, 2011 at 21:49, Junio C Hamano <gitster@pobox.com> wrote:
>
>> Doesn't this change the location of HOME used during the test as well?
>
> As long as the test only includes test-lib.sh only once - it doesn't.
> Why? Or rather, how?

I thought you moved HOME=$(pwd) across "cd somewhere-else".  Doesn't it
change what is returned from pwd?

Re: [PATCH] HOME must be set before calling git-init when creating test repositories

[Alex Riesen]

On Fri, Mar 25, 2011 at 22:30, Junio C Hamano <gitster@pobox.com> wrote:
> Alex Riesen <raa.lkml@gmail.com> writes:
>
>> On Fri, Mar 25, 2011 at 21:49, Junio C Hamano <gitster@pobox.com> wrote:
>>
>>> Doesn't this change the location of HOME used during the test as well?
>>
>> As long as the test only includes test-lib.sh only once - it doesn't.
>> Why? Or rather, how?
>
> I thought you moved HOME=$(pwd) across "cd somewhere-else".  Doesn't it
> change what is returned from pwd?
>

Oh, it does. That's why the second patch (prefixed "[PATCH, fixed]").
It makes HOME to be "$(pwd)/somewhere-else", or precisely:

  HOME="$(pwd)"/"$test"
  export HOME

Re: [PATCH] HOME must be set before calling git-init when creating test repositories

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

> On Fri, Mar 25, 2011 at 22:30, Junio C Hamano <gitster@pobox.com> wrote:
>> Alex Riesen <raa.lkml@gmail.com> writes:
>>
>>> On Fri, Mar 25, 2011 at 21:49, Junio C Hamano <gitster@pobox.com> wrote:
>>>
>>>> Doesn't this change the location of HOME used during the test as well?
>>>
>>> As long as the test only includes test-lib.sh only once - it doesn't.
>>> Why? Or rather, how?
>>
>> I thought you moved HOME=$(pwd) across "cd somewhere-else".  Doesn't it
>> change what is returned from pwd?
>>
>
> Oh, it does. That's why the second patch (prefixed "[PATCH, fixed]").
> It makes HOME to be "$(pwd)/somewhere-else", or precisely:
>
>   HOME="$(pwd)"/"$test"
>   export HOME

What happens to people who has non-empty "$root", iow, their $test begins
with '/'?

I am not saying that having HOME at t/ directory instead of t/trash-*/
directory would necessarily break things (I don't know).  I am just
pointing out that the patch changes behaviour.

Re: [PATCH] HOME must be set before calling git-init when creating test repositories

[Alex Riesen]

On Fri, Mar 25, 2011 at 23:39, Junio C Hamano <gitster@pobox.com> wrote:
> Alex Riesen <raa.lkml@gmail.com> writes:
>
>> On Fri, Mar 25, 2011 at 22:30, Junio C Hamano <gitster@pobox.com> wrote:
>>> Alex Riesen <raa.lkml@gmail.com> writes:
>>>
>>>> On Fri, Mar 25, 2011 at 21:49, Junio C Hamano <gitster@pobox.com> wrote:
>>>>
>>>>> Doesn't this change the location of HOME used during the test as well?
>>>>
>>>> As long as the test only includes test-lib.sh only once - it doesn't.
>>>> Why? Or rather, how?
>>>
>>> I thought you moved HOME=$(pwd) across "cd somewhere-else".  Doesn't it
>>> change what is returned from pwd?
>>>
>>
>> Oh, it does. That's why the second patch (prefixed "[PATCH, fixed]").
>> It makes HOME to be "$(pwd)/somewhere-else", or precisely:
>>
>>   HOME="$(pwd)"/"$test"
>>   export HOME
>
> What happens to people who has non-empty "$root", iow, their $test begins
> with '/'?

It's still under $test then.

> I am not saying that having HOME at t/ directory instead of t/trash-*/
> directory would necessarily break things (I don't know).  I am just
> pointing out that the patch changes behaviour.

It does. I still think we're better off using the test's trash directory
for a this. For instance, consider the case when a user's .gitconfig
created by one of the tests collides with .gitconfig's of the other tests.
Either when running in parallel or just sequentially: the .gitconfig in
"t/" is not cleaned up after a test finishes.

Re: [PATCH] HOME must be set before calling git-init when creating test repositories

[Jeff King]

On Sat, Mar 26, 2011 at 11:08:06AM +0100, Alex Riesen wrote:

> >> Oh, it does. That's why the second patch (prefixed "[PATCH, fixed]").
> >> It makes HOME to be "$(pwd)/somewhere-else", or precisely:
> >>
> >>   HOME="$(pwd)"/"$test"
> >>   export HOME
> >
> > What happens to people who has non-empty "$root", iow, their $test begins
> > with '/'?
> 
> It's still under $test then.

No, it's totally broken. $(pwd)/$test is nonsensical. The code right
above your change guarantees that $test is an absolute path, either
because the user gave us an absolute $root or because it has been
prepended with $TEST_DIRECTORY (which itself comes from $(pwd)).

So the change you want is HOME=$test. But note that the code looks like
this then:

  HOME=$test
  export HOME
  test_create_repo "$test"
  cd -P "$test"

meaning that test_create_repo sees a non-existent HOME. I don't
think that matters, but if it did, you could do:

  HOME=$TEST_DIRECTORY
  export HOME
  test_create_repo "$test"
  cd -P "$test"
  HOME=$test

-Peff

[PATCH] HOME must be set before calling git-init when creating test repositories

[Alex Riesen]

Otherwise the created test repositories will be affected by users ~/.gitconfig.
For example, setting core.logAllrefupdates in users config will make all
calls to "git config --unset core.logAllrefupdates" fail which will break
the first test which uses the statement and expects it to succeed.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---

Jeff King, Sat, Mar 26, 2011 15:11:18 +0100:
> On Sat, Mar 26, 2011 at 11:08:06AM +0100, Alex Riesen wrote:
> 
> > >> Oh, it does. That's why the second patch (prefixed "[PATCH, fixed]").
> > >> It makes HOME to be "$(pwd)/somewhere-else", or precisely:
> > >>
> > >>   HOME="$(pwd)"/"$test"
> > >>   export HOME
> > >
> > > What happens to people who has non-empty "$root", iow, their $test begins
> > > with '/'?
> > 
> > It's still under $test then.
> 
> No, it's totally broken. $(pwd)/$test is nonsensical. The code right
> above your change guarantees that $test is an absolute path, either
> because the user gave us an absolute $root or because it has been
> prepended with $TEST_DIRECTORY (which itself comes from $(pwd)).

I see. I mistook "$root" for the root of a filesystem, not the variable in
test-lib.sh. How about this, than?

 t/test-lib.sh |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 7cc9a52..2b24c3d 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -984,14 +984,15 @@ rm -fr "$test" || {
 	exit 1
 }
 
+HOME="$(pwd)/$test"
+test -n "$root" && HOME="$test"
+export HOME
+
 test_create_repo "$test"
 # Use -P to resolve symlinks in our working directory so that the cwd
 # in subprocesses like git equals our $PWD (for pathname comparisons).
 cd -P "$test" || exit 1
 
-HOME=$(pwd)
-export HOME
-
 this_test=${0##*/}
 this_test=${this_test%%-*}
 for skp in $GIT_SKIP_TESTS
-- 
1.7.4.1.471.gab01

Re: [PATCH] HOME must be set before calling git-init when creating test repositories

[Jeff King]

On Sat, Mar 26, 2011 at 07:21:26PM +0100, Alex Riesen wrote:

> > No, it's totally broken. $(pwd)/$test is nonsensical. The code right
> > above your change guarantees that $test is an absolute path, either
> > because the user gave us an absolute $root or because it has been
> > prepended with $TEST_DIRECTORY (which itself comes from $(pwd)).
> 
> I see. I mistook "$root" for the root of a filesystem, not the variable in
> test-lib.sh. How about this, than?

Oops, when I said "$test" I meant to say $TRASH_DIRECTORY. That is,
$TRASH_DIRECTORY is always the absolute path to the trash.

> +HOME="$(pwd)/$test"
> +test -n "$root" && HOME="$test"
> +export HOME

So you can simplify this to just:

HOME=$TRASH_DIRECTORY

and not have to worry about checking $root at all.

Sorry for the confusion.

-Peff

Re: [PATCH] HOME must be set before calling git-init when creating test repositories

[Alex Riesen]

On Sat, Mar 26, 2011 at 19:31, Jeff King <peff@peff.net> wrote:
> On Sat, Mar 26, 2011 at 07:21:26PM +0100, Alex Riesen wrote:
>
>> > No, it's totally broken. $(pwd)/$test is nonsensical. The code right
>> > above your change guarantees that $test is an absolute path, either
>> > because the user gave us an absolute $root or because it has been
>> > prepended with $TEST_DIRECTORY (which itself comes from $(pwd)).
>>
>> I see. I mistook "$root" for the root of a filesystem, not the variable in
>> test-lib.sh. How about this, than?
>
> Oops, when I said "$test" I meant to say $TRASH_DIRECTORY. That is,
> $TRASH_DIRECTORY is always the absolute path to the trash.
>
>> +HOME="$(pwd)/$test"
>> +test -n "$root" && HOME="$test"
>> +export HOME
>
> So you can simplify this to just:
>
> HOME=$TRASH_DIRECTORY
>

Aah... I should have actually looked at the "case" which
sets TRASH_DIRECTORY!

Will resend in a moment.

[PATCH] HOME must be set before calling git-init when creating test repositories

[Alex Riesen]

Otherwise the created test repositories will be affected by users ~/.gitconfig.
For example, setting core.logAllrefupdates in users config will make all
calls to "git config --unset core.logAllrefupdates" fail which will break
the first test which uses the statement and expects it to succeed.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---

Alex Riesen, Sat, Mar 26, 2011 19:42:14 +0100:
> On Sat, Mar 26, 2011 at 19:31, Jeff King <peff@peff.net> wrote:
> >
> > So you can simplify this to just:
> >
> > HOME=$TRASH_DIRECTORY
> 
> Aah... I should have actually looked at the "case" which
> sets TRASH_DIRECTORY!
> 
> Will resend in a moment.

Here.

 t/test-lib.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 7cc9a52..7965b74 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -984,14 +984,14 @@ rm -fr "$test" || {
 	exit 1
 }
 
+HOME="$TRASH_DIRECTORY"
+export HOME
+
 test_create_repo "$test"
 # Use -P to resolve symlinks in our working directory so that the cwd
 # in subprocesses like git equals our $PWD (for pathname comparisons).
 cd -P "$test" || exit 1
 
-HOME=$(pwd)
-export HOME
-
 this_test=${0##*/}
 this_test=${this_test%%-*}
 for skp in $GIT_SKIP_TESTS
-- 
1.7.4.1.471.gab01

Re: [PATCH] HOME must be set before calling git-init when creating test repositories

[Jeff King]

On Sat, Mar 26, 2011 at 07:46:34PM +0100, Alex Riesen wrote:

> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 7cc9a52..7965b74 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -984,14 +984,14 @@ rm -fr "$test" || {
>  	exit 1
>  }
>  
> +HOME="$TRASH_DIRECTORY"
> +export HOME
> +
>  test_create_repo "$test"
>  # Use -P to resolve symlinks in our working directory so that the cwd
>  # in subprocesses like git equals our $PWD (for pathname comparisons).
>  cd -P "$test" || exit 1
>  
> -HOME=$(pwd)
> -export HOME
> -

That looks right to me. Thanks.

-Peff