The function `install_branch_config`, which is used to set up
tracking branches, does not verify return codes of
`git_config_set`. Due to this we may incorrectly print that a
tracking branch has been set up when in fact we did not due to an
error.
Fix this by checking the return value of `git_config_set` and
returning early in the case of an error. The
`install_branch_config` function has been changed to return an
error code that reflects whether it has been successful.
Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
---
I've run into this issue when a stale config.lock has been left:
> git branch --set-upstream-to=origin/next
error: could not lock config file /path/to/repo/git/config: File exists
error: could not lock config file /path/to/repo/git/config: File exists
Branch next set up to track remote branch next from origin.
While we spit out some errors, we still claim that we set up the
tracking branch correctly, which is not the case.
Actually, there are quite a few places where we don't check the
return values of git_config_set and related functions. I didn't
go through them yet, but might do so if you deem this to be of
value.
branch.c | 24 ++++++++++++++++--------
branch.h | 3 ++-
2 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/branch.c b/branch.c
index d013374..5ab3ad4 100644
--- a/branch.c
+++ b/branch.c
@@ -48,22 +48,24 @@ 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)
+int install_branch_config(int flag, const char *local, const char *origin, const char *remote)
{
const char *shortname = NULL;
struct strbuf key = STRBUF_INIT;
- int rebasing = should_setup_rebase(origin);
+ int ret = 0, rebasing = should_setup_rebase(origin);
if (skip_prefix(remote, "refs/heads/", &shortname)
&& !strcmp(local, shortname)
&& !origin) {
warning(_("Not setting branch %s as its own upstream."),
local);
- return;
+ return ret;
}
strbuf_addf(&key, "branch.%s.remote", local);
- git_config_set(key.buf, origin ? origin : ".");
+ ret = git_config_set(key.buf, origin ? origin : ".");
+ if (ret)
+ goto out;
strbuf_reset(&key);
strbuf_addf(&key, "branch.%s.merge", local);
@@ -72,9 +74,10 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
if (rebasing) {
strbuf_reset(&key);
strbuf_addf(&key, "branch.%s.rebase", local);
- git_config_set(key.buf, "true");
+ ret = git_config_set(key.buf, "true");
+ if (ret)
+ goto out;
}
- strbuf_release(&key);
if (flag & BRANCH_CONFIG_VERBOSE) {
if (shortname) {
@@ -101,6 +104,10 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
local, remote);
}
}
+
+out:
+ strbuf_release(&key);
+ return ret;
}
/*
@@ -133,8 +140,9 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
return error(_("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))
+ return error(_("Could not setup tracking branch"));
free(tracking.src);
return 0;
diff --git a/branch.h b/branch.h
index d3446ed..d287884 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 zero on success, non-zero otherwise.
*/
#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
--
2.5.3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html