Re: [RFC v2] submodule: Respect requested branch on all clones
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:59:37
"W. Trevor King" [off-list ref] writes:
From: "W. Trevor King" <redacted> The previous code only checked out the requested branch in cmd_add. This commit moves the branch-checkout logic into module_clone, where it can be shared by cmd_add and cmd_update. I also update the initial checkout command to use 'rebase' to preserve branches setup during module_clone.
I want to see the log message explain the motivation behind it
(i.e. instead of stopping after saying "We used to do X, now we do
Y", but also explain why we consider that Y is better than X). Here
is my attempt.
submodule: respect requested branch on all clones
The previous code only checked out the requested branch in cmd_add
but not in cmd_update; this left the user on a detached HEAD after
an update initially cloned, and subsequent updates using rebase or
merge mode will kept the HEAD detached, unless the user moved to the
desired branch himself.
Move the branch-checkout logic into module_clone, where it can be
shared by cmd_add and cmd_update. Also update the initial checkout
command to use 'rebase' to preserve branches setup during
module_clone. This way, unless the user explicitly asks to work on
a detached HEAD, subsequent updates all happen on the specified
branch, which matches the end-user expectation much better.
Signed-off-by: W. Trevor King [off-list ref]
Signed-off-by: Junio C Hamano [off-list ref]
Please correct me if I misunderstood the intention.
Having writing all the above and then looking at the patch again, it
is not immediately obvious to me where you use "rebase" when doing
the initial checkout, though.
The current Documentation/git-submodule.txt has:
update::
Update the registered submodules, i.e. clone missing submodules
and checkout the commit specified in the index of the containing
repository. This will make the submodules HEAD be detached unless
`--rebase` or `--merge` is specified or the key
`submodule.$name.update` is set to `rebase`, `merge` or `none`.Side note but doesn't Francesco's "'checkout' is a valid update mode" need to update this part of the documentation as well?
quoted hunk
git-submodule.sh | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-)diff --git a/git-submodule.sh b/git-submodule.sh index 2979197..167d4fa 100755 --- a/git-submodule.sh +++ b/git-submodule.sh@@ -253,6 +253,7 @@ module_clone() url=$3 reference="$4" depth="$5" + branch="$6" quiet= if test -n "$GIT_QUIET" then@@ -306,7 +307,15 @@ module_clone() echo "gitdir: $rel/$a" >"$sm_path/.git" rel=$(echo $a | sed -e 's|[^/][^/]*|..|g') - (clear_local_git_env; cd "$sm_path" && GIT_WORK_TREE=. git config core.worktree "$rel/$b") + ( + clear_local_git_env + cd "$sm_path" && + GIT_WORK_TREE=. git config core.worktree "$rel/$b" && + case "$branch" in + '') git checkout -f -q ;; + ?*) git checkout -f -q -B "$branch" "origin/$branch" ;; + esac + ) || die "$(eval_gettext "Unable to setup cloned submodule '\$sm_path'")" } isnumber()@@ -469,16 +478,7 @@ Use -f if you really want to add it." >&2 echo "$(eval_gettext "Reactivating local git directory for submodule '\$sm_name'.")" fi fi - module_clone "$sm_path" "$sm_name" "$realrepo" "$reference" "$depth" || exit - ( - clear_local_git_env - cd "$sm_path" && - # ash fails to wordsplit ${branch:+-b "$branch"...} - case "$branch" in - '') git checkout -f -q ;; - ?*) git checkout -f -q -B "$branch" "origin/$branch" ;; - esac - ) || die "$(eval_gettext "Unable to checkout submodule '\$sm_path'")" + module_clone "$sm_path" "$sm_name" "$realrepo" "$reference" "$depth" "$branch" || exit fi git config submodule."$sm_name".url "$realrepo"@@ -787,7 +787,8 @@ cmd_update() fi name=$(module_name "$sm_path") || exit url=$(git config submodule."$name".url) - branch=$(get_submodule_config "$name" branch master) + config_branch=$(get_submodule_config "$name" branch) + branch="${config_branch:-master}" if ! test -z "$update" then update_module=$update@@ -815,7 +816,7 @@ Maybe you want to use 'update --init'?")" if ! test -d "$sm_path"/.git -o -f "$sm_path"/.git then - module_clone "$sm_path" "$name" "$url" "$reference" "$depth" || exit + module_clone "$sm_path" "$name" "$url" "$reference" "$depth" "$config_branch" || exit cloned_modules="$cloned_modules;$name" subsha1= else@@ -861,7 +862,12 @@ Maybe you want to use 'update --init'?")" case ";$cloned_modules;" in *";$name;"*) # then there is no local change to integrate - update_module= ;; + if test -n "$config_branch"; then + update_module="!git reset --hard -q" + else + update_module= + fi + ;; esac must_die_on_failure=