Re: [PATCH 1/4] entry.c: submodule recursing: respect force flag correctly

From: Philip Oakley <philipoakley@iee.org>
Date: Apr 12, 2017, 11:28 UTC
Message-ID: <64236D980AD8478AA295BD7C263479FF@PhilipOakley>
In-Reply-To: <20170411234923.1860-2-sbeller@google.com>

From: "Stefan Beller" <sbeller@google.com>

> In case of a non-forced worktree update, the submodule movement is tested
> in a dry run first, such that it doesn't matter if the actual update is
> done via the force flag. However for correctness, we want to give the
> flag is specified by the user. All callers have been inspected and updated

s/flag is/flag as/

> if needed.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---

-- Philip

Show 71 quoted lines

> entry.c        | 8 ++++----
> unpack-trees.c | 7 ++++++-
> 2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/entry.c b/entry.c
> index d2b512da90..d6b263f78e 100644
> --- a/entry.c
> +++ b/entry.c
> @@ -208,7 +208,8 @@ static int write_entry(struct cache_entry *ce,
>  sub = submodule_from_ce(ce);
>  if (sub)
>  return submodule_move_head(ce->name,
> - NULL, oid_to_hex(&ce->oid), SUBMODULE_MOVE_HEAD_FORCE);
> + NULL, oid_to_hex(&ce->oid),
> + state->force ? SUBMODULE_MOVE_HEAD_FORCE : 0);
>  break;
>  default:
>  return error("unknown file mode for %s in index", path);
> @@ -282,12 +283,11 @@ int checkout_entry(struct cache_entry *ce,
>  unlink_or_warn(ce->name);
>
>  return submodule_move_head(ce->name,
> - NULL, oid_to_hex(&ce->oid),
> - SUBMODULE_MOVE_HEAD_FORCE);
> + NULL, oid_to_hex(&ce->oid), 0);
>  } else
>  return submodule_move_head(ce->name,
>  "HEAD", oid_to_hex(&ce->oid),
> - SUBMODULE_MOVE_HEAD_FORCE);
> + state->force ? SUBMODULE_MOVE_HEAD_FORCE : 0);
>  }
>
>  if (!changed)
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 8333da2cc9..7316ca99c2 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -252,14 +252,18 @@ static int check_submodule_move_head(const struct 
> cache_entry *ce,
>       const char *new_id,
>       struct unpack_trees_options *o)
> {
> + unsigned flags = SUBMODULE_MOVE_HEAD_DRY_RUN;
>  const struct submodule *sub = submodule_from_ce(ce);
>  if (!sub)
>  return 0;
>
> + if (o->reset)
> + flags |= SUBMODULE_MOVE_HEAD_FORCE;
> +
>  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))
> + if (submodule_move_head(ce->name, old_id, new_id, flags))
>  return o->gently ? -1 :
>  add_rejected_path(o, ERROR_WOULD_LOSE_SUBMODULE, ce->name);
>  return 0;
> @@ -308,6 +312,7 @@ static void unlink_entry(const struct cache_entry *ce)
>  case SM_UPDATE_CHECKOUT:
>  case SM_UPDATE_REBASE:
>  case SM_UPDATE_MERGE:
> + /* state.force is set at the caller. */
>  submodule_move_head(ce->name, "HEAD", NULL,
>      SUBMODULE_MOVE_HEAD_FORCE);
>  break;
> -- 
> 2.12.2.603.g7b28dc31ba
>
>