Re: [PATCH 1/4] submodule: fix recursive path printing from non root directory

[Stefan Beller <sbeller@google.com>]

On Thu, Mar 24, 2016 at 4:38 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Thu, Mar 24, 2016 at 4:34 PM, Stefan Beller <sbeller@google.com> wrote:
>> Recursing into submodules currently works by just calling
>> (cd $submodule && eval <command>) for update, sync and status
>> command.
>>
>> Say you have the following setup
>>
>> repo/ # a superproject repository
>> repo/untracked/ # an untracked dir in repo/
>> repo/sub/ # a submodule
>> repo/sub/subsub # a submodule of a submodule
>>
>> When being in repo/untracked/ and invoking "git submodule status"
>> you would expect output like:
>>
>>     repo/untracked/$ git submodule status --recursive
>>      <sha1> ../sub (version)
>>      <sha1> ../sub/subsub (<version>)
>>
>> We need to take into account that we are in the untracked/ dir,
>> so we need to prepend ../ to the paths. By using relative_path
>> to compute the prefix, we'll have that output.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>>  git-submodule.sh | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index 43c68de..536ba68 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -825,7 +825,7 @@ Maybe you want to use 'update --init'?")"
>>                 if test -n "$recursive"
>>                 then
>>                         (
>> -                               prefix="$prefix$sm_path/"
>> +                               prefix="$(relative_path $prefix$sm_path)/"
>>                                 clear_local_git_env
>>                                 cd "$sm_path" &&
>>                                 eval cmd_update
>> @@ -1233,13 +1233,13 @@ cmd_sync()
>>                         then
>>                         (
>>                                 clear_local_git_env
>> +                               prefix=$(relative_path "$prefix$sm_path/")
>
> Not really sure why this got moved, but I don't think it hurts
> anything, though we will have prefix defined now regardless of if
> we're recursive or not. But I think that's correct.

"Because we need to move it before the cd". At least I thought so at
the time of writing. That is actually not the case.

At the time of writing this was intertangled with the next patch,
and we need to put the call to relative_path before the reassignment
of wt_prefix as relative_path depends on wt_prefix, which I put at the
same place as the cd in an initial version when coming up with that patch.

In case of a resend, consider this fixed.

>
>>                                 cd "$sm_path"
>>                                 remote=$(get_default_remote)
>>                                 git config remote."$remote".url "$sub_origin_url"
>>
>>                                 if test -n "$recursive"
>>                                 then
>> -                                       prefix="$prefix$sm_path/"
>>                                         eval cmd_sync
>>                                 fi
>>                         )
>> --
>> 2.8.0.rc4.10.g52f3f33
>>
>
> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
>
> Regards,
> Jake