From: Junio C Hamano <gitster@pobox•com>
To: "W. Trevor King" <wking@tremily•us>
Cc: Git <git@vger•kernel.org>, Jens Lehmann <Jens.Lehmann@web•de>,
Francesco Pretto <ceztko@gmail•com>,
Heiko Voigt <hvoigt@hvoigt•net>,
Jonathan Nieder <jrnieder@gmail•com>
Subject: Re: [PATCH v4 3/6] submodule: Explicit local branch creation in module_clone
Date: Thu, 16 Jan 2014 11:18:00 -0800 [thread overview]
Message-ID: <xmqqr487zqfr.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <96f9749de94f7e89f4d113f8cde69f2a960bcb88.1389837412.git.wking@tremily.us> (W. Trevor King's message of "Wed, 15 Jan 2014 20:10:24 -0800")
"W. Trevor King" <wking@tremily•us> writes:
> The previous code only checked out branches 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 'reset' to preserve branches setup during module_clone.
>
> With this change, folks cloning submodules for the first time via:
>
> $ git submodule update ...
>
> will get a local branch instead of a detached HEAD, unless they are
> using the default checkout-mode updates. This is a change from the
> previous situation where cmd_update always used checkout-mode logic
> (regardless of the requested update mode) for updates that triggered
> an initial clone, which always resulted in a detached HEAD.
>
> This commit does not change the logic for updates after the initial
> clone, which will continue to create detached HEADs for checkout-mode
> updates, and integrate remote work with the local HEAD (detached or
> not) in other modes.
>
> The motivation for the change is that developers doing local work
> inside the submodule are likely to select a non-checkout-mode for
> updates so their local work is integrated with upstream work.
> Developers who are not doing local submodule work stick with
> checkout-mode updates so any apparently local work is blown away
> during updates. For example, if upstream rolls back the remote branch
> or gitlinked commit to an earlier version, the checkout-mode developer
> wants their old submodule checkout to be rolled back as well, instead
> of getting a no-op merge/rebase with the rolled-back reference.
>
> By using the update mode to distinguish submodule developers from
> black-box submodule consumers, we can setup local branches for the
> developers who will want local branches, and stick with detached HEADs
> for the developers that don't care.
>
> Signed-off-by: W. Trevor King <wking@tremily•us>
> ---
> git-submodule.sh | 53 ++++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 36 insertions(+), 17 deletions(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 68dcbe1..4a09951 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -246,6 +246,9 @@ module_name()
> # $3 = URL to clone
> # $4 = reference repository to reuse (empty for independent)
> # $5 = depth argument for shallow clones (empty for deep)
> +# $6 = (remote-tracking) starting point for the local branch (empty for HEAD)
> +# $7 = local branch to create (empty for a detached HEAD, unless $6 is
> +# also empty, in which case the local branch is left unchanged)
> #
> # Prior to calling, cmd_update checks that a possibly existing
> # path is not a git repository.
> @@ -259,6 +262,8 @@ module_clone()
> url=$3
> reference="$4"
> depth="$5"
> + start_point="$6"
> + local_branch="$7"
> quiet=
> if test -n "$GIT_QUIET"
> then
> @@ -312,7 +317,16 @@ 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" &&
> + # ash fails to wordsplit ${local_branch:+-B "$local_branch"...}
Interesting...
> + case "$local_branch" in
> + '') git checkout -f -q ${start_point:+"$start_point"} ;;
> + ?*) git checkout -f -q -B "$local_branch" ${start_point:+"$start_point"} ;;
> + esac
I am wondering if it makes more sense if you did this instead:
git checkout -f -q ${start_point:+"$start_point"} &&
if test -n "$local_branch"
then
git checkout -B "$local_branch" HEAD
fi
The optional "re-attaching to the local_branch" done with the second
"checkout" would be a non-destructive no-op to the working tree and
to the index, but it does distinguish between creating the branch
anew and resetting the existing branch in its output (note that
there is no "-q" to squelch it). By doing it this way, when we
later teach "git branch -f" and "git checkout -B" to report more
about what commits have been lost by such a resetting, you will get
the safety for free if you made the switching with "-B" run without
"-q" here.
> + ) || die "$(eval_gettext "Unable to setup cloned submodule '\$sm_path'")"
> }
>
> isnumber()
> @@ -475,16 +489,14 @@ 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'")"
> + if test -n "$branch"
> + then
> + start_point="origin/$branch"
> + local_branch="$branch"
> + else
> + start_point=""
> + fi
I'd feel safer if the "else" clause explicitly cleared $local_branch
by assigning an empty string to it; it would make it a lot clearer
that "when $branch is an empty string here, we do not want to
trigger the new codepath to run checkout with "-B $local_branch" in
module_clone" is what you mean.
> + module_clone "$sm_path" "$sm_name" "$realrepo" "$reference" "$depth" "$start_point" "$local_branch" || exit
next prev parent reply other threads:[~2014-01-16 19:18 UTC|newest]
Thread overview: 144+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-30 1:49 [PATCH/RFC] Introduce git submodule add|update --attach Francesco Pretto
2013-12-31 20:05 ` Phil Hord
2014-01-02 18:48 ` Francesco Pretto
2014-01-13 17:31 ` Junio C Hamano
2014-01-02 20:07 ` Junio C Hamano
2014-01-02 23:42 ` Francesco Pretto
2014-01-03 0:26 ` Francesco Pretto
2014-01-03 8:49 ` Francesco Pretto
2014-01-03 18:06 ` [PATCH] submodule: Respect reqested branch on all clones W. Trevor King
2014-01-04 22:09 ` Heiko Voigt
2014-01-04 22:54 ` W. Trevor King
2014-01-05 0:39 ` Heiko Voigt
2014-01-05 1:08 ` W. Trevor King
2014-01-05 3:53 ` Francesco Pretto
2014-01-05 16:17 ` [RFC v2] submodule: Respect requested " W. Trevor King
2014-01-05 19:48 ` Heiko Voigt
2014-01-05 21:24 ` W. Trevor King
2014-01-05 22:57 ` Heiko Voigt
2014-01-05 23:39 ` W. Trevor King
2014-01-06 0:33 ` W. Trevor King
2014-01-06 1:12 ` W. Trevor King
2014-01-06 16:02 ` Heiko Voigt
2014-01-06 23:10 ` Francesco Pretto
2014-01-06 23:32 ` Francesco Pretto
2014-01-07 18:27 ` Junio C Hamano
2014-01-07 19:19 ` Francesco Pretto
2014-01-07 19:45 ` W. Trevor King
2014-01-07 19:48 ` Francesco Pretto
2014-01-07 21:37 ` W. Trevor King
2014-01-07 21:51 ` Francesco Pretto
2014-01-07 22:38 ` Heiko Voigt
2014-01-08 0:17 ` Francesco Pretto
2014-01-08 1:05 ` W. Trevor King
2014-01-08 2:12 ` Francesco Pretto
2014-01-08 23:07 ` Francesco Pretto
2014-01-09 0:03 ` W. Trevor King
2014-01-09 1:09 ` Francesco Pretto
2014-01-09 2:22 ` W. Trevor King
2014-01-09 8:31 ` Jens Lehmann
2014-01-09 17:32 ` W. Trevor King
2014-01-09 19:23 ` Jens Lehmann
2014-01-09 19:55 ` W. Trevor King
2014-01-09 21:40 ` Jens Lehmann
2014-01-09 22:18 ` W. Trevor King
2014-01-14 10:24 ` Heiko Voigt
2014-01-14 16:57 ` W. Trevor King
2014-01-14 20:58 ` Heiko Voigt
2014-01-14 21:42 ` W. Trevor King
2014-01-14 22:19 ` Heiko Voigt
2014-01-14 22:39 ` W. Trevor King
2014-01-14 21:46 ` Re: " Heiko Voigt
2014-01-14 22:22 ` W. Trevor King
2014-01-14 22:42 ` Heiko Voigt
2014-01-15 0:02 ` Francesco Pretto
2014-01-16 4:09 ` [PATCH v4 0/6] submodule: Local branch creation in module_clone W. Trevor King
2014-01-16 4:10 ` [PATCH v4 1/6] submodule: Make 'checkout' update_module explicit W. Trevor King
2014-01-16 18:46 ` Junio C Hamano
2014-01-16 19:22 ` W. Trevor King
2014-01-16 20:07 ` Francesco Pretto
2014-01-16 20:19 ` W. Trevor King
2014-01-16 4:10 ` [PATCH v4 2/6] submodule: Document module_clone arguments in comments W. Trevor King
2014-01-16 4:10 ` [PATCH v4 3/6] submodule: Explicit local branch creation in module_clone W. Trevor King
2014-01-16 19:18 ` Junio C Hamano [this message]
2014-01-16 19:29 ` W. Trevor King
2014-01-16 19:43 ` Junio C Hamano
2014-01-16 21:12 ` W. Trevor King
2014-01-16 4:10 ` [PATCH v4 4/6] t7406: Just-cloned checkouts update to the gitlinked hash with 'reset' W. Trevor King
2014-01-16 19:22 ` Junio C Hamano
2014-01-16 19:32 ` W. Trevor King
2014-01-16 20:24 ` Junio C Hamano
2014-01-16 4:10 ` [PATCH v4 5/6] t7406: Add explicit tests for head attachement after cloning updates W. Trevor King
2014-01-16 4:10 ` [PATCH v4 6/6] Documentation: Describe 'submodule update' modes in detail W. Trevor King
2014-01-16 20:21 ` Junio C Hamano
2014-01-16 20:55 ` W. Trevor King
2014-01-16 21:02 ` John Keeping
2014-01-16 21:16 ` W. Trevor King
2014-01-16 21:55 ` Junio C Hamano
2014-01-17 2:37 ` W. Trevor King
2014-01-26 20:45 ` [PATCH v5 0/4] submodule: Local branch creation in module_clone W. Trevor King
2014-01-26 20:45 ` [PATCH v5 1/4] submodule: Make 'checkout' update_module explicit W. Trevor King
2014-01-27 1:32 ` Eric Sunshine
2014-01-27 1:59 ` W. Trevor King
2014-01-26 20:45 ` [PATCH v5 2/4] submodule: Document module_clone arguments in comments W. Trevor King
2014-01-26 20:45 ` [PATCH v5 3/4] submodule: Explicit local branch creation in module_clone W. Trevor King
2014-01-26 20:45 ` [PATCH v5 4/4] Documentation: Describe 'submodule update --remote' use case W. Trevor King
2014-01-16 22:18 ` [PATCH v4 6/6] Documentation: Describe 'submodule update' modes in detail Philip Oakley
2014-01-16 22:35 ` W. Trevor King
2014-01-08 23:54 ` Re: [RFC v2] submodule: Respect requested branch on all clones Francesco Pretto
2014-01-09 0:23 ` W. Trevor King
2014-01-07 19:52 ` Francesco Pretto
2014-01-06 15:47 ` Re: " Heiko Voigt
2014-01-06 17:22 ` W. Trevor King
2014-01-05 21:27 ` Francesco Pretto
2014-01-05 21:47 ` W. Trevor King
2014-01-05 22:01 ` W. Trevor King
2014-01-06 14:47 ` Heiko Voigt
2014-01-06 16:56 ` Junio C Hamano
2014-01-06 17:37 ` W. Trevor King
2014-01-06 21:32 ` Junio C Hamano
2014-01-07 0:55 ` Francesco Pretto
2014-01-07 18:15 ` Junio C Hamano
2014-01-07 18:47 ` W. Trevor King
2014-01-07 19:21 ` Junio C Hamano
2014-01-07 19:50 ` W. Trevor King
2014-01-05 2:50 ` [PATCH 1/2] git-submodule.sh: Support 'checkout' as a valid update command Francesco Pretto
2014-01-05 2:50 ` [PATCH 2/2] Introduce git submodule attached update Francesco Pretto
2014-01-05 19:55 ` Francesco Pretto
2014-01-05 20:33 ` Heiko Voigt
2014-01-05 21:46 ` Francesco Pretto
2014-01-06 14:06 ` Heiko Voigt
2014-01-06 17:47 ` Francesco Pretto
2014-01-06 19:21 ` David Engster
2014-01-07 19:27 ` W. Trevor King
2014-01-07 4:10 ` W. Trevor King
2014-01-07 22:36 ` Preferred local submodule branches (was: Introduce git submodule attached update) W. Trevor King
2014-01-07 23:52 ` W. Trevor King
2014-01-08 3:47 ` Preferred local submodule branches W. Trevor King
2014-01-08 4:06 ` W. Trevor King
2014-01-09 6:17 ` [RFC v3 0/4] " W. Trevor King
2014-01-09 6:17 ` [RFC v3 1/4] submodule: Add helpers for configurable local branches W. Trevor King
2014-01-09 6:17 ` [RFC v3 2/4] submodule: Teach 'update' to preserve " W. Trevor King
2014-01-09 6:17 ` [RFC v3 3/4] submodule: Teach 'add' about a configurable local-branch W. Trevor King
2014-01-15 0:18 ` Francesco Pretto
2014-01-15 1:02 ` W. Trevor King
2014-01-09 6:17 ` [RFC v3 4/4] submodule: Add a new 'checkout' command W. Trevor King
2014-01-12 1:08 ` Tight submodule bindings (was: Preferred local submodule branches) W. Trevor King
2014-01-13 19:37 ` Tight submodule bindings Jens Lehmann
2014-01-13 20:07 ` W. Trevor King
2014-01-13 22:13 ` Junio C Hamano
2014-01-14 2:44 ` W. Trevor King
2014-01-07 22:51 ` Re: [PATCH 2/2] Introduce git submodule attached update Heiko Voigt
2014-01-07 23:14 ` W. Trevor King
2014-01-07 18:56 ` Junio C Hamano
2014-01-07 19:44 ` Francesco Pretto
2014-01-07 19:07 ` Junio C Hamano
2014-01-07 19:25 ` Francesco Pretto
2014-01-05 23:22 ` Francesco Pretto
2014-01-06 14:18 ` Heiko Voigt
2014-01-06 15:58 ` W. Trevor King
2014-01-05 20:20 ` [PATCH 1/2] git-submodule.sh: Support 'checkout' as a valid update command Heiko Voigt
2014-01-05 20:44 ` W. Trevor King
2014-01-06 16:20 ` Junio C Hamano
2014-01-06 17:42 ` Junio C Hamano
2014-01-06 17:52 ` Francesco Pretto
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqr487zqfr.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=Jens.Lehmann@web$(echo .)de \
--cc=ceztko@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=hvoigt@hvoigt$(echo .)net \
--cc=jrnieder@gmail$(echo .)com \
--cc=wking@tremily$(echo .)us \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox