* [PATCH v4 1/2] auto gc: don't write bitmaps for incremental repacks
@ 2016-12-28 18:11 David Turner
2016-12-28 18:12 ` [PATCH v4 2/2] repack: die on incremental + write-bitmap-index David Turner
0 siblings, 1 reply; 3+ messages in thread
From: David Turner @ 2016-12-28 18:11 UTC (permalink / raw)
To: git, peff; +Cc: David Turner
When git gc --auto does an incremental repack of loose objects, we do
not expect to be able to write a bitmap; it is very likely that
objects in the new pack will have references to objects outside of the
pack. So we shouldn't try to write a bitmap, because doing so will
likely issue a warning.
This warning was making its way into gc.log. When the gc.log was
present, future auto gc runs would refuse to run.
Patch by Jeff King.
Bug report, test, and commit message by David Turner.
Signed-off-by: David Turner <dturner@twosigma.com>
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/gc.c | 9 ++++++++-
t/t6500-gc.sh | 25 +++++++++++++++++++++++++
2 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 069950d..331f219 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -191,6 +191,11 @@ static void add_repack_all_option(void)
}
}
+static void add_repack_incremental_option(void)
+{
+ argv_array_push(&repack, "--no-write-bitmap-index");
+}
+
static int need_to_gc(void)
{
/*
@@ -208,7 +213,9 @@ static int need_to_gc(void)
*/
if (too_many_packs())
add_repack_all_option();
- else if (!too_many_loose_objects())
+ else if (too_many_loose_objects())
+ add_repack_incremental_option();
+ else
return 0;
if (run_hook_le(NULL, "pre-auto-gc", NULL))
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 5d7d414..def2aca 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -43,4 +43,29 @@ test_expect_success 'gc is not aborted due to a stale symref' '
)
'
+test_expect_success 'auto gc with too many loose objects does not attempt to create bitmaps' '
+ test_config gc.auto 3 &&
+ test_config gc.autodetach false &&
+ test_config pack.writebitmaps true &&
+ # We need to create two object whose sha1s start with 17
+ # since this is what git gc counts. As it happens, these
+ # two blobs will do so.
+ test_commit 263 &&
+ test_commit 410 &&
+ # Our first gc will create a pack; our second will create a second pack
+ git gc --auto &&
+ ls .git/objects/pack | grep -v bitmap | sort >existing_packs &&
+ test_commit 523 &&
+ test_commit 790 &&
+
+ git gc --auto 2>err &&
+ test_i18ngrep ! "^warning:" err &&
+ ls .git/objects/pack/ | grep -v bitmap | sort >post_packs &&
+ comm --output-delimiter , -1 -3 existing_packs post_packs >new &&
+ comm --output-delimiter , -2 -3 existing_packs post_packs >del &&
+ test_line_count = 0 del && # No packs are deleted
+ test_line_count = 2 new # There is one new pack and its .idx
+'
+
+
test_done
--
2.8.0.rc4.22.g8ae061a
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH v4 2/2] repack: die on incremental + write-bitmap-index
2016-12-28 18:11 [PATCH v4 1/2] auto gc: don't write bitmaps for incremental repacks David Turner
@ 2016-12-28 18:12 ` David Turner
2016-12-28 22:32 ` Johannes Sixt
0 siblings, 1 reply; 3+ messages in thread
From: David Turner @ 2016-12-28 18:12 UTC (permalink / raw)
To: git, peff; +Cc: David Turner
The bitmap index only works for single packs, so requesting an
incremental repack with bitmap indexes makes no sense.
Signed-off-by: David Turner <dturner@twosigma.com>
---
builtin/repack.c | 9 +++++++++
t/t5310-pack-bitmaps.sh | 8 +++-----
t/t6500-gc.sh | 8 ++++----
3 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index 80dd06b..9c3dd09 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -18,6 +18,12 @@ static const char *const git_repack_usage[] = {
NULL
};
+static const char incremental_bitmap_conflict_error[] = N_(
+"Incremental repacks are incompatible with bitmap indexes. Use \n"
+"--no-write-bitmap-index or disable the pack.writebitmaps configuration."
+);
+
+
static int repack_config(const char *var, const char *value, void *cb)
{
if (!strcmp(var, "repack.usedeltabaseoffset")) {
@@ -206,6 +212,9 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
if (pack_kept_objects < 0)
pack_kept_objects = write_bitmaps;
+ if (write_bitmaps && !(pack_everything & ALL_INTO_ONE))
+ die(incremental_bitmap_conflict_error);
+
packdir = mkpathdup("%s/pack", get_object_directory());
packtmp = mkpathdup("%s/.tmp-%d-pack", packdir, (int)getpid());
diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
index b4c7a6f..424bec7 100755
--- a/t/t5310-pack-bitmaps.sh
+++ b/t/t5310-pack-bitmaps.sh
@@ -118,12 +118,10 @@ test_expect_success 'fetch (partial bitmap)' '
test_cmp expect actual
'
-test_expect_success 'incremental repack cannot create bitmaps' '
+test_expect_success 'incremental repack fails when bitmaps are requested' '
test_commit more-1 &&
- find .git/objects/pack -name "*.bitmap" >expect &&
- git repack -d &&
- find .git/objects/pack -name "*.bitmap" >actual &&
- test_cmp expect actual
+ test_must_fail git repack -d 2>err &&
+ test_i18ngrep "Incremental repacks are incompatible with bitmap" err
'
test_expect_success 'incremental repack can disable bitmaps' '
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index def2aca..1762dfa 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -54,15 +54,15 @@ test_expect_success 'auto gc with too many loose objects does not attempt to cre
test_commit 410 &&
# Our first gc will create a pack; our second will create a second pack
git gc --auto &&
- ls .git/objects/pack | grep -v bitmap | sort >existing_packs &&
+ ls .git/objects/pack | sort >existing_packs &&
test_commit 523 &&
test_commit 790 &&
git gc --auto 2>err &&
test_i18ngrep ! "^warning:" err &&
- ls .git/objects/pack/ | grep -v bitmap | sort >post_packs &&
- comm --output-delimiter , -1 -3 existing_packs post_packs >new &&
- comm --output-delimiter , -2 -3 existing_packs post_packs >del &&
+ ls .git/objects/pack/ | sort >post_packs &&
+ comm -1 -3 existing_packs post_packs >new &&
+ comm -2 -3 existing_packs post_packs >del &&
test_line_count = 0 del && # No packs are deleted
test_line_count = 2 new # There is one new pack and its .idx
'
--
2.8.0.rc4.22.g8ae061a
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v4 2/2] repack: die on incremental + write-bitmap-index
2016-12-28 18:12 ` [PATCH v4 2/2] repack: die on incremental + write-bitmap-index David Turner
@ 2016-12-28 22:32 ` Johannes Sixt
0 siblings, 0 replies; 3+ messages in thread
From: Johannes Sixt @ 2016-12-28 22:32 UTC (permalink / raw)
To: David Turner; +Cc: git, peff
Am 28.12.2016 um 19:12 schrieb David Turner:
> +static const char incremental_bitmap_conflict_error[] = N_(
> +"Incremental repacks are incompatible with bitmap indexes. Use \n"
The SP before LF could be removed.
> +"--no-write-bitmap-index or disable the pack.writebitmaps configuration."
> +);
The string is marked for translation here...
> +
> +
> static int repack_config(const char *var, const char *value, void *cb)
> {
> if (!strcmp(var, "repack.usedeltabaseoffset")) {
> @@ -206,6 +212,9 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
> if (pack_kept_objects < 0)
> pack_kept_objects = write_bitmaps;
>
> + if (write_bitmaps && !(pack_everything & ALL_INTO_ONE))
> + die(incremental_bitmap_conflict_error);
... but then any translations remain unused. This should be
die(_(incremental_bitmap_conflict_error));
-- Hannes
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-12-28 22:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-28 18:11 [PATCH v4 1/2] auto gc: don't write bitmaps for incremental repacks David Turner
2016-12-28 18:12 ` [PATCH v4 2/2] repack: die on incremental + write-bitmap-index David Turner
2016-12-28 22:32 ` Johannes Sixt
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).