RFC patchIntroduce git submodule add|update --attach

>
> by default "git submodule" performs its add or update operations on a detached
> HEAD. This works well when using an existing full-fledged/indipendent project as
> the submodule, as there's less frequent need to update it or commit back
> changes. When the submodule is actually a large portion of shareable code
> between  different projects, and the superproject needs to track very closely
> the evolution of the submodule (or the other way around), I feel more confortable
> to reattach the HEAD of the submodule with an existing branch. This can be as
> simple as having a superproject "project1" in branch "master" with a submodule
> "common" attached to the branch "master-project1" or, in a more development
> workflow, "project1" in branch "featureA" with the same submodule "common"
> attached to a similarly named branch "featureA". Doing this in git requires me
> the following:
>
> # Maintainer
> $ git submodule add --branch "master-project1" <repository> common
> $ git commit -m "Added submodule"
> $ git config -f .gitmodules submodule.common.ignore all
> $ git push
> $ cd <path>
> $ git checkout "master-project1"
>
> # Developer
> $ git pull
> $ git submodule init
> $ git submodule update --remote
> $ cd <path>
> $ branch="$(git config -f ..\.gitmodules submodule.common.branch)"; git checkout $branch
>
> While the burden for the repository maitainer/administrator is acceptable, in
> the developer point of view there are two problems:
> 1) Checking out an attached HEAD of a specified branch as when using "--remote"
> is not really simple as it could be and could require lauching of scrips or
> reading some repository specific documentation. Also in Windows platform the
> syntax for inline shell evaluation of commands is less known between users;
> 2) There's no way to store a similar default behaviour in the repository except
> by using scripts. Also recently submodule.<modulename>.update custom !commands
> in no more supported when stored in .gitmodules [1].
>
> The attached patch tries to solve these problems by introducing an "--attach"
> switch to the "add" and "update" submodule commands and a "--detach" switch just
> for the "update" command. It also add the support for an 'submodule.<name>.attach'
> property when updating. Using the "--attach" switch when adding a submodule does:
> - create the submodule checking out an attached HEAD;
> - set the 'submodule.<name>.attach' property to 'true';
> - set the 'submodule.<name>.ignore' property to 'all' (this is useful as
> attaching to the branch doesn't require tracking of revision sha1).
>
> The rationale of setting 'attach' and 'ignore' properties when adding a
> submodule with the "--attach" switch is to give a convenient default behaviour.
> No other properties are set: the repository responsible will still be required
> to configure a different 'submodule.<name>.update' behaviour separetely, if he
> wants that.
>
> When updating, using the '--attach' switch or operating in a repository with
> 'submodule.<name>.attach' set to 'true' will:
> - checkout a branch with an attached HEAD if the repository was just cloned;
> - perform a fast-forward only merge of changes if it's a 'checkout' update,
> keeping the HEAD attached;
> - reattach the HEAD prior performing a 'merge', 'rebase' or '!command' update
> operation if the HEAD was found detached.

> '--attach' or 'submodule.<name>.attach' set to true also implies '--remote', as
> it's needed the origin HEAD sha1 to verify the current HEAD state.
>
> A '--detach' switch is also available. Using the '--detach' switch or
> operating in a repository with 'submodule.<name>.attach' set to 'false' during
> update will:
> - checkout a detached HEAD if the repository was just cloned (same behaviour as
> before);
> - detach the HEAD prior performing a 'merge', 'rebase' or '!command' update
> operation if the HEAD was found attached.
>
> 'submodule.<name>.attach' works the same way as 'submodule.<name>.update'
> property: git copies the values found in ".gitmodules" in ".git/config" when
> performing an "init" command. "update" looks for the values in ".git/config"
> only.
>
> '--attach' and '--detach' switches override an opposite behaviour of 'submodule.<name>.attach'
> properties.
>
> The patch is small (touches only git-submodule.sh) and 100% additive with
> respect to currently documented behaviour: when using "add" and "update"
> commands without the introduced switches and properties, git shall operate as
> before. As a bonus (but this was done to ease conditionals and keep the code
> clean) it also clarifies and validates the content of 'submodule.<name>.update'
> during 'update' command, warning the user if it's not one of the supported
> values 'checkout', 'merge', 'rebase' and 'none'. Please note that 'checkout'
> update command was documented in upstream "Documentation/gitmodules.txt" [2] as
> a valid 'submodule.<name>.update' value and code in upstream "git-submodule.sh"
> implicitly assumes it as recognized value. "Documentation/git-submodule.txt"
> doesn't mention it properly and I guess this is the reason why it wasn't
> considered for the validation in [1] patch.
>
> Using this patch, the previous workflow becomes:
>
> # Maintainer
> $ git submodule add --branch "master-project1" --attach <repository> <path>
> $ git commit -m "Added submodule"
> $ git push
>
> # Developer
> $ git pull
> $ git submodule init
> $ git submodule update
>
> Is there interest in supporting this workflow and seeing this patch applied?
>
> Thanks,
> Francesco
>
> [1] http://marc.info/?l=git&m=138610752125816&w=2
> [2] Documentation/gitmodules.txt: "If 'checkout' (the default)..."
>
> Signed-off-by: Francesco Pretto <ceztko@gmail.com>
> ---
>  Documentation/git-submodule.txt |  37 ++++++---
>  Documentation/gitmodules.txt    |  11 ++-
>  git-submodule.sh                | 172 +++++++++++++++++++++++++++++++++++++---
>  3 files changed, 197 insertions(+), 23 deletions(-)
>
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index bfef8a0..452376d 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -10,13 +10,14 @@ SYNOPSIS
>  --------
>  [verse]
>  'git submodule' [--quiet] add [-b <branch>] [-f|--force] [--name <name>]
> -             [--reference <repository>] [--depth <depth>] [--] <repository> [<path>]
> +             [--reference <repository>] [--attach] [--depth <depth>]
> +             [--] <repository> [<path>]
>  'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
>  'git submodule' [--quiet] init [--] [<path>...]
>  'git submodule' [--quiet] deinit [-f|--force] [--] <path>...
>  'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]
> -             [-f|--force] [--rebase] [--reference <repository>] [--depth <depth>]
> -             [--merge] [--recursive] [--] [<path>...]
> +             [-f|--force] [--rebase] [--reference <repository>] [--attach | --detach]
> +             [--depth <depth>] [--merge] [--recursive] [--] [<path>...]
>  'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
>               [commit] [--] [<path>...]
>  'git submodule' [--quiet] foreach [--recursive] <command>
> @@ -107,6 +108,10 @@ is the superproject and submodule repositories will be kept
>  together in the same relative location, and only the
>  superproject's URL needs to be provided: git-submodule will correctly
>  locate the submodule using the relative URL in .gitmodules.
> ++
> +If `--attach` is specified, the submodule will be registered to be
> +checked out with an an attached HEAD. Also `submodule.<name>.attach` will
> +be set to `true` and `submodule.<name>.ignore` will be set to `all`.
>
>  status::
>         Show the status of the submodules. This will print the SHA-1 of the
> @@ -156,12 +161,15 @@ it contains local modifications.
>  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`. `none` can be overridden by specifying
> -       `--checkout`. Setting the key `submodule.$name.update` to `!command`
> -       will cause `command` to be run. `command` can be any arbitrary shell
> -       command that takes a single argument, namely the sha1 to update to.
> +       This will make the submodules HEAD be detached unless `--attach` is
> +       specified or `submodule.$name.attach` is set to `true`. The last setting
> +       can always be overridden specifying `--detach`. Update mode can be
> +       selected specifying `--checkout`, `--rebase` or `--merge` switches
> +       or setting the key `submodule.$name.update` to `checkout`, `rebase`,
> +       `merge` or `none`. `none` will cause the submodule to be skipped during
> +       the update. Setting the key `submodule.$name.update` to `!command` will
> +       cause `command` to be run. `command` can be any arbitrary shell command
> +       that takes a single argument, namely the sha1 to update to.
>  +
>  If the submodule is not yet initialized, and you just want to use the
>  setting as stored in .gitmodules, you can automatically initialize the
> @@ -270,6 +278,17 @@ OPTIONS
>         be overridden by setting the `submodule.<name>.branch` option in
>         either `.gitmodules` or `.git/config` (with `.git/config` taking
>         precedence).
> +
> +--attach::
> +       This option is only valid for the add and update commands. Cause the

> +       default to `master`. Note: for the update command `--attach` also
> +       implies `--remote`.
> +
> +--detach::
> +       This option is only valid for the update command. Cause the result

>  +
>  This works for any of the supported update procedures (`--checkout`,
>  `--rebase`, etc.).  The only change is the source of the target SHA-1.
> diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
> index f7be93f..e6c3360 100644
> --- a/Documentation/gitmodules.txt
> +++ b/Documentation/gitmodules.txt
> @@ -37,8 +37,10 @@ submodule.<name>.url::
>
>  submodule.<name>.update::
>         Defines what to do when the submodule is updated by the superproject.
> -       If 'checkout' (the default), the new commit specified in the
> -       superproject will be checked out in the submodule on a detached HEAD.
> +       If 'checkout' (the default), the new commit (or the branch, when using
> +       the '--attach' switch or the 'submodule.<name>.attach' property is set
> +       to 'true' during an update operation) specified in the superproject will
> +       be checked out in the submodule.

> @@ -54,6 +56,11 @@ submodule.<name>.branch::
>         If the option is not specified, it defaults to 'master'.  See the
>         `--remote` documentation in linkgit:git-submodule[1] for details.
>
> +submodule.<name>.attach::
> +       Determine if the update operation will produce a detached HEAD or not.
> +       Valid values are `true` or `false`. If the property is set to `true`
> +       and `submodule.<name>.branch`, it will default to `master`

> +
>  submodule.<name>.fetchRecurseSubmodules::
>         This option can be used to control recursive fetching of this
>         submodule. If this option is also present in the submodules entry in
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 2677f2e..3951fa2 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -5,11 +5,11 @@
>  # Copyright (c) 2007 Lars Hjemli
>
>  dashless=$(basename "$0" | sed -e 's/-/ /')
> -USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [--] <repository> [<path>]
> +USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [--attach] [--] <repository> [<path>]
>     or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
>     or: $dashless [--quiet] init [--] [<path>...]
>     or: $dashless [--quiet] deinit [-f|--force] [--] <path>...
> -   or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
> +   or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--attach | --detach] [--merge] [--recursive] [--] [<path>...]
>     or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
>     or: $dashless [--quiet] foreach [--recursive] <command>
>     or: $dashless [--quiet] sync [--recursive] [--] [<path>...]"
> @@ -36,6 +36,7 @@ update=
>  prefix=
>  custom_name=
>  depth=
> +attach=
>
>  # The function takes at most 2 arguments. The first argument is the
>  # URL that navigates to the submodule origin repo. When relative, this URL
> @@ -352,6 +353,9 @@ cmd_add()
>                         custom_name=$2
>                         shift
>                         ;;
> +               --attach)
> +                       attach="true"
> +                       ;;
>                 --depth)
>                         case "$2" in '') usage ;; esac
>                         depth="--depth=$2"
> @@ -475,8 +479,17 @@ Use -f if you really want to add it." >&2
>                         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" ;;
> +                       '')
> +                               git checkout -f -q
> +                               ;;

> +                       ?*)
> +                               if test -n "$attach"
> +                               then
> +                                       git checkout -f -q "$branch"
> +                               else
> +                                       git checkout -f -q -B "$branch" "origin/$branch"
> +                               fi
> +                               ;;
>                         esac
>                 ) || die "$(eval_gettext "Unable to checkout submodule '\$sm_path'")"
>         fi
> @@ -491,6 +504,12 @@ Use -f if you really want to add it." >&2
>         then
>                 git config -f .gitmodules submodule."$sm_name".branch "$branch"
>         fi &&
> +       if test -n "$attach"
> +       then
> +               # We'll stay stick to the HEAD, no need to track revision sha1
> +               git config -f .gitmodules submodule."$sm_name".attach "true"
> +               git config -f .gitmodules submodule."$sm_name".ignore "all"
> +       fi &&
>         git add --force .gitmodules ||
>         die "$(eval_gettext "Failed to register submodule '\$sm_path'")"
>  }
> @@ -622,7 +641,7 @@ cmd_init()
>                    test -z "$(git config submodule."$name".update)"
>                 then
>                         case "$upd" in
> -                       rebase | merge | none)
> +                       checkout | rebase | merge | none)

>                                 ;; # known modes of updating
>                         *)
>                                 echo >&2 "warning: unknown update mode '$upd' suggested for submodule '$name'"
> @@ -632,6 +651,23 @@ cmd_init()
>                         git config submodule."$name".update "$upd" ||
>                         die "$(eval_gettext "Failed to register update mode for submodule path '\$displaypath'")"
>                 fi
> +
> +               # Copy "attach" setting when it is not set yet
> +               if attached="$(git config -f .gitmodules submodule."$name".attach)" &&
> +                  test -n "$attached" &&
> +                  test -z "$(git config submodule."$name".attach)"
> +               then
> +                       case "$attached" in
> +                       true | false)
> +                               ;; # Valid attach flag values
> +                       *)
> +                               echo >&2 "warning: invalid attach flag value for submodule '$name'"
> +                               upd=none
> +                               ;;
> +                       esac
> +                       git config submodule."$name".attach "$attached" ||
> +                       die "$(eval_gettext "Failed to register attached option for submodule path '\$displaypath'")"
> +               fi
>         done
>  }
>
> @@ -750,6 +786,14 @@ cmd_update()
>                 --reference=*)
>                         reference="$1"
>                         ;;
> +               --attach)
> +                       if test "$attach" = "false" ; then usage ; fi
> +                       attach="true"
> +                       ;;
> +               --detach)
> +                       if test "$attach" = "true" ; then usage ; fi
> +                       attach="false"
> +                       ;;
>                 -m|--merge)
>                         update="merge"
>                         ;;
> @@ -800,11 +844,44 @@ cmd_update()
>                 name=$(module_name "$sm_path") || exit
>                 url=$(git config submodule."$name".url)
>                 branch=$(get_submodule_config "$name" branch master)
> +               if test -n "$attach"
> +               then
> +                       attach_module=$attach
> +               else
> +                       attach_module=$(git config submodule."$name".attach)
> +                       case "$attach_module" in
> +                       '')
> +                               ;; # Unset attach flag
> +                       true|false)
> +                               ;; # Valid attach flag values
> +                       *)
> +                               echo >&2 "warning: invalid attach flag value for submodule '$name'"
> +                               attach_module=
> +                               ;;
> +                       esac
> +               fi
> +               if test "$attach_module" = "false"
> +               then
> +                       # Normalize attach 'false' flag value
> +                       attach_module=
> +               fi
>                 if ! test -z "$update"
>                 then
>                         update_module=$update
>                 else
>                         update_module=$(git config submodule."$name".update)
> +                       case "$update_module" in
> +                       '')
> +                               ;; # Unset update mode
> +                       checkout | rebase | merge | none)
> +                               ;; # Known update modes
> +                       !*)
> +                               ;; # Custom update command
> +                       *)
> +                               update_module=
> +                               echo >&2 "warning: invalid update mode for submodule '$name'"
> +                               ;;
> +                       esac

>                 fi
>
>                 displaypath=$(relative_path "$prefix$sm_path")
> @@ -836,7 +913,7 @@ Maybe you want to use 'update --init'?")"
>                         die "$(eval_gettext "Unable to find current revision in submodule path '\$displaypath'")"
>                 fi
>
> -               if test -n "$remote"
> +               if test -n "$remote" -o -n "$attach_module"
>                 then
>                         if test -z "$nofetch"
>                         then
> @@ -850,7 +927,16 @@ Maybe you want to use 'update --init'?")"
>                         die "$(eval_gettext "Unable to find current ${remote_name}/${branch} revision in submodule path '\$sm_path'")"
>                 fi
>
> -               if test "$subsha1" != "$sha1" -o -n "$force"
> +               head_rev_ref=$(clear_local_git_env; cd "$sm_path" && git rev-parse --abbrev-ref HEAD) ||
> +               die "$(eval_gettext "Unable to determine revision ref in submodule path '\$sm_path'")"
> +               if test "$head_rev_ref" = "HEAD"
> +               then
> +                       # Determine if the HEAD is detached
> +                       head_detached="true"
> +               fi
> +
> +               if test "$subsha1" != "$sha1" || test -n "$attach_module" -a -n "$head_detached" ||
> +                       test -z "$attach_module" -a -z "$head_detached" || test -n "$force"
>                 then
>                         subforce=$force
>                         # If we don't already have a -f flag and the submodule has never been checked out
> @@ -870,40 +956,102 @@ Maybe you want to use 'update --init'?")"
>                         fi
>
>                         # Is this something we just cloned?
> +                       just_cloned=
>                         case ";$cloned_modules;" in
>                         *";$name;"*)
>                                 # then there is no local change to integrate
> -                               update_module= ;;
> +                               update_module="checkout"
> +                               just_cloned=yes
> +                               ;;
>                         esac
>
> +                       if test -z "$update_module"
> +                       then
> +                               # Fallback to checkout
> +                               update_module="checkout"
> +                       fi
> +
> +                       command_pre=:
> +                       suffix_pre=
> +                       if test "$update_module" = "checkout"
> +                       then
> +                               if test -n "$attach_module" -a -z "$just_cloned"
> +                               then
> +                                       # We need to attach to the HEAD/switch the branch prior
> +                                       # performing any fast-forward update
> +                                       command_pre="git checkout $subforce -q"
> +                                       suffix_pre=$branch
> +                               fi
> +                       else
> +                               if test -n "$attach_module" -a -n "$head_detached"
> +                               then
> +                                       # We need to reattach to the head
> +                                       command_pre="git checkout $subforce -q"
> +                                       suffix_pre=$branch
> +                               elif test -z "$attach_module" -a -z "$head_detached"
> +                               then
> +                                       # We need to detach from the head
> +                                       command_pre="git checkout $subforce -q"
> +                                       suffix_pre=$sha1
> +                               fi
> +                       fi
> +
>                         must_die_on_failure=
> +                       custom_update=
> +                       suffix=
>                         case "$update_module" in
>                         rebase)
>                                 command="git rebase"
> +                               suffix=$sha1
>                                 die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$displaypath'")"
>                                 say_msg="$(eval_gettext "Submodule path '\$displaypath': rebased into '\$sha1'")"
>                                 must_die_on_failure=yes
>                                 ;;
>                         merge)
>                                 command="git merge"
> +                               suffix=$sha1
>                                 die_msg="$(eval_gettext "Unable to merge '\$sha1' in submodule path '\$displaypath'")"
>                                 say_msg="$(eval_gettext "Submodule path '\$displaypath': merged in '\$sha1'")"
>                                 must_die_on_failure=yes
>                                 ;;
> +                       checkout)
> +                               if test -n "$attach_module"
> +                               then
> +                                       if test -n "$just_cloned"
> +                                       then
> +                                               command="git checkout $subforce -q"
> +                                               suffix=$branch
> +                                       else
> +                                               # Perform a fast-forward only merge of the origin
> +                                               command="git merge $subforce --ff-only"
> +                                               suffix="origin/$branch"
> +                                       fi
> +                                       die_msg="$(eval_gettext "Unable to checkout banch '\$branch' in submodule path '\$displaypath'")"
> +                                       say_msg="$(eval_gettext "Submodule path '\$displaypath': checked out branch '\$branch'")"
> +                               else
> +                                       command="git checkout $subforce -q"
> +                                       suffix=$sha1
> +                                       die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$displaypath'")"
> +                                       say_msg="$(eval_gettext "Submodule path '\$displaypath': checked out '\$sha1'")"
> +                               fi
> +                               ;;
>                         !*)
>                                 command="${update_module#!}"
> +                               suffix=$sha1
>                                 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'")"
>                                 must_die_on_failure=yes
> +                               custom_update=yes
>                                 ;;
>                         *)
> -                               command="git checkout $subforce -q"
> -                               die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$displaypath'")"
> -                               say_msg="$(eval_gettext "Submodule path '\$displaypath': checked out '\$sha1'")"
> +                               # Valid user configurable update modes are already filtered above
> +                               die "$(eval_gettext "Unexpected update mode in the current flow")"
>                                 ;;
>                         esac
>
> -                       if (clear_local_git_env; cd "$sm_path" && $command "$sha1")
> +                       if (clear_local_git_env; cd "$sm_path" &&
> +                               $command_pre "$suffix_pre" &&
> +                               $command "$suffix")
>                         then
>                                 say "$say_msg"
>                         elif test -n "$must_die_on_failure"

>
> On Sun, Dec 29, 2013 at 8:49 PM, Francesco Pretto <ceztko@gmail.com> wrote:
>  [...]
> >
> > When updating, using the '--attach' switch or operating in a repository with
> > 'submodule.<name>.attach' set to 'true' will:
> > - checkout a branch with an attached HEAD if the repository was just cloned;
> > - perform a fast-forward only merge of changes if it's a 'checkout' update,
> > keeping the HEAD attached;
> > - reattach the HEAD prior performing a 'merge', 'rebase' or '!command' update
> > operation if the HEAD was found detached.
>
> I need to understand this "reattach the HEAD" case better. Can you
> give some examples of the expected behavior when merge, rebase or
> !command is encountered?
>

>
> > +
> > +--attach::
> > +       This option is only valid for the add and update commands. Cause the
>
> Grammar: 'Causes the result'
>

>
> > +       result of an add or update operation to be an attached HEAD. In the
> > +       update command , if `submodule.<name>.branch` is not set, it will
>
> typo: space before comma.
>

>
> Also, the pronoun "it" here is unclear to me.  Does this convey the
> correct meaning?
>
>    In the update operation, the branch named by 'submodule.<name>.branch' is
>    checked out as the new HEAD of the submodule repository. If
>    'submodule.<name>.branch' is not set, the 'master' branch is
> checked out as the
>    new HEAD of the submodule.
>

>
> > +       default to `master`. Note: for the update command `--attach` also
> > +       implies `--remote`.
> > +
> > +--detach::
> > +       This option is only valid for the update command. Cause the result
>
>  Grammar: 'Causes the result'
>

>
> > +       of the update operation to be forcedly a detached HEAD.
>
> "Forcedly" is a bit strong, maybe, slightly misplaced, and not a word,
> besides.   How's this, instead:
>
>    Forces the result of the update operation to be a detached HEAD in
> the submodule.

>
> >  submodule.<name>.update::
> >         Defines what to do when the submodule is updated by the superproject.
> > -       If 'checkout' (the default), the new commit specified in the
> > -       superproject will be checked out in the submodule on a detached HEAD.
> > +       If 'checkout' (the default), the new commit (or the branch, when using
> > +       the '--attach' switch or the 'submodule.<name>.attach' property is set
> > +       to 'true' during an update operation) specified in the superproject will
> > +       be checked out in the submodule.
>
> IMHO, this wording is overcomplicated by this change.  How about:
>
>        If 'checkout' (the default), the new commit specified in the superproject
>        (or branch, with '--attach') will be checked out in the submodule.
>

>
> >         If 'rebase', the current branch of the submodule will be rebased onto
> >         the commit specified in the superproject. If 'merge', the commit
> >         specified in the superproject will be merged into the current branch
>
> Does the 'merge', 'rebase' and '!command' description need to be
> updated, too?  Here and above it seems to still suggest the old
> behavior is kept when --attach is used.
>

>
> > @@ -54,6 +56,11 @@ submodule.<name>.branch::
> >         If the option is not specified, it defaults to 'master'.  See the
> >         `--remote` documentation in linkgit:git-submodule[1] for details.
> >
> > +submodule.<name>.attach::
> > +       Determine if the update operation will produce a detached HEAD or not.
> > +       Valid values are `true` or `false`. If the property is set to `true`
> > +       and `submodule.<name>.branch`, it will default to `master`
>
> I think you mean "...and 'submodule.<name>.branch' is not set, it
> will...", right?

>
> > @@ -475,8 +479,17 @@ Use -f if you really want to add it." >&2
> >                         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" ;;
> > +                       '')
> > +                               git checkout -f -q
> > +                               ;;
>
> Is this whitespace change intentional and necessary?
>

>
> >  }
> > @@ -622,7 +641,7 @@ cmd_init()
> >                    test -z "$(git config submodule."$name".update)"
> >                 then
> >                         case "$upd" in
> > -                       rebase | merge | none)
> > +                       checkout | rebase | merge | none)
>
> This belongs in a commit of its own.
>

>
> >                                 ;; # known modes of updating
> >                         *)
> >                                 echo >&2 "warning: unknown update mode '$upd' suggested for submodule '$name'"
> > @@ -632,6 +651,23 @@ cmd_init()
> >                         git config submodule."$name".update "$upd" ||
> >                         die "$(eval_gettext "Failed to register update mode for submodule path '\$displaypath'")"
> >                 fi
> > +
> > +               # Copy "attach" setting when it is not set yet
> > +               if attached="$(git config -f .gitmodules submodule."$name".attach)" &&
> > +                  test -n "$attached" &&
> > +                  test -z "$(git config submodule."$name".attach)"
> > +               then
> > +                       case "$attached" in
> > +                       true | false)
> > +                               ;; # Valid attach flag values
> > +                       *)
> > +                               echo >&2 "warning: invalid attach flag value for submodule '$name'"
> > +                               upd=none
> > +                               ;;
> > +                       esac
> > +                       git config submodule."$name".attach "$attached" ||
> > +                       die "$(eval_gettext "Failed to register attached option for submodule path '\$displaypath'")"
> > +               fi
> >         done
> >  }
> >
> > @@ -750,6 +786,14 @@ cmd_update()
> >                 --reference=*)
> >                         reference="$1"
> >                         ;;
> > +               --attach)
> > +                       if test "$attach" = "false" ; then usage ; fi
> > +                       attach="true"
> > +                       ;;
> > +               --detach)
> > +                       if test "$attach" = "true" ; then usage ; fi
> > +                       attach="false"
> > +                       ;;
> >                 -m|--merge)
> >                         update="merge"
> >                         ;;
> > @@ -800,11 +844,44 @@ cmd_update()
> >                 name=$(module_name "$sm_path") || exit
> >                 url=$(git config submodule."$name".url)
> >                 branch=$(get_submodule_config "$name" branch master)
> > +               if test -n "$attach"
> > +               then
> > +                       attach_module=$attach
> > +               else
> > +                       attach_module=$(git config submodule."$name".attach)
> > +                       case "$attach_module" in
> > +                       '')
> > +                               ;; # Unset attach flag
> > +                       true|false)
> > +                               ;; # Valid attach flag values
> > +                       *)
> > +                               echo >&2 "warning: invalid attach flag value for submodule '$name'"
> > +                               attach_module=
> > +                               ;;
> > +                       esac
> > +               fi
> > +               if test "$attach_module" = "false"
> > +               then
> > +                       # Normalize attach 'false' flag value
> > +                       attach_module=
> > +               fi
> >                 if ! test -z "$update"
> >                 then
> >                         update_module=$update
> >                 else
> >                         update_module=$(git config submodule."$name".update)
> > +                       case "$update_module" in
> > +                       '')
> > +                               ;; # Unset update mode
> > +                       checkout | rebase | merge | none)
> > +                               ;; # Known update modes
> > +                       !*)
> > +                               ;; # Custom update command
> > +                       *)
> > +                               update_module=
> > +                               echo >&2 "warning: invalid update mode for submodule '$name'"
> > +                               ;;
> > +                       esac
>
> Probably belongs to the same "other" commit mentioned before.
>

>
> I didn't have time to parse out all these conditional completion
> commands in this review, but the feature seems sane to me, as I
> understand it.
>

> Thanks for the comments, my replies below. Before, a couple of general
> questions:
> - I'm also writing some tests, should I commit them together with the
> feature patch?
> - to determine the attached/detached state I did this:
>
> head_detached=
> if test "$(rev-parse --abbrev-ref HEAD)" = "HEAD"
> then
>     head_detached="true"
> fi

> by default "git submodule" performs its add or update operations on a detached
> HEAD. This works well when using an existing full-fledged/indipendent project as
> the submodule, as there's less frequent need to update it or commit back
> changes. When the submodule is actually a large portion of shareable code
> between  different projects, and the superproject needs to track very closely
> the evolution of the submodule (or the other way around), I feel more confortable
> to reattach the HEAD of the submodule with an existing branch.

> Francesco Pretto <ceztko@gmail.com> writes:
>
>> by default "git submodule" performs its add or update operations on a detached
>> HEAD. This works well when using an existing full-fledged/indipendent project as
>> the submodule, as there's less frequent need to update it or commit back
>> changes. When the submodule is actually a large portion of shareable code
>> between  different projects, and the superproject needs to track very closely
>> the evolution of the submodule (or the other way around), I feel more confortable
>> to reattach the HEAD of the submodule with an existing branch.
>
> I may be missing some fundamental assumption in your mind when you
> did this change, but in a workflow where somebody wants submodule
> checkout to be on branches (as opposed to detached), wouldn't it
> make more sense not to detach in the first place, rather than
> introducing yet another option to "re-attach"?  The documentation of
> "submodule update" seems to say that its "merge" and "rebase" modes
> do not detach in the first place (and it alludes to "--checkout" but
> it is unclear what it does purely from the documentation, as "git
> submodule --help" does not even list it as one of the options).
>

> And if there is a good reason why detaching to update and then
> (perhaps after verifying the result or cleaning it up?  I dunno what
> the expected use case is, so I am purely guessing) attaching the
> result to a specific branch in separate steps, does it make sense to
> give "--attach" option to "update" in the first place?  That makes
> the whole thing into a single step, not giving the user a chance to
> do anything in between, which I am guessing is the whole point of
> your not using the existing "do not detach, work on a branch" modes.
>

> Concluding, my point is that at the current state submodules in git
> seem to be flawed because of the inconsistent HEAD state between "add"
> and "update" users. With my patch applied the attached HEAD behavior
> would be fully supported. At some point "git submodule add" (without
> the "--attached" switch) could be also modified to produce a detached
> HEAD by default, removing any remaining inconsistency.
>

> Concluding, my point is that at the current state submodules in git
> seem to be flawed because of the inconsistent HEAD state between "add"
> and "update" users. With my patch applied the attached HEAD behavior
> would be fully supported. At some point "git submodule add" (without
> the "--attached" switch) could be also modified to produce a detached
> HEAD by default, removing any remaining inconsistency.

> From: "W. Trevor King" <wking@tremily.us>
> 
> 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.
> 
> Signed-off-by: W. Trevor King <wking@tremily.us>
> ---
> 
> On Fri, Jan 03, 2014 at 09:49:01AM +0100, Francesco Pretto wrote:
> > - there's a developer "update" user. He will clone the submodule
> > repository with an *attached* HEAD. Subsequent "merge" or "rebase"
> > update operations will keep the HEAD attached.
> 
> 'merge' and 'rebase' updates don't change the HEAD attachment.
> Branches stay branches and detached HEADs stay detached.  If you've
> moved away from the 'checkout' update mechanism, the only thing you
> still need is a way to get an initial checkout on a branch.  This
> should do it (I can add tests if folks like the general approach).
> 
>  git-submodule.sh | 30 +++++++++++++++++-------------
>  1 file changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 2979197..e2e5a6c 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,14 @@ 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" &&
> +		if test -n "$branch"; then
> +			git checkout -f -q -B "$branch" "origin/$branch" && echo "checked out $branch"
> +		fi
> +	) || die "$(eval_gettext "Unable to setup cloned submodule '\$sm_path'")"
>  }
>  
>  isnumber()
> @@ -469,16 +477,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"
>  
> @@ -815,7 +814,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" "$branch" || exit
>  			cloned_modules="$cloned_modules;$name"
>  			subsha1=
>  		else
> @@ -861,7 +860,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 "$branch"; then
> +					update_module="!git reset --hard -q"

> On Fri, Jan 03, 2014 at 10:06:11AM -0800, W. Trevor King wrote:
> > @@ -306,7 +307,14 @@ 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")
> 
> Why should this line be removed? Is it not needed for correct
> worktree <-> repo linking of submodules?
> 
> > +	(
> > +		clear_local_git_env
> > +		cd "$sm_path" &&
> > +		GIT_WORK_TREE=. git config core.worktree "$rel/$b" &&
> > +		if test -n "$branch"; then
> > +			git checkout -f -q -B "$branch" "origin/$branch" && echo "checked out $branch"
> > +		fi
> > +	) || die "$(eval_gettext "Unable to setup cloned submodule '\$sm_path'")"
> >  }

> > @@ -861,7 +860,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 "$branch"; then
> > +					update_module="!git reset --hard -q"
> 
> Does that not put the user in danger of loosing changes?

> If submodule.<name>.branch is configured:
> 
> 	git submodule update
> 
> will checkout the configured branch instead of the sha1?

> On Sat, Jan 04, 2014 at 11:09:15PM +0100, Heiko Voigt wrote:
> > On Fri, Jan 03, 2014 at 10:06:11AM -0800, W. Trevor King wrote:
> > > @@ -306,7 +307,14 @@ 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")
> > 
> > Why should this line be removed? Is it not needed for correct
> > worktree <-> repo linking of submodules?
> > 
> > > +	(
> > > +		clear_local_git_env
> > > +		cd "$sm_path" &&
> > > +		GIT_WORK_TREE=. git config core.worktree "$rel/$b" &&
> > > +		if test -n "$branch"; then
> > > +			git checkout -f -q -B "$branch" "origin/$branch" && echo "checked out $branch"
> > > +		fi
> > > +	) || die "$(eval_gettext "Unable to setup cloned submodule '\$sm_path'")"
> > >  }
> 
> It's not removed, just merged with the branch manipulation that also
> happens in a clean local environment in $sm_path.  Spawning two
> sequential subshells with the same setup seemed like overkill.

> > > @@ -861,7 +860,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 "$branch"; then
> > > +					update_module="!git reset --hard -q"
> > 
> > Does that not put the user in danger of loosing changes?
> 
> No, because this is only happens for just-cloned modules.  The user
> hasn't had time to make local changes yet.

> > If submodule.<name>.branch is configured:
> > 
> > 	git submodule update
> > 
> > will checkout the configured branch instead of the sha1?
> 
> The use case described by Francesco, a submodule maintainer Alice sets
> up the submodule, which downstream developer Bob want to checkout to a
> branch.  I think that matching the exact commit specified by Alice in
> Bob's checkout is important, even if the upstream developer Charlie
> has advanced the referenced branch in the meantime.  Shifting the
> referenced submodule commit should be an explicit decision made by
> Alice, not a clone-time accident for Bob.

> # Developer
> $ git pull
> $ git submodule init
> $ git submodule update --remote
> $ cd <path>
> $ branch="$(git config -f ..\.gitmodules submodule.common.branch)"; git checkout $branch

> On Sat, Jan 04, 2014 at 02:54:01PM -0800, W. Trevor King wrote:
> > On Sat, Jan 04, 2014 at 11:09:15PM +0100, Heiko Voigt wrote:
> > > On Fri, Jan 03, 2014 at 10:06:11AM -0800, W. Trevor King wrote:
> > > > @@ -861,7 +860,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 "$branch"; then
> > > > +					update_module="!git reset --hard -q"
> > > 
> > > Does that not put the user in danger of loosing changes?
> > 
> > No, because this is only happens for just-cloned modules.  The user
> > hasn't had time to make local changes yet.
> 
> Ah ok I see. But why the reset then? Doesn't the earlier git
> checkout in your patch take care of checking out the branch and thus
> update to the right revision?

> > > If submodule.<name>.branch is configured:
> > > 
> > > 	git submodule update
> > > 
> > > will checkout the configured branch instead of the sha1?
> > 
> > The use case described by Francesco, a submodule maintainer Alice
> > sets up the submodule, which downstream developer Bob want to
> > checkout to a branch.  I think that matching the exact commit
> > specified by Alice in Bob's checkout is important, even if the
> > upstream developer Charlie has advanced the referenced branch in
> > the meantime.  Shifting the referenced submodule commit should be
> > an explicit decision made by Alice, not a clone-time accident for
> > Bob.
> 
> But from what I understand of this part of Francesco's use-case
> description:
> 
> > # Developer
> > $ git pull
> > $ git submodule init
> > $ git submodule update --remote
> > $ cd <path>
> > $ branch="$(git config -f ..\.gitmodules submodule.common.branch)"; git checkout $branch
> 
> Is that he wants to allow the developer to switch to following a
> branch instead of an exact sha1 while some extension in the common
> module is still under development. That makes it easier to develop
> in parallel in the submodule and the superproject because you do not
> need to update the sha1 all the time.

>
> 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.
> [...]
> @@ -306,7 +307,14 @@ 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" &&
> +               if test -n "$branch"; then
> +                       git checkout -f -q -B "$branch" "origin/$branch" && echo "checked out $branch"
> +               fi
> +       ) || die "$(eval_gettext "Unable to setup cloned submodule '\$sm_path'")"
>  }

> From: "W. Trevor King" <wking@tremily.us>
> 
> 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.
> 
> Signed-off-by: W. Trevor King <wking@tremily.us>
> ---
> Changes since v1:
> * Fix a 'reqested' -> 'requested' typo in the subject/summary.
> * Restore a post-clone 'git checkout -f -q' for the empty-branch case
>   in module_clone().
> * Distinguish between $branch (which defaults to 'master') and a new
>   $config_branch (which defaults to empty) in cmd_update
> 
> After these fixes, all the existing submodule tests pass.  If we want
> to merge this, we'd still want new tests that demonstrate the new
> functionality.

> On Sun, Jan 05, 2014 at 04:53:12AM +0100, Francesco Pretto wrote:
> > If I understand it correctly, looking at your intervention in
> > module_clone and cmd_update, when "submodule.<module>.branch" is set
> > during "update" the resulting first clone will always be a branch
> > checkout (cause $branch is filled with "branch" property).
> 
> That's correct.
> 
> > I believe this will break a lot of tests,
> 
> Thanks for prompting me to run the tests.  This v2 now passes all of
> the current submodule tests, and the functionality actually matches my
> earlier descriptions of it ;).
> 
> > as the the documentation says that in this configuration the HEAD
> > should be detached.
> 
> 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`.
> 
> It's not clear if this refers to the initial-clone update, future
> post-clone updates, or both.  Ideally, the behavior should be the
> same, but in the initial-clone case we don't have an existing
> checked-out branch to work with.

> > Also it could break some users that rely on the current behavior.
> 
> The current code always has a detached HEAD after an initial-clone
> update, regardless of submodule.<name>.update, which doesn't match
> those docs either.  Adding a check to only checkout
> submodule.<name>.branch if submodule.<name>.update was 'rebase',
> 'merge', or 'none' would be easy, but I don't think that makes much
> sense.  I can't see any reason for folks who specify
> submodule.<name>.branch to prefer a detached HEAD over a local branch
> matching the remote branch's name.  If they prefer checkout updates,
> the first such update will return them to a detached HEAD.  If they
> prefer merge/rebase updates, future updates will keep them on the same
> branch.  All my commit does is setup that initial branch for folks who
> get the submodule via 'update', in the same way it's currently setup
> for folks who get the submodule via 'add'.

>  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"

> On Sun, Jan 05, 2014 at 08:17:00AM -0800, W. Trevor King wrote:
> > It's not clear if this refers to the initial-clone update, future
> > post-clone updates, or both.  Ideally, the behavior should be the
> > same, but in the initial-clone case we don't have an existing
> > checked-out branch to work with.
> 
> I do not think that its actually important to end up with a detached
> HEAD. The documentation just states it because in most cases there
> is no other option. But I do not think anything will break if a
> branch points to the exact sha1 we would checkout and we checkout
> the branch instead.

> > -			module_clone "$sm_path" "$name" "$url" "$reference" "$depth" || exit
> > +			module_clone "$sm_path" "$name" "$url" "$reference" "$depth" "$config_branch" || exit
> 
> In the simple case (update=checkout, no branch specified) with the
> new checkout branch stuff in module_clone() this code here ends up
> calling checkout twice.  First for master and then here later with
> the sha1.  This feels a little bit double.

> > @@ -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'")"
> >  }

> > -				update_module= ;;
> > +				if test -n "$config_branch"; then
> > +					update_module="!git reset --hard -q"
> 
> If we get here the checkout has already been done. Shouldn't this
> rather specify a noop. I.E. like
> 
> 	update_module="!true"

> On Sun, Jan 05, 2014 at 08:48:50PM +0100, Heiko Voigt wrote:
> > On Sun, Jan 05, 2014 at 08:17:00AM -0800, W. Trevor King wrote:
> > > It's not clear if this refers to the initial-clone update, future
> > > post-clone updates, or both.  Ideally, the behavior should be the
> > > same, but in the initial-clone case we don't have an existing
> > > checked-out branch to work with.
> > 
> > I do not think that its actually important to end up with a detached
> > HEAD. The documentation just states it because in most cases there
> > is no other option. But I do not think anything will break if a
> > branch points to the exact sha1 we would checkout and we checkout
> > the branch instead.
> 
> There's no "if the remote-tracking branch points to the exact sha1"
> logic in my patch.

> Thinking through this more, perhaps the
> logic should be:
> 
> * If submodule.<name>.update (defaulting to checkout) is checkout,
>   create a detached HEAD.
> * Otherwise, create a new branch submodule.<name>.branch (defaulting
>   to master).

> The motivation is that if submodule.<name>.update is checkout, the
> user is unlikely to be developing locally in the submodule, as
> subsequent updates would clobber their local commits.  Having a
> detached HEAD is a helpful "don't develop here" reminder ;).  If
> submodule.<name>.update is set, the user is likely to be developing
> locally, and will probably want a local branch already checked out to
> facilitate that.
> 
> > > -			module_clone "$sm_path" "$name" "$url" "$reference" "$depth" || exit
> > > +			module_clone "$sm_path" "$name" "$url" "$reference" "$depth" "$config_branch" || exit
> > 
> > In the simple case (update=checkout, no branch specified) with the
> > new checkout branch stuff in module_clone() this code here ends up
> > calling checkout twice.  First for master and then here later with
> > the sha1.  This feels a little bit double.
> 
> There is no guarantee that the remote master and the exact sha1 point
> at the same commit.  Ideally we'd just clone the exact sha1 in this
> case.
> 
> > I would prefer if we skip the checkout in module_clone() if its not
> > necessary.
> 
> When I tried to drop the '' case here:
> 
> > > @@ -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'")"
> > >  }
> 
> I got test-suite errors that I didn't get to the bottom of.  However…
> 
> > How about we move the whole "what to checkout"-decision into one place
> > instead of having it in update() and moving it from add() into
> > module_clone() ?
> 
> …this sounds like a good idea to me.  However, it would be a more
> intrusive change, and there may be conflicts with Francesco's proposed
> attach/detach functionality.  I'll wait until we have a clearer idea
> of where that is headed before I attempt a more complete
> consolidation.

> > > -				update_module= ;;
> > > +				if test -n "$config_branch"; then
> > > +					update_module="!git reset --hard -q"
> > 
> > If we get here the checkout has already been done. Shouldn't this
> > rather specify a noop. I.E. like
> > 
> > 	update_module="!true"
> 
> We are on a local branch at this point, but not neccessarily pointing
> at the gitlinked sha1.  The reset here ensures that the new local
> branch does indeed point at the gitlinked sha1.

> On Sun, Jan 05, 2014 at 01:24:58PM -0800, W. Trevor King wrote:
> > If submodule.<name>.branch is set, it *always* creates a new local
> > branch of that name pointing to the exact sha1.  If
> > submodule.<name>.branch is not set, we still create a
> > detached-HEAD checkout of the exact sha1.
> 
> Thanks for this clarification. Since the usual usage with --remote
> is with a remote-tracking branch, I confused this here. I am not
> sure whether blindly creating a local branch from the recorded sha1
> is the right thing to do. In what situations would that be helpful?

> The reason why one would set a branch option here is to share the
> superproject branch with colleagues. He can make sure they can
> always fetch and checkout the submodule even though the branch there
> is still under cleanup and thus will be rebased often. The commit
> referenced by sha1 would not be available to a developer fetching
> after a rebase.

> > Thinking through this more, perhaps the logic should be:
> > 
> > * If submodule.<name>.update (defaulting to checkout) is checkout,
> >   create a detached HEAD.
> > * Otherwise, create a new branch submodule.<name>.branch
> >   (defaulting to master).
> 
> Why not trigger the attached state with the submodule.<name>.branch
> configuration option? If there is a local branch available use that,
> if not the tracking branch (as it is currently). Then a developer
> can start working on the branch with:
> 
> 	cd submodule; git checkout -t origin/<branchname>
> 
> assuming that submodule update learns some more support for this.

> > > > -				update_module= ;;
> > > > +				if test -n "$config_branch"; then
> > > > +					update_module="!git reset --hard -q"
> > > 
> > > If we get here the checkout has already been done. Shouldn't
> > > this rather specify a noop. I.E. like
> > > 
> > > 	update_module="!true"
> > 
> > We are on a local branch at this point, but not neccessarily
> > pointing at the gitlinked sha1.  The reset here ensures that the
> > new local branch does indeed point at the gitlinked sha1.
> 
> But isn't this a fresh clone? Why should the branch point at
> anything else?

> On Sun, Jan 05, 2014 at 11:57:33PM +0100, Heiko Voigt wrote:
> > The reason why one would set a branch option here is to share the
> > superproject branch with colleagues. He can make sure they can
> > always fetch and checkout the submodule even though the branch there
> > is still under cleanup and thus will be rebased often. The commit
> > referenced by sha1 would not be available to a developer fetching
> > after a rebase.
> 
> Yeah, floating gitlinks are something else.  I'd be happy to have
> that functionality (gitlinks pointing to references) be built into
> gitlinks themselves, not added as an additional layer in the
> submodule script.  This "gitlinked sha1 rebased out of existence"
> scenario is the first I've heard where I think gitlinked references
> would be useful.

> The only people who would need *automatic* rebase recovery would be
> superproject devs update-cloning the subproject.  That's a small
> enough cross-section that I don't think it deserves the ambiguity of
> gitlink-to-reference.  In that case, all you really need is a way to
> force a recovery gitlink (i.e. add a 'commit' object to the tree by
> hand).

> On Sun, Jan 05, 2014 at 04:33:14PM -0800, W. Trevor King wrote:
> > The only people who would need *automatic* rebase recovery would be
> > superproject devs update-cloning the subproject.  That's a small
> > enough cross-section that I don't think it deserves the ambiguity of
> > gitlink-to-reference.  In that case, all you really need is a way to
> > force a recovery gitlink (i.e. add a 'commit' object to the tree by
> > hand).
> 
> Actually, you recovering by hand is a lot easier.  Setup a
> rebased-away gitlink target:
> 
>   mkdir subproject &&
>   (
>     cd subproject &&
>     git init
>     echo 'Subproject' > README &&
>     git add README &&
>     git commit -m 'Subproject v1' &&
>     echo 'Changes' >> README &&
>     git commit -am 'Subproject v2'
>   ) &&
>   mkdir superproject &&
>   (
>     cd superproject &&
>     git init
>     git submodule add ../subproject &&
>     git commit -m 'Superproject v1'
>   ) &&
>   (
>     cd subproject &&
>     git reset --hard HEAD^ &&
>     git reflog expire --expire=now --all &&
>     git gc --aggressive --prune=now
>   )
> 
> Then a recursive clone of the superproject dies:
> 
>   $ git clone --recursive superproject super2
>   Cloning into 'super2'...
>   done.
>   Submodule 'subproject' (/tmp/x/subproject) registered for path 'subproject'
>   Cloning into 'subproject'...
>   done.
>   fatal: reference is not a tree: f589144d16282d1a80d17a9032c6f1d332e38dd0
>   Unable to checkout 'f589144d16282d1a80d17a9032c6f1d332e38dd0' in submodule path 'subproject'
> 
> But you still have the submodule checkout (up until the $sha1 setup):
> 
>   $ cd super2
>   $ git diff
>   diff --git a/subproject b/subproject
>   index f589144..82d4553 160000
>   --- a/subproject
>   +++ b/subproject
>   @@ -1 +1 @@
>   -Subproject commit f589144d16282d1a80d17a9032c6f1d332e38dd0
>   +Subproject commit 82d4553fe437ae014f22bbc87a082c6d19e5d9f9-dirty
> 
> And you can automatically update to match the upstream remote:
> 
>   $ git submodule update --remote --force
>   Submodule path 'subproject': checked out '82d4553fe437ae014f22bbc87a082c6d19e5d9f9'
>   $ git diff
>   diff --git a/subproject b/subproject
>   index f589144..82d4553 160000
>   --- a/subproject
>   +++ b/subproject
>   @@ -1 +1 @@
>   -Subproject commit f589144d16282d1a80d17a9032c6f1d332e38dd0
>   +Subproject commit 82d4553fe437ae014f22bbc87a082c6d19e5d9f9
> 
> When explicitly updating to the superproject or subproject's
> (--remote) new tip is so easy, I don't see a need for floating the
> gitlinks themselves.

>
> I agree. If we were to support this more easily we could add a
> configuration option so you can omit the --remote (i.e.:
> submodule.<name>.remote=true, as I also suggested in the other email).
>
> That way the developer checking out a branch in flight does not even
> need to know whether (and which) submodules sha1s are still in flight
> and temporarily set this configuration in the branches .gitmodules file.
>

> Maybe that could actually be the attach operation Francesco is
> suggesting:
>
>         git submodule attach [--pull] <submodule path> <branchname>
>
> will attach the specified submodule to a branch. That means it changes
> the .gitmodule file accordingly and stages it. With the --pull switch
> one can specify whether a local branch tracking the remote branch should
> be automatically created. Names and the command format are just a
> suggestion here.
>
> That way we can support the
>
>         fork superproject needing submodule changes and send submodule
>         changes upstream first.
>

> To not break the existing behavior what it's really needed here, IMO,
> is a "submodule.<name>.attached" property that says two things:
> - at the first clone on "git submodule update" stay attached to
> "submodule.<name>.branch";
> - implies "--remote", as it's the only thing that makes sense when the
> submodules are attached.
>

> 2014/1/7 Francesco Pretto <ceztko@gmail.com>:
>> To not break the existing behavior what it's really needed here, IMO,
>> is a "submodule.<name>.attached" property that says two things:
>> - at the first clone on "git submodule update" stay attached to
>> "submodule.<name>.branch";
>> - implies "--remote", as it's the only thing that makes sense when the
>> submodules are attached.
>>
>
> Unless you decide to go with the proposed approach of Trevor, where
> "submodule.<name>.branch" set means attached (if it's not changed:
> this thread is quite hard to follow...). To this end, Junio could sync
> with more "long-timers" (Heiko?) submodule users/devs to understand if
> this breaks too much or not.

>> Unless you decide to go with the proposed approach of Trevor, where
>> "submodule.<name>.branch" set means attached (if it's not changed:
>> this thread is quite hard to follow...). To this end, Junio could sync
>> with more "long-timers" (Heiko?) submodule users/devs to understand if
>> this breaks too much or not.
>
> It is not immediately obvious to me why anybody who specifies the
> submodule.*.branch variable to say "I want _that_ branch" not to
> want to be on that branch but in a detached state, so from that
> perspective, submodule.*.attach feels superfluous.
>

> 2014/1/7 Junio C Hamano <gitster@pobox.com>:
> > It is not immediately obvious to me why anybody who specifies the
> > submodule.*.branch variable to say "I want _that_ branch" not to
> > want to be on that branch but in a detached state, so from that
> > perspective, submodule.*.attach feels superfluous.
> 
> Junio, for what it concerns me I fully support this patch as, IMO, it
> makes cleaner the role of the property "submodule.<name>.branch".

> On Tue, Jan 07, 2014 at 08:19:49PM +0100, Francesco Pretto wrote:
> > 2014/1/7 Junio C Hamano <gitster@pobox.com>:
> > > It is not immediately obvious to me why anybody who specifies the
> > > submodule.*.branch variable to say "I want _that_ branch" not to
> > > want to be on that branch but in a detached state, so from that
> > > perspective, submodule.*.attach feels superfluous.
> > 
> > Junio, for what it concerns me I fully support this patch as, IMO, it
> > makes cleaner the role of the property "submodule.<name>.branch".
> 
> No, submodule.<name>.branch is the name of the remote-tracking branch
> for 'update --remote'.  In this patch, I'm using it as a hint for the
> preferred local branch name [1], which I now think was a bad idea.
> After [2], I think that we should just define the preferred local
> branch name explicitly (submodule.<name>.local-branch?).

> One thing is missing though (and I think thats where Francesco came
> from): What if the developer already has a detached HEAD in the
> submodule?
>
> How does he attach to a branch? For this we need something similar to
> Francescos attach/detach or Trevors submodule checkout with Junio's checkout
> HEAD~0 from here[1].
>
> I am still undecided how we should call it. Because of my
>
> Idea for feature branch support
> - -------------------------------
>
> For the branch attaching feature I would also like something that can actually
> modify .git/config and for me more importantly .gitmodules.
>
> So e.g. if I want to work on a longer lived feature branch in a submodule which
> I need in a feature branch in the superproject I would do something like this:
>
> $ git submodule checkout --gitmodules --merge -b hv/my-cool-feature
>

> # Unset  ".git/config" 'submodule.<module>.branch'
> # Also attach or detach the HEAD according to what is in ".gitmodules":
> # with Trevor's patch 'submodule.<module>.branch' set means attached,
> # unset means detached
> $ git submodule head --reset <module>

> NOTE: feature branch part!
> 
> # Set ".gitmodules" 'submodule.<module>.branch' to <branch>
> $ git submodule head -b <branch> --attach --index <module>
> 
> # Unset ".gitmodules" 'submodule.<module>.branch'
> $ git submodule head --reset --index <module>
> ---------------------------------------------------------------------

> On Wed, Jan 08, 2014 at 01:17:49AM +0100, Francesco Pretto wrote:
>> # Attach the submodule HEAD to <branch>.
>> # Also set ".git/config" 'submodule.<module>.branch' to <branch>
>> $ git submodule head -b <branch> --attach <module>
> [...]
> I also prefer 'checkout' to 'head', because 'checkout'
> already exists in non-submodule Git for switching between local
> branches.
>

>> # Attach the submodule HEAD to 'submodule.<module>.branch'.
>> # If it does not exists defaults to <remote>/master
>> $ git submodule head --attach <module>
>
> Defaulting to the configured local branch is fine, but I think it
> should default to 'master' if no local branch is configured.  This
> should not have anything to do with remote-tracking branches (that's
> what 'submodule update' already handles).  I don't understand why
> remote-tracking-branch integration keeps getting mixed up with
> local-branch checkout.
>

>> # Unset  ".git/config" 'submodule.<module>.branch'
>> # Also attach or detach the HEAD according to what is in ".gitmodules":
>> # with Trevor's patch 'submodule.<module>.branch' set means attached,
>> # unset means detached
>> $ git submodule head --reset <module>
>
> To me this reads “always detach HEAD” (because it unsets
> submodule.<name>.branch, and submodule.<name>.branch unset means
> detached).

>> NOTE: feature branch part!
>>
>> # Set ".gitmodules" 'submodule.<module>.branch' to <branch>
>> $ git submodule head -b <branch> --attach --index <module>
>>
>> # Unset ".gitmodules" 'submodule.<module>.branch'
>> $ git submodule head --reset --index <module>
>> ---------------------------------------------------------------------
>
> These are just manipulating .gitmodules.  I think we also need
> per-superproject-branch configs under the superproject's .git/ for
> developer overrides.
>

> To elaborate the idea I sketched out here [2], say
> you want:
>
>   Superproject branch  Submodule branch  Upstream branch
>   ===================  ================  ===============
>   master               master            master
>   super-feature        master            master
>   my-feature           my-feature        master
>   other-feature        other-feature     other-feature
>
> That's only going to work with per-superproject-branch configs for
> both the local and remote branches.  Using the same name for both
> local and remote branches does not work.
>

>
>   [branch "my-feature"]
>         remote = origin
>         merge = refs/heads/my-feature
>         [submodule "submod"]
>             local-branch = "my-feature"
>
> and I don't think Git's config supports such nesting.
>

>
> I can resuscitate that if folks want, but Heiko felt that my initial
> consolidation didn't go far enough [2].  If it turns out that we're ok
> with the current level of consolidation, would you be ok with
> "non-checkout submodule.<name>.update" as the trigger [3]?

> 2014/1/9 W. Trevor King <wking@tremily.us>:
> >   [branch "my-feature"]
> >         remote = origin
> >         merge = refs/heads/my-feature
> >         [submodule "submod"]
> >             local-branch = "my-feature"
> >
> > and I don't think Git's config supports such nesting.
> 
> Aesthetically, It doesn't look very nice.

> 2014/1/9 W. Trevor King <wking@tremily.us>:
>>
>> However, submodule.<name>.local-branch has nothing to do with remote
>> repositories or tracking branches.
> 
> My bad: this means the feature is still not entirely clear to me.
> 
>>
>>   [branch "my-feature"]
>>         remote = origin
>>         merge = refs/heads/my-feature
>>         [submodule "submod"]
>>             local-branch = "my-feature"
>>
>> and I don't think Git's config supports such nesting.
>>
> 
> Aesthetically, It doesn't look very nice.

>> I can resuscitate that if folks want, but Heiko felt that my initial
>> consolidation didn't go far enough [2].  If it turns out that we're ok
>> with the current level of consolidation, would you be ok with
>> "non-checkout submodule.<name>.update" as the trigger [3]?
> 
> For me it was ok with what you did:
> -------------------------------------------------
> if "just_cloned" and "config_branch"
> then
>      !git reset --hard -q"
> fi
> -------------------------------------------------
> 
> So yes: at the first clone 'checkout' keeps attached HEAD, while
> 'merge' and 'rebase' attach to the branch.

>>  I think
>> that adding a halfway step between the current status and full(ish)
>> submodule.<name>.local-branch support is just going to confuse people
> 
> Well, for now you got some success in confusing me with this "local-branch" :)
> 
> At certain point  you may ask maintainers what are the accepted
> features (because all these debates should be about getting, or not
> getting, endorsement about something) and finalize a patch so people
> can further review.

> Am 09.01.2014 02:09, schrieb Francesco Pretto:
> > 2014/1/9 W. Trevor King <wking@tremily.us>:
> >>
> >> However, submodule.<name>.local-branch has nothing to do with remote
> >> repositories or tracking branches.
> > 
> > My bad: this means the feature is still not entirely clear to me.
> > 
> >>
> >>   [branch "my-feature"]
> >>         remote = origin
> >>         merge = refs/heads/my-feature
> >>         [submodule "submod"]
> >>             local-branch = "my-feature"
> >>
> >> and I don't think Git's config supports such nesting.
> >>
> > 
> > Aesthetically, It doesn't look very nice.
> 
> And I'm not sure we even need that. What's wrong with having the
> branch setting in the .gitmodules file of the my-feature branch?
> The only problem I can imagine is accidentally merging that into
> a branch where that isn't set, but that could be solved by a merge
> helper for the .gitmodules file.

> >> I can resuscitate that if folks want, but Heiko felt that my initial
> >> consolidation didn't go far enough [2].  If it turns out that we're ok
> >> with the current level of consolidation, would you be ok with
> >> "non-checkout submodule.<name>.update" as the trigger [3]?
> > 
> > For me it was ok with what you did:
> > -------------------------------------------------
> > if "just_cloned" and "config_branch"
> > then
> >      !git reset --hard -q"
> > fi
> > -------------------------------------------------
> > 
> > So yes: at the first clone 'checkout' keeps attached HEAD, while
> > 'merge' and 'rebase' attach to the branch.
> 
> It have the impression that attaching the head to the given branch
> for merge and rebase might be the sensible thing to do, but it
> would be great to hear from users of merge and rebase if that
> would break anything for them in their current use cases for these
> settings.

> > If it's not the first clone, you should take no action (and your
> > original patch was ok about this).
> 
> I'm not sure this is the right thing to do, after all you
> configured git to follow that branch so I'd expect it to be
> updated later too, no? Otherwise you might end up with an old
> version of your branch while upstream is a zillion commits
> ahead.

> And from reading this discussion I believe we need another value for
> the ignore option which only ignores changes to the SHA-1 but not to
> work tree modifications of a submodule work tree relative to its HEAD
> (or make that two: another one which ignores untracked files too and
> only shows modification of tracked files). Otherwise users of the
> floating or attached model can easily miss submodule modifications.

> On Thu, Jan 09, 2014 at 09:31:13AM +0100, Jens Lehmann wrote:
>> Am 09.01.2014 02:09, schrieb Francesco Pretto:
>>> 2014/1/9 W. Trevor King <wking@tremily.us>:
>>>>
>>>> However, submodule.<name>.local-branch has nothing to do with remote
>>>> repositories or tracking branches.
>>>
>>> My bad: this means the feature is still not entirely clear to me.
>>>
>>>>
>>>>   [branch "my-feature"]
>>>>         remote = origin
>>>>         merge = refs/heads/my-feature
>>>>         [submodule "submod"]
>>>>             local-branch = "my-feature"
>>>>
>>>> and I don't think Git's config supports such nesting.
>>>>
>>>
>>> Aesthetically, It doesn't look very nice.
>>
>> And I'm not sure we even need that. What's wrong with having the
>> branch setting in the .gitmodules file of the my-feature branch?
>> The only problem I can imagine is accidentally merging that into
>> a branch where that isn't set, but that could be solved by a merge
>> helper for the .gitmodules file.
> 
> .gitmodules is fine so long as the config can live in the versioned
> tree.  Many (all?) .gitmodules settings can be overridden in
> .git/config.

>  However, the local-branch setting needs to be both
> per-submodule and per-superproject-branch, so .git/config doesn't work
> very well.  I think it's better to use something like my
> .git/modules/<submodule-name>/config implementation [1] to set this
> override.

>>>> I can resuscitate that if folks want, but Heiko felt that my initial
>>>> consolidation didn't go far enough [2].  If it turns out that we're ok
>>>> with the current level of consolidation, would you be ok with
>>>> "non-checkout submodule.<name>.update" as the trigger [3]?
>>>
>>> For me it was ok with what you did:
>>> -------------------------------------------------
>>> if "just_cloned" and "config_branch"
>>> then
>>>      !git reset --hard -q"
>>> fi
>>> -------------------------------------------------
>>>
>>> So yes: at the first clone 'checkout' keeps attached HEAD, while
>>> 'merge' and 'rebase' attach to the branch.
>>
>> It have the impression that attaching the head to the given branch
>> for merge and rebase might be the sensible thing to do, but it
>> would be great to hear from users of merge and rebase if that
>> would break anything for them in their current use cases for these
>> settings.
> 
> Which local branch would you attach to before merging?  I think 'git
> submodule update' should always use the current submodule state
> (attached branch or detached HEAD) [3], and we should have a separate
> call that explicitly checked out the desired submodule branch [4].

>>> If it's not the first clone, you should take no action (and your
>>> original patch was ok about this).
>>
>> I'm not sure this is the right thing to do, after all you
>> configured git to follow that branch so I'd expect it to be
>> updated later too, no? Otherwise you might end up with an old
>> version of your branch while upstream is a zillion commits
>> ahead.
> 
> Non-clone updates should not change the submodule's *local* branch
> *name*.  They should change the commit that that branch references,
> otherwise 'git submodule update' would be a no-op ;).

>> First I'd like to see a real consensus about what exactly should
>> happen when a branch is configured to be checked out (and if I
>> missed such a summary in this thread, please point me to it ;-).
> 
> I don't think we have a consensus yet.  A stand-alone outline of my
> current position is in my v3 RFC [5], but I don't have any buy-in yet
> ;).

>> And we should contrast that to the exact checkout and floating
>> branch use cases.
> 
> With my v3 series, there are no more detached HEADs.  Folks using
> checkout updates get a local master branch.  I do not change any of
> the exact checkout (superproject gitlinked sha1) vs. floating
> (subproject's remote submodule.<name>.branch via 'update --remote')
> logic, because that already works well.  The problem is the local
> branch handling, not the update/integration logic.

>> So what should happen on initial clone,
> 
> For 'add', clone the command line URL and create a new branch 'master'
> pointing at the commit referenced by the remote's HEAD (or other
> branch with --branch).
> 
> For 'update', do the same, except use a local-branch setting to
> determine the name for the local branch, falling back to 'master' if
> it is not set.

>> later updates,
> 
> The same thing that currently happens, with the exception that
> checkout-style updates should use reset to update the
> currently-checked out branch (or detached-HEAD), instead of always
> detaching the HEAD.

>> updates where the local and the remote branch diverged,
> 
> The same thing that currently happens.  For local (non --remote)
> updates, the integrated branch is the superproject's gitlinked sha1.
> For --remote updates, the integrated branch is the remote subproject's
> submodule.<name>.branch.  We integrate that with the
> currently-checked-out local branch (or detached HEAD) using the user's
> preferred submodule.<name>.update strategy.

>> when superproject branches are merged (with and without conflicts),
> 
> I don't think this currently does anything to the submodule itself,
> and that makes sense to me (use 'submodule update' or my 'submodule
> checkout' if you want such effects).  We should keep the current logic
> for updating the gitlinked $sha.  In the case that the
> .gitmodule-configured local-branches disagree, we should give the
> usual conflict warning (and <<<===>>> markup) and let the user resolve
> the conflict in the usual way.

>> on a rebase in the superproject and so on.
> 
> Same as the merge case.  Why would configuring a preferred
> local-branch that only affects checkout (and the initial clone) have
> anything to do with rebases and merges?

>> After that we can discuss about how to implement them (even though I
>> believe we won't need a new submodule command at the end of this
>> process, simply because if it isn't configurable we cannot teach git
>> checkout and friends to do that automatically for us later).
> 
> I think it is configurable, and I have an implementation that works
> (modulo bugs) [5].  I think we should teach 'git checkout
> --recurse-submodules' this logic, and then we can drop my 'git
> submodule checkout' entirely.

>> And from reading this discussion I believe we need another value for
>> the ignore option which only ignores changes to the SHA-1 but not to
>> work tree modifications of a submodule work tree relative to its HEAD
>> (or make that two: another one which ignores untracked files too and
>> only shows modification of tracked files). Otherwise users of the
>> floating or attached model can easily miss submodule modifications.
> 
> I am ignore-agnostic ;).  Do whatever you like here.

> Am 09.01.2014 18:32, schrieb W. Trevor King:
> >  However, the local-branch setting needs to be both
> > per-submodule and per-superproject-branch, so .git/config doesn't work
> > very well.  I think it's better to use something like my
> > .git/modules/<submodule-name>/config implementation [1] to set this
> > override.
> 
> Yes, the local branch should be set in the submodule's .git/config
> to make operations done inside the submodule work seamlessly.

> > This lack of per-superproject-branch overrides applies to all of the
> > submodule.<name>.* settings, but you're unlikely to want an
> > out-of-tree override for 'path' or a per-superproject-branch override
> > for 'url', 'ignore', 'update', or 'chRecurseSubmodules'.
> 
> Unlikely it is not ;-) We do have people who set update=none in
> the .git/config of the superproject for submodules they don't have
> access to (and which is not necessary for their work).

> > On the other hand, maybe an in-tree .gitmodules is good enough,
> > and folks who want a local override can just edit .gitmodules in
> > their local branch?  I've never felt the need to override
> > .gitmodules myself (for any setting), so feedback from someone who
> > has would be useful.
> 
> That way these changes would propagate to others working on the same
> branch when pushing, which I believe is a feature.

> >> It have the impression that attaching the head to the given
> >> branch for merge and rebase might be the sensible thing to do,
> >> but it would be great to hear from users of merge and rebase if
> >> that would break anything for them in their current use cases for
> >> these settings.
> > 
> > Which local branch would you attach to before merging?  I think
> > 'git submodule update' should always use the current submodule
> > state (attached branch or detached HEAD) [3], and we should have a
> > separate call that explicitly checked out the desired submodule
> > branch [4].
> 
> Like we currently do with "git submodule update --remote" (where you
> have to have an explicit command telling git when to advance the
> branch)? Having a separate call that does something *after* a git
> command is exactly the problem I'm trying to fix with recursive
> update, so I'm not terribly excited ;-)

> >>> If it's not the first clone, you should take no action (and your
> >>> original patch was ok about this).
> >>
> >> I'm not sure this is the right thing to do, after all you
> >> configured git to follow that branch so I'd expect it to be
> >> updated later too, no? Otherwise you might end up with an old
> >> version of your branch while upstream is a zillion commits
> >> ahead.
> > 
> > Non-clone updates should not change the submodule's *local* branch
> > *name*.  They should change the commit that that branch references,
> > otherwise 'git submodule update' would be a no-op ;).
> 
> Okay, I seem to have misunderstood that. But what happens when the
> branch setting in .gitmodules changes, shouldn't that be updated?

> >> First I'd like to see a real consensus about what exactly should
> >> happen when a branch is configured to be checked out (and if I
> >> missed such a summary in this thread, please point me to it ;-).
> > 
> > I don't think we have a consensus yet.  A stand-alone outline of my
> > current position is in my v3 RFC [5], but I don't have any buy-in yet
> > ;).
> 
> I'll volunteer to prepare a table explaining the different modes
> in my github wiki. Will scan this thread and your pointers for input
> and will come back soon when I have something ready.

> >> And we should contrast that to the exact checkout and floating
> >> branch use cases.
> > 
> > With my v3 series, there are no more detached HEADs.  Folks using
> > checkout updates get a local master branch.  I do not change any of
> > the exact checkout (superproject gitlinked sha1) vs. floating
> > (subproject's remote submodule.<name>.branch via 'update --remote')
> > logic, because that already works well.  The problem is the local
> > branch handling, not the update/integration logic.
> 
> Ok. Maybe we could use the "<remote>:<local>" notation to store both
> remote and local branch in a single setting?

> >> later updates,
> > 
> > The same thing that currently happens, with the exception that
> > checkout-style updates should use reset to update the
> > currently-checked out branch (or detached-HEAD), instead of always
> > detaching the HEAD.
> 
> Won't the user loose any modifications to his local branch here?

> >> updates where the local and the remote branch diverged,
> > 
> > The same thing that currently happens.  For local (non --remote)
> > updates, the integrated branch is the superproject's gitlinked sha1.
> > For --remote updates, the integrated branch is the remote subproject's
> > submodule.<name>.branch.  We integrate that with the
> > currently-checked-out local branch (or detached HEAD) using the user's
> > preferred submodule.<name>.update strategy.
> 
> And for checkout I can easily overwrite the upstream branch with
> my local changes?

> >> when superproject branches are merged (with and without conflicts),
> > 
> > I don't think this currently does anything to the submodule itself,
> > and that makes sense to me (use 'submodule update' or my 'submodule
> > checkout' if you want such effects).  We should keep the current logic
> > for updating the gitlinked $sha.  In the case that the
> > .gitmodule-configured local-branches disagree, we should give the
> > usual conflict warning (and <<<===>>> markup) and let the user resolve
> > the conflict in the usual way.
> 
> For me it makes lots of sense that in recursive checkout mode the
> merged submodules are already checked out (if possible) right after
> a superproject merge, making another "submodule update" unnecessary
> (the whole point of recursive update is to make "submodule update"
> obsolete, except for "--remote").

> On Thu, Jan 09, 2014 at 08:23:07PM +0100, Jens Lehmann wrote:
>> Am 09.01.2014 18:32, schrieb W. Trevor King:
>>>  However, the local-branch setting needs to be both
>>> per-submodule and per-superproject-branch, so .git/config doesn't work
>>> very well.  I think it's better to use something like my
>>> .git/modules/<submodule-name>/config implementation [1] to set this
>>> override.
>>
>> Yes, the local branch should be set in the submodule's .git/config
>> to make operations done inside the submodule work seamlessly.
> 
> Once you're inside the submodule my local-branch setting shouldn't
> matter, because it just connects superproject branches with submodule
> branches. The submodule's config is just a convenient out-of-tree
> place to store per-submodule overrides.

>>> This lack of per-superproject-branch overrides applies to all of the
>>> submodule.<name>.* settings, but you're unlikely to want an
>>> out-of-tree override for 'path' or a per-superproject-branch override
>>> for 'url', 'ignore', 'update', or 'chRecurseSubmodules'.
>>
>> Unlikely it is not ;-) We do have people who set update=none in
>> the .git/config of the superproject for submodules they don't have
>> access to (and which is not necessary for their work).
> 
> That is not a per-superproject-branch override.  local-branch is the
> only per-submodule config I can think of where I can imagine a sane
> person actually wanting an out-of-tree per-superproject-branch
> override.

>> And it isn't a "per-superproject-branch override" but a
>> "per-superproject-branch default" which can be overridden in
>> .git/config (except for 'update', but I intend to fix that).
> 
> You're talking about .gitmodules vs. .git/config here, but for
> local-branch, I'm talking about a fallback chain like [1]:
> 
> 1. superproject.<superproject-branch>.local-branch in the submodule's
>    config (superproject/.git/modules/≤submodule-name>/config).
> 2. submodule.<submodule-name>.local-branch in the superproject's
>    config (.git/config).
> 3. submodule.<submodule-name>.local-branch in the superproject's
>    .gitmodules file.
> 4. default to 'master'
> 
> Only #1 is a new idea.

>>> On the other hand, maybe an in-tree .gitmodules is good enough,
>>> and folks who want a local override can just edit .gitmodules in
>>> their local branch?  I've never felt the need to override
>>> .gitmodules myself (for any setting), so feedback from someone who
>>> has would be useful.
>>
>> That way these changes would propagate to others working on the same
>> branch when pushing, which I believe is a feature.
> 
> Sure.  Unless they don't want to propagate them, at which point they
> use an out-of-tree override masking the .gitmodules value.  The
> question is, would folks want local overrides for local-branch (like
> they do for submodule.<name>.update), or not?  Since it's easy to do
> [1], I don't see the point of *not* supporting per-superproject-branch
> overrides.

>>>> It have the impression that attaching the head to the given
>>>> branch for merge and rebase might be the sensible thing to do,
>>>> but it would be great to hear from users of merge and rebase if
>>>> that would break anything for them in their current use cases for
>>>> these settings.
>>>
>>> Which local branch would you attach to before merging?  I think
>>> 'git submodule update' should always use the current submodule
>>> state (attached branch or detached HEAD) [3], and we should have a
>>> separate call that explicitly checked out the desired submodule
>>> branch [4].
>>
>> Like we currently do with "git submodule update --remote" (where you
>> have to have an explicit command telling git when to advance the
>> branch)? Having a separate call that does something *after* a git
>> command is exactly the problem I'm trying to fix with recursive
>> update, so I'm not terribly excited ;-)
> 
> I'm all for rolling my 'git submodule checkout' into 'git checkout
> --recurse-submodules' [2].  It was just faster to mock up in shell
> while we decide how it should work.

>>>>> If it's not the first clone, you should take no action (and your
>>>>> original patch was ok about this).
>>>>
>>>> I'm not sure this is the right thing to do, after all you
>>>> configured git to follow that branch so I'd expect it to be
>>>> updated later too, no? Otherwise you might end up with an old
>>>> version of your branch while upstream is a zillion commits
>>>> ahead.
>>>
>>> Non-clone updates should not change the submodule's *local* branch
>>> *name*.  They should change the commit that that branch references,
>>> otherwise 'git submodule update' would be a no-op ;).
>>
>> Okay, I seem to have misunderstood that. But what happens when the
>> branch setting in .gitmodules changes, shouldn't that be updated?
> 
> Not by 'git submodule update'.  If there are no out-of-tree overrides
> and the user calls 'git submodule checkout' with a new local-branch in
> .gitmodules, *that* should checkout a new submodule branch.

>>>> First I'd like to see a real consensus about what exactly should
>>>> happen when a branch is configured to be checked out (and if I
>>>> missed such a summary in this thread, please point me to it ;-).
>>>
>>> I don't think we have a consensus yet.  A stand-alone outline of my
>>> current position is in my v3 RFC [5], but I don't have any buy-in yet
>>> ;).
>>
>> I'll volunteer to prepare a table explaining the different modes
>> in my github wiki. Will scan this thread and your pointers for input
>> and will come back soon when I have something ready.
> 
> Thanks :).

>>>> And we should contrast that to the exact checkout and floating
>>>> branch use cases.
>>>
>>> With my v3 series, there are no more detached HEADs.  Folks using
>>> checkout updates get a local master branch.  I do not change any of
>>> the exact checkout (superproject gitlinked sha1) vs. floating
>>> (subproject's remote submodule.<name>.branch via 'update --remote')
>>> logic, because that already works well.  The problem is the local
>>> branch handling, not the update/integration logic.
>>
>> Ok. Maybe we could use the "<remote>:<local>" notation to store both
>> remote and local branch in a single setting?
> 
> Meh :p.  I'm fine with (remote-)branch and local-branch as separate
> settings, or a single combined '<remote>:<local>' setting.  However, I
> think the local branch name is going to be more closely associated
> with the superproject branch than with the subproject's remote branch,
> and expect folks to want to override the local-branch on a
> per-superproject-branch basis much more often then they'll override
> the latter.

>>>> later updates,
>>>
>>> The same thing that currently happens, with the exception that
>>> checkout-style updates should use reset to update the
>>> currently-checked out branch (or detached-HEAD), instead of always
>>> detaching the HEAD.
>>
>> Won't the user loose any modifications to his local branch here?
> 
> They just called for a checkout-style update, so yes.  If they want to
> keep local modifications, chose an integration mode that preserves
> local changes.

>>>> updates where the local and the remote branch diverged,
>>>
>>> The same thing that currently happens.  For local (non --remote)
>>> updates, the integrated branch is the superproject's gitlinked sha1.
>>> For --remote updates, the integrated branch is the remote subproject's
>>> submodule.<name>.branch.  We integrate that with the
>>> currently-checked-out local branch (or detached HEAD) using the user's
>>> preferred submodule.<name>.update strategy.
>>
>> And for checkout I can easily overwrite the upstream branch with
>> my local changes?
> 
> ?  I don't understand.  How would you overwrite something in the
> upstream repository?

>>>> when superproject branches are merged (with and without conflicts),
>>>
>>> I don't think this currently does anything to the submodule itself,
>>> and that makes sense to me (use 'submodule update' or my 'submodule
>>> checkout' if you want such effects).  We should keep the current logic
>>> for updating the gitlinked $sha.  In the case that the
>>> .gitmodule-configured local-branches disagree, we should give the
>>> usual conflict warning (and <<<===>>> markup) and let the user resolve
>>> the conflict in the usual way.
>>
>> For me it makes lots of sense that in recursive checkout mode the
>> merged submodules are already checked out (if possible) right after
>> a superproject merge, making another "submodule update" unnecessary
>> (the whole point of recursive update is to make "submodule update"
>> obsolete, except for "--remote").
> 
> If you force the user to have the configured local-branch checked out
> before a non-checkout operations with checkout side-effects (as we
> currently do for other kinds of dirty trees), I think you'll avoid
> most (all?) of the branch-clobbering problems.

> Am 09.01.2014 20:55, schrieb W. Trevor King:
> > On Thu, Jan 09, 2014 at 08:23:07PM +0100, Jens Lehmann wrote:
> >> Am 09.01.2014 18:32, schrieb W. Trevor King:
> >>>  However, the local-branch setting needs to be both
> >>> per-submodule and per-superproject-branch, so .git/config doesn't work
> >>> very well.  I think it's better to use something like my
> >>> .git/modules/<submodule-name>/config implementation [1] to set this
> >>> override.
> >>
> >> Yes, the local branch should be set in the submodule's .git/config
> >> to make operations done inside the submodule work seamlessly.
> > 
> > Once you're inside the submodule my local-branch setting shouldn't
> > matter, because it just connects superproject branches with submodule
> > branches. The submodule's config is just a convenient out-of-tree
> > place to store per-submodule overrides.
> 
> Now I get it, you want to be able to override a submodule branch for
> every superproject branch. I'm not sure I'd add that in the first
> iteration though, as it seems to add quite some complexity and I'm
> not convinced yet users really need it (but I won't object when we
> find real world use cases for that).

> >> And it isn't a "per-superproject-branch override" but a
> >> "per-superproject-branch default" which can be overridden in
> >> .git/config (except for 'update', but I intend to fix that).
> > 
> > You're talking about .gitmodules vs. .git/config here, but for
> > local-branch, I'm talking about a fallback chain like [1]:
> > 
> > 1. superproject.<superproject-branch>.local-branch in the submodule's
> >    config (superproject/.git/modules/≤submodule-name>/config).
> > 2. submodule.<submodule-name>.local-branch in the superproject's
> >    config (.git/config).
> > 3. submodule.<submodule-name>.local-branch in the superproject's
> >    .gitmodules file.
> > 4. default to 'master'
> > 
> > Only #1 is a new idea.
> 
> Thanks for the explanation, now I understand what you're aiming at.

> >>> On the other hand, maybe an in-tree .gitmodules is good enough,
> >>> and folks who want a local override can just edit .gitmodules in
> >>> their local branch?  I've never felt the need to override
> >>> .gitmodules myself (for any setting), so feedback from someone
> >>> who has would be useful.
> >>
> >> That way these changes would propagate to others working on the
> >> same branch when pushing, which I believe is a feature.
> > 
> > Sure.  Unless they don't want to propagate them, at which point
> > they use an out-of-tree override masking the .gitmodules value.
> > The question is, would folks want local overrides for local-branch
> > (like they do for submodule.<name>.update), or not?  Since it's
> > easy to do [1], I don't see the point of *not* supporting
> > per-superproject-branch overrides.
> 
> Unless actual use cases are shown I'd vote for YAGNI here. A new
> config option means considerable maintenance burden, no matter how
> easy it is to implement in the first place.

> > I'm all for rolling my 'git submodule checkout' into 'git checkout
> > --recurse-submodules' [2].  It was just faster to mock up in shell
> > while we decide how it should work.
> 
> Sure. As I said that's perfectly fine for testing this approach,
> but we should do that right in "git checkout" and friends and not
> add yet another submodule command.

> >>>>> If it's not the first clone, you should take no action (and your
> >>>>> original patch was ok about this).
> >>>>
> >>>> I'm not sure this is the right thing to do, after all you
> >>>> configured git to follow that branch so I'd expect it to be
> >>>> updated later too, no? Otherwise you might end up with an old
> >>>> version of your branch while upstream is a zillion commits
> >>>> ahead.
> >>>
> >>> Non-clone updates should not change the submodule's *local* branch
> >>> *name*.  They should change the commit that that branch references,
> >>> otherwise 'git submodule update' would be a no-op ;).
> >>
> >> Okay, I seem to have misunderstood that. But what happens when the
> >> branch setting in .gitmodules changes, shouldn't that be updated?
> > 
> > Not by 'git submodule update'.  If there are no out-of-tree overrides
> > and the user calls 'git submodule checkout' with a new local-branch in
> > .gitmodules, *that* should checkout a new submodule branch.
> 
> Hmm, but isn't "submodule sync" the command that copies changed
> upstream config values (currently only the url) into the local config?
> Then a subsequent "submodule update" could do the actual checkout.

> >>>> later updates,
> >>>
> >>> The same thing that currently happens, with the exception that
> >>> checkout-style updates should use reset to update the
> >>> currently-checked out branch (or detached-HEAD), instead of
> >>> always detaching the HEAD.
> >>
> >> Won't the user loose any modifications to his local branch here?
> > 
> > They just called for a checkout-style update, so yes.  If they
> > want to keep local modifications, chose an integration mode that
> > preserves local changes.
> 
> Hmm, as current "submodule updates" already makes it too easy to
> loose commits, this does not look right to me. I'd prefer to stop at
> that point and tell the user what he can do to solve the conflict.

> > Maybe you meant "for checkout I can easily overwrite the local
> > changes with the upstream branch", which is what I understand
> > checkout to do.
> 
> But which I find really unfriendly and would not like to see in a
> new feature. We should protect the user from loosing any local
> changes, not simply throw them away. Recursive update makes sure it
> won't overwrite any local modification before it checks out anything
> and will abort before doing so (unless forced of course).

> >>>> when superproject branches are merged (with and without conflicts),
> >>>
> >>> I don't think this currently does anything to the submodule itself,
> >>> and that makes sense to me (use 'submodule update' or my 'submodule
> >>> checkout' if you want such effects).  We should keep the current logic
> >>> for updating the gitlinked $sha.  In the case that the
> >>> .gitmodule-configured local-branches disagree, we should give the
> >>> usual conflict warning (and <<<===>>> markup) and let the user resolve
> >>> the conflict in the usual way.
> >>
> >> For me it makes lots of sense that in recursive checkout mode the
> >> merged submodules are already checked out (if possible) right after
> >> a superproject merge, making another "submodule update" unnecessary
> >> (the whole point of recursive update is to make "submodule update"
> >> obsolete, except for "--remote").
> > 
> > If you force the user to have the configured local-branch checked out
> > before a non-checkout operations with checkout side-effects (as we
> > currently do for other kinds of dirty trees), I think you'll avoid
> > most (all?) of the branch-clobbering problems.
> 
> I'm thinking that a local branch works in two directions: It should
> make it easy to follow an upstream branch and also make changes to it
> (and publish those) if necessary.

> On Thu, Jan 09, 2014 at 10:40:52PM +0100, Jens Lehmann wrote:
> > Am 09.01.2014 20:55, schrieb W. Trevor King:
> > > On Thu, Jan 09, 2014 at 08:23:07PM +0100, Jens Lehmann wrote:
> > >> Am 09.01.2014 18:32, schrieb W. Trevor King:
> > >>>> later updates,
> > >>>
> > >>> The same thing that currently happens, with the exception that
> > >>> checkout-style updates should use reset to update the
> > >>> currently-checked out branch (or detached-HEAD), instead of
> > >>> always detaching the HEAD.
> > >>
> > >> Won't the user loose any modifications to his local branch here?
> > > 
> > > They just called for a checkout-style update, so yes.  If they
> > > want to keep local modifications, chose an integration mode that
> > > preserves local changes.
> > 
> > Hmm, as current "submodule updates" already makes it too easy to
> > loose commits, this does not look right to me. I'd prefer to stop at
> > that point and tell the user what he can do to solve the conflict.
> 
> Users who are worried about loosing local updates should not be using
> a checkout-style updates.  If they are using a checkout-style update,
> and they ask for an update, they're specifically requesting that we
> blow away their local work and checkout/reset to the new sha1.
> Solving update conflicts is the whole point of the non-checkout update
> modes.

> > > Maybe you meant "for checkout I can easily overwrite the local
> > > changes with the upstream branch", which is what I understand
> > > checkout to do.
> > 
> > But which I find really unfriendly and would not like to see in a
> > new feature. We should protect the user from loosing any local
> > changes, not simply throw them away. Recursive update makes sure it
> > won't overwrite any local modification before it checks out anything
> > and will abort before doing so (unless forced of course).
> 
> If you want to get rid of checkout-mode updates, I'm fine with that.
> However, I don't think it supports use-cases like Heiko's (implied) “I
> don't care what's happening upstream, I never touch that submodule,
> just checkout what the superproject maintainer says should be checked
> out for this branch.  Even if they have been rebasing or whatever”
> [3].

> > or be asked to resolve the conflict manually when "checkout" is
> > configured and the branches diverged.
> 
> I still think that checkout-mode updates should be destructive.  See
> my paraphrased-version of Heiko's use case above.  How are they going
> to resolve this manually?  Merge or rebase?  Why weren't they using
> that update mode in the first place?

> On Thu, Jan 09, 2014 at 02:18:40PM -0800, W. Trevor King wrote:
> > Users who are worried about loosing local updates should not be
> > using a checkout-style updates.  If they are using a
> > checkout-style update, and they ask for an update, they're
> > specifically requesting that we blow away their local work and
> > checkout/reset to the new sha1.  Solving update conflicts is the
> > whole point of the non-checkout update modes.
> 
> But checkout is different from reset --hard in that it does not blow
> away local uncommitted changes. Please see below.

> > On Thu, Jan 09, 2014 at 10:40:52PM +0100, Jens Lehmann wrote:
> > > Am 09.01.2014 20:55, schrieb W. Trevor King:
> > > > Maybe you meant "for checkout I can easily overwrite the local
> > > > changes with the upstream branch", which is what I understand
> > > > checkout to do.
> > > 
> > > But which I find really unfriendly and would not like to see in
> > > a new feature. We should protect the user from loosing any local
> > > changes, not simply throw them away. Recursive update makes sure
> > > it won't overwrite any local modification before it checks out
> > > anything and will abort before doing so (unless forced of
> > > course).
> > 
> > If you want to get rid of checkout-mode updates, I'm fine with
> > that.  However, I don't think it supports use-cases like Heiko's
> > (implied) “I don't care what's happening upstream, I never touch
> > that submodule, just checkout what the superproject maintainer
> > says should be checked out for this branch.  Even if they have
> > been rebasing or whatever” [3].
> 
> I never stated that in that post.

> The recursive checkout Jens is working on is simply implementing the
> current checkout-mode to the places where the superproject checks
> out its files. That way submodules get checked out when the
> superproject is checked out. If the submodule does not match the
> sha1 registered in the superproject it either stays there (if the
> checkout would not need to update the submodule) or (if checkout
> would need to update) git will not do the checkout and bail out with
> "you have local modifications to ... .

> > > or be asked to resolve the conflict manually when "checkout" is
> > > configured and the branches diverged.
> > 
> > I still think that checkout-mode updates should be destructive.
> > See my paraphrased-version of Heiko's use case above.  How are
> > they going to resolve this manually?  Merge or rebase?  Why
> > weren't they using that update mode in the first place?
> 
> I strongly disagree. They should only be destructive in the sense
> that another commit get checked out but never loose local
> modifications.

> > > On Thu, Jan 09, 2014 at 10:40:52PM +0100, Jens Lehmann wrote:
> > > > Am 09.01.2014 20:55, schrieb W. Trevor King:
> > > > > Maybe you meant "for checkout I can easily overwrite the local
> > > > > changes with the upstream branch", which is what I understand
> > > > > checkout to do.
> > > > 
> > > > But which I find really unfriendly and would not like to see in
> > > > a new feature. We should protect the user from loosing any local
> > > > changes, not simply throw them away. Recursive update makes sure
> > > > it won't overwrite any local modification before it checks out
> > > > anything and will abort before doing so (unless forced of
> > > > course).
> > > 
> > > If you want to get rid of checkout-mode updates, I'm fine with
> > > that.  However, I don't think it supports use-cases like Heiko's
> > > (implied) “I don't care what's happening upstream, I never touch
> > > that submodule, just checkout what the superproject maintainer
> > > says should be checked out for this branch.  Even if they have
> > > been rebasing or whatever” [3].
> > 
> > I never stated that in that post.
> 
> Sorry for misunderstanding.  I think I'm just unclear on your
> workflow?

> > The recursive checkout Jens is working on is simply implementing the
> > current checkout-mode to the places where the superproject checks
> > out its files. That way submodules get checked out when the
> > superproject is checked out. If the submodule does not match the
> > sha1 registered in the superproject it either stays there (if the
> > checkout would not need to update the submodule) or (if checkout
> > would need to update) git will not do the checkout and bail out with
> > "you have local modifications to ... .
> 
> Sounds good to me as far as it goes.  I think it misses the “what
> should we do if the gitlinked hashes are different” case, because the
> checkout will always leave you with a detached HEAD.
> 
> > > > or be asked to resolve the conflict manually when "checkout" is
> > > > configured and the branches diverged.
> > > 
> > > I still think that checkout-mode updates should be destructive.
> > > See my paraphrased-version of Heiko's use case above.  How are
> > > they going to resolve this manually?  Merge or rebase?  Why
> > > weren't they using that update mode in the first place?
> > 
> > I strongly disagree. They should only be destructive in the sense
> > that another commit get checked out but never loose local
> > modifications.
> 
> I think the key I'm missing is an example workflow where a developer
> wants to make local submodule changes, but also wants to use
> checkout-mode updates instead of merge/rebase updates.  Can you sketch
> one out for me?

> A typical workflow where a feature in a project needs some extension or
> change in a submodule goes like this:
> 
> 1. The developer does his changes locally implementing everything
>    needed. To commit he creates a local branch in the submodule and in
>    the superproject (most of the times from the current HEAD that is
>    checked out).
> 
> 2. For convenience I usually commit the resulting commit sha1 of the
>    submodule in the commit that needs the change. That way when I switch
>    to a different branch and back I can simply say: git submodule update
>    and get the correct code everywhere.

> How about the use-case I sketched above? Is that what you are searching
> for? In that use-case we have to update to the new master after a
> submodule change was merged. That could be achieved by
> 
> 	git submodule update --remote <submodule>
> 
> with the wanted stable branch configured. But in practise something
> along the lines of
> 
> 	(cd <submodule> && git checkout origin/<stable>)
> 
> is usually used and simple enough.

> On Tue, Jan 14, 2014 at 09:58:30PM +0100, Heiko Voigt wrote:
> > A typical workflow where a feature in a project needs some extension or
> > change in a submodule goes like this:
> > 
> > 1. The developer does his changes locally implementing everything
> >    needed. To commit he creates a local branch in the submodule and in
> >    the superproject (most of the times from the current HEAD that is
> >    checked out).
> > 
> > 2. For convenience I usually commit the resulting commit sha1 of the
> >    submodule in the commit that needs the change. That way when I switch
> >    to a different branch and back I can simply say: git submodule update
> >    and get the correct code everywhere.
> 
> This checkout functionality is exactly what my
> submodule.<name>.localBranch is designed to automate [1].  I think
> that should be different from integrating local and external changes,
> which is what 'git submodule update' is about.  For example, after you
> run 'git submodule update' here, you'll have your original commit
> checked out, but you'll be on a detached HEAD instead of your original
> branch.  If you want to further develop the submodule feature branch,
> you currently have to cd into the submodule and check the branch out
> by hand.

> > How about the use-case I sketched above? Is that what you are searching
> > for? In that use-case we have to update to the new master after a
> > submodule change was merged. That could be achieved by
> > 
> > 	git submodule update --remote <submodule>
> > 
> > with the wanted stable branch configured. But in practise something
> > along the lines of
> > 
> > 	(cd <submodule> && git checkout origin/<stable>)
> > 
> > is usually used and simple enough.
> 
> The “gitlinked commits must be in the subproject's master” rule
> protects you from blowing stuff away here.  You could use rebase- or
> merge-style integration as well, assuming the maintainer didn't have
> dirty local work in their submodule.

> > We have a tool in our git gui configuration that does
> > 
> > 	git submodule foreach 'git fetch && git checkout origin/master'
> 
> I agree that with 'submodule update' seems superfluous.  With proper
> out-of-tree submodule configs specifying remote URLs and upstream
> branches,
> 
>   git submodule foreach 'git fetch && git checkout @{upstream}'
> 
> (or merge/rebase/…) should cover this case more generically and with
> less mental overhead.
> 
> > I hope that draws a clear picture of how we use submodules.
> 
> It's certainly clearer, thanks :).  I'm not sure where checkout-mode
> is specifically important, though.  In your step-2, it doesn't restore
> your original branch.  In your “update the superproject's master”
> step, you aren't even using 'submodule update' :p.

> On Tue, Jan 14, 2014 at 01:42:09PM -0800, W. Trevor King wrote:
> > The “gitlinked commits must be in the subproject's master” rule
> > protects you from blowing stuff away here.  You could use rebase- or
> > merge-style integration as well, assuming the maintainer didn't have
> > dirty local work in their submodule.
> 
> No we can't. Developers are not allowed to merge in some submodules.
> The most central ones have maintainers and only they are allowed to
> merge into the stable branch. So we need to track exact commits on the
> stable branch, no local merge (except the fast-forward case of course)
> allowed. Thats why the developer does an exact checkout here.

> > > We have a tool in our git gui configuration that does
> > > 
> > > 	git submodule foreach 'git fetch && git checkout origin/master'
> > 
> > I agree that with 'submodule update' seems superfluous.  With proper
> > out-of-tree submodule configs specifying remote URLs and upstream
> > branches,
> > 
> >   git submodule foreach 'git fetch && git checkout @{upstream}'
> > 
> > (or merge/rebase/…) should cover this case more generically and with
> > less mental overhead.
> > 
> > > I hope that draws a clear picture of how we use submodules.
> > 
> > It's certainly clearer, thanks :).  I'm not sure where checkout-mode
> > is specifically important, though.  In your step-2, it doesn't restore
> > your original branch.  In your “update the superproject's master”
> > step, you aren't even using 'submodule update' :p.
> 
> Ah sorry I though that was clear. The "others" are using submodule update ;-)
> 
> I mean someone who gets stuff from a stable superproject branch by
> either rebasing their development branch or updating their local copy of
> a stable branch (e.g. master) by using
> 
> 	git checkout master
> 	git pull --ff --ff-only
> 	git submodule update --init --recursive
> 
> This also prevents the pure "updaters" from creating unnecessary merges.

> I would like to step back a bit and get back to the original problem
> at hand: Francescos original use case of an attached head for direct
> commits on a stable branch in a submodule. How about we finish
> discussing the exact solution of that first. AFAIK that is already
> solved with the following:
> 
>  * Trevor's first patch[2] to create a branch on initial clone of a submodule

> Also we (Jens and I) would first like to proceed with the recursive
> checkout / fetch (for which the plan is clear) as the next
> complicated step.
> 
> Once that is done and people gain some experience with it we can
> still extend further.

> On Tue, Jan 14, 2014 at 10:46:08PM +0100, Heiko Voigt wrote:
> > I would like to step back a bit and get back to the original problem
> > at hand: Francescos original use case of an attached head for direct
> > commits on a stable branch in a submodule. How about we finish
> > discussing the exact solution of that first. AFAIK that is already
> > solved with the following:
> > 
> >  * Trevor's first patch[2] to create a branch on initial clone of a submodule
> 
> v1 broke a bunch of tests.  Are you ok with v2 [1]?  v2 still needs a
> clearer commit message, a test, and a possible transition to
> triggering on non-checkout submodule.<name>.update instead of
> non-empty submodule.<name>.branch [2].

> > Lets not implement more than currently is needed. We can revisit the
> > ideas once some other real use-case manifests.
> 
> I have most of a real use case already.  I have a repository with
> submodules in one branch (master) and a subtree version in another
> (assembled) [4].  The *tree* is the same in each case, so I have to
> 'git rm -rf .'  to clear the submodules out of master before I can
> checkout assembled.
> 
>   $ git checkout assembled
>   error: The following untracked working tree files would be overwritten by checkout:
>           modular/README.md
>           modular/shell/README.md
>           …
>   $ git rm -rf .
>   $ git checkout assembled
> 
> That leaves some extra stuff removed:
> 
>   $ git status
>   On branch assembled
>   Changes to be committed:
>     (use "git reset HEAD <file>..." to unstage)
> 
>           deleted:    .gitignore
>           deleted:    .mailmap
>           deleted:    CONTRIBUTING.md
>           deleted:    LICENSE.md
>           deleted:    instructor.md
> 
> so I need to check that out by hand:
> 
>   $ git reset --hard HEAD
> 
> Now I can work in the assembled branch.  Going back to master is a bit
> less tedious:
> 
>   $ git checkout master
>   $ git submodule update --recursive
> 
> Luckily for me, I don't have a third superproject branch where the
> submodules are on a different, so the submodule's HEADs are preserved.
> As I understand it, the new recursive checkout functionality [5] would
> checkout my submodules with detached HEADs.  The fact that they are
> only accidentally preserved now is not comforting ;).

> > Also we (Jens and I) would first like to proceed with the recursive
> > checkout / fetch (for which the plan is clear) as the next
> > complicated step.
> > 
> > Once that is done and people gain some experience with it we can
> > still extend further.
> 
> This is quite reasonable.  Given the need for backwards compatibility,
> I just wanted to make sure my ideal UI was clear before we went
> forward.  There's no need to break fingers twice ;), but if tight
> binding with localBranch is too big a chunk to bite off now, I'm happy
> to kick that can down the road.

> [1]: http://article.gmane.org/gmane.comp.version-control.git/239967
> [2]: http://article.gmane.org/gmane.comp.version-control.git/239973
> [3]: http://article.gmane.org/gmane.comp.version-control.git/240139
> [4]: (gitweb) http://git.tremily.us/?p=swc-boot-camp.git
> [5]: http://article.gmane.org/gmane.comp.version-control.git/239695

> On Tue, Jan 14, 2014 at 02:22:31PM -0800, W. Trevor King wrote:
>> On Tue, Jan 14, 2014 at 10:46:08PM +0100, Heiko Voigt wrote:
>> > I would like to step back a bit and get back to the original problem
>> > at hand: Francescos original use case of an attached head for direct
>> > commits on a stable branch in a submodule. How about we finish
>> > discussing the exact solution of that first. AFAIK that is already
>> > solved with the following:
>> >
>> >  * Trevor's first patch[2] to create a branch on initial clone of a submodule
> [...]
>
>> > That should be all (and IIRC Francesco agreed) needed for that use-case.
>>
>> That was my understanding [3] ;).
>
>

> This avoids the current awkwardness of having either '' or 'checkout'
> for checkout-mode updates, which makes testing for checkout-mode
> updates (or non-checkout-mode updates) easier.
>
> Signed-off-by: W. Trevor King <wking@tremily.us>
> ---
>  git-submodule.sh | 27 +++++++++++----------------
>  1 file changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 5247f78..5e8776c 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -803,17 +803,10 @@ cmd_update()
>  			update_module=$update
>  		else
>  			update_module=$(git config submodule."$name".update)
> -			case "$update_module" in
> -			'')
> -				;; # Unset update mode
> -			checkout | rebase | merge | none)
> -				;; # Known update modes
> -			!*)
> -				;; # Custom update command
> -			*)
> -				die "$(eval_gettext "Invalid update mode '$update_module' for submodule '$name'")"
> -				;;
> -			esac
> +			if test -z "$update_module"
> +			then
> +				update_module="checkout"
> +			fi
>  		fi

>  		displaypath=$(relative_path "$prefix$sm_path")
> @@ -882,11 +875,16 @@ Maybe you want to use 'update --init'?")"
>  			case ";$cloned_modules;" in
>  			*";$name;"*)
>  				# then there is no local change to integrate
> -				update_module= ;;
> +				update_module=checkout ;;
>  			esac
>  
>  			must_die_on_failure=
>  			case "$update_module" in
> +			checkout)
> +				command="git checkout $subforce -q"
> +				die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$displaypath'")"
> +				say_msg="$(eval_gettext "Submodule path '\$displaypath': checked out '\$sha1'")"
> +				;;
>  ...
>  			*)
> -				command="git checkout $subforce -q"
> -				die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$displaypath'")"
> -				say_msg="$(eval_gettext "Submodule path '\$displaypath': checked out '\$sha1'")"
> -				;;
> +				die "$(eval_gettext "Invalid update mode '$update_module' for submodule '$name'")"
>  			esac

> "W. Trevor King" <wking@tremily.us> writes:
> > @@ -803,17 +803,10 @@ cmd_update()
> >  			update_module=$update
> >  		else
> >  			update_module=$(git config submodule."$name".update)
> > -			case "$update_module" in
> > -			'')
> > -				;; # Unset update mode
> > -			checkout | rebase | merge | none)
> > -				;; # Known update modes
> > -			!*)
> > -				;; # Custom update command
> > -			*)
> > -				die "$(eval_gettext "Invalid update mode '$update_module' for submodule '$name'")"
> > -				;;
> > -			esac
> > +			if test -z "$update_module"
> > +			then
> > +				update_module="checkout"
> > +			fi
> >  		fi
> 
> Is this a good change?
> 
> It removes the code to prevent a broken configuration value from
> slipping through.  The code used to stop early to give the user a
> chance to fix it before actually letting "submodule update" to go
> into the time consuming part, e.g. a call to module_clone, but that
> code is now lost.

> @@ -803,17 +803,10 @@ cmd_update()
>                         update_module=$update
>                 else
>                         update_module=$(git config submodule."$name".update)
> -                       case "$update_module" in
> -                       '')
> -                               ;; # Unset update mode
> -                       checkout | rebase | merge | none)
> -                               ;; # Known update modes
> -                       !*)
> -                               ;; # Custom update command
> -                       *)
> -                               die "$(eval_gettext "Invalid update mode '$update_module' for submodule '$name'")"
> -                               ;;
> -                       esac

> 2014/1/16 W. Trevor King <wking@tremily.us>:
> > Avoiding useless clones is probably more important than avoiding
> > duplicate "Invalid update mode" messages.
> 
> No, it's not duplicate code.

> 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"...}

> +	) || 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

> "W. Trevor King" <wking@tremily.us> writes:
> > @@ -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.

> > @@ -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.

> @@ -817,11 +831,15 @@ cmd_update()
>  
>  		displaypath=$(relative_path "$prefix$sm_path")
>  
> -		if test "$update_module" = "none"
> -		then
> +		case "$update_module" in
> +		none)
>  			echo "Skipping submodule '$displaypath'"
>  			continue
> -		fi
> +			;;
> +		checkout)
> +			local_branch=""
> +			;;
> +		esac

> "W. Trevor King" <wking@tremily.us> writes:
> 
> > @@ -817,11 +831,15 @@ cmd_update()
> >  
> >  		displaypath=$(relative_path "$prefix$sm_path")
> >  
> > -		if test "$update_module" = "none"
> > -		then
> > +		case "$update_module" in
> > +		none)
> >  			echo "Skipping submodule '$displaypath'"
> >  			continue
> > -		fi
> > +			;;
> > +		checkout)
> > +			local_branch=""
> > +			;;
> > +		esac
> 
> I wonder if there is a way to avoid detaching (and you may need to
> update the coddpath that resets the submodule to the commit
> specified by the superproject tree) when it is safe to do so.
>
> …
> 
> Perhaps that kind of "'git submodule update' is parallel to 'git
> pull' in the project without submodules" is better done with other
> update modes like --rebase or --merge.  If so, how should we explain
> what 'submodule update --checkout' is to the end users?  Is it
> supposed to be like "git fetch && git checkout origin/master"?

> To preserve the local branch, for situations where we're not on a
> detached HEAD.
>
> Signed-off-by: W. Trevor King <wking@tremily.us>
> ---

>  t/t7406-submodule-update.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
> index 0825a92..5aa9591 100755
> --- a/t/t7406-submodule-update.sh
> +++ b/t/t7406-submodule-update.sh
> @@ -703,7 +703,7 @@ test_expect_success 'submodule update places git-dir in superprojects git-dir re
>  	git clone super_update_r super_update_r2 &&
>  	(cd super_update_r2 &&
>  	 git submodule update --init --recursive >actual &&
> -	 test_i18ngrep "Submodule path .submodule/subsubmodule.: checked out" actual &&
> +	 test_i18ngrep "Submodule path .submodule/subsubmodule.: .git reset --hard -q" actual &&
>  	 (cd submodule/subsubmodule &&
>  	  git log > ../../expected
>  	 ) &&

> "W. Trevor King" <wking@tremily.us> writes:
> 
> > To preserve the local branch, for situations where we're not on a
> > detached HEAD.
> >
> > Signed-off-by: W. Trevor King <wking@tremily.us>
> > ---
> 
> This should be a part of some other change that actually changes how
> this "git submodule update" checks out the submodule, no?

> On Thu, Jan 16, 2014 at 11:22:52AM -0800, Junio C Hamano wrote:
>> "W. Trevor King" <wking@tremily.us> writes:
>> 
>> > To preserve the local branch, for situations where we're not on a
>> > detached HEAD.
>> >
>> > Signed-off-by: W. Trevor King <wking@tremily.us>
>> > ---
>> 
>> This should be a part of some other change that actually changes how
>> this "git submodule update" checks out the submodule, no?
>
> Sure, we can squash both this test fix and the subsequent new test
> patch into patch #3 in v5.  I was just splitting them out because
> backwards compatibility was a concern, and separate patches makes it
> easy for me to explain why the results changed here without getting
> lost in patch #3's implementation details.

> The old documentation did not distinguish between cloning and
> non-cloning updates and lacked clarity on which operations would lead
> to detached HEADs, and which would not.  The new documentation
> addresses these issues while updating the docs to reflect the changes
> introduced by this branch's explicit local branch creation in
> module_clone.
>
> I also add '--checkout' to the usage summary and group the update-mode
> options into a single set.
>
> Signed-off-by: W. Trevor King <wking@tremily.us>
> ---
>  Documentation/git-submodule.txt | 36 +++++++++++++++++++++++++++---------
>  Documentation/gitmodules.txt    |  4 ++++
>  2 files changed, 31 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index bfef8a0..02500b4 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -15,8 +15,8 @@ SYNOPSIS
>  'git submodule' [--quiet] init [--] [<path>...]
>  'git submodule' [--quiet] deinit [-f|--force] [--] <path>...
>  'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]
> -	      [-f|--force] [--rebase] [--reference <repository>] [--depth <depth>]
> -	      [--merge] [--recursive] [--] [<path>...]
> +	      [-f|--force] [--rebase|--merge|--checkout] [--reference <repository>]
> +	      [--depth <depth>] [--recursive] [--] [<path>...]
>  'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
>  	      [commit] [--] [<path>...]
>  'git submodule' [--quiet] foreach [--recursive] <command>
> @@ -155,13 +155,31 @@ it contains local modifications.
>  
>  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`. `none` can be overridden by specifying
> -	`--checkout`. Setting the key `submodule.$name.update` to `!command`
> -	will cause `command` to be run. `command` can be any arbitrary shell
> -	command that takes a single argument, namely the sha1 to update to.
> +	checkout the commit specified in the index of the containing
> +	repository.  The update mode defaults to 'checkout', but be
> +	configured with the 'submodule.<name>.update' setting or the
> +	'--rebase', '--merge', or 'checkout' options.

> ++
> +For updates that clone missing submodules, checkout-mode updates will
> +create submodules with detached HEADs; all other modes will create
> +submodules with a local branch named after 'submodule.<path>.branch'.
> ++
> +For updates that do not clone missing submodules, the submodule's HEAD

> +is only touched when the remote reference does not match the
> +submodule's HEAD (for none-mode updates, the submodule is never
> +touched).  The remote reference is usually the gitlinked commit from
> +the superproject's tree, but with '--remote' it is the upstream
> +subproject's 'submodule.<name>.branch'.  This remote reference is
> +integrated with the submodule's HEAD using the specified update mode.

> +For checkout-mode updates, that will result in a detached HEAD.  For
> +rebase- and merge-mode updates, the commit referenced by the
> +submodule's HEAD may change, but the symbolic reference will remain
> +unchanged (i.e. checked-out branches will still be checked-out
> +branches, and detached HEADs will still be detached HEADs).  If none
> +of the builtin modes fit your needs, set 'submodule.<name>.update' to
> +'!command' to configure a custom integration command.  'command' can
> +be any arbitrary shell command that takes a single argument, namely
> +the sha1 to update to.
>  +
>  If the submodule is not yet initialized, and you just want to use the
>  setting as stored in .gitmodules, you can automatically initialize the
> diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
> index f7be93f..36e5447 100644
> --- a/Documentation/gitmodules.txt
> +++ b/Documentation/gitmodules.txt
> @@ -53,6 +53,10 @@ submodule.<name>.branch::
>  	A remote branch name for tracking updates in the upstream submodule.
>  	If the option is not specified, it defaults to 'master'.  See the
>  	`--remote` documentation in linkgit:git-submodule[1] for details.
> ++
> +This branch name is also used for the local branch created by
> +non-checkout cloning updates.  See the 'update' documentation in
> +linkgit:git-submodule[1] for details.
>  
>  submodule.<name>.fetchRecurseSubmodules::
>  	This option can be used to control recursive fetching of this

> "W. Trevor King" <wking@tremily.us> writes:
> > @@ -155,13 +155,31 @@ it contains local modifications.
> >  
> >  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`. `none` can be overridden by specifying
> > -	`--checkout`. Setting the key `submodule.$name.update` to `!command`
> > -	will cause `command` to be run. `command` can be any arbitrary shell
> > -	command that takes a single argument, namely the sha1 to update to.
> > +	checkout the commit specified in the index of the containing
> > +	repository.  The update mode defaults to 'checkout', but be
> > +	configured with the 'submodule.<name>.update' setting or the
> > +	'--rebase', '--merge', or 'checkout' options.
> 
> Not '--checkout'?

> > ++
> > +For updates that clone missing submodules, checkout-mode updates will
> > +create submodules with detached HEADs; all other modes will create
> > +submodules with a local branch named after 'submodule.<path>.branch'.
> > ++
> > +For updates that do not clone missing submodules, the submodule's HEAD
> 
> That is, updates that update submodules that are already checked out?

> > +is only touched when the remote reference does not match the
> > +submodule's HEAD (for none-mode updates, the submodule is never
> > +touched).  The remote reference is usually the gitlinked commit from
> > +the superproject's tree, but with '--remote' it is the upstream
> > +subproject's 'submodule.<name>.branch'.  This remote reference is
> > +integrated with the submodule's HEAD using the specified update mode.
> 
> I think copying some motivation from the log message of 06b1abb5
> (submodule update: add --remote for submodule's upstream changes,
> 2012-12-19) would help the readers here.

> If I am reading 06b1abb5 correctly, the primary motivation behind
> "--remote" seems to be that it is exactly to help the person who
> wants to update superproject to satisify the "... are maintained
> properly" part by fetching the latest in each of the submodules in
> his superproject in preparation to 'git add .' them.  I still do not
> think "--remote" was a better way than the "foreach", but that is a
> separate topic.

> On Thu, Jan 16, 2014 at 12:21:04PM -0800, Junio C Hamano wrote:
> > "W. Trevor King" <wking@tremily.us> writes:
> > > @@ -155,13 +155,31 @@ it contains local modifications.
> > >  
> > >  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`. `none` can be overridden by specifying
> > > -	`--checkout`. Setting the key `submodule.$name.update` to `!command`
> > > -	will cause `command` to be run. `command` can be any arbitrary shell
> > > -	command that takes a single argument, namely the sha1 to update to.
> > > +	checkout the commit specified in the index of the containing
> > > +	repository.  The update mode defaults to 'checkout', but be
> > > +	configured with the 'submodule.<name>.update' setting or the
> > > +	'--rebase', '--merge', or 'checkout' options.
> > 
> > Not '--checkout'?
> 
> Oops, will fix in v5.

> > > ++
> > > +For updates that clone missing submodules, checkout-mode updates will
> > > +create submodules with detached HEADs; all other modes will create
> > > +submodules with a local branch named after 'submodule.<path>.branch'.
> > > ++
> > > +For updates that do not clone missing submodules, the submodule's HEAD
> > 
> > That is, updates that update submodules that are already checked out?
> 
> It's submodules for which $sm_path/.git does not exist.
> 
> > > +is only touched when the remote reference does not match the
> > > +submodule's HEAD (for none-mode updates, the submodule is never
> > > +touched).  The remote reference is usually the gitlinked commit from
> > > +the superproject's tree, but with '--remote' it is the upstream
> > > +subproject's 'submodule.<name>.branch'.  This remote reference is
> > > +integrated with the submodule's HEAD using the specified update mode.
> > 
> > I think copying some motivation from the log message of 06b1abb5
> > (submodule update: add --remote for submodule's upstream changes,
> > 2012-12-19) would help the readers here.
> 
> I think a brief reference to --remote is best here, mentioning that it
> has something to do with the upstream subproject.  More detail on when
> you should use --remote should probably go under the docs for --remote
> ;).
> 
> > A naïve expectation from a casual reader of the above would be "The
> > superproject's tree ought to point at the same commit as the tip of
> > the branch used in the submodule (modulo mirroring delays and
> > somesuch),
> 
> What is the branch used in the submodule?  The remote subproject's
> current submodule.<name>.branch?  The local submodule's
> submodule.<name>.branch (or localBranch) branch?  The submodule's
> current HEAD?
> 
> > if the repository of the superproject and submodules are maintained
> > properly", which would lead to "when would any sane person need to
> > use --remote in the first place???".
> 
> --remote is not for sane people (who will probably be pulling from
> withing the submodule itself).  --remote is for folks who track too
> many submodules to care about them as individuals, who just want to
> blindly update to whatever the upstream subproject maintainer has in
> submodule.<name>.branch.  For example, if you are a distribution with
> a hundred submodules tracking all the projects you package, and want
> to bump them all to a their current trunk tip in one fell swoop.
> 
> > If I am reading 06b1abb5 correctly, the primary motivation behind
> > "--remote" seems to be that it is exactly to help the person who
> > wants to update superproject to satisify the "... are maintained
> > properly" part by fetching the latest in each of the submodules in
> > his superproject in preparation to 'git add .' them.  I still do not
> > think "--remote" was a better way than the "foreach", but that is a
> > separate topic.
> 
> I agree now ;), the problems with:
> 
>   $ git submodule foreach 'git pull'
> 
> are:
> 
> 1. You may not be on the “right” local branch to begin with, and
> 2. You may not have out-of-tree submodule configs setting up the
>    “right” upstream for that branch.
> 
> 06b1abb did not address the former, and added a new .gitmodules-level
> submodule.<name>.branch to help with the latter.  I now think all of
> this would be easier if we had automatic submodule local-branch
> checkouts (fixing problem 1) and synchronized out-of-tree submodule
> configs (fixing problem 2).  Fixing problem 1 is all you need if you
> aren't interested in sharing your out-of-tree configs.
> 
> I think 06b1abb was inspired by “we already have a way to integrate
> changes in the gitlinked commit, we should reuse those to integrate
> other changes”.  In hindsight, changes in the gitlinked commit are
> really a checkout-time issue, while changes in other places (like the
> remote subproject) are pull-time issues.  Mixing them together was
> more confusing than it was worth.
> 
> Cheers,
> Trevor
> 
> -- 
> This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
> For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

> On Thu, Jan 16, 2014 at 12:55:21PM -0800, W. Trevor King wrote:
> > On Thu, Jan 16, 2014 at 12:21:04PM -0800, Junio C Hamano wrote:
> > > Not '--checkout'?
> > 
> > Oops, will fix in v5.
> 
> Shouldn't this also be `--checkout` (backticks), according to
> CodingGuidelines.  This applies to all this options in this patch I
> think.

>> A naïve expectation from a casual reader of the above would be "The
>> superproject's tree ought to point at the same commit as the tip of
>> the branch used in the submodule (modulo mirroring delays and
>> somesuch),
>
> What is the branch used in the submodule?  The remote subproject's
> current submodule.<name>.branch?  The local submodule's
> submodule.<name>.branch (or localBranch) branch?  The submodule's
> current HEAD?

> "W. Trevor King" <wking@tremily.us> writes:
> > On Thu, Jan 16, 2014 at 12:21:04PM -0800, Junio C Hamano wrote:
> >> "W. Trevor King" <wking@tremily.us> writes:
> >>> +is only touched when the remote reference does not match the
> >>> +submodule's HEAD (for none-mode updates, the submodule is never
> >>> +touched).  The remote reference is usually the gitlinked commit from
> >>> +the superproject's tree, but with '--remote' it is the upstream
> >>> +subproject's 'submodule.<name>.branch'.  This remote reference is
> >>> +integrated with the submodule's HEAD using the specified update mode.
> >> …
> >> A naïve expectation from a casual reader of the above would be
> >> "The superproject's tree ought to point at the same commit as the
> >> tip of the branch used in the submodule (modulo mirroring delays
> >> and somesuch),
> >
> > What is the branch used in the submodule?  The remote subproject's
> > current submodule.<name>.branch?  The local submodule's
> > submodule.<name>.branch (or localBranch) branch?  The submodule's
> > current HEAD?
>
> They are good questions that such casual readers would have, and
> giving answers to them in this part of the documentation would be a
> good way to give them a clear picture of how the command is designed
> to be used.

> This avoids the current awkwardness of having either '' or 'checkout'
> for checkout-mode updates, which makes testing for checkout-mode
> updates (or non-checkout-mode updates) easier.
>
> Signed-off-by: W. Trevor King <wking@tremily.us>
> ---
>  git-submodule.sh | 27 +++++++++++----------------
>  1 file changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 5247f78..5e8776c 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -803,17 +803,10 @@ cmd_update()
>                         update_module=$update
>                 else
>                         update_module=$(git config submodule."$name".update)
> -                       case "$update_module" in
> -                       '')
> -                               ;; # Unset update mode
> -                       checkout | rebase | merge | none)
> -                               ;; # Known update modes
> -                       !*)
> -                               ;; # Custom update command
> -                       *)
> -                               die "$(eval_gettext "Invalid update mode '$update_module' for submodule '$name'")"
> -                               ;;
> -                       esac
> +                       if test -z "$update_module"
> +                       then
> +                               update_module="checkout"

> +                       fi
>                 fi
>
>                 displaypath=$(relative_path "$prefix$sm_path")
> @@ -882,11 +875,16 @@ Maybe you want to use 'update --init'?")"
>                         case ";$cloned_modules;" in
>                         *";$name;"*)
>                                 # then there is no local change to integrate
> -                               update_module= ;;
> +                               update_module=checkout ;;

>                         esac
>
>                         must_die_on_failure=
>                         case "$update_module" in
> +                       checkout)
> +                               command="git checkout $subforce -q"
> +                               die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$displaypath'")"
> +                               say_msg="$(eval_gettext "Submodule path '\$displaypath': checked out '\$sha1'")"
> +                               ;;
>                         rebase)
>                                 command="git rebase"
>                                 die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$displaypath'")"
> @@ -906,10 +904,7 @@ Maybe you want to use 'update --init'?")"
>                                 must_die_on_failure=yes
>                                 ;;
>                         *)
> -                               command="git checkout $subforce -q"
> -                               die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$displaypath'")"
> -                               say_msg="$(eval_gettext "Submodule path '\$displaypath': checked out '\$sha1'")"
> -                               ;;
> +                               die "$(eval_gettext "Invalid update mode '$update_module' for submodule '$name'")"
>                         esac
>
>                         if (clear_local_git_env; cd "$sm_path" && $command "$sha1")
> --
> 1.8.5.2.8.g0f6c0d1

> On Sun, Jan 26, 2014 at 3:45 PM, W. Trevor King <wking@tremily.us> wrote:
> > +                               update_module="checkout"
> 
> Here, you (unnecessarily) quote 'checkout'...
> 
> > -                               update_module= ;;
> > +                               update_module=checkout ;;
> 
> But here you use bare (unquoted) 'checkout'. Bare is probably more
> idiomatic.

> I think copying some motivation from the log message of 06b1abb5
> (submodule update: add --remote for submodule's upstream changes,
> 2012-12-19) would help the readers here.  A naïve expectation from a
> casual reader of the above would be "The superproject's tree ought
> to point at the same commit as the tip of the branch used in the
> submodule (modulo mirroring delays and somesuch), if the repository
> of the superproject and submodules are maintained properly", which
> would lead to "when would any sane person need to use --remote in
> the first place???".

>> @@ -155,13 +155,31 @@ it contains local modifications.
>>
>>  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`. `none` can be overridden by specifying
>> - `--checkout`. Setting the key `submodule.$name.update` to 
>> `!command`
>> - will cause `command` to be run. `command` can be any arbitrary 
>> shell
>> - command that takes a single argument, namely the sha1 to update to.
>> + checkout the commit specified in the index of the containing
>> + repository.  The update mode defaults to 'checkout', but be

> From: "Junio C Hamano" <gitster@pobox.com>
> > "W. Trevor King" <wking@tremily.us> writes:
> >> + repository.  The update mode defaults to 'checkout', but be
> 
> nit:   s/but be/but can be/  ?

> On Wed, Jan 08, 2014 at 01:17:49AM +0100, Francesco Pretto wrote:
>> # Attach the submodule HEAD to <branch>.
>> # Also set ".git/config" 'submodule.<module>.branch' to <branch>
>> $ git submodule head -b <branch> --attach <module>
>
> I prefer submodule.<name>.local-branch for the submodule's local
> branch name.

> 2014/1/7 Junio C Hamano <gitster@pobox.com>:
>> It is not immediately obvious to me why anybody who specifies the
>> submodule.*.branch variable to say "I want _that_ branch" not to
>> want to be on that branch but in a detached state, so from that
>> perspective, submodule.*.attach feels superfluous.
>>
>
> Junio, for what it concerns me I fully support *this* patch as,

> On Sun, Jan 05, 2014 at 11:57:33PM +0100, Heiko Voigt wrote:
> > On Sun, Jan 05, 2014 at 01:24:58PM -0800, W. Trevor King wrote:
> > > If submodule.<name>.branch is set, it *always* creates a new local
> > > branch of that name pointing to the exact sha1.  If
> > > submodule.<name>.branch is not set, we still create a
> > > detached-HEAD checkout of the exact sha1.
> > 
> > Thanks for this clarification. Since the usual usage with --remote
> > is with a remote-tracking branch, I confused this here. I am not
> > sure whether blindly creating a local branch from the recorded sha1
> > is the right thing to do. In what situations would that be helpful?
> 
> In any situation where your going to develop the submodule locally,
> you're going to want a branch to develop in.  Starting local-submodule
> developers off on a branch seems useful, even if we can only use
> submodule.<name>.branch to guess at their preferred local branch name.
> Sometimes (often?) the guess will be right.  However, A detached HEAD
> will never be right for local development, so being right sometimes is
> still an improvement ;).

> > At $dayjob we usually use feature branches for our work. So if
> > someone wants to work in a submodule you simply create a branch at
> > the current sha1 which you then send out for review.
> 
> I'm all for named feature branches for development, and in this case
> submodule.<name>.branch is likely to be the wrong choice.  However,
> it's still safer to develop in that branch and then rename the branch
> to match your feature than it would be to develop your fix with a
> detached HEAD.  If your developers have enough discipline to always
> checkout their feature branch before starting development, my patch
> won't affect them.  However, I know a number of folks who go into
> fight-or-flight mode when they have a detached HEAD :p.

> > The reason why one would set a branch option here is to share the
> > superproject branch with colleagues. He can make sure they can
> > always fetch and checkout the submodule even though the branch there
> > is still under cleanup and thus will be rebased often. The commit
> > referenced by sha1 would not be available to a developer fetching
> > after a rebase.
> 
> Yeah, floating gitlinks are something else.  I'd be happy to have that
> functionality (gitlinks pointing to references) should be built into
> gitlinks themselves, not added as an additional layer in the submodule
> script.  This "gitlinked sha1 rebased out of existence" scenario is
> the first I've heard where I think gitlinked references would be
> useful.

> > > Thinking through this more, perhaps the logic should be:
> > > 
> > > * If submodule.<name>.update (defaulting to checkout) is checkout,
> > >   create a detached HEAD.
> > > * Otherwise, create a new branch submodule.<name>.branch
> > >   (defaulting to master).
> > 
> > Why not trigger the attached state with the submodule.<name>.branch
> > configuration option? If there is a local branch available use that,
> > if not the tracking branch (as it is currently). Then a developer
> > can start working on the branch with:
> > 
> > 	cd submodule; git checkout -t origin/<branchname>
> > 
> > assuming that submodule update learns some more support for this.
> 
> Isn't that already what 'git update --remote <submodule>' already
> does?

> > > > > -				update_module= ;;
> > > > > +				if test -n "$config_branch"; then
> > > > > +					update_module="!git reset --hard -q"
> > > > 
> > > > If we get here the checkout has already been done. Shouldn't
> > > > this rather specify a noop. I.E. like
> > > > 
> > > > 	update_module="!true"
> > > 
> > > We are on a local branch at this point, but not neccessarily
> > > pointing at the gitlinked sha1.  The reset here ensures that the
> > > new local branch does indeed point at the gitlinked sha1.
> > 
> > But isn't this a fresh clone? Why should the branch point at
> > anything else?
> 
> We don't pass $sha1 to module_clone().  Before my patch, we don't even
> pass $branch to module_clone().  That means that module_clone() will
> only checkout the gitlinked sha1 when the upstream HEAD (or $branch
> with my patch) happens to point to the gitlinked sha1.  For example,
> if Alice adds Charie's repo as a submodule (gitlinking his current
> master d2dbd39), then Charlie pushes a new commit d0de817 to his
> master, and then Bob clones Alice's superproject.  Post-clone,
> Charlie's submodule will have checked out Charlie's new d0de817, and
> we need update's additional:
> 
>   git reset --hard -q d2dbd39
> 
> to rewind to Alice's gitlinked sha1.

> On Sun, Jan 05, 2014 at 03:39:43PM -0800, W. Trevor King wrote:
> > On Sun, Jan 05, 2014 at 11:57:33PM +0100, Heiko Voigt wrote:
> > > On Sun, Jan 05, 2014 at 01:24:58PM -0800, W. Trevor King wrote:
> > > > Thinking through this more, perhaps the logic should be:
> > > > 
> > > > * If submodule.<name>.update (defaulting to checkout) is checkout,
> > > >   create a detached HEAD.
> > > > * Otherwise, create a new branch submodule.<name>.branch
> > > >   (defaulting to master).
> > > 
> > > Why not trigger the attached state with the
> > > submodule.<name>.branch configuration option? If there is a
> > > local branch available use that, if not the tracking branch (as
> > > it is currently). Then a developer can start working on the
> > > branch with:
> > > 
> > > 	cd submodule; git checkout -t origin/<branchname>
> > > 
> > > assuming that submodule update learns some more support for this.
> > 
> > Isn't that already what 'git update --remote <submodule>' already
> > does?
> 
> Does it? As far as I understood (not using the branch option yet) it
> only does
> 
> 	git checkout origin/<branchname>
> 
> so there is no local branch created that tracks the remote branch (-t).

> What I was thinking is that when submodule.<name>.branch is set a
> 
> 	git submodule update
> 
> will:
> 
> 1. if no local branch with that name exists:
> 
>    checkout the remote/<branch>
> 
> 2. If a local branch with that name exists:
> 
>    checkout the local branch and possibly advance it according to
>    its setting.

> submodule update will then
> 
> 1. if no local branch with that name exists:
> 
>    - automatically create the branch based on the referenced sha1
>    - set up that its tracking remote/<branch>

> > > > We are on a local branch at this point, but not neccessarily
> > > > pointing at the gitlinked sha1.  The reset here ensures that
> > > > the new local branch does indeed point at the gitlinked sha1.
> > > 
> > > But isn't this a fresh clone? Why should the branch point at
> > > anything else?
> > 
> > We don't pass $sha1 to module_clone().  Before my patch, we don't
> > even pass $branch to module_clone().  That means that
> > module_clone() will only checkout the gitlinked sha1 when the
> > upstream HEAD (or $branch with my patch) happens to point to the
> > gitlinked sha1.  For example, if Alice adds Charie's repo as a
> > submodule (gitlinking his current master d2dbd39), then Charlie
> > pushes a new commit d0de817 to his master, and then Bob clones
> > Alice's superproject.  Post-clone, Charlie's submodule will have
> > checked out Charlie's new d0de817, and we need update's
> > additional:
> > 
> >   git reset --hard -q d2dbd39
> > 
> > to rewind to Alice's gitlinked sha1.
> 
> Ah yeah, sorry I was confusing this with the checkout of
> remote/<branch> here again. Since I have done that twice already
> maybe we should be careful about not confusing users with this as
> well...
>
> After wrapping my head around the fact that you want to simply create a
> local branch on the referenced sha1 (and hopefully remembering it) I
> still would like to think a little more about it and let it settle a
> bit.

> On Sun, Jan 05, 2014 at 04:53:12AM +0100, Francesco Pretto wrote:
>> Also it could break some users that rely on the current behavior.
>
> The current code always has a detached HEAD after an initial-clone
> update, regardless of submodule.<name>.update, which doesn't match
> those docs either.

> Adding a check to only checkout
> submodule.<name>.branch if submodule.<name>.update was 'rebase',
> 'merge', or 'none' would be easy, but I don't think that makes much
> sense.  I can't see any reason for folks who specify
> submodule.<name>.branch to prefer a detached HEAD over a local branch
> matching the remote branch's name.

> If they prefer checkout updates,
> the first such update will return them to a detached HEAD.  If they
> prefer merge/rebase updates, future updates will keep them on the same
> branch.  All my commit does is setup that initial branch for folks who
> get the submodule via 'update', in the same way it's currently setup
> for folks who get the submodule via 'add'.
>

> 2014/1/5 W. Trevor King:
> > Adding a check to only checkout submodule.<name>.branch if
> > submodule.<name>.update was 'rebase', 'merge', or 'none' would be
> > easy, but I don't think that makes much sense.  I can't see any
> > reason for folks who specify submodule.<name>.branch to prefer a
> > detached HEAD over a local branch matching the remote branch's
> > name.
> 
> I think the reason is that it still matches the original use case of
> submodules devs:
> - the maintainer decides the specific commit developers should have;
> - developers checkout that commit and don't pull (you can't do "git
> pull" in a detached HEAD);
> - they optionally get the upstream commit from the specified
> "submodule.<name>.branch" with "--remote". They are still in a
> detached HEAD and can't do "git pull".
> 
> Maybe who coded submodules at first was thinking that the best way to
> contribute to a project is to checkout that repository, and not work
> in the submodule. As said, this works well when the submodule
> repository is a full project, and not a bunch of shared code.

> > If they prefer checkout updates, the first such update will return
> > them to a detached HEAD.  If they prefer merge/rebase updates,
> > future updates will keep them on the same branch.  All my commit
> > does is setup that initial branch for folks who get the submodule
> > via 'update', in the same way it's currently setup for folks who
> > get the submodule via 'add'.
> 
> My patch is in the same spirit but wants to obtain the same by being
> 100% additive and by not altering existing behavior in any way.

> Also it covers:
> - an "autoremote" behavior when the user wants an attached HEAD:
> with your patch "--remote" is still needed to really update the
> branch with "git submodule update":
> - voluntary reattach/detach the HEAD with command line;
> - ff-only merge of changes in the case of "checkout" in an attached
> HEAD (doing "git checkout <branch>" is not enough);
> - reattach of the HEAD with orphaned commits.

> On Sun, Jan 05, 2014 at 10:27:19PM +0100, Francesco Pretto wrote:
> > 2014/1/5 W. Trevor King:
> > Also it covers:
> > - an "autoremote" behavior when the user wants an attached HEAD:
> > with your patch "--remote" is still needed to really update the
> > branch with "git submodule update":
> > - voluntary reattach/detach the HEAD with command line;
> > - ff-only merge of changes in the case of "checkout" in an attached
> > HEAD (doing "git checkout <branch>" is not enough);
> > - reattach of the HEAD with orphaned commits.
> 
> Personally, I don't think autoremote updates are worth the additional
> UI complication (hence my alternative patch ;), but I'm open to
> discussion on this point.  Can you make a case for why and explicit
> --remote update is burdensome?
> 
> I'm also not entirely clear on the problems avoided or workflows
> enhanced via the last two entries.  Could you sketch an example
> workflow that makes that more obvious?

> 2014/1/5 W. Trevor King <wking@tremily.us>:
> > On Sun, Jan 05, 2014 at 04:53:12AM +0100, Francesco Pretto wrote:
> >> Also it could break some users that rely on the current behavior.
> >
> > The current code always has a detached HEAD after an initial-clone
> > update, regardless of submodule.<name>.update, which doesn't match
> > those docs either.
> 
> I perfectly agree with you that the documentation is a bit
> contradictory with regard to "update" command and detached HEAD.
> That's why it's so hard to add a feature and keep the same spirit of
> those that coded submodules at first. Also, I think that submodules
> didn't get much feedback with regards to these pitfalls because many
> people try to setup them, they see that "update" detaches the HEAD and
> they think "hmmm, maybe submodules are not what I was looking for".

> > Adding a check to only checkout
> > submodule.<name>.branch if submodule.<name>.update was 'rebase',
> > 'merge', or 'none' would be easy, but I don't think that makes much
> > sense.  I can't see any reason for folks who specify
> > submodule.<name>.branch to prefer a detached HEAD over a local branch
> > matching the remote branch's name.
> 
> I think the reason is that it still matches the original use case of
> submodules devs:
> - the maintainer decides the specific commit developers should have;

> On Sun, Jan 05, 2014 at 10:27:19PM +0100, Francesco Pretto wrote:
>> 2014/1/5 W. Trevor King <wking@tremily.us>:
>> > On Sun, Jan 05, 2014 at 04:53:12AM +0100, Francesco Pretto wrote:
>> >> Also it could break some users that rely on the current behavior.
>> >
>> > The current code always has a detached HEAD after an initial-clone
>> > update, regardless of submodule.<name>.update, which doesn't match
>> > those docs either.
>> 
>> I perfectly agree with you that the documentation is a bit
>> contradictory with regard to "update" command and detached HEAD.
>> That's why it's so hard to add a feature and keep the same spirit of
>> those that coded submodules at first. Also, I think that submodules
>> didn't get much feedback with regards to these pitfalls because many
>> people try to setup them, they see that "update" detaches the HEAD and
>> they think "hmmm, maybe submodules are not what I was looking for".
>
> I am not so sure about that. Why should detached HEAD make you think
> like that? For us at $dayjob we have a pre-commit hook that denies you
> to commit on a detached HEAD and asks you to create a branch first.

>> - developers checkout that commit and don't pull (you can't do "git
>> pull" in a detached HEAD);
>
> Exactly. We consider pull evil ;-) Seriously: To update we only do fast
> forward merges of local stable branches. 
> ...
> Yes, why would you do a git pull in a submodule? Don't you want to do
> something like
>
> 	git checkout -t -b dev/my-topic origin/master
>
> to start your development?

> Heiko Voigt <hvoigt@hvoigt.net> writes:
> > Yes, why would you do a git pull in a submodule? Don't you want to do
> > something like
> >
> > 	git checkout -t -b dev/my-topic origin/master
> >
> > to start your development?
> 
> As long as origin/master contains the commit specified by the
> superproject, that would work, but it may be a good thing to use a
> mode of submodule.*.update that does not have to drop the user into
> a detached state in the first place.  I somehow thought that was
> what rebase (or merge) was about, that is, starting from the state
> where a branch is checked out in the submodule working tree, an
> update in the superproject would cause that branch checked out in
> the submodule brought up to date with respect to the commit
> specified in the superproject tree.

>> And wouldn't it make it unnecessary to have a new "re-attach" option
>> if such a mode that never have to detach is used?
>
> I think so, but we currently don't have a "never detached" route for
> folks that are cloning submodules via update (instead of via
> 'submodule add').  Currently, new clone-updates will always leave you
> with a detached HEAD (unless you have branch-creation in your update
> !command).  My patch aims to close this detached-HEAD gap, for folks
> we expect will be doing local development, by creating an initial
> branch at clone-update time.

>
> As long as origin/master contains the commit specified by the
> superproject, that would work, but it may be a good thing to use a
> mode of submodule.*.update that does not have to drop the user into
> a detached state in the first place.  I somehow thought that was
> what rebase (or merge) was about, that is, starting from the state
> where a branch is checked out in the submodule working tree, an
> update in the superproject would cause that branch checked out in
> the submodule brought up to date with respect to the commit
> specified in the superproject tree.  If that is not how it is
> supposed to work, please correct me---and we may have to add another
> mode that does so (or even make rebase/merge do so as a bugfix).
>

> From: "W. Trevor King" <wking@tremily.us>
>
> 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.

> 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`.

>  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=

>     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.

> "W. Trevor King" <wking@tremily.us> writes:
> > 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?

> On Tue, Jan 07, 2014 at 10:15:25AM -0800, Junio C Hamano wrote:
>>     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.
>
> This looks reasonable to me, but there are still changes I'd like to
> make for a v3 (e.g. using submodule.<name>.update to trigger local
> branch checkout).  However, I'm currently leaning towards a new 'git
> submodule checkout' command with explicit preferred local submodule
> branches (see [1]).  Maybe this should all wait until Jens rolls out
> his update implementation [2]?

>> 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.
>
> It's used to shift the local branch reference from from the
> (arbitrary) cloned remote branch tip to the explicit submodule $sha1.

> "W. Trevor King" <wking@tremily.us> writes:
> > On Tue, Jan 07, 2014 at 10:15:25AM -0800, Junio C Hamano wrote:
> >> 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.
> >
> > It's used to shift the local branch reference from from the
> > (arbitrary) cloned remote branch tip to the explicit submodule $sha1.
> 
> The objective is not what I was questioning. In the patch I see
> 
> 	git checkout -f -q -B "$branch" "origin/$branch"
> 
> used in the module_clone (which I think makes sense), and then
> cmd_update uses "git reset --hard -q" to make sure that the working
> tree matches the commit $sha1 obtained from the superproject's
> gitlink (which may make $branch diverged from origin/$branch, but
> still I think it makes sense).
> 
> But there is no 'rebase' I can see in the codepath, which was what I
> was puzzled about.