handle_revision_arg: reset "dotdot" consistently

When we are parsing a range like "a..b", we write a temporary NUL over the first ".", so that we can access the names "a" and "b" as C strings. But our restoration of the original "." is done at inconsistent times, which can lead to confusing results. For most calls, we restore the "." after we resolve the names, but before we call verify_non_filename(). This means that when we later call add_pending_object(), the name for the left-hand "a" has been re-expanded to "a..b". You can see this with: git log --source a...b where "b" will be correctly marked with "b", but "a" will be marked with "a...b". Likewise with "a..b" (though you need to use --boundary to even see "a" at all in that case). To top off the confusion, when the REVARG_CANNOT_BE_FILENAME flag is set, we skip the non-filename check, and leave the NUL in place. That means we do report the correct name for "a" in the pending array. But some code paths try to show the whole "a...b" name in error messages, and these erroneously show only "a" instead of "a...b". E.g.: $ git cherry-pick HEAD:foo...HEAD:foo error: object d95f3ad14dee633a758d2e331151e950dd13e4ed is a blob, not a commit error: object d95f3ad14dee633a758d2e331151e950dd13e4ed is a blob, not a commit fatal: Invalid symmetric difference expression HEAD:foo (That last message should be "HEAD:foo...HEAD:foo"; I used cherry-pick because it passes the CANNOT_BE_FILENAME flag). As an interesting side note, cherry-pick actually looks at and re-resolves the arguments from the pending->name fields. So it would have been visibly broken by the first bug, but the effect was canceled out by the second one. This patch makes the whole function consistent by re-writing the NUL immediately after calling verify_non_filename(), and then restoring the "." as appropriate in some error-printing and early-return code paths. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed May 23, 2017 at 15:51 UTC ed79b2cf034b81473dd1fa9648593b245c07daea
2 files changed +12
revision.c
+3
@@ -1477,12 +1477,14 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
1477 if (!cant_be_filename) {
1478 *dotdot = '.';
1479 verify_non_filename(revs->prefix, arg);
1480 + *dotdot = '\0';
1481 }
1482
1483 a_obj = parse_object(from_sha1);
1484 b_obj = parse_object(sha1);
1485 if (!a_obj || !b_obj) {
1486 missing:
1487 + *dotdot = '.';
1488 if (revs->ignore_missing)
1489 return 0;
1490 die(symmetric
@@ -1525,6 +1527,7 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
1527 REV_CMD_RIGHT, flags);
1528 add_pending_object(revs, a_obj, this);
1529 add_pending_object(revs, b_obj, next);
1530 + *dotdot = '.';
1531 return 0;
1532 }
1533 *dotdot = '.';
t/t4202-log.sh
+9
@@ -1380,4 +1380,13 @@ test_expect_success 'log --source paints tag names' '
1380 test_cmp expect actual
1381 '
1382
1383 +test_expect_success 'log --source paints symmetric ranges' '
1384 + cat >expect <<-\EOF &&
1385 + 09e12a9 source-b three
1386 + 8e393e1 source-a two
1387 + EOF
1388 + git log --oneline --source source-a...source-b >actual &&
1389 + test_cmp expect actual
1390 +'
1391 +
1392 test_done