Stefan Beller <sbeller@xxxxxxxxxx> writes:
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 43c68de..d83608c 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -825,8 +825,9 @@ Maybe you want to use 'update --init'?")"
> if test -n "$recursive"
> then
> (
> - prefix="$prefix$sm_path/"
> + prefix="$(relative_path $prefix$sm_path)/"
Don't you need to protect yourself from IFS chars in $prefix and
$sm_path here (the other invocation of relative_path in this patch
does not have this issue, I would think).
As I think about it more, I am more inclined to say "-C $prefix" is
going in a wrong direction, so justifying this change solely on that
move is somewhat sad.
- Making things relative should not hurt.
- Clearing wt_prefix feels like the right thing, because we are
moving to the root of the working tree of a different repository
and wt_prefix that tells us where the user originally was in the
superproject is not useful in the context of the submodule, which
after all is a separate project. The user however cannot refer
to things on the filesystem (including use of pathspecs), as they
are taken relative to the root level of each submodule by
clearing wt_prefix, though. i.e.
$ git submodule $cmd --read-from-this-file=../m/file
that is started from t/ subdirectory of the superproject that
recurses into the submodule at m/ (sitting next to t/) should be
able to read from "file" sitting at the root level of the working
tree of submodule m/ by stripping ../m/ (and relative-path should
be able to help with that), but that may become impossible
because the fact that the user named ../m/file relative to t/
subdirectory is lost by clearing wt_prefix that used to hold t/
here.
is the closest justification I can come to, which is weak ("should
not hurt" does not justify, and "users cannot ever do this without
undoing this change" does not, either).
I think the worst part of this change is that the log message does
not make it clear why it is OK not to clear wt_prefix and not to use
relative_path if you use --prefix, while the "-C $prefix" approach
has problems without these changes. Without that explanation, these
symptoms, i.e. the fact that you need the changes in this patch,
only smells like an indication that "-C $prefix" approach is somehow
flawed. I cannot quite pinpoint what that "somehow" is, though.
> clear_local_git_env
> + wt_prefix=
> cd "$sm_path" &&
> eval cmd_update
> )
> @@ -1159,6 +1160,7 @@ cmd_status()
> (
> prefix="$displaypath/"
> clear_local_git_env
> + wt_prefix=
> cd "$sm_path" &&
> eval cmd_status
> ) ||
> @@ -1239,7 +1241,8 @@ cmd_sync()
>
> if test -n "$recursive"
> then
> - prefix="$prefix$sm_path/"
> + prefix=$(relative_path "$prefix$sm_path/")
> + wt_prefix=
> eval cmd_sync
> fi
> )
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html