From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Gregory Szorc" <gregory.szorc@gmail.com>,
"Eric Wong" <e@80x24.org>,
git@vger.kernel.org
Subject: Re: [PATCH 3/3] repack: simplify handling of auto-bitmaps and .keep files
Date: Wed, 31 Jul 2019 15:34:04 -0700 [thread overview]
Message-ID: <xmqqzhktomub.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20190731054055.GC16941@sigill.intra.peff.net> (Jeff King's message of "Wed, 31 Jul 2019 01:40:56 -0400")
Jeff King <peff@peff.net> writes:
> Commit 7328482253 (repack: disable bitmaps-by-default if .keep files
> exist, 2019-06-29) taught repack to prefer disabling bitmaps to
> duplicating objects (unless bitmaps were asked for explicitly).
>
> But there's an easier way to do this: if we keep passing the
> --honor-pack-keep flag to pack-objects when auto-enabling bitmaps, then
> pack-objects already makes the same decision (it will disable bitmaps
> rather than duplicate). Better still, pack-objects can actually decide
> to do so based not just on the presence of a .keep file, but on whether
> that .keep file actually impacts the new pack we're making (so if we're
> racing with a push or fetch, for example, their temporary .keep file
> will not block us from generating bitmaps if they haven't yet updated
> their refs).
>
> And because repack uses the --write-bitmap-index-quiet flag, we don't
> have to worry about pack-objects generating confusing warnings when it
> does see a .keep file. We can confirm this by tweaking the .keep test to
> check repack's stderr.
This change is a bit too dense so I'll need to think about it a bit
longer, but in the meantime it is queued alongside the other two.
Thanks.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> builtin/repack.c | 17 ++---------------
> t/t7700-repack.sh | 3 ++-
> 2 files changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/builtin/repack.c b/builtin/repack.c
> index db93ca3660..632c0c0a79 100644
> --- a/builtin/repack.c
> +++ b/builtin/repack.c
> @@ -89,17 +89,6 @@ static void remove_pack_on_signal(int signo)
> raise(signo);
> }
>
> -static int has_pack_keep_file(void)
> -{
> - struct packed_git *p;
> -
> - for (p = get_all_packs(the_repository); p; p = p->next) {
> - if (p->pack_keep)
> - return 1;
> - }
> - return 0;
> -}
> -
> /*
> * Adds all packs hex strings to the fname list, which do not
> * have a corresponding .keep file. These packs are not to
> @@ -346,13 +335,11 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
>
> if (write_bitmaps < 0) {
> if (!(pack_everything & ALL_INTO_ONE) ||
> - !is_bare_repository() ||
> - keep_pack_list.nr != 0 ||
> - has_pack_keep_file())
> + !is_bare_repository())
> write_bitmaps = 0;
> }
> if (pack_kept_objects < 0)
> - pack_kept_objects = !!write_bitmaps;
> + pack_kept_objects = write_bitmaps > 0;
>
> if (write_bitmaps && !(pack_everything & ALL_INTO_ONE))
> die(_(incremental_bitmap_conflict_error));
> diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
> index 54f815b8b9..4e855bc21b 100755
> --- a/t/t7700-repack.sh
> +++ b/t/t7700-repack.sh
> @@ -245,7 +245,8 @@ test_expect_success 'no bitmaps created if .keep files present' '
> keep=${pack%.pack}.keep &&
> test_when_finished "rm -f \"\$keep\"" &&
> >"$keep" &&
> - git -C bare.git repack -ad &&
> + git -C bare.git repack -ad 2>stderr &&
> + test_must_be_empty stderr &&
> find bare.git/objects/pack/ -type f -name "*.bitmap" >actual &&
> test_must_be_empty actual
> '
prev parent reply other threads:[~2019-07-31 22:34 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-26 2:18 Warnings in gc.log can prevent gc --auto from running Gregory Szorc
2019-07-29 10:07 ` Jeff King
2019-07-29 12:50 ` Ævar Arnfjörð Bjarmason
2019-07-31 4:28 ` Jeff King
2019-07-31 5:37 ` [PATCH 0/3] handling warnings due to auto-enabled bitmaps Jeff King
2019-07-31 5:37 ` [PATCH 1/3] t7700: clean up .keep file in bitmap-writing test Jeff King
2019-07-31 5:39 ` [PATCH 2/3] repack: silence warnings when auto-enabled bitmaps cannot be built Jeff King
2019-07-31 20:26 ` Junio C Hamano
2019-07-31 21:11 ` Jeff King
2019-07-31 5:40 ` [PATCH 3/3] repack: simplify handling of auto-bitmaps and .keep files Jeff King
2019-07-31 22:34 ` Junio C Hamano [this message]
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqzhktomub.fsf@gitster-ct.c.googlers.com \
--to=gitster@pobox.com \
--cc=avarab@gmail.com \
--cc=e@80x24.org \
--cc=git@vger.kernel.org \
--cc=gregory.szorc@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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).