Am 30.03.2011 09:56, schrieb Nicolas Morey-Chaisemartin: > By default git submodule update runs a simple checkout on submodules > that are not up-to-date. > If the submodules contains modified or untracked files, the command may > exit sanely with an error: > > $ git submodule update > error: Your local changes to the following files would be overwritten by > checkout: > file > Please, commit your changes or stash them before you can switch branches. > Aborting > Unable to checkout '1b69c6e55606b48d3284a3a9efe4b58bfb7e8c9e' in > submodule path 'test1' > > This implies that to reset a whole git submodule tree, a user has to run > first 'git submodule foreach --recursive git checkout -f' to then be > able to run git submodule update. > > This patch adds a --force option for the update command (only used for > submodules without --rebase or --merge options). It passes the --force > option to git checkout which will throw away the local changes. > Also when --force is specified, git checkout -f is always called on > submodules whether their HEAD matches the reference or not. I like where this is going (and it has tests too :-). Maybe we can simplify the patch and simplify one of the tests (see below) but apart from that I'm all for it. > Signed-off-by: Nicolas Morey-Chaisemartin <nicolas@xxxxxxxxxxxxxxxxxxxxxx> > --- > Documentation/git-submodule.txt | 5 ++- > git-submodule.sh | 68 ++++++++++++++++++++------------------ > t/t7406-submodule-update.sh | 23 +++++++++++++ > 3 files changed, 62 insertions(+), 34 deletions(-) > > diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt > index 3a5aa01..6482a84 100644 > --- a/Documentation/git-submodule.txt > +++ b/Documentation/git-submodule.txt > @@ -185,8 +185,9 @@ OPTIONS > > -f:: > --force:: > - This option is only valid for the add command. > - Allow adding an otherwise ignored submodule path. > + This option is only valid for add and update commands. > + When running add, allow adding an otherwise ignored submodule path. > + When running update, throw away local changes in submodules. > > --cached:: > This option is only valid for status and summary commands. These > diff --git a/git-submodule.sh b/git-submodule.sh > index 3a13397..a195879 100755 > --- a/git-submodule.sh > +++ b/git-submodule.sh > @@ -8,7 +8,7 @@ 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] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...] > + or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...] > or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...] > or: $dashless [--quiet] foreach [--recursive] <command> > or: $dashless [--quiet] sync [--] [<path>...]" > @@ -385,6 +385,9 @@ cmd_update() > -N|--no-fetch) > nofetch=1 > ;; > + -f | --force) > + force=$1 > + ;; > -r|--rebase) > update="rebase" > ;; All looking good up to here. But I wonder if the rest of git-submodule.sh could be changed a bit less invasive ... maybe as simple as this? @@ -458,7 +461,6 @@ cmd_update() if test "$subsha1" != "$sha1" then - force= if test -z "$subsha1" then force="-f" Now force will not be cleared and thus contain "-f" if the user provided it on the command line. All tests (including your new ones) are running fine with this simplification ... am I missing something? > @@ -430,6 +433,7 @@ cmd_update() > name=$(module_name "$path") || exit > url=$(git config submodule."$name".url) > update_module=$(git config submodule."$name".update) > + force_checkout= > if test -z "$url" > then > # Only mention uninitialized submodules when its > @@ -456,13 +460,38 @@ cmd_update() > update_module=$update > fi > > - if test "$subsha1" != "$sha1" > + if test -z "$subsha1" -a -z "$force" > + then > + force_checkout="-f" > + fi > + > + # Is this something we just cloned? > + case ";$cloned_modules;" in > + *";$name;"*) > + # then there is no local change to integrate > + update_module= ;; > + esac > + > + case "$update_module" in > + rebase) > + command="git rebase" > + action="rebase" > + msg="rebased onto" > + ;; > + merge) > + command="git merge" > + action="merge" > + msg="merged in" > + ;; > + *) > + command="git checkout $force $force_checkout -q" > + action="checkout" > + msg="checked out" > + ;; > + esac > + > + if test "$subsha1" != "$sha1" || test -n "$force" -a "$action" = "checkout" > then > - force= > - if test -z "$subsha1" > - then > - force="-f" > - fi > > if test -z "$nofetch" > then > @@ -471,31 +500,6 @@ cmd_update() > die "Unable to fetch in submodule path '$path'" > fi > > - # Is this something we just cloned? > - case ";$cloned_modules;" in > - *";$name;"*) > - # then there is no local change to integrate > - update_module= ;; > - esac > - > - case "$update_module" in > - rebase) > - command="git rebase" > - action="rebase" > - msg="rebased onto" > - ;; > - merge) > - command="git merge" > - action="merge" > - msg="merged in" > - ;; > - *) > - command="git checkout $force -q" > - action="checkout" > - msg="checked out" > - ;; > - esac > - > (clear_local_git_env; cd "$path" && $command "$sha1") || > die "Unable to $action '$sha1' in submodule path '$path'" > say "Submodule path '$path': $msg '$sha1'" And if I'm right all stuff up to here can go. > diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh > index fa9d23a..5d24d9f 100755 > --- a/t/t7406-submodule-update.sh > +++ b/t/t7406-submodule-update.sh > @@ -74,6 +74,29 @@ test_expect_success 'submodule update detaching the HEAD ' ' > ) > ' > > +test_expect_success 'submodule update should fail due to local changes' ' > + (cd super/submodule && > + git reset --hard HEAD~1 && > + echo "local change" > file > + ) && > + (cd super && > + (cd submodule && > + compare_head > + ) && > + test_must_fail git submodule update submodule > + ) > +' This test is shorter and might be easier to understand rewritten as: +test_expect_success 'submodule update should fail due to local changes' ' + (cd super && + (cd submodule && + git reset --hard HEAD~1 && + echo "local change" > file + compare_head + ) && + test_must_fail git submodule update submodule + ) +' > +test_expect_success 'submodule update should throw away changes with --force ' ' > + (cd super && > + (cd submodule && > + compare_head > + ) && > + git submodule update --force submodule && > + cd submodule && > + ! compare_head > + ) > +' > + > test_expect_success 'submodule update --rebase staying on master' ' > (cd super/submodule && > git checkout master -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html