[RFC,08/10] pack-objects: introduce pack.allowPackReuse
diff mbox series

Message ID 20190913130226.7449-9-chriscool@tuxfamily.org
State New
Headers show
Series
  • Rewrite packfile reuse code
Related show

Commit Message

Christian Couder Sept. 13, 2019, 1:02 p.m. UTC
From: Jeff King <peff@peff.net>

Let's make it possible to configure if we want pack reuse or not.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/pack-objects.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Junio C Hamano Sept. 13, 2019, 9:37 p.m. UTC | #1
Christian Couder <christian.couder@gmail.com> writes:

> From: Jeff King <peff@peff.net>
>
> Let's make it possible to configure if we want pack reuse or not.
>
> Signed-off-by: Jeff King <peff@peff.net>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  builtin/pack-objects.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index c11b2ea8d4..1664969c97 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -96,6 +96,7 @@ static off_t reuse_packfile_offset;
>  
>  static int use_bitmap_index_default = 1;
>  static int use_bitmap_index = -1;
> +static int allow_pack_reuse = 1;
>  static enum {
>  	WRITE_BITMAP_FALSE = 0,
>  	WRITE_BITMAP_QUIET,
> @@ -2719,6 +2720,10 @@ static int git_pack_config(const char *k, const char *v, void *cb)
>  		sparse = git_config_bool(k, v);
>  		return 0;
>  	}
> +	if (!strcmp(k, "pack.allowpackreuse")) {
> +		allow_pack_reuse = git_config_bool(k, v);
> +		return 0;
> +	}
>  	if (!strcmp(k, "pack.threads")) {
>  		delta_search_threads = git_config_int(k, v);
>  		if (delta_search_threads < 0)
> @@ -3063,7 +3068,8 @@ static int get_object_list_from_bitmap(struct rev_info *revs)
>  	if (!(bitmap_git = prepare_bitmap_walk(revs)))
>  		return -1;
>  
> -	if (pack_options_allow_reuse() &&
> +	if (allow_pack_reuse &&
> +	    pack_options_allow_reuse() &&

It somehow looks strange to have this code check for both
allow_pack_reuse and pack_options_allow_reuse().  I would have
expected that the referene to the new variable to be contained in
the existing helper function, so that any future code that wants to
ask "are we allowed to reuse?" would get the same answer from the
helper consistently, without having to check the new variable.

>  	    !reuse_partial_packfile_from_bitmap(
>  			bitmap_git,
>  			&reuse_packfile,

Patch
diff mbox series

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index c11b2ea8d4..1664969c97 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -96,6 +96,7 @@  static off_t reuse_packfile_offset;
 
 static int use_bitmap_index_default = 1;
 static int use_bitmap_index = -1;
+static int allow_pack_reuse = 1;
 static enum {
 	WRITE_BITMAP_FALSE = 0,
 	WRITE_BITMAP_QUIET,
@@ -2719,6 +2720,10 @@  static int git_pack_config(const char *k, const char *v, void *cb)
 		sparse = git_config_bool(k, v);
 		return 0;
 	}
+	if (!strcmp(k, "pack.allowpackreuse")) {
+		allow_pack_reuse = git_config_bool(k, v);
+		return 0;
+	}
 	if (!strcmp(k, "pack.threads")) {
 		delta_search_threads = git_config_int(k, v);
 		if (delta_search_threads < 0)
@@ -3063,7 +3068,8 @@  static int get_object_list_from_bitmap(struct rev_info *revs)
 	if (!(bitmap_git = prepare_bitmap_walk(revs)))
 		return -1;
 
-	if (pack_options_allow_reuse() &&
+	if (allow_pack_reuse &&
+	    pack_options_allow_reuse() &&
 	    !reuse_partial_packfile_from_bitmap(
 			bitmap_git,
 			&reuse_packfile,