@matheusbsbs / matheus-bagliano / commits / 0e9f62dab9

interpret_branch_name: allow callers to restrict expansions

The interpret_branch_name() function converts names like @{-1} and @{upstream} into branch names. The expanded ref names are not fully qualified, and may be outside of the refs/heads/ namespace (e.g., "@" expands to "HEAD", and "@{upstream}" is likely to be in "refs/remotes/"). This is OK for callers like dwim_ref() which are primarily interested in resolving the resulting name, no matter where it is. But callers like "git branch" treat the result as a branch name in refs/heads/. When we expand to a ref outside that namespace, the results are very confusing (e.g., "git branch @" tries to create refs/heads/HEAD, which is nonsense). Callers can't know from the returned string how the expansion happened (e.g., did the user really ask for a branch named "HEAD", or did we do a bogus expansion?). One fix would be to return some out-parameters describing the types of expansion that occurred. This has the benefit that the caller can generate precise error messages ("I understood @{upstream} to mean origin/master, but that is a remote tracking branch, so you cannot create it as a local name"). However, out-parameters make the function interface somewhat cumbersome. Instead, let's do the opposite: let the caller tell us which elements to expand. That's easier to pass in, and none of the callers give more precise error messages than "@{upstream} isn't a valid branch name" anyway (which should be sufficient). The strbuf_branchname() function needs a similar parameter, as most of the callers access interpret_branch_name() through it. We can break the callers down into two groups: 1. Callers that are happy with any kind of ref in the result. We pass "0" here, so they continue to work without restrictions. This includes merge_name(), the reflog handling in add_pending_object_with_path(), and substitute_branch_name(). This last is what powers dwim_ref(). 2. Callers that have funny corner cases (mostly in git-branch and git-checkout). These need to make use of the new parameter, but I've left them as "0" in this patch, and will address them individually in follow-on patches. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Mar 2, 2017 at 03:23 UTC 0e9f62dab9fcce57279751bba47718d3f8baf3c8

8 files changed +69 -28

builtin/branch.c

+1 -1

@@ -215,7 +215,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,

215 char *target = NULL;

216 int flags = 0;

217

218 - strbuf_branchname(&bname, argv[i]);

218 + strbuf_branchname(&bname, argv[i], 0);

219 free(name);

220 name = mkpathdup(fmt, bname.buf);

221

builtin/checkout.c

+1 -1

@@ -452,7 +452,7 @@ static void setup_branch_path(struct branch_info *branch)

452 {

453 struct strbuf buf = STRBUF_INIT;

454

455 - strbuf_branchname(&buf, branch->name);

455 + strbuf_branchname(&buf, branch->name, 0);

456 if (strcmp(buf.buf, branch->name))

457 branch->name = xstrdup(buf.buf);

458 strbuf_splice(&buf, 0, 0, "refs/heads/", 11);

builtin/merge.c

+1 -1

@@ -434,7 +434,7 @@ static void merge_name(const char *remote, struct strbuf *msg)

434 char *found_ref;

435 int len, early;

436

437 - strbuf_branchname(&bname, remote);

437 + strbuf_branchname(&bname, remote, 0);

438 remote = bname.buf;

439

440 memset(branch_head, 0, sizeof(branch_head));

cache.h

+11 -2

@@ -1312,8 +1312,17 @@ extern char *oid_to_hex(const struct object_id *oid); /* same static buffer as s

1312 *

1313 * If the input was ok but there are not N branch switches in the

1314 * reflog, it returns 0.

1315 - */

1316 -extern int interpret_branch_name(const char *str, int len, struct strbuf *);

1315 + *

1316 + * If "allowed" is non-zero, it is a treated as a bitfield of allowable

1317 + * expansions: local branches ("refs/heads/"), remote branches

1318 + * ("refs/remotes/"), or "HEAD". If no "allowed" bits are set, any expansion is

1319 + * allowed, even ones to refs outside of those namespaces.

1320 + */

1321 +#define INTERPRET_BRANCH_LOCAL (1<<0)

1322 +#define INTERPRET_BRANCH_REMOTE (1<<1)

1323 +#define INTERPRET_BRANCH_HEAD (1<<2)

1324 +extern int interpret_branch_name(const char *str, int len, struct strbuf *,

1325 + unsigned allowed);

1326 extern int get_oid_mb(const char *str, struct object_id *oid);

1327

1328 extern int validate_headref(const char *ref);

refs.c

+1 -1

@@ -404,7 +404,7 @@ int refname_match(const char *abbrev_name, const char *full_name)

404 static char *substitute_branch_name(const char **string, int *len)

405 {

406 struct strbuf buf = STRBUF_INIT;

407 - int ret = interpret_branch_name(*string, *len, &buf);

407 + int ret = interpret_branch_name(*string, *len, &buf, 0);

408

409 if (ret == *len) {

410 size_t size;

revision.c

+1 -1

@@ -147,7 +147,7 @@ static void add_pending_object_with_path(struct rev_info *revs,

147 revs->no_walk = 0;

148 if (revs->reflog_info && obj->type == OBJ_COMMIT) {

149 struct strbuf buf = STRBUF_INIT;

150 - int len = interpret_branch_name(name, 0, &buf);

150 + int len = interpret_branch_name(name, 0, &buf, 0);

151 int st;

152

153 if (0 < len && name[len] && buf.len)

sha1_name.c

+48 -20

@@ -1176,7 +1176,8 @@ static int interpret_empty_at(const char *name, int namelen, int len, struct str

1176 return 1;

1177 }

1178

1179 -static int reinterpret(const char *name, int namelen, int len, struct strbuf *buf)

1179 +static int reinterpret(const char *name, int namelen, int len,

1180 + struct strbuf *buf, unsigned allowed)

1181 {

1182 /* we have extra data, which might need further processing */

1183 struct strbuf tmp = STRBUF_INIT;

@@ -1184,7 +1185,7 @@ static int reinterpret(const char *name, int namelen, int len, struct strbuf *bu

1185 int ret;

1186

1187 strbuf_add(buf, name + len, namelen - len);

1187 - ret = interpret_branch_name(buf->buf, buf->len, &tmp);

1188 + ret = interpret_branch_name(buf->buf, buf->len, &tmp, allowed);

1189 /* that data was not interpreted, remove our cruft */

1190 if (ret < 0) {

1191 strbuf_setlen(buf, used);

@@ -1205,11 +1206,27 @@ static void set_shortened_ref(struct strbuf *buf, const char *ref)

1206 free(s);

1207 }

1208

1209 +static int branch_interpret_allowed(const char *refname, unsigned allowed)

1210 +{

1211 + if (!allowed)

1212 + return 1;

1213 +

1214 + if ((allowed & INTERPRET_BRANCH_LOCAL) &&

1215 + starts_with(refname, "refs/heads/"))

1216 + return 1;

1217 + if ((allowed & INTERPRET_BRANCH_REMOTE) &&

1218 + starts_with(refname, "refs/remotes/"))

1219 + return 1;

1220 +

1221 + return 0;

1222 +}

1223 +

1224 static int interpret_branch_mark(const char *name, int namelen,

1225 int at, struct strbuf *buf,

1226 int (*get_mark)(const char *, int),

1227 const char *(*get_data)(struct branch *,

1212 - struct strbuf *))

1228 + struct strbuf *),

1229 + unsigned allowed)

1230 {

1231 int len;

1232 struct branch *branch;

@@ -1234,11 +1251,15 @@ static int interpret_branch_mark(const char *name, int namelen,

1251 if (!value)

1252 die("%s", err.buf);

1253

1254 + if (!branch_interpret_allowed(value, allowed))

1255 + return -1;

1256 +

1257 set_shortened_ref(buf, value);

1258 return len + at;

1259 }

1260

1241 -int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)

1261 +int interpret_branch_name(const char *name, int namelen, struct strbuf *buf,

1262 + unsigned allowed)

1263 {

1264 char *at;

1265 const char *start;

@@ -1247,31 +1268,38 @@ int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)

1268 if (!namelen)

1269 namelen = strlen(name);

1270

1250 - len = interpret_nth_prior_checkout(name, namelen, buf);

1251 - if (!len) {

1252 - return len; /* syntax Ok, not enough switches */

1253 - } else if (len > 0) {

1254 - if (len == namelen)

1255 - return len; /* consumed all */

1256 - else

1257 - return reinterpret(name, namelen, len, buf);

1271 + if (!allowed || (allowed & INTERPRET_BRANCH_LOCAL)) {

1272 + len = interpret_nth_prior_checkout(name, namelen, buf);

1273 + if (!len) {

1274 + return len; /* syntax Ok, not enough switches */

1275 + } else if (len > 0) {

1276 + if (len == namelen)

1277 + return len; /* consumed all */

1278 + else

1279 + return reinterpret(name, namelen, len, buf, allowed);

1280 + }

1281 }

1282

1283 for (start = name;

1284 (at = memchr(start, '@', namelen - (start - name)));

1285 start = at + 1) {

1286

1264 - len = interpret_empty_at(name, namelen, at - name, buf);

1265 - if (len > 0)

1266 - return reinterpret(name, namelen, len, buf);

1287 + if (!allowed || (allowed & INTERPRET_BRANCH_HEAD)) {

1288 + len = interpret_empty_at(name, namelen, at - name, buf);

1289 + if (len > 0)

1290 + return reinterpret(name, namelen, len, buf,

1291 + allowed);

1292 + }

1293

1294 len = interpret_branch_mark(name, namelen, at - name, buf,

1269 - upstream_mark, branch_get_upstream);

1295 + upstream_mark, branch_get_upstream,

1296 + allowed);

1297 if (len > 0)

1298 return len;

1299

1300 len = interpret_branch_mark(name, namelen, at - name, buf,

1274 - push_mark, branch_get_push);

1301 + push_mark, branch_get_push,

1302 + allowed);

1303 if (len > 0)

1304 return len;

1305 }

@@ -1279,10 +1307,10 @@ int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)

1307 return -1;

1308 }

1309

1282 -void strbuf_branchname(struct strbuf *sb, const char *name)

1310 +void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)

1311 {

1312 int len = strlen(name);

1285 - int used = interpret_branch_name(name, len, sb);

1313 + int used = interpret_branch_name(name, len, sb, allowed);

1314

1315 if (used < 0)

1316 used = 0;

@@ -1291,7 +1319,7 @@ void strbuf_branchname(struct strbuf *sb, const char *name)

1319

1320 int strbuf_check_branch_ref(struct strbuf *sb, const char *name)

1321 {

1294 - strbuf_branchname(sb, name);

1322 + strbuf_branchname(sb, name, 0);

1323 if (name[0] == '-')

1324 return -1;

1325 strbuf_splice(sb, 0, 0, "refs/heads/", 11);

strbuf.h

+5 -1

@@ -569,8 +569,12 @@ static inline void strbuf_complete_line(struct strbuf *sb)

569 * "refs/remotes/origin/master").

570 *

571 * Note that the resulting name may not be a syntactically valid refname.

572 + *

573 + * If "allowed" is non-zero, restrict the set of allowed expansions. See

574 + * interpret_branch_name() for details.

575 */

573 -extern void strbuf_branchname(struct strbuf *sb, const char *name);

576 +extern void strbuf_branchname(struct strbuf *sb, const char *name,

577 + unsigned allowed);

578

579 /*

580 * Like strbuf_branchname() above, but confirm that the result is