Re: What's cooking in git.git (May 2013, #09; Wed, 29)
Jens Lehmann jens.lehm...@web.de writes: Am 02.06.2013 20:50, schrieb Junio C Hamano: Jens Lehmann jens.lehm...@web.de writes: Am 30.05.2013 01:58, schrieb Junio C Hamano: * jl/submodule-mv (2013-04-23) 5 commits (merged to 'next' on 2013-04-23 at c04f574) + submodule.c: duplicate real_path's return value (merged to 'next' on 2013-04-19 at 45ae3c9) + rm: delete .gitmodules entry of submodules removed from the work tree + Teach mv to update the path entry in .gitmodules for moved submodules + Teach mv to move submodules using a gitfile + Teach mv to move submodules together with their work trees git mv A B when moving a submodule A does the right thing, inclusing relocating its working tree and adjusting the paths in the .gitmodules file. detailed discussion snipped So my gut feeling of the fix at this point in the evolution of the program may be to error out with a message like You have local changes to .gitmodules; please stash it before moving or removing. Yeah, me too thinks that this is a sane short term solution (even though a git submodule add currently happily stages any unstaged modifications to the .gitmodules file too, that should not stop us from doing better for rm and mv ;-). And I also agree that in the long run the the git-config aware merge driver together with the 3-way merge of a modified .gitmodules file you described is the best solution. But I'd really like to complete the recursive update before tackling that, so for now I just added these two to the to-do list on my GitHub wiki page. I'll resubmit this series with the strlen() fix and the erroring out in case of unstaged modifications of the .gitmodules file as soon as I find some time. Ping? No need to hurry, but just to make sure this didn't disappear from everybody's radar. Thanks. -- 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 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: What's cooking in git.git (May 2013, #09; Wed, 29)
On Tue, Jun 04, 2013 at 11:39:25PM +0200, Jens Lehmann wrote:
Am 04.06.2013 14:48, schrieb John Keeping:
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.
Exactly.
The only way I can see to resolve the ambiguity is to die when we hit
this particular case.
Hmm, I'm not so sure about that. Don't the first three lines in
resolve_relative_url() show how to distinguish between these two
cases?
resolve_relative_url ()
{
remote=$(get_default_remote)
remoteurl=$(git config remote.$remote.url) ||
remoteurl=$(pwd) # the repository is its own authoritative
upstream
...
If it's this simple, yes. But I think there's also a third possibility
that combines both of these: what if the local directory structure is
the same as that on the origin remote? Then origin exists but we
still want to adjust for the subdirectory.
The risk is that I can't see a behaviour that doesn't seem to choose
whether to convert the given path or not arbitrarily. Even knowing the
rules, I expect that I could end up being surprised by this if I create
a new repository and haven't set up origin yet.
--
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: What's cooking in git.git (May 2013, #09; Wed, 29)
John Keeping j...@keeping.me.uk writes: I started looking at this over the weekend but didn't get time to get something ready to be submitted. I did find a couple of issues in cmd_foreach that make me think this topic should be dropped when next is rewound and held in pu waiting for a re-roll. Thanks for a heads-up. -- 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: What's cooking in git.git (May 2013, #09; Wed, 29)
Am 02.06.2013 20:50, schrieb Junio C Hamano: Jens Lehmann jens.lehm...@web.de writes: Am 30.05.2013 01:58, schrieb Junio C Hamano: * jl/submodule-mv (2013-04-23) 5 commits (merged to 'next' on 2013-04-23 at c04f574) + submodule.c: duplicate real_path's return value (merged to 'next' on 2013-04-19 at 45ae3c9) + rm: delete .gitmodules entry of submodules removed from the work tree + Teach mv to update the path entry in .gitmodules for moved submodules + Teach mv to move submodules using a gitfile + Teach mv to move submodules together with their work trees git mv A B when moving a submodule A does the right thing, inclusing relocating its working tree and adjusting the paths in the .gitmodules file. detailed discussion snipped So my gut feeling of the fix at this point in the evolution of the program may be to error out with a message like You have local changes to .gitmodules; please stash it before moving or removing. Yeah, me too thinks that this is a sane short term solution (even though a git submodule add currently happily stages any unstaged modifications to the .gitmodules file too, that should not stop us from doing better for rm and mv ;-). And I also agree that in the long run the the git-config aware merge driver together with the 3-way merge of a modified .gitmodules file you described is the best solution. But I'd really like to complete the recursive update before tackling that, so for now I just added these two to the to-do list on my GitHub wiki page. I'll resubmit this series with the strlen() fix and the erroring out in case of unstaged modifications of the .gitmodules file as soon as I find some time. -- 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: What's cooking in git.git (May 2013, #09; Wed, 29)
Am 31.05.2013 21:40, schrieb John Keeping: On Thu, May 30, 2013 at 09:23:40PM +0200, Jens Lehmann wrote: Am 30.05.2013 01:58, schrieb Junio C Hamano: * jk/submodule-subdirectory-ok (2013-04-24) 3 commits (merged to 'next' on 2013-04-24 at 6306b29) + submodule: fix quoting in relative_path() (merged to 'next' on 2013-04-22 at f211e25) + submodule: drop the top-level requirement + rev-parse: add --prefix option Allow various subcommands of git submodule to be run not from the top of the working tree of the superproject. The summary and status commands are looking good in this version (they are now showing the submodule directory paths relative to the current directory). Apart from that my other remarks from gmane $221575 still seem to apply. And this series has only tests for status, summary and add (and that just with an absolute URL), I'd rather like to see a test for each submodule command (and a relative add to) to document the desired behavior. To summarize what I think are the outstanding issues from your email: * Should '$sm_path' be relative in submodule foreach? * submodule add with a relative path * submodule init initializes all submodules * Tests The current version does make '$sm_path' relative in submodule foreach, although it's hard to spot because we have to leave doing so until right before the eval. Yes. If I read the code correctly the submodule is cd'ed in before the foreach command is executed, so $sm_path should only be used for displaying info about where the command is executed anyway. Looks like your code is doing the right thing adjusting $sm_path to be relative to the directory the user is in. But a test showing that would really be nice ;-) I'm not sure what you mean about submodule add - the new version treats the path argument as relative (providing it is not an absolute path). The repository argument is not changed by running from a subdirectory but I think that's correct since it is documented as being relative to the superproject's origin repository. 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- submodule init is behaving in the same way as deinit - if you say submodule init . then it will only initialize submodules below the current directory. The difference is that deinit dies if it is not given any arguments whereas init will initialize everything from the top level down. I'm not sure whether to change this; given the direction git add -u is heading in for 2.0 I think the current behaviour is the most consistent with the rest of Git. I meant that both commands still print the submodule names from the top-level directory, not the one the user is in. -- 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: What's cooking in git.git (May 2013, #09; Wed, 29)
On Mon, Jun 03, 2013 at 11:47:23PM +0200, Jens Lehmann wrote: Am 31.05.2013 21:40, schrieb John Keeping: The current version does make '$sm_path' relative in submodule foreach, although it's hard to spot because we have to leave doing so until right before the eval. Yes. If I read the code correctly the submodule is cd'ed in before the foreach command is executed, so $sm_path should only be used for displaying info about where the command is executed anyway. Looks like your code is doing the right thing adjusting $sm_path to be relative to the directory the user is in. But a test showing that would really be nice ;-) Agreed. I've also noticed that the legacy path variable hasn't been adjusted and the printing of the module paths does not make them relative. I'll fix them in the next version. I'm not sure what you mean about submodule add - the new version treats the path argument as relative (providing it is not an absolute path). The repository argument is not changed by running from a subdirectory but I think that's correct since it is documented as being relative to the superproject's origin repository. 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. submodule init is behaving in the same way as deinit - if you say submodule init . then it will only initialize submodules below the current directory. The difference is that deinit dies if it is not given any arguments whereas init will initialize everything from the top level down. I'm not sure whether to change this; given the direction git add -u is heading in for 2.0 I think the current behaviour is the most consistent with the rest of Git. I meant that both commands still print the submodule names from the top-level directory, not the one the user is in. Will fix. -- 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
Re: What's cooking in git.git (May 2013, #09; Wed, 29)
Jens Lehmann jens.lehm...@web.de writes:
Am 30.05.2013 01:58, schrieb Junio C Hamano:
* jl/submodule-mv (2013-04-23) 5 commits
(merged to 'next' on 2013-04-23 at c04f574)
+ submodule.c: duplicate real_path's return value
(merged to 'next' on 2013-04-19 at 45ae3c9)
+ rm: delete .gitmodules entry of submodules removed from the work tree
+ Teach mv to update the path entry in .gitmodules for moved submodules
+ Teach mv to move submodules using a gitfile
+ Teach mv to move submodules together with their work trees
git mv A B when moving a submodule A does the right thing,
inclusing relocating its working tree and adjusting the paths in
the .gitmodules file.
There are only two issues I'm aware of:
*) When the .gitmodules file is already modified but unchanged
running rm or mv on a submodule will stage those changes too.
*) There is a harmless but unnecessary double invocation of strlen()
in the function (fixed by the diff below).
I plan to fix the first issue in another patch which would also get
rid of the second issue, as exactly that code would have to be touched
anyways.
Does that make sense?
In general I think whatever you think that makes sense in this area
would make sense ;-).
I do not feel confident that I am reading what you mean by already
modified but unchanged right. Do you mean the working tree version
is different from both HEAD and the index (HEAD and the index may or
may not match and that does not change the situation)? Assuming
that is the case, i.e. the situation you are worried about is:
When .gitmodules has a local modification the user chose not
to add yet. Then the user does git rm/mv on a submodule
that is unrelated to the submodule whose setting the user has
changes for.
I am curious what the plan is to fix this. An obviously safe
thing to do is to error out with a You have local modification;
please 'git add .gitmodules' first. but if that advice/suggestion
is always the right course of action for the user, it invites then
why doesn't Git do so for me?, which would in turn support that it
is not an issue in the first place (it deserves to be mentioned in a
warning, adding your local modifications together with change
needed for rm/mv, though).
I think in the ideal world, you may want to apply the change needed
for rm/mv to the version in the index, and then update the working
tree version by doing a 3-way merge. We already know that eventually
we would need a merge driver that is specific to the file format
that git-config uses, possibly even taking an advantage of the
knowledge of not just the file format but also the semantics of
individual variables [*1*], so we may want to keep it in mind that a
three-way merge would be the eventual goal, while settling on an
error out on local mod just like git checkout anotherbranch used
to always stop (before we taught the -m option to it) when a local
change needed a 3-way merge to be carried along to the new branch.
So my gut feeling of the fix at this point in the evolution of the
program may be to error out with a message like You have local
changes to .gitmodules; please stash it before moving or removing.
[Footnote]
*1* I think all the variables in .gitmodules are single-valued, so
the original submodule.dir.path's value was dir, the local
modification by the user was to make it folder, and rm wants
to delete an unrelated submodule.mod.* altogether, we can apply
the usual 3-way merge policy per variable basis to update
submodule.dir.pah to folder.
A more general merge driver to handle git-config format files
would have a way to be told that some variables are additive
with the -Xdriver-specific-option mechanism. When variable
foo.bar is specified as a multi-valued set of variables, the
original has a single foo.bar=xyzzy, one side adds a
foo.bar=frotz while the other side modifies the original to
foo.bar=nitfol, the ideal way for such a merge driver to
operate is to leave two definitions (xyzzy will be gone and
frotz and nitfol remain).
But I highly suspect that would need a much larger change to the
configuration file parser and writer that is totally out of
scope with this change, and that is why my recommendation at
this point is just to follow the example of pre--m era of git
checkout anotherbranch.
--8-
diff --git a/submodule.c b/submodule.c
index edfc23c..4670af7 100644
--- a/submodule.c
+++ b/submodule.c
@@ -102,7 +102,7 @@ void stage_updated_gitmodules(void)
struct cache_entry *ce;
int namelen = strlen(.gitmodules);
- pos = cache_name_pos(.gitmodules, strlen(.gitmodules));
+ pos = cache_name_pos(.gitmodules, namelen);
if (pos 0) {
warning(_(could not find .gitmodules in index));
return;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a
Re: What's cooking in git.git (May 2013, #09; Wed, 29)
Thomas Rast tr...@inf.ethz.ch writes:
This interacts with the tests from
* fc/at-head (2013-05-08) 13 commits
to fail valgrind in t1508 like so:
==22927== Invalid read of size 1
==22927==at 0x4C2C71B: __GI_strnlen (mc_replace_strmem.c:377)
==22927==by 0x567FF6B: vfprintf (in /lib64/libc-2.17.so)
==22927==by 0x56AC194: vsnprintf (in /lib64/libc-2.17.so)
==22927==by 0x4EAC39: vreportf (usage.c:12)
==22927==by 0x4EACA4: die_builtin (usage.c:36)
==22927==by 0x4EAEE0: die (usage.c:103)
==22927==by 0x4D8C51: get_sha1_1 (sha1_name.c:542)
==22927==by 0x4D8E5D: get_sha1_with_context_1 (sha1_name.c:1299)
==22927==by 0x4D992A: get_sha1_with_context (sha1_name.c:1417)
==22927==by 0x4D99E1: get_sha1 (sha1_name.c:1124)
==22927==by 0x45E1AC: cmd_rev_parse (rev-parse.c:761)
==22927==by 0x4051B3: handle_internal_command (git.c:284)
==22927== Address 0x5bebccb is 11 bytes inside a block of size 22 free'd
==22927==at 0x4C2ACDA: free (vg_replace_malloc.c:468)
==22927==by 0x4D8C37: get_sha1_1 (sha1_name.c:541)
==22927==by 0x4D8E5D: get_sha1_with_context_1 (sha1_name.c:1299)
==22927==by 0x4D992A: get_sha1_with_context (sha1_name.c:1417)
==22927==by 0x4D99E1: get_sha1 (sha1_name.c:1124)
==22927==by 0x45E1AC: cmd_rev_parse (rev-parse.c:761)
==22927==by 0x4051B3: handle_internal_command (git.c:284)
==22927==by 0x4053E7: main (git.c:492)
I think it's enough to squash this little change; leaking some memory
immediately before die() is not too bad, especially if we're going to
pass real_ref+11 into die()...
Good catch, thanks. when !len and real_ref is the current branch,
str just points into real_ref that is geting freed.
diff --git i/sha1_name.c w/sha1_name.c
index 5ea16ff..a07558d 100644
--- i/sha1_name.c
+++ w/sha1_name.c
@@ -538,7 +538,6 @@ static int get_sha1_basic(const char *str, int len,
unsigned char *sha1)
back to %s., len, str,
show_date(co_time, co_tz,
DATE_RFC2822));
else {
- free(real_ref);
die(Log for '%.*s' only has %d entries.,
len, str, co_cnt);
}
--
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: What's cooking in git.git (May 2013, #09; Wed, 29)
Junio C Hamano gitster at pobox.com writes: * kb/status-ignored-optim-2 (2013-05-29) 1 commit - dir.c: fix ignore processing within not-ignored directories Fix 1.8.3 regressions in the .gitignore path exclusion logic. Hi, I see that the Tested-by line in kb/status-ignored-optim-2 (3973cbd) doesn't contain my e-mail address. I personally have no particular preference whether it's there or not, but I noticed that it usually is on Tested-by lines, so I'm wondering if I have slipped on some standard practice. Would be useful to know in case I get the chance to help out more :) Thanks, Øsse -- 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: What's cooking in git.git (May 2013, #09; Wed, 29)
On Thu, May 30, 2013 at 09:23:40PM +0200, Jens Lehmann wrote: Am 30.05.2013 01:58, schrieb Junio C Hamano: * jk/submodule-subdirectory-ok (2013-04-24) 3 commits (merged to 'next' on 2013-04-24 at 6306b29) + submodule: fix quoting in relative_path() (merged to 'next' on 2013-04-22 at f211e25) + submodule: drop the top-level requirement + rev-parse: add --prefix option Allow various subcommands of git submodule to be run not from the top of the working tree of the superproject. The summary and status commands are looking good in this version (they are now showing the submodule directory paths relative to the current directory). Apart from that my other remarks from gmane $221575 still seem to apply. And this series has only tests for status, summary and add (and that just with an absolute URL), I'd rather like to see a test for each submodule command (and a relative add to) to document the desired behavior. To summarize what I think are the outstanding issues from your email: * Should '$sm_path' be relative in submodule foreach? * submodule add with a relative path * submodule init initializes all submodules * Tests The current version does make '$sm_path' relative in submodule foreach, although it's hard to spot because we have to leave doing so until right before the eval. I'm not sure what you mean about submodule add - the new version treats the path argument as relative (providing it is not an absolute path). The repository argument is not changed by running from a subdirectory but I think that's correct since it is documented as being relative to the superproject's origin repository. submodule init is behaving in the same way as deinit - if you say submodule init . then it will only initialize submodules below the current directory. The difference is that deinit dies if it is not given any arguments whereas init will initialize everything from the top level down. I'm not sure whether to change this; given the direction git add -u is heading in for 2.0 I think the current behaviour is the most consistent with the rest of Git. But I'm not sure if it's better to have another iteration of this series or to address the open issues a follow-up series. Having status, summary and add - at least with absolute URLs - lose the toplevel requirement is already a huge improvement IMO. Opinions? I think the only thing outstanding is tests. I'm happy to add those as a follow-up or in a re-roll. -- 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: What's cooking in git.git (May 2013, #09; Wed, 29)
Junio C Hamano gits...@pobox.com writes:
* rr/die-on-missing-upstream (2013-05-22) 2 commits
- sha1_name: fix error message for @{N}, @{date}
- sha1_name: fix error message for @{u}
When a reflog notation is used for implicit current branch, we
did not say which branch and worse said branch ''.
Will merge to 'next'.
This interacts with the tests from
* fc/at-head (2013-05-08) 13 commits
to fail valgrind in t1508 like so:
==22927== Invalid read of size 1
==22927==at 0x4C2C71B: __GI_strnlen (mc_replace_strmem.c:377)
==22927==by 0x567FF6B: vfprintf (in /lib64/libc-2.17.so)
==22927==by 0x56AC194: vsnprintf (in /lib64/libc-2.17.so)
==22927==by 0x4EAC39: vreportf (usage.c:12)
==22927==by 0x4EACA4: die_builtin (usage.c:36)
==22927==by 0x4EAEE0: die (usage.c:103)
==22927==by 0x4D8C51: get_sha1_1 (sha1_name.c:542)
==22927==by 0x4D8E5D: get_sha1_with_context_1 (sha1_name.c:1299)
==22927==by 0x4D992A: get_sha1_with_context (sha1_name.c:1417)
==22927==by 0x4D99E1: get_sha1 (sha1_name.c:1124)
==22927==by 0x45E1AC: cmd_rev_parse (rev-parse.c:761)
==22927==by 0x4051B3: handle_internal_command (git.c:284)
==22927== Address 0x5bebccb is 11 bytes inside a block of size 22 free'd
==22927==at 0x4C2ACDA: free (vg_replace_malloc.c:468)
==22927==by 0x4D8C37: get_sha1_1 (sha1_name.c:541)
==22927==by 0x4D8E5D: get_sha1_with_context_1 (sha1_name.c:1299)
==22927==by 0x4D992A: get_sha1_with_context (sha1_name.c:1417)
==22927==by 0x4D99E1: get_sha1 (sha1_name.c:1124)
==22927==by 0x45E1AC: cmd_rev_parse (rev-parse.c:761)
==22927==by 0x4051B3: handle_internal_command (git.c:284)
==22927==by 0x4053E7: main (git.c:492)
I think it's enough to squash this little change; leaking some memory
immediately before die() is not too bad, especially if we're going to
pass real_ref+11 into die()...
diff --git i/sha1_name.c w/sha1_name.c
index 5ea16ff..a07558d 100644
--- i/sha1_name.c
+++ w/sha1_name.c
@@ -538,7 +538,6 @@ static int get_sha1_basic(const char *str, int len,
unsigned char *sha1)
back to %s., len, str,
show_date(co_time, co_tz,
DATE_RFC2822));
else {
- free(real_ref);
die(Log for '%.*s' only has %d entries.,
len, str, co_cnt);
}
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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: What's cooking in git.git (May 2013, #09; Wed, 29)
Am 30.05.2013 01:58, schrieb Junio C Hamano:
* jl/submodule-mv (2013-04-23) 5 commits
(merged to 'next' on 2013-04-23 at c04f574)
+ submodule.c: duplicate real_path's return value
(merged to 'next' on 2013-04-19 at 45ae3c9)
+ rm: delete .gitmodules entry of submodules removed from the work tree
+ Teach mv to update the path entry in .gitmodules for moved submodules
+ Teach mv to move submodules using a gitfile
+ Teach mv to move submodules together with their work trees
git mv A B when moving a submodule A does the right thing,
inclusing relocating its working tree and adjusting the paths in
the .gitmodules file.
There are only two issues I'm aware of:
*) When the .gitmodules file is already modified but unchanged
running rm or mv on a submodule will stage those changes too.
*) There is a harmless but unnecessary double invocation of strlen()
in the function (fixed by the diff below).
I plan to fix the first issue in another patch which would also get
rid of the second issue, as exactly that code would have to be touched
anyways.
Does that make sense?
--8-
diff --git a/submodule.c b/submodule.c
index edfc23c..4670af7 100644
--- a/submodule.c
+++ b/submodule.c
@@ -102,7 +102,7 @@ void stage_updated_gitmodules(void)
struct cache_entry *ce;
int namelen = strlen(.gitmodules);
- pos = cache_name_pos(.gitmodules, strlen(.gitmodules));
+ pos = cache_name_pos(.gitmodules, namelen);
if (pos 0) {
warning(_(could not find .gitmodules in index));
return;
--
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: What's cooking in git.git (May 2013, #09; Wed, 29)
Am 30.05.2013 01:58, schrieb Junio C Hamano: * jk/submodule-subdirectory-ok (2013-04-24) 3 commits (merged to 'next' on 2013-04-24 at 6306b29) + submodule: fix quoting in relative_path() (merged to 'next' on 2013-04-22 at f211e25) + submodule: drop the top-level requirement + rev-parse: add --prefix option Allow various subcommands of git submodule to be run not from the top of the working tree of the superproject. The summary and status commands are looking good in this version (they are now showing the submodule directory paths relative to the current directory). Apart from that my other remarks from gmane $221575 still seem to apply. And this series has only tests for status, summary and add (and that just with an absolute URL), I'd rather like to see a test for each submodule command (and a relative add to) to document the desired behavior. But I'm not sure if it's better to have another iteration of this series or to address the open issues a follow-up series. Having status, summary and add - at least with absolute URLs - lose the toplevel requirement is already a huge improvement IMO. Opinions? -- 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
What's cooking in git.git (May 2013, #09; Wed, 29)
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.
Usually I avoid issuing What's cooking back-to-back, but because I
will be offline for a few days, here is a snapshot of tonight's
status.
Some topics that have been cooking in 'next' from the previous cycle
have now graduated to 'master', so the RelNotes have been updated.
Nothing new in 'next', and it has not been rewound yet, which will
hopefully happen this weekend, and after that post 1.8.3 cycle
really starts.
Also a rather serious regression on path-exclusion logic (most
notably seen in .gitignore) has been found and quickly patched (it
hasn't been merged to 'master' yet, though).
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--
[Graduated to master]
* as/check-ignore (2013-04-29) 6 commits
(merged to 'next' on 2013-04-30 at 646931f)
+ t0008: use named pipe (FIFO) to test check-ignore streaming
(merged to 'next' on 2013-04-21 at 7515aa8)
+ Documentation: add caveats about I/O buffering for check-{attr,ignore}
+ check-ignore: allow incremental streaming of queries via --stdin
+ check-ignore: move setup into cmd_check_ignore()
+ check-ignore: add -n / --non-matching option
+ t0008: remove duplicated test fixture data
Enhance check-ignore (1.8.2 update) to work more like check-attr
over bidi-pipes.
* fc/transport-helper-error-reporting (2013-05-10) 12 commits
(merged to 'next' on 2013-05-10 at 2a9af4b)
+ transport-helper: fix remote helper namespace regression
+ test: remote-helper: add missing and
(merged to 'next' on 2013-04-25 at 3358f1a)
+ t5801: VAR=VAL shell_func args is forbidden
(merged to 'next' on 2013-04-22 at 5ba6467)
+ transport-helper: update remote helper namespace
+ transport-helper: trivial code shuffle
+ transport-helper: warn when refspec is not used
+ transport-helper: clarify pushing without refspecs
+ transport-helper: update refspec documentation
+ transport-helper: clarify *:* refspec
+ transport-helper: improve push messages
+ transport-helper: mention helper name when it dies
+ transport-helper: report errors properly
(this branch is tangled with js/transport-helper-error-reporting-fix.)
Update transport helper to report errors and maintain ref hierarchy
used to keep track of remote helper state better.
* jc/prune-all (2013-04-25) 4 commits
(merged to 'next' on 2013-04-26 at 97a7387)
+ prune: introduce OPT_EXPIRY_DATE() and use it
(merged to 'next' on 2013-04-22 at b00ccf6)
+ api-parse-options.txt: document no- for non-boolean options
+ git-gc.txt, git-reflog.txt: document new expiry options
+ date.c: add parse_expiry_date()
(this branch is used by mh/packed-refs-various.)
We used the approxidate() parser for --expire=timestamp options
of various commands, but it is better to treat --expire=all and
--expire=now a bit more specially than using the current timestamp.
Update git gc and git reflog with a new parsing function for
expiry dates.
* jh/checkout-auto-tracking (2013-04-21) 8 commits
(merged to 'next' on 2013-04-22 at 2356700)
+ glossary: Update and rephrase the definition of a remote-tracking branch
+ branch.c: Validate tracking branches with refspecs instead of refs/remotes/*
+ t9114.2: Don't use --track option against svn-remote-tracking branches
+ t7201.24: Add refspec to keep --track working
+ t3200.39: tracking setup should fail if there is no matching refspec.
+ checkout: Use remote refspecs when DWIMming tracking branches
+ t2024: Show failure to use refspec when DWIMming remote branch names
+ t2024: Add tests verifying current DWIM behavior of 'git checkout branch'
Updates git checkout foo that DWIMs the intended upstream and
turns it into git checkout -t -b foo remotes/origin/foo to
correctly take existing remote definitions into account. The
remote origin may be what uniquely map its own branch to
remotes/some/where/foo but that some/where may not be origin.
* jk/lookup-object-prefer-latest (2013-05-02) 1 commit
(merged to 'next' on 2013-05-06 at cc59dcc)
+ lookup_object: prioritize recently found objects
Optimizes object lookup when the object hashtable starts to become
crowded.
* jk/subtree-do-not-push-if-split-fails (2013-05-01) 1 commit
(merged to 'next' on 2013-05-06 at 81bdf37)
+ contrib/subtree: don't delete remote branches if split fails
git subtree (in contrib/) had one codepath with loose error
checks to lose data at the remote side.
* jk/test-output (2013-05-06) 3 commits
(merged to 'next' on 2013-05-06 at 7c03af3)
+ t/Makefile: don't define TEST_RESULTS_DIRECTORY recursively
(merged to 'next' on 2013-05-01 at 63827c9)
+ test output: respect $TEST_OUTPUT_DIRECTORY
+ t/Makefile: fix result handling with TEST_OUTPUT_DIRECTORY
When
