[PATCH v2 2/3] branch: return an error from setup_tracking()

["tsukasa inoue via GitGitGadget" <gitgitgadget@gmail.com>]

From: tsukasa inoue <inoue.tsukasa.4@gmail.com>

setup_tracking() calls exit(1) when it cannot write the tracking
configuration. This exit was introduced in 27852b2c53 (branch: report
errors in tracking branch setup, 2016-02-22). Because of it, callers
cannot continue after a tracking failure, even though the ref has
already been created.

Change setup_tracking() to return an error instead of exiting, and make
create_branch() and dwim_and_setup_tracking() return that value too.

This does not change behavior. Callers still exit with status 1 on
failure, as before. Changing checkout

Signed-off-by: tsukasa inoue <inoue.tsukasa.4@gmail.com>
---
 branch.c           | 35 +++++++++++++++++++++--------------
 branch.h           | 21 +++++++++++++--------
 builtin/branch.c   | 17 ++++++++++++-----
 builtin/checkout.c | 17 +++++++++--------
 4 files changed, 55 insertions(+), 35 deletions(-)

diff --git a/branch.c b/branch.c
index bb07846911..7876892cc5 100644
--- a/branch.c
+++ b/branch.c
@@ -251,12 +251,13 @@ static int inherit_tracking(struct tracking *tracking, const char *orig_ref)
  * will not be expanded to "refs/remotes/origin/main", so it is not safe
  * for 'orig_ref' to be raw user input.
  */
-static void setup_tracking(const char *new_ref, const char *orig_ref,
-			   enum branch_track track, int quiet)
+static int setup_tracking(const char *new_ref, const char *orig_ref,
+			  enum branch_track track, int quiet)
 {
 	struct tracking tracking;
 	struct string_list tracking_srcs = STRING_LIST_INIT_DUP;
 	int config_flags = quiet ? 0 : BRANCH_CONFIG_VERBOSE;
+	int ret = 0;
 	struct find_tracked_branch_cb ftb_cb = {
 		.tracking = &tracking,
 		.ambiguous_remotes = STRING_LIST_INIT_DUP,
@@ -345,11 +346,12 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,
 		string_list_append(tracking.srcs, orig_ref);
 	if (install_branch_config_multiple_remotes(config_flags, new_ref,
 				tracking.remote, tracking.srcs) < 0)
-		exit(1);
+		ret = -1;
 
 cleanup:
 	string_list_clear(&tracking_srcs, 0);
 	string_list_clear(&ftb_cb.ambiguous_remotes, 0);
+	return ret;
 }
 
 int read_branch_desc(struct strbuf *buf, const char *branch_name)
@@ -629,10 +631,10 @@ static void dwim_branch_start(struct repository *r, const char *start_name,
 	FREE_AND_NULL(real_ref);
 }
 
-void create_branch(struct repository *r,
-		   const char *name, const char *start_name,
-		   int force, int clobber_head_ok, int reflog,
-		   int quiet, enum branch_track track, int dry_run)
+int create_branch(struct repository *r,
+		  const char *name, const char *start_name,
+		  int force, int clobber_head_ok, int reflog,
+		  int quiet, enum branch_track track, int dry_run)
 {
 	struct object_id oid;
 	char *real_ref;
@@ -641,6 +643,7 @@ void create_branch(struct repository *r,
 	struct ref_transaction *transaction;
 	struct strbuf err = STRBUF_INIT;
 	int flags = 0;
+	int ret = 0;
 	char *msg;
 
 	if (track == BRANCH_TRACK_OVERRIDE)
@@ -678,21 +681,25 @@ void create_branch(struct repository *r,
 	free(msg);
 
 	if (real_ref && track)
-		setup_tracking(ref.buf + 11, real_ref, track, quiet);
+		ret = setup_tracking(ref.buf + 11, real_ref, track, quiet);
 
 cleanup:
 	strbuf_release(&ref);
 	free(real_ref);
+	return ret;
 }
 
-void dwim_and_setup_tracking(struct repository *r, const char *new_ref,
-			     const char *orig_ref, enum branch_track track,
-			     int quiet)
+int dwim_and_setup_tracking(struct repository *r, const char *new_ref,
+			    const char *orig_ref, enum branch_track track,
+			    int quiet)
 {
 	char *real_orig_ref = NULL;
+	int ret;
+
 	dwim_branch_start(r, orig_ref, track, &real_orig_ref, NULL);
-	setup_tracking(new_ref, real_orig_ref, track, quiet);
+	ret = setup_tracking(new_ref, real_orig_ref, track, quiet);
 	free(real_orig_ref);
+	return ret;
 }
 
 /**
@@ -842,8 +849,8 @@ void create_branches_recursively(struct repository *r, const char *name,
 	 * tedious to determine whether or not tracking was set up in the
 	 * superproject.
 	 */
-	if (track)
-		setup_tracking(name, tracking_name, track, quiet);
+	if (track && setup_tracking(name, tracking_name, track, quiet))
+		exit(1);
 
 	for (i = 0; i < submodule_entry_list.entry_nr; i++) {
 		if (submodule_create_branch(
diff --git a/branch.h b/branch.h
index e9b1f7b37d..4bc4527284 100644
--- a/branch.h
+++ b/branch.h
@@ -34,10 +34,13 @@ enum branch_track {
  *     causes new_ref to track whatever orig_ref tracks.
  *
  *   - quiet suppresses tracking information.
+ *
+ * Returns 0 on success, or -1 if the tracking configuration could not be
+ * written.
  */
-void dwim_and_setup_tracking(struct repository *r, const char *new_ref,
-			     const char *orig_ref, enum branch_track track,
-			     int quiet);
+int dwim_and_setup_tracking(struct repository *r, const char *new_ref,
+			    const char *orig_ref, enum branch_track track,
+			    int quiet);
 
 /*
  * Creates a new branch, where:
@@ -63,12 +66,14 @@ void dwim_and_setup_tracking(struct repository *r, const char *new_ref,
  *
  *   - dry_run causes the branch to be validated but not created.
  *
+ * Returns 0 on success, or -1 if the tracking configuration could not be
+ * written.
  */
-void create_branch(struct repository *r,
-		   const char *name, const char *start_name,
-		   int force, int clobber_head_ok,
-		   int reflog, int quiet, enum branch_track track,
-		   int dry_run);
+int create_branch(struct repository *r,
+		  const char *name, const char *start_name,
+		  int force, int clobber_head_ok,
+		  int reflog, int quiet, enum branch_track track,
+		  int dry_run);
 
 /*
  * Creates a new branch in a repository and its submodules (and its
diff --git a/builtin/branch.c b/builtin/branch.c
index a613148fc7..b536b619bb 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -1253,9 +1253,13 @@ int cmd_branch(int argc,
 			die(_("branch '%s' does not exist"), branch->name);
 		}
 
-		dwim_and_setup_tracking(the_repository, branch->name,
-					new_upstream, BRANCH_TRACK_OVERRIDE,
-					quiet);
+		if (dwim_and_setup_tracking(the_repository, branch->name,
+					    new_upstream, BRANCH_TRACK_OVERRIDE,
+					    quiet)) {
+			strbuf_release(&buf);
+			ret = 1;
+			goto out;
+		}
 		strbuf_release(&buf);
 	} else if (unset_upstream) {
 		struct branch *branch;
@@ -1305,8 +1309,11 @@ int cmd_branch(int argc,
 			ret = 0;
 			goto out;
 		}
-		create_branch(the_repository, branch_name, start_name, force, 0,
-			      reflog, quiet, track, 0);
+		if (create_branch(the_repository, branch_name, start_name,
+				  force, 0, reflog, quiet, track, 0)) {
+			ret = 1;
+			goto out;
+		}
 	} else
 		usage_with_options(builtin_branch_usage, options);
 
diff --git a/builtin/checkout.c b/builtin/checkout.c
index c0f0d2c700..87e067b45b 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -979,14 +979,15 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
 			free(refname);
 		}
 		else
-			create_branch(the_repository,
-				      opts->new_branch, new_branch_info->name,
-				      opts->new_branch_force ? 1 : 0,
-				      opts->new_branch_force ? 1 : 0,
-				      opts->new_branch_log,
-				      opts->quiet,
-				      opts->track,
-				      0);
+			if (create_branch(the_repository,
+					  opts->new_branch, new_branch_info->name,
+					  opts->new_branch_force ? 1 : 0,
+					  opts->new_branch_force ? 1 : 0,
+					  opts->new_branch_log,
+					  opts->quiet,
+					  opts->track,
+					  0))
+				exit(1);
 		free(new_branch_info->name);
 		free(new_branch_info->refname);
 		new_branch_info->name = xstrdup(opts->new_branch);
-- 
gitgitgadget