Subject: Re: [PATCH 11/15] unpack-trees: pass old oid to verify_clean_submodule

From: Junio C Hamano <[email-masked]>
Date: Thu, 16 Feb 2017 13:19:57 -0800

Stefan Beller <[email-masked]> writes:

> The check (which uses the old oid) is yet to be implemented, but this part
> is just a refactor, so it can go separately first.

If this didn't pass an unused parameter, then the change is just a
refactor, and I do think such a "just a refactor" can be a good step
on its own to keep the future step to manageable complexity.

With an unused parameter being passed, I do not think it is a good
logical single step anymore, though.

> Signed-off-by: Stefan Beller <[email-masked]>
> ---
>  unpack-trees.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 3a8ee19fe8..616a0ae4b2 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -1407,7 +1407,8 @@ static void invalidate_ce_path(const struct cache_entry *ce,
>   * Currently, git does not checkout subprojects during a superproject
>   * checkout, so it is not going to overwrite anything.
>   */
> -static int verify_clean_submodule(const struct cache_entry *ce,
> +static int verify_clean_submodule(const char *old_sha1,
> +				  const struct cache_entry *ce,
>  				  enum unpack_trees_error_types error_type,
>  				  struct unpack_trees_options *o)
>  {
> @@ -1427,16 +1428,18 @@ static int verify_clean_subdirectory(const struct cache_entry *ce,
>  	struct dir_struct d;
>  	char *pathbuf;
>  	int cnt = 0;
> -	unsigned char sha1[20];
>  
> -	if (S_ISGITLINK(ce->ce_mode) &&
> -	    resolve_gitlink_ref(ce->name, "HEAD", sha1) == 0) {
> -		/* If we are not going to update the submodule, then
> +	if (S_ISGITLINK(ce->ce_mode)) {
> +		unsigned char sha1[20];
> +		int sub_head = resolve_gitlink_ref(ce->name, "HEAD", sha1);
> +		/*
> +		 * If we are not going to update the submodule, then
>  		 * we don't care.
>  		 */
> -		if (!hashcmp(sha1, ce->oid.hash))
> +		if (!sub_head && !hashcmp(sha1, ce->oid.hash))
>  			return 0;
> -		return verify_clean_submodule(ce, error_type, o);
> +		return verify_clean_submodule(sub_head ? NULL : sha1_to_hex(sha1),
> +					      ce, error_type, o);
>  	}
>  
>  	/*