Re: [GSoC][PATCH v5 1/3] submodule: fix buggy $path and $sm_path variable's value
From: Ramsay Jones <hidden>
Date: 2017-05-27 01:51:53
On 26/05/17 18:07, Stefan Beller wrote:
On Fri, May 26, 2017 at 9:31 AM, Ramsay Jones [off-list ref] wrote:quoted
Hmm, I'm not sure which documentation you are referring to,Quite likely our fine manual pages. ;) foreach [--recursive] <command> Evaluates an arbitrary shell command in each checked out submodule. The command has access to the variables $name, $path, $sha1 and $toplevel: $name is the name of the relevant submodule section in .gitmodules, $path is the name of the submodule directory relative to the superproject, $sha1 is the commit as recorded in the superproject, and $toplevel is the absolute path to the top-level of the superproject. Any submodules defined in the superproject but not checked out are ignored by this command. Unless given --quiet, foreach prints the name of each submodule before evaluating the command. If --recursive is given, submodules are traversed recursively (i.e. the given shell command is evaluated in nested submodules as well). A non-zero return from the command in any submodule causes the processing to terminate. This can be overridden by adding || : to the end of the command.
I suspected as much, but I was wondering specifically if $sm_path had been documented anywhere. I didn't think so, but ...
As $path is documented and $sm_path is not, we should care about $path first to be correct and either fix the documentation or the implementation such that we have a consistent world view. :)
Sure, but what is that world view? :-D I suspect that commit 091a6eb0fe did not intend (should not have) used $sm_path in that test. If we were to 'fix' that test, would it still work? Back in 2012, the submodule list was generated by filtering the output of 'git ls-files --error-unmatch --stage --'; but I don't recall if (at that time) git-ls-files required being at the top of the working tree, or if it would execute fine in a sub-directory. So, it's possible that the documentation of $path was wrong all along. ;-) At that time, by definition, $path == $sm_path. However, you know this stuff much better than me (I don't use git-submodule), so ...
quoted
but if $path != $sm_path then something is wrong. (unless their definition has changed, of course).I would lean in doing so (changing their definition): $path (as documented) is the name of the submodule directory relative to the direct superproject (so in nested submodules you go up only one level). $sm_path on the other hand is not documented at all and yields non-sense results in corner cases.
Hmm, at what point did '$sm_path yields non-sense results' start being the case? (perhaps the corner cases need to be fixed first).
With this patch it becomes less non-sensey and could be documented as:
$sm_path is the relative path from the current working directory
to the submodule (ignoring relations to the superproject or nesting
of submodules). OK.
This documentation also fits into the narrative of
the test in t7407.Hmm, does it? ATB, Ramsay Jones