| Message ID | 20201114122132.4344-2-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/3] submodule: use "fetch" logic instead of custom remote discovery | expand |
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Replace a use of the get_default_remote() function with an invocation > of "git fetch" > > The "fetch" command already has logic to discover the remote for the > current branch. However, before it learned to accept a custom > refspec *and* use its idea of the default remote, it wasn't possible > to get rid of some equivalent of the "get_default_remote" invocation > here. > > As it turns out the recently added "--stdin" option to fetch[1] gives > us a way to do that. Let's use it instead. Ah, that was what your earlier question was about. The hits from "git grep fetch_in_submodule" says that $1 is the path to the submodule, $2 is the $depth and $3 (when exists) is the object name of what we are fetching: git-submodule.sh:578: fetch_in_submodule "$sm_path" $depth || git-submodule.sh:601: fetch_in_submodule "$sm_path" $depth || git-submodule.sh:607: fetch_in_submodule "$sm_path" $depth "$sha1" || > While I'm at it simplify the "fetch_in_submodule" function. It wasn't > necessary to pass "$@" to "fetch" since we'd only ever provide one > SHA-1 as an argument in the previous "*" codepath (in addition to > "--depth=N"). Rewrite the function to more narrowly reflect its > use-case. > cd "$1" && > - case "$2" in > - '') > - git fetch ;; > - *) > - shift > - git fetch $(get_default_remote) "$@" ;; > - esac So when $2 (i.e. --depth=N) is empty, we just did the default "git fetch". Otherwise, "git fetch <remote> --depth=N [<sha1>]" was run. > + if test $# -eq 3 > + then > + echo "$3" | git fetch --stdin "$2" This is not quite equivalent in that an empty depth would have resulted in the default "git fetch" in the original code, with or without "$3", totally ignoring "$sha1". Is this a bugfix snuck in "while we are at it", or is this an unintended regression? > + elif test "$2" -ne "" > + then > + git fetch "$2" > + else > + git fetch > + fi Even if it is an unintended regression, fixing it should be trivial; we just need to flip the order of conditions, like so: if test -z "$2" then git fetch elif test $# = 3 then echo "$3" | git fetch --stdin "$2" else git fetch "$2" fi
diff --git a/git-submodule.sh b/git-submodule.sh index 7ce52872b7..d39fd226d8 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -416,13 +416,15 @@ is_tip_reachable () ( fetch_in_submodule () ( sanitize_submodule_env && cd "$1" && - case "$2" in - '') - git fetch ;; - *) - shift - git fetch $(get_default_remote) "$@" ;; - esac + if test $# -eq 3 + then + echo "$3" | git fetch --stdin "$2" + elif test "$2" -ne "" + then + git fetch "$2" + else + git fetch + fi ) #
Replace a use of the get_default_remote() function with an invocation of "git fetch" The "fetch" command already has logic to discover the remote for the current branch. However, before it learned to accept a custom refspec *and* use its idea of the default remote, it wasn't possible to get rid of some equivalent of the "get_default_remote" invocation here. As it turns out the recently added "--stdin" option to fetch[1] gives us a way to do that. Let's use it instead. While I'm at it simplify the "fetch_in_submodule" function. It wasn't necessary to pass "$@" to "fetch" since we'd only ever provide one SHA-1 as an argument in the previous "*" codepath (in addition to "--depth=N"). Rewrite the function to more narrowly reflect its use-case. 1. https://lore.kernel.org/git/87eekwf87n.fsf@evledraar.gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- git-submodule.sh | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)