threads / patch / 26863

patchHOME must be set before calling git-init when creating test repositories

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

## tl;dr

14 messages between Mar 25, 2011 and Mar 26, 2011. Diffs are folded; open one to read it.

replies: 13people: 3as markdown or json

Alex Riesen· Mar 25, 2011, 20:05 UTC · lore

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(-)
Show changes to t/test-lib.sh +3 −3
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
Alex Riesen· Mar 25, 2011, 20:44 UTC · re: Alex Riesen · lore

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

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(-)
Show changes to t/test-lib.sh +3 −3
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
Junio C Hamano· Mar 25, 2011, 20:49 UTC · re: Alex Riesen · lore

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

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?
Alex Riesen· Mar 25, 2011, 21:01 UTC · re: Junio C Hamano · lore

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

On Fri, Mar 25, 2011 at 21:49, Junio C Hamano <gitster@pobox.com> wrote:
Show 9 quoted lines
> 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?

Junio C Hamano· Mar 25, 2011, 21:30 UTC · re: Alex Riesen · lore

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

Alex Riesen <raa.lkml@gmail.com> writes:
Show 6 quoted lines
> 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?

Alex Riesen· Mar 25, 2011, 21:51 UTC · re: Junio C Hamano · lore

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

On Fri, Mar 25, 2011 at 22:30, Junio C Hamano <gitster@pobox.com> wrote:
Show 12 quoted lines
> 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
Junio C Hamano· Mar 25, 2011, 22:39 UTC · re: Alex Riesen · lore

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

Alex Riesen <raa.lkml@gmail.com> writes:
Show 19 quoted lines
> 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.

Alex Riesen· Mar 26, 2011, 10:08 UTC · re: Junio C Hamano · lore

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

On Fri, Mar 25, 2011 at 23:39, Junio C Hamano <gitster@pobox.com> wrote:
Show 24 quoted lines
> 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.

Jeff King· Mar 26, 2011, 14:11 UTC · re: Alex Riesen · lore

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

On Sat, Mar 26, 2011 at 11:08:06AM +0100, Alex Riesen wrote:
Show 10 quoted lines
> >> 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
Alex Riesen· Mar 26, 2011, 18:21 UTC · re: Jeff King · lore

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:
Show 17 quoted lines
> 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(-)
Show changes to t/test-lib.sh +4 −3
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
Jeff King· Mar 26, 2011, 18:31 UTC · re: Alex Riesen · lore

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

On Sat, Mar 26, 2011 at 07:21:26PM +0100, Alex Riesen wrote:
Show 7 quoted lines
> > 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
Alex Riesen· Mar 26, 2011, 18:42 UTC · re: Jeff King · lore

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

On Sat, Mar 26, 2011 at 19:31, Jeff King <peff@peff.net> wrote:
Show 21 quoted lines
> 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.
Alex Riesen· Mar 26, 2011, 18:46 UTC · re: Alex Riesen · lore

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:
Show 10 quoted lines
> 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(-)
Show changes to t/test-lib.sh +3 −3
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
Jeff King· Mar 26, 2011, 18:48 UTC · re: Alex Riesen · lore

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

On Sat, Mar 26, 2011 at 07:46:34PM +0100, Alex Riesen wrote:
Show 19 quoted lines
> 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

← back to recent threads