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

2013-08-30 Thread Stefan Beller
On 08/29/2013 10:53 PM, Junio C Hamano wrote:
 Stefan Beller stefanbel...@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.
 

This would be very appreciated. I am no native speaker, so please
correct any variable naming, which could potentially be missleading.

I've seen you've already put a squashing proposal on top of
origin/sb/repack-in-c, it looks great, so feel free to squash it
into the first commit.

Thanks,
Stefan




signature.asc
Description: OpenPGP digital signature


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

2013-08-29 Thread Junio C Hamano
Stefan Beller stefanbel...@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%)
--
To