Am 30.03.2011 20:50, schrieb Nicolas Morey-Chaisemartin: > On 03/30/2011 08:32 PM, Jens Lehmann wrote: >> Am 30.03.2011 09:56, schrieb Nicolas Morey-Chaisemartin: > >> 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? > > Actually, I don't think this work. > By doing that, if you run git submodule update without -f, it will set -f when you reached the first submodule not yet checked out ( -z $subsha1 ), > and the following submodules will be checkout using --force which may throw away changes the user wanted to keep. You are right, I just came to that conclusion myself ... but with a loop local variable initialized from force on every iteration it should work. > I know it is very intrusive. The main reason for that is I wanted the -f option to always behave the same (meaning throw away changes), > whether the submodule is already on the right commit or not. Hmm, I don't know if that is a good thing to do. People are used to "git submodule update" to only touch those submodule where the HEAD differs from the commit recorded in the superproject (And I often find myself using "-f" if the command didn't succeed without it). But when using "-f" touches other submodules than not using it the user might experience a rather unpleasant surprise, I'm not sure we want to go that way. > If we accept to drop this and only drop the changes when subsha1 != sha1, the patch can be much sorter by simply keeping the force flags I used and without modifying all the case/while thing. Yes. -- 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