{"thread":{"id":"31139","subject":"[PATCH] Improve tests for detached worktree in git-submodule","startedAt":"2012-07-30T16:10:10Z","lastAt":"2012-07-30T18:15:45Z","messageCount":12,"participants":["Daniel Graña","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"196151","messageId":"1343664610-479-1-git-send-email-dangra@gmail.com","threadId":"31139","inReplyTo":null,"subject":"[PATCH] Improve tests for detached worktree in git-submodule","fromName":"Daniel Graña","fromEmail":"dangra@gmail.com","sentAt":"2012-07-30T16:10:10Z","receivedAt":"2012-07-30T16:10:10Z","isPatch":true,"sender":{"key":"dangra@gmail.com","avatar":"https://avatars.githubusercontent.com/u/37369?v=4"},"body":"\nSigned-off-by: Daniel Graña \n---\n t/t7409-submodule-detached-worktree.sh | 31 ++++++++++++++++++++++++-------\n 1 files changed, 24 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t7409-submodule-detached-worktree.sh b/t/t7409-submodule-detached-worktree.sh\nindex db75642..d88f400 100755\n--- a/t/t7409-submodule-detached-worktree.sh\n+++ b/t/t7409-submodule-detached-worktree.sh\n@@ -15,7 +15,11 @@ TEST_NO_CREATE_REPO=1\n test_expect_success 'submodule on detached working tree' '\n \tgit init --bare remote &&\n \ttest_create_repo bundle1 &&\n-\t(cd bundle1 && test_commit \"shoot\") &&\n+\t(\n+\t\tcd bundle1 &&\n+\t\ttest_commit \"shoot\" &&\n+\t\tgit rev-list --max-count=1 HEAD > \"$TRASH_DIRECTORY/expect\"\n+\t) &&\n \tmkdir home &&\n \t(\n \t\tcd home &&\n@@ -23,14 +27,27 @@ test_expect_success 'submodule on detached working tree' '\n \t\tgit clone --bare ../remote .dotfiles &&\n \t\tgit submodule add ../bundle1 .vim/bundle/sogood &&\n \t\ttest_commit \"sogood\" &&\n+\t\t(\n+\t\t\tunset GIT_WORK_TREE GIT_DIR &&\n+\t\t\tcd .vim/bundle/sogood &&\n+\t\t\tgit rev-list --max-count=1 HEAD > actual &&\n+\t\t\ttest_cmp actual \"$TRASH_DIRECTORY/expect\"\n+\t\t) &&\n \t\tgit push origin master\n-\t) &&\n+\t)\n \tmkdir home2 &&\n \t(\n \t\tcd home2 &&\n-\t\texport GIT_WORK_TREE=\"$(pwd)\" GIT_DIR=\"$(pwd)/.dotfiles\" &&\n \t\tgit clone --bare ../remote .dotfiles &&\n-\t\tgit submodule update --init\n+\t\texport GIT_WORK_TREE=\"$(pwd)\" GIT_DIR=\"$(pwd)/.dotfiles\" &&\n+\t\tgit checkout master &&\n+\t\tgit submodule update --init &&\n+\t\t(\n+\t\t\tunset GIT_WORK_TREE GIT_DIR &&\n+\t\t\tcd .vim/bundle/sogood &&\n+\t\t\tgit rev-list --max-count=1 HEAD > actual &&\n+\t\t\ttest_cmp actual \"$TRASH_DIRECTORY/expect\"\n+\t\t)\n \t)\n '\n \n@@ -42,6 +59,7 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '\n \t\tgit clone --bare ../remote \"$GIT_DIR\" &&\n \t\tgit config core.bare false &&\n \t\tgit config core.worktree .. &&\n+\t\tgit checkout master &&\n \t\tgit submodule add ../bundle1 .vim/bundle/dupe &&\n \t\ttest_commit \"dupe\" &&\n \t\tgit push origin master\n@@ -52,9 +70,8 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '\n \t\tgit config core.bare false &&\n \t\tgit config core.worktree .. &&\n \t\tgit pull &&\n-\t\tgit submodule update &&\n-\t\tgit submodule status &&\n-\t\ttest -d .vim/bundle/dupe\n+\t\tgit submodule update --init &&\n+\t\ttest -e .vim/bundle/dupe/shoot.t\n \t)\n '\n \n-- \n1.7.5.4\n"},{"id":"196156","messageId":"20120730163907.GA18109@sigill.intra.peff.net","threadId":"31139","inReplyTo":"1343664610-479-1-git-send-email-dangra@gmail.com","subject":"Re: [PATCH] Improve tests for detached worktree in git-submodule","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-30T16:39:07Z","receivedAt":"2012-07-30T16:39:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 30, 2012 at 01:10:10PM -0300, Daniel Graña wrote:\n\n> Subject: Re: [PATCH] Improve tests for detached worktree in git-submodule\n> \n> Signed-off-by: Daniel Graña \n\nThe space between the subject and your S-o-b is an excellent place to\nexplain the rationale for your commit.\n\nHow are we improving them? What cases or classes of failure does this\ncatch that the original did not? It may be because I have not been\nfollowing this topic closely, but reading the patch, I am not sure what\nthe purpose is. Please make life easier for reviewers by telling us what\nto expect and why before we even get to the patch.\n\n-Peff\n"},{"id":"196162","messageId":"7v6295rxmb.fsf@alter.siamese.dyndns.org","threadId":"31139","inReplyTo":"1343664610-479-1-git-send-email-dangra@gmail.com","subject":"Re: [PATCH] Improve tests for detached worktree in git-submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-30T17:02:04Z","receivedAt":"2012-07-30T17:02:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Graña writes:\n\n> Signed-off-by: Daniel Graña \n> ---\n> t/t7409-submodule-detached-worktree.sh | 31 ++++++++++++++++++++++++-------\n> 1 files changed, 24 insertions(+), 7 deletions(-)\n>\n> diff --git a/t/t7409-submodule-detached-worktree.sh b/t/t7409-submodule-detached-worktree.sh\n> index db75642..d88f400 100755\n> --- a/t/t7409-submodule-detached-worktree.sh\n> +++ b/t/t7409-submodule-detached-worktree.sh\n> @@ -15,7 +15,11 @@ TEST_NO_CREATE_REPO=1\n> test_expect_success 'submodule on detached working tree' '\n> \tgit init --bare remote &&\n> \ttest_create_repo bundle1 &&\n> -\t(cd bundle1 && test_commit \"shoot\") &&\n> +\t(\n> +\t\tcd bundle1 &&\n> +\t\ttest_commit \"shoot\" &&\n> +\t\tgit rev-list --max-count=1 HEAD > \"$TRASH_DIRECTORY/expect\"\n\nBetter written as\n\n\tgit rev-parse --verify HEAD >../expect\n\nmethinks.\n\n> +\t) &&\n> \tmkdir home &&\n> \t(\n> \t\tcd home &&\n> @@ -23,14 +27,27 @@ test_expect_success 'submodule on detached working tree' '\n> \t\tgit clone --bare ../remote .dotfiles &&\n> \t\tgit submodule add ../bundle1 .vim/bundle/sogood &&\n> \t\ttest_commit \"sogood\" &&\n> +\t\t(\n> +\t\t\tunset GIT_WORK_TREE GIT_DIR &&\n> +\t\t\tcd .vim/bundle/sogood &&\n> +\t\t\tgit rev-list --max-count=1 HEAD > actual &&\n> +\t\t\ttest_cmp actual \"$TRASH_DIRECTORY/expect\"\n\nLikewise.\n\n\tgit rev-parse --verify HEAD >actual &&\n test_cmp ../expect actual\n\nAs test_cmp turns into \"diff -u\", comparing expect against actual\n(instead of the other way around) shows the deviation in a more\nreadable way when your test fails.\n\n> +\t\t) &&\n> \t\tgit push origin master\n> -\t) &&\n> +\t)\n> \tmkdir home2 &&\n> \t(\n> \t\tcd home2 &&\n> -\t\texport GIT_WORK_TREE=\"$(pwd)\" GIT_DIR=\"$(pwd)/.dotfiles\" &&\n> \t\tgit clone --bare ../remote .dotfiles &&\n> -\t\tgit submodule update --init\n> +\t\texport GIT_WORK_TREE=\"$(pwd)\" GIT_DIR=\"$(pwd)/.dotfiles\" &&\n\nSo you used to clone with the two environment variables in effect to\ncreate the .otfiles, but now you don't use them while cloning. That\nmakes more sense to me, especially the .otfiles is created \"bare\".\n\n> +\t\tgit checkout master &&\n\nSo you populate the newly created home2 working tree out of the .otfiles\nrepository in it.\n\n> +\t\tgit submodule update --init &&\n> +\t\t(\n> +\t\t\tunset GIT_WORK_TREE GIT_DIR &&\n> +\t\t\tcd .vim/bundle/sogood &&\n> +\t\t\tgit rev-list --max-count=1 HEAD > actual &&\n> +\t\t\ttest_cmp actual \"$TRASH_DIRECTORY/expect\"\n\nLikewise.\n\n> +\t\t)\n> \t)\n> '\n> \n> @@ -42,6 +59,7 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '\n> \t\tgit clone --bare ../remote \"$GIT_DIR\" &&\n> \t\tgit config core.bare false &&\n> \t\tgit config core.worktree .. &&\n> +\t\tgit checkout master &&\n> \t\tgit submodule add ../bundle1 .vim/bundle/dupe &&\n> \t\ttest_commit \"dupe\" &&\n> \t\tgit push origin master\n> @@ -52,9 +70,8 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '\n> \t\tgit config core.bare false &&\n> \t\tgit config core.worktree .. &&\n> \t\tgit pull &&\n> -\t\tgit submodule update &&\n> -\t\tgit submodule status &&\n> -\t\ttest -d .vim/bundle/dupe\n> +\t\tgit submodule update --init &&\n> +\t\ttest -e .vim/bundle/dupe/shoot.t\n\nIs the \"existence\" the only thing you care about? That's not all\nthat different from the old test that only checked the existence of\nthe directory dupe, no?\n\n> \t)\n> '\n"},{"id":"196163","messageId":"CAHCkQtOYC_jCEuYdEcJ73-tmKCa_skDUsTOhrcoGLJ+qdG9XgA@mail.gmail.com","threadId":"31139","inReplyTo":"CAHCkQtNyNGBm8Z8FP7BybVOW0zQNgpxjwW_akLepYfLc-U+0cg@mail.gmail.com","subject":"Fwd: [PATCH] Improve tests for detached worktree in git-submodule","fromName":"Daniel Graña","fromEmail":"dangra@gmail.com","sentAt":"2012-07-30T17:06:57Z","receivedAt":"2012-07-30T17:06:57Z","isPatch":true,"sender":{"key":"dangra@gmail.com","avatar":"https://avatars.githubusercontent.com/u/37369?v=4"},"body":"Forwardning for the record as vger rejected my previous HTML email.\n\n---------- Forwarded message ----------\nFrom: Daniel Graña \nDate: Mon, Jul 30, 2012 at 2:04 PM\nSubject: Re: [PATCH] Improve tests for detached worktree in git-submodule\nTo: Jeff King \nCc: Junio C Hamano , git@vger.kernel.org\n\n\nHi Jeff,\n\nI understand your complain and I must apologize for confusing you with Jens\nfrom this thread\nhttp://thread.gmane.org/gmane.comp.version-control.git/201851\n\nThis patch is a follow up to that thread (that I fail to include as\nreply-to) and tries to address the issues noted by Junio and Jens there.\n\n\n\nOn Mon, Jul 30, 2012 at 1:39 PM, Jeff King wrote:\n>\n> On Mon, Jul 30, 2012 at 01:10:10PM -0300, Daniel Graña wrote:\n>\n> > Subject: Re: [PATCH] Improve tests for detached worktree in\n> > git-submodule\n> >\n> > Signed-off-by: Daniel Graña \n>\n> The space between the subject and your S-o-b is an excellent place to\n> explain the rationale for your commit.\n>\n> How are we improving them? What cases or classes of failure does this\n> catch that the original did not? It may be because I have not been\n> following this topic closely, but reading the patch, I am not sure what\n> the purpose is. Please make life easier for reviewers by telling us what\n> to expect and why before we even get to the patch.\n>\n> -Peff\n"},{"id":"196166","messageId":"CAHCkQtPmJdFBk7G-EP70FwtkRk11Utp+rppjNrEZFtZgOVrObA@mail.gmail.com","threadId":"31139","inReplyTo":"7v6295rxmb.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Improve tests for detached worktree in git-submodule","fromName":"Daniel Graña","fromEmail":"dangra@gmail.com","sentAt":"2012-07-30T17:18:22Z","receivedAt":"2012-07-30T17:18:22Z","isPatch":true,"sender":{"key":"dangra@gmail.com","avatar":"https://avatars.githubusercontent.com/u/37369?v=4"},"body":"On Mon, Jul 30, 2012 at 2:02 PM, Junio C Hamano wrote:\n> Daniel Graña writes:\n>\n>> Signed-off-by: Daniel Graña \n>> ---\n>> t/t7409-submodule-detached-worktree.sh | 31 ++++++++++++++++++++++++-------\n>> 1 files changed, 24 insertions(+), 7 deletions(-)\n>>\n>> diff --git a/t/t7409-submodule-detached-worktree.sh b/t/t7409-submodule-detached-worktree.sh\n>> index db75642..d88f400 100755\n>> --- a/t/t7409-submodule-detached-worktree.sh\n>> +++ b/t/t7409-submodule-detached-worktree.sh\n>> @@ -15,7 +15,11 @@ TEST_NO_CREATE_REPO=1\n>> test_expect_success 'submodule on detached working tree' '\n>> git init --bare remote &&\n>> test_create_repo bundle1 &&\n>> - (cd bundle1 && test_commit \"shoot\") &&\n>> + (\n>> + cd bundle1 &&\n>> + test_commit \"shoot\" &&\n>> + git rev-list --max-count=1 HEAD > \"$TRASH_DIRECTORY/expect\"\n>\n> Better written as\n>\n> git rev-parse --verify HEAD >../expect\n>\n> methinks.\n\nYou rule here,\n\nis it still better than \"git rev-parse --max-count=1 HEAD\" seen in\nt7406-submodule.update.sh?\n\n>\n>> + ) &&\n>> mkdir home &&\n>> (\n>> cd home &&\n>> @@ -23,14 +27,27 @@ test_expect_success 'submodule on detached working tree' '\n>> git clone --bare ../remote .dotfiles &&\n>> git submodule add ../bundle1 .vim/bundle/sogood &&\n>> test_commit \"sogood\" &&\n>> + (\n>> + unset GIT_WORK_TREE GIT_DIR &&\n>> + cd .vim/bundle/sogood &&\n>> + git rev-list --max-count=1 HEAD > actual &&\n>> + test_cmp actual \"$TRASH_DIRECTORY/expect\"\n>\n> Likewise.\n>\n> git rev-parse --verify HEAD >actual &&\n> test_cmp ../expect actual\n\nI tried to avoid the too many \"..\" usage, in that case it'd be:\n\n test_cmp ../../../../expect actual\n\n>\n> As test_cmp turns into \"diff -u\", comparing expect against actual\n> (instead of the other way around) shows the deviation in a more\n> readable way when your test fails.\n\nmust agree, thanks.\n\n\n>\n>> + ) &&\n>> git push origin master\n>> - ) &&\n>> + )\n>> mkdir home2 &&\n>> (\n>> cd home2 &&\n>> - export GIT_WORK_TREE=\"$(pwd)\" GIT_DIR=\"$(pwd)/.dotfiles\" &&\n>> git clone --bare ../remote .dotfiles &&\n>> - git submodule update --init\n>> + export GIT_WORK_TREE=\"$(pwd)\" GIT_DIR=\"$(pwd)/.dotfiles\" &&\n>\n> So you used to clone with the two environment variables in effect to\n> create the .otfiles, but now you don't use them while cloning. That\n> makes more sense to me, especially the .otfiles is created \"bare\".\n>\n>> + git checkout master &&\n>\n> So you populate the newly created home2 working tree out of the .otfiles\n> repository in it.\n\nright, before it wasn't creating ~/.gitmodules and \"git subodule\nupdate --init\" wasn't taking effect.\n\n>\n>> + git submodule update --init &&\n>> + (\n>> + unset GIT_WORK_TREE GIT_DIR &&\n>> + cd .vim/bundle/sogood &&\n>> + git rev-list --max-count=1 HEAD > actual &&\n>> + test_cmp actual \"$TRASH_DIRECTORY/expect\"\n>\n> Likewise.\n>\n>> + )\n>> )\n>> '\n>>\n>> @@ -42,6 +59,7 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '\n>> git clone --bare ../remote \"$GIT_DIR\" &&\n>> git config core.bare false &&\n>> git config core.worktree .. &&\n>> + git checkout master &&\n>> git submodule add ../bundle1 .vim/bundle/dupe &&\n>> test_commit \"dupe\" &&\n>> git push origin master\n>> @@ -52,9 +70,8 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '\n>> git config core.bare false &&\n>> git config core.worktree .. &&\n>> git pull &&\n>> - git submodule update &&\n>> - git submodule status &&\n>> - test -d .vim/bundle/dupe\n>> + git submodule update --init &&\n>> + test -e .vim/bundle/dupe/shoot.t\n>\n> Is the \"existence\" the only thing you care about? That's not all\n> that different from the old test that only checked the existence of\n> the directory dupe, no?\n\nExcept the submodule wasn't updating but the directory still existed\nso test passed, now it check for a file that exists only if the\nsubmodule update works.\nI check for file existence as a shortcut for checking for submodule\nrevision and because I am not sure what else too check honestly.\n\nthe real purpose of this test is the use or \"core.worktree\" instead of\nGIT_WORK_TREE.\n"},{"id":"196172","messageId":"1343670212-2992-1-git-send-email-dangra@gmail.com","threadId":"31139","inReplyTo":"7v6295rxmb.fsf@alter.siamese.dyndns.org","subject":"[PATCH] Improve tests for detached worktree in git-submodule","fromName":"Daniel Graña","fromEmail":"dangra@gmail.com","sentAt":"2012-07-30T17:43:32Z","receivedAt":"2012-07-30T17:43:32Z","isPatch":true,"sender":{"key":"dangra@gmail.com","avatar":"https://avatars.githubusercontent.com/u/37369?v=4"},"body":"* Check submodule is correctly initialized and updated after cloning .dotfiles\n\nSigned-off-by: Daniel Graña \n---\n\nRemove $TRASH_DIRECTORY and \"git rev-parse --verify HEAD\" as suggested by Junio\n\n t/t7409-submodule-detached-worktree.sh | 31 ++++++++++++++++++++++++-------\n 1 files changed, 24 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t7409-submodule-detached-worktree.sh b/t/t7409-submodule-detached-worktree.sh\nindex db75642..1d5a4c5 100755\n--- a/t/t7409-submodule-detached-worktree.sh\n+++ b/t/t7409-submodule-detached-worktree.sh\n@@ -15,7 +15,11 @@ TEST_NO_CREATE_REPO=1\n test_expect_success 'submodule on detached working tree' '\n \tgit init --bare remote &&\n \ttest_create_repo bundle1 &&\n-\t(cd bundle1 && test_commit \"shoot\") &&\n+\t(\n+\t\tcd bundle1 &&\n+\t\ttest_commit \"shoot\" &&\n+\t\tgit rev-parse --verify HEAD > ../expect\n+\t) &&\n \tmkdir home &&\n \t(\n \t\tcd home &&\n@@ -23,14 +27,27 @@ test_expect_success 'submodule on detached working tree' '\n \t\tgit clone --bare ../remote .dotfiles &&\n \t\tgit submodule add ../bundle1 .vim/bundle/sogood &&\n \t\ttest_commit \"sogood\" &&\n+\t\t(\n+\t\t\tunset GIT_WORK_TREE GIT_DIR &&\n+\t\t\tcd .vim/bundle/sogood &&\n+\t\t\tgit rev-parse --verify HEAD > actual &&\n+\t\t\ttest_cmp ../../../../expect actual\n+\t\t) &&\n \t\tgit push origin master\n-\t) &&\n+\t)\n \tmkdir home2 &&\n \t(\n \t\tcd home2 &&\n-\t\texport GIT_WORK_TREE=\"$(pwd)\" GIT_DIR=\"$(pwd)/.dotfiles\" &&\n \t\tgit clone --bare ../remote .dotfiles &&\n-\t\tgit submodule update --init\n+\t\texport GIT_WORK_TREE=\"$(pwd)\" GIT_DIR=\"$(pwd)/.dotfiles\" &&\n+\t\tgit checkout master &&\n+\t\tgit submodule update --init &&\n+\t\t(\n+\t\t\tunset GIT_WORK_TREE GIT_DIR &&\n+\t\t\tcd .vim/bundle/sogood &&\n+\t\t\tgit rev-parse --verify HEAD > actual &&\n+\t\t\ttest_cmp ../../../../expect actual\n+\t\t)\n \t)\n '\n \n@@ -42,6 +59,7 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '\n \t\tgit clone --bare ../remote \"$GIT_DIR\" &&\n \t\tgit config core.bare false &&\n \t\tgit config core.worktree .. &&\n+\t\tgit checkout master &&\n \t\tgit submodule add ../bundle1 .vim/bundle/dupe &&\n \t\ttest_commit \"dupe\" &&\n \t\tgit push origin master\n@@ -52,9 +70,8 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '\n \t\tgit config core.bare false &&\n \t\tgit config core.worktree .. &&\n \t\tgit pull &&\n-\t\tgit submodule update &&\n-\t\tgit submodule status &&\n-\t\ttest -d .vim/bundle/dupe\n+\t\tgit submodule update --init &&\n+\t\ttest -e .vim/bundle/dupe/shoot.t\n \t)\n '\n \n-- \n1.7.5.4\n"},{"id":"196173","messageId":"7vsjc9qh33.fsf@alter.siamese.dyndns.org","threadId":"31139","inReplyTo":"CAHCkQtPmJdFBk7G-EP70FwtkRk11Utp+rppjNrEZFtZgOVrObA@mail.gmail.com","subject":"Re: [PATCH] Improve tests for detached worktree in git-submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-30T17:44:32Z","receivedAt":"2012-07-30T17:44:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Graña writes:\n\n> On Mon, Jul 30, 2012 at 2:02 PM, Junio C Hamano wrote:\n>> Daniel Graña writes:\n>>\n>>> Signed-off-by: Daniel Graña \n>>> ---\n>>> t/t7409-submodule-detached-worktree.sh | 31 ++++++++++++++++++++++++-------\n>>> 1 files changed, 24 insertions(+), 7 deletions(-)\n>>>\n>>> diff --git a/t/t7409-submodule-detached-worktree.sh b/t/t7409-submodule-detached-worktree.sh\n>>> index db75642..d88f400 100755\n>>> --- a/t/t7409-submodule-detached-worktree.sh\n>>> +++ b/t/t7409-submodule-detached-worktree.sh\n>>> @@ -15,7 +15,11 @@ TEST_NO_CREATE_REPO=1\n>>> test_expect_success 'submodule on detached working tree' '\n>>> git init --bare remote &&\n>>> test_create_repo bundle1 &&\n>>> - (cd bundle1 && test_commit \"shoot\") &&\n>>> + (\n>>> + cd bundle1 &&\n>>> + test_commit \"shoot\" &&\n>>> + git rev-list --max-count=1 HEAD > \"$TRASH_DIRECTORY/expect\"\n>>\n>> Better written as\n>>\n>> git rev-parse --verify HEAD >../expect\n>>\n>> methinks.\n>\n> You rule here,\n>\n> is it still better than \"git rev-parse --max-count=1 HEAD\" seen in\n> t7406-submodule.update.sh?\n\n\"git rev-parse --max-count=1 HEAD\" will show \"--max-count=1\\n\"\nfollowed by the value of HEAD, so if the expected result and the\nactual result were both prepared by that command, the comparison\nwould succeed (as the irrelevant --max-count=1 line will appear in\nboth output), but honestly, I do not think it makes any sense.\n\nWho wrote that crap?\n\n>\n>>\n>>> + ) &&\n>>> mkdir home &&\n>>> (\n>>> cd home &&\n>>> @@ -23,14 +27,27 @@ test_expect_success 'submodule on detached working tree' '\n>>> git clone --bare ../remote .dotfiles &&\n>>> git submodule add ../bundle1 .vim/bundle/sogood &&\n>>> test_commit \"sogood\" &&\n>>> + (\n>>> + unset GIT_WORK_TREE GIT_DIR &&\n>>> + cd .vim/bundle/sogood &&\n>>> + git rev-list --max-count=1 HEAD > actual &&\n>>> + test_cmp actual \"$TRASH_DIRECTORY/expect\"\n>>\n>> Likewise.\n>>\n>> git rev-parse --verify HEAD >actual &&\n>> test_cmp ../expect actual\n>\n> I tried to avoid the too many \"..\" usage, in that case it'd be:\n>\n> test_cmp ../../../../expect actual\n\n\"$TRASH_DIRECTORY/expect\" is fine as well. Just drop the extra SP\nbetween the redirection '>' and the filename, and make sure the\nfilename is inside double quotes for some versions of bash that\nissue an unnecessary warning.\n\n>>> + git checkout master &&\n>>\n>> So you populate the newly created home2 working tree out of the .otfiles\n>> repository in it.\n>\n> right, before it wasn't creating ~/.gitmodules and \"git subodule\n> update --init\" wasn't taking effect.\n\nGood.\n\n>> Is the \"existence\" the only thing you care about? That's not all\n>> that different from the old test that only checked the existence of\n>> the directory dupe, no?\n>\n> Except the submodule wasn't updating but the directory still existed\n> so test passed, now it check for a file that exists only if the\n> submodule update works.\n\nOK.\n"},{"id":"196174","messageId":"CAHCkQtPS4+eoV_765DhbiZH_4zy98PKNEZ2f1XGx5rdLatZK7w@mail.gmail.com","threadId":"31139","inReplyTo":"1343670212-2992-1-git-send-email-dangra@gmail.com","subject":"Re: [PATCH] Improve tests for detached worktree in git-submodule","fromName":"Daniel Graña","fromEmail":"dangra@gmail.com","sentAt":"2012-07-30T17:50:02Z","receivedAt":"2012-07-30T17:50:02Z","isPatch":true,"sender":{"key":"dangra@gmail.com","avatar":"https://avatars.githubusercontent.com/u/37369?v=4"},"body":"On Mon, Jul 30, 2012 at 2:43 PM, Daniel Graña wrote:\n> * Check submodule is correctly initialized and updated after cloning .dotfiles\n>\n> Signed-off-by: Daniel Graña \n> ---\n\nIgnore this patch, I am sending another without extra space after\nstandard output redirection.\n\n\n\n>\n> Remove $TRASH_DIRECTORY and \"git rev-parse --verify HEAD\" as suggested by Junio\n>\n> t/t7409-submodule-detached-worktree.sh | 31 ++++++++++++++++++++++++-------\n> 1 files changed, 24 insertions(+), 7 deletions(-)\n>\n> diff --git a/t/t7409-submodule-detached-worktree.sh b/t/t7409-submodule-detached-worktree.sh\n> index db75642..1d5a4c5 100755\n> --- a/t/t7409-submodule-detached-worktree.sh\n> +++ b/t/t7409-submodule-detached-worktree.sh\n> @@ -15,7 +15,11 @@ TEST_NO_CREATE_REPO=1\n> test_expect_success 'submodule on detached working tree' '\n> git init --bare remote &&\n> test_create_repo bundle1 &&\n> - (cd bundle1 && test_commit \"shoot\") &&\n> + (\n> + cd bundle1 &&\n> + test_commit \"shoot\" &&\n> + git rev-parse --verify HEAD > ../expect\n> + ) &&\n> mkdir home &&\n> (\n> cd home &&\n> @@ -23,14 +27,27 @@ test_expect_success 'submodule on detached working tree' '\n> git clone --bare ../remote .dotfiles &&\n> git submodule add ../bundle1 .vim/bundle/sogood &&\n> test_commit \"sogood\" &&\n> + (\n> + unset GIT_WORK_TREE GIT_DIR &&\n> + cd .vim/bundle/sogood &&\n> + git rev-parse --verify HEAD > actual &&\n> + test_cmp ../../../../expect actual\n> + ) &&\n> git push origin master\n> - ) &&\n> + )\n> mkdir home2 &&\n> (\n> cd home2 &&\n> - export GIT_WORK_TREE=\"$(pwd)\" GIT_DIR=\"$(pwd)/.dotfiles\" &&\n> git clone --bare ../remote .dotfiles &&\n> - git submodule update --init\n> + export GIT_WORK_TREE=\"$(pwd)\" GIT_DIR=\"$(pwd)/.dotfiles\" &&\n> + git checkout master &&\n> + git submodule update --init &&\n> + (\n> + unset GIT_WORK_TREE GIT_DIR &&\n> + cd .vim/bundle/sogood &&\n> + git rev-parse --verify HEAD > actual &&\n> + test_cmp ../../../../expect actual\n> + )\n> )\n> '\n>\n> @@ -42,6 +59,7 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '\n> git clone --bare ../remote \"$GIT_DIR\" &&\n> git config core.bare false &&\n> git config core.worktree .. &&\n> + git checkout master &&\n> git submodule add ../bundle1 .vim/bundle/dupe &&\n> test_commit \"dupe\" &&\n> git push origin master\n> @@ -52,9 +70,8 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '\n> git config core.bare false &&\n> git config core.worktree .. &&\n> git pull &&\n> - git submodule update &&\n> - git submodule status &&\n> - test -d .vim/bundle/dupe\n> + git submodule update --init &&\n> + test -e .vim/bundle/dupe/shoot.t\n> )\n> '\n>\n> --\n> 1.7.5.4\n>\n"},{"id":"196175","messageId":"7vobmxqgqv.fsf@alter.siamese.dyndns.org","threadId":"31139","inReplyTo":"7vsjc9qh33.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Improve tests for detached worktree in git-submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-30T17:51:52Z","receivedAt":"2012-07-30T17:51:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Daniel Graña writes:\n> ...\n>> is it still better than \"git rev-parse --max-count=1 HEAD\" seen in\n>> t7406-submodule.update.sh?\n>\n> \"git rev-parse --max-count=1 HEAD\" will show \"--max-count=1\\n\"\n> followed by the value of HEAD, so if the expected result and the\n> actual result were both prepared by that command, the comparison\n> would succeed (as the irrelevant --max-count=1 line will appear in\n> both output), but honestly, I do not think it makes any sense.\n\nThanks for noticing; I'll queue this.\n\n-- >8 --\nSubject: [PATCH] t7406: fix misleading \"rev-parse --max-count=1 HEAD\"\n\nThe test happened to use \"rev-parse --max-count=1 HEAD\" consistently\nto prepare the expected output and the actual output, so the\ncomparison between them gave us a correct success/failure because\nboth output had irrelevant \"--max-count=1\" in it.\n\nBut that is not an excuse to keep it broken. Replace it a more\nmeaningful construct \"rev-parse --verify HEAD\".\n\nNoticed by Daniel Graña while working on his submodule tests.\n\nSigned-off-by: Junio C Hamano \n---\n t/t7406-submodule-update.sh | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex 807f761..f4e6602 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -305,7 +305,7 @@ test_expect_success 'submodule update continues after checkout error' '\n \t git submodule init &&\n \t git commit -am \"new_submodule\" &&\n \t (cd submodule2 &&\n-\t git rev-parse --max-count=1 HEAD > ../expect\n+\t git rev-parse --verify HEAD >../expect\n \t ) &&\n \t (cd submodule &&\n \t test_commit \"update_submodule\" file\n@@ -322,7 +322,7 @@ test_expect_success 'submodule update continues after checkout error' '\n \t git checkout HEAD^ &&\n \t test_must_fail git submodule update &&\n \t (cd submodule2 &&\n-\t git rev-parse --max-count=1 HEAD > ../actual\n+\t git rev-parse --verify HEAD >../actual\n \t ) &&\n \t test_cmp expect actual\n \t)\n@@ -351,7 +351,7 @@ test_expect_success 'submodule update continues after recursive checkout error'\n \t test_commit \"update_submodule_again_again\" file\n \t ) &&\n \t (cd submodule2 &&\n-\t git rev-parse --max-count=1 HEAD > ../expect &&\n+\t git rev-parse --verify HEAD >../expect &&\n \t test_commit \"update_submodule2_again\" file\n \t ) &&\n \t git add submodule &&\n@@ -366,7 +366,7 @@ test_expect_success 'submodule update continues after recursive checkout error'\n \t ) &&\n \t test_must_fail git submodule update --recursive &&\n \t (cd submodule2 &&\n-\t git rev-parse --max-count=1 HEAD > ../actual\n+\t git rev-parse --verify HEAD >../actual\n \t ) &&\n \t test_cmp expect actual\n \t)\n@@ -398,12 +398,12 @@ test_expect_success 'submodule update exit immediately in case of merge conflict\n \t ) &&\n \t git checkout HEAD^ &&\n \t (cd submodule2 &&\n-\t git rev-parse --max-count=1 HEAD > ../expect\n+\t git rev-parse --verify HEAD >../expect\n \t ) &&\n \t git config submodule.submodule.update merge &&\n \t test_must_fail git submodule update &&\n \t (cd submodule2 &&\n-\t git rev-parse --max-count=1 HEAD > ../actual\n+\t git rev-parse --verify HEAD >../actual\n \t ) &&\n \t test_cmp expect actual\n \t)\n@@ -432,12 +432,12 @@ test_expect_success 'submodule update exit immediately after recursive rebase er\n \t ) &&\n \t git checkout HEAD^ &&\n \t (cd submodule2 &&\n-\t git rev-parse --max-count=1 HEAD > ../expect\n+\t git rev-parse --verify HEAD >../expect\n \t ) &&\n \t git config submodule.submodule.update rebase &&\n \t test_must_fail git submodule update &&\n \t (cd submodule2 &&\n-\t git rev-parse --max-count=1 HEAD > ../actual\n+\t git rev-parse --verify HEAD >../actual\n \t ) &&\n \t test_cmp expect actual\n \t)\n-- \n1.7.12.rc0.80.gb93a609\n"},{"id":"196176","messageId":"1343670719-3635-1-git-send-email-dangra@gmail.com","threadId":"31139","inReplyTo":"7vsjc9qh33.fsf@alter.siamese.dyndns.org","subject":"[PATCH] Improve tests for detached worktree in git-submodule","fromName":"Daniel Graña","fromEmail":"dangra@gmail.com","sentAt":"2012-07-30T17:51:59Z","receivedAt":"2012-07-30T17:51:59Z","isPatch":true,"sender":{"key":"dangra@gmail.com","avatar":"https://avatars.githubusercontent.com/u/37369?v=4"},"body":"* Check submodule is correctly initialized and updated after cloning .dotfiles\n\nSigned-off-by: Daniel Graña \n---\n t/t7409-submodule-detached-worktree.sh | 31 ++++++++++++++++++++++++-------\n 1 files changed, 24 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t7409-submodule-detached-worktree.sh b/t/t7409-submodule-detached-worktree.sh\nindex db75642..f82a757 100755\n--- a/t/t7409-submodule-detached-worktree.sh\n+++ b/t/t7409-submodule-detached-worktree.sh\n@@ -15,7 +15,11 @@ TEST_NO_CREATE_REPO=1\n test_expect_success 'submodule on detached working tree' '\n \tgit init --bare remote &&\n \ttest_create_repo bundle1 &&\n-\t(cd bundle1 && test_commit \"shoot\") &&\n+\t(\n+\t\tcd bundle1 &&\n+\t\ttest_commit \"shoot\" &&\n+\t\tgit rev-parse --verify HEAD >../expect\n+\t) &&\n \tmkdir home &&\n \t(\n \t\tcd home &&\n@@ -23,14 +27,27 @@ test_expect_success 'submodule on detached working tree' '\n \t\tgit clone --bare ../remote .dotfiles &&\n \t\tgit submodule add ../bundle1 .vim/bundle/sogood &&\n \t\ttest_commit \"sogood\" &&\n+\t\t(\n+\t\t\tunset GIT_WORK_TREE GIT_DIR &&\n+\t\t\tcd .vim/bundle/sogood &&\n+\t\t\tgit rev-parse --verify HEAD >actual &&\n+\t\t\ttest_cmp ../../../../expect actual\n+\t\t) &&\n \t\tgit push origin master\n-\t) &&\n+\t)\n \tmkdir home2 &&\n \t(\n \t\tcd home2 &&\n-\t\texport GIT_WORK_TREE=\"$(pwd)\" GIT_DIR=\"$(pwd)/.dotfiles\" &&\n \t\tgit clone --bare ../remote .dotfiles &&\n-\t\tgit submodule update --init\n+\t\texport GIT_WORK_TREE=\"$(pwd)\" GIT_DIR=\"$(pwd)/.dotfiles\" &&\n+\t\tgit checkout master &&\n+\t\tgit submodule update --init &&\n+\t\t(\n+\t\t\tunset GIT_WORK_TREE GIT_DIR &&\n+\t\t\tcd .vim/bundle/sogood &&\n+\t\t\tgit rev-parse --verify HEAD >actual &&\n+\t\t\ttest_cmp ../../../../expect actual\n+\t\t)\n \t)\n '\n \n@@ -42,6 +59,7 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '\n \t\tgit clone --bare ../remote \"$GIT_DIR\" &&\n \t\tgit config core.bare false &&\n \t\tgit config core.worktree .. &&\n+\t\tgit checkout master &&\n \t\tgit submodule add ../bundle1 .vim/bundle/dupe &&\n \t\ttest_commit \"dupe\" &&\n \t\tgit push origin master\n@@ -52,9 +70,8 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '\n \t\tgit config core.bare false &&\n \t\tgit config core.worktree .. &&\n \t\tgit pull &&\n-\t\tgit submodule update &&\n-\t\tgit submodule status &&\n-\t\ttest -d .vim/bundle/dupe\n+\t\tgit submodule update --init &&\n+\t\ttest -e .vim/bundle/dupe/shoot.t\n \t)\n '\n \n-- \n1.7.5.4\n"},{"id":"196179","messageId":"7vfw89qfx2.fsf@alter.siamese.dyndns.org","threadId":"31139","inReplyTo":"1343670719-3635-1-git-send-email-dangra@gmail.com","subject":"Re: [PATCH] Improve tests for detached worktree in git-submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-30T18:09:45Z","receivedAt":"2012-07-30T18:09:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Graña writes:\n\n> * Check submodule is correctly initialized and updated after cloning .dotfiles\n>\n> Signed-off-by: Daniel Graña \n> ---\n\nThanks.\n\n> @@ -23,14 +27,27 @@ test_expect_success 'submodule on detached working tree' '\n> \t\tgit clone --bare ../remote .dotfiles &&\n> \t\tgit submodule add ../bundle1 .vim/bundle/sogood &&\n> \t\ttest_commit \"sogood\" &&\n> +\t\t(\n> +\t\t\tunset GIT_WORK_TREE GIT_DIR &&\n> +\t\t\tcd .vim/bundle/sogood &&\n> +\t\t\tgit rev-parse --verify HEAD >actual &&\n> +\t\t\ttest_cmp ../../../../expect actual\n> +\t\t) &&\n> \t\tgit push origin master\n> -\t) &&\n> +\t)\n\nI do not think you meant to break the && chain here on purpose.\nI'll queue with a minor fix-up here.\n\n> \tmkdir home2 &&\n> \t(\n> \t\tcd home2 &&\n> -\t\texport GIT_WORK_TREE=\"$(pwd)\" GIT_DIR=\"$(pwd)/.dotfiles\" &&\n> \t\tgit clone --bare ../remote .dotfiles &&\n> -\t\tgit submodule update --init\n> +\t\texport GIT_WORK_TREE=\"$(pwd)\" GIT_DIR=\"$(pwd)/.dotfiles\" &&\n> +\t\tgit checkout master &&\n> +\t\tgit submodule update --init &&\n> +\t\t(\n> +\t\t\tunset GIT_WORK_TREE GIT_DIR &&\n> +\t\t\tcd .vim/bundle/sogood &&\n> +\t\t\tgit rev-parse --verify HEAD >actual &&\n> +\t\t\ttest_cmp ../../../../expect actual\n> +\t\t)\n> \t)\n> '\n> \n> @@ -42,6 +59,7 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '\n> \t\tgit clone --bare ../remote \"$GIT_DIR\" &&\n> \t\tgit config core.bare false &&\n> \t\tgit config core.worktree .. &&\n> +\t\tgit checkout master &&\n> \t\tgit submodule add ../bundle1 .vim/bundle/dupe &&\n> \t\ttest_commit \"dupe\" &&\n> \t\tgit push origin master\n> @@ -52,9 +70,8 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '\n> \t\tgit config core.bare false &&\n> \t\tgit config core.worktree .. &&\n> \t\tgit pull &&\n> -\t\tgit submodule update &&\n> -\t\tgit submodule status &&\n> -\t\ttest -d .vim/bundle/dupe\n> +\t\tgit submodule update --init &&\n> +\t\ttest -e .vim/bundle/dupe/shoot.t\n> \t)\n> '\n"},{"id":"196180","messageId":"CAHCkQtNUKHkK8DYT-cBD3pbaqXPFJ-cNBsKqkAHdEUs9rxVa7A@mail.gmail.com","threadId":"31139","inReplyTo":"7vfw89qfx2.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Improve tests for detached worktree in git-submodule","fromName":"Daniel Graña","fromEmail":"dangra@gmail.com","sentAt":"2012-07-30T18:15:45Z","receivedAt":"2012-07-30T18:15:45Z","isPatch":true,"sender":{"key":"dangra@gmail.com","avatar":"https://avatars.githubusercontent.com/u/37369?v=4"},"body":"On Mon, Jul 30, 2012 at 3:09 PM, Junio C Hamano wrote:\n> Daniel Graña writes:\n>\n>> * Check submodule is correctly initialized and updated after cloning .dotfiles\n>>\n>> Signed-off-by: Daniel Graña \n>> ---\n>\n> Thanks.\n>\n>> @@ -23,14 +27,27 @@ test_expect_success 'submodule on detached working tree' '\n>> git clone --bare ../remote .dotfiles &&\n>> git submodule add ../bundle1 .vim/bundle/sogood &&\n>> test_commit \"sogood\" &&\n>> + (\n>> + unset GIT_WORK_TREE GIT_DIR &&\n>> + cd .vim/bundle/sogood &&\n>> + git rev-parse --verify HEAD >actual &&\n>> + test_cmp ../../../../expect actual\n>> + ) &&\n>> git push origin master\n>> - ) &&\n>> + )\n>\n> I do not think you meant to break the && chain here on purpose.\n> I'll queue with a minor fix-up here.\n\ngreat, thanks.\n\n>\n>> mkdir home2 &&\n>> (\n>> cd home2 &&\n>> - export GIT_WORK_TREE=\"$(pwd)\" GIT_DIR=\"$(pwd)/.dotfiles\" &&\n>> git clone --bare ../remote .dotfiles &&\n>> - git submodule update --init\n>> + export GIT_WORK_TREE=\"$(pwd)\" GIT_DIR=\"$(pwd)/.dotfiles\" &&\n>> + git checkout master &&\n>> + git submodule update --init &&\n>> + (\n>> + unset GIT_WORK_TREE GIT_DIR &&\n>> + cd .vim/bundle/sogood &&\n>> + git rev-parse --verify HEAD >actual &&\n>> + test_cmp ../../../../expect actual\n>> + )\n>> )\n>> '\n>>\n>> @@ -42,6 +59,7 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '\n>> git clone --bare ../remote \"$GIT_DIR\" &&\n>> git config core.bare false &&\n>> git config core.worktree .. &&\n>> + git checkout master &&\n>> git submodule add ../bundle1 .vim/bundle/dupe &&\n>> test_commit \"dupe\" &&\n>> git push origin master\n>> @@ -52,9 +70,8 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '\n>> git config core.bare false &&\n>> git config core.worktree .. &&\n>> git pull &&\n>> - git submodule update &&\n>> - git submodule status &&\n>> - test -d .vim/bundle/dupe\n>> + git submodule update --init &&\n>> + test -e .vim/bundle/dupe/shoot.t\n>> )\n>> '\n"}]}