git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>,
	Elijah Newren <newren@gmail.com>
Subject: [PATCH 0/5] repack: introduce '--combine-cruft-below-size'
Date: Mon, 17 Mar 2025 19:00:12 -0400	[thread overview]
Message-ID: <cover.1742252411.git.me@ttaylorr.com> (raw)

(This is based on tb/multi-cruft-pack-refresh-fix from Junio's tree,
which is at 08f612ba70 (builtin/pack-objects.c: freshen objects from
existing cruft packs, 2025-03-13) at the time of writing).

This series replaces something close to the existing behavior of
repack's '--max-cruft-size' flag with '--combine-cruft-below-size'.

The new flag is much clearer in its intent and function, and avoids the
lack of clarity between the two that was discussed in

  <cover.1741648467.git.me@ttaylorr.com>

The new behavior is as follows:

  - '--max-cruft-size' is a cruft pack-specific override for repack's
    '--max-pack-size' command-line flag.

  - '--combine-cruft-below-size' instructs repack to only combine cruft
    packs which are smaller than the given threshold. This will likely
    result in packs which are larger than the threshold. But that is OK:
    the point is to limit the size of the individual packs on input, not
    the size of the outgoing pack.

This series does break the existing behavior of '--max-cruft-size'. But
I think breaking backwards compatibility here is OK, since the existing
behavior was so broken to begin with. I'm happy to consider other
alternatives if others feel that this isn't OK.

The series has an interesting structure that I feel may be worth calling
out. The first three patches are trivial test cleanups. The fourth patch
modifies the existing behavior of '--max-cruft-size', but does so while
keeping some of the old infrastructure around.

That may seem like an unnecessarily complicated approach, but it greatly
simplifies introducing the new behavior in the following (and final)
commit. I think that this results in a series that is a little easier to
review (since we don't see a ton of code being removed in one commit and
then re-added in another immediately following it). But if others feel
like this was a mistake, please let me know ;-).

Thanks in advance for your review!

Taylor Blau (5):
  t/t5329-pack-objects-cruft.sh: evict 'repack'-related tests
  t/t7704-repack-cruft.sh: clarify wording in --max-cruft-size tests
  t/t7704-repack-cruft.sh: consolidate `write_blob()`
  repack: avoid combining cruft packs with `--max-cruft-size`
  repack: begin combining cruft packs with `--combine-cruft-below-size`

 Documentation/git-repack.adoc |  20 ++-
 builtin/repack.c              |  62 +++----
 t/t5329-pack-objects-cruft.sh | 302 ++++++----------------------------
 t/t7704-repack-cruft.sh       | 293 ++++++++++++++++++++++++++++++---
 4 files changed, 354 insertions(+), 323 deletions(-)


base-commit: 08f612ba7000bf181ef6d8baed9ece322e567efd
-- 
2.49.0.rc0.6.g7f120c35e9

             reply	other threads:[~2025-03-17 23:00 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-17 23:00 Taylor Blau [this message]
2025-03-17 23:00 ` [PATCH 1/5] t/t5329-pack-objects-cruft.sh: evict 'repack'-related tests Taylor Blau
2025-03-19 14:20   ` Elijah Newren
2025-03-17 23:00 ` [PATCH 2/5] t/t7704-repack-cruft.sh: clarify wording in --max-cruft-size tests Taylor Blau
2025-03-19 14:20   ` Elijah Newren
2025-03-17 23:00 ` [PATCH 3/5] t/t7704-repack-cruft.sh: consolidate `write_blob()` Taylor Blau
2025-03-17 23:00 ` [PATCH 4/5] repack: avoid combining cruft packs with `--max-cruft-size` Taylor Blau
2025-03-17 23:00 ` [PATCH 5/5] repack: begin combining cruft packs with `--combine-cruft-below-size` Taylor Blau
2025-03-18 16:30   ` Junio C Hamano
2025-03-19 22:40     ` Taylor Blau
2025-03-19 14:21   ` Elijah Newren
2025-03-19 22:50     ` Taylor Blau
2025-03-19 22:52 ` [PATCH v2 0/5] repack: introduce '--combine-cruft-below-size' Taylor Blau
2025-03-19 22:52   ` [PATCH v2 1/5] t/t5329-pack-objects-cruft.sh: evict 'repack'-related tests Taylor Blau
2025-03-19 22:52   ` [PATCH v2 2/5] t/t7704-repack-cruft.sh: clarify wording in --max-cruft-size tests Taylor Blau
2025-03-19 22:52   ` [PATCH v2 3/5] t/t7704-repack-cruft.sh: consolidate `write_blob()` Taylor Blau
2025-03-19 22:52   ` [PATCH v2 4/5] repack: avoid combining cruft packs with `--max-cruft-size` Taylor Blau
2025-03-19 22:52   ` [PATCH v2 5/5] repack: begin combining cruft packs with `--combine-cruft-below-size` Taylor Blau
2025-03-20  4:30   ` [PATCH v2 0/5] repack: introduce '--combine-cruft-below-size' Elijah Newren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1742252411.git.me@ttaylorr.com \
    --to=me@ttaylorr.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).