branch: report errors in tracking branch setup

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Feb 22, 2016 at 12:23 UTC 27852b2c5347ecd815301e668e7415509f1dae07
3 files changed +46 -16
branch.c
+36 -14
@@ -49,7 +49,13 @@ static int should_setup_rebase(const char *origin)
49 return 0;
50 }
51
52 -void install_branch_config(int flag, const char *local, const char *origin, const char *remote)
52 +static const char tracking_advice[] =
53 +N_("\n"
54 +"After fixing the error cause you may try to fix up\n"
55 +"the remote tracking information by invoking\n"
56 +"\"git branch --set-upstream-to=%s%s%s\".");
57 +
58 +int install_branch_config(int flag, const char *local, const char *origin, const char *remote)
59 {
60 const char *shortname = NULL;
61 struct strbuf key = STRBUF_INIT;
@@ -60,20 +66,23 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
66 && !origin) {
67 warning(_("Not setting branch %s as its own upstream."),
68 local);
63 - return;
69 + return 0;
70 }
71
72 strbuf_addf(&key, "branch.%s.remote", local);
67 - git_config_set(key.buf, origin ? origin : ".");
73 + if (git_config_set(key.buf, origin ? origin : ".") < 0)
74 + goto out_err;
75
76 strbuf_reset(&key);
77 strbuf_addf(&key, "branch.%s.merge", local);
71 - git_config_set(key.buf, remote);
78 + if (git_config_set(key.buf, remote) < 0)
79 + goto out_err;
80
81 if (rebasing) {
82 strbuf_reset(&key);
83 strbuf_addf(&key, "branch.%s.rebase", local);
76 - git_config_set(key.buf, "true");
84 + if (git_config_set(key.buf, "true") < 0)
85 + goto out_err;
86 }
87 strbuf_release(&key);
88
@@ -102,6 +111,19 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
111 local, remote);
112 }
113 }
114 +
115 + return 0;
116 +
117 +out_err:
118 + strbuf_release(&key);
119 + error(_("Unable to write upstream branch configuration"));
120 +
121 + advise(_(tracking_advice),
122 + origin ? origin : "",
123 + origin ? "/" : "",
124 + shortname ? shortname : remote);
125 +
126 + return -1;
127 }
128
129 /*
@@ -109,8 +131,8 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
131 * to infer the settings for branch.<new_ref>.{remote,merge} from the
132 * config.
133 */
112 -static int setup_tracking(const char *new_ref, const char *orig_ref,
113 - enum branch_track track, int quiet)
134 +static void setup_tracking(const char *new_ref, const char *orig_ref,
135 + enum branch_track track, int quiet)
136 {
137 struct tracking tracking;
138 int config_flags = quiet ? 0 : BRANCH_CONFIG_VERBOSE;
@@ -118,7 +140,7 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
140 memset(&tracking, 0, sizeof(tracking));
141 tracking.spec.dst = (char *)orig_ref;
142 if (for_each_remote(find_tracked_branch, &tracking))
121 - return 1;
143 + return;
144
145 if (!tracking.matches)
146 switch (track) {
@@ -127,18 +149,18 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
149 case BRANCH_TRACK_OVERRIDE:
150 break;
151 default:
130 - return 1;
152 + return;
153 }
154
155 if (tracking.matches > 1)
134 - return error(_("Not tracking: ambiguous information for ref %s"),
135 - orig_ref);
156 + die(_("Not tracking: ambiguous information for ref %s"),
157 + orig_ref);
158
137 - install_branch_config(config_flags, new_ref, tracking.remote,
138 - tracking.src ? tracking.src : orig_ref);
159 + if (install_branch_config(config_flags, new_ref, tracking.remote,
160 + tracking.src ? tracking.src : orig_ref) < 0)
161 + exit(-1);
162
163 free(tracking.src);
141 - return 0;
164 }
165
166 int read_branch_desc(struct strbuf *buf, const char *branch_name)
branch.h
+2 -1
@@ -43,9 +43,10 @@ void remove_branch_state(void);
43 /*
44 * Configure local branch "local" as downstream to branch "remote"
45 * from remote "origin". Used by git branch --set-upstream.
46 + * Returns 0 on success.
47 */
48 #define BRANCH_CONFIG_VERBOSE 01
48 -extern void install_branch_config(int flag, const char *local, const char *origin, const char *remote);
49 +extern int install_branch_config(int flag, const char *local, const char *origin, const char *remote);
50
51 /*
52 * Read branch description
t/t3200-branch.sh
+8 -1
@@ -446,6 +446,13 @@ test_expect_success '--set-upstream-to fails on a non-ref' '
446 test_must_fail git branch --set-upstream-to HEAD^{}
447 '
448
449 +test_expect_success '--set-upstream-to fails on locked config' '
450 + test_when_finished "rm -f .git/config.lock" &&
451 + >.git/config.lock &&
452 + git branch locked &&
453 + test_must_fail git branch --set-upstream-to locked
454 +'
455 +
456 test_expect_success 'use --set-upstream-to modify HEAD' '
457 test_config branch.master.remote foo &&
458 test_config branch.master.merge foo &&
@@ -579,7 +586,7 @@ test_expect_success 'avoid ambiguous track' '
586 git config remote.ambi1.fetch refs/heads/lalala:refs/heads/master &&
587 git config remote.ambi2.url lilili &&
588 git config remote.ambi2.fetch refs/heads/lilili:refs/heads/master &&
582 - git branch all1 master &&
589 + test_must_fail git branch all1 master &&
590 test -z "$(git config branch.all1.merge)"
591 '
592