Re: [PATCH] push: do not add submodule odb as an alternate when recursing on demand

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Aug 15, 2017, 23:14 UTC
Message-ID: <20170815231414.GF13924@aiede.mtv.corp.google.com>
In-Reply-To: <20170815224332.22730-1-sbeller@google.com>

Stefan Beller wrote:

> Use is_submodule_populated_gently instead, which is simpler and
> cheaper.

[...]

Show 11 quoted lines

> --- a/submodule.c
> +++ b/submodule.c
> @@ -966,7 +966,9 @@ static int push_submodule(const char *path,
>  			  const struct string_list *push_options,
>  			  int dry_run)
>  {
> -	if (add_submodule_odb(path))
> +	int code;
> +
> +	if (!is_submodule_populated_gently(path, &code))
>  		return 1;

Ah, I forgot about this detail. I don't think it should block this patch (so my Reviewed-by still stands), but I wonder why this needs to be gentle. add_submodule_odb is gentle so that is the conservative thing to do, but that doesn't mean it is the *right* thing to do.

If this passed NULL instead of &code as the second argument, would anything break?

Could there be a comment explaining what kind of error we are expecting and why it is okay to continue when that error is encountered without any error handling?

Thanks, Jonathan