Re: [PATCH v7 2/3] submodule update: add --remote for submodule's upstream changes

2012-12-12 Thread Phil Hord
Thanks for looking after this.


On Tue, Dec 11, 2012 at 1:58 PM, W. Trevor King wk...@tremily.us wrote:
 From: W. Trevor King wk...@tremily.us

 The current `update` command incorporates the superproject's gitlinked
 SHA-1 ($sha1) into the submodule HEAD ($subsha1).  Depending on the
 options you use, it may checkout $sha1, rebase the $subsha1 onto
 $sha1, or merge $sha1 into $subsha1.  This helps you keep up with
 changes in the upstream superproject.

 However, it's also useful to stay up to date with changes in the
 upstream subproject.  Previous workflows for incorporating such
 changes include the ungainly:

   $ git submodule foreach 'git checkout $(git config --file 
 $toplevel/.gitmodules submodule.$name.branch)  git pull'

 With this patch, all of the useful functionality for incorporating
 superproject changes can be reused to incorporate upstream subproject
 updates.  When you specify --remote, the target $sha1 is replaced with
 a $sha1 of the submodule's origin/master tracking branch.  If you want
 to merge a different tracking branch, you can configure the
 `submodule.name.branch` option in `.gitmodules`.  You can override
 the `.gitmodules` configuration setting for a particular superproject
 by configuring the option in that superproject's default configuration
 (using the usual configuration hierarchy, e.g. `.git/config`,
 `~/.gitconfig`, etc.).

 Previous use of submodule.name.branch
 ===

 Because we're adding a new configuration option, it's a good idea to
 check if anyone else is already using the option.  The foreach-pull
 example above was described by Ævar in

   commit f030c96d8643fa0a1a9b2bd9c2f36a77721fb61f
   Author: Ævar Arnfjörð Bjarmason ava...@gmail.com
   Date:   Fri May 21 16:10:10 2010 +

 git-submodule foreach: Add $toplevel variable

 Gerrit uses the same interpretation for the setting, but because
 Gerrit has direct access to the subproject repositories, it updates
 the superproject repositories automatically when a subproject changes.
 Gerrit also accepts the special value '.', which it expands into the
 superproject's branch name.

 Although the --remote functionality is using `submodule.name.branch`
 slightly differently, the effect is the same.  The foreach-pull
 example uses the option to record the name of the local branch to
 checkout before pulls.  The tracking branch to be pulled is recorded
 in `.git/modules/name/config`, which was initialized by the module
 clone during `submodule add` or `submodule init`.  Because the branch
 name stored in `submodule.name.branch` was likely the same as the
 branch name used during the initial `submodule add`, the same branch
 will be pulled in each workflow.

 Implementation details
 ==

 In order to ensure a current tracking branch state, `update --remote`
 fetches the submodule's remote repository before calculating the
 SHA-1.  However, I didn't change the logic guarding the existing fetch:

   if test -z $nofetch
   then
 # Run fetch only if $sha1 isn't present or it
 # is not reachable from a ref.
 (clear_local_git_env; cd $path 
   ( (rev=$(git rev-list -n 1 $sha1 --not --all 2/dev/null) 
test -z $rev) || git-fetch)) ||
 die $(eval_gettext Unable to fetch in submodule path '\$path')
   fi

 There will not be a double-fetch, because the new $sha1 determined
 after the `--remote` triggered fetch should always exist in the
 repository.  If it doesn't, it's because some racy process removed it
 from the submodule's repository and we *should* be re-fetching.

 Signed-off-by: W. Trevor King wk...@tremily.us
 ---
  Documentation/config.txt|  7 ++-
  Documentation/git-submodule.txt | 25 -
  Documentation/gitmodules.txt|  5 +
  git-submodule.sh| 22 +-
  t/t7406-submodule-update.sh | 31 +++
  5 files changed, 87 insertions(+), 3 deletions(-)

 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index 11f320b..6f4663c 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -1998,7 +1998,12 @@ submodule.name.update::
 for a submodule.  These variables are initially populated
 by 'git submodule init'; edit them to override the
 URL and other values found in the `.gitmodules` file.  See
 -   linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
 +
 +submodule.name.branch::
 +   The remote branch name for a submodule, used by `git submodule
 +   update --remote`.  Set this option to override the value found in
 +   the `.gitmodules` file.  See linkgit:git-submodule[1] and
 +   linkgit:gitmodules[5] for details.

  submodule.name.fetchRecurseSubmodules::
 This option can be used to control recursive fetching of this
 diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
 index b4683bb..72dd52f 100644
 --- 

Re: [PATCH v7 2/3] submodule update: add --remote for submodule's upstream changes

2012-12-12 Thread Junio C Hamano
Phil Hord phil.h...@gmail.com writes:

 +   if test -n $remote
 +   then
 +   if test -z $nofetch
 +   then
 +   # Fetch remote before determining tracking 
 $sha1
 +   (clear_local_git_env; cd $sm_path  
 git-fetch) ||

 You should 'git fetch $remote_name' here, and of course, initialize
 remote_name before this.  But how can we know the remote_name in the
 first place?  Is it safe to assume the submodule remote names will
 match those in the superproject?

 +   die $(eval_gettext Unable to fetch in 
 submodule path '\$sm_path')
 +   fi
 +   remote_name=$(get_default_remote)

 This get_default_remote finds the remote for the remote-tracking
 branch for HEAD in the superproject.  It is possible that HEAD !=
 $branch, so we have very few clues to go on here to get a more
 reasonable answer, so I do not have any good suggestions to improve
 this.

 One option would be to find the remote given for
 submodule.$branch.merge, but this would suppose there is some
 remote-tracking branch configured in the submodule, and that is not
 likely to be the case.

 +   sha1=$(clear_local_git_env; cd $sm_path 
 +   git rev-parse --verify 
 ${remote_name}/${branch}) ||

 This does assume the submodule remote names will match those in the
 superproject.  Is this safe?

All good points.  Thanks for reviewing.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 2/3] submodule update: add --remote for submodule's upstream changes

2012-12-12 Thread W. Trevor King
On Wed, Dec 12, 2012 at 12:43:23PM -0500, Phil Hord wrote:
 On Tue, Dec 11, 2012 at 1:58 PM, W. Trevor King wk...@tremily.us wrote:
  diff --git a/Documentation/git-submodule.txt 
  b/Documentation/git-submodule.txt
  …
  +--remote::
  [snip some --remote documentation]
  +In order to ensure a current tracking branch state, `update --remote`
  +fetches the submodule's remote repository before calculating the
  +SHA-1.  This makes `submodule update --remote --merge` similar to
  +running `git pull` in the submodule.  If you don't want to fetch (for
  +something closer to `git merge`), you should use `submodule update
  +--remote --no-fetch --merge`.
 
 I assume the same can be said for 'submodue update --remote --rebase',
 right?

Yes.

 I wonder if this can be made merge/rebase-agnostic.  Is it still
 true if I word it like this?:
 
In order to ensure a current tracking branch state, `update --remote`
fetches the submodule's remote repository before calculating the
SHA-1.  If you don't want to fetch, you should use `submodule update
 --remote --no-fetch`.

Works for me.  Will change in v8 (which I'll base on 'master').

  diff --git a/git-submodule.sh b/git-submodule.sh
  index f969f28..1395079 100755
  --- a/git-submodule.sh
  +++ b/git-submodule.sh
  @@ -8,7 +8,8 @@ dashless=$(basename $0 | sed -e 's/-/ /')
   USAGE=[--quiet] add [-b branch] [-f|--force] [--reference repository] 
  [--] repository [path]
  or: $dashless [--quiet] status [--cached] [--recursive] [--] [path...]
  or: $dashless [--quiet] init [--] [path...]
  -   or: $dashless [--quiet] update [--init] [-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] [--merge] [--recursive] 
  [--] [path...]
  +ges
 
 I think there's an unintentionally added line here with ges.

That is embarrassing :p.  Will fix in v8.

  +   if test -n $remote
  +   then
  +   if test -z $nofetch
  +   then
  +   # Fetch remote before determining tracking 
  $sha1
  +   (clear_local_git_env; cd $sm_path  
  git-fetch) ||
 
 You should 'git fetch $remote_name' here, and of course, initialize
 remote_name before this.  But how can we know the remote_name in the
 first place?  Is it safe to assume the submodule remote names will
 match those in the superproject?

The other git-fetch call from git-submodule.sh is also bare (i.e. no
specified remote).  When the remote needs to be specified, other
portions of git-submodule.sh use $(get_default_remote), which is (I
think) what the user should expect.  v6 of this series had a
configurable remote name, but Junio wasn't keen on the additional
configuration option.  I don't really mind either way.

 
  +   die $(eval_gettext Unable to fetch in 
  submodule path '\$sm_path')
  +   fi
  +   remote_name=$(get_default_remote)
 
 This get_default_remote finds the remote for the remote-tracking
 branch for HEAD in the superproject.  It is possible that HEAD !=
 $branch, so we have very few clues to go on here to get a more
 reasonable answer, so I do not have any good suggestions to improve
 this.

For detached HEADs, get_default_remote should fall back to 'origin',
which seems sane.  If the user wants a different default, they've
likely checkout out a branch in the submodule, setup that branch's
remote, and will be using --merge or --rebase.  If anyone expects
users who will be using detached heads to *want* to specify a
different remote than 'origin', that would be a good argument for
reinstating my submodule.name.remote patch from v6.

  +   sha1=$(clear_local_git_env; cd $sm_path 
  +   git rev-parse --verify 
  ${remote_name}/${branch}) ||
 
 This does assume the submodule remote names will match those in the
 superproject.  Is this safe?

Another good catch.  I should be calling get_default_remote after
cd-ing into the submodule.  Will change in v8.

Thanks for the feedback :)
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


signature.asc
Description: OpenPGP digital signature


[PATCH v7 2/3] submodule update: add --remote for submodule's upstream changes

2012-12-11 Thread W. Trevor King
From: W. Trevor King wk...@tremily.us

The current `update` command incorporates the superproject's gitlinked
SHA-1 ($sha1) into the submodule HEAD ($subsha1).  Depending on the
options you use, it may checkout $sha1, rebase the $subsha1 onto
$sha1, or merge $sha1 into $subsha1.  This helps you keep up with
changes in the upstream superproject.

However, it's also useful to stay up to date with changes in the
upstream subproject.  Previous workflows for incorporating such
changes include the ungainly:

  $ git submodule foreach 'git checkout $(git config --file 
$toplevel/.gitmodules submodule.$name.branch)  git pull'

With this patch, all of the useful functionality for incorporating
superproject changes can be reused to incorporate upstream subproject
updates.  When you specify --remote, the target $sha1 is replaced with
a $sha1 of the submodule's origin/master tracking branch.  If you want
to merge a different tracking branch, you can configure the
`submodule.name.branch` option in `.gitmodules`.  You can override
the `.gitmodules` configuration setting for a particular superproject
by configuring the option in that superproject's default configuration
(using the usual configuration hierarchy, e.g. `.git/config`,
`~/.gitconfig`, etc.).

Previous use of submodule.name.branch
===

Because we're adding a new configuration option, it's a good idea to
check if anyone else is already using the option.  The foreach-pull
example above was described by Ævar in

  commit f030c96d8643fa0a1a9b2bd9c2f36a77721fb61f
  Author: Ævar Arnfjörð Bjarmason ava...@gmail.com
  Date:   Fri May 21 16:10:10 2010 +

git-submodule foreach: Add $toplevel variable

Gerrit uses the same interpretation for the setting, but because
Gerrit has direct access to the subproject repositories, it updates
the superproject repositories automatically when a subproject changes.
Gerrit also accepts the special value '.', which it expands into the
superproject's branch name.

Although the --remote functionality is using `submodule.name.branch`
slightly differently, the effect is the same.  The foreach-pull
example uses the option to record the name of the local branch to
checkout before pulls.  The tracking branch to be pulled is recorded
in `.git/modules/name/config`, which was initialized by the module
clone during `submodule add` or `submodule init`.  Because the branch
name stored in `submodule.name.branch` was likely the same as the
branch name used during the initial `submodule add`, the same branch
will be pulled in each workflow.

Implementation details
==

In order to ensure a current tracking branch state, `update --remote`
fetches the submodule's remote repository before calculating the
SHA-1.  However, I didn't change the logic guarding the existing fetch:

  if test -z $nofetch
  then
# Run fetch only if $sha1 isn't present or it
# is not reachable from a ref.
(clear_local_git_env; cd $path 
  ( (rev=$(git rev-list -n 1 $sha1 --not --all 2/dev/null) 
   test -z $rev) || git-fetch)) ||
die $(eval_gettext Unable to fetch in submodule path '\$path')
  fi

There will not be a double-fetch, because the new $sha1 determined
after the `--remote` triggered fetch should always exist in the
repository.  If it doesn't, it's because some racy process removed it
from the submodule's repository and we *should* be re-fetching.

Signed-off-by: W. Trevor King wk...@tremily.us
---
 Documentation/config.txt|  7 ++-
 Documentation/git-submodule.txt | 25 -
 Documentation/gitmodules.txt|  5 +
 git-submodule.sh| 22 +-
 t/t7406-submodule-update.sh | 31 +++
 5 files changed, 87 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 11f320b..6f4663c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1998,7 +1998,12 @@ submodule.name.update::
for a submodule.  These variables are initially populated
by 'git submodule init'; edit them to override the
URL and other values found in the `.gitmodules` file.  See
-   linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
+
+submodule.name.branch::
+   The remote branch name for a submodule, used by `git submodule
+   update --remote`.  Set this option to override the value found in
+   the `.gitmodules` file.  See linkgit:git-submodule[1] and
+   linkgit:gitmodules[5] for details.
 
 submodule.name.fetchRecurseSubmodules::
This option can be used to control recursive fetching of this
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index b4683bb..72dd52f 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -13,7 +13,7 @@ SYNOPSIS
  [--reference repository] [--] repository [path]
 'git submodule' [--quiet] status