revision: do not peel tags used in range notation

A range notation "A..B" means exactly the same thing as what "^A B" means, i.e. the set of commits that are reachable from B but not from A. But the internal representation after the revision parser parsed these two notations are subtly different. - "rev-list ^A B" leaves A and B in the revs->pending.objects[] array, with the former marked as UNINTERESTING and the revision traversal machinery propagates the mark to underlying commit objects A^0 and B^0. - "rev-list A..B" peels tags and leaves A^0 (marked as UNINTERESTING) and B^0 in revs->pending.objects[] array before the traversal machinery kicks in. This difference usually does not matter, but starts to matter when the --objects option is used. For example, we see this: $ git rev-list --objects v1.8.4^1..v1.8.4 | grep $(git rev-parse v1.8.4) $ git rev-list --objects v1.8.4 ^v1.8.4^1 | grep $(git rev-parse v1.8.4) 04f013dc38d7512eadb915eba22efc414f18b869 v1.8.4 With the former invocation, the revision traversal machinery never hears about the tag v1.8.4 (it only sees the result of peeling it, i.e. the commit v1.8.4^0), and the tag itself does not appear in the output. The latter does send the tag object itself to the output. Make the range notation keep the unpeeled objects and feed them to the traversal machinery to fix this inconsistency. Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Sep 19, 2013 at 14:20 UTC 895c5ba3c1c927c68e581ebb0e13d9a123e477ab
2 files changed +45 -22
revision.c
+37 -22
@@ -1157,41 +1157,56 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
1157 }
1158 if (!get_sha1_committish(this, from_sha1) &&
1159 !get_sha1_committish(next, sha1)) {
1160 - struct commit *a, *b;
1161 - struct commit_list *exclude;
1162 -
1163 - a = lookup_commit_reference(from_sha1);
1164 - b = lookup_commit_reference(sha1);
1165 - if (!a || !b) {
1166 - if (revs->ignore_missing)
1167 - return 0;
1168 - die(symmetric ?
1169 - "Invalid symmetric difference expression %s...%s" :
1170 - "Invalid revision range %s..%s",
1171 - arg, next);
1172 - }
1160 + struct object *a_obj, *b_obj;
1161
1162 if (!cant_be_filename) {
1163 *dotdot = '.';
1164 verify_non_filename(revs->prefix, arg);
1165 }
1166
1179 - if (symmetric) {
1167 + a_obj = parse_object(from_sha1);
1168 + b_obj = parse_object(sha1);
1169 + if (!a_obj || !b_obj) {
1170 + missing:
1171 + if (revs->ignore_missing)
1172 + return 0;
1173 + die(symmetric
1174 + ? "Invalid symmetric difference expression %s"
1175 + : "Invalid revision range %s", arg);
1176 + }
1177 +
1178 + if (!symmetric) {
1179 + /* just A..B */
1180 + a_flags = flags_exclude;
1181 + } else {
1182 + /* A...B -- find merge bases between the two */
1183 + struct commit *a, *b;
1184 + struct commit_list *exclude;
1185 +
1186 + a = (a_obj->type == OBJ_COMMIT
1187 + ? (struct commit *)a_obj
1188 + : lookup_commit_reference(a_obj->sha1));
1189 + b = (b_obj->type == OBJ_COMMIT
1190 + ? (struct commit *)b_obj
1191 + : lookup_commit_reference(b_obj->sha1));
1192 + if (!a || !b)
1193 + goto missing;
1194 exclude = get_merge_bases(a, b, 1);
1195 add_pending_commit_list(revs, exclude,
1196 flags_exclude);
1197 free_commit_list(exclude);
1198 +
1199 a_flags = flags | SYMMETRIC_LEFT;
1185 - } else
1186 - a_flags = flags_exclude;
1187 - a->object.flags |= a_flags;
1188 - b->object.flags |= flags;
1189 - add_rev_cmdline(revs, &a->object, this,
1200 + }
1201 +
1202 + a_obj->flags |= a_flags;
1203 + b_obj->flags |= flags;
1204 + add_rev_cmdline(revs, a_obj, this,
1205 REV_CMD_LEFT, a_flags);
1191 - add_rev_cmdline(revs, &b->object, next,
1206 + add_rev_cmdline(revs, b_obj, next,
1207 REV_CMD_RIGHT, flags);
1193 - add_pending_object(revs, &a->object, this);
1194 - add_pending_object(revs, &b->object, next);
1208 + add_pending_object(revs, a_obj, this);
1209 + add_pending_object(revs, b_obj, next);
1210 return 0;
1211 }
1212 *dotdot = '.';
t/t6000-rev-list-misc.sh
+8
@@ -48,4 +48,12 @@ test_expect_success 'rev-list --objects with pathspecs and copied files' '
48 ! grep one output
49 '
50
51 +test_expect_success 'rev-list A..B and rev-list ^A B are the same' '
52 + git commit --allow-empty -m another &&
53 + git tag -a -m "annotated" v1.0 &&
54 + git rev-list --objects ^v1.0^ v1.0 >expect &&
55 + git rev-list --objects v1.0^..v1.0 >actual &&
56 + test_cmp expect actual
57 +'
58 +
59 test_done