On 0, "Shawn O. Pearce" <spearce@xxxxxxxxxxx> wrote:
> Johan Herland <johan@xxxxxxxxxxx> wrote:
> > On Wednesday 24 September 2008, David Aguilar wrote:
> > > Instead of just doing an "|| exit" shouldn't it report an explanation
> > > of the error?
> > > Other than that, it looks good to me.
> >
> > Fixed. Thanks.
>
> OK, time for the drive-by patch commenting. I've largely stayed
> out of git-submodule related code, but I just looked at in the
> context of applying this patch.
>
> There are three callers to resolve_relative_url in master and next.
> All three callers just "|| exit" when resolve_relative_url fails.
>
> The only reason resolve_relative_url can fail is when there is no
> remote.$remote.url configuration option set for the current default
> remote ("origin"?).
>
> I guess I'm unclear about why cmd_sync is different from the
> existing callers.
Right, it's not. resolve_relative_url() is already calling
die() when that error condition is met, so the "|| exit" thing
can be removed entirely. I sent a patch on top of Johan's
first patch to remove the "|| exit" calls in all callers of
resolve_relative_url. That seems like the right thing to do;
in the very least it makes it easier to read. What do you
think?
>
> > diff --git a/git-submodule.sh b/git-submodule.sh
> > index 1c39b59..f89bdbe 100755
> > --- a/git-submodule.sh
> > +++ b/git-submodule.sh
> > @@ -634,6 +634,15 @@ cmd_sync()
> > do
> > name=$(module_name "$path")
> > url=$(git config -f .gitmodules --get submodule."$name".url)
> > +
> > + # Possibly a url relative to parent
> > + case "$url" in
> > + ./*|../*)
> > + url=$(resolve_relative_url "$url") ||
> > + die "failed to resolve relative submodule url for '$name'"
> > + ;;
> > + esac
> > +
> > if test -e "$path"/.git
> > then
> > (
>
> --
> Shawn.
--
David
--
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
