Re: [PATCH] submodule: teach foreach command a --revision tree-ish option
Junio C Hamano gits...@pobox.com writes: Assuming that the above guess is correct (which is a huge assumption, given the lack of clarity in the description), I think the feature might make sense. The example would have been a lot easier to follow if it were something like this: $ git submodule foreach --revision v1.0 'git grep -e frotz $sha1' Imagine you have a checkout of v2.0 of the superproject in your working tree, and you run git submodule foreach --revision v1.0. Further imagine a submodule S that used to exist back when the superproject was at v1.0 no longer exists in the current codebase (hence there is no such submodule in the working tree). Shouldn't the above foreach ... grep still try to find 'frotz' in the submodule S that was bound to v1.0 of the superproject? Given that your patch does not touch the part of cmd_foreach where it decides which submodule to descend into, it still will base its decision solely on the set of submodules that are bound to and have been git submodule inited in the version of the superproject that is _currently_ checked out, no? -- 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: [PATCH] submodule: teach foreach command a --revision tree-ish option
On Tue, Oct 9, 2012 at 2:12 AM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: Assuming that the above guess is correct (which is a huge assumption, given the lack of clarity in the description), I think the feature might make sense. The example would have been a lot easier to follow if it were something like this: $ git submodule foreach --revision v1.0 'git grep -e frotz $sha1' Imagine you have a checkout of v2.0 of the superproject in your working tree, and you run git submodule foreach --revision v1.0. Further imagine a submodule S that used to exist back when the superproject was at v1.0 no longer exists in the current codebase (hence there is no such submodule in the working tree). Shouldn't the above foreach ... grep still try to find 'frotz' in the submodule S that was bound to v1.0 of the superproject? Given that your patch does not touch the part of cmd_foreach where it decides which submodule to descend into, it still will base its decision solely on the set of submodules that are bound to and have been git submodule inited in the version of the superproject that is _currently_ checked out, no? That's a good observation. My use-case for this (poorly explained in the commit message) is as part of a release process, where I wish to apply corresponding tags to the superproject and its submodules like so: $ cd /path/to/superproject $ git tag -m 1.0 v1.0 deadbeef $ git submodule foreach --revision deadbeef \ 'git tag -m superproject 1.0 superproject-1.0 $sha1' Typically deadbeef may be a day or two behind HEAD and it's nice to be able to tag it and the submodules w/o having to switch everything to a detached HEAD. In my case, tagging and updating submodule revisions are somewhat common, while adding/removing submodules is a rare event. I didn't mention this issue explicitly because I thought it was covered by the existing documentation: Any submodules defined in the superproject but not checked out are ignored by this command. As you previously stated, I need to improve the documentation that goes along with this patch, so I'll call-out this limitation. I'm not sure what else can be done. You can't descend into a submodule that isn't there. j. -- 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: [PATCH] submodule: teach foreach command a --revision tree-ish option
Jay Soffian jaysoff...@gmail.com writes: On Tue, Oct 9, 2012 at 2:12 AM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: Assuming that the above guess is correct (which is a huge assumption, given the lack of clarity in the description), I think the feature might make sense. The example would have been a lot easier to follow if it were something like this: $ git submodule foreach --revision v1.0 'git grep -e frotz $sha1' Imagine you have a checkout of v2.0 of the superproject in your working tree, and you run git submodule foreach --revision v1.0. Further imagine a submodule S that used to exist back when the superproject was at v1.0 no longer exists in the current codebase (hence there is no such submodule in the working tree). Shouldn't the above foreach ... grep still try to find 'frotz' in the submodule S that was bound to v1.0 of the superproject? Given that your patch does not touch the part of cmd_foreach where it decides which submodule to descend into, it still will base its decision solely on the set of submodules that are bound to and have been git submodule inited in the version of the superproject that is _currently_ checked out, no? That's a good observation. My use-case for this (poorly explained in ... As you previously stated, I need to improve the documentation that goes along with this patch, so I'll call-out this limitation. I'm not sure what else can be done. You can't descend into a submodule that isn't there. As recent submodule rm work by Jens indicates, since 501770e (Move git-dir for submodules, 2011-08-15), you should be able to peek into submodules that have been git submodule inited but do not exist in the current checkout of the superproject. I think the right approach to implement this recurse foreach in the superproject tree that is not checkout out to the working tree feature should be: - Advertise it so that it is crystal clear that the command run by foreach may have to run in a bare repository of submodule to look at submodule's commit bound to the historical tree of the superproject; - Anytime you look at .gitmodules in the superproject, read from the historical tree's .gitmodules instead of from the working tree (I think this is necessary in order to get the $sm_name vs $sm_path mapping right); and - Locate submodule's $GIT_DIR in $GIT_DIR/modules/$sm_name of the superproject that corresponds to the submodule found in the historical tree in the superproject (or if it is the same repository as that is currently checked out, use $sm_path/.git), and error out when it is not available. An implementation that works only when all the submodules necessary in the historical tree in the superproject are still in the current checkout of the superproject may be fine as a quick throw-away hack, and it may even be acceptable as a good first step towards the real feature, but at least it needs to be protected by an error checking upfront (perhaps running diff-tree -r between the index and the historical tree to make sure there are no removed submodules that existed in the historical tree), if it does not bother to check with $GIT_DIR/modules/$sm_name in the superproject. Jens, anything I missed? -- 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: [PATCH] submodule: teach foreach command a --revision tree-ish option
Am 09.10.2012 20:24, schrieb Junio C Hamano: Jay Soffian jaysoff...@gmail.com writes: On Tue, Oct 9, 2012 at 2:12 AM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: Assuming that the above guess is correct (which is a huge assumption, given the lack of clarity in the description), I think the feature might make sense. The example would have been a lot easier to follow if it were something like this: $ git submodule foreach --revision v1.0 'git grep -e frotz $sha1' Imagine you have a checkout of v2.0 of the superproject in your working tree, and you run git submodule foreach --revision v1.0. Further imagine a submodule S that used to exist back when the superproject was at v1.0 no longer exists in the current codebase (hence there is no such submodule in the working tree). Shouldn't the above foreach ... grep still try to find 'frotz' in the submodule S that was bound to v1.0 of the superproject? Given that your patch does not touch the part of cmd_foreach where it decides which submodule to descend into, it still will base its decision solely on the set of submodules that are bound to and have been git submodule inited in the version of the superproject that is _currently_ checked out, no? That's a good observation. My use-case for this (poorly explained in ... As you previously stated, I need to improve the documentation that goes along with this patch, so I'll call-out this limitation. I'm not sure what else can be done. You can't descend into a submodule that isn't there. As recent submodule rm work by Jens indicates, since 501770e (Move git-dir for submodules, 2011-08-15), you should be able to peek into submodules that have been git submodule inited but do not exist in the current checkout of the superproject. I think the right approach to implement this recurse foreach in the superproject tree that is not checkout out to the working tree feature should be: - Advertise it so that it is crystal clear that the command run by foreach may have to run in a bare repository of submodule to look at submodule's commit bound to the historical tree of the superproject; I think we should even try to enforce that the user shouldn't use the work tree at all (although at the moment I can't come up with an idea how we could do that), as the work tree *will* be out of sync almost always when you need this option. Otherwise strange things would happen when using git submodule foreach --revision ... with a command which examines the work tree, as that won't be updated to the given revision. - Anytime you look at .gitmodules in the superproject, read from the historical tree's .gitmodules instead of from the working tree (I think this is necessary in order to get the $sm_name vs $sm_path mapping right); and Yes, that is definitely necessary in case submodules are added, removed or moved. - Locate submodule's $GIT_DIR in $GIT_DIR/modules/$sm_name of the superproject that corresponds to the submodule found in the historical tree in the superproject (or if it is the same repository as that is currently checked out, use $sm_path/.git), and error out when it is not available. Looking in $GIT_DIR/modules/$sm_name could make sense to tag even those submodules which aren't currently populated. But IIRC the tags in such repositories could not be pushed using current git even when you use the --recurse-submodules option because that only honors populated submodules. So for now it would suffice to only recurse into populated submodules. An implementation that works only when all the submodules necessary in the historical tree in the superproject are still in the current checkout of the superproject may be fine as a quick throw-away hack, and it may even be acceptable as a good first step towards the real feature, but at least it needs to be protected by an error checking upfront (perhaps running diff-tree -r between the index and the historical tree to make sure there are no removed submodules that existed in the historical tree), if it does not bother to check with $GIT_DIR/modules/$sm_name in the superproject. Jens, anything I missed? Nothing I can think of right now, the above is a pretty good summary. My gut feeling is that having git submodule foreach --revision ... recurse through submodules whose work trees are out of sync is pretty fragile and could easily lead to inconsistencies. So I tend to think adding a custom script to the release process Jay uses which does the tagging itself might be a better solution here. 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
Re: [PATCH] submodule: teach foreach command a --revision tree-ish option
On Tue, Oct 9, 2012 at 5:21 PM, Jens Lehmann jens.lehm...@web.de wrote: Nothing I can think of right now, the above is a pretty good summary. My gut feeling is that having git submodule foreach --revision ... recurse through submodules whose work trees are out of sync is pretty fragile and could easily lead to inconsistencies. So I tend to think adding a custom script to the release process Jay uses which does the tagging itself might be a better solution here. Opinions? I agree now that this is a perilous option, and that its use case may be so narrow that it may not worth adding. I am indeed already using a custom script, and maybe I should leave it at that. j. -- 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: [PATCH] submodule: teach foreach command a --revision tree-ish option
Jens Lehmann jens.lehm...@web.de writes: Am 09.10.2012 20:24, schrieb Junio C Hamano: ... I think the right approach to implement this recurse foreach in the superproject tree that is not checkout out to the working tree feature should be: - Advertise it so that it is crystal clear that the command run by foreach may have to run in a bare repository of submodule to look at submodule's commit bound to the historical tree of the superproject; I think we should even try to enforce that the user shouldn't use the work tree at all (although at the moment I can't come up with an idea how we could do that), as the work tree *will* be out of sync almost always when you need this option. Very good point. - Locate submodule's $GIT_DIR in $GIT_DIR/modules/$sm_name of the superproject that corresponds to the submodule found in the historical tree in the superproject (or if it is the same repository as that is currently checked out, use $sm_path/.git), and error out when it is not available. Looking in $GIT_DIR/modules/$sm_name could make sense to tag even those submodules which aren't currently populated. But IIRC the tags in such repositories could not be pushed using current git even when you use the --recurse-submodules option because that only honors populated submodules. So for now it would suffice to only recurse into populated submodules. There are million reasons why we shouldn't lightly think recurse submodules is a good idea, and I think this may be one of them. But you can always go to $GIT_DIR/modules/$sm_name and push from there, so I do not see it as a huge problem. Jens, anything I missed? Nothing I can think of right now, the above is a pretty good summary. My gut feeling is that having git submodule foreach --revision ... recurse through submodules whose work trees are out of sync is pretty fragile and could easily lead to inconsistencies. So I tend to think adding a custom script to the release process Jay uses which does the tagging itself might be a better solution here. Opinions? Well, I am not a good judge for that, as I've never been a big fan of submodule recurse myself anyway. But I think an addition that works only when the user never uses commands that use the working tree or the index is still a good thing to have. We could export a magic environment while running foreach script and make NEED_WORK_TREE check fail when it is set, or something, but we need to be careful about performance implications. foreach is not something that is worth sacrificing the general performance over. -- 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
[PATCH] submodule: teach foreach command a --revision tree-ish option
Teach git submodule foreach a --revision tree-ish option. This is useful in combination with $sha1 to perform git commands that take a revision argument. For example: $ git submodule foreach --revision v1.0 'git tag v1.0 $sha1' Previously, this would have required multiple steps: $ git checkout v1.0 $ git submodule update $ git submodule foreach 'git tag v1.0' Signed-off-by: Jay Soffian jaysoff...@gmail.com --- Documentation/git-submodule.txt | 7 ++- git-submodule.sh| 27 --- t/t7407-submodule-foreach.sh| 15 +++ 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index b4683bba1b..6c889f5fd6 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -17,7 +17,8 @@ SYNOPSIS [--reference repository] [--merge] [--recursive] [--] [path...] 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) n] [commit] [--] [path...] -'git submodule' [--quiet] foreach [--recursive] command +'git submodule' [--quiet] foreach [--recursive] [--revision tree-ish] + command 'git submodule' [--quiet] sync [--] [path...] @@ -180,6 +181,10 @@ foreach:: of each submodule before evaluating the command. If `--recursive` is given, submodules are traversed recursively (i.e. the given shell command is evaluated in nested submodules as well). + If `--revision tree-ish` is given, submodules are traversed starting + at the given tree-ish. Though this does not alter the submodule check + outs, it may be combined with $sha1 to perform git commands that can + operate on a particular commit, such as linkgit:git-tag[1]. A non-zero return from the command in any submodule causes the processing to terminate. This can be overridden by adding '|| :' to the end of the command. diff --git a/git-submodule.sh b/git-submodule.sh index ab6b1107b6..5e7458e155 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -10,7 +10,7 @@ USAGE=[--quiet] add [-b branch] [-f|--force] [--reference repository] [--] r or: $dashless [--quiet] init [--] [path...] or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference repository] [--merge] [--recursive] [--] [path...] or: $dashless [--quiet] summary [--cached|--files] [--summary-limit n] [commit] [--] [path...] - or: $dashless [--quiet] foreach [--recursive] command + or: $dashless [--quiet] foreach [--recursive] [--revision tree-ish] command or: $dashless [--quiet] sync [--] [path...] OPTIONS_SPEC= . git-sh-setup @@ -379,6 +379,7 @@ Use -f if you really want to add it. 2 cmd_foreach() { # parse $args after submodule ... foreach. + revision= while test $# -ne 0 do case $1 in @@ -388,6 +389,11 @@ cmd_foreach() --recursive) recursive=1 ;; + --revision) + git rev-parse --quiet --verify $2 /dev/null || usage + revision=$2 + shift + ;; -*) usage ;; @@ -404,7 +410,17 @@ cmd_foreach() # command in the subshell (and a recursive call to this function) exec 30 - module_list | + if test -n $revision + then + # make ls-tree output look like ls-files output + git ls-tree -r $revision | grep '^16 ' | + while read mode unused sha1 sm_path + do + echo $mode $sha1 0 $sm_path + done + else + module_list + fi | while read mode sha1 stage sm_path do die_if_unmatched $mode @@ -421,7 +437,12 @@ cmd_foreach() eval $@ if test -n $recursive then - cmd_foreach --recursive $@ + if test -n $revision + then + cmd_foreach --recursive --revision $sha1 $@ + else + cmd_foreach --recursive $@ + fi fi ) 3 3- || die $(eval_gettext Stopping at '\$sm_path'; script returned non-zero status.) diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh index 9b69fe2e14..5c798b901b 100755 --- a/t/t7407-submodule-foreach.sh +++ b/t/t7407-submodule-foreach.sh @@ -179,6 +179,21 @@ test_expect_success 'test foreach --quiet --recursive' ' test_cmp expect
Re: [PATCH] submodule: teach foreach command a --revision tree-ish option
Jay Soffian jaysoff...@gmail.com writes: Teach git submodule foreach a --revision tree-ish option. This is useful in combination with $sha1 to perform git commands that take a revision argument. The above says: - --revision T is added. OK. There is no information whatsoever what it does to convince us why it is useful. - This is useful. Huh? How can anybody supposed to agree or disagree with that claim, when nothing is said about what it does in the first place? For example: $ git submodule foreach --revision v1.0 'git tag v1.0 $sha1' Whose v1.0 does this example refer to? The first line of the proposed log message says it is tree-ish, which means that you can safely substitute --revision T with --revision $(git rev-parse T^{tree}), so it must name a concrete single object that is a tree (not a tree-ish). In which repository is that object found? The top-level superproject? All submodule repositories share the same object store with the superproject? The description doesn't make _any_ sense to me. The feature might be something worth considering about with a better description, but with the above, I can't tell if it is. + If `--revision tree-ish` is given, submodules are traversed starting + at the given tree-ish. What does are traversed starting at the given tree-ish? The desired or expected state of each submodule is recorded as a commit object name (not even commit-ish) in its superproject. Did you mean commit-ish? + Though this does not alter the submodule check + outs, it may be combined with $sha1 to perform git commands that can + operate on a particular commit, such as linkgit:git-tag[1]. Here is what I am guessing, partially with help from the horrible example: $ git submodule foreach --revision v1.0 'git tag v1.0 $sha1' Previously, this would have required multiple steps: $ git checkout v1.0 $ git submodule update $ git submodule foreach 'git tag v1.0' where there appears two v1.0 that are used for totally different purposes which does not help guessing. Perhaps --revision names a tree-ish taken from the top-level superproject, and for each submodule that appear in the tree in the superproject, the command specified by foreach is run with the usual $sha1, $name, $path set to the state in the submodules that top-level tree wants to have, and this is done without actually checking anything out. So the first v1.0 in that confusing example is about specifying a tree in the superproject repository, and the second v1.0 does not have any relationship with that first v1.0 (the first one could have been HEAD~2 when you have committed twice in the superproject since you tagged v1.0 and remembered that you forgot to tag its submodules). Assuming that the above guess is correct (which is a huge assumption, given the lack of clarity in the description), I think the feature might make sense. The example would have been a lot easier to follow if it were something like this: $ git submodule foreach --revision v1.0 'git grep -e frotz $sha1' @@ -379,6 +379,7 @@ Use -f if you really want to add it. 2 cmd_foreach() { # parse $args after submodule ... foreach. + revision= while test $# -ne 0 do case $1 in @@ -388,6 +389,11 @@ cmd_foreach() --recursive) recursive=1 ;; + --revision) + git rev-parse --quiet --verify $2 /dev/null || usage + revision=$2 Shouldn't this part of the code verify $2^{tree} instead to ensure that $2 is a tree-ish? + shift + ;; -*) usage ;; @@ -404,7 +410,17 @@ cmd_foreach() # command in the subshell (and a recursive call to this function) exec 30 - module_list | + if test -n $revision + then + # make ls-tree output look like ls-files output + git ls-tree -r $revision | grep '^16 ' | + while read mode unused sha1 sm_path + do + echo $mode $sha1 0 $sm_path + done + else + module_list + fi | Hrm, it is somewhat unfortunate that you cannot limit the set of submodules to apply foreach to, like other commands like init, update, status, etc. (not a new problem). -- 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