Junio C Hamano [off-list ref] writes:
FWIW, it bothers me a lot more that expression does not anchor
matches against path elements with explicit '/' than the issue your patch
addresses, i.e.
sed -e 's|[^/][^/]*/|../|g'
Note that this requires readding a trailing slash to the path, which
complicates the whole thing a bit. The effect of * has always been to
use the longest match.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Andreas Schwab [off-list ref] writes:
Junio C Hamano [off-list ref] writes:
quoted
FWIW, it bothers me a lot more that expression does not anchor
matches against path elements with explicit '/' than the issue your patch
addresses, i.e.
sed -e 's|[^/][^/]*/|../|g'
Note that this requires readding a trailing slash to the path, which
complicates the whole thing a bit.
Or simply swapping order of the stripping and sed invocation, which is not
a big deal. I personally find the pattern with trailing '/' easier to grok
but it is also not a big deal.
Excerpts from Junio C Hamano's message of Tue Apr 10 12:10:22 -0400 2012:
Andreas Schwab [off-list ref] writes:
quoted
Junio C Hamano [off-list ref] writes:
quoted
FWIW, it bothers me a lot more that expression does not anchor
matches against path elements with explicit '/' than the issue your patch
addresses, i.e.
sed -e 's|[^/][^/]*/|../|g'
Note that this requires readding a trailing slash to the path, which
complicates the whole thing a bit.
Or simply swapping order of the stripping and sed invocation, which
is not a big deal. I personally find the pattern with trailing '/'
easier to grok but it is also not a big deal.
Sorry for the delay here, I haven't had a chance to look at this in a
few days. I tried the version with [^/][^/]*/ tonight and it was more
invasive than required for such a simple fix, I think.
Switching the order of the calls is ok, but then you need to use a
rela and relb as the second substitution relies on $a being
/-terminated and the creation of gitdir:... requires the opposite. So
now, you're switching the order, moving other lines, renaming
variables and also altering the lines used to spit out the paths in
order to avoid also stripping the / from the rel* variables.
Updated patch to follow, but please don't hesitate to request the
alternate version if it really is what you'd like to see. (I've done
the work already.)
Thanks
-Ben
--
Ben Walton
Systems Programmer - CHASS
University of Toronto
C:416.407.5610 | W:416.978.4302
The sed provided by Solaris in /usr/xpg4/bin has a bug whereby an
unanchored regex using * for zero or more repetitions sees two
separate matches fed to the substitution engine in some cases.
This is evidenced by:
$ for sed in /usr/xpg4/bin/sed /usr/bin/sed /opt/csw/gnu/sed; do \
echo 'ab' | $sed -e 's|[a]*|X|g'; \
done
XXbX
XbX
XbX
This bug was triggered during a git submodule clone operation as
exercised in the setup stage of t5526-fetch-submodules when using the
default SANE_TOOL_PATH for Solaris. It led to paths such as
..../.. being used in the submodule .git gitdir reference.
As we do not need to handle fully qualfied paths we can make the regex
match 1 or more instead of 0 or more non-/ characters so use
's|[^/][^/]*|..|g' instead, which is correctly handled by all tested
sed implementations. This expression is semantically different than
the original one. It will not place leading '..' on a fully qualified
path as the original expression did. None of the paths passed to the
regex relied on this behaviour so changing it shouldn't have negative
impact.
Signed-off-by: Ben Walton <redacted>
---
git-submodule.sh | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index efc86ad..7aa9e95 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -167,10 +167,10 @@ module_clone()
a=${a%/}
b=${b%/}
- rel=$(echo $b | sed -e 's|[^/]*|..|g')
+ rel=$(echo $b | sed -e 's|[^/][^/]*|..|g')
echo "gitdir: $rel/$a" >"$path/.git"
- rel=$(echo $a | sed -e 's|[^/]*|..|g')
+ rel=$(echo $a | sed -e 's|[^/][^/]*|..|g')
(clear_local_git_env; cd "$path" && GIT_WORK_TREE=. git config core.worktree "$rel/$b")
}
--
1.7.9