[PATCH 12/15] unpack-trees: check if we can perform the operation for submodules

2017-02-23 Thread Stefan Beller 
Signed-off-by: Stefan Beller 
---
 unpack-trees.c | 131 +
 unpack-trees.h |   1 +
 2 files changed, 123 insertions(+), 9 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index 616a0ae4b2..8333da2cc9 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -10,6 +10,8 @@
 #include "attr.h"
 #include "split-index.h"
 #include "dir.h"
+#include "submodule.h"
+#include "submodule-config.h"
 
 /*
  * Error messages expected by scripts out of plumbing commands such as
@@ -45,6 +47,9 @@ static const char 
*unpack_plumbing_errors[NB_UNPACK_TREES_ERROR_TYPES] = {
 
/* ERROR_WOULD_LOSE_ORPHANED_REMOVED */
"Working tree file '%s' would be removed by sparse checkout update.",
+
+   /* ERROR_WOULD_LOSE_SUBMODULE */
+   "Submodule '%s' cannot checkout new HEAD.",
 };
 
 #define ERRORMSG(o,type) \
@@ -161,6 +166,8 @@ void setup_unpack_trees_porcelain(struct 
unpack_trees_options *opts,
_("The following working tree files would be overwritten by 
sparse checkout update:\n%s");
msgs[ERROR_WOULD_LOSE_ORPHANED_REMOVED] =
_("The following working tree files would be removed by sparse 
checkout update:\n%s");
+   msgs[ERROR_WOULD_LOSE_SUBMODULE] =
+   _("Submodule '%s' cannot checkout new HEAD");
 
opts->show_all_errors = 1;
/* rejected paths may not have a static buffer */
@@ -240,12 +247,75 @@ static void display_error_msgs(struct 
unpack_trees_options *o)
fprintf(stderr, _("Aborting\n"));
 }
 
+static int check_submodule_move_head(const struct cache_entry *ce,
+const char *old_id,
+const char *new_id,
+struct unpack_trees_options *o)
+{
+   const struct submodule *sub = submodule_from_ce(ce);
+   if (!sub)
+   return 0;
+
+   switch (sub->update_strategy.type) {
+   case SM_UPDATE_UNSPECIFIED:
+   case SM_UPDATE_CHECKOUT:
+   if (submodule_move_head(ce->name, old_id, new_id, 
SUBMODULE_MOVE_HEAD_DRY_RUN))
+   return o->gently ? -1 :
+   add_rejected_path(o, 
ERROR_WOULD_LOSE_SUBMODULE, ce->name);
+   return 0;
+   case SM_UPDATE_NONE:
+   return 0;
+   case SM_UPDATE_REBASE:
+   case SM_UPDATE_MERGE:
+   case SM_UPDATE_COMMAND:
+   default:
+   warning(_("submodule update strategy not supported for 
submodule '%s'"), ce->name);
+   return -1;
+   }
+}
+
+static void reload_gitmodules_file(struct index_state *index,
+  struct checkout *state)
+{
+   int i;
+   for (i = 0; i < index->cache_nr; i++) {
+   struct cache_entry *ce = index->cache[i];
+   if (ce->ce_flags & CE_UPDATE) {
+   int r = strcmp(ce->name, ".gitmodules");
+   if (r < 0)
+   continue;
+   else if (r == 0) {
+   submodule_free();
+   checkout_entry(ce, state, NULL);
+   gitmodules_config();
+   git_config(submodule_config, NULL);
+   } else
+   break;
+   }
+   }
+}
+
 /*
  * Unlink the last component and schedule the leading directories for
  * removal, such that empty directories get removed.
  */
 static void unlink_entry(const struct cache_entry *ce)
 {
+   const struct submodule *sub = submodule_from_ce(ce);
+   if (sub) {
+   switch (sub->update_strategy.type) {
+   case SM_UPDATE_UNSPECIFIED:
+   case SM_UPDATE_CHECKOUT:
+   case SM_UPDATE_REBASE:
+   case SM_UPDATE_MERGE:
+   submodule_move_head(ce->name, "HEAD", NULL,
+   SUBMODULE_MOVE_HEAD_FORCE);
+   break;
+   case SM_UPDATE_NONE:
+   case SM_UPDATE_COMMAND:
+   return; /* Do not touch the submodule. */
+   }
+   }
if (!check_leading_path(ce->name, ce_namelen(ce)))
return;
if (remove_or_warn(ce->ce_mode, ce->name))
@@ -301,6 +371,9 @@ static int check_updates(struct unpack_trees_options *o)
remove_marked_cache_entries(index);
remove_scheduled_dirs();
 
+   if (should_update_submodules() && o->update && !o->dry_run)
+   reload_gitmodules_file(index, );
+
for (i = 0; i < index->cache_nr; i++) {
struct cache_entry *ce = index->cache[i];
 
@@ -1358,17 +1431,26 @@ static int verify_uptodate_1(const struct cache_entry 
*ce,
if (!lstat(ce->name, )) {
int flags =

Re: [PATCH 12/15] unpack-trees: check if we can perform the operation for submodules

2017-02-21 Thread Jacob Keller 
On Tue, Feb 21, 2017 at 3:44 PM, Stefan Beller  wrote:
> On Tue, Feb 21, 2017 at 3:35 PM, Jacob Keller  wrote:
>> On Tue, Feb 21, 2017 at 2:16 PM, Stefan Beller  wrote:
>>> On Fri, Feb 17, 2017 at 10:42 AM, Jacob Keller  
>>> wrote:
 On Wed, Feb 15, 2017 at 4:38 PM, Stefan Beller  wrote:
> +   if (is_active_submodule_with_strategy(ce, SM_UPDATE_UNSPECIFIED))

 Here, and in other cases where we use
 is_active_submodule_with_strategy(), why do we only ever check
 SM_UPDATE_UNSPECIFIED? It seems really weird that we're only going to
 check submodules who's strategy is unspecified, when that defaults to
 checkout if I recall correctly? Shouldn't we check both? This applies
 to pretty much everywhere that you call this function that I noticed,
 which is why I removed the context.
>>>
>>> I am torn between this.
>>>
>>> submodule..update = {rebase, merge, checkout, none !command}
>>> is currently documented in GIT-CONFIG(1) as
>>>
>>>submodule..update
>>>The default update procedure for a submodule. This variable is
>>>populated by git submodule init from the gitmodules(5) file. See
>>>description of update command in git-submodule(1).
>>>
>>> and in GIT-SUBMODULE(1) as
>>>
>>>update
>>>[...] can be done in several ways
>>>depending on command line options and the value of
>>>submodule..update configuration variable. Supported update
>>>procedures are:
>>>
>>>checkout
>>>[...] or no option is given, and
>>>submodule..update is unset, or if it is set to 
>>> checkout.
>>>
>>> So the "update" config clearly only applies to the "submodule update"
>>> command, right?
>>>
>>> Well no, "checkout --recurse-submodules" is very similar
>>> to running "submodule update", except with a bit more checks, so you could
>>> think that such an option applies to checkout as well. (and eventually
>>> rebase/merge etc. are supported as well.)
>>>
>>> So initially I assumed both "unspecified" as well as "checkout"
>>> are good matches to support in the first round.
>>>
>>> Then I flip flopped to think that we should not interfere with these
>>> settings at all (The checkout command does checkout and checkout only;
>>> no implicit rebase/merge ever in the future, because that would be
>>> confusing). So ignoring that option seemed like the way to go.
>>
>> Hmm. So it's a bit complicated.
>>
>>>
>>> But ignoring that option is also not the right approach.
>>> What if you have set it to "none" and really *expect* Git to not touch
>>> that submodule?
>>
>> Or set it to "rebase" and suddenly git-checkout is ignoring you and
>> just checking things out anyways.
>>
>>>
>>> So I dunno. Maybe it is a documentation issue, we need to spell out
>>> in the man page for checkout that --recurse-submodules is
>>> following one of these models. Now which is the best default model here?
>>
>> Personally, I would go with that the config option sets the general
>> strategy used by the submodule whenever its updated, regardless of
>> how.
>>
>> So, for example, setting it to none, means that recurse-submoduls will
>> ignore it when checking out. Setting it to rebase, or merge, and the
>> checkout will try to do those things?
>
> That is generally a sound idea when it comes to git-checkout.
>
> What about other future things like git-revert?
> (Ok I already brought up this example too many times; it should have
> a revert-submodules as well switch, which is neither of the current 
> strategies,
> so we'd have to invent a new strategy and make that the default for
> revert. That strategy would make no sense in any other command though)
>

This is where things get tricky, IMHO. The problem is that the
strategy now wants to encompass more things.

> What about "git-rebase --recurse-submodules"?
> Should git-rebase merge the submodules when it is configured to "merge"
> Or just "checkout" (the possibly non-fast-forward-y old sha1) ?
>
> The only sane option IMO is "rebase" as well in the submodules, rewriting
> the submodule pointers in the rebased commits in the superproject.
>

I'm not even really sure what rebase should do here at all. I assume
by this you mean "what would a git-rebase that also rebased submodules
do"

Ofcourse the sane answer might be something like "uhh you have to
decide that for yourself manually" I think this is a really complex
problem to solve, and in this case I do not think rebase should even
rely on the strategy. a "recurse-submodules rebase" would do something
like:

rebase parent as normal, but if a commit changes the submodule, then
it needs to re-create that submodule change using its own rebase
inside  the submodule based on the (new) parent from the parent
projects history change, and then commit that as the committed change?

But

Re: [PATCH 12/15] unpack-trees: check if we can perform the operation for submodules

2017-02-21 Thread Stefan Beller 
On Tue, Feb 21, 2017 at 3:35 PM, Jacob Keller  wrote:
> On Tue, Feb 21, 2017 at 2:16 PM, Stefan Beller  wrote:
>> On Fri, Feb 17, 2017 at 10:42 AM, Jacob Keller  
>> wrote:
>>> On Wed, Feb 15, 2017 at 4:38 PM, Stefan Beller  wrote:
 +   if (is_active_submodule_with_strategy(ce, SM_UPDATE_UNSPECIFIED))
>>>
>>> Here, and in other cases where we use
>>> is_active_submodule_with_strategy(), why do we only ever check
>>> SM_UPDATE_UNSPECIFIED? It seems really weird that we're only going to
>>> check submodules who's strategy is unspecified, when that defaults to
>>> checkout if I recall correctly? Shouldn't we check both? This applies
>>> to pretty much everywhere that you call this function that I noticed,
>>> which is why I removed the context.
>>
>> I am torn between this.
>>
>> submodule..update = {rebase, merge, checkout, none !command}
>> is currently documented in GIT-CONFIG(1) as
>>
>>submodule..update
>>The default update procedure for a submodule. This variable is
>>populated by git submodule init from the gitmodules(5) file. See
>>description of update command in git-submodule(1).
>>
>> and in GIT-SUBMODULE(1) as
>>
>>update
>>[...] can be done in several ways
>>depending on command line options and the value of
>>submodule..update configuration variable. Supported update
>>procedures are:
>>
>>checkout
>>[...] or no option is given, and
>>submodule..update is unset, or if it is set to checkout.
>>
>> So the "update" config clearly only applies to the "submodule update"
>> command, right?
>>
>> Well no, "checkout --recurse-submodules" is very similar
>> to running "submodule update", except with a bit more checks, so you could
>> think that such an option applies to checkout as well. (and eventually
>> rebase/merge etc. are supported as well.)
>>
>> So initially I assumed both "unspecified" as well as "checkout"
>> are good matches to support in the first round.
>>
>> Then I flip flopped to think that we should not interfere with these
>> settings at all (The checkout command does checkout and checkout only;
>> no implicit rebase/merge ever in the future, because that would be
>> confusing). So ignoring that option seemed like the way to go.
>
> Hmm. So it's a bit complicated.
>
>>
>> But ignoring that option is also not the right approach.
>> What if you have set it to "none" and really *expect* Git to not touch
>> that submodule?
>
> Or set it to "rebase" and suddenly git-checkout is ignoring you and
> just checking things out anyways.
>
>>
>> So I dunno. Maybe it is a documentation issue, we need to spell out
>> in the man page for checkout that --recurse-submodules is
>> following one of these models. Now which is the best default model here?
>
> Personally, I would go with that the config option sets the general
> strategy used by the submodule whenever its updated, regardless of
> how.
>
> So, for example, setting it to none, means that recurse-submoduls will
> ignore it when checking out. Setting it to rebase, or merge, and the
> checkout will try to do those things?

That is generally a sound idea when it comes to git-checkout.

What about other future things like git-revert?
(Ok I already brought up this example too many times; it should have
a revert-submodules as well switch, which is neither of the current strategies,
so we'd have to invent a new strategy and make that the default for
revert. That strategy would make no sense in any other command though)

What about "git-rebase --recurse-submodules"?
Should git-rebase merge the submodules when it is configured to "merge"
Or just "checkout" (the possibly non-fast-forward-y old sha1) ?

The only sane option IMO is "rebase" as well in the submodules, rewriting
the submodule pointers in the rebased commits in the superproject.

>
> Or, if that's not really feasible, have the checkout go "hey.. you
> asked me to recurse, but uhhh these submodules don't allow me to do
> checkout, so I'm gonna fail"? I think that's the best approach for
> now.

So you'd propose to generally use the submodule..update
strategies with aggressive error-out but also keeping in mind
that the strategies might grow by a lot in the future (well only revert
comes to mind here).

ok, let's do that then.

Thanks,
Stefan

Re: [PATCH 12/15] unpack-trees: check if we can perform the operation for submodules

2017-02-21 Thread Jacob Keller 
On Tue, Feb 21, 2017 at 2:16 PM, Stefan Beller  wrote:
> On Fri, Feb 17, 2017 at 10:42 AM, Jacob Keller  wrote:
>> On Wed, Feb 15, 2017 at 4:38 PM, Stefan Beller  wrote:
>>> +   if (is_active_submodule_with_strategy(ce, SM_UPDATE_UNSPECIFIED))
>>
>> Here, and in other cases where we use
>> is_active_submodule_with_strategy(), why do we only ever check
>> SM_UPDATE_UNSPECIFIED? It seems really weird that we're only going to
>> check submodules who's strategy is unspecified, when that defaults to
>> checkout if I recall correctly? Shouldn't we check both? This applies
>> to pretty much everywhere that you call this function that I noticed,
>> which is why I removed the context.
>
> I am torn between this.
>
> submodule..update = {rebase, merge, checkout, none !command}
> is currently documented in GIT-CONFIG(1) as
>
>submodule..update
>The default update procedure for a submodule. This variable is
>populated by git submodule init from the gitmodules(5) file. See
>description of update command in git-submodule(1).
>
> and in GIT-SUBMODULE(1) as
>
>update
>[...] can be done in several ways
>depending on command line options and the value of
>submodule..update configuration variable. Supported update
>procedures are:
>
>checkout
>[...] or no option is given, and
>submodule..update is unset, or if it is set to checkout.
>
> So the "update" config clearly only applies to the "submodule update"
> command, right?
>
> Well no, "checkout --recurse-submodules" is very similar
> to running "submodule update", except with a bit more checks, so you could
> think that such an option applies to checkout as well. (and eventually
> rebase/merge etc. are supported as well.)
>
> So initially I assumed both "unspecified" as well as "checkout"
> are good matches to support in the first round.
>
> Then I flip flopped to think that we should not interfere with these
> settings at all (The checkout command does checkout and checkout only;
> no implicit rebase/merge ever in the future, because that would be
> confusing). So ignoring that option seemed like the way to go.

Hmm. So it's a bit complicated.

>
> But ignoring that option is also not the right approach.
> What if you have set it to "none" and really *expect* Git to not touch
> that submodule?

Or set it to "rebase" and suddenly git-checkout is ignoring you and
just checking things out anyways.

>
> So I dunno. Maybe it is a documentation issue, we need to spell out
> in the man page for checkout that --recurse-submodules is
> following one of these models. Now which is the best default model here?

Personally, I would go with that the config option sets the general
strategy used by the submodule whenever its updated, regardless of
how.

So, for example, setting it to none, means that recurse-submoduls will
ignore it when checking out. Setting it to rebase, or merge, and the
checkout will try to do those things?

Or, if that's not really feasible, have the checkout go "hey.. you
asked me to recurse, but uhhh these submodules don't allow me to do
checkout, so I'm gonna fail"? I think that's the best approach for
now.

>
> Thanks,
> Stefan

Re: [PATCH 12/15] unpack-trees: check if we can perform the operation for submodules

2017-02-21 Thread Stefan Beller 
On Fri, Feb 17, 2017 at 10:42 AM, Jacob Keller  wrote:
> On Wed, Feb 15, 2017 at 4:38 PM, Stefan Beller  wrote:
>> +   if (is_active_submodule_with_strategy(ce, SM_UPDATE_UNSPECIFIED))
>
> Here, and in other cases where we use
> is_active_submodule_with_strategy(), why do we only ever check
> SM_UPDATE_UNSPECIFIED? It seems really weird that we're only going to
> check submodules who's strategy is unspecified, when that defaults to
> checkout if I recall correctly? Shouldn't we check both? This applies
> to pretty much everywhere that you call this function that I noticed,
> which is why I removed the context.

I am torn between this.

submodule..update = {rebase, merge, checkout, none !command}
is currently documented in GIT-CONFIG(1) as

   submodule..update
   The default update procedure for a submodule. This variable is
   populated by git submodule init from the gitmodules(5) file. See
   description of update command in git-submodule(1).

and in GIT-SUBMODULE(1) as

   update
   [...] can be done in several ways
   depending on command line options and the value of
   submodule..update configuration variable. Supported update
   procedures are:

   checkout
   [...] or no option is given, and
   submodule..update is unset, or if it is set to checkout.

So the "update" config clearly only applies to the "submodule update"
command, right?

Well no, "checkout --recurse-submodules" is very similar
to running "submodule update", except with a bit more checks, so you could
think that such an option applies to checkout as well. (and eventually
rebase/merge etc. are supported as well.)

So initially I assumed both "unspecified" as well as "checkout"
are good matches to support in the first round.

Then I flip flopped to think that we should not interfere with these
settings at all (The checkout command does checkout and checkout only;
no implicit rebase/merge ever in the future, because that would be
confusing). So ignoring that option seemed like the way to go.

But ignoring that option is also not the right approach.
What if you have set it to "none" and really *expect* Git to not touch
that submodule?

So I dunno. Maybe it is a documentation issue, we need to spell out
in the man page for checkout that --recurse-submodules is
following one of these models. Now which is the best default model here?

Thanks,
Stefan

Re: [PATCH 12/15] unpack-trees: check if we can perform the operation for submodules

2017-02-17 Thread Jacob Keller 
On Wed, Feb 15, 2017 at 4:38 PM, Stefan Beller  wrote:
> +   if (is_active_submodule_with_strategy(ce, SM_UPDATE_UNSPECIFIED))

Here, and in other cases where we use
is_active_submodule_with_strategy(), why do we only ever check
SM_UPDATE_UNSPECIFIED? It seems really weird that we're only going to
check submodules who's strategy is unspecified, when that defaults to
checkout if I recall correctly? Shouldn't we check both? This applies
to pretty much everywhere that you call this function that I noticed,
which is why I removed the context.

Thanks,
Jake

Re: [PATCH 12/15] unpack-trees: check if we can perform the operation for submodules

2017-02-16 Thread Junio C Hamano 
Stefan Beller  writes:

> +
> + /* ERROR_WOULD_LOSE_UNTRACKED_SUBMODULE */
> + "Submodule '%s' cannot be deleted as it contains untracked files.",

OK.

> + msgs[ERROR_WOULD_LOSE_UNTRACKED_SUBMODULE] =
> + _("Submodule '%s' cannot be deleted as it contains untracked 
> files.");

OK again.

> @@ -240,12 +246,44 @@ static void display_error_msgs(struct 
> unpack_trees_options *o)
>   fprintf(stderr, _("Aborting\n"));
>  }
>  
> +static int submodule_check_from_to(const struct cache_entry *ce, const char 
> *old_id, const char *new_id, struct unpack_trees_options *o)
> +{
> + if (submodule_go_from_to(ce->name, old_id,
> +  new_id, 1, o->reset))
> + return o->gently ? -1 :
> + add_rejected_path(o, 
> ERROR_WOULD_LOSE_UNTRACKED_SUBMODULE, ce->name);

Is potential loss of untracked paths the only reason
submodule_go_from_to() would fail?  I somehow thought that it would
not even care about untracked paths but cared deeply about already
added changes.

Re: [PATCH 12/15] unpack-trees: check if we can perform the operation for submodules

2017-02-16 Thread Brandon Williams 
On 02/15, Stefan Beller wrote:
> +static void reload_gitmodules_file(struct index_state *index,
> +struct checkout *state)
> +{
> + int i;
> + for (i = 0; i < index->cache_nr; i++) {
> + struct cache_entry *ce = index->cache[i];
> + if (ce->ce_flags & CE_UPDATE) {
> +
> + int r = strcmp(ce->name, ".gitmodules");
> + if (r < 0)
> + continue;
> + else if (r == 0) {
> + checkout_entry(ce, state, NULL);
> + } else
> + break;
> + }
> + }
> + gitmodules_config();
> + git_config(submodule_config, NULL);
> +}

If we are reloading the gitmodules file do you think it would makes
sense to add in a call to 'submodule_free()' to clear the cache used to
store the gitmodules config?

-- 
Brandon Williams

[PATCH 12/15] unpack-trees: check if we can perform the operation for submodules

2017-02-15 Thread Stefan Beller 
Signed-off-by: Stefan Beller 
---
 unpack-trees.c | 98 --
 unpack-trees.h |  1 +
 2 files changed, 90 insertions(+), 9 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index 616a0ae4b2..40af8e9b5f 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -10,6 +10,7 @@
 #include "attr.h"
 #include "split-index.h"
 #include "dir.h"
+#include "submodule.h"
 
 /*
  * Error messages expected by scripts out of plumbing commands such as
@@ -45,6 +46,9 @@ static const char 
*unpack_plumbing_errors[NB_UNPACK_TREES_ERROR_TYPES] = {
 
/* ERROR_WOULD_LOSE_ORPHANED_REMOVED */
"Working tree file '%s' would be removed by sparse checkout update.",
+
+   /* ERROR_WOULD_LOSE_UNTRACKED_SUBMODULE */
+   "Submodule '%s' cannot be deleted as it contains untracked files.",
 };
 
 #define ERRORMSG(o,type) \
@@ -161,6 +165,8 @@ void setup_unpack_trees_porcelain(struct 
unpack_trees_options *opts,
_("The following working tree files would be overwritten by 
sparse checkout update:\n%s");
msgs[ERROR_WOULD_LOSE_ORPHANED_REMOVED] =
_("The following working tree files would be removed by sparse 
checkout update:\n%s");
+   msgs[ERROR_WOULD_LOSE_UNTRACKED_SUBMODULE] =
+   _("Submodule '%s' cannot be deleted as it contains untracked 
files.");
 
opts->show_all_errors = 1;
/* rejected paths may not have a static buffer */
@@ -240,12 +246,44 @@ static void display_error_msgs(struct 
unpack_trees_options *o)
fprintf(stderr, _("Aborting\n"));
 }
 
+static int submodule_check_from_to(const struct cache_entry *ce, const char 
*old_id, const char *new_id, struct unpack_trees_options *o)
+{
+   if (submodule_go_from_to(ce->name, old_id,
+new_id, 1, o->reset))
+   return o->gently ? -1 :
+   add_rejected_path(o, 
ERROR_WOULD_LOSE_UNTRACKED_SUBMODULE, ce->name);
+   return 0;
+}
+
+static void reload_gitmodules_file(struct index_state *index,
+  struct checkout *state)
+{
+   int i;
+   for (i = 0; i < index->cache_nr; i++) {
+   struct cache_entry *ce = index->cache[i];
+   if (ce->ce_flags & CE_UPDATE) {
+
+   int r = strcmp(ce->name, ".gitmodules");
+   if (r < 0)
+   continue;
+   else if (r == 0) {
+   checkout_entry(ce, state, NULL);
+   } else
+   break;
+   }
+   }
+   gitmodules_config();
+   git_config(submodule_config, NULL);
+}
+
 /*
  * Unlink the last component and schedule the leading directories for
  * removal, such that empty directories get removed.
  */
 static void unlink_entry(const struct cache_entry *ce)
 {
+   if (is_active_submodule_with_strategy(ce, SM_UPDATE_UNSPECIFIED))
+   submodule_go_from_to(ce->name, "HEAD", NULL, 0, 1);
if (!check_leading_path(ce->name, ce_namelen(ce)))
return;
if (remove_or_warn(ce->ce_mode, ce->name))
@@ -301,6 +339,9 @@ static int check_updates(struct unpack_trees_options *o)
remove_marked_cache_entries(index);
remove_scheduled_dirs();
 
+   if (touch_submodules_in_worktree() && o->update && !o->dry_run)
+   reload_gitmodules_file(index, );
+
for (i = 0; i < index->cache_nr; i++) {
struct cache_entry *ce = index->cache[i];
 
@@ -1358,17 +1399,27 @@ static int verify_uptodate_1(const struct cache_entry 
*ce,
if (!lstat(ce->name, )) {
int flags = CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE;
unsigned changed = ie_match_stat(o->src_index, ce, , flags);
+
+   if (is_active_submodule_with_strategy(ce, 
SM_UPDATE_UNSPECIFIED)) {
+   int r;
+   r = submodule_check_from_to(ce,
+   "HEAD", oid_to_hex(>oid), o);
+   if (r)
+   return o->gently ? -1 :
+   add_rejected_path(o, error_type, 
ce->name);
+   return 0;
+   }
+
if (!changed)
return 0;
/*
-* NEEDSWORK: the current default policy is to allow
-* submodule to be out of sync wrt the superproject
-* index.  This needs to be tightened later for
-* submodules that are marked to be automatically
-* checked out.
+* Historic default policy was to allow submodule to be out
+* of sync wrt the superproject index. If the submodule was
+* not considered interesting above, we don't care here.
 */
if