{"thread":{"id":"26863","subject":"[PATCH] HOME must be set before calling git-init when creating test repositories","startedAt":"2011-03-25T20:05:28Z","lastAt":"2011-03-26T18:48:06Z","messageCount":14,"participants":["Alex Riesen","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"164324","messageId":"20110325200528.GA7302@blimp.localdomain","threadId":"26863","inReplyTo":null,"subject":"[PATCH] HOME must be set before calling git-init when creating test repositories","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2011-03-25T20:05:28Z","receivedAt":"2011-03-25T20:05:28Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Otherwise the created test repositories will be affected by users ~/.gitconfig.\nFor example, setting core.logAllrefupdates in users config will make all\ncalls to \"git config --unset core.logAllrefupdates\" fail which will break\nthe first test which uses the statement and expects it to succeed.\n\nSigned-off-by: Alex Riesen \n---\n\nThe first test which fails is t2017, btw.\nI still wonder if this should be moved even further up.\n\n t/test-lib.sh | 6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 7cc9a52..4f394c3 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -984,14 +984,14 @@ rm -fr \"$test\" || {\n \texit 1\n }\n \n+HOME=$(pwd)\n+export HOME\n+\n test_create_repo \"$test\"\n # Use -P to resolve symlinks in our working directory so that the cwd\n # in subprocesses like git equals our $PWD (for pathname comparisons).\n cd -P \"$test\" || exit 1\n \n-HOME=$(pwd)\n-export HOME\n-\n this_test=${0##*/}\n this_test=${this_test%%-*}\n for skp in $GIT_SKIP_TESTS\n-- \n1.7.4.1.271.g4540f\n"},{"id":"164325","messageId":"20110325204454.GA14031@blimp.localdomain","threadId":"26863","inReplyTo":"20110325200528.GA7302@blimp.localdomain","subject":"[PATCH, fixed] HOME must be set before calling git-init when creating test repositories","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2011-03-25T20:44:54Z","receivedAt":"2011-03-25T20:44:54Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Otherwise the created test repositories will be affected by users ~/.gitconfig.\nFor example, setting core.logAllrefupdates in users config will make all\ncalls to \"git config --unset core.logAllrefupdates\" fail which will break\nthe first test which uses the statement and expects it to succeed.\n\nSigned-off-by: Alex Riesen \n---\n\nAah... Sorry. Missed the test's part of the new HOME.\n\n t/test-lib.sh | 6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 7cc9a52..8792e4a 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -984,14 +984,14 @@ rm -fr \"$test\" || {\n \texit 1\n }\n \n+HOME=$(pwd)/\"$test\"\n+export HOME\n+\n test_create_repo \"$test\"\n # Use -P to resolve symlinks in our working directory so that the cwd\n # in subprocesses like git equals our $PWD (for pathname comparisons).\n cd -P \"$test\" || exit 1\n \n-HOME=$(pwd)\n-export HOME\n-\n this_test=${0##*/}\n this_test=${this_test%%-*}\n for skp in $GIT_SKIP_TESTS\n-- \n1.7.4.1.271.g4540f\n"},{"id":"164326","messageId":"7vsjub53j2.fsf@alter.siamese.dyndns.org","threadId":"26863","inReplyTo":"20110325200528.GA7302@blimp.localdomain","subject":"Re: [PATCH] HOME must be set before calling git-init when creating test repositories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-25T20:49:37Z","receivedAt":"2011-03-25T20:49:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n> Otherwise the created test repositories will be affected by users ~/.gitconfig.\n> For example, setting core.logAllrefupdates in users config will make all\n> calls to \"git config --unset core.logAllrefupdates\" fail which will break\n> the first test which uses the statement and expects it to succeed.\n\nDoesn't this change the location of HOME used during the test as well?\n"},{"id":"164327","messageId":"AANLkTimQzPVOWCUx1hr+DEmRfdFB8=UeY_xCaxzVSwFy@mail.gmail.com","threadId":"26863","inReplyTo":"7vsjub53j2.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] HOME must be set before calling git-init when creating test repositories","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2011-03-25T21:01:55Z","receivedAt":"2011-03-25T21:01:55Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Fri, Mar 25, 2011 at 21:49, Junio C Hamano wrote:\n> Alex Riesen writes:\n>\n>> Otherwise the created test repositories will be affected by users ~/.gitconfig.\n>> For example, setting core.logAllrefupdates in users config will make all\n>> calls to \"git config --unset core.logAllrefupdates\" fail which will break\n>> the first test which uses the statement and expects it to succeed.\n>\n> Doesn't this change the location of HOME used during the test as well?\n>\n\nAs long as the test only includes test-lib.sh only once - it doesn't.\nWhy? Or rather, how?\n"},{"id":"164329","messageId":"7voc4y6g6v.fsf@alter.siamese.dyndns.org","threadId":"26863","inReplyTo":"AANLkTimQzPVOWCUx1hr+DEmRfdFB8=UeY_xCaxzVSwFy@mail.gmail.com","subject":"Re: [PATCH] HOME must be set before calling git-init when creating test repositories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-25T21:30:48Z","receivedAt":"2011-03-25T21:30:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n> On Fri, Mar 25, 2011 at 21:49, Junio C Hamano wrote:\n>\n>> Doesn't this change the location of HOME used during the test as well?\n>\n> As long as the test only includes test-lib.sh only once - it doesn't.\n> Why? Or rather, how?\n\nI thought you moved HOME=$(pwd) across \"cd somewhere-else\". Doesn't it\nchange what is returned from pwd?\n"},{"id":"164334","messageId":"AANLkTikO4=TtFtpsQ_JtmGKqQ1NzgTuo7e8cdaQca+T7@mail.gmail.com","threadId":"26863","inReplyTo":"7voc4y6g6v.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] HOME must be set before calling git-init when creating test repositories","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2011-03-25T21:51:58Z","receivedAt":"2011-03-25T21:51:58Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Fri, Mar 25, 2011 at 22:30, Junio C Hamano wrote:\n> Alex Riesen writes:\n>\n>> On Fri, Mar 25, 2011 at 21:49, Junio C Hamano wrote:\n>>\n>>> Doesn't this change the location of HOME used during the test as well?\n>>\n>> As long as the test only includes test-lib.sh only once - it doesn't.\n>> Why? Or rather, how?\n>\n> I thought you moved HOME=$(pwd) across \"cd somewhere-else\".  Doesn't it\n> change what is returned from pwd?\n>\n\nOh, it does. That's why the second patch (prefixed \"[PATCH, fixed]\").\nIt makes HOME to be \"$(pwd)/somewhere-else\", or precisely:\n\n HOME=\"$(pwd)\"/\"$test\"\n export HOME\n"},{"id":"164338","messageId":"7vvcz64ygm.fsf@alter.siamese.dyndns.org","threadId":"26863","inReplyTo":"AANLkTikO4=TtFtpsQ_JtmGKqQ1NzgTuo7e8cdaQca+T7@mail.gmail.com","subject":"Re: [PATCH] HOME must be set before calling git-init when creating test repositories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-25T22:39:05Z","receivedAt":"2011-03-25T22:39:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n> On Fri, Mar 25, 2011 at 22:30, Junio C Hamano wrote:\n>> Alex Riesen writes:\n>>\n>>> On Fri, Mar 25, 2011 at 21:49, Junio C Hamano wrote:\n>>>\n>>>> Doesn't this change the location of HOME used during the test as well?\n>>>\n>>> As long as the test only includes test-lib.sh only once - it doesn't.\n>>> Why? Or rather, how?\n>>\n>> I thought you moved HOME=$(pwd) across \"cd somewhere-else\".  Doesn't it\n>> change what is returned from pwd?\n>>\n>\n> Oh, it does. That's why the second patch (prefixed \"[PATCH, fixed]\").\n> It makes HOME to be \"$(pwd)/somewhere-else\", or precisely:\n>\n> HOME=\"$(pwd)\"/\"$test\"\n> export HOME\n\nWhat happens to people who has non-empty \"$root\", iow, their $test begins\nwith '/'?\n\nI am not saying that having HOME at t/ directory instead of t/trash-*/\ndirectory would necessarily break things (I don't know). I am just\npointing out that the patch changes behaviour.\n"},{"id":"164357","messageId":"AANLkTi=+SZGxLyP8vFPpmK8DZvke6-Tu-crwq5+89qWx@mail.gmail.com","threadId":"26863","inReplyTo":"7vvcz64ygm.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] HOME must be set before calling git-init when creating test repositories","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2011-03-26T10:08:06Z","receivedAt":"2011-03-26T10:08:06Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Fri, Mar 25, 2011 at 23:39, Junio C Hamano wrote:\n> Alex Riesen writes:\n>\n>> On Fri, Mar 25, 2011 at 22:30, Junio C Hamano wrote:\n>>> Alex Riesen writes:\n>>>\n>>>> On Fri, Mar 25, 2011 at 21:49, Junio C Hamano wrote:\n>>>>\n>>>>> Doesn't this change the location of HOME used during the test as well?\n>>>>\n>>>> As long as the test only includes test-lib.sh only once - it doesn't.\n>>>> Why? Or rather, how?\n>>>\n>>> I thought you moved HOME=$(pwd) across \"cd somewhere-else\".  Doesn't it\n>>> change what is returned from pwd?\n>>>\n>>\n>> Oh, it does. That's why the second patch (prefixed \"[PATCH, fixed]\").\n>> It makes HOME to be \"$(pwd)/somewhere-else\", or precisely:\n>>\n>>   HOME=\"$(pwd)\"/\"$test\"\n>>   export HOME\n>\n> What happens to people who has non-empty \"$root\", iow, their $test begins\n> with '/'?\n\nIt's still under $test then.\n\n> I am not saying that having HOME at t/ directory instead of t/trash-*/\n> directory would necessarily break things (I don't know).  I am just\n> pointing out that the patch changes behaviour.\n\nIt does. I still think we're better off using the test's trash directory\nfor a this. For instance, consider the case when a user's .gitconfig\ncreated by one of the tests collides with .gitconfig's of the other tests.\nEither when running in parallel or just sequentially: the .gitconfig in\n\"t/\" is not cleaned up after a test finishes.\n"},{"id":"164363","messageId":"20110326141118.GA3475@sigill.intra.peff.net","threadId":"26863","inReplyTo":"AANLkTi=+SZGxLyP8vFPpmK8DZvke6-Tu-crwq5+89qWx@mail.gmail.com","subject":"Re: [PATCH] HOME must be set before calling git-init when creating test repositories","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-26T14:11:18Z","receivedAt":"2011-03-26T14:11:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 26, 2011 at 11:08:06AM +0100, Alex Riesen wrote:\n\n> >> Oh, it does. That's why the second patch (prefixed \"[PATCH, fixed]\").\n> >> It makes HOME to be \"$(pwd)/somewhere-else\", or precisely:\n> >>\n> >>   HOME=\"$(pwd)\"/\"$test\"\n> >>   export HOME\n> >\n> > What happens to people who has non-empty \"$root\", iow, their $test begins\n> > with '/'?\n> \n> It's still under $test then.\n\nNo, it's totally broken. $(pwd)/$test is nonsensical. The code right\nabove your change guarantees that $test is an absolute path, either\nbecause the user gave us an absolute $root or because it has been\nprepended with $TEST_DIRECTORY (which itself comes from $(pwd)).\n\nSo the change you want is HOME=$test. But note that the code looks like\nthis then:\n\n HOME=$test\n export HOME\n test_create_repo \"$test\"\n cd -P \"$test\"\n\nmeaning that test_create_repo sees a non-existent HOME. I don't\nthink that matters, but if it did, you could do:\n\n HOME=$TEST_DIRECTORY\n export HOME\n test_create_repo \"$test\"\n cd -P \"$test\"\n HOME=$test\n\n-Peff\n"},{"id":"164372","messageId":"20110326182126.GA13496@blimp.localdomain","threadId":"26863","inReplyTo":"20110326141118.GA3475@sigill.intra.peff.net","subject":"[PATCH] HOME must be set before calling git-init when creating test repositories","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2011-03-26T18:21:26Z","receivedAt":"2011-03-26T18:21:26Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Otherwise the created test repositories will be affected by users ~/.gitconfig.\nFor example, setting core.logAllrefupdates in users config will make all\ncalls to \"git config --unset core.logAllrefupdates\" fail which will break\nthe first test which uses the statement and expects it to succeed.\n\nSigned-off-by: Alex Riesen \n---\n\nJeff King, Sat, Mar 26, 2011 15:11:18 +0100:\n> On Sat, Mar 26, 2011 at 11:08:06AM +0100, Alex Riesen wrote:\n> \n> > >> Oh, it does. That's why the second patch (prefixed \"[PATCH, fixed]\").\n> > >> It makes HOME to be \"$(pwd)/somewhere-else\", or precisely:\n> > >>\n> > >>   HOME=\"$(pwd)\"/\"$test\"\n> > >>   export HOME\n> > >\n> > > What happens to people who has non-empty \"$root\", iow, their $test begins\n> > > with '/'?\n> > \n> > It's still under $test then.\n> \n> No, it's totally broken. $(pwd)/$test is nonsensical. The code right\n> above your change guarantees that $test is an absolute path, either\n> because the user gave us an absolute $root or because it has been\n> prepended with $TEST_DIRECTORY (which itself comes from $(pwd)).\n\nI see. I mistook \"$root\" for the root of a filesystem, not the variable in\ntest-lib.sh. How about this, than?\n\n t/test-lib.sh | 7 ++++---\n 1 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 7cc9a52..2b24c3d 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -984,14 +984,15 @@ rm -fr \"$test\" || {\n \texit 1\n }\n \n+HOME=\"$(pwd)/$test\"\n+test -n \"$root\" && HOME=\"$test\"\n+export HOME\n+\n test_create_repo \"$test\"\n # Use -P to resolve symlinks in our working directory so that the cwd\n # in subprocesses like git equals our $PWD (for pathname comparisons).\n cd -P \"$test\" || exit 1\n \n-HOME=$(pwd)\n-export HOME\n-\n this_test=${0##*/}\n this_test=${this_test%%-*}\n for skp in $GIT_SKIP_TESTS\n-- \n1.7.4.1.471.gab01\n"},{"id":"164373","messageId":"20110326183102.GA3796@sigill.intra.peff.net","threadId":"26863","inReplyTo":"20110326182126.GA13496@blimp.localdomain","subject":"Re: [PATCH] HOME must be set before calling git-init when creating test repositories","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-26T18:31:02Z","receivedAt":"2011-03-26T18:31:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 26, 2011 at 07:21:26PM +0100, Alex Riesen wrote:\n\n> > No, it's totally broken. $(pwd)/$test is nonsensical. The code right\n> > above your change guarantees that $test is an absolute path, either\n> > because the user gave us an absolute $root or because it has been\n> > prepended with $TEST_DIRECTORY (which itself comes from $(pwd)).\n> \n> I see. I mistook \"$root\" for the root of a filesystem, not the variable in\n> test-lib.sh. How about this, than?\n\nOops, when I said \"$test\" I meant to say $TRASH_DIRECTORY. That is,\n$TRASH_DIRECTORY is always the absolute path to the trash.\n\n> +HOME=\"$(pwd)/$test\"\n> +test -n \"$root\" && HOME=\"$test\"\n> +export HOME\n\nSo you can simplify this to just:\n\nHOME=$TRASH_DIRECTORY\n\nand not have to worry about checking $root at all.\n\nSorry for the confusion.\n\n-Peff\n"},{"id":"164375","messageId":"AANLkTi=Pt_Pw9BvL6y8Wq34PeRHVOZpr6ZekRYqfDXq6@mail.gmail.com","threadId":"26863","inReplyTo":"20110326183102.GA3796@sigill.intra.peff.net","subject":"Re: [PATCH] HOME must be set before calling git-init when creating test repositories","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2011-03-26T18:42:14Z","receivedAt":"2011-03-26T18:42:14Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Sat, Mar 26, 2011 at 19:31, Jeff King wrote:\n> On Sat, Mar 26, 2011 at 07:21:26PM +0100, Alex Riesen wrote:\n>\n>> > No, it's totally broken. $(pwd)/$test is nonsensical. The code right\n>> > above your change guarantees that $test is an absolute path, either\n>> > because the user gave us an absolute $root or because it has been\n>> > prepended with $TEST_DIRECTORY (which itself comes from $(pwd)).\n>>\n>> I see. I mistook \"$root\" for the root of a filesystem, not the variable in\n>> test-lib.sh. How about this, than?\n>\n> Oops, when I said \"$test\" I meant to say $TRASH_DIRECTORY. That is,\n> $TRASH_DIRECTORY is always the absolute path to the trash.\n>\n>> +HOME=\"$(pwd)/$test\"\n>> +test -n \"$root\" && HOME=\"$test\"\n>> +export HOME\n>\n> So you can simplify this to just:\n>\n> HOME=$TRASH_DIRECTORY\n>\n\nAah... I should have actually looked at the \"case\" which\nsets TRASH_DIRECTORY!\n\nWill resend in a moment.\n"},{"id":"164376","messageId":"20110326184634.GB13496@blimp.localdomain","threadId":"26863","inReplyTo":"AANLkTi=Pt_Pw9BvL6y8Wq34PeRHVOZpr6ZekRYqfDXq6@mail.gmail.com","subject":"[PATCH] HOME must be set before calling git-init when creating test repositories","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2011-03-26T18:46:34Z","receivedAt":"2011-03-26T18:46:34Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Otherwise the created test repositories will be affected by users ~/.gitconfig.\nFor example, setting core.logAllrefupdates in users config will make all\ncalls to \"git config --unset core.logAllrefupdates\" fail which will break\nthe first test which uses the statement and expects it to succeed.\n\nSigned-off-by: Alex Riesen \n---\n\nAlex Riesen, Sat, Mar 26, 2011 19:42:14 +0100:\n> On Sat, Mar 26, 2011 at 19:31, Jeff King wrote:\n> >\n> > So you can simplify this to just:\n> >\n> > HOME=$TRASH_DIRECTORY\n> \n> Aah... I should have actually looked at the \"case\" which\n> sets TRASH_DIRECTORY!\n> \n> Will resend in a moment.\n\nHere.\n\n t/test-lib.sh | 6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 7cc9a52..7965b74 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -984,14 +984,14 @@ rm -fr \"$test\" || {\n \texit 1\n }\n \n+HOME=\"$TRASH_DIRECTORY\"\n+export HOME\n+\n test_create_repo \"$test\"\n # Use -P to resolve symlinks in our working directory so that the cwd\n # in subprocesses like git equals our $PWD (for pathname comparisons).\n cd -P \"$test\" || exit 1\n \n-HOME=$(pwd)\n-export HOME\n-\n this_test=${0##*/}\n this_test=${this_test%%-*}\n for skp in $GIT_SKIP_TESTS\n-- \n1.7.4.1.471.gab01\n"},{"id":"164377","messageId":"20110326184806.GA4488@sigill.intra.peff.net","threadId":"26863","inReplyTo":"20110326184634.GB13496@blimp.localdomain","subject":"Re: [PATCH] HOME must be set before calling git-init when creating test repositories","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-26T18:48:06Z","receivedAt":"2011-03-26T18:48:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 26, 2011 at 07:46:34PM +0100, Alex Riesen wrote:\n\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 7cc9a52..7965b74 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -984,14 +984,14 @@ rm -fr \"$test\" || {\n> \texit 1\n> }\n> \n> +HOME=\"$TRASH_DIRECTORY\"\n> +export HOME\n> +\n> test_create_repo \"$test\"\n> # Use -P to resolve symlinks in our working directory so that the cwd\n> # in subprocesses like git equals our $PWD (for pathname comparisons).\n> cd -P \"$test\" || exit 1\n> \n> -HOME=$(pwd)\n> -export HOME\n> -\n\nThat looks right to me. Thanks.\n\n-Peff\n"}]}