On Tue, Jun 4, 2013 at 8:48 AM, John Keeping <john@xxxxxxxxxxxxx> wrote: > On Tue, Jun 04, 2013 at 09:17:17PM +1000, Heiko Voigt wrote: >> On Tue, Jun 04, 2013 at 09:10:45AM +0100, John Keeping wrote: >> > On Tue, Jun 04, 2013 at 03:29:51PM +1000, Heiko Voigt wrote: >> > > On Mon, Jun 03, 2013 at 11:23:41PM +0100, John Keeping wrote: >> > > > > Sorry, I should have been more specific here. I saw that you did some >> > > > > changes to make "submodule add" do the right thing with relative paths, >> > > > > but the following change to t7406 does not work like I believe it >> > > > > should but instead makes the test fail: >> > > > > -------------------8<--------------------- >> > > > > diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh >> > > > > index a4ffea0..9766b9e 100755 >> > > > > --- a/t/t7406-submodule-update.sh >> > > > > +++ b/t/t7406-submodule-update.sh >> > > > > @@ -559,7 +559,9 @@ test_expect_success 'add different submodules to the same pa >> > > > > test_expect_success 'submodule add places git-dir in superprojects git-dir' ' >> > > > > (cd super && >> > > > > mkdir deeper && >> > > > > - git submodule add ../submodule deeper/submodule && >> > > > > + (cd deeper && >> > > > > + git submodule add ../../submodule submodule >> > > > > + ) && >> > > > > (cd deeper/submodule && >> > > > > git log > ../../expected >> > > > > ) && >> > > > > -------------------8<--------------------- >> > > > >> > > > Ah, ok. I think this case is problematic because the repository >> > > > argument is either relative to "remote.origin.url" or to the top of the >> > > > working tree if there is no "origin" remote. I wonder if we should just >> > > > die when a relative path is given for the repository and we're not at >> > > > the top of the working tree. >> > > >> > > Why not behave as if we are at the top of the working tree for relative >> > > paths? If there is an origin remote thats fine. If there is no origin >> > > remote you could warn that the path used is taken relative from the root >> > > of the superproject during add. What do you think? >> > >> > That's what the patch currently queued on "pu" does, which Jens wants to >> > change, isn't it? >> >> True I did not realize this when reading it the first time. But I think >> we should still not die when in a subdirectory. After all this series is >> trying to archive that the submodule command works in subdirectories >> seamlessly right? So you probably want to translate a relative path >> without "origin" remote given from a subdirectory to the superproject >> level and use that. Then you do not have to die. > > The problem is that sometimes you do want to adjust the path and > sometimes you don't. Reading git-submodule(1), it says: > > This may be either an absolute URL, or (if it begins with ./ or > ../), the location relative to the superproject’s origin > repository. > [snip] > If the superproject doesn’t have an origin configured the > superproject is its own authoritative upstream and the current > working directory is used instead. > > So I think it's quite reasonable to have a server layout that looks like > this: > > project > |- libs > | |- libA > | `- libB > |- core.git > > and with only core.git on your local system do: > > cd core/libs > git submodule add ../libs/libB > > expecting that to point to libB. But if we adjust the path then the > user has to do: > > git submodule add ../../libs/libB > > However, it is also perfectly reasonable to have no remote configured > and the library next to the repository itself. In which case we do want > to specify the additional "../" so that shell completion works in the > natural way. In submodule add, the leading '../' prefix on the repository url has always meant that the url is relative to the url of the current repo. The given repo-url is precisely what ends up in .gitmodules' submodule.$name.url. It works this way whether there is a remote configured or not. It does seem like we need to be cautious around this change. > The only way I can see to resolve the ambiguity is to die when we hit > this particular case. This should be acceptable because people > shouldn't be adding new submodules anywhere near as often as they > perform other submodule operations and it doesn't affect absolute URLs. I don't think I like that. But I don't know if I like anything I dreamed up, either. P -- 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