{"thread":{"id":"42660","subject":"[BUG REPORT] git 2.9.0 clone --recursive fails on cloning a submodule","startedAt":"2016-06-19T07:17:49Z","lastAt":"2017-04-19T18:54:30Z","messageCount":17,"participants":["Vadim Eisenberg","Jeff King","Junio C Hamano","Stefan Beller","Dennis Kaarsemaker","Duy Nguyen","Ævar Arnfjörð Bjarmason"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"289610","messageId":"OFE09D48F2.D1D14F49-ONC2257FD7.00280736-C2257FD7.0028245A@notes.na.collabserv.com","threadId":"42660","inReplyTo":"OFC76C15DC.FC882C57-ONC2257FD7.00261552-C2257FD7.002660FC@LocalDomain","subject":"[BUG REPORT] git 2.9.0 clone --recursive fails on cloning a submodule","fromName":"Vadim Eisenberg","fromEmail":"vadime@il.ibm.com","sentAt":"2016-06-19T07:17:36Z","receivedAt":"2016-06-19T07:17:49Z","isPatch":false,"sender":{"key":"vadime@il.ibm.com","avatar":null},"body":"/usr/local/bin/git clone --recursive --depth 10 \nhttps://github.com/IBM-Swift/Kitura-net.git\nCloning into 'Kitura-net'...\nremote: Counting objects: 253, done.\nremote: Compressing objects: 100% (142/142), done.\nremote: Total 253 (delta 134), reused 188 (delta 86), pack-reused 0\nReceiving objects: 100% (253/253), 63.28 KiB | 0 bytes/s, done.\nResolving deltas: 100% (134/134), done.\nChecking connectivity... done.\nSubmodule 'Kitura-Build' (https://github.com/IBM-Swift/Kitura-Build.git) \nregistered for path 'Kitura-Build'\nCloning into '/home/vadime/Kitura-net/Kitura-Build'...\nerror: no such remote ref d0d9d6c739a79627641e6438fe4f39bd0eba83bb\nFetched in submodule path 'Kitura-Build', but it did not contain \nd0d9d6c739a79627641e6438fe4f39bd0eba83bb. Direct fetching of that commit \nfailed.\n\nLinux version:\n4.2.0-25-generic #30-Ubuntu SMP Mon Jan 18 12:31:50 UTC 2016 x86_64 x86_64 \nx86_64 GNU/Linux\n\n\n\n"},{"id":"289617","messageId":"20160619100051.GA14584@sigill.intra.peff.net","threadId":"42660","inReplyTo":"OFE09D48F2.D1D14F49-ONC2257FD7.00280736-C2257FD7.0028245A@notes.na.collabserv.com","subject":"Re: [BUG REPORT] git 2.9.0 clone --recursive fails on cloning a submodule","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-19T10:00:51Z","receivedAt":"2016-06-19T10:00:58Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 19, 2016 at 10:17:36AM +0300, Vadim Eisenberg wrote:\n\n> /usr/local/bin/git clone --recursive --depth 10 \n> https://github.com/IBM-Swift/Kitura-net.git\n> Cloning into 'Kitura-net'...\n> remote: Counting objects: 253, done.\n> remote: Compressing objects: 100% (142/142), done.\n> remote: Total 253 (delta 134), reused 188 (delta 86), pack-reused 0\n> Receiving objects: 100% (253/253), 63.28 KiB | 0 bytes/s, done.\n> Resolving deltas: 100% (134/134), done.\n> Checking connectivity... done.\n> Submodule 'Kitura-Build' (https://github.com/IBM-Swift/Kitura-Build.git) \n> registered for path 'Kitura-Build'\n> Cloning into '/home/vadime/Kitura-net/Kitura-Build'...\n> error: no such remote ref d0d9d6c739a79627641e6438fe4f39bd0eba83bb\n> Fetched in submodule path 'Kitura-Build', but it did not contain \n> d0d9d6c739a79627641e6438fe4f39bd0eba83bb. Direct fetching of that commit \n> failed.\n\nThe problem seems to be the shallow clone. The super-project points to a\ncommit in the submodule that is not near the tip of any branch, so\nshallow-cloning the submodule means we don't get that commit. Prior to\nd22eb04 (clone: add `--shallow-submodules` flag, 2016-04-25), submodules\nwere _always_ cloned fully.\n\nThe immediate workaround is to add \"--no-shallow-submodules\" to your\nclone invocation.\n\nStefan, I think it might be worth revisiting the default set by d22eb04\nto propagate shallowness from the super-project clone. In an ideal\nworld, we would be asking each submodule for the actual commit we are\ninterested in, and shallowness would not matter. But until\nuploadpack.allowReachableSHA1InWant works everywhere, I suspect this is\ngoing to be a problem.\n\n-Peff\n"},{"id":"289622","messageId":"OFC46A8782.5F47CDBA-ONC2257FD7.00475209-C2257FD7.00483117@notes.na.collabserv.com","threadId":"42660","inReplyTo":"20160619100051.GA14584@sigill.intra.peff.net","subject":"Re: [BUG REPORT] git 2.9.0 clone --recursive fails on cloning a submodule","fromName":"Vadim Eisenberg","fromEmail":"vadime@il.ibm.com","sentAt":"2016-06-19T13:07:41Z","receivedAt":"2016-06-19T13:07:52Z","isPatch":false,"sender":{"key":"vadime@il.ibm.com","avatar":null},"body":"The problem is with other tools that use git, like Swift Package Manager - \nhttps://swift.org/package-manager/.\nVersions of git before 2.9.0 have no option --no-shallow-submodules. So \nthe tools that use git would have to check if the version of git is \ngreater than or equal 2.9.0 to know to specify that option.\n\nBest Regards,\nVadim\n\nJeff King wrote on 06/19/2016 01:00:51 PM:\n\n> From: Jeff King \n> To: Vadim Eisenberg/Haifa/IBM@IBMIL\n> Cc: Stefan Beller , git@vger.kernel.org\n> Date: 06/19/2016 01:01 PM\n> Subject: Re: [BUG REPORT] git 2.9.0 clone --recursive fails on \n> cloning a submodule\n> \n> On Sun, Jun 19, 2016 at 10:17:36AM +0300, Vadim Eisenberg wrote:\n> \n> > /usr/local/bin/git clone --recursive --depth 10 \n> > https://github.com/IBM-Swift/Kitura-net.git\n> > Cloning into 'Kitura-net'...\n> > remote: Counting objects: 253, done.\n> > remote: Compressing objects: 100% (142/142), done.\n> > remote: Total 253 (delta 134), reused 188 (delta 86), pack-reused 0\n> > Receiving objects: 100% (253/253), 63.28 KiB | 0 bytes/s, done.\n> > Resolving deltas: 100% (134/134), done.\n> > Checking connectivity... done.\n> > Submodule 'Kitura-Build' (\nhttps://github.com/IBM-Swift/Kitura-Build.git) \n> > registered for path 'Kitura-Build'\n> > Cloning into '/home/vadime/Kitura-net/Kitura-Build'...\n> > error: no such remote ref d0d9d6c739a79627641e6438fe4f39bd0eba83bb\n> > Fetched in submodule path 'Kitura-Build', but it did not contain \n> > d0d9d6c739a79627641e6438fe4f39bd0eba83bb. Direct fetching of that \ncommit \n> > failed.\n> \n> The problem seems to be the shallow clone. The super-project points to a\n> commit in the submodule that is not near the tip of any branch, so\n> shallow-cloning the submodule means we don't get that commit. Prior to\n> d22eb04 (clone: add `--shallow-submodules` flag, 2016-04-25), submodules\n> were _always_ cloned fully.\n> \n> The immediate workaround is to add \"--no-shallow-submodules\" to your\n> clone invocation.\n> \n> Stefan, I think it might be worth revisiting the default set by d22eb04\n> to propagate shallowness from the super-project clone. In an ideal\n> world, we would be asking each submodule for the actual commit we are\n> interested in, and shallowness would not matter. But until\n> uploadpack.allowReachableSHA1InWant works everywhere, I suspect this is\n> going to be a problem.\n> \n> -Peff\n> \n\n\n"},{"id":"289625","messageId":"20160619144628.GA7402@sigill.intra.peff.net","threadId":"42660","inReplyTo":"OFC46A8782.5F47CDBA-ONC2257FD7.00475209-C2257FD7.00483117@notes.na.collabserv.com","subject":"Re: [BUG REPORT] git 2.9.0 clone --recursive fails on cloning a submodule","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-19T14:46:28Z","receivedAt":"2016-06-19T14:47:54Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 19, 2016 at 04:07:41PM +0300, Vadim Eisenberg wrote:\n\n> The problem is with other tools that use git, like Swift Package Manager - \n> https://swift.org/package-manager/.\n> Versions of git before 2.9.0 have no option --no-shallow-submodules. So \n> the tools that use git would have to check if the version of git is \n> greater than or equal 2.9.0 to know to specify that option.\n\nOh, I agree it's something we should treat as a regression. Using\n`--no-shallow-submodules` is just a workaround.\n\n-Peff\n"},{"id":"289642","messageId":"xmqq7fdkx5oz.fsf@gitster.mtv.corp.google.com","threadId":"42660","inReplyTo":"20160619100051.GA14584@sigill.intra.peff.net","subject":"Re: [BUG REPORT] git 2.9.0 clone --recursive fails on cloning a submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-19T20:51:56Z","receivedAt":"2016-06-19T20:52:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Stefan, I think it might be worth revisiting the default set by d22eb04\n> to propagate shallowness from the super-project clone. In an ideal\n> world, we would be asking each submodule for the actual commit we are\n> interested in, and shallowness would not matter. But until\n> uploadpack.allowReachableSHA1InWant works everywhere, I suspect this is\n> going to be a problem.\n\nYup, something like this on top of d22eb04 to be merged before\nv2.9.1 for the maintenance track would be necessary.\n\n-- >8 --\nSubject: clone: do not let --depth imply --shallow-submodules\n\nIn v2.9.0, we prematurely flipped the default to force cloning\nsubmodules shallowly, when the superproject is getting cloned\nshallowly. This is likely to fail when the upstream repositories\nsubmodules are cloned from a repository that is not prepared to\nserve histories that ends at a commit that is not at the tip of a\nbranch, and we know the world is not yet ready.\n\nUse a safer default to clone the submodules fully, unless the user\ntells us that she knows that the upstream repository of the\nsubmodules are willing to cooperate with \"--shallow-submodules\"\noption.\n\nNoticed-by: Vadim Eisenberg \nHelped-by: Jeff King \nSigned-off-by: Junio C Hamano \n---\n Documentation/git-clone.txt | 5 ++---\n builtin/clone.c | 5 ++---\n t/t5614-clone-submodules.sh | 4 ++--\n 3 files changed, 6 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex e1a21b7..c5a1ce2 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -192,9 +192,8 @@ objects from the source repository into a pack in the cloned repository.\n \tCreate a 'shallow' clone with a history truncated to the\n \tspecified number of revisions. Implies `--single-branch` unless\n \t`--no-single-branch` is given to fetch the histories near the\n-\ttips of all branches. This implies `--shallow-submodules`. If\n-\tyou want to have a shallow superproject clone, but full submodules,\n-\talso pass `--no-shallow-submodules`.\n+\ttips of all branches. If you want to clone submodules shallowly,\n+\talso pass `--shallow-submodules`.\n \n --[no-]single-branch::\n \tClone only the history leading to the tip of a single branch,\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex ecdf308..f267742 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -40,7 +40,7 @@ static const char * const builtin_clone_usage[] = {\n \n static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;\n static int option_local = -1, option_no_hardlinks, option_shared, option_recursive;\n-static int option_shallow_submodules = -1;\n+static int option_shallow_submodules;\n static char *option_template, *option_depth;\n static char *option_origin = NULL;\n static char *option_branch = NULL;\n@@ -730,8 +730,7 @@ static int checkout(void)\n \t\tstruct argv_array args = ARGV_ARRAY_INIT;\n \t\targv_array_pushl(&args, \"submodule\", \"update\", \"--init\", \"--recursive\", NULL);\n \n-\t\tif (option_shallow_submodules == 1\n-\t\t || (option_shallow_submodules == -1 && option_depth))\n+\t\tif (option_shallow_submodules == 1)\n \t\t\targv_array_push(&args, \"--depth=1\");\n \n \t\tif (max_jobs != -1)\ndiff --git a/t/t5614-clone-submodules.sh b/t/t5614-clone-submodules.sh\nindex 62044c5..f7c630b 100755\n--- a/t/t5614-clone-submodules.sh\n+++ b/t/t5614-clone-submodules.sh\n@@ -37,9 +37,9 @@ test_expect_success 'nonshallow clone implies nonshallow submodule' '\n \t)\n '\n \n-test_expect_success 'shallow clone implies shallow submodule' '\n+test_expect_success 'shallow clone does not imply shallow submodule' '\n \ttest_when_finished \"rm -rf super_clone\" &&\n-\tgit clone --recurse-submodules --depth 2 \"file://$pwd/.\" super_clone &&\n+\tgit clone --recurse-submodules --depth 2 --shallow-submodules \"file://$pwd/.\" super_clone &&\n \t(\n \t\tcd super_clone &&\n \t\tgit log --oneline >lines &&\n"},{"id":"289648","messageId":"20160620001332.GA10101@sigill.intra.peff.net","threadId":"42660","inReplyTo":"xmqq7fdkx5oz.fsf@gitster.mtv.corp.google.com","subject":"Re: [BUG REPORT] git 2.9.0 clone --recursive fails on cloning a submodule","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-20T00:13:32Z","receivedAt":"2016-06-20T00:20:24Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 19, 2016 at 01:51:56PM -0700, Junio C Hamano wrote:\n\n> Yup, something like this on top of d22eb04 to be merged before\n> v2.9.1 for the maintenance track would be necessary.\n> \n> -- >8 --\n> Subject: clone: do not let --depth imply --shallow-submodules\n> \n> In v2.9.0, we prematurely flipped the default to force cloning\n> submodules shallowly, when the superproject is getting cloned\n> shallowly. This is likely to fail when the upstream repositories\n> submodules are cloned from a repository that is not prepared to\n> serve histories that ends at a commit that is not at the tip of a\n> branch, and we know the world is not yet ready.\n> \n> Use a safer default to clone the submodules fully, unless the user\n> tells us that she knows that the upstream repository of the\n> submodules are willing to cooperate with \"--shallow-submodules\"\n> option.\n\nYeah, this looks good. To minor comments:\n\n> @@ -730,8 +730,7 @@ static int checkout(void)\n> \t\tstruct argv_array args = ARGV_ARRAY_INIT;\n> \t\targv_array_pushl(&args, \"submodule\", \"update\", \"--init\", \"--recursive\", NULL);\n> \n> -\t\tif (option_shallow_submodules == 1\n> -\t\t || (option_shallow_submodules == -1 && option_depth))\n> +\t\tif (option_shallow_submodules == 1)\n> \t\t\targv_array_push(&args, \"--depth=1\");\n\nI hadn't paid much attention to this topic originally, but was surprised\nthat \"--depth 10\" in the clone implies \"--depth 1\" in the submodule.\nThis is not really related to your patch (in fact, your patch makes the\nlogic go away). But maybe something to consider if it's ever resurrected\n(or possibly if somebody runs \"--shallow-submodules --depth 5\" we should\npass --depth=1; I dunno).\n\n> -test_expect_success 'shallow clone implies shallow submodule' '\n> +test_expect_success 'shallow clone does not imply shallow submodule' '\n> \ttest_when_finished \"rm -rf super_clone\" &&\n> -\tgit clone --recurse-submodules --depth 2 \"file://$pwd/.\" super_clone &&\n> +\tgit clone --recurse-submodules --depth 2 --shallow-submodules \"file://$pwd/.\" super_clone &&\n> \t(\n> \t\tcd super_clone &&\n> \t\tgit log --oneline >lines &&\n\nWe are not really testing \"does not imply\" here, but \"passing\n--shallow-submodules works\". The \"does not imply\" test would be cloning\nwithout the option and checking that the resulting submodules are not\nshallow.\n\n-Peff\n"},{"id":"289649","messageId":"CAGZ79kZ9NF57EyEZ4PgOhJw50ujt=QmHs+w1ZNFeDO4zitksVQ@mail.gmail.com","threadId":"42660","inReplyTo":"20160620001332.GA10101@sigill.intra.peff.net","subject":"Re: [BUG REPORT] git 2.9.0 clone --recursive fails on cloning a submodule","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-06-20T01:09:28Z","receivedAt":"2016-06-20T01:10:23Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sun, Jun 19, 2016 at 5:13 PM, Jeff King wrote:\n> On Sun, Jun 19, 2016 at 01:51:56PM -0700, Junio C Hamano wrote:\n>\n>> Yup, something like this on top of d22eb04 to be merged before\n>> v2.9.1 for the maintenance track would be necessary.\n>>\n>> -- >8 --\n>> Subject: clone: do not let --depth imply --shallow-submodules\n>>\n>> In v2.9.0, we prematurely flipped the default to force cloning\n>> submodules shallowly, when the superproject is getting cloned\n>> shallowly. This is likely to fail when the upstream repositories\n>> submodules are cloned from a repository that is not prepared to\n>> serve histories that ends at a commit that is not at the tip of a\n>> branch, and we know the world is not yet ready.\n>>\n>> Use a safer default to clone the submodules fully, unless the user\n>> tells us that she knows that the upstream repository of the\n>> submodules are willing to cooperate with \"--shallow-submodules\"\n>> option.\n>\n> Yeah, this looks good. To minor comments:\n\nI agree, but I find the second concern a bit more than just minor.\n\n>\n>> @@ -730,8 +730,7 @@ static int checkout(void)\n>> struct argv_array args = ARGV_ARRAY_INIT;\n>> argv_array_pushl(&args, \"submodule\", \"update\", \"--init\", \"--recursive\", NULL);\n>>\n>> - if (option_shallow_submodules == 1\n>> - || (option_shallow_submodules == -1 && option_depth))\n>> + if (option_shallow_submodules == 1)\n>> argv_array_push(&args, \"--depth=1\");\n>\n> I hadn't paid much attention to this topic originally, but was surprised\n> that \"--depth 10\" in the clone implies \"--depth 1\" in the submodule.\n> This is not really related to your patch (in fact, your patch makes the\n> logic go away). But maybe something to consider if it's ever resurrected\n> (or possibly if somebody runs \"--shallow-submodules --depth 5\" we should\n> pass --depth=1; I dunno).\n\nHow often do we see a depth != 1 in practice?\nI have the impression (and no data to back up my claim) that a binary\nswitch for nonshallow or depth 1 would serve us just as good, which is why\nI did not want to ad complexity to the submodule depth.\n(What if you want submodule A with depth 2 and B with 5? In that\ncase get them all shallow and deepen as appropriate, would be my answer)\n\n>\n>> -test_expect_success 'shallow clone implies shallow submodule' '\n>> +test_expect_success 'shallow clone does not imply shallow submodule' '\n>> test_when_finished \"rm -rf super_clone\" &&\n>> - git clone --recurse-submodules --depth 2 \"file://$pwd/.\" super_clone &&\n>> + git clone --recurse-submodules --depth 2 --shallow-submodules \"file://$pwd/.\" super_clone &&\n>> (\n>> cd super_clone &&\n>> git log --oneline >lines &&\n>\n> We are not really testing \"does not imply\" here, but \"passing\n> --shallow-submodules works\". The \"does not imply\" test would be cloning\n> without the option and checking that the resulting submodules are not\n> shallow.\n\nIn case we want to be sure that it works for 2.9.1, i.e. we treat it\nas a regression,\nwe need to test the \"does not imply\" a bit more I would think. I can send that\ntest on top tomorrow if you'd like to.\n\nThanks,\nStefan\n\n>\n> -Peff\n"},{"id":"289651","messageId":"OFA474D4C5.A6C331B9-ONC2257FD8.00104FEB-C2257FD8.0010AA61@notes.na.collabserv.com","threadId":"42660","inReplyTo":"CAGZ79kZ9NF57EyEZ4PgOhJw50ujt=QmHs+w1ZNFeDO4zitksVQ@mail.gmail.com","subject":"Re: [BUG REPORT] git 2.9.0 clone --recursive fails on cloning a submodule","fromName":"Vadim Eisenberg","fromEmail":"vadime@il.ibm.com","sentAt":"2016-06-20T03:01:08Z","receivedAt":"2016-06-20T03:16:25Z","isPatch":false,"sender":{"key":"vadime@il.ibm.com","avatar":null},"body":"> How often do we see a depth != 1 in practice? \n\nFYI: Here is an example of depth 10 from Swift Package Manager which is \ngit-based \nhttps://github.com/apple/swift-package-manager/blob/61bdff3e9c2b8456be31861b4cda87a31440d517/Sources/Get/Git.swift\n\ntry system(Git.tool, \"clone\",\n \"--recursive\", // get submodules too so that developers can use \nthese if they so choose\n \"--depth\", \"10\",\n\nStefan Beller wrote on 06/20/2016 04:09:28 AM:\n\n> From: Stefan Beller \n> To: Jeff King \n> Cc: Junio C Hamano , Vadim Eisenberg/Haifa/\n> IBM@IBMIL, \"git@vger.kernel.org\" \n> Date: 06/20/2016 04:09 AM\n> Subject: Re: [BUG REPORT] git 2.9.0 clone --recursive fails on \n> cloning a submodule\n> \n> On Sun, Jun 19, 2016 at 5:13 PM, Jeff King wrote:\n> > On Sun, Jun 19, 2016 at 01:51:56PM -0700, Junio C Hamano wrote:\n> >\n> >> Yup, something like this on top of d22eb04 to be merged before\n> >> v2.9.1 for the maintenance track would be necessary.\n> >>\n> >> -- >8 --\n> >> Subject: clone: do not let --depth imply --shallow-submodules\n> >>\n> >> In v2.9.0, we prematurely flipped the default to force cloning\n> >> submodules shallowly, when the superproject is getting cloned\n> >> shallowly. This is likely to fail when the upstream repositories\n> >> submodules are cloned from a repository that is not prepared to\n> >> serve histories that ends at a commit that is not at the tip of a\n> >> branch, and we know the world is not yet ready.\n> >>\n> >> Use a safer default to clone the submodules fully, unless the user\n> >> tells us that she knows that the upstream repository of the\n> >> submodules are willing to cooperate with \"--shallow-submodules\"\n> >> option.\n> >\n> > Yeah, this looks good. To minor comments:\n> \n> I agree, but I find the second concern a bit more than just minor.\n> \n> >\n> >> @@ -730,8 +730,7 @@ static int checkout(void)\n> >> struct argv_array args = ARGV_ARRAY_INIT;\n> >> argv_array_pushl(&args, \"submodule\", \"update\", \"--\n> init\", \"--recursive\", NULL);\n> >>\n> >> - if (option_shallow_submodules == 1\n> >> - || (option_shallow_submodules == -1 && \noption_depth))\n> >> + if (option_shallow_submodules == 1)\n> >> argv_array_push(&args, \"--depth=1\");\n> >\n> > I hadn't paid much attention to this topic originally, but was \nsurprised\n> > that \"--depth 10\" in the clone implies \"--depth 1\" in the submodule.\n> > This is not really related to your patch (in fact, your patch makes \nthe\n> > logic go away). But maybe something to consider if it's ever \nresurrected\n> > (or possibly if somebody runs \"--shallow-submodules --depth 5\" we \nshould\n> > pass --depth=1; I dunno).\n> \n> How often do we see a depth != 1 in practice?\n> I have the impression (and no data to back up my claim) that a binary\n> switch for nonshallow or depth 1 would serve us just as good, which is \nwhy\n> I did not want to ad complexity to the submodule depth.\n> (What if you want submodule A with depth 2 and B with 5? In that\n> case get them all shallow and deepen as appropriate, would be my answer)\n> \n> >\n> >> -test_expect_success 'shallow clone implies shallow submodule' '\n> >> +test_expect_success 'shallow clone does not imply shallow submodule' \n'\n> >> test_when_finished \"rm -rf super_clone\" &&\n> >> - git clone --recurse-submodules --depth 2 \"file://$pwd/.\" \n> super_clone &&\n> >> + git clone --recurse-submodules --depth 2 --shallow-submodules \"\n> file://$pwd/.\" super_clone &&\n> >> (\n> >> cd super_clone &&\n> >> git log --oneline >lines &&\n> >\n> > We are not really testing \"does not imply\" here, but \"passing\n> > --shallow-submodules works\". The \"does not imply\" test would be \ncloning\n> > without the option and checking that the resulting submodules are not\n> > shallow.\n> \n> In case we want to be sure that it works for 2.9.1, i.e. we treat it\n> as a regression,\n> we need to test the \"does not imply\" a bit more I would think. I cansend \nthat\n> test on top tomorrow if you'd like to.\n> \n> Thanks,\n> Stefan\n> \n> >\n> > -Peff\n> \n\n\n"},{"id":"289652","messageId":"1466400663.20305.0.camel@kaarsemaker.net","threadId":"42660","inReplyTo":"CAGZ79kZ9NF57EyEZ4PgOhJw50ujt=QmHs+w1ZNFeDO4zitksVQ@mail.gmail.com","subject":"Re: [BUG REPORT] git 2.9.0 clone --recursive fails on cloning a submodule","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2016-06-20T05:31:03Z","receivedAt":"2016-06-20T05:31:10Z","isPatch":false,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"On zo, 2016-06-19 at 18:09 -0700, Stefan Beller wrote:\n\n> How often do we see a depth != 1 in practice?\n\nTravis clones with --depth=50\n\nD.\n"},{"id":"289664","messageId":"20160620100256.GA14058@sigill.intra.peff.net","threadId":"42660","inReplyTo":"CAGZ79kZ9NF57EyEZ4PgOhJw50ujt=QmHs+w1ZNFeDO4zitksVQ@mail.gmail.com","subject":"Re: [BUG REPORT] git 2.9.0 clone --recursive fails on cloning a submodule","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-20T10:02:57Z","receivedAt":"2016-06-20T10:10:53Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 19, 2016 at 06:09:28PM -0700, Stefan Beller wrote:\n\n> > I hadn't paid much attention to this topic originally, but was surprised\n> > that \"--depth 10\" in the clone implies \"--depth 1\" in the submodule.\n> > This is not really related to your patch (in fact, your patch makes the\n> > logic go away). But maybe something to consider if it's ever resurrected\n> > (or possibly if somebody runs \"--shallow-submodules --depth 5\" we should\n> > pass --depth=1; I dunno).\n> \n> How often do we see a depth != 1 in practice?\n> I have the impression (and no data to back up my claim) that a binary\n> switch for nonshallow or depth 1 would serve us just as good, which is why\n> I did not want to ad complexity to the submodule depth.\n> (What if you want submodule A with depth 2 and B with 5? In that\n> case get them all shallow and deepen as appropriate, would be my answer)\n\nTo be honest, I don't know why people use anything except --depth=1, but\nit's clear from my experience that they do. This example has --depth=10,\nand on the server side at GitHub I have seen similar numbers from clients,\nespecially CI services.\n\n(I take special note of such cases because --shallow quite often causes\nperformance problems on the server side, though generally --depth=10 is\nnot any worse than --depth=1. The worst case is really\n\"--no-single-branch --depth=1\", which wants a ton of objects but has to\nthrow away all of the on-disk deltas).\n\n> > We are not really testing \"does not imply\" here, but \"passing\n> > --shallow-submodules works\". The \"does not imply\" test would be cloning\n> > without the option and checking that the resulting submodules are not\n> > shallow.\n> \n> In case we want to be sure that it works for 2.9.1, i.e. we treat it\n> as a regression,\n> we need to test the \"does not imply\" a bit more I would think. I can send that\n> test on top tomorrow if you'd like to.\n\nI think it's worth doing (and testing both: the default behavior, and\nthat the --shallow-submodules feature works). Thanks.\n\n-Peff\n"},{"id":"289682","messageId":"1466441998-18896-1-git-send-email-sbeller@google.com","threadId":"42660","inReplyTo":"xmqq7fdkx5oz.fsf@gitster.mtv.corp.google.com","subject":"[PATCH] shallow clone to not imply shallow submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-06-20T16:59:58Z","receivedAt":"2016-06-20T17:01:06Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n\nHi Junio, Peff,\n\nI thought about this patch squashed into \n\"clone: do not let --depth imply --shallow-submodules\" will actually test\nfor the regression.\n\nThanks,\nStefan\n\n t/t5614-clone-submodules.sh | 17 ++++++++++++++++-\n 1 file changed, 16 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t5614-clone-submodules.sh b/t/t5614-clone-submodules.sh\nindex f7c630b..a9aaa01 100755\n--- a/t/t5614-clone-submodules.sh\n+++ b/t/t5614-clone-submodules.sh\n@@ -37,7 +37,7 @@ test_expect_success 'nonshallow clone implies nonshallow submodule' '\n \t)\n '\n \n-test_expect_success 'shallow clone does not imply shallow submodule' '\n+test_expect_success 'shallow clone with shallow submodule' '\n \ttest_when_finished \"rm -rf super_clone\" &&\n \tgit clone --recurse-submodules --depth 2 --shallow-submodules \"file://$pwd/.\" super_clone &&\n \t(\n@@ -52,6 +52,21 @@ test_expect_success 'shallow clone does not imply shallow submodule' '\n \t)\n '\n \n+test_expect_success 'shallow clone does not imply shallow submodule' '\n+\ttest_when_finished \"rm -rf super_clone\" &&\n+\tgit clone --recurse-submodules --depth 2 \"file://$pwd/.\" super_clone &&\n+\t(\n+\t\tcd super_clone &&\n+\t\tgit log --oneline >lines &&\n+\t\ttest_line_count = 2 lines\n+\t) &&\n+\t(\n+\t\tcd super_clone/sub &&\n+\t\tgit log --oneline >lines &&\n+\t\ttest_line_count = 3 lines\n+\t)\n+'\n+\n test_expect_success 'shallow clone with non shallow submodule' '\n \ttest_when_finished \"rm -rf super_clone\" &&\n \tgit clone --recurse-submodules --depth 2 --no-shallow-submodules \"file://$pwd/.\" super_clone &&\n-- \n2.7.0.rc0.40.g5328432.dirty\n\n"},{"id":"289685","messageId":"CAGZ79kbKoch0odOOz2QWhoGoSOBOgYyQ_QgbxPXy2iJJVqP6Vw@mail.gmail.com","threadId":"42660","inReplyTo":"20160620171312.GA1880@sigill.intra.peff.net","subject":"Re: [PATCH] shallow clone to not imply shallow submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-06-20T17:14:47Z","receivedAt":"2016-06-20T17:15:35Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Jun 20, 2016 at 10:13 AM, Jeff King wrote:\n> On Mon, Jun 20, 2016 at 09:59:58AM -0700, Stefan Beller wrote:\n>\n>> Signed-off-by: Stefan Beller \n>> ---\n>>\n>> Hi Junio, Peff,\n>>\n>> I thought about this patch squashed into\n>> \"clone: do not let --depth imply --shallow-submodules\" will actually test\n>> for the regression.\n>\n> Yep, it looks good to me.\n>\n>> +test_expect_success 'shallow clone does not imply shallow submodule' '\n>> + test_when_finished \"rm -rf super_clone\" &&\n>> + git clone --recurse-submodules --depth 2 \"file://$pwd/.\" super_clone &&\n>> + (\n>> + cd super_clone &&\n>> + git log --oneline >lines &&\n>> + test_line_count = 2 lines\n>> + ) &&\n>> + (\n>> + cd super_clone/sub &&\n>> + git log --oneline >lines &&\n>> + test_line_count = 3 lines\n>> + )\n>> +'\n>\n> This follows the style of the other tests, so it's the right thing here.\n> But as a style suggestion, I think:\n>\n> git -C super_clone/sub log --oneline >lines &&\n> test_line_count = 3 lines\n>\n> is nicer than the subshell. It's more succinct, and it saves a process.\n\nwhich we would want to refactor to in a follow up, but not merge it\nthrough to 2.9.1.\n\nThanks,\nStefan\n\n>\n> -Peff\n"},{"id":"289686","messageId":"20160620171821.GA2316@sigill.intra.peff.net","threadId":"42660","inReplyTo":"CAGZ79kbKoch0odOOz2QWhoGoSOBOgYyQ_QgbxPXy2iJJVqP6Vw@mail.gmail.com","subject":"Re: [PATCH] shallow clone to not imply shallow submodules","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-20T17:18:22Z","receivedAt":"2016-06-20T17:19:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 20, 2016 at 10:14:47AM -0700, Stefan Beller wrote:\n\n> > This follows the style of the other tests, so it's the right thing here.\n> > But as a style suggestion, I think:\n> >\n> > git -C super_clone/sub log --oneline >lines &&\n> > test_line_count = 3 lines\n> >\n> > is nicer than the subshell. It's more succinct, and it saves a process.\n> \n> which we would want to refactor to in a follow up, but not merge it\n> through to 2.9.1.\n\nYeah, exactly. That was what I meant by \"here\".\n\n-Peff\n"},{"id":"289688","messageId":"20160620171312.GA1880@sigill.intra.peff.net","threadId":"42660","inReplyTo":"1466441998-18896-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH] shallow clone to not imply shallow submodules","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-20T17:13:12Z","receivedAt":"2016-06-20T17:40:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 20, 2016 at 09:59:58AM -0700, Stefan Beller wrote:\n\n> Signed-off-by: Stefan Beller \n> ---\n> \n> Hi Junio, Peff,\n> \n> I thought about this patch squashed into \n> \"clone: do not let --depth imply --shallow-submodules\" will actually test\n> for the regression.\n\nYep, it looks good to me.\n\n> +test_expect_success 'shallow clone does not imply shallow submodule' '\n> +\ttest_when_finished \"rm -rf super_clone\" &&\n> +\tgit clone --recurse-submodules --depth 2 \"file://$pwd/.\" super_clone &&\n> +\t(\n> +\t\tcd super_clone &&\n> +\t\tgit log --oneline >lines &&\n> +\t\ttest_line_count = 2 lines\n> +\t) &&\n> +\t(\n> +\t\tcd super_clone/sub &&\n> +\t\tgit log --oneline >lines &&\n> +\t\ttest_line_count = 3 lines\n> +\t)\n> +'\n\nThis follows the style of the other tests, so it's the right thing here.\nBut as a style suggestion, I think:\n\n git -C super_clone/sub log --oneline >lines &&\n test_line_count = 3 lines\n\nis nicer than the subshell. It's more succinct, and it saves a process.\n\n-Peff\n"},{"id":"289783","messageId":"CACsJy8DZ8jR=jZoC9F6k+WKr4mvZHMHtvLpT5+Mt9shX5-Wj8g@mail.gmail.com","threadId":"42660","inReplyTo":"20160619100051.GA14584@sigill.intra.peff.net","subject":"Re: [BUG REPORT] git 2.9.0 clone --recursive fails on cloning a submodule","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-21T16:48:19Z","receivedAt":"2016-06-21T16:48:53Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Jun 19, 2016 at 12:00 PM, Jeff King wrote:\n> Stefan, I think it might be worth revisiting the default set by d22eb04\n> to propagate shallowness from the super-project clone. In an ideal\n> world, we would be asking each submodule for the actual commit we are\n> interested in, and shallowness would not matter. But until\n> uploadpack.allowReachableSHA1InWant works everywhere, I suspect this is\n> going to be a problem.\n\nMaybe we can pass an option to subsequent clones that say \"if\nallow-...-sha1-in-want is advertised, do a shallow clone, otherwise\nfall back to full clone\"?\n-- \nDuy\n"},{"id":"317225","messageId":"CACBZZX5goUsb4JTn6aGkJ-_S4W0Ymg7-HTzryDjC=AJBiLC3Fg@mail.gmail.com","threadId":"42660","inReplyTo":"xmqq7fdkx5oz.fsf@gitster.mtv.corp.google.com","subject":"Re: [BUG REPORT] git 2.9.0 clone --recursive fails on cloning a submodule","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-04-19T11:30:05Z","receivedAt":"2017-04-19T11:30:32Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sun, Jun 19, 2016 at 10:51 PM, Junio C Hamano wrote:\n> Jeff King writes:\n>\n>> Stefan, I think it might be worth revisiting the default set by d22eb04\n>> to propagate shallowness from the super-project clone. In an ideal\n>> world, we would be asking each submodule for the actual commit we are\n>> interested in, and shallowness would not matter. But until\n>> uploadpack.allowReachableSHA1InWant works everywhere, I suspect this is\n>> going to be a problem.\n>\n> Yup, something like this on top of d22eb04 to be merged before\n> v2.9.1 for the maintenance track would be necessary.\n>\n> -- >8 --\n> Subject: clone: do not let --depth imply --shallow-submodules\n>\n> In v2.9.0, we prematurely flipped the default to force cloning\n> submodules shallowly, when the superproject is getting cloned\n> shallowly. This is likely to fail when the upstream repositories\n> submodules are cloned from a repository that is not prepared to\n> serve histories that ends at a commit that is not at the tip of a\n> branch, and we know the world is not yet ready.\n>\n> Use a safer default to clone the submodules fully, unless the user\n> tells us that she knows that the upstream repository of the\n> submodules are willing to cooperate with \"--shallow-submodules\"\n> option.\n>\n> Noticed-by: Vadim Eisenberg \n> Helped-by: Jeff King \n> Signed-off-by: Junio C Hamano \n> ---\n> Documentation/git-clone.txt | 5 ++---\n> builtin/clone.c | 5 ++---\n> t/t5614-clone-submodules.sh | 4 ++--\n> 3 files changed, 6 insertions(+), 8 deletions(-)\n>\n> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\n> index e1a21b7..c5a1ce2 100644\n> --- a/Documentation/git-clone.txt\n> +++ b/Documentation/git-clone.txt\n> @@ -192,9 +192,8 @@ objects from the source repository into a pack in the cloned repository.\n> Create a 'shallow' clone with a history truncated to the\n> specified number of revisions. Implies `--single-branch` unless\n> `--no-single-branch` is given to fetch the histories near the\n> - tips of all branches. This implies `--shallow-submodules`. If\n> - you want to have a shallow superproject clone, but full submodules,\n> - also pass `--no-shallow-submodules`.\n> + tips of all branches. If you want to clone submodules shallowly,\n> + also pass `--shallow-submodules`.\n>\n> --[no-]single-branch::\n> Clone only the history leading to the tip of a single branch,\n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index ecdf308..f267742 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -40,7 +40,7 @@ static const char * const builtin_clone_usage[] = {\n>\n> static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;\n> static int option_local = -1, option_no_hardlinks, option_shared, option_recursive;\n> -static int option_shallow_submodules = -1;\n> +static int option_shallow_submodules;\n> static char *option_template, *option_depth;\n> static char *option_origin = NULL;\n> static char *option_branch = NULL;\n> @@ -730,8 +730,7 @@ static int checkout(void)\n> struct argv_array args = ARGV_ARRAY_INIT;\n> argv_array_pushl(&args, \"submodule\", \"update\", \"--init\", \"--recursive\", NULL);\n>\n> - if (option_shallow_submodules == 1\n> - || (option_shallow_submodules == -1 && option_depth))\n> + if (option_shallow_submodules == 1)\n> argv_array_push(&args, \"--depth=1\");\n\nVery late reply, since I'm just looking at this now with the --no-tags\nopti,n, but that == 1 makes no sense anymore, and should just be `if\n(option_shallow_submodules)` shouldn't it? I.e. this used to be an int\nfor the depth, now is a bool, but the current == 1 check is left over\nprobably from an earlier version where the depth was configurable.\n\n> if (max_jobs != -1)\n> diff --git a/t/t5614-clone-submodules.sh b/t/t5614-clone-submodules.sh\n> index 62044c5..f7c630b 100755\n> --- a/t/t5614-clone-submodules.sh\n> +++ b/t/t5614-clone-submodules.sh\n> @@ -37,9 +37,9 @@ test_expect_success 'nonshallow clone implies nonshallow submodule' '\n> )\n> '\n>\n> -test_expect_success 'shallow clone implies shallow submodule' '\n> +test_expect_success 'shallow clone does not imply shallow submodule' '\n> test_when_finished \"rm -rf super_clone\" &&\n> - git clone --recurse-submodules --depth 2 \"file://$pwd/.\" super_clone &&\n> + git clone --recurse-submodules --depth 2 --shallow-submodules \"file://$pwd/.\" super_clone &&\n> (\n> cd super_clone &&\n> git log --oneline >lines &&\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"317261","messageId":"CAGZ79kZ1ViZterpZoH7qSjXxZf_qbrveAf4XodskA2xPXdZb6g@mail.gmail.com","threadId":"42660","inReplyTo":"CACBZZX5goUsb4JTn6aGkJ-_S4W0Ymg7-HTzryDjC=AJBiLC3Fg@mail.gmail.com","subject":"Re: [BUG REPORT] git 2.9.0 clone --recursive fails on cloning a submodule","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-04-19T18:54:21Z","receivedAt":"2017-04-19T18:54:30Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Apr 19, 2017 at 4:30 AM, Ævar Arnfjörð Bjarmason\n wrote:\n> On Sun, Jun 19, 2016 at 10:51 PM, Junio C Hamano wrote:\n>> Jeff King writes:\n>>\n>>> Stefan, I think it might be worth revisiting the default set by d22eb04\n>>> to propagate shallowness from the super-project clone. In an ideal\n>>> world, we would be asking each submodule for the actual commit we are\n>>> interested in, and shallowness would not matter. But until\n>>> uploadpack.allowReachableSHA1InWant works everywhere, I suspect this is\n>>> going to be a problem.\n>>\n>> Yup, something like this on top of d22eb04 to be merged before\n>> v2.9.1 for the maintenance track would be necessary.\n>>\n>> -- >8 --\n>> Subject: clone: do not let --depth imply --shallow-submodules\n>>\n>> In v2.9.0, we prematurely flipped the default to force cloning\n>> submodules shallowly, when the superproject is getting cloned\n>> shallowly. This is likely to fail when the upstream repositories\n>> submodules are cloned from a repository that is not prepared to\n>> serve histories that ends at a commit that is not at the tip of a\n>> branch, and we know the world is not yet ready.\n>>\n>> Use a safer default to clone the submodules fully, unless the user\n>> tells us that she knows that the upstream repository of the\n>> submodules are willing to cooperate with \"--shallow-submodules\"\n>> option.\n>>\n>> Noticed-by: Vadim Eisenberg \n>> Helped-by: Jeff King \n>> Signed-off-by: Junio C Hamano \n>> ---\n>> Documentation/git-clone.txt | 5 ++---\n>> builtin/clone.c | 5 ++---\n>> t/t5614-clone-submodules.sh | 4 ++--\n>> 3 files changed, 6 insertions(+), 8 deletions(-)\n>>\n>> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\n>> index e1a21b7..c5a1ce2 100644\n>> --- a/Documentation/git-clone.txt\n>> +++ b/Documentation/git-clone.txt\n>> @@ -192,9 +192,8 @@ objects from the source repository into a pack in the cloned repository.\n>> Create a 'shallow' clone with a history truncated to the\n>> specified number of revisions. Implies `--single-branch` unless\n>> `--no-single-branch` is given to fetch the histories near the\n>> - tips of all branches. This implies `--shallow-submodules`. If\n>> - you want to have a shallow superproject clone, but full submodules,\n>> - also pass `--no-shallow-submodules`.\n>> + tips of all branches. If you want to clone submodules shallowly,\n>> + also pass `--shallow-submodules`.\n>>\n>> --[no-]single-branch::\n>> Clone only the history leading to the tip of a single branch,\n>> diff --git a/builtin/clone.c b/builtin/clone.c\n>> index ecdf308..f267742 100644\n>> --- a/builtin/clone.c\n>> +++ b/builtin/clone.c\n>> @@ -40,7 +40,7 @@ static const char * const builtin_clone_usage[] = {\n>>\n>> static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;\n>> static int option_local = -1, option_no_hardlinks, option_shared, option_recursive;\n>> -static int option_shallow_submodules = -1;\n>> +static int option_shallow_submodules;\n>> static char *option_template, *option_depth;\n>> static char *option_origin = NULL;\n>> static char *option_branch = NULL;\n>> @@ -730,8 +730,7 @@ static int checkout(void)\n>> struct argv_array args = ARGV_ARRAY_INIT;\n>> argv_array_pushl(&args, \"submodule\", \"update\", \"--init\", \"--recursive\", NULL);\n>>\n>> - if (option_shallow_submodules == 1\n>> - || (option_shallow_submodules == -1 && option_depth))\n>> + if (option_shallow_submodules == 1)\n>> argv_array_push(&args, \"--depth=1\");\n>\n> Very late reply, since I'm just looking at this now with the --no-tags\n> opti,n, but that == 1 makes no sense anymore, and should just be `if\n> (option_shallow_submodules)` shouldn't it? I.e. this used to be an int\n> for the depth, now is a bool, but the current == 1 check is left over\n> probably from an earlier version where the depth was configurable.\n\nYes we can drop the \"== 1\" here.\n\nThanks,\nStefan\n"}]}