Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: >>> [...] >>> This is still in this series as 02/12. I think you've misunderstood >>> that code, it *is* invoking "git submodule--helper" with >>> "--super-prefix", but the option is passed as: >>> >>> git --super-prefix <path> submodule--helper >>> >>> And not as: >>> >>> git submodule--helper --super-prefix <path> >>> >>> This is thus handled by other code before builtin/submodule--helper.c, >>> and it doesn't need to handle it. >>> >>> But anyway, this is confusing, so I updated the commit message (seen >>> in the range-diff below)> >> >> Ah that's right, I forgot that we have to pass it to "git" directly. >> Thanks. >> >> I wonder why we ever needed this. 89c8626557 (submodule helper: support >> super prefix, 2016-12-08) doesn't really explain it, so it looks like >> I'll have to dig around the ML. > > It was needed, but not after b3c5f5cb048 (submodule: move core > cmd_update() logic to C, 2022-03-15) as my 02/12 discusses. > > As a quick test try to check out b3c5f5cb048 and apply this change: > > -#define SUPPORT_SUPER_PREFIX (1<<0) > +#define SUPPORT_SUPER_PREFIX 0 > > You'll find that t7406-submodule-update.sh passes, but check out its > parent (which is a commit of yours) and it'll fail, as we'll then emit > output like: > > -Submodule path '../super': checked out 'e1c658656b91df52a4634fbffeaa739807ce3521' > +Submodule path 'super': checked out 'e1c658656b91df52a4634fbffeaa739807ce3521' > > So this is just one of the things that were overly complex in > git-submodule--helper because parts of it had to bridge the gap between > *.sh and *.c land, but once we moved more parts to C we ended up getting > that for free. Ah yes you are right, as of b3c5f5cb048 we never pass `--super-prefix` to "git submodule" any more. Great! I also remember that when I was reading the patch that became b3c5f5cb048 (submodule: move core cmd_update() logic to C, 2022-03-15), I noted (internally) that we used `prefix` to set "--recursive-prefix", which was unusual because `prefix` is usually mapped to "--super-prefix". I have a patch to replace "--recursive-prefix" with "--super-prefix", but it doesn't make sense for that patch to be included in this series (it's preparation for something else entirely, and it won't make this series any cleaner.)
