Re: [PATCHv7 0/2] Rewriting repack in C

[Junio C Hamano <gitster@pobox.com>]

Stefan Beller <stefanbeller@googlemail.com> writes:

> Here is a diff since the last time sending this patch series:

This is very readable.

There may be people who misread "LOOSE" as "LOSE"; the option -A is 
about making the unreachable ones loose so that they can be expired,
so let's rename it LOOSEN_UNREACHABLE to avoid confusion.

Thanks.

>
> diff --git a/builtin/repack.c b/builtin/repack.c
> index 0ace2a3..0cc823d 100644
> --- a/builtin/repack.c
> +++ b/builtin/repack.c
> @@ -41,10 +41,10 @@ static void remove_temporary_files(void)
>  
>  	strbuf_addstr(&buf, packdir);
>  
> -	/* dirlen holds the length of the path before the file name */
> +	/* Point at the slash at the end of ".../objects/pack/" */
>  	dirlen = buf.len + 1;
>  	strbuf_addf(&buf, "%s", packtmp);
> -	/* prefixlen holds the length of the prefix */
> +	/* Point at the dash at the end of ".../.tmp-%d-pack-" */
>  	prefixlen = buf.len - dirlen;
>  
>  	while ((e = readdir(dir))) {
> @@ -109,9 +109,12 @@ static void remove_redundant_pack(const char *path_prefix, const char *hex)
>  	}
>  }
>  
> +#define ALL_INTO_ONE 1
> +#define LOOSE_UNREACHABLE 2
> +
>  int cmd_repack(int argc, const char **argv, const char *prefix)
>  {
> -	const char *exts[2] = {".idx", ".pack"};
> +	const char *exts[2] = {".pack", ".idx"};
>  	struct child_process cmd;
>  	struct string_list_item *item;
>  	struct argv_array cmd_args = ARGV_ARRAY_INIT;
> @@ -124,7 +127,6 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
>  
>  	/* variables to be filled by option parsing */
>  	int pack_everything = 0;
> -	int pack_everything_but_loose = 0;
>  	int delete_redundant = 0;
>  	char *unpack_unreachable = NULL;
>  	int window = 0, window_memory = 0;
> @@ -136,10 +138,10 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
>  	int local = 0;
>  
>  	struct option builtin_repack_options[] = {
> -		OPT_BOOL('a', NULL, &pack_everything,
> -				N_("pack everything in a single pack")),
> -		OPT_BOOL('A', NULL, &pack_everything_but_loose,
> -				N_("same as -a, and turn unreachable objects loose")),
> +		OPT_BIT('a', NULL, &pack_everything,
> +				N_("pack everything in a single pack"), ALL_INTO_ONE),
> +		OPT_BIT('A', NULL, &pack_everything,
> +				N_("same as -a, and turn unreachable objects loose"), LOOSE_UNREACHABLE),
>  		OPT_BOOL('d', NULL, &delete_redundant,
>  				N_("remove redundant packs, and run git-prune-packed")),
>  		OPT_BOOL('f', NULL, &no_reuse_delta,
> @@ -193,7 +195,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
>  	if (no_reuse_object)
>  		argv_array_pushf(&cmd_args, "--no-reuse-object");
>  
> -	if (!pack_everything && !pack_everything_but_loose) {
> +	if (!pack_everything) {
>  		argv_array_push(&cmd_args, "--unpacked");
>  		argv_array_push(&cmd_args, "--incremental");
>  	} else {
> @@ -204,7 +206,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
>  				argv_array_pushf(&cmd_args,
>  						"--unpack-unreachable=%s",
>  						unpack_unreachable);
> -			else if (pack_everything_but_loose)
> +			else if (pack_everything & LOOSE_UNREACHABLE)
>  				argv_array_push(&cmd_args,
>  						"--unpack-unreachable");
>  		}
> @@ -246,6 +248,12 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
>  	if (!nr_packs && !quiet)
>  		printf("Nothing new to pack.\n");
>  
> +	/*
> +	 * Ok we have prepared all new packfiles.
> +	 * First see if there are packs of the same name and if so
> +	 * if we can move them out of the way (this can happen if we
> +	 * repacked immediately after packing fully.
> +	 */
>  	failed = 0;
>  	for_each_string_list_item(item, &names) {
>  		for (ext = 0; ext < 2; ext++) {
> @@ -366,5 +374,6 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
>  		run_command(&cmd);
>  		argv_array_clear(&cmd_args);
>  	}
> +	remove_temporary_files();
>  	return 0;
>  }
> -- 
> 1.8.4
>
> Stefan Beller (2):
>   repack: rewrite the shell script in C
>   repack: retain the return value of pack-objects
>
>  Makefile                                        |   2 +-
>  builtin.h                                       |   1 +
>  builtin/repack.c                                | 379 ++++++++++++++++++++++++
>  git-repack.sh => contrib/examples/git-repack.sh |   0
>  git.c                                           |   1 +
>  5 files changed, 382 insertions(+), 1 deletion(-)
>  create mode 100644 builtin/repack.c
>  rename git-repack.sh => contrib/examples/git-repack.sh (100%)