diff mbox series

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

Message ID patch-v3-28.32-6d9bccb34c3-20220821T130231Z-avarab@gmail.com (mailing list archive)
State Superseded
Headers show
Series submodule--helper: add tests, rm dead code, refactor & leak prep | expand

Commit Message

Ævar Arnfjörð Bjarmason Aug. 21, 2022, 1:57 p.m. UTC
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
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(-)

Comments

Glen Choo Aug. 23, 2022, 11:27 p.m. UTC | #1
Æ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

> 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
diff mbox series

Patch

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);
 	}