diff mbox series

[v2,08/23] pack-bitmap: make `bitmap_writer_push_bitmapped_commit()` public

Message ID dfd4b73d12edc1833e5090c956cda6f28046c9e4.1714422410.git.me@ttaylorr.com (mailing list archive)
State Superseded
Headers show
Series pack-bitmap: pseudo-merge reachability bitmaps | expand

Commit Message

Taylor Blau April 29, 2024, 8:43 p.m. UTC
The pseudo-merge selection code will be added in a subsequent commit,
and will need a way to push the allocated commit structures into the
bitmap writer from a separate compilation unit.

Make the `bitmap_writer_push_bitmapped_commit()` function part of the
pack-bitmap.h header in order to make this possible.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 pack-bitmap-write.c | 4 ++--
 pack-bitmap.h       | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

Comments

Jeff King May 13, 2024, 6:50 p.m. UTC | #1
On Mon, Apr 29, 2024 at 04:43:22PM -0400, Taylor Blau wrote:

> The pseudo-merge selection code will be added in a subsequent commit,
> and will need a way to push the allocated commit structures into the
> bitmap writer from a separate compilation unit.
> 
> Make the `bitmap_writer_push_bitmapped_commit()` function part of the
> pack-bitmap.h header in order to make this possible.

I was a little surprised that this function and the one in the previous
commit needed to be public, since this whole topic is restricted to
writing, which is mostly contained to pack-bitmap-write.c. But you've
pulled the pseudo-merge bits out to pseudo-merge.[ch], and they need
access, which makes sense.

One could argue that it could all get stuffed into pack-bitmap-write.c,
but that is already getting to be a pretty large and complex file. So
this is probably the best route.

-Peff
Taylor Blau May 14, 2024, 12:54 a.m. UTC | #2
On Mon, May 13, 2024 at 02:50:55PM -0400, Jeff King wrote:
> On Mon, Apr 29, 2024 at 04:43:22PM -0400, Taylor Blau wrote:
>
> > The pseudo-merge selection code will be added in a subsequent commit,
> > and will need a way to push the allocated commit structures into the
> > bitmap writer from a separate compilation unit.
> >
> > Make the `bitmap_writer_push_bitmapped_commit()` function part of the
> > pack-bitmap.h header in order to make this possible.
>
> I was a little surprised that this function and the one in the previous
> commit needed to be public, since this whole topic is restricted to
> writing, which is mostly contained to pack-bitmap-write.c. But you've
> pulled the pseudo-merge bits out to pseudo-merge.[ch], and they need
> access, which makes sense.
>
> One could argue that it could all get stuffed into pack-bitmap-write.c,
> but that is already getting to be a pretty large and complex file. So
> this is probably the best route.

I had originally written the series like that, but the new bits nearly
doubled the line count of pack-bitmap-write.c:

    $ wc -l pack-bitmap-write.c pseudo-merge.c
     1031 pack-bitmap-write.c
      752 pseudo-merge.c
     1783 total

so I ended up splitting it out into pseudo-merge.ch in the end.

Thanks,
Taylor
diff mbox series

Patch

diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index c7514a58407..dab5bdea806 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -132,8 +132,8 @@  int bitmap_writer_has_bitmapped_object_id(const struct object_id *oid)
  * Compute the actual bitmaps
  */
 
-static void bitmap_writer_push_bitmapped_commit(struct commit *commit,
-						unsigned pseudo_merge)
+void bitmap_writer_push_bitmapped_commit(struct commit *commit,
+					 unsigned pseudo_merge)
 {
 	if (writer.selected_nr >= writer.selected_alloc) {
 		writer.selected_alloc = (writer.selected_alloc + 32) * 2;
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 995d664cc89..0f539d79cfd 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -99,6 +99,8 @@  int bitmap_has_oid_in_uninteresting(struct bitmap_index *, const struct object_i
 off_t get_disk_usage_from_bitmap(struct bitmap_index *, struct rev_info *);
 
 int bitmap_writer_has_bitmapped_object_id(const struct object_id *oid);
+void bitmap_writer_push_bitmapped_commit(struct commit *commit,
+					 unsigned pseudo_merge);
 
 void bitmap_writer_init(struct repository *r);
 void bitmap_writer_show_progress(int show);