Re: [PATCH v3 2/2] submodule: drop the top-level requirement

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 19, 2013, 16:45 UTC
Message-ID: <7vmwsu31vh.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130419074632.GC2278@serenity.lan>

John Keeping <john@keeping.me.uk> writes:

Show 28 quoted lines

> On Thu, Apr 18, 2013 at 03:40:41PM -0700, Junio C Hamano wrote:
>> John Keeping <john@keeping.me.uk> writes:
>> 
>> > +relative_path ()
>> > +{
>> > +	local target curdir result
>> > +	target=$1
>> > +	curdir=${2-$wt_prefix}
>> > +	curdir=${curdir%/}
>> > +	result=
>> > +
>> > +	while test -n "$curdir"
>> > +	do
>> > +		case "$target" in
>> > +		"$curdir/"*)
>> > +			target=${target#$curdir/}
>> > +			break
>> > +			;;
>> > +		esac
>> 
>> Could $curdir have glob wildcard to throw this part of the logic
>> off?  It is OK to have limitations like "you cannot have a glob
>> characters in your path to submodule working tree" (at least until
>> we start rewriting these in C or Perl or Python), but we need to be
>> aware of them.
>
> I think the use of "#" instead of "##" saves us here because even with a
> wildcard in $curdir the case statement matches literally,

If you have curdir=a*b and target=adropb/c/d/e, the chopping itself

	target=${target#$curdir/}

would happily chop "adropb/" from the target, but because the dq around "$curdir/"* in the case arm label enforces that target must literally match curdir followed by a slash, we do not even come to the chomping part.

I still have not convinced myself that it is impossible for somebody more clever than I to craft a pair of target and curdir that breaks it, though. (target="a*b/c/d", curdir="a*b") is correctly chopped, so that is not it.