From 64d3dc94687bf4a8a422ab6d5cffcdb44315d49e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 10 Jun 2014 16:08:38 -0400 Subject: repack: do not accidentally pack kept objects by default Commit ee34a2b (repack: add `repack.packKeptObjects` config var, 2014-03-03) added a flag which could duplicate kept objects, but did not mean to turn it on by default. Instead, the option is tied by default to the decision to write bitmaps, like: if (pack_kept_objects < 0) pack_kept_objects = write_bitmap; after which we expect pack_kept_objects to be a boolean 0 or 1. However, that assignment neglects that write_bitmap is _also_ a tri-state with "-1" as the default, and with neither option given, we accidentally turn the option on. This patch is the minimal fix to restore the desired behavior for the default state. Further patches will fix the more complicated cases. Note the update to t7700. It failed to turn on bitmaps, meaning we were actually confirming the wrong behavior! Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/repack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'builtin') diff --git a/builtin/repack.c b/builtin/repack.c index 50cc28152..a02ef0dcd 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -191,7 +191,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) git_repack_usage, 0); if (pack_kept_objects < 0) - pack_kept_objects = write_bitmap; + pack_kept_objects = write_bitmap > 0; packdir = mkpathdup("%s/pack", get_object_directory()); packtmp = mkpathdup("%s/.tmp-%d-pack", packdir, (int)getpid()); -- cgit v1.2.1 From 3198b89fb225ef8e0200e9486a48be8068e85d15 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 10 Jun 2014 16:09:23 -0400 Subject: repack: respect pack.writebitmaps The config option to turn on bitmaps is read all the way down in the plumbing of pack-objects. This makes it hard for other options in the porcelain of repack to make decisions based on the bitmap setting. For example, repack.packKeptObjects tries to kick in by default only when bitmaps are turned on. But it can't do so reliably because it doesn't yet know whether we are using bitmaps. This patch teaches repack to respect pack.writebitmaps. It means we pass a redundant command-line flag to pack-objects, but that's OK; it shouldn't affect the outcome. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/repack.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'builtin') diff --git a/builtin/repack.c b/builtin/repack.c index a02ef0dcd..d6b6bb673 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -10,6 +10,7 @@ static int delta_base_offset = 1; static int pack_kept_objects = -1; +static int write_bitmap = -1; static char *packdir, *packtmp; static const char *const git_repack_usage[] = { @@ -27,6 +28,10 @@ static int repack_config(const char *var, const char *value, void *cb) pack_kept_objects = git_config_bool(var, value); return 0; } + if (!strcmp(var, "pack.writebitmaps")) { + write_bitmap = git_config_bool(var, value); + return 0; + } return git_default_config(var, value, cb); } @@ -149,7 +154,6 @@ int cmd_repack(int argc, const char **argv, const char *prefix) int no_update_server_info = 0; int quiet = 0; int local = 0; - int write_bitmap = -1; struct option builtin_repack_options[] = { OPT_BIT('a', NULL, &pack_everything, -- cgit v1.2.1 From d078d85bc3cd74bbe1f230feb26cbe28f29d6d25 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 10 Jun 2014 16:10:07 -0400 Subject: repack: s/write_bitmap/&s/ in code The config name is "writeBitmaps", so the internal variable missing the plural is unnecessarily confusing to write. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/repack.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'builtin') diff --git a/builtin/repack.c b/builtin/repack.c index d6b6bb673..0ec643f38 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -10,7 +10,7 @@ static int delta_base_offset = 1; static int pack_kept_objects = -1; -static int write_bitmap = -1; +static int write_bitmaps = -1; static char *packdir, *packtmp; static const char *const git_repack_usage[] = { @@ -29,7 +29,7 @@ static int repack_config(const char *var, const char *value, void *cb) return 0; } if (!strcmp(var, "pack.writebitmaps")) { - write_bitmap = git_config_bool(var, value); + write_bitmaps = git_config_bool(var, value); return 0; } return git_default_config(var, value, cb); @@ -172,7 +172,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) OPT__QUIET(&quiet, N_("be quiet")), OPT_BOOL('l', "local", &local, N_("pass --local to git-pack-objects")), - OPT_BOOL('b', "write-bitmap-index", &write_bitmap, + OPT_BOOL('b', "write-bitmap-index", &write_bitmaps, N_("write bitmap index")), OPT_STRING(0, "unpack-unreachable", &unpack_unreachable, N_("approxidate"), N_("with -A, do not loosen objects older than this")), @@ -195,7 +195,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) git_repack_usage, 0); if (pack_kept_objects < 0) - pack_kept_objects = write_bitmap > 0; + pack_kept_objects = write_bitmaps > 0; packdir = mkpathdup("%s/pack", get_object_directory()); packtmp = mkpathdup("%s/.tmp-%d-pack", packdir, (int)getpid()); @@ -221,9 +221,9 @@ int cmd_repack(int argc, const char **argv, const char *prefix) argv_array_pushf(&cmd_args, "--no-reuse-delta"); if (no_reuse_object) argv_array_pushf(&cmd_args, "--no-reuse-object"); - if (write_bitmap >= 0) + if (write_bitmaps >= 0) argv_array_pushf(&cmd_args, "--%swrite-bitmap-index", - write_bitmap ? "" : "no-"); + write_bitmaps ? "" : "no-"); if (pack_everything & ALL_INTO_ONE) { get_non_kept_pack_filenames(&existing_packs); -- cgit v1.2.1