{"thread":{"id":"41048","subject":"[PATCH v3 2/3] submodule: fix &&-chain breakage","startedAt":"2015-12-20T23:19:52Z","lastAt":"2016-03-20T21:27:36Z","messageCount":14,"participants":["larsxschneider@gmail.com","Lars Schneider","Stefan Beller","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":3,"patchTotal":3},"messages":[{"id":"274789","messageId":"1450653595-22676-1-git-send-email-larsxschneider@gmail.com","threadId":"41048","inReplyTo":null,"subject":"[PATCH v3 0/3] add test to demonstrate that shallow recursive clones fail","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-12-20T23:19:52Z","receivedAt":"2015-12-20T23:19:52Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\n\ndiff to v2:\n* remove workaround tests as suggested by Peff [1]\n* fix chain breakage introduced in 275cd18\n* add hints to the user if a submodule checkout fails while using the\n  depth argument [2]\n\nThanks,\nLars\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/281237\n[2] http://article.gmane.org/gmane.comp.version-control.git/281420\n\nLars Schneider (3):\n  submodule: add test to demonstrate that shallow recursive clones fail\n  submodule: fix &&-chain breakage\n  submodule: extend die message on failed checkout with depth argument\n\n git-submodule.sh               |  4 ++++\n t/t7406-submodule-update.sh    | 35 +++++++++++++++++++++++++---\n t/t7412-submodule-recursive.sh | 52 ++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 88 insertions(+), 3 deletions(-)\n create mode 100755 t/t7412-submodule-recursive.sh\n\n--\n2.5.1\n"},{"id":"274788","messageId":"1450653595-22676-2-git-send-email-larsxschneider@gmail.com","threadId":"41048","inReplyTo":"1450653595-22676-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v3 1/3] submodule: add test to demonstrate that shallow recursive clones fail","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-12-20T23:19:53Z","receivedAt":"2015-12-20T23:19:53Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\n\n\"git clone --recursive --depth 1 --single-branch <url>\" clones the\nsubmodules successfully. However, it does not obey \"--depth 1\" for\nsubmodule cloning.\n\nSigned-off-by: Lars Schneider <larsxschneider@gmail.com>\n---\n t/t7412-submodule-recursive.sh | 52 ++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 52 insertions(+)\n create mode 100755 t/t7412-submodule-recursive.sh\n\ndiff --git a/t/t7412-submodule-recursive.sh b/t/t7412-submodule-recursive.sh\nnew file mode 100755\nindex 0000000..58b3eb9\n--- /dev/null\n+++ b/t/t7412-submodule-recursive.sh\n@@ -0,0 +1,52 @@\n+#!/bin/sh\n+\n+test_description='Test shallow cloning of repos with submodules'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tgit checkout -b master &&\n+\techo file >file &&\n+\tgit add file &&\n+\ttest_tick &&\n+\tgit commit -m \"master commit 1\" &&\n+\n+\tgit checkout -b branch &&\n+\techo file >branch-file &&\n+\tgit add branch-file &&\n+\ttest_tick &&\n+\tgit commit -m \"branch commit 1\" &&\n+\n+\tgit checkout master &&\n+\tgit clone . repo &&\n+\t(\n+\t\tcd repo &&\n+\t\tgit checkout master &&\n+\t\tgit submodule add ../. submodule &&\n+\t\t(\n+\t\t\tcd submodule &&\n+\t\t\tgit checkout branch\n+\t\t) &&\n+\t\tgit add submodule &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"master commit 2\"\n+\t)\n+'\n+\n+test_expect_failure shallow-clone-recursive '\n+\tURL=\"file://$(pwd | sed \"s/[[:space:]]/%20/g\")/repo\" &&\n+\techo $URL &&\n+\tgit clone --recursive --depth 1 --single-branch $URL clone-recursive &&\n+\t(\n+\t\tcd \"clone-recursive\" &&\n+\t\tgit log --oneline >lines &&\n+\t\ttest_line_count = 1 lines\n+\t) &&\n+\t(\n+\t\tcd \"clone-recursive/submodule\" &&\n+\t\tgit log --oneline >lines &&\n+\t\ttest_line_count = 1 lines\n+\t)\n+'\n+\n+test_done\n-- \n2.5.1\n"},{"id":"274786","messageId":"1450653595-22676-3-git-send-email-larsxschneider@gmail.com","threadId":"41048","inReplyTo":"1450653595-22676-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v3 2/3] submodule: fix &&-chain breakage","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-12-20T23:19:54Z","receivedAt":"2015-12-20T23:19:54Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\n\n---\n t/t7406-submodule-update.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex dda3929..b5bd976 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -758,7 +758,7 @@ test_expect_success 'submodule update clone shallow submodule' '\n \t(cd super3 &&\n \t sed -e \"s#url = ../#url = file://$pwd/#\" <.gitmodules >.gitmodules.tmp &&\n \t mv -f .gitmodules.tmp .gitmodules &&\n-\t git submodule update --init --depth=3\n+\t git submodule update --init --depth=3 &&\n \t (cd submodule &&\n \t  test 1 = $(git log --oneline | wc -l)\n \t )\n-- \n2.5.1\n"},{"id":"274787","messageId":"1450653595-22676-4-git-send-email-larsxschneider@gmail.com","threadId":"41048","inReplyTo":"1450653595-22676-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v3 3/3] submodule: extend die message on failed checkout with depth argument","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-12-20T23:19:55Z","receivedAt":"2015-12-20T23:19:55Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\n\nA submodule checkout might fail due missing hashes caused by a shallow\nfetch operation triggered with the \"--depth\" argument. Make the user\naware of this with an extended die message.\n\nSigned-off-by: Lars Schneider <larsxschneider@gmail.com>\n---\n git-submodule.sh            |  4 ++++\n t/t7406-submodule-update.sh | 33 +++++++++++++++++++++++++++++++--\n 2 files changed, 35 insertions(+), 2 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 9bc5c5f..b555d87 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -764,6 +764,10 @@ Maybe you want to use 'update --init'?\")\"\n \t\t\t\tcommand=\"git checkout $subforce -q\"\n \t\t\t\tdie_msg=\"$(eval_gettext \"Unable to checkout '\\$sha1' in submodule path '\\$displaypath'\")\"\n \t\t\t\tsay_msg=\"$(eval_gettext \"Submodule path '\\$displaypath': checked out '\\$sha1'\")\"\n+\t\t\t\tif ! test -z \"$depth\"\n+\t\t\t\tthen\n+\t\t\t\t\tdie_msg=\"$die_msg $(eval_gettext \"Commit is probably not on the default branch. Try to remove the '\\$depth' argument!\")\"\n+\t\t\t\tfi\n \t\t\t\t;;\n \t\t\trebase)\n \t\t\t\tcommand=\"git rebase\"\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex b5bd976..52b7120 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -31,6 +31,14 @@ test_expect_success 'setup a submodule tree' '\n \tgit clone super rebasing &&\n \tgit clone super merging &&\n \tgit clone super none &&\n+\tgit clone super non-default-branch &&\n+\t(cd non-default-branch &&\n+\t git checkout -b non-default &&\n+\t echo \"non-default\" >file &&\n+\t git add file &&\n+\t git commit -m \"Commit on non-default branch\" &&\n+\t git checkout master\n+\t) &&\n \t(cd super &&\n \t git submodule add ../submodule submodule &&\n \t test_tick &&\n@@ -63,6 +71,15 @@ test_expect_success 'setup a submodule tree' '\n \t git submodule add ../none none &&\n \t test_tick &&\n \t git commit -m \"none\"\n+\t) &&\n+\t(cd super &&\n+\t git submodule add ../non-default-branch non-default-branch &&\n+\t (cd non-default-branch &&\n+\t  git checkout non-default\n+\t ) &&\n+     git add non-default-branch &&\n+\t test_tick &&\n+\t git commit -m \"non-default-branch\"\n \t)\n '\n \n@@ -752,17 +769,29 @@ test_expect_success SYMLINKS 'submodule update can handle symbolic links in pwd'\n \t)\n '\n \n-test_expect_success 'submodule update clone shallow submodule' '\n+test_expect_success 'submodule update clone shallow submodule on default branch' '\n \tgit clone cloned super3 &&\n \tpwd=$(pwd) &&\n \t(cd super3 &&\n \t sed -e \"s#url = ../#url = file://$pwd/#\" <.gitmodules >.gitmodules.tmp &&\n \t mv -f .gitmodules.tmp .gitmodules &&\n+\t git config submodule.non-default-branch.update none &&\n \t git submodule update --init --depth=3 &&\n \t (cd submodule &&\n \t  test 1 = $(git log --oneline | wc -l)\n \t )\n-)\n+\t)\n+'\n+\n+test_expect_success 'submodule update clone shallow submodule on non-default branch' '\n+\tgit clone cloned super4 &&\n+\tpwd=$(pwd) &&\n+\t(cd super4 &&\n+\t sed -e \"s#url = ../#url = file://$pwd/#\" <.gitmodules >.gitmodules.tmp &&\n+\t mv -f .gitmodules.tmp .gitmodules &&\n+\t test_must_fail git submodule update --init --depth=3 2>submodule.out &&\n+\t test_i18ngrep --count \"Commit is probably not on the default branch.\" submodule.out\n+    )\n '\n \n test_expect_success 'submodule update --recursive drops module name before recursing' '\n-- \n2.5.1\n"},{"id":"275505","messageId":"79770C7D-B6A1-4239-A1EF-0A986CCD24AA@gmail.com","threadId":"41048","inReplyTo":"1450653595-22676-1-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v3 0/3] add test to demonstrate that shallow recursive clones fail","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-01-07T21:50:28Z","receivedAt":"2016-01-07T21:50:28Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"Hi,\n\ndoes anyone have a few free cycles to take a look at this patch series?\nI wonder if you deem it as not interesting or if it got lost.\n\nThank you,\nLars\n\n\nOn 21 Dec 2015, at 00:19, larsxschneider@gmail.com wrote:\n\n> From: Lars Schneider <larsxschneider@gmail.com>\n> \n> diff to v2:\n> * remove workaround tests as suggested by Peff [1]\n> * fix chain breakage introduced in 275cd18\n> * add hints to the user if a submodule checkout fails while using the\n>  depth argument [2]\n> \n> Thanks,\n> Lars\n> \n> [1] http://article.gmane.org/gmane.comp.version-control.git/281237\n> [2] http://article.gmane.org/gmane.comp.version-control.git/281420\n> \n> Lars Schneider (3):\n>  submodule: add test to demonstrate that shallow recursive clones fail\n>  submodule: fix &&-chain breakage\n>  submodule: extend die message on failed checkout with depth argument\n> \n> git-submodule.sh               |  4 ++++\n> t/t7406-submodule-update.sh    | 35 +++++++++++++++++++++++++---\n> t/t7412-submodule-recursive.sh | 52 ++++++++++++++++++++++++++++++++++++++++++\n> 3 files changed, 88 insertions(+), 3 deletions(-)\n> create mode 100755 t/t7412-submodule-recursive.sh\n> \n> --\n> 2.5.1\n> \n"},{"id":"275655","messageId":"CAGZ79ka9+KxqbogbN1UnNxQ_G6U_incYKLiuadTroc90VmEWpA@mail.gmail.com","threadId":"41048","inReplyTo":"1450653595-22676-4-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v3 3/3] submodule: extend die message on failed checkout with depth argument","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-11T16:26:18Z","receivedAt":"2016-01-11T16:26:18Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sun, Dec 20, 2015 at 3:19 PM,  <larsxschneider@gmail.com> wrote:\n\n> +test_expect_success 'submodule update clone shallow submodule on non-default branch' '\n> +       git clone cloned super4 &&\n> +       pwd=$(pwd) &&\n> +       (cd super4 &&\n> +        sed -e \"s#url = ../#url = file://$pwd/#\" <.gitmodules >.gitmodules.tmp &&\n> +        mv -f .gitmodules.tmp .gitmodules &&\n> +        test_must_fail git submodule update --init --depth=3 2>submodule.out &&\n\nWhy do we choose a depth of 3 here?\n\n> +        test_i18ngrep --count \"Commit is probably not on the default branch.\" submodule.out\n> +    )\n>  '\n>\n>  test_expect_success 'submodule update --recursive drops module name before recursing' '\n> --\n> 2.5.1\n>\n"},{"id":"275656","messageId":"CAGZ79kZz+U3FvYwsJ+v6BQP=z-Z-Y4-=6KxeiqzjTKxQXiFtDg@mail.gmail.com","threadId":"41048","inReplyTo":"79770C7D-B6A1-4239-A1EF-0A986CCD24AA@gmail.com","subject":"Re: [PATCH v3 0/3] add test to demonstrate that shallow recursive clones fail","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-11T16:29:18Z","receivedAt":"2016-01-11T16:29:18Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Jan 7, 2016 at 1:50 PM, Lars Schneider <larsxschneider@gmail.com> wrote:\n> Hi,\n>\n> does anyone have a few free cycles to take a look at this patch series?\n> I wonder if you deem it as not interesting or if it got lost.\n\nPatch 1&2 look good to me.\n\n>\n> Thank you,\n> Lars\n>\n>\n> On 21 Dec 2015, at 00:19, larsxschneider@gmail.com wrote:\n>\n>> From: Lars Schneider <larsxschneider@gmail.com>\n>>\n>> diff to v2:\n>> * remove workaround tests as suggested by Peff [1]\n>> * fix chain breakage introduced in 275cd18\n>> * add hints to the user if a submodule checkout fails while using the\n>>  depth argument [2]\n>>\n>> Thanks,\n>> Lars\n>>\n>> [1] http://article.gmane.org/gmane.comp.version-control.git/281237\n>> [2] http://article.gmane.org/gmane.comp.version-control.git/281420\n>>\n>> Lars Schneider (3):\n>>  submodule: add test to demonstrate that shallow recursive clones fail\n>>  submodule: fix &&-chain breakage\n>>  submodule: extend die message on failed checkout with depth argument\n>>\n>> git-submodule.sh               |  4 ++++\n>> t/t7406-submodule-update.sh    | 35 +++++++++++++++++++++++++---\n>> t/t7412-submodule-recursive.sh | 52 ++++++++++++++++++++++++++++++++++++++++++\n>> 3 files changed, 88 insertions(+), 3 deletions(-)\n>> create mode 100755 t/t7412-submodule-recursive.sh\n>>\n>> --\n>> 2.5.1\n>>\n>\n"},{"id":"275769","messageId":"BCA2C81C-8DB4-4695-8A44-79B51103A684@gmail.com","threadId":"41048","inReplyTo":"CAGZ79ka9+KxqbogbN1UnNxQ_G6U_incYKLiuadTroc90VmEWpA@mail.gmail.com","subject":"Re: [PATCH v3 3/3] submodule: extend die message on failed checkout with depth argument","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-01-12T09:29:30Z","receivedAt":"2016-01-12T09:29:30Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 11 Jan 2016, at 17:26, Stefan Beller <sbeller@google.com> wrote:\n\n> On Sun, Dec 20, 2015 at 3:19 PM,  <larsxschneider@gmail.com> wrote:\n> \n>> +test_expect_success 'submodule update clone shallow submodule on non-default branch' '\n>> +       git clone cloned super4 &&\n>> +       pwd=$(pwd) &&\n>> +       (cd super4 &&\n>> +        sed -e \"s#url = ../#url = file://$pwd/#\" <.gitmodules >.gitmodules.tmp &&\n>> +        mv -f .gitmodules.tmp .gitmodules &&\n>> +        test_must_fail git submodule update --init --depth=3 2>submodule.out &&\n> \n> Why do we choose a depth of 3 here?\nI just copied this code from the test above. Let's change that to \"--depth=1\" to avoid confusion.\nShould I reroll or can this be fixed in the merge?\n\n> \n>> +        test_i18ngrep --count \"Commit is probably not on the default branch.\" submodule.out\nI was just reading this and wonder if we should remove \"--count\". My initial intention was to\nmake sure this error message only happens once. However, this is not checked here anyways.\n\n>> +    )\n>> '\n>> \n>> test_expect_success 'submodule update --recursive drops module name before recursing' '\n\n\nThanks,\nLars\n"},{"id":"275841","messageId":"CAGZ79kY-tzhKxYeSe_cknBP0f8hPZJ9e1+AXq88XjJevc_Dteg@mail.gmail.com","threadId":"41048","inReplyTo":"BCA2C81C-8DB4-4695-8A44-79B51103A684@gmail.com","subject":"Re: [PATCH v3 3/3] submodule: extend die message on failed checkout with depth argument","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-12T23:32:15Z","receivedAt":"2016-01-12T23:32:15Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Jan 12, 2016 at 1:29 AM, Lars Schneider\n<larsxschneider@gmail.com> wrote:\n>\n> On 11 Jan 2016, at 17:26, Stefan Beller <sbeller@google.com> wrote:\n>\n>> On Sun, Dec 20, 2015 at 3:19 PM,  <larsxschneider@gmail.com> wrote:\n>>\n>>> +test_expect_success 'submodule update clone shallow submodule on non-default branch' '\n>>> +       git clone cloned super4 &&\n>>> +       pwd=$(pwd) &&\n>>> +       (cd super4 &&\n>>> +        sed -e \"s#url = ../#url = file://$pwd/#\" <.gitmodules >.gitmodules.tmp &&\n>>> +        mv -f .gitmodules.tmp .gitmodules &&\n>>> +        test_must_fail git submodule update --init --depth=3 2>submodule.out &&\n>>\n>> Why do we choose a depth of 3 here?\n> I just copied this code from the test above. Let's change that to \"--depth=1\" to avoid confusion.\n> Should I reroll or can this be fixed in the merge?\n\nI noticed that too after reviewing 1&2 that 3 seems to be the standard\ndepth in the tests,\nso I'd think 3 is fine here, too. Which makes it not worth to reroll,\nso I'd be fine with it as is.\n\n>\n>>\n>>> +        test_i18ngrep --count \"Commit is probably not on the default branch.\" submodule.out\n> I was just reading this and wonder if we should remove \"--count\". My initial intention was to\n> make sure this error message only happens once. However, this is not checked here anyways.\n>\n>>> +    )\n>>> '\n>>>\n>>> test_expect_success 'submodule update --recursive drops module name before recursing' '\n>\n>\n> Thanks,\n> Lars\n"},{"id":"280830","messageId":"CAGZ79kYzbmzK1UwvWheugkqG0FwhpGMU0_K_jyCKOnoKb5oTxQ@mail.gmail.com","threadId":"41048","inReplyTo":"1450653595-22676-2-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v3 1/3] submodule: add test to demonstrate that shallow recursive clones fail","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-15T19:50:35Z","receivedAt":"2016-03-15T19:50:35Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sun, Dec 20, 2015 at 3:19 PM,  <larsxschneider@gmail.com> wrote:\n> From: Lars Schneider <larsxschneider@gmail.com>\n>\n> \"git clone --recursive --depth 1 --single-branch <url>\" clones the\n> submodules successfully. However, it does not obey \"--depth 1\" for\n> submodule cloning.\n\nI am about to resend \"[RFC/PATCH] clone: add `--shallow-submodules` flag\"\nwhich would need tests and I thought about this series as tests.\n\nI assume patch 2 (fixing a broken && chain in tests) made it through,\nbut patch 1 and 3 did not? I may pick up ideas from here as it will be\nslightly different tests I'd guess.\n\n\n> +test_expect_failure shallow-clone-recursive '\n> +       URL=\"file://$(pwd | sed \"s/[[:space:]]/%20/g\")/repo\" &&\n\nThis would break if the test suite is in a path containing any other white space\nthan U+0020 such as a tab? (Not that I am encouraging using such paths...)\n\nThanks,\nStefan\n\n\n> +       echo $URL &&\n> +       git clone --recursive --depth 1 --single-branch $URL clone-recursive &&\n> +       (\n> +               cd \"clone-recursive\" &&\n> +               git log --oneline >lines &&\n> +               test_line_count = 1 lines\n> +       ) &&\n> +       (\n> +               cd \"clone-recursive/submodule\" &&\n> +               git log --oneline >lines &&\n> +               test_line_count = 1 lines\n> +       )\n> +'\n> +\n> +test_done\n> --\n> 2.5.1\n>\n"},{"id":"280832","messageId":"xmqqpouv7b65.fsf@gitster.mtv.corp.google.com","threadId":"41048","inReplyTo":"CAGZ79kYzbmzK1UwvWheugkqG0FwhpGMU0_K_jyCKOnoKb5oTxQ@mail.gmail.com","subject":"Re: [PATCH v3 1/3] submodule: add test to demonstrate that shallow recursive clones fail","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-15T20:12:34Z","receivedAt":"2016-03-15T20:12:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n>> +test_expect_failure shallow-clone-recursive '\n>> +       URL=\"file://$(pwd | sed \"s/[[:space:]]/%20/g\")/repo\" &&\n>\n> This would break if the test suite is in a path containing any other white space\n> than U+0020 such as a tab? (Not that I am encouraging using such paths...)\n\nGood spotting.\n\nI thought use of [[:named character classes:]] is explicitly\ndisallowed by CodingGuidelines.\n"},{"id":"281292","messageId":"0767AEDB-36D6-45BE-A1CC-944ECAFC00FB@gmail.com","threadId":"41048","inReplyTo":"CAGZ79kYzbmzK1UwvWheugkqG0FwhpGMU0_K_jyCKOnoKb5oTxQ@mail.gmail.com","subject":"Re: [PATCH v3 1/3] submodule: add test to demonstrate that shallow recursive clones fail","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-03-20T17:05:34Z","receivedAt":"2016-03-20T17:05:34Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 15 Mar 2016, at 20:50, Stefan Beller <sbeller@google.com> wrote:\n\n> On Sun, Dec 20, 2015 at 3:19 PM,  <larsxschneider@gmail.com> wrote:\n>> From: Lars Schneider <larsxschneider@gmail.com>\n>> \n>> \"git clone --recursive --depth 1 --single-branch <url>\" clones the\n>> submodules successfully. However, it does not obey \"--depth 1\" for\n>> submodule cloning.\n> \n> I am about to resend \"[RFC/PATCH] clone: add `--shallow-submodules` flag\"\n> which would need tests and I thought about this series as tests.\nSounds good, thanks!\n\n\n> I assume patch 2 (fixing a broken && chain in tests) made it through,\n> but patch 1 and 3 did not? I may pick up ideas from here as it will be\n> slightly different tests I'd guess.\nUnfortunately patch 2 ($gmane/282776) did not made it in (I just checked \nnext 104e649). How should we proceed?\n\n> \n> \n>> +test_expect_failure shallow-clone-recursive '\n>> +       URL=\"file://$(pwd | sed \"s/[[:space:]]/%20/g\")/repo\" &&\n> \n> This would break if the test suite is in a path containing any other white space\n> than U+0020 such as a tab? (Not that I am encouraging using such paths...)\nTrue. I wonder if we really need to use the \"file://\" URL format here. Maybe\na relative would be easier?!\n\n\nThanks,\nLars"},{"id":"281304","messageId":"20160320194049.GA6288@sigill.intra.peff.net","threadId":"41048","inReplyTo":"0767AEDB-36D6-45BE-A1CC-944ECAFC00FB@gmail.com","subject":"Re: [PATCH v3 1/3] submodule: add test to demonstrate that shallow recursive clones fail","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-20T19:40:50Z","receivedAt":"2016-03-20T19:40:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Mar 20, 2016 at 06:05:34PM +0100, Lars Schneider wrote:\n\n> >> +test_expect_failure shallow-clone-recursive '\n> >> +       URL=\"file://$(pwd | sed \"s/[[:space:]]/%20/g\")/repo\" &&\n> > \n> > This would break if the test suite is in a path containing any other white space\n> > than U+0020 such as a tab? (Not that I am encouraging using such paths...)\n> True. I wonder if we really need to use the \"file://\" URL format here. Maybe\n> a relative would be easier?!\n\nI did not look closely at the patch or the tests, but generally we skip\nshallow-cloning for local-path repositories, as we don't do a regular\nobject transfer at all (and we turn that optimization off for file://\nURLs). So presumably it would defeat the purpose of the test.\n\n-Peff\n"},{"id":"281309","messageId":"56EF15C8.9060309@gmail.com","threadId":"41048","inReplyTo":"20160320194049.GA6288@sigill.intra.peff.net","subject":"Re: [PATCH v3 1/3] submodule: add test to demonstrate that shallow recursive clones fail","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2016-03-20T21:27:36Z","receivedAt":"2016-03-20T21:27:36Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 20.03.2016 12:40, Jeff King wrote:\n> On Sun, Mar 20, 2016 at 06:05:34PM +0100, Lars Schneider wrote:\n> \n>>>> +test_expect_failure shallow-clone-recursive '\n>>>> +       URL=\"file://$(pwd | sed \"s/[[:space:]]/%20/g\")/repo\" &&\n>>>\n>>> This would break if the test suite is in a path containing any other white space\n>>> than U+0020 such as a tab? (Not that I am encouraging using such paths...)\n>> True. I wonder if we really need to use the \"file://\" URL format here. Maybe\n>> a relative would be easier?!\n> \n> I did not look closely at the patch or the tests, but generally we skip\n> shallow-cloning for local-path repositories, as we don't do a regular\n> object transfer at all (and we turn that optimization off for file://\n> URLs). So presumably it would defeat the purpose of the test.\n> \n> -Peff\n\nPlease see proposed patches at $gmane/288940\n\nBy passing along the --[no-]local option we can have tests, without\nusing file:// explicitely.\n\nThanks,\nStefan\n"}]}