Re: Re: Re: What's cooking in git.git (May 2013, #09; Wed, 29)
On Tue, Jun 04, 2013 at 06:57:34PM -0400, Phil Hord wrote: On Tue, Jun 4, 2013 at 8:48 AM, John Keeping j...@keeping.me.uk wrote: 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. I've decided that I will make it die (unless anyone comes up with an obviously correct solution before I get around to sending the reroll) because it's a lot easier to loosen that in the future than to change it if we get the behaviour wrong now. I don't want to hold every other subcommand hostage to this one case. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: Re: What's cooking in git.git (May 2013, #09; Wed, 29)
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. Cheers Heiko -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: Re: What's cooking in git.git (May 2013, #09; Wed, 29)
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. 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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: Re: What's cooking in git.git (May 2013, #09; Wed, 29)
On Tue, Jun 4, 2013 at 8:48 AM, John Keeping j...@keeping.me.uk 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: What's cooking in git.git (May 2013, #09; Wed, 29)
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? Cheers Heiko -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
