From 81f4953120f021832a0023dc704a0d7eb0ddf475 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 25 Aug 2011 12:49:13 -0400 Subject: [PATCH 1/4] rev-list: Demonstrate breakage with --ancestry-path --all The option added by commit ebdc94f3 (revision: --ancestry-path, 2010-04-20) does not work properly in combination with --all, at least in the case of a criss-cross merge: b---bc / \ / a X \ / \ c---cb There are no descendants of 'cb' in the history. The command git rev-list --ancestry-path cb..bc correctly reports no commits. However, the command git rev-list --ancestry-path --all ^cb reports 'bc'. Add a test case to t6019-rev-list-ancestry-path demonstrating this breakage. Signed-off-by: Brad King Signed-off-by: Junio C Hamano --- t/t6019-rev-list-ancestry-path.sh | 35 +++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/t/t6019-rev-list-ancestry-path.sh b/t/t6019-rev-list-ancestry-path.sh index 76410293b3..aa4674f1df 100755 --- a/t/t6019-rev-list-ancestry-path.sh +++ b/t/t6019-rev-list-ancestry-path.sh @@ -70,4 +70,39 @@ test_expect_success 'rev-list --ancestry-patch D..M -- M.t' ' test_cmp expect actual ' +# b---bc +# / \ / +# a X +# \ / \ +# c---cb +test_expect_success 'setup criss-cross' ' + mkdir criss-cross && + (cd criss-cross && + git init && + test_commit A && + git checkout -b b master && + test_commit B && + git checkout -b c master && + test_commit C && + git checkout -b bc b -- && + git merge c && + git checkout -b cb c -- && + git merge b && + git checkout master) +' + +# no commits in bc descend from cb +test_expect_success 'criss-cross: rev-list --ancestry-path cb..bc' ' + (cd criss-cross && + git rev-list --ancestry-path cb..bc > actual && + test -z "$(cat actual)") +' + +# no commits in repository descend from cb +test_expect_failure 'criss-cross: rev-list --ancestry-path --all ^cb' ' + (cd criss-cross && + git rev-list --ancestry-path --all ^cb > actual && + test -z "$(cat actual)") +' + test_done From 281eee473000e5bd46e59d636626132dc9ea85cb Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 25 Aug 2011 17:35:39 -0700 Subject: [PATCH 2/4] revision: keep track of the end-user input from the command line Given a complex set of revision specifiers on the command line, it is too late to look at the flags of the objects in the initial traversal list at the beginning of limit_list() in order to determine what the objects the end-user explicitly listed on the command line were. The process to move objects from the pending array to the traversal list may have marked objects that are not mentioned as UNINTERESTING, when handle_commit() marked the parents of UNINTERESTING commits mentioned on the command line by calling mark_parents_uninteresting(). This made "rev-list --ancestry-path ^A ..." to mistakenly list commits that are descendants of A's parents but that are not descendants of A itself, as ^A from the command line causes A and its parents marked as UNINTERESTING before coming to limit_list(), and we try to enumerate the commits that are descendants of these commits that are UNINTERESTING before we start walking the history. It actually is too late even if we inspected the pending object array before calling prepare_revision_walk(), as some of the same objects might have been mentioned twice, once as positive and another time as negative. The "rev-list --some-option A --not --all" command may want to notice, even if the resulting set is empty, that the user showed some interest in "A" and do something special about it. Prepare a separate array to keep track of what syntactic element was used to cause each object to appear in the pending array from the command line, and populate it as setup_revisions() parses the command line. Signed-off-by: Junio C Hamano --- revision.c | 37 +++++++++++++++++++++++++++++++++---- revision.h | 20 ++++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/revision.c b/revision.c index 96d7fa7f14..3e87c8677e 100644 --- a/revision.c +++ b/revision.c @@ -797,6 +797,23 @@ static int limit_list(struct rev_info *revs) return 0; } +static void add_rev_cmdline(struct rev_info *revs, + struct object *item, + const char *name, + int whence, + unsigned flags) +{ + struct rev_cmdline_info *info = &revs->cmdline; + int nr = info->nr; + + ALLOC_GROW(info->rev, nr + 1, info->alloc); + info->rev[nr].item = item; + info->rev[nr].name = name; + info->rev[nr].whence = whence; + info->rev[nr].flags = flags; + info->nr++; +} + struct all_refs_cb { int all_flags; int warned_bad_reflog; @@ -809,6 +826,7 @@ static int handle_one_ref(const char *path, const unsigned char *sha1, int flag, struct all_refs_cb *cb = cb_data; struct object *object = get_reference(cb->all_revs, path, sha1, cb->all_flags); + add_rev_cmdline(cb->all_revs, object, path, REV_CMD_REF, cb->all_flags); add_pending_object(cb->all_revs, object, path); return 0; } @@ -835,6 +853,7 @@ static void handle_one_reflog_commit(unsigned char *sha1, void *cb_data) struct object *o = parse_object(sha1); if (o) { o->flags |= cb->all_flags; + /* ??? CMDLINEFLAGS ??? */ add_pending_object(cb->all_revs, o, ""); } else if (!cb->warned_bad_reflog) { @@ -871,12 +890,13 @@ static void handle_reflog(struct rev_info *revs, unsigned flags) for_each_reflog(handle_one_reflog, &cb); } -static int add_parents_only(struct rev_info *revs, const char *arg, int flags) +static int add_parents_only(struct rev_info *revs, const char *arg_, int flags) { unsigned char sha1[20]; struct object *it; struct commit *commit; struct commit_list *parents; + const char *arg = arg_; if (*arg == '^') { flags ^= UNINTERESTING; @@ -898,6 +918,7 @@ static int add_parents_only(struct rev_info *revs, const char *arg, int flags) for (parents = commit->parents; parents; parents = parents->next) { it = &parents->item->object; it->flags |= flags; + add_rev_cmdline(revs, it, arg_, REV_CMD_PARENTS_ONLY, flags); add_pending_object(revs, it, arg); } return 1; @@ -987,7 +1008,7 @@ static void prepare_show_merge(struct rev_info *revs) revs->limited = 1; } -int handle_revision_arg(const char *arg, struct rev_info *revs, +int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, int cant_be_filename) { @@ -996,6 +1017,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs, struct object *object; unsigned char sha1[20]; int local_flags; + const char *arg = arg_; dotdot = strstr(arg, ".."); if (dotdot) { @@ -1004,6 +1026,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs, const char *this = arg; int symmetric = *next == '.'; unsigned int flags_exclude = flags ^ UNINTERESTING; + unsigned int a_flags; *dotdot = 0; next += symmetric; @@ -1036,10 +1059,15 @@ int handle_revision_arg(const char *arg, struct rev_info *revs, add_pending_commit_list(revs, exclude, flags_exclude); free_commit_list(exclude); - a->object.flags |= flags | SYMMETRIC_LEFT; + a_flags = flags | SYMMETRIC_LEFT; } else - a->object.flags |= flags_exclude; + a_flags = flags_exclude; + a->object.flags |= a_flags; b->object.flags |= flags; + add_rev_cmdline(revs, &a->object, this, + REV_CMD_LEFT, a_flags); + add_rev_cmdline(revs, &b->object, next, + REV_CMD_RIGHT, flags); add_pending_object(revs, &a->object, this); add_pending_object(revs, &b->object, next); return 0; @@ -1070,6 +1098,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs, if (!cant_be_filename) verify_non_filename(revs->prefix, arg); object = get_reference(revs, arg, sha1, flags ^ local_flags); + add_rev_cmdline(revs, object, arg_, REV_CMD_REV, flags ^ local_flags); add_pending_object_with_mode(revs, object, arg, mode); return 0; } diff --git a/revision.h b/revision.h index 855464f144..031cc7c81d 100644 --- a/revision.h +++ b/revision.h @@ -23,6 +23,23 @@ struct rev_info; struct log_info; struct string_list; +struct rev_cmdline_info { + unsigned int nr; + unsigned int alloc; + struct rev_cmdline_entry { + struct object *item; + const char *name; + enum { + REV_CMD_REF, + REV_CMD_PARENTS_ONLY, + REV_CMD_LEFT, + REV_CMD_RIGHT, + REV_CMD_REV + } whence; + unsigned flags; + } *rev; +}; + struct rev_info { /* Starting list */ struct commit_list *commits; @@ -31,6 +48,9 @@ struct rev_info { /* Parents of shown commits */ struct object_array boundary_commits; + /* The end-points specified by the end user */ + struct rev_cmdline_info cmdline; + /* Basic information */ const char *prefix; const char *def; From c3502fa8822fab99ee51b3f74cf3f100492d88e2 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 25 Aug 2011 17:51:50 -0700 Subject: [PATCH 3/4] revision: do not include sibling history in --ancestry-path output If the commit specified as the bottom of the commit range has a direct parent that has another child commit that contributed to the resulting history, "rev-list --ancestry-path" was confused and listed that side history as well, due to the command line parser subtlety corrected by the previous commit. Signed-off-by: Junio C Hamano --- revision.c | 16 ++++++++++------ t/t6019-rev-list-ancestry-path.sh | 2 +- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/revision.c b/revision.c index 3e87c8677e..48a2db439f 100644 --- a/revision.c +++ b/revision.c @@ -724,12 +724,16 @@ static void limit_to_ancestry(struct commit_list *bottom, struct commit_list *li * to filter the result of "A..B" further to the ones that can actually * reach A. */ -static struct commit_list *collect_bottom_commits(struct commit_list *list) +static struct commit_list *collect_bottom_commits(struct rev_info *revs) { - struct commit_list *elem, *bottom = NULL; - for (elem = list; elem; elem = elem->next) - if (elem->item->object.flags & UNINTERESTING) - commit_list_insert(elem->item, &bottom); + struct commit_list *bottom = NULL; + int i; + for (i = 0; i < revs->cmdline.nr; i++) { + struct rev_cmdline_entry *elem = &revs->cmdline.rev[i]; + if ((elem->flags & UNINTERESTING) && + elem->item->type == OBJ_COMMIT) + commit_list_insert((struct commit *)elem->item, &bottom); + } return bottom; } @@ -743,7 +747,7 @@ static int limit_list(struct rev_info *revs) struct commit_list *bottom = NULL; if (revs->ancestry_path) { - bottom = collect_bottom_commits(list); + bottom = collect_bottom_commits(revs); if (!bottom) die("--ancestry-path given but there are no bottom commits"); } diff --git a/t/t6019-rev-list-ancestry-path.sh b/t/t6019-rev-list-ancestry-path.sh index aa4674f1df..738af73393 100755 --- a/t/t6019-rev-list-ancestry-path.sh +++ b/t/t6019-rev-list-ancestry-path.sh @@ -99,7 +99,7 @@ test_expect_success 'criss-cross: rev-list --ancestry-path cb..bc' ' ' # no commits in repository descend from cb -test_expect_failure 'criss-cross: rev-list --ancestry-path --all ^cb' ' +test_expect_success 'criss-cross: rev-list --ancestry-path --all ^cb' ' (cd criss-cross && git rev-list --ancestry-path --all ^cb > actual && test -z "$(cat actual)") From c05b988a69c10d7eef2f34932eed61923c18b4a5 Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Thu, 15 Sep 2011 10:34:31 +0200 Subject: [PATCH 4/4] t6019: avoid refname collision on case-insensitive systems The criss-cross tests kept failing for me because of collisions of 'a' with 'A' etc. Prefix the lowercase refnames with an extra letter to disambiguate. Signed-off-by: Thomas Rast Acked-by: Brad King Signed-off-by: Junio C Hamano --- t/t6019-rev-list-ancestry-path.sh | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/t/t6019-rev-list-ancestry-path.sh b/t/t6019-rev-list-ancestry-path.sh index 738af73393..39b4cb0ecd 100755 --- a/t/t6019-rev-list-ancestry-path.sh +++ b/t/t6019-rev-list-ancestry-path.sh @@ -75,33 +75,36 @@ test_expect_success 'rev-list --ancestry-patch D..M -- M.t' ' # a X # \ / \ # c---cb +# +# All refnames prefixed with 'x' to avoid confusion with the tags +# generated by test_commit on case-insensitive systems. test_expect_success 'setup criss-cross' ' mkdir criss-cross && (cd criss-cross && git init && test_commit A && - git checkout -b b master && + git checkout -b xb master && test_commit B && - git checkout -b c master && + git checkout -b xc master && test_commit C && - git checkout -b bc b -- && - git merge c && - git checkout -b cb c -- && - git merge b && + git checkout -b xbc xb -- && + git merge xc && + git checkout -b xcb xc -- && + git merge xb && git checkout master) ' # no commits in bc descend from cb test_expect_success 'criss-cross: rev-list --ancestry-path cb..bc' ' (cd criss-cross && - git rev-list --ancestry-path cb..bc > actual && + git rev-list --ancestry-path xcb..xbc > actual && test -z "$(cat actual)") ' # no commits in repository descend from cb test_expect_success 'criss-cross: rev-list --ancestry-path --all ^cb' ' (cd criss-cross && - git rev-list --ancestry-path --all ^cb > actual && + git rev-list --ancestry-path --all ^xcb > actual && test -z "$(cat actual)") '