Re: [PATCH v3 28/32] submodule--helper: libify "must_die_on_failure" code paths (for die)

From: Glen Choo <chooglen@google.com>
Date: Aug 23, 2022, 23:27 UTC
Message-ID: <kl6lk06y4k39.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <patch-v3-28.32-6d9bccb34c3-20220821T130231Z-avarab@gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> Continue the libification of codepaths that previously relied on
> "must_die_on_failure". In these cases we've always been early aborting
> by calling die(), but as we know that these codpaths will properly

s/codpaths/codepaths :p

Show 102 quoted lines

> handle return codes of 128 to mean an early abort let's have them use
> die_message() instead.
>
> This still isn't a complete migration away from die() for these
> codepaths, in particular this code in update_submodule() will still call die() in some cases:
>
> 	char *remote_name = get_default_remote_submodule(update_data->sm_path);
> 	const char *branch = remote_submodule_branch(update_data->sm_path);
>
> But as that code is used by other callers than the "update" code let's
> leave converting it for a subsequent commit.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  builtin/submodule--helper.c | 29 +++++++++++++++++------------
>  1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 851000ae30e..5f109d422ea 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -2245,9 +2245,9 @@ static int run_update_procedure(struct update_data *ud)
>  		 */
>  		if (!is_tip_reachable(ud->sm_path, &ud->oid) &&
>  		    fetch_in_submodule(ud->sm_path, ud->depth, ud->quiet, &ud->oid))
> -			die(_("Fetched in submodule path '%s', but it did not "
> -			      "contain %s. Direct fetching of that commit failed."),
> -			    ud->displaypath, oid_to_hex(&ud->oid));
> +			return die_message(_("Fetched in submodule path '%s', but it did not "
> +					     "contain %s. Direct fetching of that commit failed."),
> +					   ud->displaypath, oid_to_hex(&ud->oid));
>  	}
>  
>  	return run_update_command(ud, subforce);
> @@ -2291,13 +2291,14 @@ static const char *remote_submodule_branch(const char *path)
>  	return branch;
>  }
>  
> -static void ensure_core_worktree(const char *path)
> +static int ensure_core_worktree(const char *path)
>  {
>  	const char *cw;
>  	struct repository subrepo;
>  
>  	if (repo_submodule_init(&subrepo, the_repository, path, null_oid()))
> -		die(_("could not get a repository handle for submodule '%s'"), path);
> +		return die_message(_("could not get a repository handle for submodule '%s'"),
> +				   path);
>  
>  	if (!repo_config_get_string_tmp(&subrepo, "core.worktree", &cw)) {
>  		char *cfg_file, *abs_path;
> @@ -2315,6 +2316,8 @@ static void ensure_core_worktree(const char *path)
>  		free(abs_path);
>  		strbuf_release(&sb);
>  	}
> +
> +	return 0;
>  }
>  
>  static const char *submodule_update_type_to_label(enum submodule_update_type type)
> @@ -2390,7 +2393,9 @@ static int update_submodule(struct update_data *update_data)
>  {
>  	int ret;
>  
> -	ensure_core_worktree(update_data->sm_path);
> +	ret = ensure_core_worktree(update_data->sm_path);
> +	if (ret)
> +		return ret;
>  
>  	update_data->displaypath = get_submodule_displaypath(
>  		update_data->sm_path, update_data->prefix);
> @@ -2406,8 +2411,8 @@ static int update_submodule(struct update_data *update_data)
>  	if (update_data->just_cloned)
>  		oidcpy(&update_data->suboid, null_oid());
>  	else if (resolve_gitlink_ref(update_data->sm_path, "HEAD", &update_data->suboid))
> -		die(_("Unable to find current revision in submodule path '%s'"),
> -			update_data->displaypath);
> +		return die_message(_("Unable to find current revision in submodule path '%s'"),
> +				   update_data->displaypath);
>  
>  	if (update_data->remote) {
>  		char *remote_name = get_default_remote_submodule(update_data->sm_path);
> @@ -2417,13 +2422,13 @@ static int update_submodule(struct update_data *update_data)
>  		if (!update_data->nofetch) {
>  			if (fetch_in_submodule(update_data->sm_path, update_data->depth,
>  					      0, NULL))
> -				die(_("Unable to fetch in submodule path '%s'"),
> -				    update_data->sm_path);
> +				return die_message(_("Unable to fetch in submodule path '%s'"),
> +						   update_data->sm_path);
>  		}
>  
>  		if (resolve_gitlink_ref(update_data->sm_path, remote_ref, &update_data->oid))
> -			die(_("Unable to find %s revision in submodule path '%s'"),
> -			    remote_ref, update_data->sm_path);
> +			return die_message(_("Unable to find %s revision in submodule path '%s'"),
> +					   remote_ref, update_data->sm_path);
>  
>  		free(remote_ref);
>  	}
> -- 
> 2.37.2.1279.g64dec4e13cf