{"thread":{"id":"55941","subject":"[BUG] `git reset --hard` fails with `update = none` submodules","startedAt":"2021-06-16T00:16:20Z","lastAt":"2021-07-11T16:59:55Z","messageCount":18,"participants":["Rose Kunkel","brian m. carlson","Junio C Hamano","Philippe Blain"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"427545","messageId":"CAKjYmsELpf9r3bAJj_JUHgVegw_7z2KzyuR_6FYYngpC1XmNeg@mail.gmail.com","threadId":"55941","inReplyTo":null,"subject":"[BUG] `git reset --hard` fails with `update = none` submodules","fromName":"Rose Kunkel","fromEmail":"rose@rosekunkel.me","sentAt":"2021-06-16T00:16:06Z","receivedAt":"2021-06-16T00:16:20Z","isPatch":false,"sender":{"key":"rose@rosekunkel.me","avatar":null},"body":"# What did you do before the bug happened? (Steps to reproduce your issue)\n1. Clone a git repository that sets `update = none` in .gitmodules:\n$ git clone --recurse-submodules https://github.com/ubolonton/tree-sitter-langs\n\n2. Perform a hard reset:\n$ cd tree-sitter-langs\n$ git reset --hard\n\n# What did you expect to happen? (Expected behavior)\nThe reset should succeed and do nothing.\n\n# What happened instead? (Actual behavior)\nThe reset command fails with\n```\nfatal: not a git repository: ../../.git/modules/repos/agda\nfatal: could not reset submodule index\n```\n\n[System Info]\ngit version:\ngit version 2.32.0\ncpu: x86_64\nno commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nuname: Linux 5.12.10-hardened1-1-hardened #1 SMP PREEMPT Thu, 10 Jun\n2021 21:12:42 +0000 x86_64\ncompiler info: gnuc: 11.1\nlibc info: glibc: 2.33\n$SHELL (typically, interactive shell): /usr/bin/zsh\n"},{"id":"427553","messageId":"YMlLGwScowX8eXeJ@camp.crustytoothpaste.net","threadId":"55941","inReplyTo":"CAKjYmsELpf9r3bAJj_JUHgVegw_7z2KzyuR_6FYYngpC1XmNeg@mail.gmail.com","subject":"Re: [BUG] `git reset --hard` fails with `update = none` submodules","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-06-16T00:51:39Z","receivedAt":"2021-06-16T00:52:16Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-06-16 at 00:16:06, Rose Kunkel wrote:\n> # What did you do before the bug happened? (Steps to reproduce your issue)\n> 1. Clone a git repository that sets `update = none` in .gitmodules:\n> $ git clone --recurse-submodules https://github.com/ubolonton/tree-sitter-langs\n> \n> 2. Perform a hard reset:\n> $ cd tree-sitter-langs\n> $ git reset --hard\n> \n> # What did you expect to happen? (Expected behavior)\n> The reset should succeed and do nothing.\n\nI think we're in agreement on this.  This should be a fresh clone and so\na hard reset should change nothing.\n\n> # What happened instead? (Actual behavior)\n> The reset command fails with\n> ```\n> fatal: not a git repository: ../../.git/modules/repos/agda\n> fatal: could not reset submodule index\n> ```\n\nHmmm, I can't reproduce this behavior.  What I see is this:\n\n  $ git reset --hard\n  HEAD is now at 5d362ce Release 0.10.0\n\nI'm running git version 2.32.0.272.g935e593368 on Debian sid (with the\nexperimental packages).\n\nCan you try the clone and run a \"git status\" command in the repository\nto see if anything is modified after your clone?  Are the submodules\nchecked out when you perform the clone?  In my case, I see lines like\nthis:\n\n  Skipping submodule 'repos/agda'\n\nIf you're seeing something different, then that might contribute to the\ndifferent behavior we're seeing.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"427555","messageId":"CAKjYmsHD2MuTE+drebKichz_0wquPN8ZTLbiPkUCZJyLsSFh8Q@mail.gmail.com","threadId":"55941","inReplyTo":"YMlLGwScowX8eXeJ@camp.crustytoothpaste.net","subject":"Re: [BUG] `git reset --hard` fails with `update = none` submodules","fromName":"Rose Kunkel","fromEmail":"rose@rosekunkel.me","sentAt":"2021-06-16T00:57:18Z","receivedAt":"2021-06-16T00:57:31Z","isPatch":false,"sender":{"key":"rose@rosekunkel.me","avatar":null},"body":"Running `git status` in the resulting repository gives\n```\nOn branch master\nYour branch is up to date with 'origin/master'.\n\nnothing to commit, working tree clean\n```\n\nThis is the output from the clone command:\n```\nCloning into 'tree-sitter-langs'...\nremote: Enumerating objects: 609, done.\nremote: Counting objects: 100% (83/83), done.\nremote: Compressing objects: 100% (52/52), done.\nremote: Total 609 (delta 40), reused 58 (delta 24), pack-reused 526\nReceiving objects: 100% (609/609), 117.17 KiB | 1.05 MiB/s, done.\nResolving deltas: 100% (322/322), done.\nSubmodule 'repos/agda'\n(https://github.com/tree-sitter/tree-sitter-agda) registered for path\n'repos/agda'\nSubmodule 'repos/bash'\n(https://github.com/tree-sitter/tree-sitter-bash) registered for path\n'repos/bash'\nSubmodule 'repos/c' (https://github.com/tree-sitter/tree-sitter-c)\nregistered for path 'repos/c'\nSubmodule 'repos/c-sharp'\n(https://github.com/tree-sitter/tree-sitter-c-sharp) registered for\npath 'repos/c-sharp'\nSubmodule 'repos/cpp' (https://github.com/tree-sitter/tree-sitter-cpp)\nregistered for path 'repos/cpp'\nSubmodule 'repos/css' (https://github.com/tree-sitter/tree-sitter-css)\nregistered for path 'repos/css'\nSubmodule 'repos/elm' (https://github.com/razzeee/tree-sitter-elm)\nregistered for path 'repos/elm'\nSubmodule 'repos/fluent'\n(https://github.com/tree-sitter/tree-sitter-fluent) registered for\npath 'repos/fluent'\nSubmodule 'repos/go' (https://github.com/tree-sitter/tree-sitter-go)\nregistered for path 'repos/go'\nSubmodule 'repos/html'\n(https://github.com/tree-sitter/tree-sitter-html) registered for path\n'repos/html'\nSubmodule 'repos/janet-simple'\n(https://codeberg.org/sogaiu/tree-sitter-janet-simple) registered for\npath 'repos/janet-simple'\nSubmodule 'repos/java'\n(https://github.com/tree-sitter/tree-sitter-java) registered for path\n'repos/java'\nSubmodule 'repos/javascript'\n(https://github.com/tree-sitter/tree-sitter-javascript) registered for\npath 'repos/javascript'\nSubmodule 'repos/jsdoc'\n(https://github.com/tree-sitter/tree-sitter-jsdoc) registered for path\n'repos/jsdoc'\nSubmodule 'repos/json'\n(https://github.com/tree-sitter/tree-sitter-json) registered for path\n'repos/json'\nSubmodule 'repos/julia'\n(https://github.com/tree-sitter/tree-sitter-julia) registered for path\n'repos/julia'\nSubmodule 'repos/ocaml'\n(https://github.com/tree-sitter/tree-sitter-ocaml) registered for path\n'repos/ocaml'\nSubmodule 'repos/php' (https://github.com/tree-sitter/tree-sitter-php)\nregistered for path 'repos/php'\nSubmodule 'repos/python'\n(https://github.com/tree-sitter/tree-sitter-python) registered for\npath 'repos/python'\nSubmodule 'repos/ruby'\n(https://github.com/tree-sitter/tree-sitter-ruby) registered for path\n'repos/ruby'\nSubmodule 'repos/rust'\n(https://github.com/tree-sitter/tree-sitter-rust) registered for path\n'repos/rust'\nSubmodule 'repos/scala'\n(https://github.com/tree-sitter/tree-sitter-scala) registered for path\n'repos/scala'\nSubmodule 'repos/swift'\n(https://github.com/tree-sitter/tree-sitter-swift) registered for path\n'repos/swift'\nSubmodule 'repos/typescript'\n(https://github.com/tree-sitter/tree-sitter-typescript) registered for\npath 'repos/typescript'\nSkipping submodule 'repos/agda'\nSkipping submodule 'repos/bash'\nSkipping submodule 'repos/c'\nSkipping submodule 'repos/c-sharp'\nSkipping submodule 'repos/cpp'\nSkipping submodule 'repos/css'\nSkipping submodule 'repos/elm'\nSkipping submodule 'repos/fluent'\nSkipping submodule 'repos/go'\nSkipping submodule 'repos/html'\nSkipping submodule 'repos/janet-simple'\nSkipping submodule 'repos/java'\nSkipping submodule 'repos/javascript'\nSkipping submodule 'repos/jsdoc'\nSkipping submodule 'repos/json'\nSkipping submodule 'repos/julia'\nSkipping submodule 'repos/ocaml'\nSkipping submodule 'repos/php'\nSkipping submodule 'repos/python'\nSkipping submodule 'repos/ruby'\nSkipping submodule 'repos/rust'\nSkipping submodule 'repos/scala'\nSkipping submodule 'repos/swift'\nSkipping submodule 'repos/typescript'\n```\n\nOn Tue, Jun 15, 2021 at 5:51 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n>\n> On 2021-06-16 at 00:16:06, Rose Kunkel wrote:\n> > # What did you do before the bug happened? (Steps to reproduce your issue)\n> > 1. Clone a git repository that sets `update = none` in .gitmodules:\n> > $ git clone --recurse-submodules https://github.com/ubolonton/tree-sitter-langs\n> >\n> > 2. Perform a hard reset:\n> > $ cd tree-sitter-langs\n> > $ git reset --hard\n> >\n> > # What did you expect to happen? (Expected behavior)\n> > The reset should succeed and do nothing.\n>\n> I think we're in agreement on this.  This should be a fresh clone and so\n> a hard reset should change nothing.\n>\n> > # What happened instead? (Actual behavior)\n> > The reset command fails with\n> > ```\n> > fatal: not a git repository: ../../.git/modules/repos/agda\n> > fatal: could not reset submodule index\n> > ```\n>\n> Hmmm, I can't reproduce this behavior.  What I see is this:\n>\n>   $ git reset --hard\n>   HEAD is now at 5d362ce Release 0.10.0\n>\n> I'm running git version 2.32.0.272.g935e593368 on Debian sid (with the\n> experimental packages).\n>\n> Can you try the clone and run a \"git status\" command in the repository\n> to see if anything is modified after your clone?  Are the submodules\n> checked out when you perform the clone?  In my case, I see lines like\n> this:\n>\n>   Skipping submodule 'repos/agda'\n>\n> If you're seeing something different, then that might contribute to the\n> different behavior we're seeing.\n> --\n> brian m. carlson (he/him or they/them)\n> Toronto, Ontario, CA\n"},{"id":"427556","messageId":"CAKjYmsEHWShuKEOFWnCyU1x5rM7kFrcaN78D7KhhUay8kCvA2g@mail.gmail.com","threadId":"55941","inReplyTo":"CAKjYmsHD2MuTE+drebKichz_0wquPN8ZTLbiPkUCZJyLsSFh8Q@mail.gmail.com","subject":"Re: [BUG] `git reset --hard` fails with `update = none` submodules","fromName":"Rose Kunkel","fromEmail":"rose@rosekunkel.me","sentAt":"2021-06-16T01:03:40Z","receivedAt":"2021-06-16T01:03:52Z","isPatch":false,"sender":{"key":"rose@rosekunkel.me","avatar":null},"body":"Potentially relevant: `git config --global --list` shows\n```\nstatus.showstash=true\nstatus.submodulesummary=true\nsubmodule.recurse=true\nuser.name=Rose Kunkel\nuser.email=rose@rosekunkel.me\npull.rebase=false\ninit.defaultbranch=main\n```\n\nOn Tue, Jun 15, 2021 at 5:57 PM Rose Kunkel <rose@rosekunkel.me> wrote:\n>\n> Running `git status` in the resulting repository gives\n> ```\n> On branch master\n> Your branch is up to date with 'origin/master'.\n>\n> nothing to commit, working tree clean\n> ```\n>\n> This is the output from the clone command:\n> ```\n> Cloning into 'tree-sitter-langs'...\n> remote: Enumerating objects: 609, done.\n> remote: Counting objects: 100% (83/83), done.\n> remote: Compressing objects: 100% (52/52), done.\n> remote: Total 609 (delta 40), reused 58 (delta 24), pack-reused 526\n> Receiving objects: 100% (609/609), 117.17 KiB | 1.05 MiB/s, done.\n> Resolving deltas: 100% (322/322), done.\n> Submodule 'repos/agda'\n> (https://github.com/tree-sitter/tree-sitter-agda) registered for path\n> 'repos/agda'\n> Submodule 'repos/bash'\n> (https://github.com/tree-sitter/tree-sitter-bash) registered for path\n> 'repos/bash'\n> Submodule 'repos/c' (https://github.com/tree-sitter/tree-sitter-c)\n> registered for path 'repos/c'\n> Submodule 'repos/c-sharp'\n> (https://github.com/tree-sitter/tree-sitter-c-sharp) registered for\n> path 'repos/c-sharp'\n> Submodule 'repos/cpp' (https://github.com/tree-sitter/tree-sitter-cpp)\n> registered for path 'repos/cpp'\n> Submodule 'repos/css' (https://github.com/tree-sitter/tree-sitter-css)\n> registered for path 'repos/css'\n> Submodule 'repos/elm' (https://github.com/razzeee/tree-sitter-elm)\n> registered for path 'repos/elm'\n> Submodule 'repos/fluent'\n> (https://github.com/tree-sitter/tree-sitter-fluent) registered for\n> path 'repos/fluent'\n> Submodule 'repos/go' (https://github.com/tree-sitter/tree-sitter-go)\n> registered for path 'repos/go'\n> Submodule 'repos/html'\n> (https://github.com/tree-sitter/tree-sitter-html) registered for path\n> 'repos/html'\n> Submodule 'repos/janet-simple'\n> (https://codeberg.org/sogaiu/tree-sitter-janet-simple) registered for\n> path 'repos/janet-simple'\n> Submodule 'repos/java'\n> (https://github.com/tree-sitter/tree-sitter-java) registered for path\n> 'repos/java'\n> Submodule 'repos/javascript'\n> (https://github.com/tree-sitter/tree-sitter-javascript) registered for\n> path 'repos/javascript'\n> Submodule 'repos/jsdoc'\n> (https://github.com/tree-sitter/tree-sitter-jsdoc) registered for path\n> 'repos/jsdoc'\n> Submodule 'repos/json'\n> (https://github.com/tree-sitter/tree-sitter-json) registered for path\n> 'repos/json'\n> Submodule 'repos/julia'\n> (https://github.com/tree-sitter/tree-sitter-julia) registered for path\n> 'repos/julia'\n> Submodule 'repos/ocaml'\n> (https://github.com/tree-sitter/tree-sitter-ocaml) registered for path\n> 'repos/ocaml'\n> Submodule 'repos/php' (https://github.com/tree-sitter/tree-sitter-php)\n> registered for path 'repos/php'\n> Submodule 'repos/python'\n> (https://github.com/tree-sitter/tree-sitter-python) registered for\n> path 'repos/python'\n> Submodule 'repos/ruby'\n> (https://github.com/tree-sitter/tree-sitter-ruby) registered for path\n> 'repos/ruby'\n> Submodule 'repos/rust'\n> (https://github.com/tree-sitter/tree-sitter-rust) registered for path\n> 'repos/rust'\n> Submodule 'repos/scala'\n> (https://github.com/tree-sitter/tree-sitter-scala) registered for path\n> 'repos/scala'\n> Submodule 'repos/swift'\n> (https://github.com/tree-sitter/tree-sitter-swift) registered for path\n> 'repos/swift'\n> Submodule 'repos/typescript'\n> (https://github.com/tree-sitter/tree-sitter-typescript) registered for\n> path 'repos/typescript'\n> Skipping submodule 'repos/agda'\n> Skipping submodule 'repos/bash'\n> Skipping submodule 'repos/c'\n> Skipping submodule 'repos/c-sharp'\n> Skipping submodule 'repos/cpp'\n> Skipping submodule 'repos/css'\n> Skipping submodule 'repos/elm'\n> Skipping submodule 'repos/fluent'\n> Skipping submodule 'repos/go'\n> Skipping submodule 'repos/html'\n> Skipping submodule 'repos/janet-simple'\n> Skipping submodule 'repos/java'\n> Skipping submodule 'repos/javascript'\n> Skipping submodule 'repos/jsdoc'\n> Skipping submodule 'repos/json'\n> Skipping submodule 'repos/julia'\n> Skipping submodule 'repos/ocaml'\n> Skipping submodule 'repos/php'\n> Skipping submodule 'repos/python'\n> Skipping submodule 'repos/ruby'\n> Skipping submodule 'repos/rust'\n> Skipping submodule 'repos/scala'\n> Skipping submodule 'repos/swift'\n> Skipping submodule 'repos/typescript'\n> ```\n>\n> On Tue, Jun 15, 2021 at 5:51 PM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n> >\n> > On 2021-06-16 at 00:16:06, Rose Kunkel wrote:\n> > > # What did you do before the bug happened? (Steps to reproduce your issue)\n> > > 1. Clone a git repository that sets `update = none` in .gitmodules:\n> > > $ git clone --recurse-submodules https://github.com/ubolonton/tree-sitter-langs\n> > >\n> > > 2. Perform a hard reset:\n> > > $ cd tree-sitter-langs\n> > > $ git reset --hard\n> > >\n> > > # What did you expect to happen? (Expected behavior)\n> > > The reset should succeed and do nothing.\n> >\n> > I think we're in agreement on this.  This should be a fresh clone and so\n> > a hard reset should change nothing.\n> >\n> > > # What happened instead? (Actual behavior)\n> > > The reset command fails with\n> > > ```\n> > > fatal: not a git repository: ../../.git/modules/repos/agda\n> > > fatal: could not reset submodule index\n> > > ```\n> >\n> > Hmmm, I can't reproduce this behavior.  What I see is this:\n> >\n> >   $ git reset --hard\n> >   HEAD is now at 5d362ce Release 0.10.0\n> >\n> > I'm running git version 2.32.0.272.g935e593368 on Debian sid (with the\n> > experimental packages).\n> >\n> > Can you try the clone and run a \"git status\" command in the repository\n> > to see if anything is modified after your clone?  Are the submodules\n> > checked out when you perform the clone?  In my case, I see lines like\n> > this:\n> >\n> >   Skipping submodule 'repos/agda'\n> >\n> > If you're seeing something different, then that might contribute to the\n> > different behavior we're seeing.\n> > --\n> > brian m. carlson (he/him or they/them)\n> > Toronto, Ontario, CA\n"},{"id":"427557","messageId":"CAKjYmsFY4+jdvQ5T1UZmXpqvDrgTdy-vF5RD89dFkTqYbB-Ujg@mail.gmail.com","threadId":"55941","inReplyTo":"CAKjYmsEHWShuKEOFWnCyU1x5rM7kFrcaN78D7KhhUay8kCvA2g@mail.gmail.com","subject":"Re: [BUG] `git reset --hard` fails with `update = none` submodules","fromName":"Rose Kunkel","fromEmail":"rose@rosekunkel.me","sentAt":"2021-06-16T01:15:43Z","receivedAt":"2021-06-16T01:15:57Z","isPatch":false,"sender":{"key":"rose@rosekunkel.me","avatar":null},"body":"If I set `submodule.recurse = false` in .git/config, I get the\nbehavior you're seeing.\n\nOn Tue, Jun 15, 2021 at 6:03 PM Rose Kunkel <rose@rosekunkel.me> wrote:\n>\n> Potentially relevant: `git config --global --list` shows\n> ```\n> status.showstash=true\n> status.submodulesummary=true\n> submodule.recurse=true\n> user.name=Rose Kunkel\n> user.email=rose@rosekunkel.me\n> pull.rebase=false\n> init.defaultbranch=main\n> ```\n>\n> On Tue, Jun 15, 2021 at 5:57 PM Rose Kunkel <rose@rosekunkel.me> wrote:\n> >\n> > Running `git status` in the resulting repository gives\n> > ```\n> > On branch master\n> > Your branch is up to date with 'origin/master'.\n> >\n> > nothing to commit, working tree clean\n> > ```\n> >\n> > This is the output from the clone command:\n> > ```\n> > Cloning into 'tree-sitter-langs'...\n> > remote: Enumerating objects: 609, done.\n> > remote: Counting objects: 100% (83/83), done.\n> > remote: Compressing objects: 100% (52/52), done.\n> > remote: Total 609 (delta 40), reused 58 (delta 24), pack-reused 526\n> > Receiving objects: 100% (609/609), 117.17 KiB | 1.05 MiB/s, done.\n> > Resolving deltas: 100% (322/322), done.\n> > Submodule 'repos/agda'\n> > (https://github.com/tree-sitter/tree-sitter-agda) registered for path\n> > 'repos/agda'\n> > Submodule 'repos/bash'\n> > (https://github.com/tree-sitter/tree-sitter-bash) registered for path\n> > 'repos/bash'\n> > Submodule 'repos/c' (https://github.com/tree-sitter/tree-sitter-c)\n> > registered for path 'repos/c'\n> > Submodule 'repos/c-sharp'\n> > (https://github.com/tree-sitter/tree-sitter-c-sharp) registered for\n> > path 'repos/c-sharp'\n> > Submodule 'repos/cpp' (https://github.com/tree-sitter/tree-sitter-cpp)\n> > registered for path 'repos/cpp'\n> > Submodule 'repos/css' (https://github.com/tree-sitter/tree-sitter-css)\n> > registered for path 'repos/css'\n> > Submodule 'repos/elm' (https://github.com/razzeee/tree-sitter-elm)\n> > registered for path 'repos/elm'\n> > Submodule 'repos/fluent'\n> > (https://github.com/tree-sitter/tree-sitter-fluent) registered for\n> > path 'repos/fluent'\n> > Submodule 'repos/go' (https://github.com/tree-sitter/tree-sitter-go)\n> > registered for path 'repos/go'\n> > Submodule 'repos/html'\n> > (https://github.com/tree-sitter/tree-sitter-html) registered for path\n> > 'repos/html'\n> > Submodule 'repos/janet-simple'\n> > (https://codeberg.org/sogaiu/tree-sitter-janet-simple) registered for\n> > path 'repos/janet-simple'\n> > Submodule 'repos/java'\n> > (https://github.com/tree-sitter/tree-sitter-java) registered for path\n> > 'repos/java'\n> > Submodule 'repos/javascript'\n> > (https://github.com/tree-sitter/tree-sitter-javascript) registered for\n> > path 'repos/javascript'\n> > Submodule 'repos/jsdoc'\n> > (https://github.com/tree-sitter/tree-sitter-jsdoc) registered for path\n> > 'repos/jsdoc'\n> > Submodule 'repos/json'\n> > (https://github.com/tree-sitter/tree-sitter-json) registered for path\n> > 'repos/json'\n> > Submodule 'repos/julia'\n> > (https://github.com/tree-sitter/tree-sitter-julia) registered for path\n> > 'repos/julia'\n> > Submodule 'repos/ocaml'\n> > (https://github.com/tree-sitter/tree-sitter-ocaml) registered for path\n> > 'repos/ocaml'\n> > Submodule 'repos/php' (https://github.com/tree-sitter/tree-sitter-php)\n> > registered for path 'repos/php'\n> > Submodule 'repos/python'\n> > (https://github.com/tree-sitter/tree-sitter-python) registered for\n> > path 'repos/python'\n> > Submodule 'repos/ruby'\n> > (https://github.com/tree-sitter/tree-sitter-ruby) registered for path\n> > 'repos/ruby'\n> > Submodule 'repos/rust'\n> > (https://github.com/tree-sitter/tree-sitter-rust) registered for path\n> > 'repos/rust'\n> > Submodule 'repos/scala'\n> > (https://github.com/tree-sitter/tree-sitter-scala) registered for path\n> > 'repos/scala'\n> > Submodule 'repos/swift'\n> > (https://github.com/tree-sitter/tree-sitter-swift) registered for path\n> > 'repos/swift'\n> > Submodule 'repos/typescript'\n> > (https://github.com/tree-sitter/tree-sitter-typescript) registered for\n> > path 'repos/typescript'\n> > Skipping submodule 'repos/agda'\n> > Skipping submodule 'repos/bash'\n> > Skipping submodule 'repos/c'\n> > Skipping submodule 'repos/c-sharp'\n> > Skipping submodule 'repos/cpp'\n> > Skipping submodule 'repos/css'\n> > Skipping submodule 'repos/elm'\n> > Skipping submodule 'repos/fluent'\n> > Skipping submodule 'repos/go'\n> > Skipping submodule 'repos/html'\n> > Skipping submodule 'repos/janet-simple'\n> > Skipping submodule 'repos/java'\n> > Skipping submodule 'repos/javascript'\n> > Skipping submodule 'repos/jsdoc'\n> > Skipping submodule 'repos/json'\n> > Skipping submodule 'repos/julia'\n> > Skipping submodule 'repos/ocaml'\n> > Skipping submodule 'repos/php'\n> > Skipping submodule 'repos/python'\n> > Skipping submodule 'repos/ruby'\n> > Skipping submodule 'repos/rust'\n> > Skipping submodule 'repos/scala'\n> > Skipping submodule 'repos/swift'\n> > Skipping submodule 'repos/typescript'\n> > ```\n> >\n> > On Tue, Jun 15, 2021 at 5:51 PM brian m. carlson\n> > <sandals@crustytoothpaste.net> wrote:\n> > >\n> > > On 2021-06-16 at 00:16:06, Rose Kunkel wrote:\n> > > > # What did you do before the bug happened? (Steps to reproduce your issue)\n> > > > 1. Clone a git repository that sets `update = none` in .gitmodules:\n> > > > $ git clone --recurse-submodules https://github.com/ubolonton/tree-sitter-langs\n> > > >\n> > > > 2. Perform a hard reset:\n> > > > $ cd tree-sitter-langs\n> > > > $ git reset --hard\n> > > >\n> > > > # What did you expect to happen? (Expected behavior)\n> > > > The reset should succeed and do nothing.\n> > >\n> > > I think we're in agreement on this.  This should be a fresh clone and so\n> > > a hard reset should change nothing.\n> > >\n> > > > # What happened instead? (Actual behavior)\n> > > > The reset command fails with\n> > > > ```\n> > > > fatal: not a git repository: ../../.git/modules/repos/agda\n> > > > fatal: could not reset submodule index\n> > > > ```\n> > >\n> > > Hmmm, I can't reproduce this behavior.  What I see is this:\n> > >\n> > >   $ git reset --hard\n> > >   HEAD is now at 5d362ce Release 0.10.0\n> > >\n> > > I'm running git version 2.32.0.272.g935e593368 on Debian sid (with the\n> > > experimental packages).\n> > >\n> > > Can you try the clone and run a \"git status\" command in the repository\n> > > to see if anything is modified after your clone?  Are the submodules\n> > > checked out when you perform the clone?  In my case, I see lines like\n> > > this:\n> > >\n> > >   Skipping submodule 'repos/agda'\n> > >\n> > > If you're seeing something different, then that might contribute to the\n> > > different behavior we're seeing.\n> > > --\n> > > brian m. carlson (he/him or they/them)\n> > > Toronto, Ontario, CA\n"},{"id":"427559","messageId":"YMlS+1F9IND7vxNI@camp.crustytoothpaste.net","threadId":"55941","inReplyTo":"CAKjYmsEHWShuKEOFWnCyU1x5rM7kFrcaN78D7KhhUay8kCvA2g@mail.gmail.com","subject":"Re: [BUG] `git reset --hard` fails with `update = none` submodules","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-06-16T01:25:15Z","receivedAt":"2021-06-16T01:25:20Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-06-16 at 01:03:40, Rose Kunkel wrote:\n> Potentially relevant: `git config --global --list` shows\n> ```\n> status.showstash=true\n> status.submodulesummary=true\n> submodule.recurse=true\n\nThanks for this additional information.  This line is the critical\npiece.  Now I get this:\n\n  $ git reset --hard\n  fatal: not a git repository: ../../.git/modules/repos/agda\n  fatal: could not reset submodule index\n\nPredictably, \"git -c submodules.recurse=true reset --hard\" also results\nin the same thing.\n\nThe --recurse-submodules option for git reset says this (emphasis mine):\n\n  When the working tree is updated, using --recurse-submodules will also\n  recursively reset the working tree of all *active* submodules\n  according to the commit recorded in the superproject, also setting the\n  submodules' HEAD to be detached at that commit.\n\nOn my system, .git/config has this:\n\n  [submodule]\n          active = .\n\nSo these submodules are active, but they probably should not be, since\nwe haven't checked anything out (or, for that matter, cloned any data)\nand it wouldn't make sense to try to operate on them automatically with\nsubmodules.recurse or --recurse-submodules.\n\nMy gut tells me that we should probably mark submodules with update=none\nset on a clone as inactive.  Of course, this is a tricky area that I'm\nnot super familiar with, so opinions or thoughts are welcome.\n\nIf folks think this is a good way forward, I'll look into writing a\npatch, probably tomorrow evening since it's starting to get late here.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"427561","messageId":"CAKjYmsFVfMW4DKz4cA_fNugGim4m+-aJgOW+k0za+T-D8YHc7g@mail.gmail.com","threadId":"55941","inReplyTo":"YMlS+1F9IND7vxNI@camp.crustytoothpaste.net","subject":"Re: [BUG] `git reset --hard` fails with `update = none` submodules","fromName":"Rose Kunkel","fromEmail":"rose@rosekunkel.me","sentAt":"2021-06-16T01:39:55Z","receivedAt":"2021-06-16T01:40:08Z","isPatch":false,"sender":{"key":"rose@rosekunkel.me","avatar":null},"body":"That sounds reasonable to me.\n\nI do think it's pretty unintuitive that `update = none` means that\nsubmodules never get initialized, even with an explicit `git submodule\ninit` command. If this is intended behavior, it should be better\ndocumented. If not, fixing that would also fix this bug.\n\nOn Tue, Jun 15, 2021 at 6:25 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n>\n> On 2021-06-16 at 01:03:40, Rose Kunkel wrote:\n> > Potentially relevant: `git config --global --list` shows\n> > ```\n> > status.showstash=true\n> > status.submodulesummary=true\n> > submodule.recurse=true\n>\n> Thanks for this additional information.  This line is the critical\n> piece.  Now I get this:\n>\n>   $ git reset --hard\n>   fatal: not a git repository: ../../.git/modules/repos/agda\n>   fatal: could not reset submodule index\n>\n> Predictably, \"git -c submodules.recurse=true reset --hard\" also results\n> in the same thing.\n>\n> The --recurse-submodules option for git reset says this (emphasis mine):\n>\n>   When the working tree is updated, using --recurse-submodules will also\n>   recursively reset the working tree of all *active* submodules\n>   according to the commit recorded in the superproject, also setting the\n>   submodules' HEAD to be detached at that commit.\n>\n> On my system, .git/config has this:\n>\n>   [submodule]\n>           active = .\n>\n> So these submodules are active, but they probably should not be, since\n> we haven't checked anything out (or, for that matter, cloned any data)\n> and it wouldn't make sense to try to operate on them automatically with\n> submodules.recurse or --recurse-submodules.\n>\n> My gut tells me that we should probably mark submodules with update=none\n> set on a clone as inactive.  Of course, this is a tricky area that I'm\n> not super familiar with, so opinions or thoughts are welcome.\n>\n> If folks think this is a good way forward, I'll look into writing a\n> patch, probably tomorrow evening since it's starting to get late here.\n> --\n> brian m. carlson (he/him or they/them)\n> Toronto, Ontario, CA\n"},{"id":"427563","messageId":"CAKjYmsEjizYapQVy+pzm9mOxS=0EBwEgzqsO1xO-ct-1AdYuZQ@mail.gmail.com","threadId":"55941","inReplyTo":"CAKjYmsFVfMW4DKz4cA_fNugGim4m+-aJgOW+k0za+T-D8YHc7g@mail.gmail.com","subject":"Re: [BUG] `git reset --hard` fails with `update = none` submodules","fromName":"Rose Kunkel","fromEmail":"rose@rosekunkel.me","sentAt":"2021-06-16T01:46:23Z","receivedAt":"2021-06-16T01:46:37Z","isPatch":false,"sender":{"key":"rose@rosekunkel.me","avatar":null},"body":"Actually, nevermind that last comment. I misunderstood what `git\nsubmodule init` does. I thought it cloned missing submodules, but\napparently that's done by `git submodule update`, so the behavior does\nmake sense.\n\nOn Tue, Jun 15, 2021 at 6:39 PM Rose Kunkel <rose@rosekunkel.me> wrote:\n>\n> That sounds reasonable to me.\n>\n> I do think it's pretty unintuitive that `update = none` means that\n> submodules never get initialized, even with an explicit `git submodule\n> init` command. If this is intended behavior, it should be better\n> documented. If not, fixing that would also fix this bug.\n>\n> On Tue, Jun 15, 2021 at 6:25 PM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n> >\n> > On 2021-06-16 at 01:03:40, Rose Kunkel wrote:\n> > > Potentially relevant: `git config --global --list` shows\n> > > ```\n> > > status.showstash=true\n> > > status.submodulesummary=true\n> > > submodule.recurse=true\n> >\n> > Thanks for this additional information.  This line is the critical\n> > piece.  Now I get this:\n> >\n> >   $ git reset --hard\n> >   fatal: not a git repository: ../../.git/modules/repos/agda\n> >   fatal: could not reset submodule index\n> >\n> > Predictably, \"git -c submodules.recurse=true reset --hard\" also results\n> > in the same thing.\n> >\n> > The --recurse-submodules option for git reset says this (emphasis mine):\n> >\n> >   When the working tree is updated, using --recurse-submodules will also\n> >   recursively reset the working tree of all *active* submodules\n> >   according to the commit recorded in the superproject, also setting the\n> >   submodules' HEAD to be detached at that commit.\n> >\n> > On my system, .git/config has this:\n> >\n> >   [submodule]\n> >           active = .\n> >\n> > So these submodules are active, but they probably should not be, since\n> > we haven't checked anything out (or, for that matter, cloned any data)\n> > and it wouldn't make sense to try to operate on them automatically with\n> > submodules.recurse or --recurse-submodules.\n> >\n> > My gut tells me that we should probably mark submodules with update=none\n> > set on a clone as inactive.  Of course, this is a tricky area that I'm\n> > not super familiar with, so opinions or thoughts are welcome.\n> >\n> > If folks think this is a good way forward, I'll look into writing a\n> > patch, probably tomorrow evening since it's starting to get late here.\n> > --\n> > brian m. carlson (he/him or they/them)\n> > Toronto, Ontario, CA\n"},{"id":"427568","messageId":"xmqqeed2sdwc.fsf@gitster.g","threadId":"55941","inReplyTo":"YMlS+1F9IND7vxNI@camp.crustytoothpaste.net","subject":"Re: [BUG] `git reset --hard` fails with `update = none` submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-16T03:10:27Z","receivedAt":"2021-06-16T03:10:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> On 2021-06-16 at 01:03:40, Rose Kunkel wrote:\n>> Potentially relevant: `git config --global --list` shows\n>> ```\n>> status.showstash=true\n>> status.submodulesummary=true\n>> submodule.recurse=true\n>\n> Thanks for this additional information.  This line is the critical\n> piece.  Now I get this:\n>\n>   $ git reset --hard\n>   fatal: not a git repository: ../../.git/modules/repos/agda\n>   fatal: could not reset submodule index\n>\n> Predictably, \"git -c submodules.recurse=true reset --hard\" also results\n> in the same thing.\n>\n> The --recurse-submodules option for git reset says this (emphasis mine):\n>\n>   When the working tree is updated, using --recurse-submodules will also\n>   recursively reset the working tree of all *active* submodules\n>   according to the commit recorded in the superproject, also setting the\n>   submodules' HEAD to be detached at that commit.\n>\n> On my system, .git/config has this:\n>\n>   [submodule]\n>           active = .\n>\n> So these submodules are active, but they probably should not be, since\n> we haven't checked anything out (or, for that matter, cloned any data)\n> and it wouldn't make sense to try to operate on them automatically with\n> submodules.recurse or --recurse-submodules.\n>\n> My gut tells me that we should probably mark submodules with update=none\n> set on a clone as inactive.  Of course, this is a tricky area that I'm\n> not super familiar with, so opinions or thoughts are welcome.\n>\n> If folks think this is a good way forward, I'll look into writing a\n> patch, probably tomorrow evening since it's starting to get late here.\n\nCc'ing some folks who recently mumbled the word \"submodule\" in their\nrecent topics for input.\n\nThanks.\n"},{"id":"427609","messageId":"3b53846d-7611-521a-274c-473e00266529@gmail.com","threadId":"55941","inReplyTo":"xmqqeed2sdwc.fsf@gitster.g","subject":"Re: [BUG] `git reset --hard` fails with `update = none` submodules","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2021-06-16T13:20:44Z","receivedAt":"2021-06-16T13:20:49Z","isPatch":false,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi all,\n\nLe 2021-06-15 à 23:10, Junio C Hamano a écrit :\n> \"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n> \n>> On 2021-06-16 at 01:03:40, Rose Kunkel wrote:\n>>> Potentially relevant: `git config --global --list` shows\n>>> ```\n>>> status.showstash=true\n>>> status.submodulesummary=true\n>>> submodule.recurse=true\n>>\n>> Thanks for this additional information.  This line is the critical\n>> piece.  Now I get this:\n>>\n>>    $ git reset --hard\n>>    fatal: not a git repository: ../../.git/modules/repos/agda\n>>    fatal: could not reset submodule index\n>>\n>> Predictably, \"git -c submodules.recurse=true reset --hard\" also results\n>> in the same thing.\n>>\n>> The --recurse-submodules option for git reset says this (emphasis mine):\n>>\n>>    When the working tree is updated, using --recurse-submodules will also\n>>    recursively reset the working tree of all *active* submodules\n>>    according to the commit recorded in the superproject, also setting the\n>>    submodules' HEAD to be detached at that commit.\n>>\n>> On my system, .git/config has this:\n>>\n>>    [submodule]\n>>            active = .\n>>\n>> So these submodules are active, but they probably should not be, since\n>> we haven't checked anything out (or, for that matter, cloned any data)\n>> and it wouldn't make sense to try to operate on them automatically with\n>> submodules.recurse or --recurse-submodules.\n>>\n>> My gut tells me that we should probably mark submodules with update=none\n>> set on a clone as inactive.  Of course, this is a tricky area that I'm\n>> not super familiar with, so opinions or thoughts are welcome.\n>>\n>> If folks think this is a good way forward, I'll look into writing a\n>> patch, probably tomorrow evening since it's starting to get late here.\n\nThis is probably a good fix. 'git clone --recurse-submodules' is really\ntoo eager to write the 'submodule.active=.' config but it should be more careful;\nthe above is an example and [1] is another one.\n\nI think it is a good way forward that having 'submodule.$name.update=none' in\n'.gitmodules' means that 'git clone' would write 'submodule.$name.active=false'\nto the local config. This way it would still be the case that 'submodule.$name.update'\nitself only ever applies to 'git submodule update', which is what is documented [2].\n\nA longer term plan could be to completely deprecate 'submodule.$name.update=none' and allow\n'submodule.$name.active' in '.gitmodules'. Maybe anecdotal, but at least one user [3]\ntried to do that and found out it does not work. Having 'active' allowed in '.gitmodules'\nwould (I think) cover all uses cases for 'update=none', which could then be deprecated,\nbut that would need a transition period. But this change is not necessarily needed; it\nwould just make the whole system more consistent IMO.\n\nCheers,\n\nPhilippe.\nP.S. The current bug was reported last year [4] but I could not reproduce it at when I\nreplied. Adding '--force' to the 'git checkout other-branch' command in my reproducer\nscript does trigger the bug, though.\n\n\n[1] https://lore.kernel.org/git/20200501005432.h62dnpkx7feb7rto@glandium.org/T/#u\n[2] https://git-scm.com/docs/git-config#Documentation/git-config.txt-submoduleltnamegtupdate\n[3] https://lore.kernel.org/git/HE1PR04MB2987A255BC327320D1DDFE6692F70@HE1PR04MB2987.eurprd04.prod.outlook.com/\n[4] https://lore.kernel.org/git/CABVQXt5E-R22G62W-tQieA7XiZKZiOA8Hp2xewYhwwOS8wFh0Q@mail.gmail.com/\n"},{"id":"427798","messageId":"YMvgRjwyrwyLz4SC@camp.crustytoothpaste.net","threadId":"55941","inReplyTo":"3b53846d-7611-521a-274c-473e00266529@gmail.com","subject":"Re: [BUG] `git reset --hard` fails with `update = none` submodules","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-06-17T23:52:38Z","receivedAt":"2021-06-17T23:53:15Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-06-16 at 13:20:44, Philippe Blain wrote:\n> Hi all,\n> \n> Le 2021-06-15 à 23:10, Junio C Hamano a écrit :\n> > \"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n> > \n> > > My gut tells me that we should probably mark submodules with update=none\n> > > set on a clone as inactive.  Of course, this is a tricky area that I'm\n> > > not super familiar with, so opinions or thoughts are welcome.\n> > > \n> > > If folks think this is a good way forward, I'll look into writing a\n> > > patch, probably tomorrow evening since it's starting to get late here.\n> \n> This is probably a good fix. 'git clone --recurse-submodules' is really\n> too eager to write the 'submodule.active=.' config but it should be more careful;\n> the above is an example and [1] is another one.\n> \n> I think it is a good way forward that having 'submodule.$name.update=none' in\n> '.gitmodules' means that 'git clone' would write 'submodule.$name.active=false'\n> to the local config. This way it would still be the case that 'submodule.$name.update'\n> itself only ever applies to 'git submodule update', which is what is documented [2].\n\nI have a patch along this line and am looking into some test failures,\nso it's not that I've forgotten about it, but that it is, as usual, not\nas trivial as I'd hoped.  It may take me until the weekend to come up\nwith something nice.\n\nOur problem seems to be that once we mark it as inactive, we never end\nup initializing the repository even if something like --checkout is\npassed (at least in the tests), so I'm trying to come up with an elegant\nway to deal with that.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"427967","messageId":"20210619214449.2827705-1-sandals@crustytoothpaste.net","threadId":"55941","inReplyTo":"YMvgRjwyrwyLz4SC@camp.crustytoothpaste.net","subject":"[PATCH] submodule: mark submodules with update=none as inactive","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-06-19T21:44:49Z","receivedAt":"2021-06-19T21:45:05Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When the user recursively clones a repository with submodules and one or\nmore of those submodules is marked with the submodule.<name>.update=none\nconfiguration, the submodule will end up being active.  This is a\nproblem because we will have skipped cloning or checking out the\nsubmodule, and as a result, other commands, such as git reset or git\ncheckout, will fail if they are invoked with --recurse-submodules (or\nwhen submodule.recurse is true).\n\nThis is obviously not the behavior the user wanted, so let's fix this by\nspecifically setting the submodule as inactive in this case when we're\ncloning the repository.  That will make us properly ignore the submodule\nwhen performing recursive operations.\n\nNote that we only do this when the --require-init option is passed,\nwhich is only passed during clone.  That's because the update-clone\nsubmodule helper is also invoked during a user-invoked git submodule\nupdate, where we explicitly indicate that we override the configuration\nsetting, so we don't want to set such a configuration option then.\n\nReported-by: Rose Kunkel <rose@rosekunkel.me>\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n builtin/submodule--helper.c |  5 +++++\n t/t5601-clone.sh            | 24 ++++++++++++++++++++++++\n 2 files changed, 29 insertions(+)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex ae6174ab05..2e14cc7a26 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -2102,6 +2102,11 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \tif (suc->update.type == SM_UPDATE_NONE\n \t    || (suc->update.type == SM_UPDATE_UNSPECIFIED\n \t\t&& update_type == SM_UPDATE_NONE)) {\n+\t\tif (suc->require_init) {\n+\t\t\tkey = xstrfmt(\"submodule.%s.active\", sub->name);\n+\t\t\tgit_config_set(key, \"false\");\n+\t\t\tfree(key);\n+\t\t}\n \t\tstrbuf_addf(out, _(\"Skipping submodule '%s'\"), displaypath);\n \t\tstrbuf_addch(out, '\\n');\n \t\tgoto cleanup;\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex c0688467e7..efe6b13be0 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -752,6 +752,30 @@ test_expect_success 'batch missing blob request does not inadvertently try to fe\n \tgit clone --filter=blob:limit=0 \"file://$(pwd)/server\" client\n '\n \n+test_expect_success 'clone with submodule with update=none is not active' '\n+\trm -rf server client &&\n+\n+\ttest_create_repo server &&\n+\techo a >server/a &&\n+\techo b >server/b &&\n+\tgit -C server add a b &&\n+\tgit -C server commit -m x &&\n+\n+\techo aa >server/a &&\n+\techo bb >server/b &&\n+\tgit -C server submodule add --name c \"$(pwd)/repo_for_submodule\" c &&\n+\tgit -C server config -f .gitmodules submodule.c.update none &&\n+\tgit -C server add a b c .gitmodules &&\n+\tgit -C server commit -m x &&\n+\n+\tgit clone --recurse-submodules server client &&\n+\tgit -C client config submodule.c.active >actual &&\n+\techo false >expected &&\n+\ttest_cmp actual expected &&\n+\t# This would fail if the submodule were active, since it is not checked out.\n+\tgit -C client reset --recurse-submodules --hard\n+'\n+\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\n start_httpd\n \n"},{"id":"428173","messageId":"56b5c722-8baf-9f9c-cc9f-5b5ed49d7fc3@gmail.com","threadId":"55941","inReplyTo":"20210619214449.2827705-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH] submodule: mark submodules with update=none as inactive","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2021-06-22T03:45:45Z","receivedAt":"2021-06-22T03:45:49Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi brian,\n\nLe 2021-06-19 à 17:44, brian m. carlson a écrit :\n> When the user recursively clones a repository with submodules and one or\n> more of those submodules is marked with the submodule.<name>.update=none\n> configuration, the submodule will end up being active.  This is a\n> problem because we will have skipped cloning or checking out the\n> submodule, and as a result, other commands, such as git reset or git\n> checkout, will fail if they are invoked with --recurse-submodules (or\n> when submodule.recurse is true).\n> \n> This is obviously not the behavior the user wanted, so let's fix this by\n> specifically setting the submodule as inactive in this case when we're\n> cloning the repository. \n\nI'm not sure we want to fix it only at (recursive) clone time. The original bug report\nwas with 'git clone --recurse-submodules', but a non-recursive clone\nfollowed by the usual 'git submodule init && git submodule update' (or\nin one step 'git submodule update --init') would also lead to the same\nbad experience (I'm not sure I want to call it a bug per se... it's certainly\na UX bug :)\n\n> That will make us properly ignore the submodule\n> when performing recursive operations.\n> \n> Note that we only do this when the --require-init option is passed,\n> which is only passed during clone.  That's because the update-clone\n> submodule helper is also invoked during a user-invoked git submodule\n> update, where we explicitly indicate that we override the configuration\n> setting, so we don't want to set such a configuration option then.\n\nI'm not sure what you mean here by 'where we explicitely indicate that we\noverride the configuration setting'. For me, as I wrote above,\n'git clone --recurse-submodules' and 'git clone' followed by\n'git submodule update --init' should lead to mostly [*] the same end result.\n\nIf you mean 'git submodule update --checkout', that indeed seems to sometimes override the 'update=none'\nconfiguration (a fact which is absent from the documentation), then it's true that we\nwould not want to write 'active=false' at that invocation. As an aside, in my limited testing\nI could not always get 'git submodule update --checkout' to clone and checkout 'update=none' submdules;\nit would fail with \"fatal: could not get a repository handle for submodule 'sub1'\" because\n'git checkout/reset --recurse-submodules' leaves a bad .git/modules/sub1/config file\nwith the core.worktree setting when the command fails (this should not happen)...\n\nIn any case, that leads me to think that maybe the right place to write the 'active' setting\nwould be during 'git submodule init', thus builtin/submodule--helper.c::init_submodule ?\nThis way it would lead to the same behaviour if the clone was recursive or not,\nand it would not interfere with 'git submodule update --checkout'.\n\n[*] I say 'mostly' because in the first case you end up with a 'submodule.active=.'\nconfiguration, and no 'submodule.$name.active' configuration for each submodule,\nwhereas in the second case, there is no 'submodule.active' setting and\n'submodule.$name.active' is true for each submodule.\n\n> \n> Reported-by: Rose Kunkel <rose@rosekunkel.me>\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n>   builtin/submodule--helper.c |  5 +++++\n>   t/t5601-clone.sh            | 24 ++++++++++++++++++++++++\n>   2 files changed, 29 insertions(+)\n\nI think that such a change would warrant a mention in the doc, in\ngitsubmodules [1], git-config [2], and probably git-submodule [3] if we agree that\n'git submodule init' is the right place to set the 'active=false' flag.\n\nThanks for proposing a patch!\n\nPhilippe.\n\n[1] https://git-scm.com/docs/gitmodules#Documentation/gitmodules.txt-submoduleltnamegtupdate\n[2] https://git-scm.com/docs/git-config#Documentation/git-config.txt-submoduleltnamegtupdate\n[3] https://git-scm.com/docs/git-submodule#Documentation/git-submodule.txt-init--ltpathgt82308203\n"},{"id":"428509","messageId":"YNZgoS9R1qam+62C@camp.crustytoothpaste.net","threadId":"55941","inReplyTo":"56b5c722-8baf-9f9c-cc9f-5b5ed49d7fc3@gmail.com","subject":"Re: [PATCH] submodule: mark submodules with update=none as inactive","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-06-25T23:02:57Z","receivedAt":"2021-06-25T23:06:03Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-06-22 at 03:45:45, Philippe Blain wrote:\n> > That will make us properly ignore the submodule\n> > when performing recursive operations.\n> > \n> > Note that we only do this when the --require-init option is passed,\n> > which is only passed during clone.  That's because the update-clone\n> > submodule helper is also invoked during a user-invoked git submodule\n> > update, where we explicitly indicate that we override the configuration\n> > setting, so we don't want to set such a configuration option then.\n> \n> I'm not sure what you mean here by 'where we explicitely indicate that we\n> override the configuration setting'. For me, as I wrote above,\n> 'git clone --recurse-submodules' and 'git clone' followed by\n> 'git submodule update --init' should lead to mostly [*] the same end result.\n> \n> If you mean 'git submodule update --checkout', that indeed seems to sometimes override the 'update=none'\n> configuration (a fact which is absent from the documentation), then it's true that we\n> would not want to write 'active=false' at that invocation. As an aside, in my limited testing\n> I could not always get 'git submodule update --checkout' to clone and checkout 'update=none' submdules;\n> it would fail with \"fatal: could not get a repository handle for submodule 'sub1'\" because\n> 'git checkout/reset --recurse-submodules' leaves a bad .git/modules/sub1/config file\n> with the core.worktree setting when the command fails (this should not happen)...\n\nYes, that's what I meant.\n\n> In any case, that leads me to think that maybe the right place to write the 'active' setting\n> would be during 'git submodule init', thus builtin/submodule--helper.c::init_submodule ?\n> This way it would lead to the same behaviour if the clone was recursive or not,\n> and it would not interfere with 'git submodule update --checkout'.\n\nLet me take a look at some other approaches and see if I can come up\nwith something a little bit better.\n\nMy apologies for the delay in response; I'm in the process of moving at\nthe moment and my attention has been directed elsewhere than the list.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"428522","messageId":"67a6e80e-6eb6-10c2-2dcd-0610343884cd@gmail.com","threadId":"55941","inReplyTo":"YNZgoS9R1qam+62C@camp.crustytoothpaste.net","subject":"Re: [PATCH] submodule: mark submodules with update=none as inactive","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2021-06-26T15:12:07Z","receivedAt":"2021-06-26T15:12:14Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi brian,\n\nLe 2021-06-25 à 19:02, brian m. carlson a écrit :\n> On 2021-06-22 at 03:45:45, Philippe Blain wrote:\n>>> That will make us properly ignore the submodule\n>>> when performing recursive operations.\n>>>\n>>> Note that we only do this when the --require-init option is passed,\n>>> which is only passed during clone.  That's because the update-clone\n>>> submodule helper is also invoked during a user-invoked git submodule\n>>> update, where we explicitly indicate that we override the configuration\n>>> setting, so we don't want to set such a configuration option then.\n>>\n>> I'm not sure what you mean here by 'where we explicitely indicate that we\n>> override the configuration setting'. For me, as I wrote above,\n>> 'git clone --recurse-submodules' and 'git clone' followed by\n>> 'git submodule update --init' should lead to mostly [*] the same end result.\n>>\n>> If you mean 'git submodule update --checkout', that indeed seems to sometimes override the 'update=none'\n>> configuration (a fact which is absent from the documentation), \n\nNote that I'm taking back that statemnt about the doc; the description of 'git submodule update'\nstates:\n\n\"The \"updating\" can be done in several ways depending on command line options\nand the value of submodule.<name>.update configuration variable. The command line\noption takes precedence over the configuration variable.\"\n\nI was somehow under the impression that 'none' was a special case, but it's not.\n\nthen it's true that we\n>> would not want to write 'active=false' at that invocation. As an aside, in my limited testing\n>> I could not always get 'git submodule update --checkout' to clone and checkout 'update=none' submdules;\n>> it would fail with \"fatal: could not get a repository handle for submodule 'sub1'\" because\n>> 'git checkout/reset --recurse-submodules' leaves a bad .git/modules/sub1/config file\n>> with the core.worktree setting when the command fails (this should not happen)...\n> \n> Yes, that's what I meant.\n> \n>> In any case, that leads me to think that maybe the right place to write the 'active' setting\n>> would be during 'git submodule init', thus builtin/submodule--helper.c::init_submodule ?\n>> This way it would lead to the same behaviour if the clone was recursive or not,\n>> and it would not interfere with 'git submodule update --checkout'.\n> \n> Let me take a look at some other approaches and see if I can come up\n> with something a little bit better.\n\nI tested the following and it fixes the bug (for both recursive clone and\nnormal clone followed by 'git submodule update --init') and\nt7406-submodule-update.sh still passes:\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex d55f6262e9..a4cd86c72f 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -686,6 +686,13 @@ static void init_submodule(const char *path, const char *prefix,\n  \n  \t\tif (git_config_set_gently(sb.buf, upd))\n  \t\t\tdie(_(\"Failed to register update mode for submodule path '%s'\"), displaypath);\n+\n+\t\t/* Mark update=none submodules as inactive */\n+\t\tif (sub->update_strategy.type == SM_UPDATE_NONE) {\n+\t\t\tstrbuf_reset(&sb);\n+\t\t\tstrbuf_addf(&sb, \"submodule.%s.active\", sub->name);\n+\t\t\tgit_config_set_gently(sb.buf, \"false\");\n+\t\t}\n  \t}\n  \tstrbuf_release(&sb);\n  \tfree(displaypath);\n\n> \n> My apologies for the delay in response; I'm in the process of moving at\n> the moment and my attention has been directed elsewhere than the list.\n> \n\nNo problem of course!\n\nPhilippe.\n\n\n[1] https://git-scm.com/docs/git-submodule#Documentation/git-submodule.txt-update--init--remote-N--no-fetch--no-recommend-shallow-f--force--checkout--rebase--merge--referenceltrepositorygt--depthltdepthgt--recursive--jobsltngt--no-single-branch--ltpathgt82308203\n"},{"id":"429000","messageId":"20210701225117.909892-1-sandals@crustytoothpaste.net","threadId":"55941","inReplyTo":"YMvgRjwyrwyLz4SC@camp.crustytoothpaste.net","subject":"[PATCH v2] submodule: mark submodules with update=none as inactive","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-07-01T22:51:17Z","receivedAt":"2021-07-01T22:51:55Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When the user recursively clones a repository with submodules and one or\nmore of those submodules is marked with the submodule.<name>.update=none\nconfiguration, the submodule will end up being active.  This is a\nproblem because we will have skipped cloning or checking out the\nsubmodule, and as a result, other commands, such as git reset or git\ncheckout, will fail if they are invoked with --recurse-submodules (or\nwhen submodule.recurse is true).\n\nThis is obviously not the behavior the user wanted, so let's fix this by\nspecifically setting the submodule as inactive in this case when we're\ninitializing the repository.  That will make us properly ignore the\nsubmodule when performing recursive operations.\n\nWe only do this when initializing a submodule, since git submodule\nupdate can update the submodule with various options despite the setting\nof \"none\" and we want those options to override it as they currently do.\n\nReported-by: Rose Kunkel <rose@rosekunkel.me>\nHelped-by: Philippe Blain <levraiphilippeblain@gmail.com>\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n builtin/submodule--helper.c |  6 ++++++\n t/t5601-clone.sh            | 24 ++++++++++++++++++++++++\n 2 files changed, 30 insertions(+)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex ae6174ab05..a3f8c45d97 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -686,6 +686,12 @@ static void init_submodule(const char *path, const char *prefix,\n \n \t\tif (git_config_set_gently(sb.buf, upd))\n \t\t\tdie(_(\"Failed to register update mode for submodule path '%s'\"), displaypath);\n+\n+\t\tif (sub->update_strategy.type == SM_UPDATE_NONE) {\n+\t\t\tstrbuf_reset(&sb);\n+\t\t\tstrbuf_addf(&sb, \"submodule.%s.active\", sub->name);\n+\t\t\tgit_config_set_gently(sb.buf, \"false\");\n+\t\t}\n \t}\n \tstrbuf_release(&sb);\n \tfree(displaypath);\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex c0688467e7..efe6b13be0 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -752,6 +752,30 @@ test_expect_success 'batch missing blob request does not inadvertently try to fe\n \tgit clone --filter=blob:limit=0 \"file://$(pwd)/server\" client\n '\n \n+test_expect_success 'clone with submodule with update=none is not active' '\n+\trm -rf server client &&\n+\n+\ttest_create_repo server &&\n+\techo a >server/a &&\n+\techo b >server/b &&\n+\tgit -C server add a b &&\n+\tgit -C server commit -m x &&\n+\n+\techo aa >server/a &&\n+\techo bb >server/b &&\n+\tgit -C server submodule add --name c \"$(pwd)/repo_for_submodule\" c &&\n+\tgit -C server config -f .gitmodules submodule.c.update none &&\n+\tgit -C server add a b c .gitmodules &&\n+\tgit -C server commit -m x &&\n+\n+\tgit clone --recurse-submodules server client &&\n+\tgit -C client config submodule.c.active >actual &&\n+\techo false >expected &&\n+\ttest_cmp actual expected &&\n+\t# This would fail if the submodule were active, since it is not checked out.\n+\tgit -C client reset --recurse-submodules --hard\n+'\n+\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\n start_httpd\n \n"},{"id":"429582","messageId":"bf1893ee-6973-d8b2-659e-bb239a0a9ae2@gmail.com","threadId":"55941","inReplyTo":"20210701225117.909892-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2] submodule: mark submodules with update=none as inactive","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2021-07-09T20:26:35Z","receivedAt":"2021-07-09T20:26:44Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi brian,\n\n[re-cc'ing Emily and Jonathan who Junio cc'ed in <xmqqeed2sdwc.fsf@gitster.g>\nbut seemed to have been dropped when you sent v1 and v2 of the patch]\n\nLe 2021-07-01 à 18:51, brian m. carlson a écrit :\n> When the user recursively clones a repository with submodules \n\nHere I would add:\n\n\", or runs 'git submodule update --init' after a\nnon-recursive clone of such a repository, \"\n\n> and one or\n> more of those submodules is marked with the submodule.<name>.update=none\n> configuration, the submodule \n\n\"those submodules\" would be clearer, I think.\n\n> will end up being active.  This is a\n> problem because we will have skipped cloning or checking out the\n> submodule, and as a result, other commands, such as git reset or git\n> checkout, will fail if they are invoked with --recurse-submodules (or\n> when submodule.recurse is true).\n> \n> This is obviously not the behavior the user wanted, so let's fix this by\n> specifically setting the submodule as inactive in this case when we're\n> initializing the repository.  That will make us properly ignore the\n> submodule when performing recursive operations.\n> \n> We only do this when initializing a submodule, \n\nHere for even more clarity I would add:\n\ni.e. 'git submodule init' or 'git submodule update --init',\n\n> since git submodule\n> update can update the submodule with various options despite the setting\n> of \"none\" and we want those options to override it as they currently do.\n> \n> Reported-by: Rose Kunkel <rose@rosekunkel.me>\n> Helped-by: Philippe Blain <levraiphilippeblain@gmail.com>\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n>   builtin/submodule--helper.c |  6 ++++++\n>   t/t5601-clone.sh            | 24 ++++++++++++++++++++++++\n>   2 files changed, 30 insertions(+)\n\nAs I said in my review of v1, I think this would warrant a mention in the doc.\n\nIn general, I think 'git-submodule(1)' could be more precise about which submodules\nare touched by which subcommands. Since the topic that introduced the 'active' concept\nwas merged in a93dcb0a56 (Merge branch 'bw/submodule-is-active', 2017-03-30), these subcommand\nrecurse only in active submodules:\n\n- init (with a big caveat, see below)\n- sync\n- update\n\nThe doc makes no mention of that for sync and update. sync says it synchronizes 'all'\nsubmodules, and update says it updates 'registered' submodules ('registered' in not\ndefined formally anywhere either). And 'active' is mentioned in the description of\n'init', but not defined. It would be good to explicitely say \"see the 'Active submodules'\nsection in gitsubmodules(7) for a definition of 'active'\", or something like that.\n\nI'm not saying we need to fix that necessarily in this patch, I'm just noting\nwhat my reading of the code and of the doc reveals.\n\n> \n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index ae6174ab05..a3f8c45d97 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -686,6 +686,12 @@ static void init_submodule(const char *path, const char *prefix,\n>   \n>   \t\tif (git_config_set_gently(sb.buf, upd))\n>   \t\t\tdie(_(\"Failed to register update mode for submodule path '%s'\"), displaypath);\n> +\n> +\t\tif (sub->update_strategy.type == SM_UPDATE_NONE) {\n> +\t\t\tstrbuf_reset(&sb);\n> +\t\t\tstrbuf_addf(&sb, \"submodule.%s.active\", sub->name);\n> +\t\t\tgit_config_set_gently(sb.buf, \"false\");\n> +\t\t}\n>   \t}\n>   \tstrbuf_release(&sb);\n>   \tfree(displaypath);\n\nI did more testing with this patch applied and I fear it is not\ncompletely sufficient. There are 2 main problems, I think.\nThe first is that the following still triggers the bug:\n\n     git clone server client\n     git -C client submodule update --init\n     git -C client submodule init       # should be no-op, but isn't\n     git -C client reset --hard --recurse-submodules\n\nThat's because:\n\n1) 'git submodule init' operates on *all* submodules if 'submodule.active' is unset\n     and not <path> is given.\n     (see submodule--helper.c::module_init), or the doc [1].\n2) 'git submodule init' sets 'submodule.$name.active' to true for the submodules\n     on which it operates, unless already covered by 'submodule.active'\n     (see submodule--helper.c::init_submodule)\n3) the code we're adding to set 'active' to false if 'update=none' is only executed\n    if 'submodule.c.update' is not yet in the config, so it gets skipped if we\n    repeat 'git submodule init'. (I think this behaviour is sound).\n\nSo that's unfortunate, and is also kind of contradictory to what the doc says\nfor 'git submodule init':\n\"This command does not alter existing information in .git/config.\".\nAnd just to be clear, the behaviour I describe above is already existing, the current\npatch just makes it more obvious.\n\nI think we could manage to change that behaviour a bit\nin order to have 'submodule init' not modify the config for submodules which are already marked inactive,\n*unless* they are explitely matched by the pathspec on the command line.\nSo we would have:\n\n     git clone server client; cd client\n     git submodule init      # initial call sets 'submodule.c.active=false'\n     git submodule init      # does not touch c, it's already marked inactive\n     git submodule init c    # OK, we really want to mark it as active\n\nTo do that, we could use the same trick that we do in update_clone, i.e.\n\n     if (pathspec.nr)\n         info.explicit = 1\n\nwhere 'explicit' (tentative name) is a new field in 'struct init_cb', so that 'init_submodule'\nknows if the current submodule was explicitely listed on the command line.\n\n\nThen there is a second thing. As stated in the commit message,\n'git submodule update --checkout' should override the 'update=none'\nsetting and clone and checkout the submodule. But this behaviour\nis broken by the code we're adding, because 'submodule update' only recurse into\nactive submodules! (see the call to 'is_submodule_active' in\nsubmodule--helper.c::prepare_to_clone_next_submodule).\n\nSo this does not clone the submodule:\n\n     git clone --recurse server client   # recursive clone\n     git -C client submodule --checkout  # should clone c, doesn't\n\nNeither does this:\n    \n     git clone server client                   # non-recursive clone\n     git -C client submodule update --init\n     git -C client submodule update --checkout # should clone c, doesn't\n\nBut because of the first problem above, this works(!):\n\n     git clone server client\n     git -C client submodule update --init\n     git -C client submodule update --init --checkout\n\nBecause in the third call, c is set to 'active' by init_submodule,\nthen is *not* skipped by prepare_to_clone_next_submodule.\n\n\nSo it's all a little bit complicated! But I think that with my suggestion above,\ni.e. that 'git submodule init', in the absence of 'submodule.active', would\nonly switch inactive submodules to active if they are explicitely listed, then\nwe could get a saner behaviour, at the expense of having to explicitely init\n'update=none' submodules on the command line if we really want to '--checkout' :\n     \n     git clone server client\n     git -C client submodule update --init        # first call: set c to inactive\n     git -C client submodule update --init        # no-op\n     git -C client submodule update --checkout    # does not clone c (currently quiet)\n     git -C client submodule update --checkout c  # does not clone c, but warns (current behaviour)\n     git -C client submodule init c               # sets c to active\n     git -C client submodule update --checkout    # clones c\n\nwhere the last two command could be a single\n'git submodule update --init --checkout c' and ideally the\n4th command should also warn the user that they now have to explicitely 'init'\nc if they want to check it out, which could simply mean tweaking the already\nexisting message in next_submodule_warn_missing to also check if\nthe current submodule has 'update=none' and then display the warning\n(instead of just showing it if the submodule was listed on the command\nline, which is the current behaviour). Additionnaly, the warning should\nsay \"Maybe you want to use 'update --init %s'?\", i.e. specify the path.\n\n\nWhat do you think of my suggestions ? I can help push this forward\nby contributing patches if we agree that we should go forward with\nthis slight behaviour change in 'git submodule init' ...\n\n\n> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\n> index c0688467e7..efe6b13be0 100755\n> --- a/t/t5601-clone.sh\n> +++ b/t/t5601-clone.sh\n> @@ -752,6 +752,30 @@ test_expect_success 'batch missing blob request does not inadvertently try to fe\n>   \tgit clone --filter=blob:limit=0 \"file://$(pwd)/server\" client\n>   '\n>   \n> +test_expect_success 'clone with submodule with update=none is not active' '\n> +\trm -rf server client &&\n> +\n> +\ttest_create_repo server &&\n> +\techo a >server/a &&\n> +\techo b >server/b &&\n> +\tgit -C server add a b &&\n> +\tgit -C server commit -m x &&\n> +\n> +\techo aa >server/a &&\n> +\techo bb >server/b &&\n> +\tgit -C server submodule add --name c \"$(pwd)/repo_for_submodule\" c &&\n> +\tgit -C server config -f .gitmodules submodule.c.update none &&\n> +\tgit -C server add a b c .gitmodules &&\n> +\tgit -C server commit -m x &&\n> +\n> +\tgit clone --recurse-submodules server client &&\n> +\tgit -C client config submodule.c.active >actual &&\n> +\techo false >expected &&\n> +\ttest_cmp actual expected &&\n> +\t# This would fail if the submodule were active, since it is not checked out.\n> +\tgit -C client reset --recurse-submodules --hard\n> +'\n\nI think we might want to also test the non-recursive clone case as well,\ni.e. 'git clone' and then 'git submodule update --init', as well as\nsubsequent calls to 'git submodule init' in light of my analysis above.\n\nAlso, the only place in the test suite that I could find where\n'update=none' is tested is in t7406.35-38 in t7406-submodule-update.sh\nso maybe it would make more sense to put the test(s) there ?\n\nThanks,\n\nPhilippe.\n\n[1] https://git-scm.com/docs/git-submodule#Documentation/git-submodule.txt-init--ltpathgt82308203\n"},{"id":"429706","messageId":"YOsjZldLHR/htyx9@camp.crustytoothpaste.net","threadId":"55941","inReplyTo":"bf1893ee-6973-d8b2-659e-bb239a0a9ae2@gmail.com","subject":"Re: [PATCH v2] submodule: mark submodules with update=none as inactive","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-07-11T16:59:18Z","receivedAt":"2021-07-11T16:59:55Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-07-09 at 20:26:35, Philippe Blain wrote:\n> Hi brian,\n> \n> [re-cc'ing Emily and Jonathan who Junio cc'ed in <xmqqeed2sdwc.fsf@gitster.g>\n> but seemed to have been dropped when you sent v1 and v2 of the patch]\n> \n> Le 2021-07-01 à 18:51, brian m. carlson a écrit :\n> > When the user recursively clones a repository with submodules\n> \n> Here I would add:\n> \n> \", or runs 'git submodule update --init' after a\n> non-recursive clone of such a repository, \"\n> \n> > and one or\n> > more of those submodules is marked with the submodule.<name>.update=none\n> > configuration, the submodule\n> \n> \"those submodules\" would be clearer, I think.\n\nSure, I can make that change.\n\n> > will end up being active.  This is a\n> > problem because we will have skipped cloning or checking out the\n> > submodule, and as a result, other commands, such as git reset or git\n> > checkout, will fail if they are invoked with --recurse-submodules (or\n> > when submodule.recurse is true).\n> > \n> > This is obviously not the behavior the user wanted, so let's fix this by\n> > specifically setting the submodule as inactive in this case when we're\n> > initializing the repository.  That will make us properly ignore the\n> > submodule when performing recursive operations.\n> > \n> > We only do this when initializing a submodule,\n> \n> Here for even more clarity I would add:\n> \n> i.e. 'git submodule init' or 'git submodule update --init',\n\nOkay.\n\n> > since git submodule\n> > update can update the submodule with various options despite the setting\n> > of \"none\" and we want those options to override it as they currently do.\n> > \n> > Reported-by: Rose Kunkel <rose@rosekunkel.me>\n> > Helped-by: Philippe Blain <levraiphilippeblain@gmail.com>\n> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> > ---\n> >   builtin/submodule--helper.c |  6 ++++++\n> >   t/t5601-clone.sh            | 24 ++++++++++++++++++++++++\n> >   2 files changed, 30 insertions(+)\n> \n> As I said in my review of v1, I think this would warrant a mention in the doc.\n> \n> In general, I think 'git-submodule(1)' could be more precise about which submodules\n> are touched by which subcommands. Since the topic that introduced the 'active' concept\n> was merged in a93dcb0a56 (Merge branch 'bw/submodule-is-active', 2017-03-30), these subcommand\n> recurse only in active submodules:\n> \n> - init (with a big caveat, see below)\n> - sync\n> - update\n> \n> The doc makes no mention of that for sync and update. sync says it synchronizes 'all'\n> submodules, and update says it updates 'registered' submodules ('registered' in not\n> defined formally anywhere either). And 'active' is mentioned in the description of\n> 'init', but not defined. It would be good to explicitely say \"see the 'Active submodules'\n> section in gitsubmodules(7) for a definition of 'active'\", or something like that.\n> \n> I'm not saying we need to fix that necessarily in this patch, I'm just noting\n> what my reading of the code and of the doc reveals.\n\n\n\n> I did more testing with this patch applied and I fear it is not\n> completely sufficient. There are 2 main problems, I think.\n> The first is that the following still triggers the bug:\n> \n>     git clone server client\n>     git -C client submodule update --init\n>     git -C client submodule init       # should be no-op, but isn't\n>     git -C client reset --hard --recurse-submodules\n> \n> That's because:\n> \n> 1) 'git submodule init' operates on *all* submodules if 'submodule.active' is unset\n>     and not <path> is given.\n>     (see submodule--helper.c::module_init), or the doc [1].\n> 2) 'git submodule init' sets 'submodule.$name.active' to true for the submodules\n>     on which it operates, unless already covered by 'submodule.active'\n>     (see submodule--helper.c::init_submodule)\n> 3) the code we're adding to set 'active' to false if 'update=none' is only executed\n>    if 'submodule.c.update' is not yet in the config, so it gets skipped if we\n>    repeat 'git submodule init'. (I think this behaviour is sound).\n> \n> So that's unfortunate, and is also kind of contradictory to what the doc says\n> for 'git submodule init':\n> \"This command does not alter existing information in .git/config.\".\n> And just to be clear, the behaviour I describe above is already existing, the current\n> patch just makes it more obvious.\n\nRight, I noticed that.\n\n> I think we could manage to change that behaviour a bit\n> in order to have 'submodule init' not modify the config for submodules which are already marked inactive,\n> *unless* they are explitely matched by the pathspec on the command line.\n> So we would have:\n> \n>     git clone server client; cd client\n>     git submodule init      # initial call sets 'submodule.c.active=false'\n>     git submodule init      # does not touch c, it's already marked inactive\n>     git submodule init c    # OK, we really want to mark it as active\n\nWe could also add an option, --all, to make it work on all submodules.\nThat's because previously \"git submodule init\" did work on all the\nsubmodules in the repository in question, but it doesn't now because of\nour change.\n\n> So it's all a little bit complicated! But I think that with my suggestion above,\n> i.e. that 'git submodule init', in the absence of 'submodule.active', would\n> only switch inactive submodules to active if they are explicitely listed, then\n> we could get a saner behaviour, at the expense of having to explicitely init\n> 'update=none' submodules on the command line if we really want to '--checkout' :\n>     git clone server client\n>     git -C client submodule update --init        # first call: set c to inactive\n>     git -C client submodule update --init        # no-op\n>     git -C client submodule update --checkout    # does not clone c (currently quiet)\n>     git -C client submodule update --checkout c  # does not clone c, but warns (current behaviour)\n>     git -C client submodule init c               # sets c to active\n>     git -C client submodule update --checkout    # clones c\n> \n> where the last two command could be a single\n> 'git submodule update --init --checkout c' and ideally the\n> 4th command should also warn the user that they now have to explicitely 'init'\n> c if they want to check it out, which could simply mean tweaking the already\n> existing message in next_submodule_warn_missing to also check if\n> the current submodule has 'update=none' and then display the warning\n> (instead of just showing it if the submodule was listed on the command\n> line, which is the current behaviour). Additionnaly, the warning should\n> say \"Maybe you want to use 'update --init %s'?\", i.e. specify the path.\n> \n> \n> What do you think of my suggestions ? I can help push this forward\n> by contributing patches if we agree that we should go forward with\n> this slight behaviour change in 'git submodule init' ...\n\nWith the modification of adding --all to init so users can get a\nbehavior a little more similar to the previous, yes, that sounds good.\nIt would be great if you'd be willing to send a few patches.\n\n> > diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\n> > index c0688467e7..efe6b13be0 100755\n> > --- a/t/t5601-clone.sh\n> > +++ b/t/t5601-clone.sh\n> > @@ -752,6 +752,30 @@ test_expect_success 'batch missing blob request does not inadvertently try to fe\n> >   \tgit clone --filter=blob:limit=0 \"file://$(pwd)/server\" client\n> >   '\n> > +test_expect_success 'clone with submodule with update=none is not active' '\n> > +\trm -rf server client &&\n> > +\n> > +\ttest_create_repo server &&\n> > +\techo a >server/a &&\n> > +\techo b >server/b &&\n> > +\tgit -C server add a b &&\n> > +\tgit -C server commit -m x &&\n> > +\n> > +\techo aa >server/a &&\n> > +\techo bb >server/b &&\n> > +\tgit -C server submodule add --name c \"$(pwd)/repo_for_submodule\" c &&\n> > +\tgit -C server config -f .gitmodules submodule.c.update none &&\n> > +\tgit -C server add a b c .gitmodules &&\n> > +\tgit -C server commit -m x &&\n> > +\n> > +\tgit clone --recurse-submodules server client &&\n> > +\tgit -C client config submodule.c.active >actual &&\n> > +\techo false >expected &&\n> > +\ttest_cmp actual expected &&\n> > +\t# This would fail if the submodule were active, since it is not checked out.\n> > +\tgit -C client reset --recurse-submodules --hard\n> > +'\n> \n> I think we might want to also test the non-recursive clone case as well,\n> i.e. 'git clone' and then 'git submodule update --init', as well as\n> subsequent calls to 'git submodule init' in light of my analysis above.\n> \n> Also, the only place in the test suite that I could find where\n> 'update=none' is tested is in t7406.35-38 in t7406-submodule-update.sh\n> so maybe it would make more sense to put the test(s) there ?\n\nSure, I can do that.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"}]}