[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    [PATCH v2] submodule add: improve message when resolving a relative
From:       Jens Lehmann <Jens.Lehmann () web ! de>
Date:       2011-05-31 21:34:47
Message-ID: 4DE55EF7.9050609 () web ! de
[Download RAW message or body]

A "git submodule add ../sub" interprets "../sub" relative to the default
remote of the superproject. To be able to do that, a url for that remote
has to be set in the superproject's .git/config. If that is not the case
the command fails with:
	"remote (origin) does not have a url defined in .git/config"

This neither mentions the relative repository nor that the .git/config of
the superproject is the one with the missing url. And as a novice user
could assume that relative paths would work just like absolute paths do
in the filesystem and run into this by accident, the message is not very
helpful.

So change that to
	Cannot resolve "../sub" relative to this repository's "origin"
	remote: The remote's URL is not set in .git/config
to give the user a clue that "git submodule add" interprets a relative
path as being relative to its default remote, not the work tree.

Thanks-to: Marc Branchaud <marcnarc@xiplink.com>
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Thanks for your review, here is the updated patch.

Am 31.05.2011 22:57, schrieb Marc Branchaud:
> However, overall I think this is a pretty fragile way to handle relative
> paths.  Consider:
> 
> - The super-repo must be a clone in order for this to work at all.
> 
> - The super-repo cannot be checked out on a detached HEAD.
> 
> - The current code rewrites the URL so that any relative path is either
> rejected or munged into an absolute remote URL.
> 
> It seems to me that this feature will only work in a fairly narrow set of
> circumstances, and even when it does work it's likely to do something
> unexpected (think of a super-repo with several remotes).

And even worse: it defies the principle of least surprise when I can't
just replace an absolute filesystem path with a relative one just like
almost everywhere else ... an option enabling this behavior might have
been a better way in hindsight.

> Back when Junio accepted the original patch, he said "If you maintain and
> serve a set related projects you need to give the users a single URL (per
> where the user is and how to reach the server)."  I'm not sure I understand
> that:  Why would the users be adding their own submodules to the
> superproject?  Wouldn't the superproject define the submodules in for them?

I can't tell about that reasoning, but it might make sense in a way I don't
understand yet ...

> I think it would be better to either just reject relative paths entirely, or
> accept any relative path as-is and display a warning that the submodule is
> only valid on the local machine.  (Perhaps one day receive-pack could even be
> taught to reject any pushes with a .gitmodules file containing a relative URL.)

Breaking backwards compatibility should not be considered easily. I tend to
not change such things until I understand the use cases for the solution,
especially as submodule use cases tend to be very different ...


 git-submodule.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index d189a24..587d74e 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -34,7 +34,7 @@ resolve_relative_url ()
 {
 	remote=$(get_default_remote)
 	remoteurl=$(git config "remote.$remote.url") ||
-		die "remote ($remote) does not have a url defined in .git/config"
+		die "Cannot resolve \"$1\" relative to this repository's \"$remote\" remote: The \
remote's URL is not set in .git/config"  url="$1"
 	remoteurl=${remoteurl%/}
 	sep=/
-- 
1.7.5.3.445.g9c0c23

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[prev in list] [next in list] [prev in thread] [next in thread]