From fb43e31f2b43076e7a30c9cd00d0241cb8cf97eb Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Tue, 23 Feb 2016 19:32:13 -0800 Subject: submodule: try harder to fetch needed sha1 by direct fetching sha1 When reviewing a change that also updates a submodule in Gerrit, a common review practice is to download and cherry-pick the patch locally to test it. However when testing it locally, the 'git submodule update' may fail fetching the correct submodule sha1 as the corresponding commit in the submodule is not yet part of the project history, but also just a proposed change. If $sha1 was not part of the default fetch, we try to fetch the $sha1 directly. Some servers however do not support direct fetch by sha1, which leads git-fetch to fail quickly. We can fail ourselves here as the still missing sha1 would lead to a failure later in the checkout stage anyway, so failing here is as good as we can get. Helped-by: Junio C Hamano Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- git-submodule.sh | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) (limited to 'git-submodule.sh') diff --git a/git-submodule.sh b/git-submodule.sh index 9bc5c5f94d..43c68deee9 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -591,6 +591,24 @@ cmd_deinit() done } +is_tip_reachable () ( + clear_local_git_env + cd "$1" && + rev=$(git rev-list -n 1 "$2" --not --all 2>/dev/null) && + test -z "$rev" +) + +fetch_in_submodule () ( + clear_local_git_env + cd "$1" && + case "$2" in + '') + git fetch ;; + *) + git fetch $(get_default_remote) "$2" ;; + esac +) + # # Update each submodule path to correct revision, using clone and checkout as needed # @@ -745,10 +763,15 @@ Maybe you want to use 'update --init'?")" then # Run fetch only if $sha1 isn't present or it # is not reachable from a ref. - (clear_local_git_env; cd "$sm_path" && - ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) && - test -z "$rev") || git-fetch)) || + is_tip_reachable "$sm_path" "$sha1" || + fetch_in_submodule "$sm_path" || die "$(eval_gettext "Unable to fetch in submodule path '\$displaypath'")" + + # Now we tried the usual fetch, but $sha1 may + # not be reachable from any of the refs + is_tip_reachable "$sm_path" "$sha1" || + fetch_in_submodule "$sm_path" "$sha1" || + die "$(eval_gettext "Fetched in submodule path '\$displaypath', but it did not contain $sha1. Direct fetching of that commit failed.")" fi # Is this something we just cloned? -- cgit v1.2.1 From ea2fa1040d14f1b7aab8fd78cc3ff4d41abc57a1 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Tue, 29 Mar 2016 18:27:41 -0700 Subject: submodule foreach: correct path display in recursive submodules The `prefix` was put in front of the display path unconditionally. This is wrong as any relative path computation would need to be at the front, so include the prefix into the display path. The new test replicates the previous test with the difference of executing from a sub directory. By executing from a sub directory all we would expect all displayed paths to be prefixed by '../'. Prior to this patch the test would report Entering 'nested1/nested2/../nested3' instead of the expected Entering '../nested1/nested2/nested3' Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- git-submodule.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'git-submodule.sh') diff --git a/git-submodule.sh b/git-submodule.sh index 43c68deee9..b3f248c3fe 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -413,8 +413,8 @@ cmd_foreach() die_if_unmatched "$mode" if test -e "$sm_path"/.git then - displaypath=$(relative_path "$sm_path") - say "$(eval_gettext "Entering '\$prefix\$displaypath'")" + displaypath=$(relative_path "$prefix$sm_path") + say "$(eval_gettext "Entering '\$displaypath'")" name=$(git submodule--helper name "$sm_path") ( prefix="$prefix$sm_path/" @@ -434,7 +434,7 @@ cmd_foreach() cmd_foreach "--recursive" "$@" fi ) <&3 3<&- || - die "$(eval_gettext "Stopping at '\$prefix\$displaypath'; script returned non-zero status.")" + die "$(eval_gettext "Stopping at '\$displaypath'; script returned non-zero status.")" fi done } -- cgit v1.2.1 From c1ab00fb267fb0166376ff2c81083ef2cada6d9d Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Tue, 29 Mar 2016 18:27:42 -0700 Subject: submodule update --init: correct path handling in recursive submodules When calling `git submodule init` from a recursive instance of `git submodule update --recursive`, the reported path is wrong as it skips the nested submodules. The new test demonstrates a failure in the code prior to this patch. Instead of getting the expected Submodule 'submodule' (${pwd}/submodule) registered for path '../super/submodule' the `super` directory is omitted and you get Submodule 'submodule' (${pwd}/submodule) registered for path '../submodule' instead. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- git-submodule.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'git-submodule.sh') diff --git a/git-submodule.sh b/git-submodule.sh index b3f248c3fe..9fffa5c54a 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -473,7 +473,7 @@ cmd_init() die_if_unmatched "$mode" name=$(git submodule--helper name "$sm_path") || exit - displaypath=$(relative_path "$sm_path") + displaypath=$(relative_path "$prefix$sm_path") # Copy url setting when it is not set yet if test -z "$(git config "submodule.$name.url")" -- cgit v1.2.1 From 10450cf72b51baf3bac6a779fb4e47241af7ae5e Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Tue, 29 Mar 2016 18:27:43 -0700 Subject: submodule status: correct path handling in recursive submodules The new test which is a replica of the previous test except that it executes from a sub directory. Prior to this patch the test failed by having too many '../' prefixed: --- expect 2016-03-29 19:02:33.087336115 +0000 +++ actual 2016-03-29 19:02:33.359343311 +0000 @@ -1,7 +1,7 @@ b23f134787d96fae589a6b76da41f4db112fc8db ../nested1 (heads/master) -+25d56d1ddfb35c3e91ff7d8f12331c2e53147dcc ../nested1/nested2 (file2) - 5ec83512b76a0b8170b899f8e643913c3e9b72d9 ../nested1/nested2/nested3 (heads/master) - 509f622a4f36a3e472affcf28fa959174f3dd5b5 ../nested1/nested2/nested3/submodule (heads/master) ++25d56d1ddfb35c3e91ff7d8f12331c2e53147dcc ../../nested1/nested2 (file2) + 5ec83512b76a0b8170b899f8e643913c3e9b72d9 ../../../nested1/nested2/nested3 (heads/master) + 509f622a4f36a3e472affcf28fa959174f3dd5b5 ../../../../nested1/nested2/nested3/submodule (heads/master) 0c90624ab7f1aaa301d3bb79f60dcfed1ec4897f ../sub1 (0c90624) 0c90624ab7f1aaa301d3bb79f60dcfed1ec4897f ../sub2 (0c90624) 509f622a4f36a3e472affcf28fa959174f3dd5b5 ../sub3 (heads/master) The path code in question: displaypath=$(relative_path "$prefix$sm_path") prefix=$displaypath if recursive: eval cmd_status That way we change `prefix` each iteration to contain another '../', because of the the relative_path computation is done on an already computed relative path. We must call relative_path exactly once with `wt_prefix` non empty. Further calls in recursive instances to to calculate the displaypath already incorporate the correct prefix from before. Fix the issue by clearing `wt_prefix` in recursive calls. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- git-submodule.sh | 1 + 1 file changed, 1 insertion(+) (limited to 'git-submodule.sh') diff --git a/git-submodule.sh b/git-submodule.sh index 9fffa5c54a..1024774f61 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -1159,6 +1159,7 @@ cmd_status() ( prefix="$displaypath/" clear_local_git_env + wt_prefix= cd "$sm_path" && eval cmd_status ) || -- cgit v1.2.1 From b08238ac3f5e4033d26824afc65e7397fbb61847 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Tue, 29 Mar 2016 18:27:44 -0700 Subject: submodule update: align reporting path for custom command execution In the predefined actions (merge, rebase, none, checkout), we use the display path, which is relative to the current working directory. Also use the display path when running a custom command. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- git-submodule.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'git-submodule.sh') diff --git a/git-submodule.sh b/git-submodule.sh index 1024774f61..753a90d307 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -802,8 +802,8 @@ Maybe you want to use 'update --init'?")" ;; !*) command="${update_module#!}" - die_msg="$(eval_gettext "Execution of '\$command \$sha1' failed in submodule path '\$prefix\$sm_path'")" - say_msg="$(eval_gettext "Submodule path '\$prefix\$sm_path': '\$command \$sha1'")" + die_msg="$(eval_gettext "Execution of '\$command \$sha1' failed in submodule path '\$displaypath'")" + say_msg="$(eval_gettext "Submodule path '\$displaypath': '\$command \$sha1'")" must_die_on_failure=yes ;; *) -- cgit v1.2.1