From e41bf352e3280e6990605a18ebbbd40c6f1c0d6d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 1 May 2015 18:44:41 -0400 Subject: [PATCH 01/16] remote.c: drop default_remote_name variable When we read the remote config from disk, we update a default_remote_name variable if we see branch.*.remote config for the current branch. This isn't wrong, or even all that complicated, but it is a bit simpler (because it reduces our overall state) to just lazily compute the default when we need it. The ulterior motive here is that the push config uses a similar structure, and _is_ much more complicated as a result. That will be simplified in a future patch, and it's more readable if the logic for remotes and push-remotes matches. Note that we also used default_remote_name as a signal that the remote config has been loaded; after this patch, we now use an explicit flag. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- remote.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/remote.c b/remote.c index 68901b0070..bec8b311b0 100644 --- a/remote.c +++ b/remote.c @@ -49,10 +49,8 @@ static int branches_alloc; static int branches_nr; static struct branch *current_branch; -static const char *default_remote_name; static const char *branch_pushremote_name; static const char *pushremote_name; -static int explicit_default_remote_name; static struct rewrites rewrites; static struct rewrites rewrites_push; @@ -367,12 +365,7 @@ static int handle_config(const char *key, const char *value, void *cb) return 0; branch = make_branch(name, subkey - name); if (!strcmp(subkey, ".remote")) { - if (git_config_string(&branch->remote_name, key, value)) - return -1; - if (branch == current_branch) { - default_remote_name = branch->remote_name; - explicit_default_remote_name = 1; - } + return git_config_string(&branch->remote_name, key, value); } else if (!strcmp(subkey, ".pushremote")) { if (branch == current_branch) if (git_config_string(&branch_pushremote_name, key, value)) @@ -501,12 +494,15 @@ static void alias_all_urls(void) static void read_config(void) { + static int loaded; unsigned char sha1[20]; const char *head_ref; int flag; - if (default_remote_name) /* did this already */ + + if (loaded) return; - default_remote_name = "origin"; + loaded = 1; + current_branch = NULL; head_ref = resolve_ref_unsafe("HEAD", 0, sha1, &flag); if (head_ref && (flag & REF_ISSYMREF) && @@ -708,8 +704,11 @@ static struct remote *remote_get_1(const char *name, const char *pushremote_name name = pushremote_name; name_given = 1; } else { - name = default_remote_name; - name_given = explicit_default_remote_name; + if (current_branch && current_branch->remote_name) { + name = current_branch->remote_name; + name_given = 1; + } else + name = "origin"; } } From ee2499fe38d93fcbf4cbecdf83b32a495b0b12b9 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 May 2015 00:45:09 -0400 Subject: [PATCH 02/16] remote.c: refactor setup of branch->merge list When we call branch_get() to lookup or create a "struct branch", we make sure the "merge" field is filled in so that callers can access it. But the conditions under which we do so are a little confusing, and can lead to two funny situations: 1. If there's no branch.*.remote config, we cannot provide branch->merge (because it is really just an application of branch.*.merge to our remote's refspecs). But branch->merge_nr may be non-zero, leading callers to be believe they can access branch->merge (e.g., in branch_merge_matches and elsewhere). It doesn't look like this can cause a segfault in practice, as most code paths dealing with merge config will bail early if there is no remote defined. But it's a bit of a dangerous construct. We can fix this by setting merge_nr to "0" explicitly when we realize that we have no merge config. Note that merge_nr also counts the "merge_name" fields (which we _do_ have; that's how merge_nr got incremented), so we will "lose" access to them, in the sense that we forget how many we had. But no callers actually care; we use merge_name only while iteratively reading the config, and then convert it to the final "merge" form the first time somebody calls branch_get(). 2. We set up the "merge" field every time branch_get is called, even if it has already been done. This leaks memory. It's not a big deal in practice, since most code paths will access only one branch, or perhaps each branch only one time. But if you want to be pathological, you can leak arbitrary memory with: yes @{upstream} | head -1000 | git rev-list --stdin We can fix this by skipping setup when branch->merge is already non-NULL. In addition to those two fixes, this patch pushes the "do we need to setup merge?" logic down into set_merge, where it is a bit easier to follow. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- remote.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/remote.c b/remote.c index bec8b311b0..ac17e66c09 100644 --- a/remote.c +++ b/remote.c @@ -1636,6 +1636,19 @@ static void set_merge(struct branch *ret) unsigned char sha1[20]; int i; + if (!ret) + return; /* no branch */ + if (ret->merge) + return; /* already run */ + if (!ret->remote_name || !ret->merge_nr) { + /* + * no merge config; let's make sure we don't confuse callers + * with a non-zero merge_nr but a NULL merge + */ + ret->merge_nr = 0; + return; + } + ret->merge = xcalloc(ret->merge_nr, sizeof(*ret->merge)); for (i = 0; i < ret->merge_nr; i++) { ret->merge[i] = xcalloc(1, sizeof(**ret->merge)); @@ -1660,11 +1673,9 @@ struct branch *branch_get(const char *name) ret = current_branch; else ret = make_branch(name, 0); - if (ret && ret->remote_name) { + if (ret && ret->remote_name) ret->remote = remote_get(ret->remote_name); - if (ret->merge_nr) - set_merge(ret); - } + set_merge(ret); return ret; } From 9e3751d4437b43e72497178774c74be1ceac28b9 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 May 2015 00:45:13 -0400 Subject: [PATCH 03/16] remote.c: drop "remote" pointer from "struct branch" When we create each branch struct, we fill in the "remote_name" field from the config, and then fill in the actual "remote" field (with a "struct remote") based on that name. However, it turns out that nobody really cares about the latter field. The only two sites that access it at all are: 1. git-merge, which uses it to notice when the branch does not have a remote defined. But we can easily replace this with looking at remote_name instead. 2. remote.c itself, when setting up the @{upstream} merge config. But we don't need to save the "remote" in the "struct branch" for that; we can just look it up for the duration of the operation. So there is no need to have both fields; they are redundant with each other (the struct remote contains the name, or you can look up the struct from the name). It would be nice to simplify this, especially as we are going to add matching pushremote config in a future patch (and it would be nice to keep them consistent). So which one do we keep and which one do we get rid of? If we had a lot of callers accessing the struct, it would be more efficient to keep it (since you have to do a lookup to go from the name to the struct, but not vice versa). But we don't have a lot of callers; we have exactly one, so efficiency doesn't matter. We can decide this based on simplicity and readability. And the meaning of the struct value is somewhat unclear. Is it always the remote matching remote_name? If remote_name is NULL (i.e., no per-branch config), does the struct fall back to the "origin" remote, or is it also NULL? These questions will get even more tricky with pushremotes, whose fallback behavior is more complicated. So let's just store the name, which pretty clearly represents the branch.*.remote config. Any lookup or fallback behavior can then be implemented in helper functions. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/technical/api-remote.txt | 4 ---- builtin/merge.c | 2 +- remote.c | 7 ++++--- remote.h | 1 - 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/Documentation/technical/api-remote.txt b/Documentation/technical/api-remote.txt index 5d245aa9d1..2cfdd224a8 100644 --- a/Documentation/technical/api-remote.txt +++ b/Documentation/technical/api-remote.txt @@ -97,10 +97,6 @@ It contains: The name of the remote listed in the configuration. -`remote`:: - - The struct remote for that remote. - `merge_name`:: An array of the "merge" lines in the configuration. diff --git a/builtin/merge.c b/builtin/merge.c index 3b0f8f96d4..1840317118 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -955,7 +955,7 @@ static int setup_with_upstream(const char ***argv) if (!branch) die(_("No current branch.")); - if (!branch->remote) + if (!branch->remote_name) die(_("No remote for the current branch.")); if (!branch->merge_nr) die(_("No default upstream defined for the current branch.")); diff --git a/remote.c b/remote.c index ac17e66c09..c298a43a1c 100644 --- a/remote.c +++ b/remote.c @@ -1632,6 +1632,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, static void set_merge(struct branch *ret) { + struct remote *remote; char *ref; unsigned char sha1[20]; int i; @@ -1649,11 +1650,13 @@ static void set_merge(struct branch *ret) return; } + remote = remote_get(ret->remote_name); + ret->merge = xcalloc(ret->merge_nr, sizeof(*ret->merge)); for (i = 0; i < ret->merge_nr; i++) { ret->merge[i] = xcalloc(1, sizeof(**ret->merge)); ret->merge[i]->src = xstrdup(ret->merge_name[i]); - if (!remote_find_tracking(ret->remote, ret->merge[i]) || + if (!remote_find_tracking(remote, ret->merge[i]) || strcmp(ret->remote_name, ".")) continue; if (dwim_ref(ret->merge_name[i], strlen(ret->merge_name[i]), @@ -1673,8 +1676,6 @@ struct branch *branch_get(const char *name) ret = current_branch; else ret = make_branch(name, 0); - if (ret && ret->remote_name) - ret->remote = remote_get(ret->remote_name); set_merge(ret); return ret; } diff --git a/remote.h b/remote.h index 02d66ceff5..4bb6672735 100644 --- a/remote.h +++ b/remote.h @@ -203,7 +203,6 @@ struct branch { const char *refname; const char *remote_name; - struct remote *remote; const char **merge_name; struct refspec **merge; From f052154db332e48ea35b1a0d783361a40a361250 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 May 2015 00:45:16 -0400 Subject: [PATCH 04/16] remote.c: hoist branch.*.remote lookup out of remote_get_1 We'll want to use this logic as a fallback when looking up the pushremote, so let's pull it out into its own function. We don't technically need to make this available outside of remote.c, but doing so will provide a consistent API with pushremote_for_branch, which we will add later. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- remote.c | 21 ++++++++++++++------- remote.h | 1 + 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/remote.c b/remote.c index c298a43a1c..0d2976b36b 100644 --- a/remote.c +++ b/remote.c @@ -692,6 +692,18 @@ static int valid_remote_nick(const char *name) return !strchr(name, '/'); /* no slash */ } +const char *remote_for_branch(struct branch *branch, int *explicit) +{ + if (branch && branch->remote_name) { + if (explicit) + *explicit = 1; + return branch->remote_name; + } + if (explicit) + *explicit = 0; + return "origin"; +} + static struct remote *remote_get_1(const char *name, const char *pushremote_name) { struct remote *ret; @@ -703,13 +715,8 @@ static struct remote *remote_get_1(const char *name, const char *pushremote_name if (pushremote_name) { name = pushremote_name; name_given = 1; - } else { - if (current_branch && current_branch->remote_name) { - name = current_branch->remote_name; - name_given = 1; - } else - name = "origin"; - } + } else + name = remote_for_branch(current_branch, &name_given); } ret = make_remote(name, 0); diff --git a/remote.h b/remote.h index 4bb6672735..2a7e7a698b 100644 --- a/remote.h +++ b/remote.h @@ -211,6 +211,7 @@ struct branch { }; struct branch *branch_get(const char *name); +const char *remote_for_branch(struct branch *branch, int *explicit); int branch_has_merge_config(struct branch *branch); int branch_merge_matches(struct branch *, int n, const char *); From da66b2743cf7244e52c4b9d91646b782cd4f7eeb Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 May 2015 00:45:20 -0400 Subject: [PATCH 05/16] remote.c: provide per-branch pushremote name When remote.c loads its config, it records the branch.*.pushremote for the current branch along with the global remote.pushDefault value, and then binds them into a single value: the default push for the current branch. We then pass this value (which may be NULL) to remote_get_1 when looking up a remote for push. This has a few downsides: 1. It's confusing. The early-binding of the "current value" led to bugs like the one fixed by 98b406f (remote: handle pushremote config in any order, 2014-02-24). And the fact that pushremotes fall back to ordinary remotes is not explicit at all; it happens because remote_get_1 cannot tell the difference between "we are not asking for the push remote" and "there is no push remote configured". 2. It throws away intermediate data. After read_config() finishes, we have no idea what the value of remote.pushDefault was, because the string has been overwritten by the current branch's branch.*.pushremote. 3. It doesn't record other data. We don't note the branch.*.pushremote value for anything but the current branch. Let's make this more like the fetch-remote config. We'll record the pushremote for each branch, and then explicitly compute the correct remote for the current branch at the time of reading. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- remote.c | 40 ++++++++++++++++++++++------------------ remote.h | 2 ++ 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/remote.c b/remote.c index 0d2976b36b..a91d06396e 100644 --- a/remote.c +++ b/remote.c @@ -49,7 +49,6 @@ static int branches_alloc; static int branches_nr; static struct branch *current_branch; -static const char *branch_pushremote_name; static const char *pushremote_name; static struct rewrites rewrites; @@ -367,9 +366,7 @@ static int handle_config(const char *key, const char *value, void *cb) if (!strcmp(subkey, ".remote")) { return git_config_string(&branch->remote_name, key, value); } else if (!strcmp(subkey, ".pushremote")) { - if (branch == current_branch) - if (git_config_string(&branch_pushremote_name, key, value)) - return -1; + return git_config_string(&branch->pushremote_name, key, value); } else if (!strcmp(subkey, ".merge")) { if (!value) return config_error_nonbool(key); @@ -510,10 +507,6 @@ static void read_config(void) current_branch = make_branch(head_ref, 0); } git_config(handle_config, NULL); - if (branch_pushremote_name) { - free((char *)pushremote_name); - pushremote_name = branch_pushremote_name; - } alias_all_urls(); } @@ -704,20 +697,31 @@ const char *remote_for_branch(struct branch *branch, int *explicit) return "origin"; } -static struct remote *remote_get_1(const char *name, const char *pushremote_name) +const char *pushremote_for_branch(struct branch *branch, int *explicit) +{ + if (branch && branch->pushremote_name) { + if (explicit) + *explicit = 1; + return branch->pushremote_name; + } + if (pushremote_name) { + if (explicit) + *explicit = 1; + return pushremote_name; + } + return remote_for_branch(branch, explicit); +} + +static struct remote *remote_get_1(const char *name, + const char *(*get_default)(struct branch *, int *)) { struct remote *ret; int name_given = 0; if (name) name_given = 1; - else { - if (pushremote_name) { - name = pushremote_name; - name_given = 1; - } else - name = remote_for_branch(current_branch, &name_given); - } + else + name = get_default(current_branch, &name_given); ret = make_remote(name, 0); if (valid_remote_nick(name)) { @@ -738,13 +742,13 @@ static struct remote *remote_get_1(const char *name, const char *pushremote_name struct remote *remote_get(const char *name) { read_config(); - return remote_get_1(name, NULL); + return remote_get_1(name, remote_for_branch); } struct remote *pushremote_get(const char *name) { read_config(); - return remote_get_1(name, pushremote_name); + return remote_get_1(name, pushremote_for_branch); } int remote_is_configured(const char *name) diff --git a/remote.h b/remote.h index 2a7e7a698b..30a11dac10 100644 --- a/remote.h +++ b/remote.h @@ -203,6 +203,7 @@ struct branch { const char *refname; const char *remote_name; + const char *pushremote_name; const char **merge_name; struct refspec **merge; @@ -212,6 +213,7 @@ struct branch { struct branch *branch_get(const char *name); const char *remote_for_branch(struct branch *branch, int *explicit); +const char *pushremote_for_branch(struct branch *branch, int *explicit); int branch_has_merge_config(struct branch *branch); int branch_merge_matches(struct branch *, int n, const char *); From 8770e6fbb28dffdf9e00d05365120e438d3d236f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 May 2015 00:45:23 -0400 Subject: [PATCH 06/16] remote.c: hoist read_config into remote_get_1 Before the previous commit, we had to make sure that read_config() was called before entering remote_get_1, because we needed to pass pushremote_name by value. But now that we pass a function, we can let remote_get_1 handle loading the config itself, turning our wrappers into true one-liners. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- remote.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/remote.c b/remote.c index a91d06396e..e6b29b34b6 100644 --- a/remote.c +++ b/remote.c @@ -718,6 +718,8 @@ static struct remote *remote_get_1(const char *name, struct remote *ret; int name_given = 0; + read_config(); + if (name) name_given = 1; else @@ -741,13 +743,11 @@ static struct remote *remote_get_1(const char *name, struct remote *remote_get(const char *name) { - read_config(); return remote_get_1(name, remote_for_branch); } struct remote *pushremote_get(const char *name) { - read_config(); return remote_get_1(name, pushremote_for_branch); } From a9f9f8cc1f59104257eb1a11a2d048f54dd92ee6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 May 2015 00:45:28 -0400 Subject: [PATCH 07/16] remote.c: introduce branch_get_upstream helper All of the information needed to find the @{upstream} of a branch is included in the branch struct, but callers have to navigate a series of possible-NULL values to get there. Let's wrap that logic up in an easy-to-read helper. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/branch.c | 8 +++----- builtin/for-each-ref.c | 5 ++--- builtin/log.c | 7 ++----- remote.c | 12 +++++++++--- remote.h | 7 +++++++ 5 files changed, 23 insertions(+), 16 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 1d150378e9..bd1fa0b43a 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -123,14 +123,12 @@ static int branch_merged(int kind, const char *name, if (kind == REF_LOCAL_BRANCH) { struct branch *branch = branch_get(name); + const char *upstream = branch_get_upstream(branch); unsigned char sha1[20]; - if (branch && - branch->merge && - branch->merge[0] && - branch->merge[0]->dst && + if (upstream && (reference_name = reference_name_to_free = - resolve_refdup(branch->merge[0]->dst, RESOLVE_REF_READING, + resolve_refdup(upstream, RESOLVE_REF_READING, sha1, NULL)) != NULL) reference_rev = lookup_commit_reference(sha1); } diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 83f9cf9163..dc2a201a45 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -664,10 +664,9 @@ static void populate_value(struct refinfo *ref) continue; branch = branch_get(ref->refname + 11); - if (!branch || !branch->merge || !branch->merge[0] || - !branch->merge[0]->dst) + refname = branch_get_upstream(branch); + if (!refname) continue; - refname = branch->merge[0]->dst; } else if (starts_with(name, "color:")) { char color[COLOR_MAXLEN] = ""; diff --git a/builtin/log.c b/builtin/log.c index dd8f3fcfc4..fb61c08ee5 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1632,16 +1632,13 @@ int cmd_cherry(int argc, const char **argv, const char *prefix) break; default: current_branch = branch_get(NULL); - if (!current_branch || !current_branch->merge - || !current_branch->merge[0] - || !current_branch->merge[0]->dst) { + upstream = branch_get_upstream(current_branch); + if (!upstream) { fprintf(stderr, _("Could not find a tracked" " remote branch, please" " specify manually.\n")); usage_with_options(cherry_usage, options); } - - upstream = current_branch->merge[0]->dst; } init_revisions(&revs, prefix); diff --git a/remote.c b/remote.c index e6b29b34b6..dca3442aba 100644 --- a/remote.c +++ b/remote.c @@ -1705,6 +1705,13 @@ int branch_merge_matches(struct branch *branch, return refname_match(branch->merge[i]->src, refname); } +const char *branch_get_upstream(struct branch *branch) +{ + if (!branch || !branch->merge || !branch->merge[0]) + return NULL; + return branch->merge[0]->dst; +} + static int ignore_symref_update(const char *refname) { unsigned char sha1[20]; @@ -1914,12 +1921,11 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs) int rev_argc; /* Cannot stat unless we are marked to build on top of somebody else. */ - if (!branch || - !branch->merge || !branch->merge[0] || !branch->merge[0]->dst) + base = branch_get_upstream(branch); + if (!base) return 0; /* Cannot stat if what we used to build on no longer exists */ - base = branch->merge[0]->dst; if (read_ref(base, sha1)) return -1; theirs = lookup_commit_reference(sha1); diff --git a/remote.h b/remote.h index 30a11dac10..d96895282e 100644 --- a/remote.h +++ b/remote.h @@ -218,6 +218,13 @@ const char *pushremote_for_branch(struct branch *branch, int *explicit); int branch_has_merge_config(struct branch *branch); int branch_merge_matches(struct branch *, int n, const char *); +/** + * Return the fully-qualified refname of the tracking branch for `branch`. + * I.e., what "branch@{upstream}" would give you. Returns NULL if no + * upstream is defined. + */ +const char *branch_get_upstream(struct branch *branch); + /* Flags to match_refs. */ enum match_refs_flags { MATCH_REFS_NONE = 0, From 3a429d0af342d85ef6d561e3a60ae8793a34ae78 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 May 2015 00:45:32 -0400 Subject: [PATCH 08/16] remote.c: report specific errors from branch_get_upstream When the previous commit introduced the branch_get_upstream helper, there was one call-site that could not be converted: the one in sha1_name.c, which gives detailed error messages for each possible failure. Let's teach the helper to optionally report these specific errors. This lets us convert another callsite, and means we can use the helper in other locations that want to give the same error messages. The logic and error messages come straight from sha1_name.c, with the exception that we start each error with a lowercase letter, as is our usual style (note that a few tests need updated as a result). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/branch.c | 2 +- builtin/for-each-ref.c | 2 +- builtin/log.c | 2 +- remote.c | 33 +++++++++++++++++++++++++++++---- remote.h | 6 +++++- sha1_name.c | 25 +++++++------------------ t/t1507-rev-parse-upstream.sh | 8 ++++---- 7 files changed, 48 insertions(+), 30 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index bd1fa0b43a..b7202b3399 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -123,7 +123,7 @@ static int branch_merged(int kind, const char *name, if (kind == REF_LOCAL_BRANCH) { struct branch *branch = branch_get(name); - const char *upstream = branch_get_upstream(branch); + const char *upstream = branch_get_upstream(branch, NULL); unsigned char sha1[20]; if (upstream && diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index dc2a201a45..18d209bc9a 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -664,7 +664,7 @@ static void populate_value(struct refinfo *ref) continue; branch = branch_get(ref->refname + 11); - refname = branch_get_upstream(branch); + refname = branch_get_upstream(branch, NULL); if (!refname) continue; } else if (starts_with(name, "color:")) { diff --git a/builtin/log.c b/builtin/log.c index fb61c08ee5..6faeb82c8e 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1632,7 +1632,7 @@ int cmd_cherry(int argc, const char **argv, const char *prefix) break; default: current_branch = branch_get(NULL); - upstream = branch_get_upstream(current_branch); + upstream = branch_get_upstream(current_branch, NULL); if (!upstream) { fprintf(stderr, _("Could not find a tracked" " remote branch, please" diff --git a/remote.c b/remote.c index dca3442aba..1b7051a187 100644 --- a/remote.c +++ b/remote.c @@ -1705,10 +1705,35 @@ int branch_merge_matches(struct branch *branch, return refname_match(branch->merge[i]->src, refname); } -const char *branch_get_upstream(struct branch *branch) +__attribute((format (printf,2,3))) +static const char *error_buf(struct strbuf *err, const char *fmt, ...) { - if (!branch || !branch->merge || !branch->merge[0]) - return NULL; + if (err) { + va_list ap; + va_start(ap, fmt); + strbuf_vaddf(err, fmt, ap); + va_end(ap); + } + return NULL; +} + +const char *branch_get_upstream(struct branch *branch, struct strbuf *err) +{ + if (!branch) + return error_buf(err, _("HEAD does not point to a branch")); + if (!branch->merge || !branch->merge[0] || !branch->merge[0]->dst) { + if (!ref_exists(branch->refname)) + return error_buf(err, _("no such branch: '%s'"), + branch->name); + if (!branch->merge) + return error_buf(err, + _("no upstream configured for branch '%s'"), + branch->name); + return error_buf(err, + _("upstream branch '%s' not stored as a remote-tracking branch"), + branch->merge[0]->src); + } + return branch->merge[0]->dst; } @@ -1921,7 +1946,7 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs) int rev_argc; /* Cannot stat unless we are marked to build on top of somebody else. */ - base = branch_get_upstream(branch); + base = branch_get_upstream(branch, NULL); if (!base) return 0; diff --git a/remote.h b/remote.h index d96895282e..03ca0058fe 100644 --- a/remote.h +++ b/remote.h @@ -222,8 +222,12 @@ int branch_merge_matches(struct branch *, int n, const char *); * Return the fully-qualified refname of the tracking branch for `branch`. * I.e., what "branch@{upstream}" would give you. Returns NULL if no * upstream is defined. + * + * If `err` is not NULL and no upstream is defined, a more specific error + * message is recorded there (if the function does not return NULL, then + * `err` is not touched). */ -const char *branch_get_upstream(struct branch *branch); +const char *branch_get_upstream(struct branch *branch, struct strbuf *err); /* Flags to match_refs. */ enum match_refs_flags { diff --git a/sha1_name.c b/sha1_name.c index 6d10f052b5..461157a5bc 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -1059,27 +1059,16 @@ static const char *get_upstream_branch(const char *branch_buf, int len) { char *branch = xstrndup(branch_buf, len); struct branch *upstream = branch_get(*branch ? branch : NULL); + struct strbuf err = STRBUF_INIT; + const char *ret; - /* - * Upstream can be NULL only if branch refers to HEAD and HEAD - * points to something different than a branch. - */ - if (!upstream) - die(_("HEAD does not point to a branch")); - if (!upstream->merge || !upstream->merge[0]->dst) { - if (!ref_exists(upstream->refname)) - die(_("No such branch: '%s'"), branch); - if (!upstream->merge) { - die(_("No upstream configured for branch '%s'"), - upstream->name); - } - die( - _("Upstream branch '%s' not stored as a remote-tracking branch"), - upstream->merge[0]->src); - } free(branch); - return upstream->merge[0]->dst; + ret = branch_get_upstream(upstream, &err); + if (!ret) + die("%s", err.buf); + + return ret; } static int interpret_upstream_mark(const char *name, int namelen, diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh index 1978947c41..46ef1f22dc 100755 --- a/t/t1507-rev-parse-upstream.sh +++ b/t/t1507-rev-parse-upstream.sh @@ -150,7 +150,7 @@ test_expect_success 'branch@{u} works when tracking a local branch' ' test_expect_success 'branch@{u} error message when no upstream' ' cat >expect <<-EOF && - fatal: No upstream configured for branch ${sq}non-tracking${sq} + fatal: no upstream configured for branch ${sq}non-tracking${sq} EOF error_message non-tracking@{u} 2>actual && test_i18ncmp expect actual @@ -158,7 +158,7 @@ test_expect_success 'branch@{u} error message when no upstream' ' test_expect_success '@{u} error message when no upstream' ' cat >expect <<-EOF && - fatal: No upstream configured for branch ${sq}master${sq} + fatal: no upstream configured for branch ${sq}master${sq} EOF test_must_fail git rev-parse --verify @{u} 2>actual && test_i18ncmp expect actual @@ -166,7 +166,7 @@ test_expect_success '@{u} error message when no upstream' ' test_expect_success 'branch@{u} error message with misspelt branch' ' cat >expect <<-EOF && - fatal: No such branch: ${sq}no-such-branch${sq} + fatal: no such branch: ${sq}no-such-branch${sq} EOF error_message no-such-branch@{u} 2>actual && test_i18ncmp expect actual @@ -183,7 +183,7 @@ test_expect_success '@{u} error message when not on a branch' ' test_expect_success 'branch@{u} error message if upstream branch not fetched' ' cat >expect <<-EOF && - fatal: Upstream branch ${sq}refs/heads/side${sq} not stored as a remote-tracking branch + fatal: upstream branch ${sq}refs/heads/side${sq} not stored as a remote-tracking branch EOF error_message bad-upstream@{u} 2>actual && test_i18ncmp expect actual From 1ca41a19323d455cf0028001677f3adfae0d4cc4 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 May 2015 20:46:43 -0400 Subject: [PATCH 09/16] remote.c: untangle error logic in branch_get_upstream The error-diagnosis logic in branch_get_upstream was copied straight from sha1_name.c in the previous commit. However, because we check all error cases and upfront and then later diagnose them, the logic is a bit tangled. In particular: - if branch->merge[0] is NULL, we may end up dereferencing it for an error message (in practice, it should never be NULL, so this is probably not a triggerable bug). - We may enter the code path because branch->merge[0]->dst is NULL, but we then start our error diagnosis by checking whether our local branch exists. But that is only relevant to diagnosing missing merge config, not a missing tracking ref; our diagnosis may hide the real problem. Instead, let's just use a sequence of "if" blocks to check for each error type, diagnose it, and return NULL. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- remote.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/remote.c b/remote.c index 1b7051a187..d2519c22ce 100644 --- a/remote.c +++ b/remote.c @@ -1721,18 +1721,25 @@ const char *branch_get_upstream(struct branch *branch, struct strbuf *err) { if (!branch) return error_buf(err, _("HEAD does not point to a branch")); - if (!branch->merge || !branch->merge[0] || !branch->merge[0]->dst) { + + if (!branch->merge || !branch->merge[0]) { + /* + * no merge config; is it because the user didn't define any, + * or because it is not a real branch, and get_branch + * auto-vivified it? + */ if (!ref_exists(branch->refname)) return error_buf(err, _("no such branch: '%s'"), branch->name); - if (!branch->merge) - return error_buf(err, - _("no upstream configured for branch '%s'"), - branch->name); + return error_buf(err, + _("no upstream configured for branch '%s'"), + branch->name); + } + + if (!branch->merge[0]->dst) return error_buf(err, _("upstream branch '%s' not stored as a remote-tracking branch"), branch->merge[0]->src); - } return branch->merge[0]->dst; } From 979cb245e29b35812d2d71a04069ba7a4580981f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 May 2015 20:49:11 -0400 Subject: [PATCH 10/16] remote.c: return upstream name from stat_tracking_info After calling stat_tracking_info, callers often want to print the name of the upstream branch (in addition to the tracking count). To do this, they have to access branch->merge->dst[0] themselves. This is not wrong, as the return value from stat_tracking_info tells us whether we have an upstream branch or not. But it is a bit leaky, as we make an assumption about how it calculated the upstream name. Instead, let's add an out-parameter that lets the caller know the upstream name we found. As a bonus, we can get rid of the unusual tri-state return from the function. We no longer need to use it to differentiate between "no tracking config" and "tracking ref does not exist" (since you can check the upstream_name for that), so we can just use the usual 0/-1 convention for success/error. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/branch.c | 16 +++++----------- builtin/for-each-ref.c | 4 ++-- remote.c | 35 +++++++++++++++++------------------ remote.h | 3 ++- wt-status.c | 18 ++++++------------ 5 files changed, 32 insertions(+), 44 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index b7202b3399..e4d184d69a 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -425,25 +425,19 @@ static void fill_tracking_info(struct strbuf *stat, const char *branch_name, int ours, theirs; char *ref = NULL; struct branch *branch = branch_get(branch_name); + const char *upstream; struct strbuf fancy = STRBUF_INIT; int upstream_is_gone = 0; int added_decoration = 1; - switch (stat_tracking_info(branch, &ours, &theirs)) { - case 0: - /* no base */ - return; - case -1: - /* with "gone" base */ + if (stat_tracking_info(branch, &ours, &theirs, &upstream) < 0) { + if (!upstream) + return; upstream_is_gone = 1; - break; - default: - /* with base */ - break; } if (show_upstream_ref) { - ref = shorten_unambiguous_ref(branch->merge[0]->dst, 0); + ref = shorten_unambiguous_ref(upstream, 0); if (want_color(branch_use_color)) strbuf_addf(&fancy, "%s%s%s", branch_get_color(BRANCH_COLOR_UPSTREAM), diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 18d209bc9a..92bd2b2665 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -716,7 +716,7 @@ static void populate_value(struct refinfo *ref) char buf[40]; if (stat_tracking_info(branch, &num_ours, - &num_theirs) != 1) + &num_theirs, NULL)) continue; if (!num_ours && !num_theirs) @@ -738,7 +738,7 @@ static void populate_value(struct refinfo *ref) assert(branch); if (stat_tracking_info(branch, &num_ours, - &num_theirs) != 1) + &num_theirs, NULL)) continue; if (!num_ours && !num_theirs) diff --git a/remote.c b/remote.c index d2519c22ce..c8845746b6 100644 --- a/remote.c +++ b/remote.c @@ -1938,12 +1938,15 @@ int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1) /* * Compare a branch with its upstream, and save their differences (number - * of commits) in *num_ours and *num_theirs. + * of commits) in *num_ours and *num_theirs. The name of the upstream branch + * (or NULL if no upstream is defined) is returned via *upstream_name, if it + * is not itself NULL. * - * Return 0 if branch has no upstream (no base), -1 if upstream is missing - * (with "gone" base), otherwise 1 (with base). + * Returns -1 if num_ours and num_theirs could not be filled in (e.g., no + * upstream defined, or ref does not exist), 0 otherwise. */ -int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs) +int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs, + const char **upstream_name) { unsigned char sha1[20]; struct commit *ours, *theirs; @@ -1954,8 +1957,10 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs) /* Cannot stat unless we are marked to build on top of somebody else. */ base = branch_get_upstream(branch, NULL); + if (upstream_name) + *upstream_name = base; if (!base) - return 0; + return -1; /* Cannot stat if what we used to build on no longer exists */ if (read_ref(base, sha1)) @@ -1973,7 +1978,7 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs) /* are we the same? */ if (theirs == ours) { *num_theirs = *num_ours = 0; - return 1; + return 0; } /* Run "rev-list --left-right ours...theirs" internally... */ @@ -2009,7 +2014,7 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs) /* clear object flags smudged by the above traversal */ clear_commit_marks(ours, ALL_REV_FLAGS); clear_commit_marks(theirs, ALL_REV_FLAGS); - return 1; + return 0; } /* @@ -2018,23 +2023,17 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs) int format_tracking_info(struct branch *branch, struct strbuf *sb) { int ours, theirs; + const char *full_base; char *base; int upstream_is_gone = 0; - switch (stat_tracking_info(branch, &ours, &theirs)) { - case 0: - /* no base */ - return 0; - case -1: - /* with "gone" base */ + if (stat_tracking_info(branch, &ours, &theirs, &full_base) < 0) { + if (!full_base) + return 0; upstream_is_gone = 1; - break; - default: - /* with base */ - break; } - base = shorten_unambiguous_ref(branch->merge[0]->dst, 0); + base = shorten_unambiguous_ref(full_base, 0); if (upstream_is_gone) { strbuf_addf(sb, _("Your branch is based on '%s', but the upstream is gone.\n"), diff --git a/remote.h b/remote.h index 03ca0058fe..357a90963d 100644 --- a/remote.h +++ b/remote.h @@ -239,7 +239,8 @@ enum match_refs_flags { }; /* Reporting of tracking info */ -int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs); +int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs, + const char **upstream_name); int format_tracking_info(struct branch *branch, struct strbuf *sb); struct ref *get_local_heads(void); diff --git a/wt-status.c b/wt-status.c index 853419f05f..4313b9d845 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1532,21 +1532,15 @@ static void wt_shortstatus_print_tracking(struct wt_status *s) color_fprintf(s->fp, branch_color_local, "%s", branch_name); - switch (stat_tracking_info(branch, &num_ours, &num_theirs)) { - case 0: - /* no base */ - fputc(s->null_termination ? '\0' : '\n', s->fp); - return; - case -1: - /* with "gone" base */ + if (stat_tracking_info(branch, &num_ours, &num_theirs, &base) < 0) { + if (!base) { + fputc(s->null_termination ? '\0' : '\n', s->fp); + return; + } + upstream_is_gone = 1; - break; - default: - /* with base */ - break; } - base = branch->merge[0]->dst; base = shorten_unambiguous_ref(base, 0); color_fprintf(s->fp, header_color, "..."); color_fprintf(s->fp, branch_color_remote, "%s", base); From e291c75a95d60862cbe12897b5cffb01ba4cedd5 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 May 2015 00:45:36 -0400 Subject: [PATCH 11/16] remote.c: add branch_get_push In a triangular workflow, the place you pull from and the place you push to may be different. As we have branch_get_upstream for the former, this patch adds branch_get_push for the latter (and as the former implements @{upstream}, so will this implement @{push} in a future patch). Note that the memory-handling for the return value bears some explanation. Some code paths require allocating a new string, and some let us return an existing string. We should provide a consistent interface to the caller, so it knows whether to free the result or not. We could do so by xstrdup-ing any existing strings, and having the caller always free. But that makes us inconsistent with branch_get_upstream, so we would prefer to simply take ownership of the resulting string. We do so by storing it inside the "struct branch", just as we do with the upstream refname (in that case we compute it when the branch is created, but there's no reason not to just fill it in lazily in this case). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- remote.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ remote.h | 10 +++++++ 2 files changed, 95 insertions(+) diff --git a/remote.c b/remote.c index c8845746b6..a467d4ff07 100644 --- a/remote.c +++ b/remote.c @@ -1744,6 +1744,91 @@ const char *branch_get_upstream(struct branch *branch, struct strbuf *err) return branch->merge[0]->dst; } +static const char *tracking_for_push_dest(struct remote *remote, + const char *refname, + struct strbuf *err) +{ + char *ret; + + ret = apply_refspecs(remote->fetch, remote->fetch_refspec_nr, refname); + if (!ret) + return error_buf(err, + _("push destination '%s' on remote '%s' has no local tracking branch"), + refname, remote->name); + return ret; +} + +static const char *branch_get_push_1(struct branch *branch, struct strbuf *err) +{ + struct remote *remote; + + if (!branch) + return error_buf(err, _("HEAD does not point to a branch")); + + remote = remote_get(pushremote_for_branch(branch, NULL)); + if (!remote) + return error_buf(err, + _("branch '%s' has no remote for pushing"), + branch->name); + + if (remote->push_refspec_nr) { + char *dst; + const char *ret; + + dst = apply_refspecs(remote->push, remote->push_refspec_nr, + branch->refname); + if (!dst) + return error_buf(err, + _("push refspecs for '%s' do not include '%s'"), + remote->name, branch->name); + + ret = tracking_for_push_dest(remote, dst, err); + free(dst); + return ret; + } + + if (remote->mirror) + return tracking_for_push_dest(remote, branch->refname, err); + + switch (push_default) { + case PUSH_DEFAULT_NOTHING: + return error_buf(err, _("push has no destination (push.default is 'nothing')")); + + case PUSH_DEFAULT_MATCHING: + case PUSH_DEFAULT_CURRENT: + return tracking_for_push_dest(remote, branch->refname, err); + + case PUSH_DEFAULT_UPSTREAM: + return branch_get_upstream(branch, err); + + case PUSH_DEFAULT_UNSPECIFIED: + case PUSH_DEFAULT_SIMPLE: + { + const char *up, *cur; + + up = branch_get_upstream(branch, err); + if (!up) + return NULL; + cur = tracking_for_push_dest(remote, branch->refname, err); + if (!cur) + return NULL; + if (strcmp(cur, up)) + return error_buf(err, + _("cannot resolve 'simple' push to a single destination")); + return cur; + } + } + + die("BUG: unhandled push situation"); +} + +const char *branch_get_push(struct branch *branch, struct strbuf *err) +{ + if (!branch->push_tracking_ref) + branch->push_tracking_ref = branch_get_push_1(branch, err); + return branch->push_tracking_ref; +} + static int ignore_symref_update(const char *refname) { unsigned char sha1[20]; diff --git a/remote.h b/remote.h index 357a90963d..312b7ca131 100644 --- a/remote.h +++ b/remote.h @@ -209,6 +209,8 @@ struct branch { struct refspec **merge; int merge_nr; int merge_alloc; + + const char *push_tracking_ref; }; struct branch *branch_get(const char *name); @@ -229,6 +231,14 @@ int branch_merge_matches(struct branch *, int n, const char *); */ const char *branch_get_upstream(struct branch *branch, struct strbuf *err); +/** + * Return the tracking branch that corresponds to the ref we would push to + * given a bare `git push` while `branch` is checked out. + * + * The return value and `err` conventions match those of `branch_get_upstream`. + */ +const char *branch_get_push(struct branch *branch, struct strbuf *err); + /* Flags to match_refs. */ enum match_refs_flags { MATCH_REFS_NONE = 0, From a1ad0eb0cb1c9c83492b17d7a218be084466bf9a Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 May 2015 00:45:39 -0400 Subject: [PATCH 12/16] sha1_name: refactor upstream_mark We will be adding new mark types in the future, so separate the suffix data from the logic. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- sha1_name.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 461157a5bc..1005f45d75 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -415,12 +415,12 @@ static int ambiguous_path(const char *path, int len) return slash; } -static inline int upstream_mark(const char *string, int len) +static inline int at_mark(const char *string, int len, + const char **suffix, int nr) { - const char *suffix[] = { "@{upstream}", "@{u}" }; int i; - for (i = 0; i < ARRAY_SIZE(suffix); i++) { + for (i = 0; i < nr; i++) { int suffix_len = strlen(suffix[i]); if (suffix_len <= len && !memcmp(string, suffix[i], suffix_len)) @@ -429,6 +429,12 @@ static inline int upstream_mark(const char *string, int len) return 0; } +static inline int upstream_mark(const char *string, int len) +{ + const char *suffix[] = { "@{upstream}", "@{u}" }; + return at_mark(string, len, suffix, ARRAY_SIZE(suffix)); +} + static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned lookup_flags); static int interpret_nth_prior_checkout(const char *name, int namelen, struct strbuf *buf); From 48c58471c2d4d7293272448a18801cd27555f6b5 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 May 2015 00:45:43 -0400 Subject: [PATCH 13/16] sha1_name: refactor interpret_upstream_mark Now that most of the logic for our local get_upstream_branch has been pushed into the generic branch_get_upstream, we can fold the remainder into interpret_upstream_mark. Furthermore, what remains is generic to any branch-related "@{foo}" we might add in the future, and there's enough boilerplate that we'd like to reuse it. Let's parameterize the two operations (parsing the mark and computing its value) so that we can reuse this for "@{push}" in the near future. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- sha1_name.c | 44 +++++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 1005f45d75..506e0c92ee 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -1061,35 +1061,36 @@ static void set_shortened_ref(struct strbuf *buf, const char *ref) free(s); } -static const char *get_upstream_branch(const char *branch_buf, int len) -{ - char *branch = xstrndup(branch_buf, len); - struct branch *upstream = branch_get(*branch ? branch : NULL); - struct strbuf err = STRBUF_INIT; - const char *ret; - - free(branch); - - ret = branch_get_upstream(upstream, &err); - if (!ret) - die("%s", err.buf); - - return ret; -} - -static int interpret_upstream_mark(const char *name, int namelen, - int at, struct strbuf *buf) +static int interpret_branch_mark(const char *name, int namelen, + int at, struct strbuf *buf, + int (*get_mark)(const char *, int), + const char *(*get_data)(struct branch *, + struct strbuf *)) { int len; + struct branch *branch; + struct strbuf err = STRBUF_INIT; + const char *value; - len = upstream_mark(name + at, namelen - at); + len = get_mark(name + at, namelen - at); if (!len) return -1; if (memchr(name, ':', at)) return -1; - set_shortened_ref(buf, get_upstream_branch(name, at)); + if (at) { + char *name_str = xmemdupz(name, at); + branch = branch_get(name_str); + free(name_str); + } else + branch = branch_get(NULL); + + value = get_data(branch, &err); + if (!value) + die("%s", err.buf); + + set_shortened_ref(buf, value); return len + at; } @@ -1140,7 +1141,8 @@ int interpret_branch_name(const char *name, int namelen, struct strbuf *buf) if (len > 0) return reinterpret(name, namelen, len, buf); - len = interpret_upstream_mark(name, namelen, at - name, buf); + len = interpret_branch_mark(name, namelen, at - name, buf, + upstream_mark, branch_get_upstream); if (len > 0) return len; } From adfe5d04345631299f9a4518d56c6dd3d47eb0b3 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 May 2015 00:45:47 -0400 Subject: [PATCH 14/16] sha1_name: implement @{push} shorthand In a triangular workflow, each branch may have two distinct points of interest: the @{upstream} that you normally pull from, and the destination that you normally push to. There isn't a shorthand for the latter, but it's useful to have. For instance, you may want to know which commits you haven't pushed yet: git log @{push}.. Or as a more complicated example, imagine that you normally pull changes from origin/master (which you set as your @{upstream}), and push changes to your own personal fork (e.g., as myfork/topic). You may push to your fork from multiple machines, requiring you to integrate the changes from the push destination, rather than upstream. With this patch, you can just do: git rebase @{push} rather than typing out the full name. The heavy lifting is all done by branch_get_push; here we just wire it up to the "@{push}" syntax. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/revisions.txt | 25 +++++++++++++++ sha1_name.c | 14 ++++++++- t/t1514-rev-parse-push.sh | 63 +++++++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 1 deletion(-) create mode 100755 t/t1514-rev-parse-push.sh diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt index 07961185fe..d85e303364 100644 --- a/Documentation/revisions.txt +++ b/Documentation/revisions.txt @@ -98,6 +98,31 @@ some output processing may assume ref names in UTF-8. `branch..merge`). A missing branchname defaults to the current one. +'@\{push\}', e.g. 'master@\{push\}', '@\{push\}':: + The suffix '@\{push}' reports the branch "where we would push to" if + `git push` were run while `branchname` was checked out (or the current + 'HEAD' if no branchname is specified). Since our push destination is + in a remote repository, of course, we report the local tracking branch + that corresponds to that branch (i.e., something in 'refs/remotes/'). ++ +Here's an example to make it more clear: ++ +------------------------------ +$ git config push.default current +$ git config remote.pushdefault myfork +$ git checkout -b mybranch origin/master + +$ git rev-parse --symbolic-full-name @{upstream} +refs/remotes/origin/master + +$ git rev-parse --symbolic-full-name @{push} +refs/remotes/myfork/mybranch +------------------------------ ++ +Note in the example that we set up a triangular workflow, where we pull +from one location and push to another. In a non-triangular workflow, +'@\{push}' is the same as '@\{upstream}', and there is no need for it. + '{caret}', e.g. 'HEAD{caret}, v1.5.1{caret}0':: A suffix '{caret}' to a revision parameter means the first parent of that commit object. '{caret}' means the th parent (i.e. diff --git a/sha1_name.c b/sha1_name.c index 506e0c92ee..10969435bb 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -435,6 +435,12 @@ static inline int upstream_mark(const char *string, int len) return at_mark(string, len, suffix, ARRAY_SIZE(suffix)); } +static inline int push_mark(const char *string, int len) +{ + const char *suffix[] = { "@{push}" }; + return at_mark(string, len, suffix, ARRAY_SIZE(suffix)); +} + static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned lookup_flags); static int interpret_nth_prior_checkout(const char *name, int namelen, struct strbuf *buf); @@ -482,7 +488,8 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1, nth_prior = 1; continue; } - if (!upstream_mark(str + at, len - at)) { + if (!upstream_mark(str + at, len - at) && + !push_mark(str + at, len - at)) { reflog_len = (len-1) - (at+2); len = at; } @@ -1145,6 +1152,11 @@ int interpret_branch_name(const char *name, int namelen, struct strbuf *buf) upstream_mark, branch_get_upstream); if (len > 0) return len; + + len = interpret_branch_mark(name, namelen, at - name, buf, + push_mark, branch_get_push); + if (len > 0) + return len; } return -1; diff --git a/t/t1514-rev-parse-push.sh b/t/t1514-rev-parse-push.sh new file mode 100755 index 0000000000..7214f5b33f --- /dev/null +++ b/t/t1514-rev-parse-push.sh @@ -0,0 +1,63 @@ +#!/bin/sh + +test_description='test @{push} syntax' +. ./test-lib.sh + +resolve () { + echo "$2" >expect && + git rev-parse --symbolic-full-name "$1" >actual && + test_cmp expect actual +} + +test_expect_success 'setup' ' + git init --bare parent.git && + git init --bare other.git && + git remote add origin parent.git && + git remote add other other.git && + test_commit base && + git push origin HEAD && + git branch --set-upstream-to=origin/master master && + git branch --track topic origin/master && + git push origin topic && + git push other topic +' + +test_expect_success '@{push} with default=nothing' ' + test_config push.default nothing && + test_must_fail git rev-parse master@{push} +' + +test_expect_success '@{push} with default=simple' ' + test_config push.default simple && + resolve master@{push} refs/remotes/origin/master +' + +test_expect_success 'triangular @{push} fails with default=simple' ' + test_config push.default simple && + test_must_fail git rev-parse topic@{push} +' + +test_expect_success '@{push} with default=current' ' + test_config push.default current && + resolve topic@{push} refs/remotes/origin/topic +' + +test_expect_success '@{push} with default=matching' ' + test_config push.default matching && + resolve topic@{push} refs/remotes/origin/topic +' + +test_expect_success '@{push} with pushremote defined' ' + test_config push.default current && + test_config branch.topic.pushremote other && + resolve topic@{push} refs/remotes/other/topic +' + +test_expect_success '@{push} with push refspecs' ' + test_config push.default nothing && + test_config remote.origin.push refs/heads/*:refs/heads/magic/* && + git push && + resolve topic@{push} refs/remotes/origin/magic/topic +' + +test_done From 3dbe9db01bd9c0b0701f72a631ac15b1791f6642 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 May 2015 00:45:51 -0400 Subject: [PATCH 15/16] for-each-ref: use skip_prefix instead of starts_with This saves us having to maintain a magic number to skip past the matched prefix. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/for-each-ref.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 92bd2b2665..2bd19caa9c 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -659,10 +659,12 @@ static void populate_value(struct refinfo *ref) else if (starts_with(name, "symref")) refname = ref->symref ? ref->symref : ""; else if (starts_with(name, "upstream")) { + const char *branch_name; /* only local branches may have an upstream */ - if (!starts_with(ref->refname, "refs/heads/")) + if (!skip_prefix(ref->refname, "refs/heads/", + &branch_name)) continue; - branch = branch_get(ref->refname + 11); + branch = branch_get(branch_name); refname = branch_get_upstream(branch, NULL); if (!refname) From 29bc88505f22068d7ee6694240e6b13fddb5d059 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 May 2015 00:45:55 -0400 Subject: [PATCH 16/16] for-each-ref: accept "%(push)" format Just as we have "%(upstream)" to report the "@{upstream}" for each ref, this patch adds "%(push)" to match "@{push}". It supports the same tracking format modifiers as upstream (because you may want to know, for example, which branches have commits to push). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/git-for-each-ref.txt | 6 ++++++ builtin/for-each-ref.c | 17 +++++++++++++++-- t/t6300-for-each-ref.sh | 13 ++++++++++++- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 42408752d0..7f8d9a5b5f 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -97,6 +97,12 @@ upstream:: or "=" (in sync). Has no effect if the ref does not have tracking information associated with it. +push:: + The name of a local ref which represents the `@{push}` location + for the displayed ref. Respects `:short`, `:track`, and + `:trackshort` options as `upstream` does. Produces an empty + string if no `@{push}` ref is configured. + HEAD:: '*' if HEAD matches current ref (the checked out branch), ' ' otherwise. diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 2bd19caa9c..05dd23d2a3 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -74,6 +74,7 @@ static struct { { "contents:body" }, { "contents:signature" }, { "upstream" }, + { "push" }, { "symref" }, { "flag" }, { "HEAD" }, @@ -669,6 +670,16 @@ static void populate_value(struct refinfo *ref) refname = branch_get_upstream(branch, NULL); if (!refname) continue; + } else if (starts_with(name, "push")) { + const char *branch_name; + if (!skip_prefix(ref->refname, "refs/heads/", + &branch_name)) + continue; + branch = branch_get(branch_name); + + refname = branch_get_push(branch, NULL); + if (!refname) + continue; } else if (starts_with(name, "color:")) { char color[COLOR_MAXLEN] = ""; @@ -714,7 +725,8 @@ static void populate_value(struct refinfo *ref) refname = shorten_unambiguous_ref(refname, warn_ambiguous_refs); else if (!strcmp(formatp, "track") && - starts_with(name, "upstream")) { + (starts_with(name, "upstream") || + starts_with(name, "push"))) { char buf[40]; if (stat_tracking_info(branch, &num_ours, @@ -736,7 +748,8 @@ static void populate_value(struct refinfo *ref) } continue; } else if (!strcmp(formatp, "trackshort") && - starts_with(name, "upstream")) { + (starts_with(name, "upstream") || + starts_with(name, "push"))) { assert(branch); if (stat_tracking_info(branch, &num_ours, diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index c66bf7981c..24fc2ba55d 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -28,7 +28,10 @@ test_expect_success setup ' git update-ref refs/remotes/origin/master master && git remote add origin nowhere && git config branch.master.remote origin && - git config branch.master.merge refs/heads/master + git config branch.master.merge refs/heads/master && + git remote add myfork elsewhere && + git config remote.pushdefault myfork && + git config push.default current ' test_atom() { @@ -47,6 +50,7 @@ test_atom() { test_atom head refname refs/heads/master test_atom head upstream refs/remotes/origin/master +test_atom head push refs/remotes/myfork/master test_atom head objecttype commit test_atom head objectsize 171 test_atom head objectname $(git rev-parse refs/heads/master) @@ -83,6 +87,7 @@ test_atom head HEAD '*' test_atom tag refname refs/tags/testtag test_atom tag upstream '' +test_atom tag push '' test_atom tag objecttype tag test_atom tag objectsize 154 test_atom tag objectname $(git rev-parse refs/tags/testtag) @@ -347,6 +352,12 @@ test_expect_success 'Check that :track[short] works when upstream is invalid' ' test_cmp expected actual ' +test_expect_success '%(push) supports tracking specifiers, too' ' + echo "[ahead 1]" >expected && + git for-each-ref --format="%(push:track)" refs/heads >actual && + test_cmp expected actual +' + cat >expected <