{"thread":{"id":"50221","subject":"[PATCH] new-workdir: Never try to recurse into submodules on the initial checkout.","startedAt":"2019-01-14T17:34:22Z","lastAt":"2019-01-15T18:38:20Z","messageCount":8,"participants":["Marc Branchaud","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"366659","messageId":"20190114172702.19959-1-marcnarc@xiplink.com","threadId":"50221","inReplyTo":null,"subject":"[PATCH] new-workdir: Never try to recurse into submodules on the initial checkout.","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2019-01-14T17:27:02Z","receivedAt":"2019-01-14T17:34:22Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"The new workdir is empty before the checkout, so attempts to recurse into\na non-existent submodule directory fail.\n\nSigned-off-by: Marc Branchaud <marcnarc@xiplink.com>\n---\n\nUntil the worktree command supports submodules I've gone back to using the\ngit-new-workdir script, but it fails if my config has\nsubmdodule.recurse=true.\n\nWith this patch, the checkout succeeds and the workdir has empty submodules,\nwhich is the script's normal behaviour.\n\n\t\tM.\n\n contrib/workdir/git-new-workdir | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/workdir/git-new-workdir b/contrib/workdir/git-new-workdir\nindex 888c34a521..5de1dc3c58 100755\n--- a/contrib/workdir/git-new-workdir\n+++ b/contrib/workdir/git-new-workdir\n@@ -102,4 +102,4 @@ trap - $siglist\n \n # checkout the branch (either the same as HEAD from the original repository,\n # or the one that was asked for)\n-git checkout -f $branch\n+git -c submodule.recurse=false checkout -f $branch\n-- \n2.20.1.1.gfb6d716d28\n\n"},{"id":"366677","messageId":"20190114213430.GC162110@google.com","threadId":"50221","inReplyTo":"20190114172702.19959-1-marcnarc@xiplink.com","subject":"Re: [PATCH] new-workdir: Never try to recurse into submodules on the initial checkout.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2019-01-14T21:34:30Z","receivedAt":"2019-01-14T21:34:35Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nMarc Branchaud wrote:\n\n> The new workdir is empty before the checkout, so attempts to recurse into\n> a non-existent submodule directory fail.\n>\n> Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>\n> ---\n\nThanks for reporting.  Can you describe the error message when it fails\nhere?\n\n> Until the worktree command supports submodules I've gone back to using the\n> git-new-workdir script, but it fails if my config has\n> submdodule.recurse=true.\n\nOh, dear.  In general, the project does a better job at supporting \"git\nworktree\" than \"git new-workdir\", but I don't blame you about this.\n\nNoting locally as another vote for getting submodules to play well with\nworktrees soon.\n\n[...]\n>  contrib/workdir/git-new-workdir | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/contrib/workdir/git-new-workdir b/contrib/workdir/git-new-workdir\n> index 888c34a521..5de1dc3c58 100755\n> --- a/contrib/workdir/git-new-workdir\n> +++ b/contrib/workdir/git-new-workdir\n> @@ -102,4 +102,4 @@ trap - $siglist\n> \n>  # checkout the branch (either the same as HEAD from the original repository,\n>  # or the one that was asked for)\n> -git checkout -f $branch\n> +git -c submodule.recurse=false checkout -f $branch\n\nnit: can this use \"git checkout --no-recurse-submodules\" instead\nof -c?\n\nIn general, we tend to recommend that kind of option instead of\n--config in scripts.\n\nThanks,\nJonathan\n"},{"id":"366679","messageId":"xmqqwon6ud7e.fsf@gitster-ct.c.googlers.com","threadId":"50221","inReplyTo":"20190114213430.GC162110@google.com","subject":"Re: [PATCH] new-workdir: Never try to recurse into submodules on the initial checkout.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-01-14T21:44:05Z","receivedAt":"2019-01-14T21:44:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n>> The new workdir is empty before the checkout, so attempts to recurse into\n>> a non-existent submodule directory fail.\n>>\n>> Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>\n>> ---\n>\n> Thanks for reporting.  Can you describe the error message when it fails\n> here?\n>\n>> Until the worktree command supports submodules I've gone back to using the\n>> git-new-workdir script, but it fails if my config has\n>> submdodule.recurse=true.\n>\n> Oh, dear.  In general, the project does a better job at supporting \"git\n> worktree\" than \"git new-workdir\", but I don't blame you about this.\n>\n> Noting locally as another vote for getting submodules to play well with\n> worktrees soon.\n>\n> [...]\n>>  contrib/workdir/git-new-workdir | 2 +-\n>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/contrib/workdir/git-new-workdir b/contrib/workdir/git-new-workdir\n>> index 888c34a521..5de1dc3c58 100755\n>> --- a/contrib/workdir/git-new-workdir\n>> +++ b/contrib/workdir/git-new-workdir\n>> @@ -102,4 +102,4 @@ trap - $siglist\n>> \n>>  # checkout the branch (either the same as HEAD from the original repository,\n>>  # or the one that was asked for)\n>> -git checkout -f $branch\n>> +git -c submodule.recurse=false checkout -f $branch\n>\n> nit: can this use \"git checkout --no-recurse-submodules\" instead\n> of -c?\n>\n> In general, we tend to recommend that kind of option instead of\n> --config in scripts.\n\nI am not sure if either approach makes sense.  Wouldn't the ideal\nendgame to allow recursive checkout if the user wants to have it,\nbut not enable it by default?\n\nStepping back a bit, if the user has recursive checkout configured\nsomewhere valid for this repository (or worktree), shouldn't the\ninitial checkout also recurse and do a \"submodule init\" if that is\nnecessary before doing so?\n\nIOW, at the point in that script where we call \"git checkout -f\", if\nwe changed it to \"git checkout --recurse-submodules -f\", what breaks\nand why?  Shouldn't it succeed instead?\n"},{"id":"366681","messageId":"20190114215804.GE162110@google.com","threadId":"50221","inReplyTo":"xmqqwon6ud7e.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] new-workdir: Never try to recurse into submodules on the initial checkout.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2019-01-14T21:58:04Z","receivedAt":"2019-01-14T21:58:09Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJunio C Hamano wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n>> Marc Branchaud wrote:\n\n>>> The new workdir is empty before the checkout, so attempts to recurse into\n>>> a non-existent submodule directory fail.\n>>>\n>>> Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>\n>>> ---\n>>\n>> Thanks for reporting.  Can you describe the error message when it fails\n>> here?\n[...]\n> IOW, at the point in that script where we call \"git checkout -f\", if\n> we changed it to \"git checkout --recurse-submodules -f\", what breaks\n> and why?  Shouldn't it succeed instead?\n\nI think that's a similar question to the one I asked.\n\nBut I have a good guess about what goes wrong.  It's related to the same\nissue as the \"git worktree\" problem Marc described.\n\nInside the superproject's $GIT_DIR, we see\n\n\tconfig\n\tmodules/\n\t\ta/\n\t\t\tconfig\n\t\tb/\n\t\t\tconfig\n\t...\n\nThe question is what to do with the modules/ directory when you have\nmultiple working directories making use of the refs and objects from\nthis $GIT_DIR.\n\nIn general, the most useful answer is that the additional working\ndirectories should make use for modules/ from this $GIT_DIR as well.\nAfter all, each submodule has its own refs and objects, and the same\nmotivation that pushes us to share the refs and objects from the\nsuperproject would drive us to share them in the submodules as well.\n\nHowever, if you do this in the most naive way, it will not work.  In\nthe config file, there is a core.worktree setting that ensures that\ncommands run from a submodule affect the correct working directory.\nWhich worktree should it point to?  All of them.\n\nThere's still an obvious \"most useful\" answer: each submodule should\ncontain its own worktrees/ directory with metadata specific to each\nworktree.  This should work fine and is the future work that Marc and\nI alluded to.  Let me call it (*), for later reference.\n\nAnything done today is papering over the sad truth that that future\nwork (*) has not been done yet.\n\ncontrib/workdir is currently naive about all this: it does *not*\nsymlink across the modules/ directory, so each workdir gets its own\nindependent copy of all the submodules.  Which kind of defeats the\npoint of this kind of setup.\n\nThat said, it's better than nothing at all, which is why Marc proposes\nmaking it not attempt to check out the submodules right away, instead\npermitting the user to make the best of things.  I suppose another\nthing that is missing is a warning message to ensure the user knows\nthat once (*) arrives, they need to be ready for it.  (Or not: this is\ncontrib/workdir, and there would be no need to make use of it in place\nof \"git worktree\" once that moment arrives.)\n\nTo reiterate, this is all about papering over (*) not having been\ndone.\n\nMarc, did I understand correctly?\n\nThanks,\nJonathan\n"},{"id":"366715","messageId":"20190115150719.19277-1-marcnarc@xiplink.com","threadId":"50221","inReplyTo":"20190114172702.19959-1-marcnarc@xiplink.com","subject":"[PATCHv2] new-workdir: Never try to recurse into submodules on the initial checkout.","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2019-01-15T15:07:19Z","receivedAt":"2019-01-15T15:07:22Z","isPatch":false,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"The new workdir is empty before the checkout, so attempts to recurse into\na non-existent submodule directory fail.\n\nSigned-off-by: Marc Branchaud <marcnarc@xiplink.com>\n---\n\nChanged to use --no-recurse-submodules instead of -c submodule.recurse=false,\nas Jonathan suggested.\n\n\t\tM.\n\n contrib/workdir/git-new-workdir | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/workdir/git-new-workdir b/contrib/workdir/git-new-workdir\nindex 888c34a521..d88765e73f 100755\n--- a/contrib/workdir/git-new-workdir\n+++ b/contrib/workdir/git-new-workdir\n@@ -102,4 +102,4 @@ trap - $siglist\n \n # checkout the branch (either the same as HEAD from the original repository,\n # or the one that was asked for)\n-git checkout -f $branch\n+git checkout --no-recurse-submodules -f $branch\n-- \n2.20.1.1.gfb6d716d28\n\n"},{"id":"366716","messageId":"0f4aadfc-6fc5-155a-3232-277298df1b54@xiplink.com","threadId":"50221","inReplyTo":"20190114215804.GE162110@google.com","subject":"Re: [PATCH] new-workdir: Never try to recurse into submodules on the initial checkout.","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2019-01-15T15:02:02Z","receivedAt":"2019-01-15T15:11:45Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"On 2019-01-14 4:58 p.m., Jonathan Nieder wrote:\n> Hi,\n> \n> Junio C Hamano wrote:\n>> Jonathan Nieder <jrnieder@gmail.com> writes:\n>>> Marc Branchaud wrote:\n> \n>>>> The new workdir is empty before the checkout, so attempts to recurse into\n>>>> a non-existent submodule directory fail.\n>>>>\n>>>> Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>\n>>>> ---\n>>>\n>>> Thanks for reporting.  Can you describe the error message when it fails\n>>> here?\n> [...]\n>> IOW, at the point in that script where we call \"git checkout -f\", if\n>> we changed it to \"git checkout --recurse-submodules -f\", what breaks\n>> and why?  Shouldn't it succeed instead?\n> \n> I think that's a similar question to the one I asked.\n> \n> But I have a good guess about what goes wrong.  It's related to the same\n> issue as the \"git worktree\" problem Marc described.\n> \n> Inside the superproject's $GIT_DIR, we see\n> \n> \tconfig\n> \tmodules/\n> \t\ta/\n> \t\t\tconfig\n> \t\tb/\n> \t\t\tconfig\n> \t...\n> \n> The question is what to do with the modules/ directory when you have\n> multiple working directories making use of the refs and objects from\n> this $GIT_DIR.\n> \n> In general, the most useful answer is that the additional working\n> directories should make use for modules/ from this $GIT_DIR as well.\n> After all, each submodule has its own refs and objects, and the same\n> motivation that pushes us to share the refs and objects from the\n> superproject would drive us to share them in the submodules as well.\n> \n> However, if you do this in the most naive way, it will not work.  In\n> the config file, there is a core.worktree setting that ensures that\n> commands run from a submodule affect the correct working directory.\n> Which worktree should it point to?  All of them.\n> \n> There's still an obvious \"most useful\" answer: each submodule should\n> contain its own worktrees/ directory with metadata specific to each\n> worktree.  This should work fine and is the future work that Marc and\n> I alluded to.  Let me call it (*), for later reference.\n> \n> Anything done today is papering over the sad truth that that future\n> work (*) has not been done yet.\n> \n> contrib/workdir is currently naive about all this: it does *not*\n> symlink across the modules/ directory, so each workdir gets its own\n> independent copy of all the submodules.  Which kind of defeats the\n> point of this kind of setup.\n> \n> That said, it's better than nothing at all, which is why Marc proposes\n> making it not attempt to check out the submodules right away, instead\n> permitting the user to make the best of things.  I suppose another\n> thing that is missing is a warning message to ensure the user knows\n> that once (*) arrives, they need to be ready for it.  (Or not: this is\n> contrib/workdir, and there would be no need to make use of it in place\n> of \"git worktree\" once that moment arrives.)\n> \n> To reiterate, this is all about papering over (*) not having been\n> done.\n> \n> Marc, did I understand correctly?\n\nYup!\n\nI just hope to keep git-new-workdir hobbling along until worktree can \nfully replace it.\n\nI agree with Junio about what should happen when submodule.recurse=true. \n  But that work should be done in worktree instead of git-new-workdir.\n\n\t\tM.\n\n> Thanks,\n> Jonathan\n> \n"},{"id":"366717","messageId":"d1e1ceb3-a1b8-ff5b-8ebc-79d2ee9267dc@xiplink.com","threadId":"50221","inReplyTo":"20190114213430.GC162110@google.com","subject":"Re: [PATCH] new-workdir: Never try to recurse into submodules on the initial checkout.","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2019-01-15T15:01:59Z","receivedAt":"2019-01-15T15:11:46Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"On 2019-01-14 4:34 p.m., Jonathan Nieder wrote:\n> Hi,\n> \n> Marc Branchaud wrote:\n> \n>> The new workdir is empty before the checkout, so attempts to recurse into\n>> a non-existent submodule directory fail.\n>>\n>> Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>\n>> ---\n> \n> Thanks for reporting.  Can you describe the error message when it fails\n> here?\n\nThe error is:\n\nfatal: exec '--super-prefix=external/submodule/': cd to \n'external/submodule' failed: No such file or directory\n\nThe created workdir has only the .git directory.  The .git/HEAD file \ncontains the expected ref, so the workdir repo's status simply shows \nthat everything has been deleted.\n\n\nNote that git-worktree also fails when submodule.recurse=true, with the \nsame error:\n\n# git worktree add ~/Code/foo/test-worktree\nPreparing worktree (new branch 'test-worktree')\nfatal: exec '--super-prefix=external/submodule/': cd to \n'external/submodule' failed: No such file or directory\nerror: Submodule 'external/submodule' could not be updated.\nerror: Submodule 'external/submodule' cannot checkout new HEAD.\nfatal: Could not reset index file to revision 'HEAD'.\n\nI had assumed that this was simply an aspect of submodules not working, \nso I was holding off reporting it until more of the submodule support \nwas complete.\n\n>> Until the worktree command supports submodules I've gone back to using the\n>> git-new-workdir script, but it fails if my config has\n>> submdodule.recurse=true.\n> \n> Oh, dear.  In general, the project does a better job at supporting \"git\n> worktree\" than \"git new-workdir\", but I don't blame you about this.\n> \n> Noting locally as another vote for getting submodules to play well with\n> worktrees soon.\n> \n> [...]\n>>   contrib/workdir/git-new-workdir | 2 +-\n>>   1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/contrib/workdir/git-new-workdir b/contrib/workdir/git-new-workdir\n>> index 888c34a521..5de1dc3c58 100755\n>> --- a/contrib/workdir/git-new-workdir\n>> +++ b/contrib/workdir/git-new-workdir\n>> @@ -102,4 +102,4 @@ trap - $siglist\n>>\n>>   # checkout the branch (either the same as HEAD from the original repository,\n>>   # or the one that was asked for)\n>> -git checkout -f $branch\n>> +git -c submodule.recurse=false checkout -f $branch\n> \n> nit: can this use \"git checkout --no-recurse-submodules\" instead\n> of -c?\n> \n> In general, we tend to recommend that kind of option instead of\n> --config in scripts.\n\n--no-recurse-submodules does work.  I'll send a v2.\n\n\t\tM.\n\n> Thanks,\n> Jonathan\n> \n"},{"id":"366731","messageId":"xmqqmuo1sr53.fsf@gitster-ct.c.googlers.com","threadId":"50221","inReplyTo":"20190114215804.GE162110@google.com","subject":"Re: [PATCH] new-workdir: Never try to recurse into submodules on the initial checkout.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-01-15T18:38:16Z","receivedAt":"2019-01-15T18:38:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> That said, it's better than nothing at all, which is why Marc proposes\n> making it not attempt to check out the submodules right away, instead\n> permitting the user to make the best of things.\n\nOK.  Then I do agree with the hardcoded \"--no-recurse\" given to \"git\ncheckout\" at the last step in the code.\n\nThanks, both.\n"}]}