Thomas Rast <trast@xxxxxxxxxxxxxxx> writes:
>> > +test_expect_success checkout '
>> > + git checkout -b new_branch :/first
>> > +'
>> > +
>> > +test_done
>>
>> ...it looks like this can just be added to the end of
>> t2018-checkout-branch.sh instead of creating a new test. Creating a
>> new file just for a single test for such a simple feature is a bit of
>> an overkill.
>
> It should also use test_expect_failure unless you expect to have a fix
> soon, otherwise it would stop the test suite from running through.
I think this is an ancient bug from the very beginning of the ":/"
notation. Because it temporarily uses one bit in the object flags, but
the get_sha1_oneline() parser can be called multiple times, it tries to
clear the flag bits it smudged at the end, but fails.
The basic pattern for traversing the ancestry while avoiding duplication
is:
prepare commits you traverse from in "list";
while (list) {
commit = pop_most_recent_commit(&list, MARK);
try to use commit;
}
pop_most_recent_commit() removes a commit from the list, pushes parents of
the commit that are _not_ marked with any of the bits in the MARK to the
given list, and returns that commit.
While introducing the feature ':/', however, 28a4d94 (object name:
introduce ':/<oneline prefix>' notation, 2007-02-24) wanted to avoid
leaving the mark used in the loop before leaving the function (which is a
right thing to do), by doing this:
prepare commits you traverse from in "list";
+ keep a copy of "list" in "backup";
while (list) {
commit = pop_most_recent_commit(&list, MARK);
try to use commit;
}
+ for (commit in backup)
+ clear_commit_marks(commit, MARK);
which is wrong, as clear_commit_marks() will ignore if the starting commit
does not have the MARK.
I think the fix should be just as simple as this. Essentially, X_SEEN
mark means "we have marked and placed this item on the list (so do not put
it again back on the list when it appears as a parent of some other item)"
so marking commits we initially put on the list with the bit should be the
right thing to do regardless of this clean-up business.
sha1_name.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index 4f2af8d..b935688 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -704,8 +704,10 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
die("Invalid search pattern: %s", prefix);
for_each_ref(handle_one_ref, &list);
- for (l = list; l; l = l->next)
+ for (l = list; l; l = l->next) {
commit_list_insert(l->item, &backup);
+ l->item->object.flags |= ONELINE_SEEN;
+ }
while (list) {
char *p;
struct commit *commit;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html