| Message ID | c7e0ee0712034f654f018361f52c09b1043a8441.1723743050.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 25b78668de62ac2f503ee5957e174474bd51ae6c |
| Headers | show |
| Series | pseudo-merge: avoid empty and non-closed pseudo-merge commits | expand |
On Thu, Aug 15, 2024 at 01:31:17PM -0400, Taylor Blau wrote: > The previous commit demonstrated it is possible to generate empty > pseudo-merge commits, which is not useful as such pseudo-merges carry no > information. > > Ensure that we only generate non-empty groups by not pushing a new > commit onto the bitmap_writer when that commit has no parents. Makes sense, and the patch seems pretty straight-forward. It's a little tempting to suggest that we keep some way of writing these, so that we can exercise the reading side in the tests (we _shouldn't_ see these if we're not writing them, but as an on-disk format, we may see examples from other sources). But it is probably not worth the complexity. If we really want to test them, we can probably generate a single example by hand (and we can probably even wait on that until we see something in the wild that makes it worth doing). -Peff
On Sat, Aug 17, 2024 at 06:38:22AM -0400, Jeff King wrote: > [...] If we really want to test them, we can probably generate a > single example by hand (and we can probably even wait on that until we > see something in the wild that makes it worth doing). Yeah, I had a similar thought as you. I considered adding a test fixture that has a .bitmap with a pseudo-merge bitmap containing zero parents, but figured that it was probably overkill since this feature is still considered experimental. Thanks, Taylor
diff --git a/pseudo-merge.c b/pseudo-merge.c index f0fde13c47..6422be979c 100644 --- a/pseudo-merge.c +++ b/pseudo-merge.c @@ -357,8 +357,10 @@ static void select_pseudo_merges_1(struct bitmap_writer *writer, p = commit_list_append(c, p); } while (j % group->stable_size); - bitmap_writer_push_commit(writer, merge, 1); - writer->pseudo_merges_nr++; + if (merge->parents) { + bitmap_writer_push_commit(writer, merge, 1); + writer->pseudo_merges_nr++; + } } /* make up to group->max_merges pseudo merges for unstable commits */ @@ -398,8 +400,9 @@ static void select_pseudo_merges_1(struct bitmap_writer *writer, p = commit_list_append(c, p); } - bitmap_writer_push_commit(writer, merge, 1); - writer->pseudo_merges_nr++; + if (merge->parents) { + bitmap_writer_push_commit(writer, merge, 1); + writer->pseudo_merges_nr++; } if (end >= matches->unstable_nr) break; } diff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh index 0288691340..aa1a7d26f1 100755 --- a/t/t5333-pseudo-merge-bitmaps.sh +++ b/t/t5333-pseudo-merge-bitmaps.sh @@ -390,7 +390,7 @@ test_expect_success 'pseudo-merge reuse' ' ) ' -test_expect_failure 'empty pseudo-merge group' ' +test_expect_success 'empty pseudo-merge group' ' git init pseudo-merge-empty-group && ( cd pseudo-merge-empty-group &&
The previous commit demonstrated it is possible to generate empty pseudo-merge commits, which is not useful as such pseudo-merges carry no information. Ensure that we only generate non-empty groups by not pushing a new commit onto the bitmap_writer when that commit has no parents. Signed-off-by: Taylor Blau <me@ttaylorr.com> --- pseudo-merge.c | 11 +++++++---- t/t5333-pseudo-merge-bitmaps.sh | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-)