git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v5 02/15] branch: report errors in tracking branch setup

From
Patrick Steinhardt <ps@pks.im>
Date
Feb 16, 2016, 12:56 UTC
Message-ID
<1455627402-752-3-git-send-email-ps@pks.im>
In-Reply-To
<1455627402-752-1-git-send-email-ps@pks.im>

When setting up a new tracking branch fails due to issues with the configuration file we do not report any errors to the user and pretend setting the tracking branch succeeded.

Setting up the tracking branch is handled by the `install_branch_config` function. We do not want to simply die there as the function is not only invoked when explicitly setting upstream information with `git branch --set-upstream-to=`, but also by `git push --set-upstream` and `git clone`. While it is reasonable to die in the explict first case, we would lose information in the latter two cases, so we only print the error message but continue the program as usual.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 branch.c          | 45 +++++++++++++++++++++++++++++++--------------
 branch.h          |  3 ++-
 t/t3200-branch.sh |  9 ++++++++-
 3 files changed, 41 insertions(+), 16 deletions(-)
diff --git a/branch.c b/branch.c
index 7ff3f20..3b9ccea 100644
--- a/branch.c
+++ b/branch.c
@@ -49,7 +49,13 @@ static int should_setup_rebase(const char *origin)
 	return 0;
 }
 
-void install_branch_config(int flag, const char *local, const char *origin, const char *remote)
+static const char tracking_advice[] =
+N_("\n"
+"After fixing the error cause you may try to fix up\n"
+"the remote tracking information by invoking\n"
+"\"git branch --set-upstream-to=\".");
+
+int install_branch_config(int flag, const char *local, const char *origin, const char *remote)
 {
 	const char *shortname = NULL;
 	struct strbuf key = STRBUF_INIT;
@@ -60,20 +66,23 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
 	    && !origin) {
 		warning(_("Not setting branch %s as its own upstream."),
 			local);
-		return;
+		return 0;
 	}
 
 	strbuf_addf(&key, "branch.%s.remote", local);
-	git_config_set(key.buf, origin ? origin : ".");
+	if (git_config_set(key.buf, origin ? origin : ".") < 0)
+		goto out_err;
 
 	strbuf_reset(&key);
 	strbuf_addf(&key, "branch.%s.merge", local);
-	git_config_set(key.buf, remote);
+	if (git_config_set(key.buf, remote) < 0)
+		goto out_err;
 
 	if (rebasing) {
 		strbuf_reset(&key);
 		strbuf_addf(&key, "branch.%s.rebase", local);
-		git_config_set(key.buf, "true");
+		if (git_config_set(key.buf, "true") < 0)
+		    goto out_err;
 	}
 	strbuf_release(&key);
 
@@ -102,6 +111,14 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
 					  local, remote);
 		}
 	}
+
+	return 0;
+
+out_err:
+	strbuf_release(&key);
+	error(_("Unable to write upstream branch configuration"));
+	advise(_(tracking_advice));
+	return -1;
 }
 
 /*
@@ -109,8 +126,8 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
  * to infer the settings for branch.<new_ref>.{remote,merge} from the
  * config.
  */
-static int setup_tracking(const char *new_ref, const char *orig_ref,
-			  enum branch_track track, int quiet)
+static void setup_tracking(const char *new_ref, const char *orig_ref,
+			   enum branch_track track, int quiet)
 {
 	struct tracking tracking;
 	int config_flags = quiet ? 0 : BRANCH_CONFIG_VERBOSE;
@@ -118,7 +135,7 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
 	memset(&tracking, 0, sizeof(tracking));
 	tracking.spec.dst = (char *)orig_ref;
 	if (for_each_remote(find_tracked_branch, &tracking))
-		return 1;
+		return;
 
 	if (!tracking.matches)
 		switch (track) {
@@ -127,18 +144,18 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
 		case BRANCH_TRACK_OVERRIDE:
 			break;
 		default:
-			return 1;
+			return;
 		}
 
 	if (tracking.matches > 1)
-		return error(_("Not tracking: ambiguous information for ref %s"),
-				orig_ref);
+		die(_("Not tracking: ambiguous information for ref %s"),
+		    orig_ref);
 
-	install_branch_config(config_flags, new_ref, tracking.remote,
-			      tracking.src ? tracking.src : orig_ref);
+	if (install_branch_config(config_flags, new_ref, tracking.remote,
+			      tracking.src ? tracking.src : orig_ref) < 0)
+		exit(-1);
 
 	free(tracking.src);
-	return 0;
 }
 
 int read_branch_desc(struct strbuf *buf, const char *branch_name)
diff --git a/branch.h b/branch.h
index 58aa45f..78ad438 100644
--- a/branch.h
+++ b/branch.h
@@ -43,9 +43,10 @@ void remove_branch_state(void);
 /*
  * Configure local branch "local" as downstream to branch "remote"
  * from remote "origin".  Used by git branch --set-upstream.
+ * Returns 0 on success.
  */
 #define BRANCH_CONFIG_VERBOSE 01
-extern void install_branch_config(int flag, const char *local, const char *origin, const char *remote);
+extern int install_branch_config(int flag, const char *local, const char *origin, const char *remote);
 
 /*
  * Read branch description
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index cdaf6f6..dd776b3 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -446,6 +446,13 @@ test_expect_success '--set-upstream-to fails on a non-ref' '
 	test_must_fail git branch --set-upstream-to HEAD^{}
 '
 
+test_expect_success '--set-upstream-to fails on locked config' '
+	test_when_finished "rm -f .git/config.lock" &&
+	>.git/config.lock &&
+	git branch locked &&
+	test_must_fail git branch --set-upstream-to locked
+'
+
 test_expect_success 'use --set-upstream-to modify HEAD' '
 	test_config branch.master.remote foo &&
 	test_config branch.master.merge foo &&
@@ -579,7 +586,7 @@ test_expect_success 'avoid ambiguous track' '
 	git config remote.ambi1.fetch refs/heads/lalala:refs/heads/master &&
 	git config remote.ambi2.url lilili &&
 	git config remote.ambi2.fetch refs/heads/lilili:refs/heads/master &&
-	git branch all1 master &&
+	test_must_fail git branch all1 master &&
 	test -z "$(git config branch.all1.merge)"
 '
 

-- 
2.7.1
Previous: Patrick SteinhardtNext: Junio C Hamano
Message 3 of 25 in “config: make git_config_set die on failure”
  1. 00/15 config: make git_config_set die on failurePatrick Steinhardt, Feb 16, 2016
  2. 01/15 config: introduce set_or_die wrappersPatrick Steinhardt, Feb 16, 2016
  3. 02/15 branch: report errors in tracking branch setupPatrick Steinhardt, Feb 16, 2016
  4. Junio C HamanoFeb 16, 2016
  5. Patrick SteinhardtFeb 17, 2016
  6. 03/15 branch: die on config error when unsetting upstreamPatrick Steinhardt, Feb 16, 2016
  7. 04/15 branch: die on config error when editing branch descriptionPatrick Steinhardt, Feb 16, 2016
  8. 05/15 submodule: die on config error when linking modulesPatrick Steinhardt, Feb 16, 2016
  9. 06/15 submodule--helper: die on config error when cloning modulePatrick Steinhardt, Feb 16, 2016
  10. 07/15 remote: die on config error when setting URLPatrick Steinhardt, Feb 16, 2016
  11. 08/15 remote: die on config error when setting/adding branchesPatrick Steinhardt, Feb 16, 2016
  12. 09/15 remote: die on config error when manipulating remotesPatrick Steinhardt, Feb 16, 2016
  13. 10/15 clone: die on config error in cmd_clonePatrick Steinhardt, Feb 16, 2016
  14. 11/15 init-db: die on config errors when initializing empty repoPatrick Steinhardt, Feb 16, 2016
  15. 12/15 sequencer: die on config error when saving replay optsPatrick Steinhardt, Feb 16, 2016
  16. 13/15 compat: die when unable to set core.precomposeunicodePatrick Steinhardt, Feb 16, 2016
  17. Lars SchneiderFeb 17, 2016
  18. Patrick SteinhardtFeb 17, 2016
  19. 14/15 config: rename git_config_set to git_config_set_gentlyPatrick Steinhardt, Feb 16, 2016
  20. 15/15 config: rename git_config_set_or_die to git_config_setPatrick Steinhardt, Feb 16, 2016
  21. Michael BlumeFeb 17, 2016
  22. Michael BlumeFeb 17, 2016
  23. Junio C HamanoFeb 17, 2016
  24. Eric SunshineFeb 16, 2016
  25. Patrick SteinhardtFeb 17, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.