{"thread":{"id":"9888","subject":"[PATCH 1/8] Export matches_pack_name() and fix its return value","startedAt":"2007-09-17T08:44:47Z","lastAt":"2007-09-18T03:01:56Z","messageCount":18,"participants":["Junio C Hamano","Johannes Schindelin","Andreas Ericsson","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"53267","messageId":"11900186941912-git-send-email-gitster@pobox.com","threadId":"9888","inReplyTo":null,"subject":"[PATCH 1/8] Export matches_pack_name() and fix its return value","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-17T08:44:47Z","receivedAt":"2007-09-17T08:44:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The function sounds boolean; make it behave as one, not \"0 for\nsuccess, non-zero for failure\".\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n cache.h     |    1 +\n sha1_file.c |   14 +++++++-------\n 2 files changed, 8 insertions(+), 7 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 70abbd5..3fa5b8e 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -529,6 +529,7 @@ extern void *unpack_entry(struct packed_git *, off_t, enum object_type *, unsign\n extern unsigned long unpack_object_header_gently(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);\n extern unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t);\n extern const char *packed_object_info_detail(struct packed_git *, off_t, unsigned long *, unsigned long *, unsigned int *, unsigned char *);\n+extern int matches_pack_name(struct packed_git *p, const char *name);\n \n /* Dumb servers support */\n extern int update_server_info(int);\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 9978a58..5801c3e 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1684,22 +1684,22 @@ off_t find_pack_entry_one(const unsigned char *sha1,\n \treturn 0;\n }\n \n-static int matches_pack_name(struct packed_git *p, const char *ig)\n+int matches_pack_name(struct packed_git *p, const char *name)\n {\n \tconst char *last_c, *c;\n \n-\tif (!strcmp(p->pack_name, ig))\n-\t\treturn 0;\n+\tif (!strcmp(p->pack_name, name))\n+\t\treturn 1;\n \n \tfor (c = p->pack_name, last_c = c; *c;)\n \t\tif (*c == '/')\n \t\t\tlast_c = ++c;\n \t\telse\n \t\t\t++c;\n-\tif (!strcmp(last_c, ig))\n-\t\treturn 0;\n+\tif (!strcmp(last_c, name))\n+\t\treturn 1;\n \n-\treturn 1;\n+\treturn 0;\n }\n \n static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e, const char **ignore_packed)\n@@ -1717,7 +1717,7 @@ static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e, cons\n \t\tif (ignore_packed) {\n \t\t\tconst char **ig;\n \t\t\tfor (ig = ignore_packed; *ig; ig++)\n-\t\t\t\tif (!matches_pack_name(p, *ig))\n+\t\t\t\tif (matches_pack_name(p, *ig))\n \t\t\t\t\tbreak;\n \t\t\tif (*ig)\n \t\t\t\tgoto next;\n-- \n1.5.3.1.967.g6bb01\n"},{"id":"53268","messageId":"11900186973144-git-send-email-gitster@pobox.com","threadId":"9888","inReplyTo":"11900186941912-git-send-email-gitster@pobox.com","subject":"[PATCH 2/8] pack-objects --keep-unreachable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-17T08:44:48Z","receivedAt":"2007-09-17T08:44:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This new option is meant to be used in conjunction with the\noptions \"git repack -a -d\" usually invokes the underlying\npack-objects.  When this option is given, objects unreachable\nfrom the refs in packs named with --unpacked= option are added\nto the resulting pack, in addition to the reachable objects that\nare not in packs marked with *.keep files.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-pack-objects.c |   95 +++++++++++++++++++++++++++++++++++++++++++++++-\n 1 files changed, 93 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 12509fa..ba7c8da 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -21,7 +21,7 @@ git-pack-objects [{ -q | --progress | --all-progress }] \\n\\\n \t[--window=N] [--window-memory=N] [--depth=N] \\n\\\n \t[--no-reuse-delta] [--no-reuse-object] [--delta-base-offset] \\n\\\n \t[--non-empty] [--revs [--unpacked | --all]*] [--reflog] \\n\\\n-\t[--stdout | base-name] [<ref-list | <object-list]\";\n+\t[--stdout | base-name] [--keep-unreachable] [<ref-list | <object-list]\";\n \n struct object_entry {\n \tstruct pack_idx_entry idx;\n@@ -57,7 +57,7 @@ static struct object_entry **written_list;\n static uint32_t nr_objects, nr_alloc, nr_result, nr_written;\n \n static int non_empty;\n-static int no_reuse_delta, no_reuse_object;\n+static int no_reuse_delta, no_reuse_object, keep_unreachable;\n static int local;\n static int incremental;\n static int allow_ofs_delta;\n@@ -1625,15 +1625,19 @@ static void read_object_list_from_stdin(void)\n \t}\n }\n \n+#define OBJECT_ADDED (1u<<20)\n+\n static void show_commit(struct commit *commit)\n {\n \tadd_object_entry(commit->object.sha1, OBJ_COMMIT, NULL, 0);\n+\tcommit->object.flags |= OBJECT_ADDED;\n }\n \n static void show_object(struct object_array_entry *p)\n {\n \tadd_preferred_base_object(p->name);\n \tadd_object_entry(p->item->sha1, p->item->type, p->name, 0);\n+\tp->item->flags |= OBJECT_ADDED;\n }\n \n static void show_edge(struct commit *commit)\n@@ -1641,6 +1645,86 @@ static void show_edge(struct commit *commit)\n \tadd_preferred_base(commit->object.sha1);\n }\n \n+struct in_pack_object {\n+\toff_t offset;\n+\tstruct object *object;\n+};\n+\n+struct in_pack {\n+\tint alloc;\n+\tint nr;\n+\tstruct in_pack_object *array;\n+};\n+\n+static void mark_in_pack_object(struct object *object, struct packed_git *p, struct in_pack *in_pack)\n+{\n+\tin_pack->array[in_pack->nr].offset = find_pack_entry_one(object->sha1, p);\n+\tin_pack->array[in_pack->nr].object = object;\n+\tin_pack->nr++;\n+}\n+\n+/*\n+ * Compare the objects in the offset order, in order to emulate the\n+ * \"git-rev-list --objects\" output that produced the pack originally.\n+ */\n+static int ofscmp(const void *a_, const void *b_)\n+{\n+\tstruct in_pack_object *a = (struct in_pack_object *)a_;\n+\tstruct in_pack_object *b = (struct in_pack_object *)b_;\n+\n+\tif (a->offset < b->offset)\n+\t\treturn -1;\n+\telse if (a->offset > b->offset)\n+\t\treturn 1;\n+\telse\n+\t\treturn hashcmp(a->object->sha1, b->object->sha1);\n+}\n+\n+static void add_objects_in_unpacked_packs(struct rev_info *revs)\n+{\n+\tstruct packed_git *p;\n+\tstruct in_pack in_pack;\n+\tuint32_t i;\n+\n+\tmemset(&in_pack, 0, sizeof(in_pack));\n+\n+\tfor (p = packed_git; p; p = p->next) {\n+\t\tconst unsigned char *sha1;\n+\t\tstruct object *o;\n+\n+\t\tfor (i = 0; i < revs->num_ignore_packed; i++) {\n+\t\t\tif (matches_pack_name(p, revs->ignore_packed[i]))\n+\t\t\t\tbreak;\n+\t\t}\n+\t\tif (revs->num_ignore_packed <= i)\n+\t\t\tcontinue;\n+\t\tif (open_pack_index(p))\n+\t\t\tdie(\"cannot open pack index\");\n+\n+\t\tALLOC_GROW(in_pack.array,\n+\t\t\t   in_pack.nr + p->num_objects,\n+\t\t\t   in_pack.alloc);\n+\n+\t\tfor (i = 0; i < p->num_objects; i++) {\n+\t\t\tsha1 = nth_packed_object_sha1(p, i);\n+\t\t\to = lookup_unknown_object(sha1);\n+\t\t\tif (!(o->flags & OBJECT_ADDED))\n+\t\t\t\tmark_in_pack_object(o, p, &in_pack);\n+\t\t\to->flags |= OBJECT_ADDED;\n+\t\t}\n+\t}\n+\n+\tif (in_pack.nr) {\n+\t\tqsort(in_pack.array, in_pack.nr, sizeof(in_pack.array[0]),\n+\t\t      ofscmp);\n+\t\tfor (i = 0; i < in_pack.nr; i++) {\n+\t\t\tstruct object *o = in_pack.array[i].object;\n+\t\t\tadd_object_entry(o->sha1, o->type, \"\", 0);\n+\t\t}\n+\t}\n+\tfree(in_pack.array);\n+}\n+\n static void get_object_list(int ac, const char **av)\n {\n \tstruct rev_info revs;\n@@ -1672,6 +1756,9 @@ static void get_object_list(int ac, const char **av)\n \tprepare_revision_walk(&revs);\n \tmark_edges_uninteresting(revs.commits, &revs, show_edge);\n \ttraverse_commit_list(&revs, show_commit, show_object);\n+\n+\tif (keep_unreachable)\n+\t\tadd_objects_in_unpacked_packs(&revs);\n }\n \n static int adjust_perm(const char *path, mode_t mode)\n@@ -1789,6 +1876,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\t\tuse_internal_rev_list = 1;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!strcmp(\"--keep-unreachable\", arg)) {\n+\t\t\tkeep_unreachable = 1;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!strcmp(\"--unpacked\", arg) ||\n \t\t    !prefixcmp(arg, \"--unpacked=\") ||\n \t\t    !strcmp(\"--reflog\", arg) ||\n-- \n1.5.3.1.967.g6bb01\n"},{"id":"53272","messageId":"11900187002882-git-send-email-gitster@pobox.com","threadId":"9888","inReplyTo":"11900186941912-git-send-email-gitster@pobox.com","subject":"[PATCH 3/8] repack -A -d: use --keep-unreachable when repacking","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-17T08:44:49Z","receivedAt":"2007-09-17T08:44:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is a safer variant of \"repack -a -d\" that does not drop\nunreachable objects that are in packs.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n git-repack.sh |   14 +++++++++++---\n 1 files changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/git-repack.sh b/git-repack.sh\nindex 156c5e8..204084e 100755\n--- a/git-repack.sh\n+++ b/git-repack.sh\n@@ -3,17 +3,19 @@\n # Copyright (c) 2005 Linus Torvalds\n #\n \n-USAGE='[-a] [-d] [-f] [-l] [-n] [-q] [--max-pack-size=N] [--window=N] [--window-memory=N] [--depth=N]'\n+USAGE='[-a|-A] [-d] [-f] [-l] [-n] [-q] [--max-pack-size=N] [--window=N] [--window-memory=N] [--depth=N]'\n SUBDIRECTORY_OK='Yes'\n . git-sh-setup\n \n-no_update_info= all_into_one= remove_redundant=\n+no_update_info= all_into_one= remove_redundant= keep_unreachable=\n local= quiet= no_reuse= extra=\n while case \"$#\" in 0) break ;; esac\n do\n \tcase \"$1\" in\n \t-n)\tno_update_info=t ;;\n \t-a)\tall_into_one=t ;;\n+\t-A)\tall_into_one=t\n+\t\tkeep_unreachable=t ;;\n \t-d)\tremove_redundant=t ;;\n \t-q)\tquiet=-q ;;\n \t-f)\tno_reuse=--no-reuse-object ;;\n@@ -59,7 +61,13 @@ case \",$all_into_one,\" in\n \t\t\tfi\n \t\tdone\n \tfi\n-\t[ -z \"$args\" ] && args='--unpacked --incremental'\n+\tif test -z \"$args\"\n+\tthen\n+\t\targs='--unpacked --incremental'\n+\telif test -n \"$keep_unreachable\"\n+\tthen\n+\t\targs=\"$args --keep-unreachable\"\n+\tfi\n \t;;\n esac\n \n-- \n1.5.3.1.967.g6bb01\n"},{"id":"53273","messageId":"1190018703643-git-send-email-gitster@pobox.com","threadId":"9888","inReplyTo":"11900186941912-git-send-email-gitster@pobox.com","subject":"[PATCH 4/8] git-gc --auto: move threshold check to need_to_gc() function.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-17T08:44:50Z","receivedAt":"2007-09-17T08:44:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"That is where we decide if we are going to run gc\nautomatically.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-gc.c |    9 +++++++--\n 1 files changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-gc.c b/builtin-gc.c\nindex 093b3dd..f046a2a 100644\n--- a/builtin-gc.c\n+++ b/builtin-gc.c\n@@ -80,6 +80,13 @@ static int need_to_gc(void)\n \tint num_loose = 0;\n \tint needed = 0;\n \n+\t/*\n+\t * Setting gc.auto to 0 or negative can disable the\n+\t * automatic gc\n+\t */\n+\tif (gc_auto_threshold <= 0)\n+\t\treturn 0;\n+\n \tif (sizeof(path) <= snprintf(path, sizeof(path), \"%s/17\", objdir)) {\n \t\twarning(\"insanely long object directory %.*s\", 50, objdir);\n \t\treturn 0;\n@@ -129,8 +136,6 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(arg, \"--auto\")) {\n-\t\t\tif (gc_auto_threshold <= 0)\n-\t\t\t\treturn 0;\n \t\t\tauto_gc = 1;\n \t\t\tcontinue;\n \t\t}\n-- \n1.5.3.1.967.g6bb01\n"},{"id":"53269","messageId":"11900187073420-git-send-email-gitster@pobox.com","threadId":"9888","inReplyTo":"11900186941912-git-send-email-gitster@pobox.com","subject":"[PATCH 5/8] git-gc --auto: add documentation.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-17T08:44:51Z","receivedAt":"2007-09-17T08:44:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This documents the auto-packing of loose objects performed by\ngit-gc --auto.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/config.txt |    7 +++++++\n Documentation/git-gc.txt |   11 ++++++++++-\n 2 files changed, 17 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 866e053..3643c0b 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -439,6 +439,13 @@ gc.aggressiveWindow::\n \talgorithm used by 'git gc --aggressive'.  This defaults\n \tto 10.\n \n+gc.auto::\n+\tWhen there are approximately more than this many loose\n+\tobjects in the repository, `git gc --auto` that is\n+\tinvoked by some Porcelain commands will create a new\n+\tpack and prune them.  Setting this to 0 disables the\n+\tauto garbage collection.\n+\n gc.packrefs::\n \t`git gc` does not run `git pack-refs` in a bare repository by\n \tdefault so that older dumb-transport clients can still fetch\ndiff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt\nindex c7742ca..40c1ce4 100644\n--- a/Documentation/git-gc.txt\n+++ b/Documentation/git-gc.txt\n@@ -8,7 +8,7 @@ git-gc - Cleanup unnecessary files and optimize the local repository\n \n SYNOPSIS\n --------\n-'git-gc' [--prune] [--aggressive]\n+'git-gc' [--prune] [--aggressive] [--auto]\n \n DESCRIPTION\n -----------\n@@ -43,6 +43,15 @@ OPTIONS\n \tpersistent, so this option only needs to be used occasionally; every\n \tfew hundred changesets or so.\n \n+--auto::\n+\tWith this option, `git gc` checks if there are too many\n+\tloose objects in the repository and runs\n+\tgitlink:git-repack[1] with `-d -l` option to pack them.\n+\tThe threshold is set with `gc.auto` configuration\n+\tvariable, and can be disabled by setting it to 0.  Some\n+\tPorcelain commands use this after they perform operation\n+\tthat could create many loose objects automatically.\n+\n Configuration\n -------------\n \n-- \n1.5.3.1.967.g6bb01\n"},{"id":"53270","messageId":"11900187102681-git-send-email-gitster@pobox.com","threadId":"9888","inReplyTo":"11900186941912-git-send-email-gitster@pobox.com","subject":"[PATCH 6/8] git-gc --auto: protect ourselves from accumulated cruft","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-17T08:44:52Z","receivedAt":"2007-09-17T08:44:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Deciding to run \"repack -d -l\" when there are too many\nloose objects would backfire when there are too many loose\nobjects that are unreachable, because repacking that way would\nnever improve the situation.  Detect that case by checking the\nnumber of loose objects again after automatic garbage collection\nruns, and issue an warning to run \"prune\" manually.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-gc.c |   25 +++++++++++++++++--------\n 1 files changed, 17 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin-gc.c b/builtin-gc.c\nindex f046a2a..bf29f5e 100644\n--- a/builtin-gc.c\n+++ b/builtin-gc.c\n@@ -64,7 +64,7 @@ static void append_option(const char **cmd, const char *opt, int max_length)\n \tcmd[i] = NULL;\n }\n \n-static int need_to_gc(void)\n+static int too_many_loose_objects(void)\n {\n \t/*\n \t * Quickly check if a \"gc\" is needed, by estimating how\n@@ -80,13 +80,6 @@ static int need_to_gc(void)\n \tint num_loose = 0;\n \tint needed = 0;\n \n-\t/*\n-\t * Setting gc.auto to 0 or negative can disable the\n-\t * automatic gc\n-\t */\n-\tif (gc_auto_threshold <= 0)\n-\t\treturn 0;\n-\n \tif (sizeof(path) <= snprintf(path, sizeof(path), \"%s/17\", objdir)) {\n \t\twarning(\"insanely long object directory %.*s\", 50, objdir);\n \t\treturn 0;\n@@ -109,6 +102,18 @@ static int need_to_gc(void)\n \treturn needed;\n }\n \n+static int need_to_gc(void)\n+{\n+\t/*\n+\t * Setting gc.auto to 0 or negative can disable the\n+\t * automatic gc\n+\t */\n+\tif (gc_auto_threshold <= 0)\n+\t\treturn 0;\n+\n+\treturn too_many_loose_objects();\n+}\n+\n int cmd_gc(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n@@ -170,5 +175,9 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \tif (run_command_v_opt(argv_rerere, RUN_GIT_CMD))\n \t\treturn error(FAILED_RUN, argv_rerere[0]);\n \n+\tif (auto_gc && too_many_loose_objects())\n+\t\twarning(\"There are too many unreachable loose objects; \"\n+\t\t\t\"run 'git prune' to remove them.\");\n+\n \treturn 0;\n }\n-- \n1.5.3.1.967.g6bb01\n"},{"id":"53271","messageId":"1190018713416-git-send-email-gitster@pobox.com","threadId":"9888","inReplyTo":"11900186941912-git-send-email-gitster@pobox.com","subject":"[PATCH 7/8] git-gc --auto: restructure the way \"repack\" command line is built.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-17T08:44:53Z","receivedAt":"2007-09-17T08:44:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We used to build the command line to run repack outside of\nneed_to_gc() but with the next patch we would want to tweak the\ncommand line depending on the nature of need.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-gc.c |   15 ++++++++++-----\n 1 files changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin-gc.c b/builtin-gc.c\nindex bf29f5e..34ce35b 100644\n--- a/builtin-gc.c\n+++ b/builtin-gc.c\n@@ -29,8 +29,6 @@ static const char *argv_repack[MAX_ADD] = {\"repack\", \"-a\", \"-d\", \"-l\", NULL};\n static const char *argv_prune[] = {\"prune\", NULL};\n static const char *argv_rerere[] = {\"rerere\", \"gc\", NULL};\n \n-static const char *argv_repack_auto[] = {\"repack\", \"-d\", \"-l\", NULL};\n-\n static int gc_config(const char *var, const char *value)\n {\n \tif (!strcmp(var, \"gc.packrefs\")) {\n@@ -104,6 +102,8 @@ static int too_many_loose_objects(void)\n \n static int need_to_gc(void)\n {\n+\tint ac = 0;\n+\n \t/*\n \t * Setting gc.auto to 0 or negative can disable the\n \t * automatic gc\n@@ -111,7 +111,14 @@ static int need_to_gc(void)\n \tif (gc_auto_threshold <= 0)\n \t\treturn 0;\n \n-\treturn too_many_loose_objects();\n+\tif (!too_many_loose_objects())\n+\t\treturn 0;\n+\n+\targv_repack[ac++] = \"repack\";\n+\targv_repack[ac++] = \"-d\";\n+\targv_repack[ac++] = \"-l\";\n+\targv_repack[ac++] = NULL;\n+\treturn 1;\n }\n \n int cmd_gc(int argc, const char **argv, const char *prefix)\n@@ -154,8 +161,6 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\t * Auto-gc should be least intrusive as possible.\n \t\t */\n \t\tprune = 0;\n-\t\tfor (i = 0; i < ARRAY_SIZE(argv_repack_auto); i++)\n-\t\t\targv_repack[i] = argv_repack_auto[i];\n \t\tif (!need_to_gc())\n \t\t\treturn 0;\n \t}\n-- \n1.5.3.1.967.g6bb01\n"},{"id":"53274","messageId":"1190018716666-git-send-email-gitster@pobox.com","threadId":"9888","inReplyTo":"11900186941912-git-send-email-gitster@pobox.com","subject":"[PATCH 8/8] git-gc --auto: run \"repack -A -d -l\" as necessary.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-17T08:44:54Z","receivedAt":"2007-09-17T08:44:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This teaches \"git-gc --auto\" to consolidate many packs into one\nwithout losing unreachable objects in them by using \"repack -A\"\nwhen there are too many packfiles that are not marked with *.keep\nin the repository.  gc.autopacklimit configuration can be used\nto set the maximum number of packs a repository is allowed to\nhave before this mechanism kicks in.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/config.txt |    9 +++++-\n Documentation/git-gc.txt |    7 +++++-\n builtin-gc.c             |   57 +++++++++++++++++++++++++++++++++++++++++-----\n 3 files changed, 64 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 3643c0b..f5136c3 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -443,8 +443,13 @@ gc.auto::\n \tWhen there are approximately more than this many loose\n \tobjects in the repository, `git gc --auto` that is\n \tinvoked by some Porcelain commands will create a new\n-\tpack and prune them.  Setting this to 0 disables the\n-\tauto garbage collection.\n+\tpack and prune them.  Setting this to 0 disables this.\n+\n+gc.autopacklimit::\n+\tWhen there are more than this many packs that are not\n+\tmarked with `*.keep` file in the repository, `git gc\n+\t--auto` consolidates them into one larger pack.  Setting\n+\tthis to 0 disables this.\n \n gc.packrefs::\n \t`git gc` does not run `git pack-refs` in a bare repository by\ndiff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt\nindex 40c1ce4..b9d5660 100644\n--- a/Documentation/git-gc.txt\n+++ b/Documentation/git-gc.txt\n@@ -47,10 +47,15 @@ OPTIONS\n \tWith this option, `git gc` checks if there are too many\n \tloose objects in the repository and runs\n \tgitlink:git-repack[1] with `-d -l` option to pack them.\n-\tThe threshold is set with `gc.auto` configuration\n+\tThe threshold for loose objects is set with `gc.auto` configuration\n \tvariable, and can be disabled by setting it to 0.  Some\n \tPorcelain commands use this after they perform operation\n \tthat could create many loose objects automatically.\n+\tAdditionally, when there are too many packs are present,\n+\tthey are consolidated into one larger pack by running\n+\tthe `git-repack` command with `-A` option.  The\n+\tthreshold for number of packs is set with\n+\t`gc.autopacklimit` configuration variable.\n \n Configuration\n -------------\ndiff --git a/builtin-gc.c b/builtin-gc.c\nindex 34ce35b..a82f6be 100644\n--- a/builtin-gc.c\n+++ b/builtin-gc.c\n@@ -21,6 +21,7 @@ static const char builtin_gc_usage[] = \"git-gc [--prune] [--aggressive]\";\n static int pack_refs = 1;\n static int aggressive_window = -1;\n static int gc_auto_threshold = 6700;\n+static int gc_auto_pack_limit = 20;\n \n #define MAX_ADD 10\n static const char *argv_pack_refs[] = {\"pack-refs\", \"--all\", \"--prune\", NULL};\n@@ -46,6 +47,10 @@ static int gc_config(const char *var, const char *value)\n \t\tgc_auto_threshold = git_config_int(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"gc.autopacklimit\")) {\n+\t\tgc_auto_pack_limit = git_config_int(var, value);\n+\t\treturn 0;\n+\t}\n \treturn git_default_config(var, value);\n }\n \n@@ -78,6 +83,9 @@ static int too_many_loose_objects(void)\n \tint num_loose = 0;\n \tint needed = 0;\n \n+\tif (gc_auto_threshold <= 0)\n+\t\treturn 0;\n+\n \tif (sizeof(path) <= snprintf(path, sizeof(path), \"%s/17\", objdir)) {\n \t\twarning(\"insanely long object directory %.*s\", 50, objdir);\n \t\treturn 0;\n@@ -100,21 +108,58 @@ static int too_many_loose_objects(void)\n \treturn needed;\n }\n \n+static int too_many_packs(void)\n+{\n+\tstruct packed_git *p;\n+\tint cnt;\n+\n+\tif (gc_auto_pack_limit <= 0)\n+\t\treturn 0;\n+\n+\tfor (cnt = 0, p = packed_git; p; p = p->next) {\n+\t\tchar *suffix;\n+\t\tint keep;\n+\t\tif (!p->pack_local)\n+\t\t\tcontinue;\n+\t\tsuffix = p->pack_name + strlen(p->pack_name) - 5;\n+\t\tif (memcmp(suffix, \".pack\", 6))\n+\t\t\tcontinue;\n+\t\tmemcpy(suffix, \".keep\", 6);\n+\t\tkeep = access(p->pack_name, F_OK) && (errno == ENOENT);\n+\t\tmemcpy(suffix, \".pack\", 6);\n+\t\tif (keep)\n+\t\t\tcontinue;\n+\t\t/*\n+\t\t * Perhaps check the size of the pack and count only\n+\t\t * very small ones here?\n+\t\t */\n+\t\tcnt++;\n+\t}\n+\treturn gc_auto_pack_limit <= cnt;\n+}\n+\n static int need_to_gc(void)\n {\n \tint ac = 0;\n \n \t/*\n-\t * Setting gc.auto to 0 or negative can disable the\n-\t * automatic gc\n+\t * Setting gc.auto and gc.autopacklimit to 0 or negative can\n+\t * disable the automatic gc.\n \t */\n-\tif (gc_auto_threshold <= 0)\n-\t\treturn 0;\n-\n-\tif (!too_many_loose_objects())\n+\tif (gc_auto_threshold <= 0 && gc_auto_pack_limit <= 0)\n \t\treturn 0;\n \n+\t/*\n+\t * If there are too many loose objects, but not too many\n+\t * packs, we run \"repack -d -l\".  If there are too many packs,\n+\t * we run \"repack -A -d -l\".  Otherwise we tell the caller\n+\t * there is no need.\n+\t */\n \targv_repack[ac++] = \"repack\";\n+\tif (too_many_packs())\n+\t\targv_repack[ac++] = \"-A\";\n+\tif (!too_many_loose_objects() && ac == 1)\n+\t\treturn 0;\n \targv_repack[ac++] = \"-d\";\n \targv_repack[ac++] = \"-l\";\n \targv_repack[ac++] = NULL;\n-- \n1.5.3.1.967.g6bb01\n"},{"id":"53278","messageId":"Pine.LNX.4.64.0709171027400.28586@racer.site","threadId":"9888","inReplyTo":"11900187002882-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 3/8] repack -A -d: use --keep-unreachable when repacking","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-09-17T09:29:55Z","receivedAt":"2007-09-17T09:29:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 17 Sep 2007, Junio C Hamano wrote:\n\n> -USAGE='[-a] [-d] [-f] [-l] [-n] [-q] [--max-pack-size=N] [--window=N] [--window-memory=N] [--depth=N]'\n> +USAGE='[-a|-A] [-d] [-f] [-l] [-n] [-q] [--max-pack-size=N] [--window=N] [--window-memory=N] [--depth=N]'\n\nWould \"[-a] [-A]\" not be better?  In other usage lines, we have the \"|\" \nfor alternative forms of the _same_ option, like \"[-m|--merge]\".\n\n> +\t-A)\tall_into_one=t\n> +\t\tkeep_unreachable=t ;;\n\nWhy not \"keep_unreachable=--keep-unreachable\" and use \"$args \n$keep_unreachable\" later?\n\nCiao,\nDscho\n"},{"id":"53279","messageId":"Pine.LNX.4.64.0709171031420.28586@racer.site","threadId":"9888","inReplyTo":"11900187073420-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 5/8] git-gc --auto: add documentation.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-09-17T09:36:18Z","receivedAt":"2007-09-17T09:36:18Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 17 Sep 2007, Junio C Hamano wrote:\n\n> +\tWhen there are approximately more than this many loose\n> +\tobjects in the repository, `git gc --auto` that is\n> +\tinvoked by some Porcelain commands will create a new\n> +\tpack and prune them.\n\nIt might strike a newbie as dangerous to read \"prune them\" here.  Maybe\n\n\t... `git gc --auto` will pack the loose objects.  Some Porcelain \n\tcommands use this command, to do a light weight garbage collection\n\tfrom time to time.\n+\n*Note*: you should still run \"git gc\" from time to time, since the \n\"--auto\" option is designed to be fast, whereas \"git gc\" _without_ \nauto is designed to create small packs.\n\nHmm? (I think that this is the most visible part of the --auto business, \nso I would document the \"git gc\" note here...)\n\nCiao,\nDscho\n"},{"id":"53281","messageId":"Pine.LNX.4.64.0709171040070.28586@racer.site","threadId":"9888","inReplyTo":"1190018713416-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 7/8] git-gc --auto: restructure the way \"repack\" command line is built.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-09-17T09:41:40Z","receivedAt":"2007-09-17T09:41:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 17 Sep 2007, Junio C Hamano wrote:\n\n> @@ -154,8 +161,6 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n>  \t\t * Auto-gc should be least intrusive as possible.\n>  \t\t */\n>  \t\tprune = 0;\n> -\t\tfor (i = 0; i < ARRAY_SIZE(argv_repack_auto); i++)\n> -\t\t\targv_repack[i] = argv_repack_auto[i];\n>  \t\tif (!need_to_gc())\n>  \t\t\treturn 0;\n>  \t}\n\nThis subtly changes behaviour: --auto ran also garbage collection for \nreflogs and rerere.\n\nThis might be intended, but is not documented.\n\nCiao,\nDscho\n"},{"id":"53292","messageId":"Pine.LNX.4.64.0709171044170.28586@racer.site","threadId":"9888","inReplyTo":"1190018716666-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 8/8] git-gc --auto: run \"repack -A -d -l\" as necessary.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-09-17T09:53:37Z","receivedAt":"2007-09-17T09:53:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 17 Sep 2007, Junio C Hamano wrote:\n\n> diff --git a/builtin-gc.c b/builtin-gc.c\n> index 34ce35b..a82f6be 100644\n> --- a/builtin-gc.c\n> +++ b/builtin-gc.c\n> @@ -78,6 +83,9 @@ static int too_many_loose_objects(void)\n>  \tint num_loose = 0;\n>  \tint needed = 0;\n>  \n> +\tif (gc_auto_threshold <= 0)\n> +\t\treturn 0;\n> +\n\nHeh, patch 6/8 explicitely moved this check out of the function.\n\n> @@ -100,21 +108,58 @@ static int too_many_loose_objects(void)\n>  \treturn needed;\n>  }\n>  \n> +static int too_many_packs(void)\n> +{\n> +\tstruct packed_git *p;\n> +\tint cnt;\n> +\n> +\tif (gc_auto_pack_limit <= 0)\n> +\t\treturn 0;\n> +\n> +\tfor (cnt = 0, p = packed_git; p; p = p->next) {\n> +\t\tchar *suffix;\n> +\t\tint keep;\n> +\t\tif (!p->pack_local)\n> +\t\t\tcontinue;\n> +\t\tsuffix = p->pack_name + strlen(p->pack_name) - 5;\n\nI suspect that you need something like\n\n\t\tint len;\n\t\tlen = strlen(p->pack_name);\n\t\tif (len < 5)\n\t\t\tcontinue;\n\nbefore this.\n\n> +\t\tif (memcmp(suffix, \".pack\", 6))\n> +\t\t\tcontinue;\n> +\t\tmemcpy(suffix, \".keep\", 6);\n> +\t\tkeep = access(p->pack_name, F_OK) && (errno == ENOENT);\n> +\t\tmemcpy(suffix, \".pack\", 6);\n\nHeh.  Took me some looking around to see why this is allowed...\n\n>  static int need_to_gc(void)\n>  {\n>  \tint ac = 0;\n>  \n>  \t/*\n> -\t * Setting gc.auto to 0 or negative can disable the\n> -\t * automatic gc\n> +\t * Setting gc.auto and gc.autopacklimit to 0 or negative can\n> +\t * disable the automatic gc.\n>  \t */\n> -\tif (gc_auto_threshold <= 0)\n> -\t\treturn 0;\n> -\n> -\tif (!too_many_loose_objects())\n> +\tif (gc_auto_threshold <= 0 && gc_auto_pack_limit <= 0)\n>  \t\treturn 0;\n>  \n> +\t/*\n> +\t * If there are too many loose objects, but not too many\n> +\t * packs, we run \"repack -d -l\".  If there are too many packs,\n> +\t * we run \"repack -A -d -l\".  Otherwise we tell the caller\n> +\t * there is no need.\n> +\t */\n>  \targv_repack[ac++] = \"repack\";\n> +\tif (too_many_packs())\n> +\t\targv_repack[ac++] = \"-A\";\n> +\tif (!too_many_loose_objects() && ac == 1)\n> +\t\treturn 0;\n\nWhy not\n\n\telse if (!too_many_loose_objects())\n\t\treturn 0;\n\nHmm?\n\nCiao,\nDscho\n"},{"id":"53294","messageId":"46EE5037.2080302@op5.se","threadId":"9888","inReplyTo":"Pine.LNX.4.64.0709171027400.28586@racer.site","subject":"Re: [PATCH 3/8] repack -A -d: use --keep-unreachable when repacking","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-09-17T10:00:23Z","receivedAt":"2007-09-17T10:00:23Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Johannes Schindelin wrote:\n> Hi,\n> \n> On Mon, 17 Sep 2007, Junio C Hamano wrote:\n> \n>> -USAGE='[-a] [-d] [-f] [-l] [-n] [-q] [--max-pack-size=N] [--window=N] [--window-memory=N] [--depth=N]'\n>> +USAGE='[-a|-A] [-d] [-f] [-l] [-n] [-q] [--max-pack-size=N] [--window=N] [--window-memory=N] [--depth=N]'\n> \n> Would \"[-a] [-A]\" not be better?  In other usage lines, we have the \"|\" \n> for alternative forms of the _same_ option, like \"[-m|--merge]\".\n> \n\nWell, that depends on how you look at it. -m and --merge are (sort of) mutually\nexclusive, as the use of one voids the use of the other.\n\nLikewise, using -a and -A together on the same command-line doesn't make much\nsense (although -A implies -a rather than meaning the same thing).\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n"},{"id":"53344","messageId":"7vwsupyso8.fsf@gitster.siamese.dyndns.org","threadId":"9888","inReplyTo":"Pine.LNX.4.64.0709171040070.28586@racer.site","subject":"Re: [PATCH 7/8] git-gc --auto: restructure the way \"repack\" command line is built.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-17T19:53:59Z","receivedAt":"2007-09-17T19:53:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> On Mon, 17 Sep 2007, Junio C Hamano wrote:\n>\n>> @@ -154,8 +161,6 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n>>  \t\t * Auto-gc should be least intrusive as possible.\n>>  \t\t */\n>>  \t\tprune = 0;\n>> -\t\tfor (i = 0; i < ARRAY_SIZE(argv_repack_auto); i++)\n>> -\t\t\targv_repack[i] = argv_repack_auto[i];\n>>  \t\tif (!need_to_gc())\n>>  \t\t\treturn 0;\n>>  \t}\n>\n> This subtly changes behaviour: --auto ran also garbage collection for \n> reflogs and rerere.\n\nDoes it change any behaviour?  It \"ran\" meaning \"it used to run\nthem always\"?  I do not think so.  We always exited here if\nthere is no gc needed for object store.\n\nI however think a behaviour change might be needed around here.\n\"gc --auto\" is about being lightweight and no impact in the\nsemantics from the point fo view of the repository user.  As\nsuch, I suspect we may not want to run gc on reflogs nor rerere.\nRunning pack-refs is supposed to be \"no impact in the semantics\"\noperation so I think it is Ok, but even that would affect how\nthe ancient fetch over http implementations would interact with\nthis repository.\n\nBut skipping these would make automated \"behind the scene\" gc\nmuch less useful.  I dunno.\n"},{"id":"53346","messageId":"7vr6kxyso7.fsf@gitster.siamese.dyndns.org","threadId":"9888","inReplyTo":"Pine.LNX.4.64.0709171031420.28586@racer.site","subject":"Re: [PATCH 5/8] git-gc --auto: add documentation.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-17T19:54:00Z","receivedAt":"2007-09-17T19:54:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> On Mon, 17 Sep 2007, Junio C Hamano wrote:\n>\n>> +\tWhen there are approximately more than this many loose\n>> +\tobjects in the repository, `git gc --auto` that is\n>> +\tinvoked by some Porcelain commands will create a new\n>> +\tpack and prune them.\n>\n> It might strike a newbie as dangerous to read \"prune them\" here.  Maybe\n>\n> \t... `git gc --auto` will pack the loose objects.  Some Porcelain \n> \tcommands use this command, to do a light weight garbage collection\n> \tfrom time to time.\n> +\n> *Note*: you should still run \"git gc\" from time to time, since the \n> \"--auto\" option is designed to be fast, whereas \"git gc\" _without_ \n> auto is designed to create small packs.\n>\n> Hmm? (I think that this is the most visible part of the --auto business, \n> so I would document the \"git gc\" note here...)\n\nMakes sense.\n"},{"id":"53345","messageId":"7vlkb5yso5.fsf@gitster.siamese.dyndns.org","threadId":"9888","inReplyTo":"Pine.LNX.4.64.0709171044170.28586@racer.site","subject":"Re: [PATCH 8/8] git-gc --auto: run \"repack -A -d -l\" as necessary.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-17T19:54:02Z","receivedAt":"2007-09-17T19:54:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> diff --git a/builtin-gc.c b/builtin-gc.c\n>> index 34ce35b..a82f6be 100644\n>> --- a/builtin-gc.c\n>> +++ b/builtin-gc.c\n>> @@ -78,6 +83,9 @@ static int too_many_loose_objects(void)\n>>  \tint num_loose = 0;\n>>  \tint needed = 0;\n>>  \n>> +\tif (gc_auto_threshold <= 0)\n>> +\t\treturn 0;\n>> +\n>\n> Heh, patch 6/8 explicitely moved this check out of the function.\n\nBack in 6/8 there was only one toggle to disable auto gc\nand it made sense to have it there.  Now the two switches act\ntogether, so that you can say \"Don't count my loose objects, but\ndo check the number of packs\" if you wanted to.\n\n>> +\t\tif (!p->pack_local)\n>> +\t\t\tcontinue;\n>> +\t\tsuffix = p->pack_name + strlen(p->pack_name) - 5;\n>\n> I suspect that you need something like\n>\n> \t\tint len;\n> \t\tlen = strlen(p->pack_name);\n> \t\tif (len < 5)\n> \t\t\tcontinue;\n>\n> before this.\n\nWhile your additional check would not hurt, I actually think it\nis the other way around; the code is already overly cautious.\nIf it is linked to packed_git list, the file should have already\nbeen checked for having the suffix \".pack\".\n\n>> +\t/*\n>> +\t * If there are too many loose objects, but not too many\n>> +\t * packs, we run \"repack -d -l\".  If there are too many packs,\n>> +\t * we run \"repack -A -d -l\".  Otherwise we tell the caller\n>> +\t * there is no need.\n>> +\t */\n>>  \targv_repack[ac++] = \"repack\";\n>> +\tif (too_many_packs())\n>> +\t\targv_repack[ac++] = \"-A\";\n>> +\tif (!too_many_loose_objects() && ac == 1)\n>> +\t\treturn 0;\n>\n> Why not\n>\n> \telse if (!too_many_loose_objects())\n> \t\treturn 0;\n>\n\nOk.\n"},{"id":"53393","messageId":"20070918025926.GI3099@spearce.org","threadId":"9888","inReplyTo":"1190018716666-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 8/8] git-gc --auto: run \"repack -A -d -l\" as necessary.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-09-18T02:59:26Z","receivedAt":"2007-09-18T02:59:26Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n> +static int too_many_packs(void)\n> +{\n> +\tstruct packed_git *p;\n> +\tint cnt;\n> +\n> +\tif (gc_auto_pack_limit <= 0)\n> +\t\treturn 0;\n> +\n> +\tfor (cnt = 0, p = packed_git; p; p = p->next) {\n> +\t\tchar *suffix;\n> +\t\tint keep;\n> +\t\tif (!p->pack_local)\n> +\t\t\tcontinue;\n> +\t\tsuffix = p->pack_name + strlen(p->pack_name) - 5;\n> +\t\tif (memcmp(suffix, \".pack\", 6))\n> +\t\t\tcontinue;\n> +\t\tmemcpy(suffix, \".keep\", 6);\n> +\t\tkeep = access(p->pack_name, F_OK) && (errno == ENOENT);\n> +\t\tmemcpy(suffix, \".pack\", 6);\n\n*rubs eyes* Am I reading that right?  You are modifying pack_name\nin the middle of this function?  WHY is pack_name not const char*?\n*sigh*.  I'd NEVER consider doing that.  Not in a million years.\nBecause someday someone is going to make a change that's going to\nbreak that somehow, like by jumping out of this loop early and not\nfixing pack_name up first.\n\nYea, sure, there's no way today that this would fail.  But I like\nto think that some poor sap is going to have to read my code after\nI write it and that they should not be forced to deal with stuff\nlike this when such a time comes.\n\n-- \nShawn.\n"},{"id":"53394","messageId":"20070918030156.GJ3099@spearce.org","threadId":"9888","inReplyTo":"Pine.LNX.4.64.0709171027400.28586@racer.site","subject":"Re: [PATCH 3/8] repack -A -d: use --keep-unreachable when repacking","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-09-18T03:01:56Z","receivedAt":"2007-09-18T03:01:56Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> On Mon, 17 Sep 2007, Junio C Hamano wrote:\n> \n> > -USAGE='[-a] [-d] [-f] [-l] [-n] [-q] [--max-pack-size=N] [--window=N] [--window-memory=N] [--depth=N]'\n> > +USAGE='[-a|-A] [-d] [-f] [-l] [-n] [-q] [--max-pack-size=N] [--window=N] [--window-memory=N] [--depth=N]'\n> \n> Would \"[-a] [-A]\" not be better?  In other usage lines, we have the \"|\" \n> for alternative forms of the _same_ option, like \"[-m|--merge]\".\n> \n> > +\t-A)\tall_into_one=t\n> > +\t\tkeep_unreachable=t ;;\n> \n> Why not \"keep_unreachable=--keep-unreachable\" and use \"$args \n> $keep_unreachable\" later?\n\nI agree.  Aside from the above and my other reply about editing\npack_name in place to test the .keep file I'm happy with this series.\nI know the automatic GC thing has been a hot topic in the past with\nvarious parties on different sides of the fence.  I think this is\na pretty reasonable implementation that actually is a fairly good\ncomprimise between the two sides.\n\nThanks Junio.\n\n-- \nShawn.\n"}]}