[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    [PATCH v4 31/33] submodule--helper: libify more "die" paths for module_update()
From:       Ævar Arnfjörð Bjarmason  <avarab () gmail ! com>
Date:       2022-08-31 23:18:13
Message-ID: patch-v4-31.33-a0d71b4e8bb-20220831T230519Z-avarab () gmail ! com
[Download RAW message or body]

As noted in a preceding commit the get_default_remote_submodule() and
remote_submodule_branch() functions would invoke die(), and thus leave
update_submodule() only partially lib-ified. Let's address the former
of those cases.

Change the functions to return an int exit code (non-zero on failure),
while leaving the get_default_remote() function for the callers that
still want the die() semantics.

This change addresses 1/2 of the "die" issue in these two lines in
update_submodule():

	char *remote_name = get_default_remote_submodule(update_data->sm_path);
	const char *branch = remote_submodule_branch(update_data->sm_path);

We can safely remove the "!default_remote" case from sync_submodule(),
because our get_default_remote_submodule() function now returns a
die_message() on failure, so we can have it an dother callers check if
the exit code should be non-zero instead.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/submodule--helper.c | 58 +++++++++++++++++++++++--------------
 1 file changed, 37 insertions(+), 21 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index dfd71f0f2b2..9de3a3c921a 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -31,48 +31,57 @@
 typedef void (*each_submodule_fn)(const struct cache_entry *list_item,
 				  void *cb_data);
 
-static char *repo_get_default_remote(struct repository *repo)
+static int repo_get_default_remote(struct repository *repo, char **default_remote)
 {
-	char *dest = NULL, *ret;
+	char *dest = NULL;
 	struct strbuf sb = STRBUF_INIT;
 	struct ref_store *store = get_main_ref_store(repo);
 	const char *refname = refs_resolve_ref_unsafe(store, "HEAD", 0, NULL,
 						      NULL);
 
 	if (!refname)
-		die(_("No such ref: %s"), "HEAD");
+		return die_message(_("No such ref: %s"), "HEAD");
 
 	/* detached HEAD */
-	if (!strcmp(refname, "HEAD"))
-		return xstrdup("origin");
+	if (!strcmp(refname, "HEAD")) {
+		*default_remote = xstrdup("origin");
+		return 0;
+	}
 
 	if (!skip_prefix(refname, "refs/heads/", &refname))
-		die(_("Expecting a full ref name, got %s"), refname);
+		return die_message(_("Expecting a full ref name, got %s"),
+				   refname);
 
 	strbuf_addf(&sb, "branch.%s.remote", refname);
 	if (repo_config_get_string(repo, sb.buf, &dest))
-		ret = xstrdup("origin");
+		*default_remote = xstrdup("origin");
 	else
-		ret = dest;
+		*default_remote = dest;
 
 	strbuf_release(&sb);
-	return ret;
+	return 0;
 }
 
-static char *get_default_remote_submodule(const char *module_path)
+static int get_default_remote_submodule(const char *module_path, char **default_remote)
 {
 	struct repository subrepo;
 
 	if (repo_submodule_init(&subrepo, the_repository, module_path,
 				null_oid()) < 0)
-		die(_("could not get a repository handle for submodule '%s'"),
-		    module_path);
-	return repo_get_default_remote(&subrepo);
+		return die_message(_("could not get a repository handle for submodule '%s'"),
+				   module_path);
+	return repo_get_default_remote(&subrepo, default_remote);
 }
 
 static char *get_default_remote(void)
 {
-	return repo_get_default_remote(the_repository);
+	char *default_remote;
+	int code = repo_get_default_remote(the_repository, &default_remote);
+
+	if (code)
+		exit(code);
+
+	return default_remote;
 }
 
 static char *resolve_relative_url(const char *rel_url, const char *up_path, int quiet)
@@ -1159,6 +1168,7 @@ static void sync_submodule(const char *path, const char *prefix,
 	char *sub_origin_url, *super_config_url, *displaypath, *default_remote;
 	struct strbuf sb = STRBUF_INIT;
 	char *sub_config_path = NULL;
+	int code;
 
 	if (!is_submodule_active(the_repository, path))
 		return;
@@ -1198,10 +1208,9 @@ static void sync_submodule(const char *path, const char *prefix,
 		goto cleanup;
 
 	strbuf_reset(&sb);
-	default_remote = get_default_remote_submodule(path);
-	if (!default_remote)
-		die(_("failed to get the default remote for submodule '%s'"),
-		      path);
+	code = get_default_remote_submodule(path, &default_remote);
+	if (code)
+		exit(code);
 
 	remote_key = xstrfmt("remote.%s.url", default_remote);
 	free(default_remote);
@@ -2422,9 +2431,16 @@ static int update_submodule(struct update_data *update_data)
 				   update_data->displaypath);
 
 	if (update_data->remote) {
-		char *remote_name = get_default_remote_submodule(update_data->sm_path);
-		const char *branch = remote_submodule_branch(update_data->sm_path);
-		char *remote_ref = xstrfmt("refs/remotes/%s/%s", remote_name, branch);
+		char *remote_name;
+		const char *branch;
+		char *remote_ref;
+		int code;
+
+		code = get_default_remote_submodule(update_data->sm_path, &remote_name);
+		if (code)
+			return code;
+		branch = remote_submodule_branch(update_data->sm_path);
+		remote_ref = xstrfmt("refs/remotes/%s/%s", remote_name, branch);
 
 		if (!update_data->nofetch) {
 			if (fetch_in_submodule(update_data->sm_path, update_data->depth,
-- 
2.37.3.1420.g76f8a3d556c

[prev in list] [next in list] [prev in thread] [next in thread]