{"thread":{"id":"34687","subject":"[PATCH] Rewriting git-repack in C","startedAt":"2013-08-13T19:23:16Z","lastAt":"2013-08-22T21:03:53Z","messageCount":72,"participants":["Stefan Beller","Matthieu Moy","Antoine Pelisse","Junio C Hamano","Jeff King","Martin Fick","Duy Nguyen","René Scharfe","Kyle J. McKay","Johannes Sixt","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"225163","messageId":"1376421797-26443-1-git-send-email-stefanbeller@googlemail.com","threadId":"34687","inReplyTo":null,"subject":"[PATCH] Rewriting git-repack in C","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-08-13T19:23:16Z","receivedAt":"2013-08-13T19:23:16Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Hello,\n\na few days ago, I asked how I would proceed if I'd want to rewrite git-repack.\nThe general consensus (Duy, Junio and Matthieu) was to not touch\ngit-pack-objects, but rather translate the shell version of the git-repack\nto C.\nI'll send a very rough patch, which still contains 2 todos, so it is not yet\nfeature complete. I was using the run_command api and the string-list api\na lot.\nThe following patch is definitle not recommended for a fine review, but\nI'd still ask for feedback, whether this seems like the right approach.\n\nAlso I'd like to propose a small technical change:\nI found no easy way to get a subset of files in a specific directory,\nwhich is very easy in the shell version via * (rm file/in/dir/startswithprefix-*),\nSo maybe instead of just prefixing the temporary files such as:\n$PACKDIR/.tmp-$PID-pack-*\nwe could put all the temporary files in a temporary directory.\nThis would come handy as there already exists functions to manipulate \na directory recursively (such as remove_dir_recursively)\n\nThat patch requires the patches of jc/parseopt-command-modes\n\nStefan\n\nStefan Beller (1):\n repack: rewrite the shell script in C.\n\n Makefile | 2 +-\n builtin.h | 1 +\n builtin/repack.c | 313 +++++++++++++++++++++++++++++++++++++++++\n contrib/examples/git-repack.sh | 194 +++++++++++++++++++++++++\n git-repack.sh | 194 -------------------------\n git.c | 1 +\n 6 files changed, 510 insertions(+), 195 deletions(-)\n create mode 100644 builtin/repack.c\n create mode 100755 contrib/examples/git-repack.sh\n delete mode 100755 git-repack.sh\n\n-- \n1.8.4.rc2\n"},{"id":"225164","messageId":"1376421797-26443-2-git-send-email-stefanbeller@googlemail.com","threadId":"34687","inReplyTo":"1376421797-26443-1-git-send-email-stefanbeller@googlemail.com","subject":"[PATCH] repack: rewrite the shell script in C.","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-08-13T19:23:17Z","receivedAt":"2013-08-13T19:23:17Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This is the beginning of the rewrite of the repacking.\n\nSigned-off-by: Stefan Beller \n---\n Makefile | 2 +-\n builtin.h | 1 +\n builtin/repack.c | 313 +++++++++++++++++++++++++++++++++++++++++\n contrib/examples/git-repack.sh | 194 +++++++++++++++++++++++++\n git-repack.sh | 194 -------------------------\n git.c | 1 +\n 6 files changed, 510 insertions(+), 195 deletions(-)\n create mode 100644 builtin/repack.c\n create mode 100755 contrib/examples/git-repack.sh\n delete mode 100755 git-repack.sh\n\ndiff --git a/Makefile b/Makefile\nindex 3588ca1..69e5267 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -464,7 +464,6 @@ SCRIPT_SH += git-pull.sh\n SCRIPT_SH += git-quiltimport.sh\n SCRIPT_SH += git-rebase.sh\n SCRIPT_SH += git-remote-testgit.sh\n-SCRIPT_SH += git-repack.sh\n SCRIPT_SH += git-request-pull.sh\n SCRIPT_SH += git-stash.sh\n SCRIPT_SH += git-submodule.sh\n@@ -971,6 +970,7 @@ BUILTIN_OBJS += builtin/reflog.o\n BUILTIN_OBJS += builtin/remote.o\n BUILTIN_OBJS += builtin/remote-ext.o\n BUILTIN_OBJS += builtin/remote-fd.o\n+BUILTIN_OBJS += builtin/repack.o\n BUILTIN_OBJS += builtin/replace.o\n BUILTIN_OBJS += builtin/rerere.o\n BUILTIN_OBJS += builtin/reset.o\ndiff --git a/builtin.h b/builtin.h\nindex 8afa2de..b56cf07 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -102,6 +102,7 @@ extern int cmd_reflog(int argc, const char **argv, const char *prefix);\n extern int cmd_remote(int argc, const char **argv, const char *prefix);\n extern int cmd_remote_ext(int argc, const char **argv, const char *prefix);\n extern int cmd_remote_fd(int argc, const char **argv, const char *prefix);\n+extern int cmd_repack(int argc, const char **argv, const char *prefix);\n extern int cmd_repo_config(int argc, const char **argv, const char *prefix);\n extern int cmd_rerere(int argc, const char **argv, const char *prefix);\n extern int cmd_reset(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/repack.c b/builtin/repack.c\nnew file mode 100644\nindex 0000000..bfec56d\n--- /dev/null\n+++ b/builtin/repack.c\n@@ -0,0 +1,313 @@\n+/*\n+ * The shell version was written by Linus Torvalds (2005) and many others.\n+ * This is a translation into C by Stefan Beller (2013)\n+ */\n+\n+#include \"builtin.h\"\n+#include \"cache.h\"\n+#include \"dir.h\"\n+#include \"parse-options.h\"\n+#include \"run-command.h\"\n+#include \"sigchain.h\"\n+#include \"strbuf.h\"\n+#include \"string-list.h\"\n+\n+#include \n+#include \n+#include \n+#include \n+\n+static const char * const git_repack_usage[] = {\n+\tN_(\"git repack [options]\"),\n+\tNULL\n+};\n+\n+static int delta_base_offset = 1; // enabled by default since 22c79eab (2008-06-25)\n+\n+static int repack_config(const char *var, const char *value, void *cb)\n+{\n+\tif (!strcmp(var, \"repack.usedeltabaseoffset\")) {\n+\t\tdelta_base_offset = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\treturn git_default_config(var, value, cb);\n+}\n+\n+static void remove_pack_on_signal(int signo)\n+{\n+\tstruct strbuf packtmpbuf = STRBUF_INIT;\n+\t/* todo: the files are not in a subdirectory, but only a subset in %s/pack, see disscussion */\n+\tstrbuf_addf(&packtmpbuf, \"%s/pack/.tmp-%d-pack\", get_object_directory(), getpid());\n+\tremove_dir_recursively(&packtmpbuf, 0);\n+\tsigchain_pop(signo);\n+\traise(signo);\n+}\n+\n+int cmd_repack(int argc, const char **argv, const char *prefix) {\n+\n+\tint pack_everything = 0;\n+\tint pack_everything_but_loose = 0;\n+\tint delete_redundant = 0;\n+\tint unpack_unreachable = 0;\n+\tint window = 0, window_memory = 0;\n+\tint depth = 0;\n+\tint max_pack_size = 0;\n+\tint no_reuse_delta = 0, no_reuse_object = 0;\n+\tint no_update_server_info = 0;\n+\tint quiet = 0;\n+\tint local = 0;\n+\tchar *packdir, *packtmp;\n+\tconst char *cmd_args[20];\n+\tint cmd_i = 0;\n+\tstruct child_process cmd;\n+\n+\tstruct option builtin_repack_options[] = {\n+\t\tOPT_BOOL('a', \"all\", &pack_everything,\n+\t\t\t\tN_(\"pack everything in a single pack\")),\n+\t\tOPT_BOOL('A', \"all-but-loose\", &pack_everything_but_loose,\n+\t\t\t\tN_(\"same as -a, and turn unreachable objects loose\")),\n+\t\tOPT_BOOL('d', \"delete-redundant\", &delete_redundant,\n+\t\t\t\tN_(\"remove redundant packs, and run git-prune-packed\")),\n+\t\tOPT_BOOL('f', \"no-reuse-delta\", &no_reuse_delta,\n+\t\t\t\tN_(\"pass --no-reuse-delta to git-pack-objects\")),\n+\t\tOPT_BOOL('F', \"no-reuse-object\", &no_reuse_object,\n+\t\t\t\tN_(\"pass --no-reuse-object to git-pack-objects\")),\n+\t\tOPT_BOOL('n', NULL, &no_update_server_info,\n+\t\t\t\tN_(\"do not run git-update-server-info\")),\n+\t\tOPT__QUIET(&quiet, N_(\"be quiet\")),\n+\t\tOPT_BOOL('l', \"local\", &local,\n+\t\t\t\tN_(\"pass --local to git-pack-objects\")),\n+\t\tOPT_DATE(0, \"unpack-unreachable\", &unpack_unreachable,\n+\t\t\t\tN_(\"with -A, do not loosen objects older than this Packing constraints\")),\n+\t\tOPT_INTEGER(0, \"window\", &window,\n+\t\t\t\tN_(\"size of the window used for delta compression\")),\n+\t\tOPT_INTEGER(0, \"window-memory\", &window_memory,\n+\t\t\t\tN_(\"same as the above, but limit memory size instead of entries count\")),\n+\t\tOPT_INTEGER(0, \"depth\", &depth,\n+\t\t\t\tN_(\"limits the maximum delta depth\")),\n+\t\tOPT_INTEGER(0, \"max-pack-size\", &max_pack_size,\n+\t\t\t\tN_(\"maximum size of each packfile\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tgit_config(repack_config, NULL);\n+\n+\targc = parse_options(argc, argv, prefix, builtin_repack_options,\n+\t\t\t\tgit_repack_usage, 0);\n+\n+\tsigchain_push_common(remove_pack_on_signal);\n+\n+\tpackdir = mkpath(\"%s/pack\", get_object_directory());\n+\tpacktmp = mkpath(\"%s/.tmp-%d-pack\", packdir, getpid());\n+\n+\tstruct strbuf packtmpbuf = STRBUF_INIT;\n+\tstrbuf_addf(&packtmpbuf, \"%s\", packtmp);\n+\tremove_dir_recursively(&packtmpbuf, 0);\n+\n+\tcmd_args[cmd_i++] = \"pack-objects\";\n+\tcmd_args[cmd_i++] = \"--keep-true-parents\";\n+\tcmd_args[cmd_i++] = \"--honor-pack-keep\";\n+\tcmd_args[cmd_i++] = \"--non-empty\";\n+\tcmd_args[cmd_i++] = \"--all\";\n+\tcmd_args[cmd_i++] = \"--reflog\";\n+\n+\tif (pack_everything + pack_everything_but_loose == 0) {\n+\t\tcmd_args[cmd_i++] = \"--unpacked\";\n+\t\tcmd_args[cmd_i++] = \"--incremental\";\n+\t} else {\n+\t\t// todo:\n+\t\t// handle -a and -A option here.\n+\t\t// find all *.pack in pack dir:\n+\n+/*\n+\tif [ -d \"$PACKDIR\" ]; then\n+\t\t# cd into packdir and find all packs, sed will remove beginning ./ and ending .pack\n+\t\tfor e in `cd \"$PACKDIR\" && find . -type f -name '*.pack' \\\n+\t\t\t| sed -e 's/^\\.\\///' -e 's/\\.pack$//'`\n+\t\tdo\n+\t\t\tif [ -e \"$PACKDIR/$e.keep\" ]; then\n+\t\t\t\t: keep\n+\t\t\t\t# do nothing\n+\t\t\telse\n+\t\t\t\t# add the pack to the existing list\n+\t\t\t\texisting=\"$existing $e\"\n+\t\t\tfi\n+\t\tdone\n+\t\t# -n string True if the length of string is nonzero.\n+\t\t# -a and\n+\t\tif test -n \"$existing\" -a -n \"$unpack_unreachable\" -a \\\n+\t\t\t-n \"$remove_redundant\"\n+\t\tthen\n+\t\t\t# This may have arbitrary user arguments, so we\n+\t\t\t# have to protect it against whitespace splitting\n+\t\t\t# when it gets run as \"pack-objects $args\" later.\n+\t\t\t# Fortunately, we know it's an approxidate, so we\n+\t\t\t# can just use dots instead.\n+\t\t\targs=\"$args $(echo \"$unpack_unreachable\" | tr ' ' .)\"\n+\t\tfi\n+\tfi\n+*/\n+\t}\n+\n+\tif (local)\n+\t\tcmd_args[cmd_i++] = \"--local\";\n+\n+\tif (delta_base_offset)\n+\t\tcmd_args[cmd_i++] = \"--delta-base-offset\";\n+\n+\tcmd_args[cmd_i++] = packtmp;\n+\tcmd_args[cmd_i] = NULL;\n+\n+\tmemset(&cmd, 0, sizeof(cmd));\n+\tcmd.argv = cmd_args;\n+\tcmd.git_cmd = 1;\n+\tcmd.out = -1;\n+\tcmd.no_stdin = 1;\n+\n+\trun_command(&cmd);\n+\n+\tstruct string_list names = STRING_LIST_INIT_DUP;\n+\tstruct string_list rollback = STRING_LIST_INIT_DUP;\n+\n+\tchar line[1024];\n+\tint counter = 0;\n+\tFILE *out = xfdopen(cmd.out, \"r\");\n+\twhile (fgets(line, sizeof(line), out)) {\n+\t\tassert(strlen(line) == 41); // 40 chars for sha1 + '\\n'\n+\t\tline[40] = '\\0'; // remove '\\n'\n+\t\tstring_list_append(&names, line);\n+\t\tcounter++;\n+\t}\n+\tif (!counter)\n+\t\tprintf(\"Nothing new to pack.\\n\");\n+\tfclose(out);\n+\n+\tchar *fname, *fname_old;\n+\tfname = xmalloc(strlen(packdir) + 47); // 40 chars for the sha1\n+\tstrcpy(fname, packdir);\n+\tstrcpy(fname + strlen(packdir), \"/\");\n+\n+\tfname_old = xmalloc(strlen(packdir) + 52); // 40 chars for the sha1\n+\tstrcpy(fname_old, packdir);\n+\tstrcpy(fname_old + strlen(packdir), \"/\");\n+\tchar *exts[2] = {\".idx\", \".pack\"};\n+\n+\tint failed = 0;\n+\n+\tstruct string_list_item *item;\n+\tfor_each_string_list_item(item, &names) {\n+\t\tint ext;\n+\t\tfor (ext = 0; ext < 1; ext++) {\n+\t\t\tstrcpy(fname + strlen(packdir) + 1, item->string);\n+\t\t\tstrcpy(fname + strlen(packdir) + 41, exts[ext]);\n+\t\t\tif (!file_exists(fname))\n+\t\t\t\tcontinue;\n+\n+\t\t\tstrcpy(fname_old, packdir);\n+\t\t\tstrcpy(fname_old + strlen(packdir), \"/old-\");\n+\t\t\tstrcpy(fname_old + strlen(packdir) + 5, item->string);\n+\t\t\tstrcpy(fname_old + strlen(packdir) + 45, exts[ext]);\n+\t\t\tif (file_exists(fname_old))\n+\t\t\t\tunlink(fname_old);\n+\n+\t\t\tif (rename(fname, fname_old)) {\n+\t\t\t\tfailed = 1;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tstring_list_append(&rollback, fname);\n+\t\t}\n+\t\tif (failed)\n+\t\t\t/* set to last element to break while loop */\n+\t\t\titem = names.items + names.nr;\n+\t}\n+\tif (failed) {\n+\t\tstruct string_list rollback_failure;\n+\n+\t\tfor_each_string_list_item(item, &rollback) {\n+\t\t\tsprintf(fname, \"%s/%s\", packdir, item->string);\n+\t\t\tsprintf(fname_old, \"%s/old-%s\", packdir, item->string);\n+\t\t\tif (rename(fname_old, fname))\n+\t\t\t\tstring_list_append(&rollback_failure, fname);\n+\t\t}\n+\n+\t\tif (rollback.nr) {\n+\t\t\tint i;\n+\t\t\tfprintf(stderr,\n+\t\t\t\t\"WARNING: Some packs in use have been renamed by\\n\"\n+\t\t\t\t\"WARNING: prefixing old- to their name, in order to\\n\"\n+\t\t\t\t\"WARNING: replace them with the new version of the\\n\"\n+\t\t\t\t\"WARNING: file. But the operation failed, and\\n\"\n+\t\t\t\t\"WARNING: attempt to rename them back to their\\n\"\n+\t\t\t\t\"WARNING: original names also failed.\\n\"\n+\t\t\t\t\"WARNING: Please rename them in $PACKDIR manually:\\n\");\n+\t\t\tfor (i = 0; i < rollback.nr; i++)\n+\t\t\t\tfprintf(stderr, \"WARNING: old-%s -> %s\\n\",\n+\t\t\t\t\trollback.items[i].string,\n+\t\t\t\t\trollback.items[i].string);\n+\t\t}\n+\t\texit(1);\n+\t}\n+\n+\t// Now the ones with the same name are out of the way...\n+\tstruct string_list fullbases = STRING_LIST_INIT_DUP;\n+\tfor_each_string_list_item(item, &names) {\n+\t\tstring_list_append(&fullbases, item->string);\n+\n+\t\tsprintf(fname, \"%s/pack-%s.pack\", packdir, item->string);\n+\t\tsprintf(fname_old, \"%s-%s.pack\", packtmp, item->string);\n+\t\tchmod(fname_old, 0660);\n+\t\tif (rename(fname_old, fname))\n+\t\t\tdie(\"Could not rename packfile: %s -> %s\", fname_old, fname);\n+\n+\t\tsprintf(fname, \"%s/pack-%s.idx\", packdir, item->string);\n+\t\tsprintf(fname_old, \"%s-%s.idx\", packtmp, item->string);\n+\t\tchmod(fname_old, 0660); // todo, in shell there was \"chmod a-w ...\"\n+\t\tif (rename(fname_old, fname))\n+\t\t\tdie(\"Could not rename packfile: %s -> %s\", fname_old, fname);\n+\t}\n+\n+\t/* Remove the \"old-\" files */\n+\tfor_each_string_list_item(item, &names) {\n+\t\tsprintf(fname, \"%s/old-pack-%s.idx\", packdir, item->string);\n+\t\tif (remove_path(fname))\n+\t\t\tdie(\"Could not remove file: %s\", fname);\n+\n+\t\tsprintf(fname, \"%s/old-pack-%s.pack\", packdir, item->string);\n+\t\tif (remove_path(fname))\n+\t\t\tdie(\"Could not remove file: %s\", fname);\n+\t}\n+\n+\t/* End of pack replacement. */\n+\tif (delete_redundant) {\n+\t\t// todo\n+\t\t/*\n+\t\t\t# We know $existing are all redundant.\n+\t\t\tif [ -n \"$existing\" ]\n+\t\t\tthen\n+\t\t\t\t( cd \"$PACKDIR\" &&\n+\t\t\t\t for e in $existing\n+\t\t\t\t do\n+\t\t\t\t\tcase \" $fullbases \" in\n+\t\t\t\t\t*\" $e \"*) ;;\n+\t\t\t\t\t*)\trm -f \"$e.pack\" \"$e.idx\" \"$e.keep\" ;;\n+\t\t\t\t\tesac\n+\t\t\t\t done\n+\t\t\t\t)\n+\t\t\tfi\n+\t\t\tgit prune-packed ${GIT_QUIET:+-q}\n+\t\t*/\n+\t}\n+\n+\tif (!no_update_server_info) {\n+\t\tcmd_i = 0;\n+\t\tcmd_args[cmd_i++] = \"update-server-info\";\n+\t\tcmd_args[cmd_i++] = NULL;\n+\n+\t\tmemset(&cmd, 0, sizeof(cmd));\n+\t\tcmd.argv = cmd_args;\n+\t\tcmd.git_cmd = 1;\n+\t\trun_command(&cmd);\n+\t}\n+\treturn 0;\n+}\n+\ndiff --git a/contrib/examples/git-repack.sh b/contrib/examples/git-repack.sh\nnew file mode 100755\nindex 0000000..7579331\n--- /dev/null\n+++ b/contrib/examples/git-repack.sh\n@@ -0,0 +1,194 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2005 Linus Torvalds\n+#\n+\n+OPTIONS_KEEPDASHDASH=\n+OPTIONS_SPEC=\"\\\n+git repack [options]\n+--\n+a pack everything in a single pack\n+A same as -a, and turn unreachable objects loose\n+d remove redundant packs, and run git-prune-packed\n+f pass --no-reuse-delta to git-pack-objects\n+F pass --no-reuse-object to git-pack-objects\n+n do not run git-update-server-info\n+q,quiet be quiet\n+l pass --local to git-pack-objects\n+unpack-unreachable= with -A, do not loosen objects older than this\n+ Packing constraints\n+window= size of the window used for delta compression\n+window-memory= same as the above, but limit memory size instead of entries count\n+depth= limits the maximum delta depth\n+max-pack-size= maximum size of each packfile\n+\"\n+SUBDIRECTORY_OK='Yes'\n+. git-sh-setup\n+\n+no_update_info= all_into_one= remove_redundant= unpack_unreachable=\n+local= no_reuse= extra=\n+while test $# != 0\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\tunpack_unreachable=--unpack-unreachable ;;\n+\t--unpack-unreachable)\n+\t\tunpack_unreachable=\"--unpack-unreachable=$2\"; shift ;;\n+\t-d)\tremove_redundant=t ;;\n+\t-q)\tGIT_QUIET=t ;;\n+\t-f)\tno_reuse=--no-reuse-delta ;;\n+\t-F)\tno_reuse=--no-reuse-object ;;\n+\t-l)\tlocal=--local ;;\n+\t--max-pack-size|--window|--window-memory|--depth)\n+\t\textra=\"$extra $1=$2\"; shift ;;\n+\t--) shift; break;;\n+\t*)\tusage ;;\n+\tesac\n+\tshift\n+done\n+\n+case \"`git config --bool repack.usedeltabaseoffset || echo true`\" in\n+true)\n+\textra=\"$extra --delta-base-offset\" ;;\n+esac\n+\n+PACKDIR=\"$GIT_OBJECT_DIRECTORY/pack\"\n+PACKTMP=\"$PACKDIR/.tmp-$$-pack\"\n+rm -f \"$PACKTMP\"-*\n+trap 'rm -f \"$PACKTMP\"-*' 0 1 2 3 15\n+\n+# There will be more repacking strategies to come...\n+case \",$all_into_one,\" in\n+,,)\n+\targs='--unpacked --incremental'\n+\t;;\n+,t,)\n+\targs= existing=\n+\tif [ -d \"$PACKDIR\" ]; then\n+\t\tfor e in `cd \"$PACKDIR\" && find . -type f -name '*.pack' \\\n+\t\t\t| sed -e 's/^\\.\\///' -e 's/\\.pack$//'`\n+\t\tdo\n+\t\t\tif [ -e \"$PACKDIR/$e.keep\" ]; then\n+\t\t\t\t: keep\n+\t\t\telse\n+\t\t\t\texisting=\"$existing $e\"\n+\t\t\tfi\n+\t\tdone\n+\t\tif test -n \"$existing\" -a -n \"$unpack_unreachable\" -a \\\n+\t\t\t-n \"$remove_redundant\"\n+\t\tthen\n+\t\t\t# This may have arbitrary user arguments, so we\n+\t\t\t# have to protect it against whitespace splitting\n+\t\t\t# when it gets run as \"pack-objects $args\" later.\n+\t\t\t# Fortunately, we know it's an approxidate, so we\n+\t\t\t# can just use dots instead.\n+\t\t\targs=\"$args $(echo \"$unpack_unreachable\" | tr ' ' .)\"\n+\t\tfi\n+\tfi\n+\t;;\n+esac\n+\n+mkdir -p \"$PACKDIR\" || exit\n+\n+args=\"$args $local ${GIT_QUIET:+-q} $no_reuse$extra\"\n+names=$(git pack-objects --keep-true-parents --honor-pack-keep --non-empty --all --reflog $args &2 \"WARNING: Some packs in use have been renamed by\"\n+\t\techo >&2 \"WARNING: prefixing old- to their name, in order to\"\n+\t\techo >&2 \"WARNING: replace them with the new version of the\"\n+\t\techo >&2 \"WARNING: file. But the operation failed, and\"\n+\t\techo >&2 \"WARNING: attempt to rename them back to their\"\n+\t\techo >&2 \"WARNING: original names also failed.\"\n+\t\techo >&2 \"WARNING: Please rename them in $PACKDIR manually:\"\n+\t\tfor file in $rollback_failure\n+\t\tdo\n+\t\t\techo >&2 \"WARNING: old-$file -> $file\"\n+\t\tdone\n+\tfi\n+\texit 1\n+fi\n+\n+# Now the ones with the same name are out of the way...\n+fullbases=\n+for name in $names\n+do\n+\tfullbases=\"$fullbases pack-$name\"\n+\tchmod a-w \"$PACKTMP-$name.pack\"\n+\tchmod a-w \"$PACKTMP-$name.idx\"\n+\tmv -f \"$PACKTMP-$name.pack\" \"$PACKDIR/pack-$name.pack\" &&\n+\tmv -f \"$PACKTMP-$name.idx\" \"$PACKDIR/pack-$name.idx\" ||\n+\texit\n+done\n+\n+# Remove the \"old-\" files\n+for name in $names\n+do\n+\trm -f \"$PACKDIR/old-pack-$name.idx\"\n+\trm -f \"$PACKDIR/old-pack-$name.pack\"\n+done\n+\n+# End of pack replacement.\n+\n+if test \"$remove_redundant\" = t\n+then\n+\t# We know $existing are all redundant.\n+\tif [ -n \"$existing\" ]\n+\tthen\n+\t\t( cd \"$PACKDIR\" &&\n+\t\t for e in $existing\n+\t\t do\n+\t\t\tcase \" $fullbases \" in\n+\t\t\t*\" $e \"*) ;;\n+\t\t\t*)\trm -f \"$e.pack\" \"$e.idx\" \"$e.keep\" ;;\n+\t\t\tesac\n+\t\t done\n+\t\t)\n+\tfi\n+\tgit prune-packed ${GIT_QUIET:+-q}\n+fi\n+\n+case \"$no_update_info\" in\n+t) : ;;\n+*) git update-server-info ;;\n+esac\ndiff --git a/git-repack.sh b/git-repack.sh\ndeleted file mode 100755\nindex 7579331..0000000\n--- a/git-repack.sh\n+++ /dev/null\n@@ -1,194 +0,0 @@\n-#!/bin/sh\n-#\n-# Copyright (c) 2005 Linus Torvalds\n-#\n-\n-OPTIONS_KEEPDASHDASH=\n-OPTIONS_SPEC=\"\\\n-git repack [options]\n---\n-a pack everything in a single pack\n-A same as -a, and turn unreachable objects loose\n-d remove redundant packs, and run git-prune-packed\n-f pass --no-reuse-delta to git-pack-objects\n-F pass --no-reuse-object to git-pack-objects\n-n do not run git-update-server-info\n-q,quiet be quiet\n-l pass --local to git-pack-objects\n-unpack-unreachable= with -A, do not loosen objects older than this\n- Packing constraints\n-window= size of the window used for delta compression\n-window-memory= same as the above, but limit memory size instead of entries count\n-depth= limits the maximum delta depth\n-max-pack-size= maximum size of each packfile\n-\"\n-SUBDIRECTORY_OK='Yes'\n-. git-sh-setup\n-\n-no_update_info= all_into_one= remove_redundant= unpack_unreachable=\n-local= no_reuse= extra=\n-while test $# != 0\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\tunpack_unreachable=--unpack-unreachable ;;\n-\t--unpack-unreachable)\n-\t\tunpack_unreachable=\"--unpack-unreachable=$2\"; shift ;;\n-\t-d)\tremove_redundant=t ;;\n-\t-q)\tGIT_QUIET=t ;;\n-\t-f)\tno_reuse=--no-reuse-delta ;;\n-\t-F)\tno_reuse=--no-reuse-object ;;\n-\t-l)\tlocal=--local ;;\n-\t--max-pack-size|--window|--window-memory|--depth)\n-\t\textra=\"$extra $1=$2\"; shift ;;\n-\t--) shift; break;;\n-\t*)\tusage ;;\n-\tesac\n-\tshift\n-done\n-\n-case \"`git config --bool repack.usedeltabaseoffset || echo true`\" in\n-true)\n-\textra=\"$extra --delta-base-offset\" ;;\n-esac\n-\n-PACKDIR=\"$GIT_OBJECT_DIRECTORY/pack\"\n-PACKTMP=\"$PACKDIR/.tmp-$$-pack\"\n-rm -f \"$PACKTMP\"-*\n-trap 'rm -f \"$PACKTMP\"-*' 0 1 2 3 15\n-\n-# There will be more repacking strategies to come...\n-case \",$all_into_one,\" in\n-,,)\n-\targs='--unpacked --incremental'\n-\t;;\n-,t,)\n-\targs= existing=\n-\tif [ -d \"$PACKDIR\" ]; then\n-\t\tfor e in `cd \"$PACKDIR\" && find . -type f -name '*.pack' \\\n-\t\t\t| sed -e 's/^\\.\\///' -e 's/\\.pack$//'`\n-\t\tdo\n-\t\t\tif [ -e \"$PACKDIR/$e.keep\" ]; then\n-\t\t\t\t: keep\n-\t\t\telse\n-\t\t\t\texisting=\"$existing $e\"\n-\t\t\tfi\n-\t\tdone\n-\t\tif test -n \"$existing\" -a -n \"$unpack_unreachable\" -a \\\n-\t\t\t-n \"$remove_redundant\"\n-\t\tthen\n-\t\t\t# This may have arbitrary user arguments, so we\n-\t\t\t# have to protect it against whitespace splitting\n-\t\t\t# when it gets run as \"pack-objects $args\" later.\n-\t\t\t# Fortunately, we know it's an approxidate, so we\n-\t\t\t# can just use dots instead.\n-\t\t\targs=\"$args $(echo \"$unpack_unreachable\" | tr ' ' .)\"\n-\t\tfi\n-\tfi\n-\t;;\n-esac\n-\n-mkdir -p \"$PACKDIR\" || exit\n-\n-args=\"$args $local ${GIT_QUIET:+-q} $no_reuse$extra\"\n-names=$(git pack-objects --keep-true-parents --honor-pack-keep --non-empty --all --reflog $args &2 \"WARNING: Some packs in use have been renamed by\"\n-\t\techo >&2 \"WARNING: prefixing old- to their name, in order to\"\n-\t\techo >&2 \"WARNING: replace them with the new version of the\"\n-\t\techo >&2 \"WARNING: file. But the operation failed, and\"\n-\t\techo >&2 \"WARNING: attempt to rename them back to their\"\n-\t\techo >&2 \"WARNING: original names also failed.\"\n-\t\techo >&2 \"WARNING: Please rename them in $PACKDIR manually:\"\n-\t\tfor file in $rollback_failure\n-\t\tdo\n-\t\t\techo >&2 \"WARNING: old-$file -> $file\"\n-\t\tdone\n-\tfi\n-\texit 1\n-fi\n-\n-# Now the ones with the same name are out of the way...\n-fullbases=\n-for name in $names\n-do\n-\tfullbases=\"$fullbases pack-$name\"\n-\tchmod a-w \"$PACKTMP-$name.pack\"\n-\tchmod a-w \"$PACKTMP-$name.idx\"\n-\tmv -f \"$PACKTMP-$name.pack\" \"$PACKDIR/pack-$name.pack\" &&\n-\tmv -f \"$PACKTMP-$name.idx\" \"$PACKDIR/pack-$name.idx\" ||\n-\texit\n-done\n-\n-# Remove the \"old-\" files\n-for name in $names\n-do\n-\trm -f \"$PACKDIR/old-pack-$name.idx\"\n-\trm -f \"$PACKDIR/old-pack-$name.pack\"\n-done\n-\n-# End of pack replacement.\n-\n-if test \"$remove_redundant\" = t\n-then\n-\t# We know $existing are all redundant.\n-\tif [ -n \"$existing\" ]\n-\tthen\n-\t\t( cd \"$PACKDIR\" &&\n-\t\t for e in $existing\n-\t\t do\n-\t\t\tcase \" $fullbases \" in\n-\t\t\t*\" $e \"*) ;;\n-\t\t\t*)\trm -f \"$e.pack\" \"$e.idx\" \"$e.keep\" ;;\n-\t\t\tesac\n-\t\t done\n-\t\t)\n-\tfi\n-\tgit prune-packed ${GIT_QUIET:+-q}\n-fi\n-\n-case \"$no_update_info\" in\n-t) : ;;\n-*) git update-server-info ;;\n-esac\ndiff --git a/git.c b/git.c\nindex 2025f77..03510be 100644\n--- a/git.c\n+++ b/git.c\n@@ -396,6 +396,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"remote\", cmd_remote, RUN_SETUP },\n \t\t{ \"remote-ext\", cmd_remote_ext },\n \t\t{ \"remote-fd\", cmd_remote_fd },\n+\t\t{ \"repack\", cmd_repack, RUN_SETUP },\n \t\t{ \"replace\", cmd_replace, RUN_SETUP },\n \t\t{ \"repo-config\", cmd_repo_config, RUN_SETUP_GENTLY },\n \t\t{ \"rerere\", cmd_rerere, RUN_SETUP },\n-- \n1.8.4.rc2\n"},{"id":"225183","messageId":"vpqbo50oiec.fsf@anie.imag.fr","threadId":"34687","inReplyTo":"1376421797-26443-1-git-send-email-stefanbeller@googlemail.com","subject":"Re: [PATCH] Rewriting git-repack in C","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-08-14T07:12:27Z","receivedAt":"2013-08-14T07:12:27Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Stefan Beller writes:\n\n> Also I'd like to propose a small technical change:\n> I found no easy way to get a subset of files in a specific directory,\n> which is very easy in the shell version via * (rm file/in/dir/startswithprefix-*),\n> So maybe instead of just prefixing the temporary files such as:\n> $PACKDIR/.tmp-$PID-pack-*\n\nIf it's just a prefix, you can iterate over the full list of files, and\nthen use prefixcmp(...) to find the right files. May seem inefficient,\nbut AFAIK it's how the shell does wildcard expansion.\n\nYou should be carefull that if the operation is interrupted, the next\n\"git gc\" must remove all the garbage, so if you change the naming\nconvention, you must also change the place where the cleanup is done.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"225184","messageId":"vpqy584n36a.fsf@anie.imag.fr","threadId":"34687","inReplyTo":"1376421797-26443-2-git-send-email-stefanbeller@googlemail.com","subject":"Re: [PATCH] repack: rewrite the shell script in C.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-08-14T07:26:37Z","receivedAt":"2013-08-14T07:26:37Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Stefan Beller writes:\n\n> This is the beginning of the rewrite of the repacking.\n\n(please, mark your patch as RFC/PATCH in the subject in this case)\n\nA few quick comments on style below.\n\n> Makefile | 2 +-\n> builtin.h | 1 +\n> builtin/repack.c | 313 +++++++++++++++++++++++++++++++++++++++++\n> contrib/examples/git-repack.sh | 194 +++++++++++++++++++++++++\n> git-repack.sh | 194 -------------------------\n> git.c | 1 +\n\nI suggested that you first enrich the test suite if needed. Did you\ncheck that the testsuite had good enough coverage for git-repack?\n\n> +static const char * const git_repack_usage[] = {\n\nStyle: no space after *.\n\n> +static int delta_base_offset = 1; // enabled by default since 22c79eab (2008-06-25)\n\nNo // comments please (they're C99, not portable C90).\n\n> +int cmd_repack(int argc, const char **argv, const char *prefix) {\n\nBrace on the next line.\n\n> +\tfname = xmalloc(strlen(packdir) + 47); // 40 chars for the sha1\n\n> +\tfname_old = xmalloc(strlen(packdir) + 52); // 40 chars for the sha1\n\nI'd rather have \"40 + strlen(\"whatever\")\" explicitly.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"225208","messageId":"520BAF9F.70105@googlemail.com","threadId":"34687","inReplyTo":"vpqy584n36a.fsf@anie.imag.fr","subject":"Re: [PATCH] repack: rewrite the shell script in C.","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-08-14T16:26:07Z","receivedAt":"2013-08-14T16:26:07Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 08/14/2013 09:26 AM, Matthieu Moy wrote:\n> I suggested that you first enrich the test suite if needed. Did you\n> check that the testsuite had good enough coverage for git-repack?\n\nThere are already quite some tests using git-repack for testing other\npurposes. Also git repack seems to be called from other commands\ninternally such as \"git notes prune\" or \"git gc\".\n\nI'll look into enhancing the test suite once I got the rewritten\ncode working on the current test suite.\n\nStefan\n\n\n\n"},{"id":"225209","messageId":"1376497661-30714-1-git-send-email-stefanbeller@googlemail.com","threadId":"34687","inReplyTo":"520BAF9F.70105@googlemail.com","subject":"[RFC PATCH] repack: rewrite the shell script in C.","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-08-14T16:27:41Z","receivedAt":"2013-08-14T16:27:41Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"* Suggestions by Matthieu Moy have been included.\n* I think it is completed apart from small todos and bugfixes.\n* breaks the test suite, first breakage is t5301 (gc, sliding window)\n\nSigned-off-by: Stefan Beller \n---\n Makefile | 2 +-\n builtin.h | 1 +\n builtin/repack.c | 410 +++++++++++++++++++++++++++++++++++++++++\n contrib/examples/git-repack.sh | 194 +++++++++++++++++++\n git-repack.sh | 194 -------------------\n git.c | 1 +\n 6 files changed, 607 insertions(+), 195 deletions(-)\n create mode 100644 builtin/repack.c\n create mode 100755 contrib/examples/git-repack.sh\n delete mode 100755 git-repack.sh\n\ndiff --git a/Makefile b/Makefile\nindex ef442eb..4ec5bbe 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -464,7 +464,6 @@ SCRIPT_SH += git-pull.sh\n SCRIPT_SH += git-quiltimport.sh\n SCRIPT_SH += git-rebase.sh\n SCRIPT_SH += git-remote-testgit.sh\n-SCRIPT_SH += git-repack.sh\n SCRIPT_SH += git-request-pull.sh\n SCRIPT_SH += git-stash.sh\n SCRIPT_SH += git-submodule.sh\n@@ -972,6 +971,7 @@ BUILTIN_OBJS += builtin/reflog.o\n BUILTIN_OBJS += builtin/remote.o\n BUILTIN_OBJS += builtin/remote-ext.o\n BUILTIN_OBJS += builtin/remote-fd.o\n+BUILTIN_OBJS += builtin/repack.o\n BUILTIN_OBJS += builtin/replace.o\n BUILTIN_OBJS += builtin/rerere.o\n BUILTIN_OBJS += builtin/reset.o\ndiff --git a/builtin.h b/builtin.h\nindex 8afa2de..b56cf07 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -102,6 +102,7 @@ extern int cmd_reflog(int argc, const char **argv, const char *prefix);\n extern int cmd_remote(int argc, const char **argv, const char *prefix);\n extern int cmd_remote_ext(int argc, const char **argv, const char *prefix);\n extern int cmd_remote_fd(int argc, const char **argv, const char *prefix);\n+extern int cmd_repack(int argc, const char **argv, const char *prefix);\n extern int cmd_repo_config(int argc, const char **argv, const char *prefix);\n extern int cmd_rerere(int argc, const char **argv, const char *prefix);\n extern int cmd_reset(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/repack.c b/builtin/repack.c\nnew file mode 100644\nindex 0000000..d39c34e\n--- /dev/null\n+++ b/builtin/repack.c\n@@ -0,0 +1,410 @@\n+/*\n+ * The shell version was written by Linus Torvalds (2005) and many others.\n+ * This is a translation into C by Stefan Beller (2013)\n+ */\n+\n+#include \"builtin.h\"\n+#include \"cache.h\"\n+#include \"dir.h\"\n+#include \"parse-options.h\"\n+#include \"run-command.h\"\n+#include \"sigchain.h\"\n+#include \"strbuf.h\"\n+#include \"string-list.h\"\n+\n+#include \n+#include \n+#include \n+#include \n+\n+static const char *const git_repack_usage[] = {\n+\tN_(\"git repack [options]\"),\n+\tNULL\n+};\n+\n+/* enabled by default since 22c79eab (2008-06-25) */\n+static int delta_base_offset = 1;\n+\n+static int repack_config(const char *var, const char *value, void *cb)\n+{\n+\tif (!strcmp(var, \"repack.usedeltabaseoffset\")) {\n+\t\tdelta_base_offset = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\treturn git_default_config(var, value, cb);\n+}\n+\n+static void remove_temporary_files() {\n+\tDIR *dir;\n+\tstruct dirent *e;\n+\tchar *prefix, *path, *fname;\n+\n+\tprefix = xmalloc(strlen(\".tmp-10000-pack\") + 1);\n+\tsprintf(prefix, \".tmp-%d-pack\", getpid());\n+\n+\tpath = xmalloc(strlen(get_object_directory()) + strlen(\"/pack\") + 1);\n+\tsprintf(path, \"%s/pack\", get_object_directory());\n+\n+\tfname = xmalloc(strlen(path) + strlen(\"/\")\n+\t\t+ strlen(prefix) + strlen(\"/\")\n+\t\t+ 40 + strlen(\".pack\") + 1);\n+\n+\tdir = opendir(path);\n+\twhile ((e = readdir(dir)) != NULL) {\n+\t\tif (!prefixcmp(e->d_name, prefix)) {\n+\t\t\tsprintf(fname, \"%s/%s\", path, e->d_name);\n+\t\t\tunlink(fname);\n+\t\t}\n+\t}\n+\tfree(fname);\n+\tfree(prefix);\n+\tfree(path);\n+\tclosedir(dir);\n+}\n+\n+static void remove_pack_on_signal(int signo)\n+{\n+\tremove_temporary_files();\n+\tsigchain_pop(signo);\n+\traise(signo);\n+}\n+\n+void get_pack_sha1_list(char *packdir, struct string_list *sha1_list)\n+{\n+\tDIR *dir;\n+\tstruct dirent *e;\n+\tchar *path, *suffix;\n+\n+\tpath = xmalloc(strlen(get_object_directory()) + strlen(\"/pack\") + 1);\n+\tsprintf(path, \"%s/pack\", get_object_directory());\n+\n+\tsuffix = \".pack\";\n+\n+\tdir = opendir(path);\n+\twhile ((e = readdir(dir)) != NULL) {\n+\t\tif (!suffixcmp(e->d_name, suffix)) {\n+\t\t\tchar buf[255], *sha1;\n+\t\t\tstrcpy(buf, e->d_name);\n+\t\t\tbuf[strlen(e->d_name) - strlen(suffix)] = '\\0';\n+\t\t\tsha1 = &buf[strlen(e->d_name) - strlen(suffix) - 40];\n+\t\t\tstring_list_append(sha1_list, sha1);\n+\t\t}\n+\t}\n+\tfree(path);\n+\tclosedir(dir);\n+}\n+\n+void remove_pack(char *path, char* sha1)\n+{\n+\tDIR *dir;\n+\tstruct dirent *e;\n+\n+\tdir = opendir(path);\n+\twhile ((e = readdir(dir)) != NULL) {\n+\n+\t\tchar *sha_begin, *sha_end;\n+\t\tsha_end = e->d_name + strlen(e->d_name);\n+\t\twhile (sha_end > e->d_name && *sha_end != '.')\n+\t\t\tsha_end--;\n+\n+\t\t/* do not touch files not ending in .pack, .idx or .keep */\n+\t\tif (strcmp(sha_end, \".pack\") &&\n+\t\t\tstrcmp(sha_end, \".idx\") &&\n+\t\t\tstrcmp(sha_end, \".keep\"))\n+\t\t\tcontinue;\n+\n+\t\tsha_begin = sha_end - 40;\n+\n+\t\tif (sha_begin >= e->d_name && !strncmp(sha_begin, sha1, 40)) {\n+\t\t\tchar *fname;\n+\t\t\tfname = xmalloc(strlen(path) + 1 + strlen(e->d_name));\n+\t\t\tsprintf(fname, \"%s/%s\", path, e->d_name);\n+\t\t\tunlink(fname);\n+\t\t\t/*TODO: free(fname); fails here sometimes, needs investigation*/\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\tclosedir(dir);\n+}\n+\n+int cmd_repack(int argc, const char **argv, const char *prefix) {\n+\n+\tint pack_everything = 0;\n+\tint pack_everything_but_loose = 0;\n+\tint delete_redundant = 0;\n+\tunsigned long unpack_unreachable = 0;\n+\tint window = 0, window_memory = 0;\n+\tint depth = 0;\n+\tint max_pack_size = 0;\n+\tint no_reuse_delta = 0, no_reuse_object = 0;\n+\tint no_update_server_info = 0;\n+\tint quiet = 0;\n+\tint local = 0;\n+\tchar *packdir, *packtmp;\n+\tconst char *cmd_args[20];\n+\tint cmd_i = 0;\n+\tstruct child_process cmd;\n+\tstruct string_list_item *item;\n+\tstruct string_list existing_packs = STRING_LIST_INIT_DUP;\n+\tstruct stat statbuffer;\n+\tchar window_str[64], window_mem_str[64], depth_str[64], max_pack_size_str[64];\n+\n+\tstruct option builtin_repack_options[] = {\n+\t\tOPT_BOOL('a', \"all\", &pack_everything,\n+\t\t\t\tN_(\"pack everything in a single pack\")),\n+\t\tOPT_BOOL('A', \"all-but-loose\", &pack_everything_but_loose,\n+\t\t\t\tN_(\"same as -a, and turn unreachable objects loose\")),\n+\t\tOPT_BOOL('d', \"delete-redundant\", &delete_redundant,\n+\t\t\t\tN_(\"remove redundant packs, and run git-prune-packed\")),\n+\t\tOPT_BOOL('f', \"no-reuse-delta\", &no_reuse_delta,\n+\t\t\t\tN_(\"pass --no-reuse-delta to git-pack-objects\")),\n+\t\tOPT_BOOL('F', \"no-reuse-object\", &no_reuse_object,\n+\t\t\t\tN_(\"pass --no-reuse-object to git-pack-objects\")),\n+\t\tOPT_BOOL('n', NULL, &no_update_server_info,\n+\t\t\t\tN_(\"do not run git-update-server-info\")),\n+\t\tOPT__QUIET(&quiet, N_(\"be quiet\")),\n+\t\tOPT_BOOL('l', \"local\", &local,\n+\t\t\t\tN_(\"pass --local to git-pack-objects\")),\n+\t\tOPT_DATE(0, \"unpack-unreachable\", &unpack_unreachable,\n+\t\t\t\tN_(\"with -A, do not loosen objects older than this Packing constraints\")),\n+\t\tOPT_INTEGER(0, \"window\", &window,\n+\t\t\t\tN_(\"size of the window used for delta compression\")),\n+\t\tOPT_INTEGER(0, \"window-memory\", &window_memory,\n+\t\t\t\tN_(\"same as the above, but limit memory size instead of entries count\")),\n+\t\tOPT_INTEGER(0, \"depth\", &depth,\n+\t\t\t\tN_(\"limits the maximum delta depth\")),\n+\t\tOPT_INTEGER(0, \"max-pack-size\", &max_pack_size,\n+\t\t\t\tN_(\"maximum size of each packfile\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tgit_config(repack_config, NULL);\n+\n+\targc = parse_options(argc, argv, prefix, builtin_repack_options,\n+\t\t\t\tgit_repack_usage, 0);\n+\n+\tsigchain_push_common(remove_pack_on_signal);\n+\n+\tpackdir = mkpath(\"%s/pack\", get_object_directory());\n+\tpacktmp = xmalloc(strlen(packdir) + strlen(\"/.tmp-10000-pack\") + 1);\n+\tsprintf(packtmp, \"%s/.tmp-%d-pack\", packdir, getpid());\n+\n+\tremove_temporary_files();\n+\n+\tcmd_args[cmd_i++] = \"pack-objects\";\n+\tcmd_args[cmd_i++] = \"--keep-true-parents\";\n+\tcmd_args[cmd_i++] = \"--honor-pack-keep\";\n+\tcmd_args[cmd_i++] = \"--non-empty\";\n+\tcmd_args[cmd_i++] = \"--all\";\n+\tcmd_args[cmd_i++] = \"--reflog\";\n+\n+\tif (window) {\n+\t\tsprintf(window_str, \"--window=%u\", window);\n+\t\tcmd_args[cmd_i++] = window_str;\n+\t}\n+\tif (window_memory) {\n+\t\tsprintf(window_mem_str, \"--window-memory=%u\", window_memory);\n+\t\tcmd_args[cmd_i++] = window_str;\n+\t}\n+\tif (depth) {\n+\t\tsprintf(depth_str, \"--depth=%u\", depth);\n+\t\tcmd_args[cmd_i++] = depth_str;\n+\t}\n+\tif (max_pack_size) {\n+\t\tsprintf(max_pack_size_str, \"--max_pack_size=%u\", max_pack_size);\n+\t\tcmd_args[cmd_i++] = max_pack_size_str;\n+\t}\n+\n+\tif (pack_everything + pack_everything_but_loose == 0) {\n+\t\tcmd_args[cmd_i++] = \"--unpacked\";\n+\t\tcmd_args[cmd_i++] = \"--incremental\";\n+\t} else {\n+\t\tif (pack_everything_but_loose)\n+\t\t\tcmd_args[cmd_i++] = \"--unpack-unreachable\";\n+\n+\t\tstruct string_list sha1_list = STRING_LIST_INIT_DUP;\n+\t\tget_pack_sha1_list(packdir, &sha1_list);\n+\t\tfor_each_string_list_item(item, &sha1_list) {\n+\t\t\tchar *fname;\n+\t\t\tfname = xmalloc(strlen(packdir) + strlen(\"/\") + 40 + strlen(\".keep\"));\n+\t\t\tsprintf(fname, \"%s/%s.keep\", packdir, item->string);\n+\t\t\tif (stat(fname, &statbuffer) && S_ISREG(statbuffer.st_mode)) {\n+\t\t\t\t/* when the keep file is there, we're ignoring that pack */\n+\t\t\t} else {\n+\t\t\t\tstring_list_append(&existing_packs, item->string);\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (existing_packs.nr && unpack_unreachable && delete_redundant) {\n+\t\t\t/*\n+\t\t\t * TODO: convert unpack_unreachable (being time since epoch)\n+\t\t\t * to an aproxidate again\n+\t\t\t */\n+\t\t\tcmd_args[cmd_i++] = \"--unpack-unreachable=$DATE\";\n+\t\t}\n+\t}\n+\n+\tif (local)\n+\t\tcmd_args[cmd_i++] = \"--local\";\n+\n+\tif (delta_base_offset)\n+\t\tcmd_args[cmd_i++] = \"--delta-base-offset\";\n+\n+\tcmd_args[cmd_i++] = packtmp;\n+\tcmd_args[cmd_i] = NULL;\n+\n+\tmemset(&cmd, 0, sizeof(cmd));\n+\tcmd.argv = cmd_args;\n+\tcmd.git_cmd = 1;\n+\tcmd.out = -1;\n+\tcmd.no_stdin = 1;\n+\n+\trun_command(&cmd);\n+\n+\tstruct string_list names = STRING_LIST_INIT_DUP;\n+\tstruct string_list rollback = STRING_LIST_INIT_DUP;\n+\n+\tchar line[1024];\n+\tint counter = 0;\n+\tFILE *out = xfdopen(cmd.out, \"r\");\n+\twhile (fgets(line, sizeof(line), out)) {\n+\t\t/* a line consists of 40 hex chars + '\\n' */\n+\t\tassert(strlen(line) == 41);\n+\t\tline[40] = '\\0';\n+\t\tstring_list_append(&names, line);\n+\t\tcounter++;\n+\t}\n+\tif (!counter)\n+\t\tprintf(\"Nothing new to pack.\\n\");\n+\tfclose(out);\n+\n+\tchar *fname, *fname_old;\n+\tfname = xmalloc(strlen(packdir) + strlen(\"/old-pack-\") + 40 + strlen(\".pack\") + 1);\n+\tstrcpy(fname, packdir);\n+\tstrcpy(fname + strlen(packdir), \"/\");\n+\n+\tfname_old = xmalloc(strlen(packdir) + strlen(\"/old-pack-\") + 40 + strlen(\".pack\") + 1);\n+\tstrcpy(fname_old, packdir);\n+\tstrcpy(fname_old + strlen(packdir), \"/\");\n+\tchar *exts[2] = {\".idx\", \".pack\"};\n+\n+\tint failed = 0;\n+\n+\tfor_each_string_list_item(item, &names) {\n+\t\tint ext;\n+\t\tfor (ext = 0; ext < 1; ext++) {\n+\t\t\tstrcpy(fname + strlen(packdir) + 1, item->string);\n+\t\t\tstrcpy(fname + strlen(packdir) + 41, exts[ext]);\n+\t\t\tif (!file_exists(fname))\n+\t\t\t\tcontinue;\n+\n+\t\t\tstrcpy(fname_old, packdir);\n+\t\t\tstrcpy(fname_old + strlen(packdir), \"/old-\");\n+\t\t\tstrcpy(fname_old + strlen(packdir) + 5, item->string);\n+\t\t\tstrcpy(fname_old + strlen(packdir) + 45, exts[ext]);\n+\t\t\tif (file_exists(fname_old))\n+\t\t\t\tunlink(fname_old);\n+\n+\t\t\tif (rename(fname, fname_old)) {\n+\t\t\t\tfailed = 1;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tstring_list_append(&rollback, fname);\n+\t\t}\n+\t\tif (failed)\n+\t\t\t/* set to last element to break while loop */\n+\t\t\titem = names.items + names.nr;\n+\t}\n+\tif (failed) {\n+\t\tstruct string_list rollback_failure;\n+\n+\t\tfor_each_string_list_item(item, &rollback) {\n+\t\t\tsprintf(fname, \"%s/%s\", packdir, item->string);\n+\t\t\tsprintf(fname_old, \"%s/old-%s\", packdir, item->string);\n+\t\t\tif (rename(fname_old, fname))\n+\t\t\t\tstring_list_append(&rollback_failure, fname);\n+\t\t}\n+\n+\t\tif (rollback.nr) {\n+\t\t\tint i;\n+\t\t\tfprintf(stderr,\n+\t\t\t\t\"WARNING: Some packs in use have been renamed by\\n\"\n+\t\t\t\t\"WARNING: prefixing old- to their name, in order to\\n\"\n+\t\t\t\t\"WARNING: replace them with the new version of the\\n\"\n+\t\t\t\t\"WARNING: file. But the operation failed, and\\n\"\n+\t\t\t\t\"WARNING: attempt to rename them back to their\\n\"\n+\t\t\t\t\"WARNING: original names also failed.\\n\"\n+\t\t\t\t\"WARNING: Please rename them in $PACKDIR manually:\\n\");\n+\t\t\tfor (i = 0; i < rollback.nr; i++)\n+\t\t\t\tfprintf(stderr, \"WARNING: old-%s -> %s\\n\",\n+\t\t\t\t\trollback.items[i].string,\n+\t\t\t\t\trollback.items[i].string);\n+\t\t}\n+\t\texit(1);\n+\t}\n+\n+\t/* Now the ones with the same name are out of the way... */\n+\tstruct string_list fullbases = STRING_LIST_INIT_DUP;\n+\tfor_each_string_list_item(item, &names) {\n+\t\tstring_list_append(&fullbases, item->string);\n+\n+\t\tsprintf(fname, \"%s/pack-%s.pack\", packdir, item->string);\n+\t\tsprintf(fname_old, \"%s-%s.pack\", packtmp, item->string);\n+\t\tstat(fname_old, &statbuffer);\n+\t\tstatbuffer.st_mode &= ~S_IWOTH;\n+\t\tchmod(fname_old, statbuffer.st_mode);\n+\t\tif (rename(fname_old, fname))\n+\t\t\tdie(\"Could not rename packfile: %s -> %s\", fname_old, fname);\n+\n+\t\tsprintf(fname, \"%s/pack-%s.idx\", packdir, item->string);\n+\t\tsprintf(fname_old, \"%s-%s.idx\", packtmp, item->string);\n+\t\tstat(fname_old, &statbuffer);\n+\t\tstatbuffer.st_mode &= ~S_IWOTH;\n+\t\tchmod(fname_old, statbuffer.st_mode);\n+\t\tif (rename(fname_old, fname))\n+\t\t\tdie(\"Could not rename packfile: %s -> %s\", fname_old, fname);\n+\t}\n+\n+\t/* Remove the \"old-\" files */\n+\tfor_each_string_list_item(item, &names) {\n+\t\tsprintf(fname, \"%s/old-pack-%s.idx\", packdir, item->string);\n+\t\tif (remove_path(fname))\n+\t\t\tdie(\"Could not remove file: %s\", fname);\n+\n+\t\tsprintf(fname, \"%s/old-pack-%s.pack\", packdir, item->string);\n+\t\tif (remove_path(fname))\n+\t\t\tdie(\"Could not remove file: %s\", fname);\n+\t}\n+\n+\t/* End of pack replacement. */\n+\tif (delete_redundant) {\n+\t\tsort_string_list(&fullbases);\n+\t\tfname = xmalloc(strlen(packtmp) + strlen(\"/\") + 40 + strlen(\".pack\") + 1);\n+\t\tfor_each_string_list_item(item, &existing_packs) {\n+\t\t\tif (!string_list_has_string(&fullbases, item->string))\n+\t\t\t\tremove_pack(packdir, item->string);\n+\t\t}\n+\t\tfree(fname);\n+\t\tcmd_i = 0;\n+\t\tcmd_args[cmd_i++] = \"prune-packed\";\n+\t\tcmd_args[cmd_i++] = NULL;\n+\t\t/* TODO: pass argument: ${GIT_QUIET:+-q} */\n+\t\tmemset(&cmd, 0, sizeof(cmd));\n+\t\tcmd.argv = cmd_args;\n+\t\tcmd.git_cmd = 1;\n+\t\trun_command(&cmd);\n+\t}\n+\n+\tif (!no_update_server_info) {\n+\t\tcmd_i = 0;\n+\t\tcmd_args[cmd_i++] = \"update-server-info\";\n+\t\tcmd_args[cmd_i++] = NULL;\n+\n+\t\tmemset(&cmd, 0, sizeof(cmd));\n+\t\tcmd.argv = cmd_args;\n+\t\tcmd.git_cmd = 1;\n+\t\trun_command(&cmd);\n+\t}\n+\treturn 0;\n+}\n+\ndiff --git a/contrib/examples/git-repack.sh b/contrib/examples/git-repack.sh\nnew file mode 100755\nindex 0000000..7579331\n--- /dev/null\n+++ b/contrib/examples/git-repack.sh\n@@ -0,0 +1,194 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2005 Linus Torvalds\n+#\n+\n+OPTIONS_KEEPDASHDASH=\n+OPTIONS_SPEC=\"\\\n+git repack [options]\n+--\n+a pack everything in a single pack\n+A same as -a, and turn unreachable objects loose\n+d remove redundant packs, and run git-prune-packed\n+f pass --no-reuse-delta to git-pack-objects\n+F pass --no-reuse-object to git-pack-objects\n+n do not run git-update-server-info\n+q,quiet be quiet\n+l pass --local to git-pack-objects\n+unpack-unreachable= with -A, do not loosen objects older than this\n+ Packing constraints\n+window= size of the window used for delta compression\n+window-memory= same as the above, but limit memory size instead of entries count\n+depth= limits the maximum delta depth\n+max-pack-size= maximum size of each packfile\n+\"\n+SUBDIRECTORY_OK='Yes'\n+. git-sh-setup\n+\n+no_update_info= all_into_one= remove_redundant= unpack_unreachable=\n+local= no_reuse= extra=\n+while test $# != 0\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\tunpack_unreachable=--unpack-unreachable ;;\n+\t--unpack-unreachable)\n+\t\tunpack_unreachable=\"--unpack-unreachable=$2\"; shift ;;\n+\t-d)\tremove_redundant=t ;;\n+\t-q)\tGIT_QUIET=t ;;\n+\t-f)\tno_reuse=--no-reuse-delta ;;\n+\t-F)\tno_reuse=--no-reuse-object ;;\n+\t-l)\tlocal=--local ;;\n+\t--max-pack-size|--window|--window-memory|--depth)\n+\t\textra=\"$extra $1=$2\"; shift ;;\n+\t--) shift; break;;\n+\t*)\tusage ;;\n+\tesac\n+\tshift\n+done\n+\n+case \"`git config --bool repack.usedeltabaseoffset || echo true`\" in\n+true)\n+\textra=\"$extra --delta-base-offset\" ;;\n+esac\n+\n+PACKDIR=\"$GIT_OBJECT_DIRECTORY/pack\"\n+PACKTMP=\"$PACKDIR/.tmp-$$-pack\"\n+rm -f \"$PACKTMP\"-*\n+trap 'rm -f \"$PACKTMP\"-*' 0 1 2 3 15\n+\n+# There will be more repacking strategies to come...\n+case \",$all_into_one,\" in\n+,,)\n+\targs='--unpacked --incremental'\n+\t;;\n+,t,)\n+\targs= existing=\n+\tif [ -d \"$PACKDIR\" ]; then\n+\t\tfor e in `cd \"$PACKDIR\" && find . -type f -name '*.pack' \\\n+\t\t\t| sed -e 's/^\\.\\///' -e 's/\\.pack$//'`\n+\t\tdo\n+\t\t\tif [ -e \"$PACKDIR/$e.keep\" ]; then\n+\t\t\t\t: keep\n+\t\t\telse\n+\t\t\t\texisting=\"$existing $e\"\n+\t\t\tfi\n+\t\tdone\n+\t\tif test -n \"$existing\" -a -n \"$unpack_unreachable\" -a \\\n+\t\t\t-n \"$remove_redundant\"\n+\t\tthen\n+\t\t\t# This may have arbitrary user arguments, so we\n+\t\t\t# have to protect it against whitespace splitting\n+\t\t\t# when it gets run as \"pack-objects $args\" later.\n+\t\t\t# Fortunately, we know it's an approxidate, so we\n+\t\t\t# can just use dots instead.\n+\t\t\targs=\"$args $(echo \"$unpack_unreachable\" | tr ' ' .)\"\n+\t\tfi\n+\tfi\n+\t;;\n+esac\n+\n+mkdir -p \"$PACKDIR\" || exit\n+\n+args=\"$args $local ${GIT_QUIET:+-q} $no_reuse$extra\"\n+names=$(git pack-objects --keep-true-parents --honor-pack-keep --non-empty --all --reflog $args &2 \"WARNING: Some packs in use have been renamed by\"\n+\t\techo >&2 \"WARNING: prefixing old- to their name, in order to\"\n+\t\techo >&2 \"WARNING: replace them with the new version of the\"\n+\t\techo >&2 \"WARNING: file. But the operation failed, and\"\n+\t\techo >&2 \"WARNING: attempt to rename them back to their\"\n+\t\techo >&2 \"WARNING: original names also failed.\"\n+\t\techo >&2 \"WARNING: Please rename them in $PACKDIR manually:\"\n+\t\tfor file in $rollback_failure\n+\t\tdo\n+\t\t\techo >&2 \"WARNING: old-$file -> $file\"\n+\t\tdone\n+\tfi\n+\texit 1\n+fi\n+\n+# Now the ones with the same name are out of the way...\n+fullbases=\n+for name in $names\n+do\n+\tfullbases=\"$fullbases pack-$name\"\n+\tchmod a-w \"$PACKTMP-$name.pack\"\n+\tchmod a-w \"$PACKTMP-$name.idx\"\n+\tmv -f \"$PACKTMP-$name.pack\" \"$PACKDIR/pack-$name.pack\" &&\n+\tmv -f \"$PACKTMP-$name.idx\" \"$PACKDIR/pack-$name.idx\" ||\n+\texit\n+done\n+\n+# Remove the \"old-\" files\n+for name in $names\n+do\n+\trm -f \"$PACKDIR/old-pack-$name.idx\"\n+\trm -f \"$PACKDIR/old-pack-$name.pack\"\n+done\n+\n+# End of pack replacement.\n+\n+if test \"$remove_redundant\" = t\n+then\n+\t# We know $existing are all redundant.\n+\tif [ -n \"$existing\" ]\n+\tthen\n+\t\t( cd \"$PACKDIR\" &&\n+\t\t for e in $existing\n+\t\t do\n+\t\t\tcase \" $fullbases \" in\n+\t\t\t*\" $e \"*) ;;\n+\t\t\t*)\trm -f \"$e.pack\" \"$e.idx\" \"$e.keep\" ;;\n+\t\t\tesac\n+\t\t done\n+\t\t)\n+\tfi\n+\tgit prune-packed ${GIT_QUIET:+-q}\n+fi\n+\n+case \"$no_update_info\" in\n+t) : ;;\n+*) git update-server-info ;;\n+esac\ndiff --git a/git-repack.sh b/git-repack.sh\ndeleted file mode 100755\nindex 7579331..0000000\n--- a/git-repack.sh\n+++ /dev/null\n@@ -1,194 +0,0 @@\n-#!/bin/sh\n-#\n-# Copyright (c) 2005 Linus Torvalds\n-#\n-\n-OPTIONS_KEEPDASHDASH=\n-OPTIONS_SPEC=\"\\\n-git repack [options]\n---\n-a pack everything in a single pack\n-A same as -a, and turn unreachable objects loose\n-d remove redundant packs, and run git-prune-packed\n-f pass --no-reuse-delta to git-pack-objects\n-F pass --no-reuse-object to git-pack-objects\n-n do not run git-update-server-info\n-q,quiet be quiet\n-l pass --local to git-pack-objects\n-unpack-unreachable= with -A, do not loosen objects older than this\n- Packing constraints\n-window= size of the window used for delta compression\n-window-memory= same as the above, but limit memory size instead of entries count\n-depth= limits the maximum delta depth\n-max-pack-size= maximum size of each packfile\n-\"\n-SUBDIRECTORY_OK='Yes'\n-. git-sh-setup\n-\n-no_update_info= all_into_one= remove_redundant= unpack_unreachable=\n-local= no_reuse= extra=\n-while test $# != 0\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\tunpack_unreachable=--unpack-unreachable ;;\n-\t--unpack-unreachable)\n-\t\tunpack_unreachable=\"--unpack-unreachable=$2\"; shift ;;\n-\t-d)\tremove_redundant=t ;;\n-\t-q)\tGIT_QUIET=t ;;\n-\t-f)\tno_reuse=--no-reuse-delta ;;\n-\t-F)\tno_reuse=--no-reuse-object ;;\n-\t-l)\tlocal=--local ;;\n-\t--max-pack-size|--window|--window-memory|--depth)\n-\t\textra=\"$extra $1=$2\"; shift ;;\n-\t--) shift; break;;\n-\t*)\tusage ;;\n-\tesac\n-\tshift\n-done\n-\n-case \"`git config --bool repack.usedeltabaseoffset || echo true`\" in\n-true)\n-\textra=\"$extra --delta-base-offset\" ;;\n-esac\n-\n-PACKDIR=\"$GIT_OBJECT_DIRECTORY/pack\"\n-PACKTMP=\"$PACKDIR/.tmp-$$-pack\"\n-rm -f \"$PACKTMP\"-*\n-trap 'rm -f \"$PACKTMP\"-*' 0 1 2 3 15\n-\n-# There will be more repacking strategies to come...\n-case \",$all_into_one,\" in\n-,,)\n-\targs='--unpacked --incremental'\n-\t;;\n-,t,)\n-\targs= existing=\n-\tif [ -d \"$PACKDIR\" ]; then\n-\t\tfor e in `cd \"$PACKDIR\" && find . -type f -name '*.pack' \\\n-\t\t\t| sed -e 's/^\\.\\///' -e 's/\\.pack$//'`\n-\t\tdo\n-\t\t\tif [ -e \"$PACKDIR/$e.keep\" ]; then\n-\t\t\t\t: keep\n-\t\t\telse\n-\t\t\t\texisting=\"$existing $e\"\n-\t\t\tfi\n-\t\tdone\n-\t\tif test -n \"$existing\" -a -n \"$unpack_unreachable\" -a \\\n-\t\t\t-n \"$remove_redundant\"\n-\t\tthen\n-\t\t\t# This may have arbitrary user arguments, so we\n-\t\t\t# have to protect it against whitespace splitting\n-\t\t\t# when it gets run as \"pack-objects $args\" later.\n-\t\t\t# Fortunately, we know it's an approxidate, so we\n-\t\t\t# can just use dots instead.\n-\t\t\targs=\"$args $(echo \"$unpack_unreachable\" | tr ' ' .)\"\n-\t\tfi\n-\tfi\n-\t;;\n-esac\n-\n-mkdir -p \"$PACKDIR\" || exit\n-\n-args=\"$args $local ${GIT_QUIET:+-q} $no_reuse$extra\"\n-names=$(git pack-objects --keep-true-parents --honor-pack-keep --non-empty --all --reflog $args &2 \"WARNING: Some packs in use have been renamed by\"\n-\t\techo >&2 \"WARNING: prefixing old- to their name, in order to\"\n-\t\techo >&2 \"WARNING: replace them with the new version of the\"\n-\t\techo >&2 \"WARNING: file. But the operation failed, and\"\n-\t\techo >&2 \"WARNING: attempt to rename them back to their\"\n-\t\techo >&2 \"WARNING: original names also failed.\"\n-\t\techo >&2 \"WARNING: Please rename them in $PACKDIR manually:\"\n-\t\tfor file in $rollback_failure\n-\t\tdo\n-\t\t\techo >&2 \"WARNING: old-$file -> $file\"\n-\t\tdone\n-\tfi\n-\texit 1\n-fi\n-\n-# Now the ones with the same name are out of the way...\n-fullbases=\n-for name in $names\n-do\n-\tfullbases=\"$fullbases pack-$name\"\n-\tchmod a-w \"$PACKTMP-$name.pack\"\n-\tchmod a-w \"$PACKTMP-$name.idx\"\n-\tmv -f \"$PACKTMP-$name.pack\" \"$PACKDIR/pack-$name.pack\" &&\n-\tmv -f \"$PACKTMP-$name.idx\" \"$PACKDIR/pack-$name.idx\" ||\n-\texit\n-done\n-\n-# Remove the \"old-\" files\n-for name in $names\n-do\n-\trm -f \"$PACKDIR/old-pack-$name.idx\"\n-\trm -f \"$PACKDIR/old-pack-$name.pack\"\n-done\n-\n-# End of pack replacement.\n-\n-if test \"$remove_redundant\" = t\n-then\n-\t# We know $existing are all redundant.\n-\tif [ -n \"$existing\" ]\n-\tthen\n-\t\t( cd \"$PACKDIR\" &&\n-\t\t for e in $existing\n-\t\t do\n-\t\t\tcase \" $fullbases \" in\n-\t\t\t*\" $e \"*) ;;\n-\t\t\t*)\trm -f \"$e.pack\" \"$e.idx\" \"$e.keep\" ;;\n-\t\t\tesac\n-\t\t done\n-\t\t)\n-\tfi\n-\tgit prune-packed ${GIT_QUIET:+-q}\n-fi\n-\n-case \"$no_update_info\" in\n-t) : ;;\n-*) git update-server-info ;;\n-esac\ndiff --git a/git.c b/git.c\nindex 2025f77..03510be 100644\n--- a/git.c\n+++ b/git.c\n@@ -396,6 +396,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"remote\", cmd_remote, RUN_SETUP },\n \t\t{ \"remote-ext\", cmd_remote_ext },\n \t\t{ \"remote-fd\", cmd_remote_fd },\n+\t\t{ \"repack\", cmd_repack, RUN_SETUP },\n \t\t{ \"replace\", cmd_replace, RUN_SETUP },\n \t\t{ \"repo-config\", cmd_repo_config, RUN_SETUP_GENTLY },\n \t\t{ \"rerere\", cmd_rerere, RUN_SETUP },\n-- \n1.8.4.rc2\n"},{"id":"225212","messageId":"CALWbr2xuV+V7M354+XoA3HCvLr0Cpx4t3cLVeTCx4xeNmQQX1w@mail.gmail.com","threadId":"34687","inReplyTo":"1376497661-30714-1-git-send-email-stefanbeller@googlemail.com","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-08-14T16:49:58Z","receivedAt":"2013-08-14T16:49:58Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Wed, Aug 14, 2013 at 6:27 PM, Stefan Beller\n wrote:\n> builtin/repack.c | 410 +++++++++++++++++++++++++++++++++++++++++\n> contrib/examples/git-repack.sh | 194 +++++++++++++++++++\n> git-repack.sh | 194 -------------------\n\nI'm still not sure I understand the trade-off here.\n\nMost of what git-repack does is compute some file paths, (re)move\nthose files and call git-pack-objects, and potentially\ngit-prune-packed and git-update-server-info.\nMaybe I'm wrong, but I have the feeling that the correct tool for that\nis Shell, rather than C (and I think the code looks less intuitive in\nC for that matter).\nI'm not sure anyone would run that command a thousand times a second,\nso I'm not sure it would make a real-life performance difference.\n\nLast and very less important: I think it's OK to format-patch with -M,\nespecially when you move a file.\n\nCheers,\nAntoine\n"},{"id":"225214","messageId":"7vli44td9k.fsf@alter.siamese.dyndns.org","threadId":"34687","inReplyTo":"1376497661-30714-1-git-send-email-stefanbeller@googlemail.com","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-14T17:04:23Z","receivedAt":"2013-08-14T17:04:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> diff --git a/builtin/repack.c b/builtin/repack.c\n> new file mode 100644\n> index 0000000..d39c34e\n> --- /dev/null\n> +++ b/builtin/repack.c\n> @@ -0,0 +1,410 @@\n> +/*\n> + * The shell version was written by Linus Torvalds (2005) and many others.\n> + * This is a translation into C by Stefan Beller (2013)\n> + */\n> +\n> +#include \"builtin.h\"\n> +#include \"cache.h\"\n> +#include \"dir.h\"\n> +#include \"parse-options.h\"\n> +#include \"run-command.h\"\n> +#include \"sigchain.h\"\n> +#include \"strbuf.h\"\n> +#include \"string-list.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n\nIf you need these system-includes here, it means that our own\nplatform-portability layer \"git-compat-util.h\" is broken. On\nvarious systems, often some system header files are missing, need a\nfew feature macros to be defined before including, and/or need to be\nincluded in certain order, etc., and \"git-compat-util.h\" is meant to\nhide all such details from the programmers.\n\nI do not think the above four needs to be included in *.c, as long\nas you include either builtin.h or cache.h, both of which includes\nthe compat-util header.\n"},{"id":"225215","messageId":"520BB8A5.4010406@googlemail.com","threadId":"34687","inReplyTo":"CALWbr2xuV+V7M354+XoA3HCvLr0Cpx4t3cLVeTCx4xeNmQQX1w@mail.gmail.com","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-08-14T17:04:37Z","receivedAt":"2013-08-14T17:04:37Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 08/14/2013 06:49 PM, Antoine Pelisse wrote:\n> On Wed, Aug 14, 2013 at 6:27 PM, Stefan Beller\n> wrote:\n>> builtin/repack.c | 410 +++++++++++++++++++++++++++++++++++++++++\n>> contrib/examples/git-repack.sh | 194 +++++++++++++++++++\n>> git-repack.sh | 194 -------------------\n> \n> I'm still not sure I understand the trade-off here.\n> \n> Most of what git-repack does is compute some file paths, (re)move\n> those files and call git-pack-objects, and potentially\n> git-prune-packed and git-update-server-info.\n> Maybe I'm wrong, but I have the feeling that the correct tool for that\n> is Shell, rather than C (and I think the code looks less intuitive in\n> C for that matter).\n> I'm not sure anyone would run that command a thousand times a second,\n> so I'm not sure it would make a real-life performance difference.\n\nFrom IRC:\n IIRC repack is one of the most important scripts to port\n it's one of the rare times a script is used serverside\n heh, I picked it because of its size\n (and people want to be able to use git in chrooted enviroments\nwith few dependencies)\n\nMy contributions as of now are very small nit picking things just to\nfamiliarize with the code base, most of the time supported by tools\nsuch as static code checkers.\n\nHowever I'd like to contribute to the project in a more meaningful way,\nbut I still have the feeling to not be completely familar with the\nprojects code base (heh, it sure takes time for such large projects)\n\nSo I think the best way to get a feeling for the code base is to\nrewrite a shell script in C. I picked the smallest script, to have\nonly a little task. So I thought at least. The rewriting is larger than\noriginally assumed.\n\n\nBut apart from my blabbering, I think ivegy made a good point:\nThe C parts just don't rely on external things, but only libc and\nkernel, so it may be nicer than a shell script. Also as it is used\nserversided, the performance aspect is not negligible.\n\nI included Jeff King, who maybe could elaborate on git-repack on the\nserverside?\n\n\n> \n> Last and very less important: I think it's OK to format-patch with -M,\n> especially when you move a file.\n> \n\nNoted, thanks.\n\n> Cheers,\n> Antoine\n> \n\nStefan\n\n\n"},{"id":"225216","messageId":"20130814171956.GA6884@sigill.intra.peff.net","threadId":"34687","inReplyTo":"520BB8A5.4010406@googlemail.com","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-08-14T17:19:56Z","receivedAt":"2013-08-14T17:19:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 14, 2013 at 07:04:37PM +0200, Stefan Beller wrote:\n\n> But apart from my blabbering, I think ivegy made a good point:\n> The C parts just don't rely on external things, but only libc and\n> kernel, so it may be nicer than a shell script. Also as it is used\n> serversided, the performance aspect is not negligible.\n> \n> I included Jeff King, who maybe could elaborate on git-repack on the\n> serverside?\n\nI don't think the performance of repack as a C program versus a shell\nscript is really relevant to us at GitHub. Sure, we run a fair number of\nrepacks, but the cost is totally dominated by the pack-objects process\nitself.\n\nYou might be able to achieve some speedups if it was not simply a\nshell->C conversion, but an overall gc rewrite that did more in a single\nprocess, and reused results (for example, you can reuse all or part of\nthe history traversal from pack-object's \"counting objects\" phase to do\nthe reachability analysis during prune)[1].\n\nBut I'd be very wary of stuffing too many things in a single process.\nThere are parts of the code that make assumptions about which objects\nhave been seen in the global object hash table (I believe index-pack is\none of these; see check_objects). And there are parts of the code which\nmust run separately (e.g., the connectivity check after transfer runs in\na separate process, both because it may die(), but also because we want\na clean slate of which packs are available, with no caching of results\nwe may have seen).\n\nNone of those problems is unsolvable, but it's very hard to know when\none is going to pop up and bite you. And because the repacking and\npruning code is the most likely place for a bug to cause data loss, it\nmakes me a bit nervous.\n\n-Peff\n\n[1] Another way to reuse the history traversal is to generate the\n much-discussed pack reachability bitmaps, and then use them in\n git-prune.\n"},{"id":"225217","messageId":"201308141125.59991.mfick@codeaurora.org","threadId":"34687","inReplyTo":"CALWbr2xuV+V7M354+XoA3HCvLr0Cpx4t3cLVeTCx4xeNmQQX1w@mail.gmail.com","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Martin Fick","fromEmail":"mfick@codeaurora.org","sentAt":"2013-08-14T17:25:59Z","receivedAt":"2013-08-14T17:25:59Z","isPatch":true,"sender":{"key":"mfick@codeaurora.org","avatar":null},"body":"On Wednesday, August 14, 2013 10:49:58 am Antoine Pelisse \nwrote:\n> On Wed, Aug 14, 2013 at 6:27 PM, Stefan Beller\n> \n> wrote:\n> > builtin/repack.c | 410\n> > +++++++++++++++++++++++++++++++++++++++++\n> > contrib/examples/git-repack.sh | 194\n> > +++++++++++++++++++ git-repack.sh |\n> > 194 -------------------\n> \n> I'm still not sure I understand the trade-off here.\n> \n> Most of what git-repack does is compute some file paths,\n> (re)move those files and call git-pack-objects, and\n> potentially git-prune-packed and git-update-server-info.\n> Maybe I'm wrong, but I have the feeling that the correct\n> tool for that is Shell, rather than C (and I think the\n> code looks less intuitive in C for that matter).\n> I'm not sure anyone would run that command a thousand\n> times a second, so I'm not sure it would make a\n> real-life performance difference.\n\nI have been holding off a bit on expressing this opinion too \nbecause I don't want to squash someone's energy to improve \nthings, and because I am not yet a git dev, but since it was \nbrought up anyway...\n \nI can say that as a user, having git-repack as a shell \nscript has been very valuable. For example: we have \nmodified it for our internal use to no longer ever overwrite\nnew packfiles with the same name as the current packfile. \nThis modification was relatively easy to do and see in shell \nscript. If this were C code I can't imagine having \npersonally: 1) identified that there was an issue with the \noriginal git-repack (it temporarily makes objects \nunavailable) 2) made a simple custom fix to that policy.\n\nThe script really is mostly a policy script, and with the \ndiscussions happening in other threads about how to improve \ngit gc, I think it is helpful to potentially be able to \nquickly modify the policies in this script, it makes it \neasier to prototype things. Shell portability issues aside, \nthis script is not a low level type of tool that I feel will \nbenefit from being in C, I feel it will in fact be worse off \nin C,\n\n-Martin\n\n-- \nThe Qualcomm Innovation Center, Inc. is a member of Code \nAurora Forum, hosted by The Linux Foundation\n \n"},{"id":"225218","messageId":"7vhaestc8o.fsf@alter.siamese.dyndns.org","threadId":"34687","inReplyTo":"CALWbr2xuV+V7M354+XoA3HCvLr0Cpx4t3cLVeTCx4xeNmQQX1w@mail.gmail.com","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-14T17:26:31Z","receivedAt":"2013-08-14T17:26:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Pelisse writes:\n\n> On Wed, Aug 14, 2013 at 6:27 PM, Stefan Beller\n> wrote:\n>> builtin/repack.c | 410 +++++++++++++++++++++++++++++++++++++++++\n>> contrib/examples/git-repack.sh | 194 +++++++++++++++++++\n>> git-repack.sh | 194 -------------------\n>\n> I'm still not sure I understand the trade-off here.\n>\n> Most of what git-repack does is compute some file paths, (re)move\n> those files and call git-pack-objects, and potentially\n> git-prune-packed and git-update-server-info.\n> Maybe I'm wrong, but I have the feeling that the correct tool for that\n> is Shell, rather than C (and I think the code looks less intuitive in\n> C for that matter).\n> I'm not sure anyone would run that command a thousand times a second,\n> so I'm not sure it would make a real-life performance difference.\n\nI do not think the motivation of this patch is about performance in\nthe first place, though.\n\n> Last and very less important: I think it's OK to format-patch with -M,\n> especially when you move a file.\n>\n> Cheers,\n> Antoine\n"},{"id":"225231","messageId":"520C01C3.2060804@googlemail.com","threadId":"34687","inReplyTo":"201308141125.59991.mfick@codeaurora.org","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-08-14T22:16:35Z","receivedAt":"2013-08-14T22:16:35Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 08/14/2013 07:25 PM, Martin Fick wrote:\n> I have been holding off a bit on expressing this opinion too \n> because I don't want to squash someone's energy to improve \n> things, and because I am not yet a git dev, but since it was \n> brought up anyway...\n\nIt's ok, if you knew a better topic to work on, I'd gladly switch over.\n(Given it would be a good beginners topic.)\n\n> \n> I can say that as a user, having git-repack as a shell \n> script has been very valuable. For example: we have \n> modified it for our internal use to no longer ever overwrite\n> new packfiles with the same name as the current packfile. \n> This modification was relatively easy to do and see in shell \n> script. If this were C code I can't imagine having \n> personally: 1) identified that there was an issue with the \n> original git-repack (it temporarily makes objects \n> unavailable) 2) made a simple custom fix to that policy.\n\nLooking at the `git log -- git-repack.sh` the last commit is\nfrom April 2012 and the commit before is 2011, so I assumed it\nstable enough for porting over to C, as there is not much\nmodification going on. I'd be glad to include these changes\nyou're using into the rewrite or the shell script as of now.\n\n> \n> The script really is mostly a policy script, and with the \n> discussions happening in other threads about how to improve \n> git gc, I think it is helpful to potentially be able to \n> quickly modify the policies in this script, it makes it \n> easier to prototype things. Shell portability issues aside, \n> this script is not a low level type of tool that I feel will \n> benefit from being in C, I feel it will in fact be worse off \n> in C,\n\nSo far I have been following the git mailing list, there are\npeople dreaming of 'everything in C' and apparently those, who\nare ok with lots of shell code as well, 'because it's high level'.\nI tend to follow the first group, dreaming of everything in C.\nThanks for pointing that out, if it really hurts people from using\ngit effectively, I'd rather not contribute this patch. But I'd like\nto stress again, that the prototying should be done by now.\n\nI asked for a todo wish list a few weeks ago, but got no real answer,\nbut rather: \"Pick your choice and try to come up with good patches\".\nThis is a good policy from the projects point of view (your choice\nhelps in doing good patches, and good patches do not need as much\nreview, hence the reviewing costs are low), so I also choose this topic.\n\n> \n> -Martin\n> \n\nStefan\n\n"},{"id":"225232","messageId":"201308141628.42163.mfick@codeaurora.org","threadId":"34687","inReplyTo":"520C01C3.2060804@googlemail.com","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Martin Fick","fromEmail":"mfick@codeaurora.org","sentAt":"2013-08-14T22:28:41Z","receivedAt":"2013-08-14T22:28:41Z","isPatch":true,"sender":{"key":"mfick@codeaurora.org","avatar":null},"body":"On Wednesday, August 14, 2013 04:16:35 pm Stefan Beller \nwrote:\n> On 08/14/2013 07:25 PM, Martin Fick wrote:\n> > I have been holding off a bit on expressing this\n> > opinion too because I don't want to squash someone's\n> > energy to improve things, and because I am not yet a\n> > git dev, but since it was brought up anyway...\n> \n> It's ok, if you knew a better topic to work on, I'd\n> gladly switch over. (Given it would be a good beginners\n> topic.)\n\nSee below...\n\n> > I can say that as a user, having git-repack as a shell\n> > script has been very valuable. For example: we have\n> > modified it for our internal use to no longer ever\n> > overwrite new packfiles with the same name as the\n> > current packfile. This modification was relatively\n> > easy to do and see in shell script. If this were C\n> > code I can't imagine having personally: 1) identified\n> > that there was an issue with the original git-repack\n> > (it temporarily makes objects unavailable) 2) made a\n> > simple custom fix to that policy.\n> \n> Looking at the `git log -- git-repack.sh` the last commit\n> is from April 2012 and the commit before is 2011, so I\n> assumed it stable enough for porting over to C, as there\n> is not much modification going on. I'd be glad to\n> include these changes you're using into the rewrite or\n> the shell script as of now.\n\nOne suggestion would be to change the repack code to create \npack filenames based on the sha1 of the contents of the pack \nfile instead of on the sha1 of the objects in the packfile. \n\nSince the same objects can be stored in a packfile in many \nways (different deltification/compression options), it is \ncurrently possible to have 2 different pack files with the \nsame names. The contents are different, but the contained \nobjects are the same. This causes the object availability \nbug that I describe above in git repack when a new packfile \nis generated with the same name as a current one.\n\nI am not 100% sure if the change in naming convention I \npropose wouldn't cause any problems? But if others agree it \nis a good idea, perhaps it is something a beginner could do?\n\n-Martin\n\n-- \nThe Qualcomm Innovation Center, Inc. is a member of Code \nAurora Forum, hosted by The Linux Foundation\n \n"},{"id":"225233","messageId":"vpq7gfnj38d.fsf@anie.imag.fr","threadId":"34687","inReplyTo":"CALWbr2xuV+V7M354+XoA3HCvLr0Cpx4t3cLVeTCx4xeNmQQX1w@mail.gmail.com","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-08-14T22:51:14Z","receivedAt":"2013-08-14T22:51:14Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Antoine Pelisse writes:\n\n> On Wed, Aug 14, 2013 at 6:27 PM, Stefan Beller\n> wrote:\n>> builtin/repack.c | 410 +++++++++++++++++++++++++++++++++++++++++\n>> contrib/examples/git-repack.sh | 194 +++++++++++++++++++\n>> git-repack.sh | 194 -------------------\n>\n> I'm still not sure I understand the trade-off here.\n>\n> Most of what git-repack does is compute some file paths, (re)move\n> those files and call git-pack-objects, and potentially\n> git-prune-packed and git-update-server-info.\n> Maybe I'm wrong, but I have the feeling that the correct tool for that\n> is Shell, rather than C (and I think the code looks less intuitive in\n> C for that matter).\n\nThere's a real problem with git-repack being shell (I already mentionned\nit in the previous thread about the rewrite): it creates dependencies on\na few external binaries, and a restricted server may not have them. I\nhave this issue on a fusionforge server where Git repos are accessed in\na chroot with very few commands available: everything went OK until the\nfirst project grew enough to require a \"git gc --auto\", and then it\nstopped accepting pushes for that project.\n\nI tracked down the origin of the problem and the sysadmins disabled\nauto-gc, but that's not a very satisfactory solution.\n\nC is rather painfull to write, but as a sysadmin, drop the binary on\nyour server and it just works. That's really important. AFAIK,\ngit-repack is the only remaining shell part on the server, and it's\nrather small. I'd really love to see it disapear.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"225234","messageId":"7vhaersx74.fsf@alter.siamese.dyndns.org","threadId":"34687","inReplyTo":"520C01C3.2060804@googlemail.com","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-14T22:51:27Z","receivedAt":"2013-08-14T22:51:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> I asked for a todo wish list a few weeks ago, but got no real answer,\n> but rather: \"Pick your choice and try to come up with good patches\".\n\nHmph, I hope that wasn't me.\n\nThere are some good ones here;\n\n http://git-blame.blogspot.com/search?q=leftover\n\nSome are trivial, some are moderate complexity, and some are\npie-in-the-sky.\n"},{"id":"225235","messageId":"7vd2pfsx3j.fsf@alter.siamese.dyndns.org","threadId":"34687","inReplyTo":"201308141628.42163.mfick@codeaurora.org","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-14T22:53:36Z","receivedAt":"2013-08-14T22:53:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Fick writes:\n\n> One suggestion would be to change the repack code to create \n> pack filenames based on the sha1 of the contents of the pack \n> file instead of on the sha1 of the objects in the packfile. \n> ...\n> I am not 100% sure if the change in naming convention I \n> propose wouldn't cause any problems? But if others agree it \n> is a good idea, perhaps it is something a beginner could do?\n\nI would not be surprised if that change breaks some other people's\nreimplementation. I know we do not validate the pack name with the\nhash of the contents in the current code, but at the same time I do\nremember that was one of the planned things to be done while I and\nLinus were working on the original pack design, which was the last\ntask we did together before he retired from the maintainership of\nthis project.\n"},{"id":"225236","messageId":"vpqy583hoax.fsf@anie.imag.fr","threadId":"34687","inReplyTo":"7vhaersx74.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-08-14T22:59:02Z","receivedAt":"2013-08-14T22:59:02Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Stefan Beller writes:\n>\n>> I asked for a todo wish list a few weeks ago, but got no real answer,\n>> but rather: \"Pick your choice and try to come up with good patches\".\n>\n> Hmph, I hope that wasn't me.\n>\n> There are some good ones here;\n>\n> http://git-blame.blogspot.com/search?q=leftover\n\nSee also:\n\n https://git.wiki.kernel.org/index.php/SmallProjectsIdeas\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"225238","messageId":"201308141725.43127.mfick@codeaurora.org","threadId":"34687","inReplyTo":"vpq7gfnj38d.fsf@anie.imag.fr","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Martin Fick","fromEmail":"mfick@codeaurora.org","sentAt":"2013-08-14T23:25:42Z","receivedAt":"2013-08-14T23:25:42Z","isPatch":true,"sender":{"key":"mfick@codeaurora.org","avatar":null},"body":"On Wednesday, August 14, 2013 04:51:14 pm Matthieu Moy \nwrote:\n> Antoine Pelisse writes:\n> > On Wed, Aug 14, 2013 at 6:27 PM, Stefan Beller\n> > \n> > wrote:\n> >> builtin/repack.c | 410\n> >> +++++++++++++++++++++++++++++++++++++++++\n> >> contrib/examples/git-repack.sh | 194\n> >> +++++++++++++++++++ git-repack.sh |\n> >> 194 -------------------\n> > \n> > I'm still not sure I understand the trade-off here.\n> > \n> > Most of what git-repack does is compute some file\n> > paths, (re)move those files and call git-pack-objects,\n> > and potentially git-prune-packed and\n> > git-update-server-info.\n> > Maybe I'm wrong, but I have the feeling that the\n> > correct tool for that is Shell, rather than C (and I\n> > think the code looks less intuitive in C for that\n> > matter).\n> \n> There's a real problem with git-repack being shell (I\n> already mentionned it in the previous thread about the\n> rewrite): it creates dependencies on a few external\n> binaries, and a restricted server may not have them. I\n> have this issue on a fusionforge server where Git repos\n> are accessed in a chroot with very few commands\n> available: everything went OK until the first project\n> grew enough to require a \"git gc --auto\", and then it\n> stopped accepting pushes for that project.\n> \n> I tracked down the origin of the problem and the\n> sysadmins disabled auto-gc, but that's not a very\n> satisfactory solution.\n> \n> C is rather painfull to write, but as a sysadmin, drop\n> the binary on your server and it just works. That's\n> really important. AFAIK, git-repack is the only\n> remaining shell part on the server, and it's rather\n> small. I'd really love to see it disapear.\n\nI didn't review the proposed C version, but how was it \nplanning on removing the dependencies on these binaries? \nWas it planning to reimplement mv, cp, find? Were there \nother binaries that were problematic that you were thinking \nof? From what I can tell it also uses test, mkdir, sed, \nchmod and naturally sh, that is 8 dependencies. If those \ncan't be depended upon for existing, perhaps git should just \nconsider bundling busy-box or some other limited shell \nutils, or yikes!, even its own reimplementation of these \ninstead of implementing these independently inside other git \nprograms?\n\n-Martin\n\n-- \nThe Qualcomm Innovation Center, Inc. is a member of Code \nAurora Forum, hosted by The Linux Foundation\n \n"},{"id":"225239","messageId":"201308141728.29971.mfick@codeaurora.org","threadId":"34687","inReplyTo":"7vd2pfsx3j.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Martin Fick","fromEmail":"mfick@codeaurora.org","sentAt":"2013-08-14T23:28:29Z","receivedAt":"2013-08-14T23:28:29Z","isPatch":true,"sender":{"key":"mfick@codeaurora.org","avatar":null},"body":"On Wednesday, August 14, 2013 04:53:36 pm Junio C Hamano \nwrote:\n> Martin Fick writes:\n> > One suggestion would be to change the repack code to\n> > create pack filenames based on the sha1 of the\n> > contents of the pack file instead of on the sha1 of\n> > the objects in the packfile. ...\n> > I am not 100% sure if the change in naming convention I\n> > propose wouldn't cause any problems? But if others\n> > agree it is a good idea, perhaps it is something a\n> > beginner could do?\n> \n> I would not be surprised if that change breaks some other\n> people's reimplementation. I know we do not validate\n> the pack name with the hash of the contents in the\n> current code, but at the same time I do remember that\n> was one of the planned things to be done while I and\n> Linus were working on the original pack design, which\n> was the last task we did together before he retired from\n> the maintainership of this project.\n\nPerhaps a config option? One that becomes standard for git \n2.0?\n\n-Martin\n\n-- \nThe Qualcomm Innovation Center, Inc. is a member of Code \nAurora Forum, hosted by The Linux Foundation\n \n"},{"id":"225240","messageId":"201308141826.44898.mfick@codeaurora.org","threadId":"34687","inReplyTo":"201308141725.43127.mfick@codeaurora.org","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Martin Fick","fromEmail":"mfick@codeaurora.org","sentAt":"2013-08-15T00:26:44Z","receivedAt":"2013-08-15T00:26:44Z","isPatch":true,"sender":{"key":"mfick@codeaurora.org","avatar":null},"body":"On Wednesday, August 14, 2013 05:25:42 pm Martin Fick wrote:\n> On Wednesday, August 14, 2013 04:51:14 pm Matthieu Moy\n> \n> wrote:\n> > Antoine Pelisse writes:\n> > > On Wed, Aug 14, 2013 at 6:27 PM, Stefan Beller\n> > > \n> > > wrote:\n> > >> builtin/repack.c | 410\n> > >> +++++++++++++++++++++++++++++++++++++++++\n> > >> contrib/examples/git-repack.sh | 194\n> > >> +++++++++++++++++++ git-repack.sh \n> > >> | 194 -------------------\n> > > \n> > > I'm still not sure I understand the trade-off here.\n> > > \n> > > Most of what git-repack does is compute some file\n> > > paths, (re)move those files and call\n> > > git-pack-objects, and potentially git-prune-packed\n> > > and\n> > > git-update-server-info.\n> > > Maybe I'm wrong, but I have the feeling that the\n> > > correct tool for that is Shell, rather than C (and I\n> > > think the code looks less intuitive in C for that\n> > > matter).\n> > \n> > There's a real problem with git-repack being shell (I\n> > already mentionned it in the previous thread about the\n> > rewrite): it creates dependencies on a few external\n> > binaries, and a restricted server may not have them. I\n> > have this issue on a fusionforge server where Git repos\n> > are accessed in a chroot with very few commands\n> > available: everything went OK until the first project\n> > grew enough to require a \"git gc --auto\", and then it\n> > stopped accepting pushes for that project.\n> > \n> > I tracked down the origin of the problem and the\n> > sysadmins disabled auto-gc, but that's not a very\n> > satisfactory solution.\n> > \n> > C is rather painfull to write, but as a sysadmin, drop\n> > the binary on your server and it just works. That's\n> > really important. AFAIK, git-repack is the only\n> > remaining shell part on the server, and it's rather\n> > small. I'd really love to see it disapear.\n> \n> I didn't review the proposed C version, but how was it\n> planning on removing the dependencies on these binaries?\n> Was it planning to reimplement mv, cp, find? Were there\n> other binaries that were problematic that you were\n> thinking of? From what I can tell it also uses test,\n> mkdir, sed, chmod and naturally sh, that is 8\n> dependencies. If those can't be depended upon for\n> existing, perhaps git should just consider bundling\n> busy-box or some other limited shell utils, or yikes!,\n> even its own reimplementation of these instead of\n> implementing these independently inside other git\n> programs?\n\nSorry I didn't comprehend your email fully when I first read \nit. I guess that wouldn't really solve your problem unless \nsomeone had a way of bundling an sh program and whatever it \ncalls inside a single executable? :(\n\nI can see why you would want what you want,\n\n-Martin\n\n-- \nThe Qualcomm Innovation Center, Inc. is a member of Code \nAurora Forum, hosted by The Linux Foundation\n \n"},{"id":"225245","messageId":"CACsJy8AZ+xLURszWni1wdZw7e6ZvE0je9_ELcj6gCzaEyNNLmQ@mail.gmail.com","threadId":"34687","inReplyTo":"201308141125.59991.mfick@codeaurora.org","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-08-15T04:15:06Z","receivedAt":"2013-08-15T04:15:06Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Aug 15, 2013 at 12:25 AM, Martin Fick wrote:\n> The script really is mostly a policy script, and with the\n> discussions happening in other threads about how to improve\n> git gc, I think it is helpful to potentially be able to\n> quickly modify the policies in this script, it makes it\n> easier to prototype things. Shell portability issues aside,\n> this script is not a low level type of tool that I feel will\n> benefit from being in C, I feel it will in fact be worse off\n> in C,\n\nI think C is better for the modification you made in git-exproll.sh,\nif it gets merged to git-repack.sh. Such calculations are not a strong\npoint of shell scripting. git-repack.sh is still around for\nexperimenting, although I think perl, ruby or python is better than\nshell for prototyping.\n-- \nDuy\n"},{"id":"225246","messageId":"CACsJy8AJ4eTXXArvN0eTC-_j7oSE8V+8zCDYP3gm4KZUrH90jA@mail.gmail.com","threadId":"34687","inReplyTo":"vpq7gfnj38d.fsf@anie.imag.fr","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-08-15T04:20:39Z","receivedAt":"2013-08-15T04:20:39Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Aug 15, 2013 at 5:51 AM, Matthieu Moy\n wrote:\n> There's a real problem with git-repack being shell (I already mentionned\n> it in the previous thread about the rewrite): it creates dependencies on\n> a few external binaries, and a restricted server may not have them.\n\nThere's also the Windows port. A POSIX shell environment is required\nfor using git on Windows, but I feel one should be able to use core\nfunctionlity even without POSIX utilities. git-repack is part of this\ncore in my opinion.\n-- \nDuy\n"},{"id":"225250","messageId":"520C873A.9070900@googlemail.com","threadId":"34687","inReplyTo":"201308141725.43127.mfick@codeaurora.org","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-08-15T07:46:02Z","receivedAt":"2013-08-15T07:46:02Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 08/15/2013 01:25 AM, Martin Fick wrote:\n> On Wednesday, August 14, 2013 04:51:14 pm Matthieu Moy \n> wrote:\n>> Antoine Pelisse writes:\n>>> On Wed, Aug 14, 2013 at 6:27 PM, Stefan Beller\n>>>\n>>> wrote:\n>>>> builtin/repack.c | 410\n>>>> +++++++++++++++++++++++++++++++++++++++++\n>>>> contrib/examples/git-repack.sh | 194\n>>>> +++++++++++++++++++ git-repack.sh |\n>>>> 194 -------------------\n>>>\n>>> I'm still not sure I understand the trade-off here.\n>>>\n>>> Most of what git-repack does is compute some file\n>>> paths, (re)move those files and call git-pack-objects,\n>>> and potentially git-prune-packed and\n>>> git-update-server-info.\n>>> Maybe I'm wrong, but I have the feeling that the\n>>> correct tool for that is Shell, rather than C (and I\n>>> think the code looks less intuitive in C for that\n>>> matter).\n>>\n>> There's a real problem with git-repack being shell (I\n>> already mentionned it in the previous thread about the\n>> rewrite): it creates dependencies on a few external\n>> binaries, and a restricted server may not have them. I\n>> have this issue on a fusionforge server where Git repos\n>> are accessed in a chroot with very few commands\n>> available: everything went OK until the first project\n>> grew enough to require a \"git gc --auto\", and then it\n>> stopped accepting pushes for that project.\n>>\n>> I tracked down the origin of the problem and the\n>> sysadmins disabled auto-gc, but that's not a very\n>> satisfactory solution.\n>>\n>> C is rather painfull to write, but as a sysadmin, drop\n>> the binary on your server and it just works. That's\n>> really important. AFAIK, git-repack is the only\n>> remaining shell part on the server, and it's rather\n>> small. I'd really love to see it disapear.\n> \n> I didn't review the proposed C version, but how was it \n> planning on removing the dependencies on these binaries? \n> Was it planning to reimplement mv, cp, find? \n\nThese small programms (at least mv and cp) are just convenient\ninterfaces for system calls from within the shell.\nYou can use these system calls to achieve a similar results\ncompared to the commandline option.\nhttp://linux.die.net/man/2/rename\nhttp://linux.die.net/man/2/unlink\n\n> Were there \n> other binaries that were problematic that you were thinking \n> of? From what I can tell it also uses test, mkdir, sed, \n> chmod and naturally sh, that is 8 dependencies. \n\nmkdir, test, chmod are also easily done via system calls.\nThe system calls are usually capsulated by the libc to have\nan easy C interface. (A standard C function call)\n\nsed and find are tricky indeed, but you can get around it with\na few lines of C (maybe 10?) for each occurrence. We don't need\nthe full power of sed and find, but rather only the exact specific\nmatching regexp.\n\n If those\n> can't be depended upon for existing, perhaps git should just \n> consider bundling busy-box or some other limited shell \n> utils, or yikes!, even its own reimplementation of these \n> instead of implementing these independently inside other git \n> programs?\n> \n\nThe C version as of now has twice the lines of code than the\nshell version. And I am pretty sure I did some rookie mistakes,\nso the code can be down-sized by better use of already existing\nfunctions. So I guess the final version will have less lines than\nin the proposed patch as of now.\n\n\n"},{"id":"225251","messageId":"520C879E.6090101@googlemail.com","threadId":"34687","inReplyTo":"vpqy583hoax.fsf@anie.imag.fr","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-08-15T07:47:42Z","receivedAt":"2013-08-15T07:47:42Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 08/15/2013 12:59 AM, Matthieu Moy wrote:\n> Junio C Hamano writes:\n> \n>> Stefan Beller writes:\n>>\n>>> I asked for a todo wish list a few weeks ago, but got no real answer,\n>>> but rather: \"Pick your choice and try to come up with good patches\".\n>>\n>> Hmph, I hope that wasn't me.\n>>\n>> There are some good ones here;\n>>\n>> http://git-blame.blogspot.com/search?q=leftover\n> \n> See also:\n> \n> https://git.wiki.kernel.org/index.php/SmallProjectsIdeas\n> \n\nThanks, I bookmarked both of those pages.\nThe wiki already lists Rewrite \"git repack\" in C at the\nvery end, so my sense where to start wasn't way off. ;)\n\n\n\n"},{"id":"225252","messageId":"520C88E4.2040107@googlemail.com","threadId":"34687","inReplyTo":"7vli44td9k.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-08-15T07:53:08Z","receivedAt":"2013-08-15T07:53:08Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 08/14/2013 07:04 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n> \n>> diff --git a/builtin/repack.c b/builtin/repack.c\n>> new file mode 100644\n>> index 0000000..d39c34e\n>> --- /dev/null\n>> +++ b/builtin/repack.c\n>> @@ -0,0 +1,410 @@\n>> +/*\n>> + * The shell version was written by Linus Torvalds (2005) and many others.\n>> + * This is a translation into C by Stefan Beller (2013)\n>> + */\n>> +\n>> +#include \"builtin.h\"\n>> +#include \"cache.h\"\n>> +#include \"dir.h\"\n>> +#include \"parse-options.h\"\n>> +#include \"run-command.h\"\n>> +#include \"sigchain.h\"\n>> +#include \"strbuf.h\"\n>> +#include \"string-list.h\"\n>> +\n>> +#include \n>> +#include \n>> +#include \n>> +#include \n> \n> If you need these system-includes here, it means that our own\n> platform-portability layer \"git-compat-util.h\" is broken. On\n> various systems, often some system header files are missing, need a\n> few feature macros to be defined before including, and/or need to be\n> included in certain order, etc., and \"git-compat-util.h\" is meant to\n> hide all such details from the programmers.\n> \n> I do not think the above four needs to be included in *.c, as long\n> as you include either builtin.h or cache.h, both of which includes\n> the compat-util header.\n> \n\nThanks. It works without these includes as well. I think I got those\nincludes before I realized there is so much infrastructure already\navailable and I forgot to remove these includes once I added the\nothers.\n\n\n\n"},{"id":"225264","messageId":"201308150904.40154.mfick@codeaurora.org","threadId":"34687","inReplyTo":"520C873A.9070900@googlemail.com","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Martin Fick","fromEmail":"mfick@codeaurora.org","sentAt":"2013-08-15T15:04:39Z","receivedAt":"2013-08-15T15:04:39Z","isPatch":true,"sender":{"key":"mfick@codeaurora.org","avatar":null},"body":"On Thursday, August 15, 2013 01:46:02 am Stefan Beller \nwrote:\n> On 08/15/2013 01:25 AM, Martin Fick wrote:\n> > On Wednesday, August 14, 2013 04:51:14 pm Matthieu Moy\n> > \n> > wrote:\n> >> Antoine Pelisse writes:\n> >>> On Wed, Aug 14, 2013 at 6:27 PM, Stefan Beller\n> >>> \n> >>> wrote:\n> >>>> builtin/repack.c | 410\n> >>>> +++++++++++++++++++++++++++++++++++++++++\n> >>>> contrib/examples/git-repack.sh | 194\n> >>>> +++++++++++++++++++ git-repack.sh \n> >>>> | 194 -------------------\n> >>> \n> >>> I'm still not sure I understand the trade-off here.\n> >>> \n> >>> Most of what git-repack does is compute some file\n> >>> paths, (re)move those files and call\n> >>> git-pack-objects, and potentially git-prune-packed\n> >>> and\n> >>> git-update-server-info.\n> >>> Maybe I'm wrong, but I have the feeling that the\n> >>> correct tool for that is Shell, rather than C (and I\n> >>> think the code looks less intuitive in C for that\n> >>> matter).\n> >> \n> >> There's a real problem with git-repack being shell (I\n> >> already mentionned it in the previous thread about the\n> >> rewrite): it creates dependencies on a few external\n> >> binaries, and a restricted server may not have them. I\n> >> have this issue on a fusionforge server where Git\n> >> repos are accessed in a chroot with very few commands\n> >> available: everything went OK until the first project\n> >> grew enough to require a \"git gc --auto\", and then it\n> >> stopped accepting pushes for that project.\n> >> \n> >> I tracked down the origin of the problem and the\n> >> sysadmins disabled auto-gc, but that's not a very\n> >> satisfactory solution.\n> >> \n> >> C is rather painfull to write, but as a sysadmin, drop\n> >> the binary on your server and it just works. That's\n> >> really important. AFAIK, git-repack is the only\n> >> remaining shell part on the server, and it's rather\n> >> small. I'd really love to see it disapear.\n> > \n> > I didn't review the proposed C version, but how was it\n> > planning on removing the dependencies on these\n> > binaries? Was it planning to reimplement mv, cp, find?\n> \n> These small programms (at least mv and cp) are just\n> convenient interfaces for system calls from within the\n> shell. You can use these system calls to achieve a\n> similar results compared to the commandline option.\n> http://linux.die.net/man/2/rename\n> http://linux.die.net/man/2/unlink\n\nSure, but have you ever looked at the code to mv? It isn't \npretty. ;( But in all that ugliness is decades worth of \nportability and corner cases. Also, mv is smart enough to \ncopy when rename doesn't work (on some systems it doesn't). \nSo C may sound more portable, but I am not sure it actually \nis. Now hopefully you won't need all of that, but I think \nthat some of the design decision that went into git-repack \ndid consider some of the more eccentric filesystems out \nthere,\n\n-Martin\n\n-- \nThe Qualcomm Innovation Center, Inc. is a member of Code \nAurora Forum, hosted by The Linux Foundation\n \n"},{"id":"225266","messageId":"7v8v02swnc.fsf@alter.siamese.dyndns.org","threadId":"34687","inReplyTo":"201308141728.29971.mfick@codeaurora.org","subject":"Re: [RFC PATCH] repack: rewrite the shell script in C.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-15T17:15:35Z","receivedAt":"2013-08-15T17:15:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Fick writes:\n\n> On Wednesday, August 14, 2013 04:53:36 pm Junio C Hamano \n> wrote:\n>> Martin Fick writes:\n>> > One suggestion would be to change the repack code to\n>> > create pack filenames based on the sha1 of the\n>> > contents of the pack file instead of on the sha1 of\n>> > the objects in the packfile. ...\n>> > I am not 100% sure if the change in naming convention I\n>> > propose wouldn't cause any problems? But if others\n>> > agree it is a good idea, perhaps it is something a\n>> > beginner could do?\n>> \n>> I would not be surprised if that change breaks some other\n>> people's reimplementation. I know we do not validate\n>> the pack name with the hash of the contents in the\n>> current code, but at the same time I do remember that\n>> was one of the planned things to be done while I and\n>> Linus were working on the original pack design, which\n>> was the last task we did together before he retired from\n>> the maintainership of this project.\n>\n> Perhaps a config option? One that becomes standard for git \n> 2.0?\n\nAnything new is too late for Git 2.0, as we do not want to hold the\nswitching of push.default to \"simple\" too long. End of this year\nmight be a bit too soon, but I want 2.0 to happen by the next\nspring.\n\nYou can discuss, design the new naming and necessary transition plan\nfor existing repositories, reach a concensus and declare the name\nswitch in the future, and then schedule that for the next major\nversion bump after 2.0 happens.\n"},{"id":"225303","messageId":"1376611978-13189-1-git-send-email-stefanbeller@googlemail.com","threadId":"34687","inReplyTo":"7v8v02swnc.fsf@alter.siamese.dyndns.org","subject":"[RFC PATCHv2] repack: rewrite the shell script in C.","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-08-16T00:12:58Z","receivedAt":"2013-08-16T00:12:58Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This is the beginning of the rewrite of the repacking.\n\n * Removed unneeded system header files\n * corrected remove_pack to really remove any pack files with the given\n sha1\n * fail if pack-objects fails\n * Only test t7701 (2nd) fails now with this patch.\n\nSigned-off-by: Stefan Beller \n---\n Makefile | 2 +-\n builtin.h | 1 +\n builtin/repack.c | 411 ++++++++++++++++++++++++\n git-repack.sh => contrib/examples/git-repack.sh | 0\n git.c | 1 +\n 5 files changed, 414 insertions(+), 1 deletion(-)\n create mode 100644 builtin/repack.c\n rename git-repack.sh => contrib/examples/git-repack.sh (100%)\n\ndiff --git a/Makefile b/Makefile\nindex ef442eb..4ec5bbe 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -464,7 +464,6 @@ SCRIPT_SH += git-pull.sh\n SCRIPT_SH += git-quiltimport.sh\n SCRIPT_SH += git-rebase.sh\n SCRIPT_SH += git-remote-testgit.sh\n-SCRIPT_SH += git-repack.sh\n SCRIPT_SH += git-request-pull.sh\n SCRIPT_SH += git-stash.sh\n SCRIPT_SH += git-submodule.sh\n@@ -972,6 +971,7 @@ BUILTIN_OBJS += builtin/reflog.o\n BUILTIN_OBJS += builtin/remote.o\n BUILTIN_OBJS += builtin/remote-ext.o\n BUILTIN_OBJS += builtin/remote-fd.o\n+BUILTIN_OBJS += builtin/repack.o\n BUILTIN_OBJS += builtin/replace.o\n BUILTIN_OBJS += builtin/rerere.o\n BUILTIN_OBJS += builtin/reset.o\ndiff --git a/builtin.h b/builtin.h\nindex 8afa2de..b56cf07 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -102,6 +102,7 @@ extern int cmd_reflog(int argc, const char **argv, const char *prefix);\n extern int cmd_remote(int argc, const char **argv, const char *prefix);\n extern int cmd_remote_ext(int argc, const char **argv, const char *prefix);\n extern int cmd_remote_fd(int argc, const char **argv, const char *prefix);\n+extern int cmd_repack(int argc, const char **argv, const char *prefix);\n extern int cmd_repo_config(int argc, const char **argv, const char *prefix);\n extern int cmd_rerere(int argc, const char **argv, const char *prefix);\n extern int cmd_reset(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/repack.c b/builtin/repack.c\nnew file mode 100644\nindex 0000000..f72911d\n--- /dev/null\n+++ b/builtin/repack.c\n@@ -0,0 +1,411 @@\n+/*\n+ * The shell version was written by Linus Torvalds (2005) and many others.\n+ * This is a translation into C by Stefan Beller (2013)\n+ */\n+\n+#include \"builtin.h\"\n+#include \"cache.h\"\n+#include \"dir.h\"\n+#include \"parse-options.h\"\n+#include \"run-command.h\"\n+#include \"sigchain.h\"\n+#include \"strbuf.h\"\n+#include \"string-list.h\"\n+\n+static const char *const git_repack_usage[] = {\n+\tN_(\"git repack [options]\"),\n+\tNULL\n+};\n+\n+/* enabled by default since 22c79eab (2008-06-25) */\n+static int delta_base_offset = 1;\n+\n+static int repack_config(const char *var, const char *value, void *cb)\n+{\n+\tif (!strcmp(var, \"repack.usedeltabaseoffset\")) {\n+\t\tdelta_base_offset = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\treturn git_default_config(var, value, cb);\n+}\n+\n+static void remove_temporary_files() {\n+\tDIR *dir;\n+\tstruct dirent *e;\n+\tchar *prefix, *path, *fname;\n+\n+\tprefix = xmalloc(strlen(\".tmp-10000-pack\") + 1);\n+\tsprintf(prefix, \".tmp-%d-pack\", getpid());\n+\n+\tpath = xmalloc(strlen(get_object_directory()) + strlen(\"/pack\") + 1);\n+\tsprintf(path, \"%s/pack\", get_object_directory());\n+\n+\tfname = xmalloc(strlen(path) + strlen(\"/\")\n+\t\t+ strlen(prefix) + strlen(\"/\")\n+\t\t+ 40 + strlen(\".pack\") + 1);\n+\n+\tdir = opendir(path);\n+\twhile ((e = readdir(dir)) != NULL) {\n+\t\tif (!prefixcmp(e->d_name, prefix)) {\n+\t\t\tsprintf(fname, \"%s/%s\", path, e->d_name);\n+\t\t\tunlink(fname);\n+\t\t}\n+\t}\n+\tfree(fname);\n+\tfree(prefix);\n+\tfree(path);\n+\tclosedir(dir);\n+}\n+\n+static void remove_pack_on_signal(int signo)\n+{\n+\tremove_temporary_files();\n+\tsigchain_pop(signo);\n+\traise(signo);\n+}\n+\n+void get_pack_sha1_list(char *packdir, struct string_list *sha1_list)\n+{\n+\tDIR *dir;\n+\tstruct dirent *e;\n+\tchar *path, *suffix;\n+\n+\tpath = xmalloc(strlen(get_object_directory()) + strlen(\"/pack\") + 1);\n+\tsprintf(path, \"%s/pack\", get_object_directory());\n+\n+\tsuffix = \".pack\";\n+\n+\tdir = opendir(path);\n+\twhile ((e = readdir(dir)) != NULL) {\n+\t\tif (!suffixcmp(e->d_name, suffix)) {\n+\t\t\tchar buf[255], *sha1;\n+\t\t\tstrcpy(buf, e->d_name);\n+\t\t\tbuf[strlen(e->d_name) - strlen(suffix)] = '\\0';\n+\t\t\tsha1 = &buf[strlen(e->d_name) - strlen(suffix) - 40];\n+\t\t\tstring_list_append(sha1_list, sha1);\n+\t\t}\n+\t}\n+\tfree(path);\n+\tclosedir(dir);\n+}\n+\n+/*\n+ * remove_pack will remove any files following the pattern *${SHA1}.{EXT}\n+ * where EXT is one of {pack, idx, keep}. The SHA1 consists of 40 chars and\n+ * is specified by the sha1 parameter.\n+ * path is specifying the directory in which all found files will be deleted.\n+ */\n+void remove_pack(char *path, char* sha1)\n+{\n+\tDIR *dir;\n+\tstruct dirent *e;\n+\n+\tdir = opendir(path);\n+\twhile ((e = readdir(dir)) != NULL) {\n+\n+\t\tchar *sha_begin, *sha_end;\n+\t\tsha_end = e->d_name + strlen(e->d_name);\n+\t\twhile (sha_end > e->d_name && *sha_end != '.')\n+\t\t\tsha_end--;\n+\n+\t\t/* do not touch files not ending in .pack, .idx or .keep */\n+\t\tif (strcmp(sha_end, \".pack\") &&\n+\t\t\tstrcmp(sha_end, \".idx\") &&\n+\t\t\tstrcmp(sha_end, \".keep\"))\n+\t\t\tcontinue;\n+\n+\t\tsha_begin = sha_end - 40;\n+\n+\t\tif (sha_begin >= e->d_name && !strncmp(sha_begin, sha1, 40)) {\n+\t\t\tchar *fname;\n+\t\t\tfname = xmalloc(strlen(path) + 1 + strlen(e->d_name));\n+\t\t\tsprintf(fname, \"%s/%s\", path, e->d_name);\n+\t\t\tunlink(fname);\n+\t\t\t/*TODO: free(fname); fails here sometimes, needs investigation*/\n+\t\t}\n+\t}\n+\tclosedir(dir);\n+}\n+\n+int cmd_repack(int argc, const char **argv, const char *prefix) {\n+\n+\tint pack_everything = 0;\n+\tint pack_everything_but_loose = 0;\n+\tint delete_redundant = 0;\n+\tunsigned long unpack_unreachable = 0;\n+\tint window = 0, window_memory = 0;\n+\tint depth = 0;\n+\tint max_pack_size = 0;\n+\tint no_reuse_delta = 0, no_reuse_object = 0;\n+\tint no_update_server_info = 0;\n+\tint quiet = 0;\n+\tint local = 0;\n+\tchar *packdir, *packtmp;\n+\tconst char *cmd_args[20];\n+\tint cmd_i = 0;\n+\tstruct child_process cmd;\n+\tstruct string_list_item *item;\n+\tstruct string_list existing_packs = STRING_LIST_INIT_DUP;\n+\tstruct stat statbuffer;\n+\tchar window_str[64], window_mem_str[64], depth_str[64], max_pack_size_str[64];\n+\n+\tstruct option builtin_repack_options[] = {\n+\t\tOPT_BOOL('a', \"all\", &pack_everything,\n+\t\t\t\tN_(\"pack everything in a single pack\")),\n+\t\tOPT_BOOL('A', \"all-but-loose\", &pack_everything_but_loose,\n+\t\t\t\tN_(\"same as -a, and turn unreachable objects loose\")),\n+\t\tOPT_BOOL('d', \"delete-redundant\", &delete_redundant,\n+\t\t\t\tN_(\"remove redundant packs, and run git-prune-packed\")),\n+\t\tOPT_BOOL('f', \"no-reuse-delta\", &no_reuse_delta,\n+\t\t\t\tN_(\"pass --no-reuse-delta to git-pack-objects\")),\n+\t\tOPT_BOOL('F', \"no-reuse-object\", &no_reuse_object,\n+\t\t\t\tN_(\"pass --no-reuse-object to git-pack-objects\")),\n+\t\tOPT_BOOL('n', NULL, &no_update_server_info,\n+\t\t\t\tN_(\"do not run git-update-server-info\")),\n+\t\tOPT__QUIET(&quiet, N_(\"be quiet\")),\n+\t\tOPT_BOOL('l', \"local\", &local,\n+\t\t\t\tN_(\"pass --local to git-pack-objects\")),\n+\t\tOPT_DATE(0, \"unpack-unreachable\", &unpack_unreachable,\n+\t\t\t\tN_(\"with -A, do not loosen objects older than this Packing constraints\")),\n+\t\tOPT_INTEGER(0, \"window\", &window,\n+\t\t\t\tN_(\"size of the window used for delta compression\")),\n+\t\tOPT_INTEGER(0, \"window-memory\", &window_memory,\n+\t\t\t\tN_(\"same as the above, but limit memory size instead of entries count\")),\n+\t\tOPT_INTEGER(0, \"depth\", &depth,\n+\t\t\t\tN_(\"limits the maximum delta depth\")),\n+\t\tOPT_INTEGER(0, \"max-pack-size\", &max_pack_size,\n+\t\t\t\tN_(\"maximum size of each packfile\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tgit_config(repack_config, NULL);\n+\n+\targc = parse_options(argc, argv, prefix, builtin_repack_options,\n+\t\t\t\tgit_repack_usage, 0);\n+\n+\tsigchain_push_common(remove_pack_on_signal);\n+\n+\tpackdir = mkpath(\"%s/pack\", get_object_directory());\n+\tpacktmp = xmalloc(strlen(packdir) + strlen(\"/.tmp-10000-pack\") + 1);\n+\tsprintf(packtmp, \"%s/.tmp-%d-pack\", packdir, getpid());\n+\n+\tremove_temporary_files();\n+\n+\tcmd_args[cmd_i++] = \"pack-objects\";\n+\tcmd_args[cmd_i++] = \"--keep-true-parents\";\n+\tcmd_args[cmd_i++] = \"--honor-pack-keep\";\n+\tcmd_args[cmd_i++] = \"--non-empty\";\n+\tcmd_args[cmd_i++] = \"--all\";\n+\tcmd_args[cmd_i++] = \"--reflog\";\n+\n+\tif (window) {\n+\t\tsprintf(window_str, \"--window=%u\", window);\n+\t\tcmd_args[cmd_i++] = window_str;\n+\t}\n+\tif (window_memory) {\n+\t\tsprintf(window_mem_str, \"--window-memory=%u\", window_memory);\n+\t\tcmd_args[cmd_i++] = window_str;\n+\t}\n+\tif (depth) {\n+\t\tsprintf(depth_str, \"--depth=%u\", depth);\n+\t\tcmd_args[cmd_i++] = depth_str;\n+\t}\n+\tif (max_pack_size) {\n+\t\tsprintf(max_pack_size_str, \"--max_pack_size=%u\", max_pack_size);\n+\t\tcmd_args[cmd_i++] = max_pack_size_str;\n+\t}\n+\n+\tif (pack_everything + pack_everything_but_loose == 0) {\n+\t\tcmd_args[cmd_i++] = \"--unpacked\";\n+\t\tcmd_args[cmd_i++] = \"--incremental\";\n+\t} else {\n+\t\tif (pack_everything_but_loose)\n+\t\t\tcmd_args[cmd_i++] = \"--unpack-unreachable\";\n+\n+\t\tstruct string_list sha1_list = STRING_LIST_INIT_DUP;\n+\t\tget_pack_sha1_list(packdir, &sha1_list);\n+\t\tfor_each_string_list_item(item, &sha1_list) {\n+\t\t\tchar *fname;\n+\t\t\tfname = xmalloc(strlen(packdir) + strlen(\"/\") + 40 + strlen(\".keep\"));\n+\t\t\tsprintf(fname, \"%s/%s.keep\", packdir, item->string);\n+\t\t\tif (stat(fname, &statbuffer) && S_ISREG(statbuffer.st_mode)) {\n+\t\t\t\t/* when the keep file is there, we're ignoring that pack */\n+\t\t\t} else {\n+\t\t\t\tstring_list_append(&existing_packs, item->string);\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (existing_packs.nr && unpack_unreachable && delete_redundant) {\n+\t\t\t/*\n+\t\t\t * TODO: convert unpack_unreachable (being time since epoch)\n+\t\t\t * to an aproxidate again\n+\t\t\t */\n+\t\t\tcmd_args[cmd_i++] = \"--unpack-unreachable=$DATE\";\n+\t\t}\n+\t}\n+\n+\tif (local)\n+\t\tcmd_args[cmd_i++] = \"--local\";\n+\n+\tif (delta_base_offset)\n+\t\tcmd_args[cmd_i++] = \"--delta-base-offset\";\n+\n+\tcmd_args[cmd_i++] = packtmp;\n+\tcmd_args[cmd_i] = NULL;\n+\n+\tmemset(&cmd, 0, sizeof(cmd));\n+\tcmd.argv = cmd_args;\n+\tcmd.git_cmd = 1;\n+\tcmd.out = -1;\n+\tcmd.no_stdin = 1;\n+\n+\tif (run_command(&cmd))\n+\t\treturn 1;\n+\n+\tstruct string_list names = STRING_LIST_INIT_DUP;\n+\tstruct string_list rollback = STRING_LIST_INIT_DUP;\n+\n+\tchar line[1024];\n+\tint counter = 0;\n+\tFILE *out = xfdopen(cmd.out, \"r\");\n+\twhile (fgets(line, sizeof(line), out)) {\n+\t\t/* a line consists of 40 hex chars + '\\n' */\n+\t\tassert(strlen(line) == 41);\n+\t\tline[40] = '\\0';\n+\t\tstring_list_append(&names, line);\n+\t\tcounter++;\n+\t}\n+\tif (!counter)\n+\t\tprintf(\"Nothing new to pack.\\n\");\n+\tfclose(out);\n+\n+\tchar *fname, *fname_old;\n+\tfname = xmalloc(strlen(packdir) + strlen(\"/old-pack-\") + 40 + strlen(\".pack\") + 1);\n+\tstrcpy(fname, packdir);\n+\tstrcpy(fname + strlen(packdir), \"/\");\n+\n+\tfname_old = xmalloc(strlen(packdir) + strlen(\"/old-pack-\") + 40 + strlen(\".pack\") + 1);\n+\tstrcpy(fname_old, packdir);\n+\tstrcpy(fname_old + strlen(packdir), \"/\");\n+\tchar *exts[2] = {\".idx\", \".pack\"};\n+\n+\tint failed = 0;\n+\n+\tfor_each_string_list_item(item, &names) {\n+\t\tint ext;\n+\t\tfor (ext = 0; ext < 1; ext++) {\n+\t\t\tstrcpy(fname + strlen(packdir) + 1, item->string);\n+\t\t\tstrcpy(fname + strlen(packdir) + 41, exts[ext]);\n+\t\t\tif (!file_exists(fname))\n+\t\t\t\tcontinue;\n+\n+\t\t\tstrcpy(fname_old, packdir);\n+\t\t\tstrcpy(fname_old + strlen(packdir), \"/old-\");\n+\t\t\tstrcpy(fname_old + strlen(packdir) + 5, item->string);\n+\t\t\tstrcpy(fname_old + strlen(packdir) + 45, exts[ext]);\n+\t\t\tif (file_exists(fname_old))\n+\t\t\t\tunlink(fname_old);\n+\n+\t\t\tif (rename(fname, fname_old)) {\n+\t\t\t\tfailed = 1;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tstring_list_append(&rollback, fname);\n+\t\t}\n+\t\tif (failed)\n+\t\t\t/* set to last element to break while loop */\n+\t\t\titem = names.items + names.nr;\n+\t}\n+\tif (failed) {\n+\t\tstruct string_list rollback_failure;\n+\n+\t\tfor_each_string_list_item(item, &rollback) {\n+\t\t\tsprintf(fname, \"%s/%s\", packdir, item->string);\n+\t\t\tsprintf(fname_old, \"%s/old-%s\", packdir, item->string);\n+\t\t\tif (rename(fname_old, fname))\n+\t\t\t\tstring_list_append(&rollback_failure, fname);\n+\t\t}\n+\n+\t\tif (rollback.nr) {\n+\t\t\tint i;\n+\t\t\tfprintf(stderr,\n+\t\t\t\t\"WARNING: Some packs in use have been renamed by\\n\"\n+\t\t\t\t\"WARNING: prefixing old- to their name, in order to\\n\"\n+\t\t\t\t\"WARNING: replace them with the new version of the\\n\"\n+\t\t\t\t\"WARNING: file. But the operation failed, and\\n\"\n+\t\t\t\t\"WARNING: attempt to rename them back to their\\n\"\n+\t\t\t\t\"WARNING: original names also failed.\\n\"\n+\t\t\t\t\"WARNING: Please rename them in $PACKDIR manually:\\n\");\n+\t\t\tfor (i = 0; i < rollback.nr; i++)\n+\t\t\t\tfprintf(stderr, \"WARNING: old-%s -> %s\\n\",\n+\t\t\t\t\trollback.items[i].string,\n+\t\t\t\t\trollback.items[i].string);\n+\t\t}\n+\t\texit(1);\n+\t}\n+\n+\t/* Now the ones with the same name are out of the way... */\n+\tstruct string_list fullbases = STRING_LIST_INIT_DUP;\n+\tfor_each_string_list_item(item, &names) {\n+\t\tstring_list_append(&fullbases, item->string);\n+\n+\t\tsprintf(fname, \"%s/pack-%s.pack\", packdir, item->string);\n+\t\tsprintf(fname_old, \"%s-%s.pack\", packtmp, item->string);\n+\t\tstat(fname_old, &statbuffer);\n+\t\tstatbuffer.st_mode &= ~S_IWOTH;\n+\t\tchmod(fname_old, statbuffer.st_mode);\n+\t\tif (rename(fname_old, fname))\n+\t\t\tdie(\"Could not rename packfile: %s -> %s\", fname_old, fname);\n+\n+\t\tsprintf(fname, \"%s/pack-%s.idx\", packdir, item->string);\n+\t\tsprintf(fname_old, \"%s-%s.idx\", packtmp, item->string);\n+\t\tstat(fname_old, &statbuffer);\n+\t\tstatbuffer.st_mode &= ~S_IWOTH;\n+\t\tchmod(fname_old, statbuffer.st_mode);\n+\t\tif (rename(fname_old, fname))\n+\t\t\tdie(\"Could not rename packfile: %s -> %s\", fname_old, fname);\n+\t}\n+\n+\t/* Remove the \"old-\" files */\n+\tfor_each_string_list_item(item, &names) {\n+\t\tsprintf(fname, \"%s/old-pack-%s.idx\", packdir, item->string);\n+\t\tif (remove_path(fname))\n+\t\t\tdie(\"Could not remove file: %s\", fname);\n+\n+\t\tsprintf(fname, \"%s/old-pack-%s.pack\", packdir, item->string);\n+\t\tif (remove_path(fname))\n+\t\t\tdie(\"Could not remove file: %s\", fname);\n+\t}\n+\n+\t/* End of pack replacement. */\n+\tif (delete_redundant) {\n+\t\tsort_string_list(&fullbases);\n+\t\tfname = xmalloc(strlen(packtmp) + strlen(\"/\") + 40 + strlen(\".pack\") + 1);\n+\t\tfor_each_string_list_item(item, &existing_packs) {\n+\t\t\tif (!string_list_has_string(&fullbases, item->string))\n+\t\t\t\tremove_pack(packdir, item->string);\n+\t\t}\n+\t\tfree(fname);\n+\t\tcmd_i = 0;\n+\t\tcmd_args[cmd_i++] = \"prune-packed\";\n+\t\tcmd_args[cmd_i++] = NULL;\n+\t\t/* TODO: pass argument: ${GIT_QUIET:+-q} */\n+\t\tmemset(&cmd, 0, sizeof(cmd));\n+\t\tcmd.argv = cmd_args;\n+\t\tcmd.git_cmd = 1;\n+\t\trun_command(&cmd);\n+\t}\n+\n+\tif (!no_update_server_info) {\n+\t\tcmd_i = 0;\n+\t\tcmd_args[cmd_i++] = \"update-server-info\";\n+\t\tcmd_args[cmd_i++] = NULL;\n+\n+\t\tmemset(&cmd, 0, sizeof(cmd));\n+\t\tcmd.argv = cmd_args;\n+\t\tcmd.git_cmd = 1;\n+\t\trun_command(&cmd);\n+\t}\n+\treturn 0;\n+}\n+\ndiff --git a/git-repack.sh b/contrib/examples/git-repack.sh\nsimilarity index 100%\nrename from git-repack.sh\nrename to contrib/examples/git-repack.sh\ndiff --git a/git.c b/git.c\nindex 2025f77..03510be 100644\n--- a/git.c\n+++ b/git.c\n@@ -396,6 +396,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"remote\", cmd_remote, RUN_SETUP },\n \t\t{ \"remote-ext\", cmd_remote_ext },\n \t\t{ \"remote-fd\", cmd_remote_fd },\n+\t\t{ \"repack\", cmd_repack, RUN_SETUP },\n \t\t{ \"replace\", cmd_replace, RUN_SETUP },\n \t\t{ \"repo-config\", cmd_repo_config, RUN_SETUP_GENTLY },\n \t\t{ \"rerere\", cmd_rerere, RUN_SETUP },\n-- \n1.8.4.rc3.1.gc1ebd90\n"},{"id":"225370","messageId":"520F7BDE.9090902@web.de","threadId":"34687","inReplyTo":"1376611978-13189-1-git-send-email-stefanbeller@googlemail.com","subject":"Re: [RFC PATCHv2] repack: rewrite the shell script in C.","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2013-08-17T13:34:22Z","receivedAt":"2013-08-17T13:34:22Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":">\n>\n> This is the beginning of the rewrite of the repacking.\n>\n> * Removed unneeded system header files\n> * corrected remove_pack to really remove any pack files with the given\n> sha1\n> * fail if pack-objects fails\n> * Only test t7701 (2nd) fails now with this patch.\n>\n> Signed-off-by: Stefan Beller \n> ---\n> Makefile | 2 +-\n> builtin.h | 1 +\n> builtin/repack.c | 411 ++++++++++++++++++++++++\n> git-repack.sh => contrib/examples/git-repack.sh | 0\n> git.c | 1 +\n> 5 files changed, 414 insertions(+), 1 deletion(-)\n> create mode 100644 builtin/repack.c\n> rename git-repack.sh => contrib/examples/git-repack.sh (100%)\n>\n> diff --git a/Makefile b/Makefile\n> index ef442eb..4ec5bbe 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -464,7 +464,6 @@ SCRIPT_SH += git-pull.sh\n> SCRIPT_SH += git-quiltimport.sh\n> SCRIPT_SH += git-rebase.sh\n> SCRIPT_SH += git-remote-testgit.sh\n> -SCRIPT_SH += git-repack.sh\n> SCRIPT_SH += git-request-pull.sh\n> SCRIPT_SH += git-stash.sh\n> SCRIPT_SH += git-submodule.sh\n> @@ -972,6 +971,7 @@ BUILTIN_OBJS += builtin/reflog.o\n> BUILTIN_OBJS += builtin/remote.o\n> BUILTIN_OBJS += builtin/remote-ext.o\n> BUILTIN_OBJS += builtin/remote-fd.o\n> +BUILTIN_OBJS += builtin/repack.o\n> BUILTIN_OBJS += builtin/replace.o\n> BUILTIN_OBJS += builtin/rerere.o\n> BUILTIN_OBJS += builtin/reset.o\n> diff --git a/builtin.h b/builtin.h\n> index 8afa2de..b56cf07 100644\n> --- a/builtin.h\n> +++ b/builtin.h\n> @@ -102,6 +102,7 @@ extern int cmd_reflog(int argc, const char **argv, const char *prefix);\n> extern int cmd_remote(int argc, const char **argv, const char *prefix);\n> extern int cmd_remote_ext(int argc, const char **argv, const char *prefix);\n> extern int cmd_remote_fd(int argc, const char **argv, const char *prefix);\n> +extern int cmd_repack(int argc, const char **argv, const char *prefix);\n> extern int cmd_repo_config(int argc, const char **argv, const char *prefix);\n> extern int cmd_rerere(int argc, const char **argv, const char *prefix);\n> extern int cmd_reset(int argc, const char **argv, const char *prefix);\n> diff --git a/builtin/repack.c b/builtin/repack.c\n> new file mode 100644\n> index 0000000..f72911d\n> --- /dev/null\n> +++ b/builtin/repack.c\n> @@ -0,0 +1,411 @@\n> +/*\n> + * The shell version was written by Linus Torvalds (2005) and many others.\n> + * This is a translation into C by Stefan Beller (2013)\n> + */\n> +\n> +#include \"builtin.h\"\n> +#include \"cache.h\"\n> +#include \"dir.h\"\n> +#include \"parse-options.h\"\n> +#include \"run-command.h\"\n> +#include \"sigchain.h\"\n> +#include \"strbuf.h\"\n> +#include \"string-list.h\"\n> +\n> +static const char *const git_repack_usage[] = {\n> +\tN_(\"git repack [options]\"),\n> +\tNULL\n> +};\n> +\n> +/* enabled by default since 22c79eab (2008-06-25) */\n> +static int delta_base_offset = 1;\n> +\n> +static int repack_config(const char *var, const char *value, void *cb)\n> +{\n> +\tif (!strcmp(var, \"repack.usedeltabaseoffset\")) {\n> +\t\tdelta_base_offset = git_config_bool(var, value);\n> +\t\treturn 0;\n> +\t}\n> +\treturn git_default_config(var, value, cb);\n> +}\n> +\n> +static void remove_temporary_files() {\n> +\tDIR *dir;\n> +\tstruct dirent *e;\n> +\tchar *prefix, *path, *fname;\n> +\n> +\tprefix = xmalloc(strlen(\".tmp-10000-pack\") + 1);\n> +\tsprintf(prefix, \".tmp-%d-pack\", getpid());\n\nThis will overflow for PIDs with more than five digits. Better use a \nstrbuf and build the string with strbuf_addf. Or better, use mkpathdup, \nwhich does that under the hood.\n\n> +\n> +\tpath = xmalloc(strlen(get_object_directory()) + strlen(\"/pack\") + 1);\n> +\tsprintf(path, \"%s/pack\", get_object_directory());\n\nmkpathdup?\n\n> +\n> +\tfname = xmalloc(strlen(path) + strlen(\"/\")\n> +\t\t+ strlen(prefix) + strlen(\"/\")\n> +\t\t+ 40 + strlen(\".pack\") + 1);\n> +\n> +\tdir = opendir(path);\n> +\twhile ((e = readdir(dir)) != NULL) {\n> +\t\tif (!prefixcmp(e->d_name, prefix)) {\n> +\t\t\tsprintf(fname, \"%s/%s\", path, e->d_name);\n\nIf someone has a directory entry that begins with 'prefix' but is longer \nthan expected this will overflow. That's unlikely, but lets avoid it \noutright. You could use a strbuf instead and reset it to the length of \nthe prefix at the start of the loop (with strbuf_setlen), before adding \nthe entry's name.\n\n> +\t\t\tunlink(fname);\n> +\t\t}\n> +\t}\n> +\tfree(fname);\n> +\tfree(prefix);\n> +\tfree(path);\n> +\tclosedir(dir);\n> +}\n> +\n> +static void remove_pack_on_signal(int signo)\n> +{\n> +\tremove_temporary_files();\n> +\tsigchain_pop(signo);\n> +\traise(signo);\n> +}\n> +\n> +void get_pack_sha1_list(char *packdir, struct string_list *sha1_list)\n> +{\n> +\tDIR *dir;\n> +\tstruct dirent *e;\n> +\tchar *path, *suffix;\n> +\n> +\tpath = xmalloc(strlen(get_object_directory()) + strlen(\"/pack\") + 1);\n> +\tsprintf(path, \"%s/pack\", get_object_directory());\n\nmkpathdup again.\n\nOr would it make sense to cd into the pack directory and avoid these \nstring manipulations outright? Probably not if we need to call other \ngit functions later on.\n\n> +\n> +\tsuffix = \".pack\";\n> +\n> +\tdir = opendir(path);\n> +\twhile ((e = readdir(dir)) != NULL) {\n> +\t\tif (!suffixcmp(e->d_name, suffix)) {\n> +\t\t\tchar buf[255], *sha1;\n> +\t\t\tstrcpy(buf, e->d_name);\n> +\t\t\tbuf[strlen(e->d_name) - strlen(suffix)] = '\\0';\n> +\t\t\tsha1 = &buf[strlen(e->d_name) - strlen(suffix) - 40];\n> +\t\t\tstring_list_append(sha1_list, sha1);\n\nYou could avoid the need for a temporary buffer by using xmemdupz and \nstring_list_append_nodup instead.\n\n> +\t\t}\n> +\t}\n> +\tfree(path);\n> +\tclosedir(dir);\n> +}\n> +\n> +/*\n> + * remove_pack will remove any files following the pattern *${SHA1}.{EXT}\n> + * where EXT is one of {pack, idx, keep}. The SHA1 consists of 40 chars and\n> + * is specified by the sha1 parameter.\n> + * path is specifying the directory in which all found files will be deleted.\n> + */\n> +void remove_pack(char *path, char* sha1)\n> +{\n> +\tDIR *dir;\n> +\tstruct dirent *e;\n> +\n> +\tdir = opendir(path);\n> +\twhile ((e = readdir(dir)) != NULL) {\n> +\n\nExtra newline.\n\n> +\t\tchar *sha_begin, *sha_end;\n> +\t\tsha_end = e->d_name + strlen(e->d_name);\n> +\t\twhile (sha_end > e->d_name && *sha_end != '.')\n> +\t\t\tsha_end--;\n> +\n> +\t\t/* do not touch files not ending in .pack, .idx or .keep */\n> +\t\tif (strcmp(sha_end, \".pack\") &&\n> +\t\t\tstrcmp(sha_end, \".idx\") &&\n> +\t\t\tstrcmp(sha_end, \".keep\"))\n> +\t\t\tcontinue;\n> +\n> +\t\tsha_begin = sha_end - 40;\n> +\n> +\t\tif (sha_begin >= e->d_name && !strncmp(sha_begin, sha1, 40)) {\n> +\t\t\tchar *fname;\n> +\t\t\tfname = xmalloc(strlen(path) + 1 + strlen(e->d_name));\n> +\t\t\tsprintf(fname, \"%s/%s\", path, e->d_name);\n\nmkpathdup..\n\n> +\t\t\tunlink(fname);\n> +\t\t\t/*TODO: free(fname); fails here sometimes, needs investigation*/\n\nStrange. Perhaps valgrind can tell you what's wrong.\n\n> +\t\t}\n> +\t}\n> +\tclosedir(dir);\n> +}\n\nHmm, stepping back a bit, why not just build the paths and call unlink \nfor them right away, without readdir? The shell version only ever \ndeletes existing .pack files (those in $existing alias existing_packs) \nas well as their .idx and .keep files, if present. It doesn't use a \nglob pattern, unlike remove_pack here.\n\n> +\n> +int cmd_repack(int argc, const char **argv, const char *prefix) {\n> +\n> +\tint pack_everything = 0;\n> +\tint pack_everything_but_loose = 0;\n> +\tint delete_redundant = 0;\n> +\tunsigned long unpack_unreachable = 0;\n> +\tint window = 0, window_memory = 0;\n> +\tint depth = 0;\n> +\tint max_pack_size = 0;\n> +\tint no_reuse_delta = 0, no_reuse_object = 0;\n> +\tint no_update_server_info = 0;\n> +\tint quiet = 0;\n> +\tint local = 0;\n> +\tchar *packdir, *packtmp;\n> +\tconst char *cmd_args[20];\n> +\tint cmd_i = 0;\n> +\tstruct child_process cmd;\n> +\tstruct string_list_item *item;\n> +\tstruct string_list existing_packs = STRING_LIST_INIT_DUP;\n> +\tstruct stat statbuffer;\n> +\tchar window_str[64], window_mem_str[64], depth_str[64], max_pack_size_str[64];\n> +\n> +\tstruct option builtin_repack_options[] = {\n> +\t\tOPT_BOOL('a', \"all\", &pack_everything,\n> +\t\t\t\tN_(\"pack everything in a single pack\")),\n> +\t\tOPT_BOOL('A', \"all-but-loose\", &pack_everything_but_loose,\n> +\t\t\t\tN_(\"same as -a, and turn unreachable objects loose\")),\n> +\t\tOPT_BOOL('d', \"delete-redundant\", &delete_redundant,\n> +\t\t\t\tN_(\"remove redundant packs, and run git-prune-packed\")),\n> +\t\tOPT_BOOL('f', \"no-reuse-delta\", &no_reuse_delta,\n> +\t\t\t\tN_(\"pass --no-reuse-delta to git-pack-objects\")),\n> +\t\tOPT_BOOL('F', \"no-reuse-object\", &no_reuse_object,\n> +\t\t\t\tN_(\"pass --no-reuse-object to git-pack-objects\")),\n> +\t\tOPT_BOOL('n', NULL, &no_update_server_info,\n> +\t\t\t\tN_(\"do not run git-update-server-info\")),\n> +\t\tOPT__QUIET(&quiet, N_(\"be quiet\")),\n> +\t\tOPT_BOOL('l', \"local\", &local,\n> +\t\t\t\tN_(\"pass --local to git-pack-objects\")),\n> +\t\tOPT_DATE(0, \"unpack-unreachable\", &unpack_unreachable,\n> +\t\t\t\tN_(\"with -A, do not loosen objects older than this Packing constraints\")),\n> +\t\tOPT_INTEGER(0, \"window\", &window,\n> +\t\t\t\tN_(\"size of the window used for delta compression\")),\n> +\t\tOPT_INTEGER(0, \"window-memory\", &window_memory,\n> +\t\t\t\tN_(\"same as the above, but limit memory size instead of entries count\")),\n> +\t\tOPT_INTEGER(0, \"depth\", &depth,\n> +\t\t\t\tN_(\"limits the maximum delta depth\")),\n> +\t\tOPT_INTEGER(0, \"max-pack-size\", &max_pack_size,\n> +\t\t\t\tN_(\"maximum size of each packfile\")),\n> +\t\tOPT_END()\n> +\t};\n> +\n> +\tgit_config(repack_config, NULL);\n> +\n> +\targc = parse_options(argc, argv, prefix, builtin_repack_options,\n> +\t\t\t\tgit_repack_usage, 0);\n> +\n> +\tsigchain_push_common(remove_pack_on_signal);\n> +\n> +\tpackdir = mkpath(\"%s/pack\", get_object_directory());\n\nAh, mkpath is already used. That function is a bit tricky because you \ncan only use it with four paths concurrently and some internal functions \nmight already need one (or more) of the slots for itself. It's better \nto consume its output on the spot (like in printf(\"my path is %s\\n\", \nmkpath(...));) or use mkpathdup.\n\n> +\tpacktmp = xmalloc(strlen(packdir) + strlen(\"/.tmp-10000-pack\") + 1);\n> +\tsprintf(packtmp, \"%s/.tmp-%d-pack\", packdir, getpid());\n\nmkpathdup again..\n\n> +\n> +\tremove_temporary_files();\n> +\n> +\tcmd_args[cmd_i++] = \"pack-objects\";\n> +\tcmd_args[cmd_i++] = \"--keep-true-parents\";\n> +\tcmd_args[cmd_i++] = \"--honor-pack-keep\";\n> +\tcmd_args[cmd_i++] = \"--non-empty\";\n> +\tcmd_args[cmd_i++] = \"--all\";\n> +\tcmd_args[cmd_i++] = \"--reflog\";\n> +\n> +\tif (window) {\n> +\t\tsprintf(window_str, \"--window=%u\", window);\n> +\t\tcmd_args[cmd_i++] = window_str;\n> +\t}\n> +\tif (window_memory) {\n> +\t\tsprintf(window_mem_str, \"--window-memory=%u\", window_memory);\n> +\t\tcmd_args[cmd_i++] = window_str;\n> +\t}\n> +\tif (depth) {\n> +\t\tsprintf(depth_str, \"--depth=%u\", depth);\n> +\t\tcmd_args[cmd_i++] = depth_str;\n> +\t}\n> +\tif (max_pack_size) {\n> +\t\tsprintf(max_pack_size_str, \"--max_pack_size=%u\", max_pack_size);\n> +\t\tcmd_args[cmd_i++] = max_pack_size_str;\n> +\t}\n\n\nYou can simplify that part by using argv_array and argv_array_pushf.\n\n> +\n> +\tif (pack_everything + pack_everything_but_loose == 0) {\n> +\t\tcmd_args[cmd_i++] = \"--unpacked\";\n> +\t\tcmd_args[cmd_i++] = \"--incremental\";\n> +\t} else {\n> +\t\tif (pack_everything_but_loose)\n> +\t\t\tcmd_args[cmd_i++] = \"--unpack-unreachable\";\n> +\n> +\t\tstruct string_list sha1_list = STRING_LIST_INIT_DUP;\n> +\t\tget_pack_sha1_list(packdir, &sha1_list);\n> +\t\tfor_each_string_list_item(item, &sha1_list) {\n> +\t\t\tchar *fname;\n> +\t\t\tfname = xmalloc(strlen(packdir) + strlen(\"/\") + 40 + strlen(\".keep\"));\n> +\t\t\tsprintf(fname, \"%s/%s.keep\", packdir, item->string);\n\nmkpathdup..\n\nOr maybe go through the entries in the pack directory once, like already \ndone in get_pack_sha1_list, and instead of just making a list of .pack \nfiles, make a list of .keep files as well. Then work with those lists \ninstead of accessing the directory again with stat.\n\n> +\t\t\tif (stat(fname, &statbuffer) && S_ISREG(statbuffer.st_mode)) {\n> +\t\t\t\t/* when the keep file is there, we're ignoring that pack */\n> +\t\t\t} else {\n> +\t\t\t\tstring_list_append(&existing_packs, item->string);\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\tif (existing_packs.nr && unpack_unreachable && delete_redundant) {\n> +\t\t\t/*\n> +\t\t\t * TODO: convert unpack_unreachable (being time since epoch)\n> +\t\t\t * to an aproxidate again\n> +\t\t\t */\n> +\t\t\tcmd_args[cmd_i++] = \"--unpack-unreachable=$DATE\";\n> +\t\t}\n> +\t}\n> +\n> +\tif (local)\n> +\t\tcmd_args[cmd_i++] = \"--local\";\n> +\n> +\tif (delta_base_offset)\n> +\t\tcmd_args[cmd_i++] = \"--delta-base-offset\";\n> +\n> +\tcmd_args[cmd_i++] = packtmp;\n> +\tcmd_args[cmd_i] = NULL;\n> +\n> +\tmemset(&cmd, 0, sizeof(cmd));\n> +\tcmd.argv = cmd_args;\n> +\tcmd.git_cmd = 1;\n> +\tcmd.out = -1;\n> +\tcmd.no_stdin = 1;\n> +\n> +\tif (run_command(&cmd))\n> +\t\treturn 1;\n> +\n> +\tstruct string_list names = STRING_LIST_INIT_DUP;\n> +\tstruct string_list rollback = STRING_LIST_INIT_DUP;\n> +\n> +\tchar line[1024];\n> +\tint counter = 0;\n> +\tFILE *out = xfdopen(cmd.out, \"r\");\n> +\twhile (fgets(line, sizeof(line), out)) {\n> +\t\t/* a line consists of 40 hex chars + '\\n' */\n> +\t\tassert(strlen(line) == 41);\n> +\t\tline[40] = '\\0';\n> +\t\tstring_list_append(&names, line);\n> +\t\tcounter++;\n> +\t}\n> +\tif (!counter)\n> +\t\tprintf(\"Nothing new to pack.\\n\");\n> +\tfclose(out);\n> +\n> +\tchar *fname, *fname_old;\n> +\tfname = xmalloc(strlen(packdir) + strlen(\"/old-pack-\") + 40 + strlen(\".pack\") + 1);\n> +\tstrcpy(fname, packdir);\n> +\tstrcpy(fname + strlen(packdir), \"/\");\n> +\n> +\tfname_old = xmalloc(strlen(packdir) + strlen(\"/old-pack-\") + 40 + strlen(\".pack\") + 1);\n> +\tstrcpy(fname_old, packdir);\n> +\tstrcpy(fname_old + strlen(packdir), \"/\");\n> +\tchar *exts[2] = {\".idx\", \".pack\"};\n> +\n> +\tint failed = 0;\n> +\n> +\tfor_each_string_list_item(item, &names) {\n> +\t\tint ext;\n> +\t\tfor (ext = 0; ext < 1; ext++) {\n> +\t\t\tstrcpy(fname + strlen(packdir) + 1, item->string);\n> +\t\t\tstrcpy(fname + strlen(packdir) + 41, exts[ext]);\n> +\t\t\tif (!file_exists(fname))\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\tstrcpy(fname_old, packdir);\n> +\t\t\tstrcpy(fname_old + strlen(packdir), \"/old-\");\n> +\t\t\tstrcpy(fname_old + strlen(packdir) + 5, item->string);\n> +\t\t\tstrcpy(fname_old + strlen(packdir) + 45, exts[ext]);\n> +\t\t\tif (file_exists(fname_old))\n> +\t\t\t\tunlink(fname_old);\n> +\n> +\t\t\tif (rename(fname, fname_old)) {\n> +\t\t\t\tfailed = 1;\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t\tstring_list_append(&rollback, fname);\n\nmkpathdup with string_list_append_nodup instead?\n\n> +\t\t}\n> +\t\tif (failed)\n> +\t\t\t/* set to last element to break while loop */\n> +\t\t\titem = names.items + names.nr;\n> +\t}\n> +\tif (failed) {\n> +\t\tstruct string_list rollback_failure;\n> +\n> +\t\tfor_each_string_list_item(item, &rollback) {\n> +\t\t\tsprintf(fname, \"%s/%s\", packdir, item->string);\n> +\t\t\tsprintf(fname_old, \"%s/old-%s\", packdir, item->string);\n> +\t\t\tif (rename(fname_old, fname))\n> +\t\t\t\tstring_list_append(&rollback_failure, fname);\n\nDito.\n\n> +\t\t}\n> +\n> +\t\tif (rollback.nr) {\n> +\t\t\tint i;\n> +\t\t\tfprintf(stderr,\n> +\t\t\t\t\"WARNING: Some packs in use have been renamed by\\n\"\n> +\t\t\t\t\"WARNING: prefixing old- to their name, in order to\\n\"\n> +\t\t\t\t\"WARNING: replace them with the new version of the\\n\"\n> +\t\t\t\t\"WARNING: file. But the operation failed, and\\n\"\n> +\t\t\t\t\"WARNING: attempt to rename them back to their\\n\"\n> +\t\t\t\t\"WARNING: original names also failed.\\n\"\n> +\t\t\t\t\"WARNING: Please rename them in $PACKDIR manually:\\n\");\n> +\t\t\tfor (i = 0; i < rollback.nr; i++)\n> +\t\t\t\tfprintf(stderr, \"WARNING: old-%s -> %s\\n\",\n> +\t\t\t\t\trollback.items[i].string,\n> +\t\t\t\t\trollback.items[i].string);\n> +\t\t}\n> +\t\texit(1);\n> +\t}\n> +\n> +\t/* Now the ones with the same name are out of the way... */\n> +\tstruct string_list fullbases = STRING_LIST_INIT_DUP;\n> +\tfor_each_string_list_item(item, &names) {\n> +\t\tstring_list_append(&fullbases, item->string);\n\nWhy make a copy of 'names'? Can't you use it directly instead of \n'fullbases'? Ah, the Shell version adds a \"pack-\" at the beginning of \neach string. We don't need to do that and thus can get rid of that \nextra list, no?\n\n> +\n> +\t\tsprintf(fname, \"%s/pack-%s.pack\", packdir, item->string);\n> +\t\tsprintf(fname_old, \"%s-%s.pack\", packtmp, item->string);\n> +\t\tstat(fname_old, &statbuffer);\n> +\t\tstatbuffer.st_mode &= ~S_IWOTH;\n> +\t\tchmod(fname_old, statbuffer.st_mode);\n> +\t\tif (rename(fname_old, fname))\n> +\t\t\tdie(\"Could not rename packfile: %s -> %s\", fname_old, fname);\n> +\n> +\t\tsprintf(fname, \"%s/pack-%s.idx\", packdir, item->string);\n> +\t\tsprintf(fname_old, \"%s-%s.idx\", packtmp, item->string);\n> +\t\tstat(fname_old, &statbuffer);\n> +\t\tstatbuffer.st_mode &= ~S_IWOTH;\n> +\t\tchmod(fname_old, statbuffer.st_mode);\n> +\t\tif (rename(fname_old, fname))\n> +\t\t\tdie(\"Could not rename packfile: %s -> %s\", fname_old, fname);\n> +\t}\n> +\n> +\t/* Remove the \"old-\" files */\n> +\tfor_each_string_list_item(item, &names) {\n> +\t\tsprintf(fname, \"%s/old-pack-%s.idx\", packdir, item->string);\n> +\t\tif (remove_path(fname))\n> +\t\t\tdie(\"Could not remove file: %s\", fname);\n> +\n> +\t\tsprintf(fname, \"%s/old-pack-%s.pack\", packdir, item->string);\n> +\t\tif (remove_path(fname))\n> +\t\t\tdie(\"Could not remove file: %s\", fname);\n> +\t}\n> +\n> +\t/* End of pack replacement. */\n> +\tif (delete_redundant) {\n> +\t\tsort_string_list(&fullbases);\n> +\t\tfname = xmalloc(strlen(packtmp) + strlen(\"/\") + 40 + strlen(\".pack\") + 1);\n> +\t\tfor_each_string_list_item(item, &existing_packs) {\n> +\t\t\tif (!string_list_has_string(&fullbases, item->string))\n> +\t\t\t\tremove_pack(packdir, item->string);\n> +\t\t}\n> +\t\tfree(fname);\n\nWhy allocate memory for fname and give it back unused?\n\n> +\t\tcmd_i = 0;\n> +\t\tcmd_args[cmd_i++] = \"prune-packed\";\n> +\t\tcmd_args[cmd_i++] = NULL;\n> +\t\t/* TODO: pass argument: ${GIT_QUIET:+-q} */\n> +\t\tmemset(&cmd, 0, sizeof(cmd));\n> +\t\tcmd.argv = cmd_args;\n> +\t\tcmd.git_cmd = 1;\n> +\t\trun_command(&cmd);\n> +\t}\n> +\n> +\tif (!no_update_server_info) {\n> +\t\tcmd_i = 0;\n> +\t\tcmd_args[cmd_i++] = \"update-server-info\";\n> +\t\tcmd_args[cmd_i++] = NULL;\n> +\n> +\t\tmemset(&cmd, 0, sizeof(cmd));\n> +\t\tcmd.argv = cmd_args;\n> +\t\tcmd.git_cmd = 1;\n> +\t\trun_command(&cmd);\n> +\t}\n> +\treturn 0;\n> +}\n> +\n> diff --git a/git-repack.sh b/contrib/examples/git-repack.sh\n> similarity index 100%\n> rename from git-repack.sh\n> rename to contrib/examples/git-repack.sh\n> diff --git a/git.c b/git.c\n> index 2025f77..03510be 100644\n> --- a/git.c\n> +++ b/git.c\n> @@ -396,6 +396,7 @@ static void handle_internal_command(int argc, const char **argv)\n> \t\t{ \"remote\", cmd_remote, RUN_SETUP },\n> \t\t{ \"remote-ext\", cmd_remote_ext },\n> \t\t{ \"remote-fd\", cmd_remote_fd },\n> +\t\t{ \"repack\", cmd_repack, RUN_SETUP },\n> \t\t{ \"replace\", cmd_replace, RUN_SETUP },\n> \t\t{ \"repo-config\", cmd_repo_config, RUN_SETUP_GENTLY },\n> \t\t{ \"rerere\", cmd_rerere, RUN_SETUP },\n> -- 1.8.4.rc3.1.gc1ebd90\n>\n"},{"id":"225382","messageId":"C3ED0A6F-4C7E-4433-966C-6EE00B9AAFEE@gmail.com","threadId":"34687","inReplyTo":"520F7BDE.9090902@web.de","subject":"Re: [RFC PATCHv2] repack: rewrite the shell script in C.","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2013-08-17T19:18:24Z","receivedAt":"2013-08-17T19:18:24Z","isPatch":false,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"On Aug 17, 2013, at 06:34, René Scharfe wrote:\n> On Aug 15, 2013, at 17:12, Stefan Beller wrote:\n>> +\t\tif (sha_begin >= e->d_name && !strncmp(sha_begin, sha1, 40)) {\n>> +\t\t\tchar *fname;\n>> +\t\t\tfname = xmalloc(strlen(path) + 1 + strlen(e->d_name));\n\nThis needs another +1 because\n\n>> +\t\t\tsprintf(fname, \"%s/%s\", path, e->d_name);\n\nlen(path) + len(\"/\") + len(e->d_name) + len(\"\\0\")\n\n>\n> mkpathdup..\n>\n>> +\t\t\tunlink(fname);\n>> +\t\t\t/*TODO: free(fname); fails here sometimes, needs investigation*/\n>\n> Strange. Perhaps valgrind can tell you what's wrong.\n\nwhich is probably why it fails since the byte beyond the end of fname \nis being overwritten with a nul."},{"id":"225391","messageId":"5210DB77.3090004@googlemail.com","threadId":"34687","inReplyTo":"520F7BDE.9090902@web.de","subject":"Re: [RFC PATCHv2] repack: rewrite the shell script in C.","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-08-18T14:34:31Z","receivedAt":"2013-08-18T14:34:31Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 08/17/2013 03:34 PM, René Scharfe wrote:\n> \n> Hmm, stepping back a bit, why not just build the paths and call unlink\n> for them right away, without readdir? The shell version only ever\n> deletes existing .pack files (those in $existing alias existing_packs)\n> as well as their .idx and .keep files, if present. It doesn't use a\n> glob pattern, unlike remove_pack here.\n\nI'll meditate on that. \n\nThanks for all the other remarks. Now the code looks much more\ngit-ish, similar to other commands. \nThe lines of code went down from 411 to 385, I guess we can cut off\nmore inefficiencies there. \n\nAs you suggested, maybe we should juts have one helper function to\nread in the pack directory and keeping all the information (complete filename),\nso we do not need to find the exact filename later again by looping over\nthe directory again.\n\nStefan\n"},{"id":"225392","messageId":"1376836587-25248-1-git-send-email-stefanbeller@googlemail.com","threadId":"34687","inReplyTo":"5210DB77.3090004@googlemail.com","subject":"[RFC PATCHv3] repack: rewrite the shell script in C.","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-08-18T14:36:27Z","receivedAt":"2013-08-18T14:36:27Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This is the beginning of the rewrite of the repacking.\n\n * replace all plain string handling functions\n by git helper functions, most often mkpathdup\n * use argv-array structs to pass arguments to\n other git invocations.\n\nOnly test t7701 (2nd) fails now with this patch.\n\nSigned-off-by: Stefan Beller \n---\n Makefile | 2 +-\n builtin.h | 1 +\n builtin/repack.c | 385 ++++++++++++++++++++++++\n git-repack.sh => contrib/examples/git-repack.sh | 0\n git.c | 1 +\n 5 files changed, 388 insertions(+), 1 deletion(-)\n create mode 100644 builtin/repack.c\n rename git-repack.sh => contrib/examples/git-repack.sh (100%)\n\ndiff --git a/Makefile b/Makefile\nindex ef442eb..4ec5bbe 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -464,7 +464,6 @@ SCRIPT_SH += git-pull.sh\n SCRIPT_SH += git-quiltimport.sh\n SCRIPT_SH += git-rebase.sh\n SCRIPT_SH += git-remote-testgit.sh\n-SCRIPT_SH += git-repack.sh\n SCRIPT_SH += git-request-pull.sh\n SCRIPT_SH += git-stash.sh\n SCRIPT_SH += git-submodule.sh\n@@ -972,6 +971,7 @@ BUILTIN_OBJS += builtin/reflog.o\n BUILTIN_OBJS += builtin/remote.o\n BUILTIN_OBJS += builtin/remote-ext.o\n BUILTIN_OBJS += builtin/remote-fd.o\n+BUILTIN_OBJS += builtin/repack.o\n BUILTIN_OBJS += builtin/replace.o\n BUILTIN_OBJS += builtin/rerere.o\n BUILTIN_OBJS += builtin/reset.o\ndiff --git a/builtin.h b/builtin.h\nindex 8afa2de..b56cf07 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -102,6 +102,7 @@ extern int cmd_reflog(int argc, const char **argv, const char *prefix);\n extern int cmd_remote(int argc, const char **argv, const char *prefix);\n extern int cmd_remote_ext(int argc, const char **argv, const char *prefix);\n extern int cmd_remote_fd(int argc, const char **argv, const char *prefix);\n+extern int cmd_repack(int argc, const char **argv, const char *prefix);\n extern int cmd_repo_config(int argc, const char **argv, const char *prefix);\n extern int cmd_rerere(int argc, const char **argv, const char *prefix);\n extern int cmd_reset(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/repack.c b/builtin/repack.c\nnew file mode 100644\nindex 0000000..190eb5f\n--- /dev/null\n+++ b/builtin/repack.c\n@@ -0,0 +1,385 @@\n+/*\n+ * The shell version was written by Linus Torvalds (2005) and many others.\n+ * This is a translation into C by Stefan Beller (2013)\n+ */\n+\n+#include \"builtin.h\"\n+#include \"cache.h\"\n+#include \"dir.h\"\n+#include \"parse-options.h\"\n+#include \"run-command.h\"\n+#include \"sigchain.h\"\n+#include \"strbuf.h\"\n+#include \"string-list.h\"\n+\n+#include \"argv-array.h\"\n+\n+static const char *const git_repack_usage[] = {\n+\tN_(\"git repack [options]\"),\n+\tNULL\n+};\n+\n+/* enabled by default since 22c79eab (2008-06-25) */\n+static int delta_base_offset = 1;\n+\n+static int repack_config(const char *var, const char *value, void *cb)\n+{\n+\tif (!strcmp(var, \"repack.usedeltabaseoffset\")) {\n+\t\tdelta_base_offset = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\treturn git_default_config(var, value, cb);\n+}\n+\n+static void remove_temporary_files() {\n+\tDIR *dir;\n+\tstruct dirent *e;\n+\tchar *prefix, *path;\n+\n+\tprefix = mkpathdup(\".tmp-%d-pack\", getpid());\n+\tpath = mkpathdup(\"%s/pack\", get_object_directory());\n+\n+\tdir = opendir(path);\n+\twhile ((e = readdir(dir)) != NULL) {\n+\t\tif (!prefixcmp(e->d_name, prefix)) {\n+\t\t\tstruct strbuf fname = STRBUF_INIT;\n+\t\t\tstrbuf_addf(&fname, \"%s/%s\", path, e->d_name);\n+\t\t\tunlink(strbuf_detach(&fname, NULL));\n+\t\t}\n+\t}\n+\tfree(prefix);\n+\tfree(path);\n+\tclosedir(dir);\n+}\n+\n+static void remove_pack_on_signal(int signo)\n+{\n+\tremove_temporary_files();\n+\tsigchain_pop(signo);\n+\traise(signo);\n+}\n+\n+void get_pack_sha1_list(char *packdir, struct string_list *sha1_list)\n+{\n+\tDIR *dir;\n+\tstruct dirent *e;\n+\tchar *path, *suffix;\n+\n+\tpath = mkpathdup(\"%s/pack\", get_object_directory());\n+\tsuffix = \".pack\";\n+\n+\tdir = opendir(path);\n+\twhile ((e = readdir(dir)) != NULL) {\n+\t\tif (!suffixcmp(e->d_name, suffix)) {\n+\t\t\tchar *buf, *sha1;\n+\t\t\tbuf = xmemdupz(e->d_name, strlen(e->d_name));\n+\t\t\tbuf[strlen(e->d_name) - strlen(suffix)] = '\\0';\n+\t\t\tif (strlen(e->d_name) - strlen(suffix) > 40) {\n+\t\t\t\tsha1 = &buf[strlen(e->d_name) - strlen(suffix) - 40];\n+\t\t\t\tstring_list_append_nodup(sha1_list, sha1);\n+\t\t\t} else {\n+\t\t\t\t/*TODO: what should happen to pack files having no 40 char sha1 specifier?*/\n+\t\t\t}\n+\t\t}\n+\t}\n+\tfree(path);\n+\tclosedir(dir);\n+}\n+\n+/*\n+ * remove_pack will remove any files following the pattern *${SHA1}.{EXT}\n+ * where EXT is one of {pack, idx, keep}. The SHA1 consists of 40 chars and\n+ * is specified by the sha1 parameter.\n+ * path is specifying the directory in which all found files will be deleted.\n+ */\n+void remove_pack(char *path, char* sha1)\n+{\n+\tDIR *dir;\n+\tstruct dirent *e;\n+\n+\tdir = opendir(path);\n+\twhile ((e = readdir(dir)) != NULL) {\n+\t\tchar *sha_begin, *sha_end;\n+\t\tsha_end = e->d_name + strlen(e->d_name);\n+\t\twhile (sha_end > e->d_name && *sha_end != '.')\n+\t\t\tsha_end--;\n+\n+\t\t/* do not touch files not ending in .pack, .idx or .keep */\n+\t\tif (strcmp(sha_end, \".pack\") &&\n+\t\t\tstrcmp(sha_end, \".idx\") &&\n+\t\t\tstrcmp(sha_end, \".keep\"))\n+\t\t\tcontinue;\n+\n+\t\tsha_begin = sha_end - 40;\n+\n+\t\tif (sha_begin >= e->d_name && !strncmp(sha_begin, sha1, 40)) {\n+\t\t\tchar *fname;\n+\t\t\tfname = mkpathdup(\"%s/%s\", path, e->d_name);\n+\t\t\tunlink(fname);\n+\t\t\tfree(fname);\n+\t\t}\n+\t}\n+\tclosedir(dir);\n+}\n+\n+int cmd_repack(int argc, const char **argv, const char *prefix) {\n+\n+\tint pack_everything = 0;\n+\tint pack_everything_but_loose = 0;\n+\tint delete_redundant = 0;\n+\tchar *unpack_unreachable = NULL;\n+\tint window = 0, window_memory = 0;\n+\tint depth = 0;\n+\tint max_pack_size = 0;\n+\tint no_reuse_delta = 0, no_reuse_object = 0;\n+\tint no_update_server_info = 0;\n+\tint quiet = 0;\n+\tint local = 0;\n+\tchar *packdir, *packtmp;\n+\tstruct child_process cmd;\n+\tstruct string_list_item *item;\n+\tstruct string_list existing_packs = STRING_LIST_INIT_DUP;\n+\tstruct stat statbuffer;\n+\n+\tstruct option builtin_repack_options[] = {\n+\t\tOPT_BOOL('a', \"all\", &pack_everything,\n+\t\t\t\tN_(\"pack everything in a single pack\")),\n+\t\tOPT_BOOL('A', \"all-but-loose\", &pack_everything_but_loose,\n+\t\t\t\tN_(\"same as -a, and turn unreachable objects loose\")),\n+\t\tOPT_BOOL('d', \"delete-redundant\", &delete_redundant,\n+\t\t\t\tN_(\"remove redundant packs, and run git-prune-packed\")),\n+\t\tOPT_BOOL('f', \"no-reuse-delta\", &no_reuse_delta,\n+\t\t\t\tN_(\"pass --no-reuse-delta to git-pack-objects\")),\n+\t\tOPT_BOOL('F', \"no-reuse-object\", &no_reuse_object,\n+\t\t\t\tN_(\"pass --no-reuse-object to git-pack-objects\")),\n+\t\tOPT_BOOL('n', NULL, &no_update_server_info,\n+\t\t\t\tN_(\"do not run git-update-server-info\")),\n+\t\tOPT__QUIET(&quiet, N_(\"be quiet\")),\n+\t\tOPT_BOOL('l', \"local\", &local,\n+\t\t\t\tN_(\"pass --local to git-pack-objects\")),\n+\t\tOPT_STRING(0, \"unpack-unreachable\", &unpack_unreachable, N_(\"approxidate\"),\n+\t\t\t\tN_(\"with -A, do not loosen objects older than this Packing constraints\")),\n+\t\tOPT_INTEGER(0, \"window\", &window,\n+\t\t\t\tN_(\"size of the window used for delta compression\")),\n+\t\tOPT_INTEGER(0, \"window-memory\", &window_memory,\n+\t\t\t\tN_(\"same as the above, but limit memory size instead of entries count\")),\n+\t\tOPT_INTEGER(0, \"depth\", &depth,\n+\t\t\t\tN_(\"limits the maximum delta depth\")),\n+\t\tOPT_INTEGER(0, \"max-pack-size\", &max_pack_size,\n+\t\t\t\tN_(\"maximum size of each packfile\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tgit_config(repack_config, NULL);\n+\n+\targc = parse_options(argc, argv, prefix, builtin_repack_options,\n+\t\t\t\tgit_repack_usage, 0);\n+\n+\tsigchain_push_common(remove_pack_on_signal);\n+\n+\tpackdir = mkpathdup(\"%s/pack\", get_object_directory());\n+\tpacktmp = mkpathdup(\"%s/.tmp-%d-pack\", packdir, getpid());\n+\n+\tremove_temporary_files();\n+\n+\tstruct argv_array cmd_args = ARGV_ARRAY_INIT;\n+\targv_array_push(&cmd_args, \"pack-objects\");\n+\targv_array_push(&cmd_args, \"--keep-true-parents\");\n+\targv_array_push(&cmd_args, \"--honor-pack-keep\");\n+\targv_array_push(&cmd_args, \"--non-empty\");\n+\targv_array_push(&cmd_args, \"--all\");\n+\targv_array_push(&cmd_args, \"--reflog\");\n+\n+\tif (window)\n+\t\targv_array_pushf(&cmd_args, \"--window=%u\", window);\n+\n+\tif (window_memory)\n+\t\targv_array_pushf(&cmd_args, \"--window-memory=%u\", window_memory);\n+\n+\tif (depth)\n+\t\targv_array_pushf(&cmd_args, \"--depth=%u\", depth);\n+\n+\tif (max_pack_size)\n+\t\targv_array_pushf(&cmd_args, \"--max_pack_size=%u\", max_pack_size);\n+\n+\tif (pack_everything + pack_everything_but_loose == 0) {\n+\t\targv_array_push(&cmd_args, \"--unpacked\");\n+\t\targv_array_push(&cmd_args, \"--incremental\");\n+\t} else {\n+\t\tif (pack_everything_but_loose)\n+\t\t\targv_array_push(&cmd_args, \"--unpack-unreachable\");\n+\n+\t\tstruct string_list sha1_list = STRING_LIST_INIT_DUP;\n+\t\tget_pack_sha1_list(packdir, &sha1_list);\n+\t\tfor_each_string_list_item(item, &sha1_list) {\n+\t\t\tchar *fname;\n+\t\t\tfname = mkpathdup(\"%s/%s.keep\", packdir, item->string);\n+\t\t\tif (stat(fname, &statbuffer) && S_ISREG(statbuffer.st_mode)) {\n+\t\t\t\t/* when the keep file is there, we're ignoring that pack */\n+\t\t\t} else {\n+\t\t\t\tstring_list_append(&existing_packs, item->string);\n+\t\t\t}\n+\t\t\tfree(fname);\n+\t\t}\n+\n+\t\tif (existing_packs.nr && unpack_unreachable && delete_redundant) {\n+\t\t\targv_array_pushf(&cmd_args, \"--unpack-unreachable=%s\", unpack_unreachable);\n+\t\t}\n+\t}\n+\n+\tif (local)\n+\t\targv_array_push(&cmd_args, \"--local\");\n+\n+\tif (delta_base_offset)\n+\t\targv_array_push(&cmd_args, \"--delta-base-offset\");\n+\n+\targv_array_push(&cmd_args, packtmp);\n+\n+\tmemset(&cmd, 0, sizeof(cmd));\n+\tcmd.argv = argv_array_detach(&cmd_args, NULL);\n+\tcmd.git_cmd = 1;\n+\tcmd.out = -1;\n+\tcmd.no_stdin = 1;\n+\n+\tif (run_command(&cmd))\n+\t\treturn 1;\n+\n+\tstruct string_list names = STRING_LIST_INIT_DUP;\n+\tstruct string_list rollback = STRING_LIST_INIT_DUP;\n+\n+\tchar line[1024];\n+\tint counter = 0;\n+\tFILE *out = xfdopen(cmd.out, \"r\");\n+\twhile (fgets(line, sizeof(line), out)) {\n+\t\t/* a line consists of 40 hex chars + '\\n' */\n+\t\tassert(strlen(line) == 41);\n+\t\tline[40] = '\\0';\n+\t\tstring_list_append(&names, line);\n+\t\tcounter++;\n+\t}\n+\tif (!counter)\n+\t\tprintf(\"Nothing new to pack.\\n\");\n+\tfclose(out);\n+\n+\tchar *exts[2] = {\".idx\", \".pack\"};\n+\tint failed = 0;\n+\tfor_each_string_list_item(item, &names) {\n+\t\tint ext;\n+\t\tfor (ext = 0; ext < 1; ext++) {\n+\t\t\tchar *fname, *fname_old;\n+\t\t\tfname = mkpathdup(\"%s/%s%s\", packdir, item->string, exts[ext]);\n+\t\t\tif (!file_exists(fname)) {\n+\t\t\t\tfree(fname);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n+\t\t\tfname_old = mkpathdup(\"%s/old-%s%s\", packdir, item->string, exts[ext]);\n+\t\t\tif (file_exists(fname_old))\n+\t\t\t\tunlink(fname_old);\n+\n+\t\t\tif (rename(fname, fname_old)) {\n+\t\t\t\tfailed = 1;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tstring_list_append_nodup(&rollback, fname);\n+\t\t}\n+\t\tif (failed)\n+\t\t\t/* set to last element to break for_each loop */\n+\t\t\titem = names.items + names.nr;\n+\t}\n+\tif (failed) {\n+\t\tstruct string_list rollback_failure;\n+\t\tfor_each_string_list_item(item, &rollback) {\n+\t\t\tchar *fname, *fname_old;\n+\t\t\tfname = mkpathdup(\"%s/%s\", packdir, item->string);\n+\t\t\tfname_old = mkpathdup(\"%s/old-%s\", packdir, item->string);\n+\t\t\tif (rename(fname_old, fname))\n+\t\t\t\tstring_list_append(&rollback_failure, fname);\n+\t\t\tfree(fname);\n+\t\t\tfree(fname_old);\n+\t\t}\n+\n+\t\tif (rollback.nr) {\n+\t\t\tint i;\n+\t\t\tfprintf(stderr,\n+\t\t\t\t\"WARNING: Some packs in use have been renamed by\\n\"\n+\t\t\t\t\"WARNING: prefixing old- to their name, in order to\\n\"\n+\t\t\t\t\"WARNING: replace them with the new version of the\\n\"\n+\t\t\t\t\"WARNING: file. But the operation failed, and\\n\"\n+\t\t\t\t\"WARNING: attempt to rename them back to their\\n\"\n+\t\t\t\t\"WARNING: original names also failed.\\n\"\n+\t\t\t\t\"WARNING: Please rename them in $PACKDIR manually:\\n\");\n+\t\t\tfor (i = 0; i < rollback.nr; i++)\n+\t\t\t\tfprintf(stderr, \"WARNING: old-%s -> %s\\n\",\n+\t\t\t\t\trollback.items[i].string,\n+\t\t\t\t\trollback.items[i].string);\n+\t\t}\n+\t\texit(1);\n+\t}\n+\n+\t/* Now the ones with the same name are out of the way... */\n+\tfor_each_string_list_item(item, &names) {\n+\t\tchar *fname, *fname_old;\n+\t\tfname = mkpathdup(\"%s/pack-%s.pack\", packdir, item->string);\n+\t\tfname_old = mkpathdup(\"%s-%s.pack\", packtmp, item->string);\n+\t\tstat(fname_old, &statbuffer);\n+\t\tstatbuffer.st_mode &= ~S_IWOTH;\n+\t\tchmod(fname_old, statbuffer.st_mode);\n+\t\tif (rename(fname_old, fname))\n+\t\t\tdie(\"Could not rename packfile: %s -> %s\", fname_old, fname);\n+\t\tfree(fname);\n+\t\tfree(fname_old);\n+\n+\t\tfname = mkpathdup(\"%s/pack-%s.idx\", packdir, item->string);\n+\t\tfname_old = mkpathdup(\"%s-%s.idx\", packtmp, item->string);\n+\t\tstat(fname_old, &statbuffer);\n+\t\tstatbuffer.st_mode &= ~S_IWOTH;\n+\t\tchmod(fname_old, statbuffer.st_mode);\n+\t\tif (rename(fname_old, fname))\n+\t\t\tdie(\"Could not rename packfile: %s -> %s\", fname_old, fname);\n+\t\tfree(fname);\n+\t\tfree(fname_old);\n+\t}\n+\n+\t/* Remove the \"old-\" files */\n+\tfor_each_string_list_item(item, &names) {\n+\t\tchar *fname;\n+\t\tfname = mkpathdup(\"%s/old-pack-%s.idx\", packdir, item->string);\n+\t\tif (remove_path(fname))\n+\t\t\tdie(\"Could not remove file: %s\", fname);\n+\t\tfree(fname);\n+\n+\t\tfname = mkpathdup(\"%s/old-pack-%s.pack\", packdir, item->string);\n+\t\tif (remove_path(fname))\n+\t\t\tdie(\"Could not remove file: %s\", fname);\n+\t\tfree(fname);\n+\t}\n+\n+\t/* End of pack replacement. */\n+\tif (delete_redundant) {\n+\t\tsort_string_list(&names);\n+\t\tfor_each_string_list_item(item, &existing_packs) {\n+\t\t\tif (!string_list_has_string(&names, item->string))\n+\t\t\t\tremove_pack(packdir, item->string);\n+\t\t}\n+\t\targv_array_clear(&cmd_args);\n+\t\targv_array_push(&cmd_args, \"prune-packed\");\n+\t\t/* TODO: pass argument: ${GIT_QUIET:+-q} */\n+\t\tmemset(&cmd, 0, sizeof(cmd));\n+\t\tcmd.argv = argv_array_detach(&cmd_args, NULL);\n+\t\tcmd.git_cmd = 1;\n+\t\trun_command(&cmd);\n+\t}\n+\n+\tif (!no_update_server_info) {\n+\t\targv_array_clear(&cmd_args);\n+\t\targv_array_push(&cmd_args, \"update-server-info\");\n+\n+\t\tmemset(&cmd, 0, sizeof(cmd));\n+\t\tcmd.argv = argv_array_detach(&cmd_args, NULL);\n+\t\tcmd.git_cmd = 1;\n+\t\trun_command(&cmd);\n+\t}\n+\treturn 0;\n+}\n+\ndiff --git a/git-repack.sh b/contrib/examples/git-repack.sh\nsimilarity index 100%\nrename from git-repack.sh\nrename to contrib/examples/git-repack.sh\ndiff --git a/git.c b/git.c\nindex 2025f77..03510be 100644\n--- a/git.c\n+++ b/git.c\n@@ -396,6 +396,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"remote\", cmd_remote, RUN_SETUP },\n \t\t{ \"remote-ext\", cmd_remote_ext },\n \t\t{ \"remote-fd\", cmd_remote_fd },\n+\t\t{ \"repack\", cmd_repack, RUN_SETUP },\n \t\t{ \"replace\", cmd_replace, RUN_SETUP },\n \t\t{ \"repo-config\", cmd_repo_config, RUN_SETUP_GENTLY },\n \t\t{ \"rerere\", cmd_rerere, RUN_SETUP },\n-- \n1.8.4.rc3.2.g2c2b664\n"},{"id":"225394","messageId":"DC7C72E5-8B52-48B2-B10B-1095118865B1@gmail.com","threadId":"34687","inReplyTo":"1376836587-25248-1-git-send-email-stefanbeller@googlemail.com","subject":"Re: [RFC PATCHv3] repack: rewrite the shell script in C.","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2013-08-18T15:41:51Z","receivedAt":"2013-08-18T15:41:51Z","isPatch":false,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"On Aug 18, 2013, at 07:36, Stefan Beller wrote:\n\n> +\t\t\tfprintf(stderr,\n> +\t\t\t\t\"WARNING: Some packs in use have been renamed by\\n\"\n> +\t\t\t\t\"WARNING: prefixing old- to their name, in order to\\n\"\n> +\t\t\t\t\"WARNING: replace them with the new version of the\\n\"\n> +\t\t\t\t\"WARNING: file. But the operation failed, and\\n\"\n> +\t\t\t\t\"WARNING: attempt to rename them back to their\\n\"\n> +\t\t\t\t\"WARNING: original names also failed.\\n\"\n\nBad grammar \"But the operation failed, and attempt to rename them ...\".\n\nHow about \"But the operation failed, and the attempt to rename \nthem ...\" instead.\n"},{"id":"225395","messageId":"5210F9FE.1080801@web.de","threadId":"34687","inReplyTo":"1376836587-25248-1-git-send-email-stefanbeller@googlemail.com","subject":"Re: [RFC PATCHv3] repack: rewrite the shell script in C.","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2013-08-18T16:44:46Z","receivedAt":"2013-08-18T16:44:46Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"> +static void remove_temporary_files() {\n> +\tDIR *dir;\n> +\tstruct dirent *e;\n> +\tchar *prefix, *path;\n> +\n> +\tprefix = mkpathdup(\".tmp-%d-pack\", getpid());\n> +\tpath = mkpathdup(\"%s/pack\", get_object_directory());\n> +\n> +\tdir = opendir(path);\n> +\twhile ((e = readdir(dir)) != NULL) {\n> +\t\tif (!prefixcmp(e->d_name, prefix)) {\n> +\t\t\tstruct strbuf fname = STRBUF_INIT;\n> +\t\t\tstrbuf_addf(&fname, \"%s/%s\", path, e->d_name);\n> +\t\t\tunlink(strbuf_detach(&fname, NULL));\n\nI'm not sure I like the memory allocation done here for each file to be\ndeleted, but it's probably not worth worrying about.\n\n> +void get_pack_sha1_list(char *packdir, struct string_list *sha1_list)\n> +{\n> +\tDIR *dir;\n> +\tstruct dirent *e;\n> +\tchar *path, *suffix;\n> +\n> +\tpath = mkpathdup(\"%s/pack\", get_object_directory());\n> +\tsuffix = \".pack\";\n> +\n> +\tdir = opendir(path);\n> +\twhile ((e = readdir(dir)) != NULL) {\n> +\t\tif (!suffixcmp(e->d_name, suffix)) {\n> +\t\t\tchar *buf, *sha1;\n> +\t\t\tbuf = xmemdupz(e->d_name, strlen(e->d_name));\n> +\t\t\tbuf[strlen(e->d_name) - strlen(suffix)] = '\\0';\n> +\t\t\tif (strlen(e->d_name) - strlen(suffix) > 40) {\n> +\t\t\t\tsha1 = &buf[strlen(e->d_name) - strlen(suffix) - 40];\n> +\t\t\t\tstring_list_append_nodup(sha1_list, sha1);\n\nUnless sha1 == buf, this will crash when that string_list is freed\nbecause sha1 was not returned by malloc. If it doesn't crash for\nyou then I guess sha1_list is never freed. :) How about just\ntaking the part of d_name we need, like this?\n\n\t\t\tsize_t len = strlen(e->d_name) - strlen(suffix);\n\t\t\tif (len > 40) {\n\t\t\t\tchar *sha1 = xmemdupz(e->d_name + len - 40, 40);\n\t\t\t\tstring_list_append_nodup(sha1_list, sha1);\n\t\t\t}\n\n> +\t\t\t} else {\n> +\t\t\t\t/*TODO: what should happen to pack files having no 40 char sha1 specifier?*/\n\nWhat does the current code do with them? From a quick glance it\nlooks like it deletes them in the end, right?\n\nRené\n"},{"id":"225436","messageId":"1376864786-21367-1-git-send-email-stefanbeller@googlemail.com","threadId":"34687","inReplyTo":"5210F9FE.1080801@web.de","subject":"[RFC PATCHv4] repack: rewrite the shell script in C.","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-08-18T22:26:26Z","receivedAt":"2013-08-18T22:26:26Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This is the beginning of the rewrite of the repacking.\n\n * rename get_pack_sha1_list to get_pack_filename_list, which\n * reads the pack directory only once as suggested by Rene.\n * fix the grammar as suggested by Kyle.\n\nAll tests have been positive at least once now.\nHowever there is still one non-deterministic error occuring,\nI am tired to search for it now, I'll get it working tommorow.\n\nSigned-off-by: Stefan Beller \n---\n Makefile | 2 +-\n builtin.h | 1 +\n builtin/repack.c | 372 ++++++++++++++++++++++++\n git-repack.sh => contrib/examples/git-repack.sh | 0\n git.c | 1 +\n 5 files changed, 375 insertions(+), 1 deletion(-)\n create mode 100644 builtin/repack.c\n rename git-repack.sh => contrib/examples/git-repack.sh (100%)\n\ndiff --git a/Makefile b/Makefile\nindex ef442eb..4ec5bbe 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -464,7 +464,6 @@ SCRIPT_SH += git-pull.sh\n SCRIPT_SH += git-quiltimport.sh\n SCRIPT_SH += git-rebase.sh\n SCRIPT_SH += git-remote-testgit.sh\n-SCRIPT_SH += git-repack.sh\n SCRIPT_SH += git-request-pull.sh\n SCRIPT_SH += git-stash.sh\n SCRIPT_SH += git-submodule.sh\n@@ -972,6 +971,7 @@ BUILTIN_OBJS += builtin/reflog.o\n BUILTIN_OBJS += builtin/remote.o\n BUILTIN_OBJS += builtin/remote-ext.o\n BUILTIN_OBJS += builtin/remote-fd.o\n+BUILTIN_OBJS += builtin/repack.o\n BUILTIN_OBJS += builtin/replace.o\n BUILTIN_OBJS += builtin/rerere.o\n BUILTIN_OBJS += builtin/reset.o\ndiff --git a/builtin.h b/builtin.h\nindex 8afa2de..b56cf07 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -102,6 +102,7 @@ extern int cmd_reflog(int argc, const char **argv, const char *prefix);\n extern int cmd_remote(int argc, const char **argv, const char *prefix);\n extern int cmd_remote_ext(int argc, const char **argv, const char *prefix);\n extern int cmd_remote_fd(int argc, const char **argv, const char *prefix);\n+extern int cmd_repack(int argc, const char **argv, const char *prefix);\n extern int cmd_repo_config(int argc, const char **argv, const char *prefix);\n extern int cmd_rerere(int argc, const char **argv, const char *prefix);\n extern int cmd_reset(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/repack.c b/builtin/repack.c\nnew file mode 100644\nindex 0000000..bfaaad7\n--- /dev/null\n+++ b/builtin/repack.c\n@@ -0,0 +1,372 @@\n+/*\n+ * The shell version was written by Linus Torvalds (2005) and many others.\n+ * This is a translation into C by Stefan Beller (2013)\n+ */\n+\n+#include \"builtin.h\"\n+#include \"cache.h\"\n+#include \"dir.h\"\n+#include \"parse-options.h\"\n+#include \"run-command.h\"\n+#include \"sigchain.h\"\n+#include \"strbuf.h\"\n+#include \"string-list.h\"\n+\n+#include \"argv-array.h\"\n+\n+static const char *const git_repack_usage[] = {\n+\tN_(\"git repack [options]\"),\n+\tNULL\n+};\n+\n+/* enabled by default since 22c79eab (2008-06-25) */\n+static int delta_base_offset = 1;\n+\n+static int repack_config(const char *var, const char *value, void *cb)\n+{\n+\tif (!strcmp(var, \"repack.usedeltabaseoffset\")) {\n+\t\tdelta_base_offset = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\treturn git_default_config(var, value, cb);\n+}\n+\n+static void remove_temporary_files() {\n+\tDIR *dir;\n+\tstruct dirent *e;\n+\tchar *prefix, *path;\n+\n+\tprefix = mkpathdup(\".tmp-%d-pack\", getpid());\n+\tpath = mkpathdup(\"%s/pack\", get_object_directory());\n+\n+\tdir = opendir(path);\n+\twhile ((e = readdir(dir)) != NULL) {\n+\t\tif (!prefixcmp(e->d_name, prefix)) {\n+\t\t\tstruct strbuf fname = STRBUF_INIT;\n+\t\t\tstrbuf_addf(&fname, \"%s/%s\", path, e->d_name);\n+\t\t\tunlink(strbuf_detach(&fname, NULL));\n+\t\t}\n+\t}\n+\tfree(prefix);\n+\tfree(path);\n+\tclosedir(dir);\n+}\n+\n+static void remove_pack_on_signal(int signo)\n+{\n+\tremove_temporary_files();\n+\tsigchain_pop(signo);\n+\traise(signo);\n+}\n+\n+/*\n+ * Fills the filename list with all the files found in the pack directory\n+ * ending with .pack, without that extension.\n+ */\n+void get_pack_filenames(char *packdir, struct string_list *fname_list)\n+{\n+\tDIR *dir;\n+\tstruct dirent *e;\n+\tchar *path, *suffix, *fname;\n+\n+\tpath = mkpathdup(\"%s/pack\", get_object_directory());\n+\tsuffix = \".pack\";\n+\n+\tdir = opendir(path);\n+\twhile ((e = readdir(dir)) != NULL) {\n+\t\tif (!suffixcmp(e->d_name, suffix)) {\n+\t\t\tsize_t len = strlen(e->d_name) - strlen(suffix);\n+\t\t\tfname = xmemdupz(e->d_name, len);\n+\t\t\tstring_list_append_nodup(fname_list, fname);\n+\t\t}\n+\t}\n+\tfree(path);\n+\tclosedir(dir);\n+}\n+\n+/*\n+ * remove_pack will remove any files following the pattern *${SHA1}.{EXT}\n+ * where EXT is one of {pack, idx, keep}. The SHA1 consists of 40 chars and\n+ * is specified by the sha1 parameter.\n+ * path is specifying the directory in which all found files will be deleted.\n+ */\n+void remove_pack(char *path, char* sha1)\n+{\n+\tchar *exts[] = {\".pack\", \".idx\",\".keep\"};\n+\tchar *fname;\n+\tint ext = 0;\n+\tfor (ext = 0; ext < 3; ext++) {\n+\t\tfname = mkpathdup(\"%s/%s%s\", path, sha1, exts[ext]);\n+\t\tunlink(fname);\n+\t\tfree(fname);\n+\t}\n+}\n+\n+int cmd_repack(int argc, const char **argv, const char *prefix) {\n+\n+\tint pack_everything = 0;\n+\tint pack_everything_but_loose = 0;\n+\tint delete_redundant = 0;\n+\tchar *unpack_unreachable = NULL;\n+\tint window = 0, window_memory = 0;\n+\tint depth = 0;\n+\tint max_pack_size = 0;\n+\tint no_reuse_delta = 0, no_reuse_object = 0;\n+\tint no_update_server_info = 0;\n+\tint quiet = 0;\n+\tint local = 0;\n+\tchar *packdir, *packtmp;\n+\tstruct child_process cmd;\n+\tstruct string_list_item *item;\n+\tstruct string_list existing_packs = STRING_LIST_INIT_DUP;\n+\tstruct stat statbuffer;\n+\n+\tstruct option builtin_repack_options[] = {\n+\t\tOPT_BOOL('a', \"all\", &pack_everything,\n+\t\t\t\tN_(\"pack everything in a single pack\")),\n+\t\tOPT_BOOL('A', \"all-but-loose\", &pack_everything_but_loose,\n+\t\t\t\tN_(\"same as -a, and turn unreachable objects loose\")),\n+\t\tOPT_BOOL('d', \"delete-redundant\", &delete_redundant,\n+\t\t\t\tN_(\"remove redundant packs, and run git-prune-packed\")),\n+\t\tOPT_BOOL('f', \"no-reuse-delta\", &no_reuse_delta,\n+\t\t\t\tN_(\"pass --no-reuse-delta to git-pack-objects\")),\n+\t\tOPT_BOOL('F', \"no-reuse-object\", &no_reuse_object,\n+\t\t\t\tN_(\"pass --no-reuse-object to git-pack-objects\")),\n+\t\tOPT_BOOL('n', NULL, &no_update_server_info,\n+\t\t\t\tN_(\"do not run git-update-server-info\")),\n+\t\tOPT__QUIET(&quiet, N_(\"be quiet\")),\n+\t\tOPT_BOOL('l', \"local\", &local,\n+\t\t\t\tN_(\"pass --local to git-pack-objects\")),\n+\t\tOPT_STRING(0, \"unpack-unreachable\", &unpack_unreachable, N_(\"approxidate\"),\n+\t\t\t\tN_(\"with -A, do not loosen objects older than this Packing constraints\")),\n+\t\tOPT_INTEGER(0, \"window\", &window,\n+\t\t\t\tN_(\"size of the window used for delta compression\")),\n+\t\tOPT_INTEGER(0, \"window-memory\", &window_memory,\n+\t\t\t\tN_(\"same as the above, but limit memory size instead of entries count\")),\n+\t\tOPT_INTEGER(0, \"depth\", &depth,\n+\t\t\t\tN_(\"limits the maximum delta depth\")),\n+\t\tOPT_INTEGER(0, \"max-pack-size\", &max_pack_size,\n+\t\t\t\tN_(\"maximum size of each packfile\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tgit_config(repack_config, NULL);\n+\n+\targc = parse_options(argc, argv, prefix, builtin_repack_options,\n+\t\t\t\tgit_repack_usage, 0);\n+\n+\tsigchain_push_common(remove_pack_on_signal);\n+\n+\tpackdir = mkpathdup(\"%s/pack\", get_object_directory());\n+\tpacktmp = mkpathdup(\"%s/.tmp-%d-pack\", packdir, getpid());\n+\n+\tremove_temporary_files();\n+\n+\tstruct argv_array cmd_args = ARGV_ARRAY_INIT;\n+\targv_array_push(&cmd_args, \"pack-objects\");\n+\targv_array_push(&cmd_args, \"--keep-true-parents\");\n+\targv_array_push(&cmd_args, \"--honor-pack-keep\");\n+\targv_array_push(&cmd_args, \"--non-empty\");\n+\targv_array_push(&cmd_args, \"--all\");\n+\targv_array_push(&cmd_args, \"--reflog\");\n+\n+\tif (window)\n+\t\targv_array_pushf(&cmd_args, \"--window=%u\", window);\n+\n+\tif (window_memory)\n+\t\targv_array_pushf(&cmd_args, \"--window-memory=%u\", window_memory);\n+\n+\tif (depth)\n+\t\targv_array_pushf(&cmd_args, \"--depth=%u\", depth);\n+\n+\tif (max_pack_size)\n+\t\targv_array_pushf(&cmd_args, \"--max_pack_size=%u\", max_pack_size);\n+\n+\tif (pack_everything + pack_everything_but_loose == 0) {\n+\t\targv_array_push(&cmd_args, \"--unpacked\");\n+\t\targv_array_push(&cmd_args, \"--incremental\");\n+\t} else {\n+\t\tif (pack_everything_but_loose && delete_redundant)\n+\t\t\targv_array_push(&cmd_args, \"--unpack-unreachable\");\n+\n+\t\tstruct string_list fname_list = STRING_LIST_INIT_DUP;\n+\t\tget_pack_filenames(packdir, &fname_list);\n+\t\tfor_each_string_list_item(item, &fname_list) {\n+\t\t\tchar *fname;\n+\t\t\tfname = mkpathdup(\"%s/%s.keep\", packdir, item->string);\n+\t\t\tif (stat(fname, &statbuffer) && S_ISREG(statbuffer.st_mode)) {\n+\t\t\t\t/* when the keep file is there, we're ignoring that pack */\n+\t\t\t} else {\n+\t\t\t\tstring_list_append(&existing_packs, item->string);\n+\t\t\t}\n+\t\t\tfree(fname);\n+\t\t}\n+\n+\t\tif (existing_packs.nr && unpack_unreachable && delete_redundant) {\n+\t\t\targv_array_pushf(&cmd_args, \"--unpack-unreachable=%s\", unpack_unreachable);\n+\t\t}\n+\t}\n+\n+\tif (local)\n+\t\targv_array_push(&cmd_args, \"--local\");\n+\n+\tif (delta_base_offset)\n+\t\targv_array_push(&cmd_args, \"--delta-base-offset\");\n+\n+\targv_array_push(&cmd_args, packtmp);\n+\n+\tmemset(&cmd, 0, sizeof(cmd));\n+\tcmd.argv = argv_array_detach(&cmd_args, NULL);\n+\tcmd.git_cmd = 1;\n+\tcmd.out = -1;\n+\tcmd.no_stdin = 1;\n+\n+\tif (run_command(&cmd))\n+\t\treturn 1;\n+\n+\tstruct string_list names = STRING_LIST_INIT_DUP;\n+\tstruct string_list rollback = STRING_LIST_INIT_DUP;\n+\n+\tchar line[1024];\n+\tint counter = 0;\n+\tFILE *out = xfdopen(cmd.out, \"r\");\n+\twhile (fgets(line, sizeof(line), out)) {\n+\t\t/* a line consists of 40 hex chars + '\\n' */\n+\t\tassert(strlen(line) == 41);\n+\t\tline[40] = '\\0';\n+\t\tstring_list_append(&names, line);\n+\t\tcounter++;\n+\t}\n+\tif (!counter)\n+\t\tprintf(\"Nothing new to pack.\\n\");\n+\tfclose(out);\n+\n+\tchar *exts[2] = {\".idx\", \".pack\"};\n+\tint failed = 0;\n+\tfor_each_string_list_item(item, &names) {\n+\t\tint ext;\n+\t\tfor (ext = 0; ext < 1; ext++) {\n+\t\t\tchar *fname, *fname_old;\n+\t\t\tfname = mkpathdup(\"%s/%s%s\", packdir, item->string, exts[ext]);\n+\t\t\tif (!file_exists(fname)) {\n+\t\t\t\tfree(fname);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n+\t\t\tfname_old = mkpathdup(\"%s/old-%s%s\", packdir, item->string, exts[ext]);\n+\t\t\tif (file_exists(fname_old))\n+\t\t\t\tunlink(fname_old);\n+\n+\t\t\tif (rename(fname, fname_old)) {\n+\t\t\t\tfailed = 1;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tstring_list_append_nodup(&rollback, fname);\n+\t\t}\n+\t\tif (failed)\n+\t\t\t/* set to last element to break for_each loop */\n+\t\t\titem = names.items + names.nr;\n+\t}\n+\tif (failed) {\n+\t\tstruct string_list rollback_failure;\n+\t\tfor_each_string_list_item(item, &rollback) {\n+\t\t\tchar *fname, *fname_old;\n+\t\t\tfname = mkpathdup(\"%s/%s\", packdir, item->string);\n+\t\t\tfname_old = mkpathdup(\"%s/old-%s\", packdir, item->string);\n+\t\t\tif (rename(fname_old, fname))\n+\t\t\t\tstring_list_append(&rollback_failure, fname);\n+\t\t\tfree(fname);\n+\t\t\tfree(fname_old);\n+\t\t}\n+\n+\t\tif (rollback.nr) {\n+\t\t\tint i;\n+\t\t\tfprintf(stderr,\n+\t\t\t\t\"WARNING: Some packs in use have been renamed by\\n\"\n+\t\t\t\t\"WARNING: prefixing old- to their name, in order to\\n\"\n+\t\t\t\t\"WARNING: replace them with the new version of the\\n\"\n+\t\t\t\t\"WARNING: file. But the operation failed, and the\\n\"\n+\t\t\t\t\"WARNING: attempt to rename them back to their\\n\"\n+\t\t\t\t\"WARNING: original names also failed.\\n\"\n+\t\t\t\t\"WARNING: Please rename them in $PACKDIR manually:\\n\");\n+\t\t\tfor (i = 0; i < rollback.nr; i++)\n+\t\t\t\tfprintf(stderr, \"WARNING: old-%s -> %s\\n\",\n+\t\t\t\t\trollback.items[i].string,\n+\t\t\t\t\trollback.items[i].string);\n+\t\t}\n+\t\texit(1);\n+\t}\n+\n+\t/* Now the ones with the same name are out of the way... */\n+\tfor_each_string_list_item(item, &names) {\n+\t\tchar *fname, *fname_old;\n+\t\tfname = mkpathdup(\"%s/pack-%s.pack\", packdir, item->string);\n+\t\tfname_old = mkpathdup(\"%s-%s.pack\", packtmp, item->string);\n+\t\tstat(fname_old, &statbuffer);\n+\t\tstatbuffer.st_mode &= ~S_IWOTH;\n+\t\tchmod(fname_old, statbuffer.st_mode);\n+\t\tif (rename(fname_old, fname))\n+\t\t\tdie(\"Could not rename packfile: %s -> %s\", fname_old, fname);\n+\t\tfree(fname);\n+\t\tfree(fname_old);\n+\n+\t\tfname = mkpathdup(\"%s/pack-%s.idx\", packdir, item->string);\n+\t\tfname_old = mkpathdup(\"%s-%s.idx\", packtmp, item->string);\n+\t\tstat(fname_old, &statbuffer);\n+\t\tstatbuffer.st_mode &= ~S_IWOTH;\n+\t\tchmod(fname_old, statbuffer.st_mode);\n+\t\tif (rename(fname_old, fname))\n+\t\t\tdie(\"Could not rename packfile: %s -> %s\", fname_old, fname);\n+\t\tfree(fname);\n+\t\tfree(fname_old);\n+\t}\n+\n+\t/* Remove the \"old-\" files */\n+\tfor_each_string_list_item(item, &names) {\n+\t\tchar *fname;\n+\t\tfname = mkpathdup(\"%s/old-pack-%s.idx\", packdir, item->string);\n+\t\tif (remove_path(fname))\n+\t\t\tdie(\"Could not remove file: %s\", fname);\n+\t\tfree(fname);\n+\n+\t\tfname = mkpathdup(\"%s/old-pack-%s.pack\", packdir, item->string);\n+\t\tif (remove_path(fname))\n+\t\t\tdie(\"Could not remove file: %s\", fname);\n+\t\tfree(fname);\n+\t}\n+\n+\t/* End of pack replacement. */\n+\tif (delete_redundant) {\n+\t\tsort_string_list(&names);\n+\t\tfor_each_string_list_item(item, &existing_packs) {\n+\t\t\tchar *sha1;\n+\t\t\tsize_t len = strlen(item->string);\n+\t\t\tif (len < 40)\n+\t\t\t\tcontinue;\n+\t\t\tsha1 = item->string + len - 40;\n+\t\t\tif (!string_list_has_string(&names, sha1))\n+\t\t\t\tremove_pack(packdir, item->string);\n+\t\t}\n+\t\targv_array_clear(&cmd_args);\n+\t\targv_array_push(&cmd_args, \"prune-packed\");\n+\t\tif (quiet)\n+\t\t\targv_array_push(&cmd_args, \"--quiet\");\n+\n+\t\tmemset(&cmd, 0, sizeof(cmd));\n+\t\tcmd.argv = argv_array_detach(&cmd_args, NULL);\n+\t\tcmd.git_cmd = 1;\n+\t\trun_command(&cmd);\n+\t}\n+\n+\tif (!no_update_server_info) {\n+\t\targv_array_clear(&cmd_args);\n+\t\targv_array_push(&cmd_args, \"update-server-info\");\n+\n+\t\tmemset(&cmd, 0, sizeof(cmd));\n+\t\tcmd.argv = argv_array_detach(&cmd_args, NULL);\n+\t\tcmd.git_cmd = 1;\n+\t\trun_command(&cmd);\n+\t}\n+\treturn 0;\n+}\n+\ndiff --git a/git-repack.sh b/contrib/examples/git-repack.sh\nsimilarity index 100%\nrename from git-repack.sh\nrename to contrib/examples/git-repack.sh\ndiff --git a/git.c b/git.c\nindex 2025f77..03510be 100644\n--- a/git.c\n+++ b/git.c\n@@ -396,6 +396,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"remote\", cmd_remote, RUN_SETUP },\n \t\t{ \"remote-ext\", cmd_remote_ext },\n \t\t{ \"remote-fd\", cmd_remote_fd },\n+\t\t{ \"repack\", cmd_repack, RUN_SETUP },\n \t\t{ \"replace\", cmd_replace, RUN_SETUP },\n \t\t{ \"repo-config\", cmd_repo_config, RUN_SETUP_GENTLY },\n \t\t{ \"rerere\", cmd_rerere, RUN_SETUP },\n-- \n1.8.4.rc3.2.g2c2b664\n"},{"id":"225490","messageId":"1376954619-24314-1-git-send-email-stefanbeller@googlemail.com","threadId":"34687","inReplyTo":"1376864786-21367-1-git-send-email-stefanbeller@googlemail.com","subject":"[RFC PATCHv4] repack: rewrite the shell script in C.","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-08-19T23:23:39Z","receivedAt":"2013-08-19T23:23:39Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Hi,\n\nso today I compared the argument lists of the repack shell script with the\nC rewrite passed on to the pack-objects command and fixed some corner \ncases (-A -d --unpack-unreachable=%s should only pass \n--unpack-unreachable=%s once to pack-objects)\nAlso I fixed some missing smaller options (--quiet, --no-reuse-delta, --no-reuse-object).\n\nI have run the test suite several times successfully now,\ntrying to find a pattern for the non-deterministically bug, which appears to\nonly occur in 1 out of 4 test suite runs.\n\nThe test suite has around 40 calls to repack and 35 calls to gc, which calls\nrepack internally. That alone covers quite a lot of the repack options,\nbut the debugging for the test cases is no fun, as the the calls to repack\nare usually not the main concern of the respective tests.\n\nThere are however \n t7700-repack.sh \n t7701-repack-unpack-unreachable.sh\n \nIt was suggested earlier, and I think it's a good idea to enhance those\ntests.\n\nAnyway, here is an updated version of the repack rewrite.\n\nStefan\n\n--8<--\nFrom f6da16ac3ca71aa746fe6d9224b06e6cc4e7a104 Mon Sep 17 00:00:00 2001\nFrom: Stefan Beller \nDate: Fri, 16 Aug 2013 02:08:47 +0200\nSubject: [RFC PATCHv4] repack: rewrite the shell script in C.\n\nThis is the beginning of the rewrite of the repacking.\n\nAll tests have been positive at least once now.\nHowever there is still a non-deterministic error occuring in\nabout 1 out of 4 test suite runs (usually in 7701 or 9301,\nbut could also occur in 5501 or 3306 iirc)\n\nSigned-off-by: Stefan Beller \n---\n Makefile | 2 +-\n builtin.h | 1 +\n builtin/repack.c | 363 ++++++++++++++++++++++++\n git-repack.sh => contrib/examples/git-repack.sh | 0\n git.c | 1 +\n 5 files changed, 366 insertions(+), 1 deletion(-)\n create mode 100644 builtin/repack.c\n rename git-repack.sh => contrib/examples/git-repack.sh (100%)\n\ndiff --git a/Makefile b/Makefile\nindex ef442eb..4ec5bbe 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -464,7 +464,6 @@ SCRIPT_SH += git-pull.sh\n SCRIPT_SH += git-quiltimport.sh\n SCRIPT_SH += git-rebase.sh\n SCRIPT_SH += git-remote-testgit.sh\n-SCRIPT_SH += git-repack.sh\n SCRIPT_SH += git-request-pull.sh\n SCRIPT_SH += git-stash.sh\n SCRIPT_SH += git-submodule.sh\n@@ -972,6 +971,7 @@ BUILTIN_OBJS += builtin/reflog.o\n BUILTIN_OBJS += builtin/remote.o\n BUILTIN_OBJS += builtin/remote-ext.o\n BUILTIN_OBJS += builtin/remote-fd.o\n+BUILTIN_OBJS += builtin/repack.o\n BUILTIN_OBJS += builtin/replace.o\n BUILTIN_OBJS += builtin/rerere.o\n BUILTIN_OBJS += builtin/reset.o\ndiff --git a/builtin.h b/builtin.h\nindex 8afa2de..b56cf07 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -102,6 +102,7 @@ extern int cmd_reflog(int argc, const char **argv, const char *prefix);\n extern int cmd_remote(int argc, const char **argv, const char *prefix);\n extern int cmd_remote_ext(int argc, const char **argv, const char *prefix);\n extern int cmd_remote_fd(int argc, const char **argv, const char *prefix);\n+extern int cmd_repack(int argc, const char **argv, const char *prefix);\n extern int cmd_repo_config(int argc, const char **argv, const char *prefix);\n extern int cmd_rerere(int argc, const char **argv, const char *prefix);\n extern int cmd_reset(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/repack.c b/builtin/repack.c\nnew file mode 100644\nindex 0000000..a87900e\n--- /dev/null\n+++ b/builtin/repack.c\n@@ -0,0 +1,363 @@\n+/*\n+ * The shell version was written by Linus Torvalds (2005) and many others.\n+ * This is a translation into C by Stefan Beller (2013)\n+ */\n+\n+#include \"builtin.h\"\n+#include \"cache.h\"\n+#include \"dir.h\"\n+#include \"parse-options.h\"\n+#include \"run-command.h\"\n+#include \"sigchain.h\"\n+#include \"strbuf.h\"\n+#include \"string-list.h\"\n+#include \"argv-array.h\"\n+\n+static int delta_base_offset = 0;\n+\n+static const char *const git_repack_usage[] = {\n+\tN_(\"git repack [options]\"),\n+\tNULL\n+};\n+\n+static int repack_config(const char *var, const char *value, void *cb)\n+{\n+\tif (!strcmp(var, \"repack.usedeltabaseoffset\")) {\n+\t\tdelta_base_offset = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\treturn git_default_config(var, value, cb);\n+}\n+\n+static void remove_temporary_files() {\n+\tDIR *dir;\n+\tstruct dirent *e;\n+\tchar *prefix, *path;\n+\n+\tprefix = mkpathdup(\".tmp-%d-pack\", getpid());\n+\tpath = mkpathdup(\"%s/pack\", get_object_directory());\n+\n+\tdir = opendir(path);\n+\twhile ((e = readdir(dir)) != NULL) {\n+\t\tif (!prefixcmp(e->d_name, prefix)) {\n+\t\t\tstruct strbuf fname = STRBUF_INIT;\n+\t\t\tstrbuf_addf(&fname, \"%s/%s\", path, e->d_name);\n+\t\t\tunlink(strbuf_detach(&fname, NULL));\n+\t\t}\n+\t}\n+\tfree(prefix);\n+\tfree(path);\n+\tclosedir(dir);\n+}\n+\n+static void remove_pack_on_signal(int signo)\n+{\n+\tremove_temporary_files();\n+\tsigchain_pop(signo);\n+\traise(signo);\n+}\n+\n+/*\n+ * Fills the filename list with all the files found in the pack directory\n+ * ending with .pack, without that extension.\n+ */\n+void get_pack_filenames(char *packdir, struct string_list *fname_list)\n+{\n+\tDIR *dir;\n+\tstruct dirent *e;\n+\tchar *path, *suffix, *fname;\n+\n+\tpath = mkpathdup(\"%s/pack\", get_object_directory());\n+\tsuffix = \".pack\";\n+\n+\tdir = opendir(path);\n+\twhile ((e = readdir(dir)) != NULL) {\n+\t\tif (!suffixcmp(e->d_name, suffix)) {\n+\t\t\tsize_t len = strlen(e->d_name) - strlen(suffix);\n+\t\t\tfname = xmemdupz(e->d_name, len);\n+\t\t\tstring_list_append_nodup(fname_list, fname);\n+\t\t}\n+\t}\n+\tfree(path);\n+\tclosedir(dir);\n+}\n+\n+void remove_pack(char *path, char* sha1)\n+{\n+\tchar *exts[] = {\".pack\", \".idx\", \".keep\"};\n+\tint ext = 0;\n+\tfor (ext = 0; ext < 3; ext++) {\n+\t\tchar *fname;\n+\t\tfname = mkpathdup(\"%s/%s%s\", path, sha1, exts[ext]);\n+\t\tunlink(fname);\n+\t\tfree(fname);\n+\t}\n+}\n+\n+int cmd_repack(int argc, const char **argv, const char *prefix) {\n+\n+\tint pack_everything = 0;\n+\tint pack_everything_but_loose = 0;\n+\tint delete_redundant = 0;\n+\tchar *unpack_unreachable = NULL;\n+\tint window = 0, window_memory = 0;\n+\tint depth = 0;\n+\tint max_pack_size = 0;\n+\tint no_reuse_delta = 0, no_reuse_object = 0;\n+\tint no_update_server_info = 0;\n+\tint quiet = 0;\n+\tint local = 0;\n+\tchar *packdir, *packtmp;\n+\tstruct child_process cmd;\n+\tstruct string_list_item *item;\n+\tstruct string_list existing_packs = STRING_LIST_INIT_DUP;\n+\tstruct stat statbuffer;\n+\tint ext;\n+\tchar *exts[2] = {\".idx\", \".pack\"};\n+\n+\tstruct option builtin_repack_options[] = {\n+\t\tOPT_BOOL('a', \"all\", &pack_everything,\n+\t\t\t\tN_(\"pack everything in a single pack\")),\n+\t\tOPT_BOOL('A', \"all-but-loose\", &pack_everything_but_loose,\n+\t\t\t\tN_(\"same as -a, and turn unreachable objects loose\")),\n+\t\tOPT_BOOL('d', \"delete-redundant\", &delete_redundant,\n+\t\t\t\tN_(\"remove redundant packs, and run git-prune-packed\")),\n+\t\tOPT_BOOL('f', \"no-reuse-delta\", &no_reuse_delta,\n+\t\t\t\tN_(\"pass --no-reuse-delta to git-pack-objects\")),\n+\t\tOPT_BOOL('F', \"no-reuse-object\", &no_reuse_object,\n+\t\t\t\tN_(\"pass --no-reuse-object to git-pack-objects\")),\n+\t\tOPT_BOOL('n', NULL, &no_update_server_info,\n+\t\t\t\tN_(\"do not run git-update-server-info\")),\n+\t\tOPT__QUIET(&quiet, N_(\"be quiet\")),\n+\t\tOPT_BOOL('l', \"local\", &local,\n+\t\t\t\tN_(\"pass --local to git-pack-objects\")),\n+\t\tOPT_STRING(0, \"unpack-unreachable\", &unpack_unreachable, N_(\"approxidate\"),\n+\t\t\t\tN_(\"with -A, do not loosen objects older than this Packing constraints\")),\n+\t\tOPT_INTEGER(0, \"window\", &window,\n+\t\t\t\tN_(\"size of the window used for delta compression\")),\n+\t\tOPT_INTEGER(0, \"window-memory\", &window_memory,\n+\t\t\t\tN_(\"same as the above, but limit memory size instead of entries count\")),\n+\t\tOPT_INTEGER(0, \"depth\", &depth,\n+\t\t\t\tN_(\"limits the maximum delta depth\")),\n+\t\tOPT_INTEGER(0, \"max-pack-size\", &max_pack_size,\n+\t\t\t\tN_(\"maximum size of each packfile\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tgit_config(repack_config, NULL);\n+\n+\targc = parse_options(argc, argv, prefix, builtin_repack_options,\n+\t\t\t\tgit_repack_usage, 0);\n+\n+\tsigchain_push_common(remove_pack_on_signal);\n+\n+\tpackdir = mkpathdup(\"%s/pack\", get_object_directory());\n+\tpacktmp = mkpathdup(\"%s/.tmp-%d-pack\", packdir, getpid());\n+\n+\tremove_temporary_files();\n+\n+\tstruct argv_array cmd_args = ARGV_ARRAY_INIT;\n+\targv_array_push(&cmd_args, \"pack-objects\");\n+\targv_array_push(&cmd_args, \"--keep-true-parents\");\n+\targv_array_push(&cmd_args, \"--honor-pack-keep\");\n+\targv_array_push(&cmd_args, \"--non-empty\");\n+\targv_array_push(&cmd_args, \"--all\");\n+\targv_array_push(&cmd_args, \"--reflog\");\n+\n+\tif (window)\n+\t\targv_array_pushf(&cmd_args, \"--window=%u\", window);\n+\n+\tif (window_memory)\n+\t\targv_array_pushf(&cmd_args, \"--window-memory=%u\", window_memory);\n+\n+\tif (depth)\n+\t\targv_array_pushf(&cmd_args, \"--depth=%u\", depth);\n+\n+\tif (max_pack_size)\n+\t\targv_array_pushf(&cmd_args, \"--max_pack_size=%u\", max_pack_size);\n+\n+\tif (no_reuse_delta)\n+\t\targv_array_pushf(&cmd_args, \"--no-reuse-delta\");\n+\n+\tif (no_reuse_object)\n+\t\targv_array_pushf(&cmd_args, \"--no-reuse-object\");\n+\n+\tif (pack_everything + pack_everything_but_loose == 0) {\n+\t\targv_array_push(&cmd_args, \"--unpacked\");\n+\t\targv_array_push(&cmd_args, \"--incremental\");\n+\t} else {\n+\t\tstruct string_list fname_list = STRING_LIST_INIT_DUP;\n+\t\tget_pack_filenames(packdir, &fname_list);\n+\t\tfor_each_string_list_item(item, &fname_list) {\n+\t\t\tchar *fname;\n+\t\t\tfname = mkpathdup(\"%s/%s.keep\", packdir, item->string);\n+\t\t\tif (stat(fname, &statbuffer) && S_ISREG(statbuffer.st_mode)) {\n+\t\t\t\t/* when the keep file is there, we're ignoring that pack */\n+\t\t\t} else {\n+\t\t\t\tstring_list_append(&existing_packs, item->string);\n+\t\t\t}\n+\t\t\tfree(fname);\n+\t\t}\n+\n+\t\tif (existing_packs.nr && delete_redundant) {\n+\t\t\tif (unpack_unreachable)\n+\t\t\t\targv_array_pushf(&cmd_args, \"--unpack-unreachable=%s\", unpack_unreachable);\n+\t\t\telse if (pack_everything_but_loose)\n+\t\t\t\targv_array_push(&cmd_args, \"--unpack-unreachable\");\n+\t\t}\n+\t}\n+\n+\tif (local)\n+\t\targv_array_push(&cmd_args, \"--local\");\n+\tif (quiet)\n+\t\targv_array_push(&cmd_args, \"--quiet\");\n+\tif (delta_base_offset)\n+\t\targv_array_push(&cmd_args, \"--delta-base-offset\");\n+\n+\targv_array_push(&cmd_args, packtmp);\n+\n+\tmemset(&cmd, 0, sizeof(cmd));\n+\tcmd.argv = argv_array_detach(&cmd_args, NULL);\n+\tcmd.git_cmd = 1;\n+\tcmd.out = -1;\n+\tcmd.no_stdin = 1;\n+\n+\tif (run_command(&cmd))\n+\t\treturn 1;\n+\n+\tstruct string_list names = STRING_LIST_INIT_DUP;\n+\tstruct string_list rollback = STRING_LIST_INIT_DUP;\n+\n+\tchar line[1024];\n+\tint counter = 0;\n+\tFILE *out = xfdopen(cmd.out, \"r\");\n+\twhile (fgets(line, sizeof(line), out)) {\n+\t\t/* a line consists of 40 hex chars + '\\n' */\n+\t\tassert(strlen(line) == 41);\n+\t\tline[40] = '\\0';\n+\t\tstring_list_append(&names, line);\n+\t\tcounter++;\n+\t}\n+\tif (!counter)\n+\t\tprintf(\"Nothing new to pack.\\n\");\n+\tfclose(out);\n+\n+\tint failed = 0;\n+\tfor_each_string_list_item(item, &names) {\n+\t\tfor (ext = 0; ext < 1; ext++) {\n+\t\t\tchar *fname, *fname_old;\n+\t\t\tfname = mkpathdup(\"%s/%s%s\", packdir, item->string, exts[ext]);\n+\t\t\tif (!file_exists(fname)) {\n+\t\t\t\tfree(fname);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n+\t\t\tfname_old = mkpathdup(\"%s/old-%s%s\", packdir, item->string, exts[ext]);\n+\t\t\tif (file_exists(fname_old))\n+\t\t\t\tunlink(fname_old);\n+\n+\t\t\tif (rename(fname, fname_old)) {\n+\t\t\t\tfailed = 1;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tfree(fname_old);\n+\t\t\tstring_list_append_nodup(&rollback, fname);\n+\t\t}\n+\t\tif (failed)\n+\t\t\t/* set to last element to break for_each loop */\n+\t\t\titem = names.items + names.nr;\n+\t}\n+\tif (failed) {\n+\t\tstruct string_list rollback_failure;\n+\t\tfor_each_string_list_item(item, &rollback) {\n+\t\t\tchar *fname, *fname_old;\n+\t\t\tfname = mkpathdup(\"%s/%s\", packdir, item->string);\n+\t\t\tfname_old = mkpathdup(\"%s/old-%s\", packdir, item->string);\n+\t\t\tif (rename(fname_old, fname))\n+\t\t\t\tstring_list_append(&rollback_failure, fname);\n+\t\t\tfree(fname);\n+\t\t\tfree(fname_old);\n+\t\t}\n+\n+\t\tif (rollback.nr) {\n+\t\t\tint i;\n+\t\t\tfprintf(stderr,\n+\t\t\t\t\"WARNING: Some packs in use have been renamed by\\n\"\n+\t\t\t\t\"WARNING: prefixing old- to their name, in order to\\n\"\n+\t\t\t\t\"WARNING: replace them with the new version of the\\n\"\n+\t\t\t\t\"WARNING: file. But the operation failed, and the\\n\"\n+\t\t\t\t\"WARNING: attempt to rename them back to their\\n\"\n+\t\t\t\t\"WARNING: original names also failed.\\n\"\n+\t\t\t\t\"WARNING: Please rename them in $PACKDIR manually:\\n\");\n+\t\t\tfor (i = 0; i < rollback.nr; i++)\n+\t\t\t\tfprintf(stderr, \"WARNING: old-%s -> %s\\n\",\n+\t\t\t\t\trollback.items[i].string,\n+\t\t\t\t\trollback.items[i].string);\n+\t\t}\n+\t\texit(1);\n+\t}\n+\n+\t/* Now the ones with the same name are out of the way... */\n+\tfor_each_string_list_item(item, &names) {\n+\t\tfor (ext = 0; ext < 2; ext++) {\n+\t\t\tchar *fname, *fname_old;\n+\t\t\tfname = mkpathdup(\"%s/pack-%s%s\", packdir, item->string, exts[ext]);\n+\t\t\tfname_old = mkpathdup(\"%s-%s%s\", packtmp, item->string, exts[ext]);\n+\t\t\tstat(fname_old, &statbuffer);\n+\t\t\tstatbuffer.st_mode &= ~S_IWUSR | ~S_IWGRP | ~S_IWOTH;\n+\t\t\tchmod(fname_old, statbuffer.st_mode);\n+\t\t\tif (rename(fname_old, fname))\n+\t\t\t\tdie(\"Could not rename packfile: %s -> %s\", fname_old, fname);\n+\t\t\tfree(fname);\n+\t\t\tfree(fname_old);\n+\t\t}\n+\t}\n+\n+\t/* Remove the \"old-\" files */\n+\tfor_each_string_list_item(item, &names) {\n+\t\tchar *fname;\n+\t\tfname = mkpathdup(\"%s/old-pack-%s.idx\", packdir, item->string);\n+\t\tif (remove_path(fname))\n+\t\t\tdie(\"Could not remove file: %s\", fname);\n+\t\tfree(fname);\n+\n+\t\tfname = mkpathdup(\"%s/old-pack-%s.pack\", packdir, item->string);\n+\t\tif (remove_path(fname))\n+\t\t\tdie(\"Could not remove file: %s\", fname);\n+\t\tfree(fname);\n+\t}\n+\n+\t/* End of pack replacement. */\n+\tif (delete_redundant) {\n+\t\tsort_string_list(&names);\n+\t\tfor_each_string_list_item(item, &existing_packs) {\n+\t\t\tchar *sha1;\n+\t\t\tsize_t len = strlen(item->string);\n+\t\t\tif (len < 40)\n+\t\t\t\tcontinue;\n+\t\t\tsha1 = item->string + len - 40;\n+\t\t\tif (!string_list_has_string(&names, sha1))\n+\t\t\t\tremove_pack(packdir, item->string);\n+\t\t}\n+\t\targv_array_clear(&cmd_args);\n+\t\targv_array_push(&cmd_args, \"prune-packed\");\n+\t\tif (quiet)\n+\t\t\targv_array_push(&cmd_args, \"--quiet\");\n+\n+\t\tmemset(&cmd, 0, sizeof(cmd));\n+\t\tcmd.argv = argv_array_detach(&cmd_args, NULL);\n+\t\tcmd.git_cmd = 1;\n+\t\trun_command(&cmd);\n+\t}\n+\n+\tif (!no_update_server_info) {\n+\t\targv_array_clear(&cmd_args);\n+\t\targv_array_push(&cmd_args, \"update-server-info\");\n+\n+\t\tmemset(&cmd, 0, sizeof(cmd));\n+\t\tcmd.argv = argv_array_detach(&cmd_args, NULL);\n+\t\tcmd.git_cmd = 1;\n+\t\trun_command(&cmd);\n+\t}\n+\treturn 0;\n+}\ndiff --git a/git-repack.sh b/contrib/examples/git-repack.sh\nsimilarity index 100%\nrename from git-repack.sh\nrename to contrib/examples/git-repack.sh\ndiff --git a/git.c b/git.c\nindex 2025f77..03510be 100644\n--- a/git.c\n+++ b/git.c\n@@ -396,6 +396,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"remote\", cmd_remote, RUN_SETUP },\n \t\t{ \"remote-ext\", cmd_remote_ext },\n \t\t{ \"remote-fd\", cmd_remote_fd },\n+\t\t{ \"repack\", cmd_repack, RUN_SETUP },\n \t\t{ \"replace\", cmd_replace, RUN_SETUP },\n \t\t{ \"repo-config\", cmd_repo_config, RUN_SETUP_GENTLY },\n \t\t{ \"rerere\", cmd_rerere, RUN_SETUP },\n-- \n1.8.4.rc3.1.gc1ebd90\n"},{"id":"225506","messageId":"52136F9C.6030308@kdbg.org","threadId":"34687","inReplyTo":"1376954619-24314-1-git-send-email-stefanbeller@googlemail.com","subject":"Re: [RFC PATCHv4] repack: rewrite the shell script in C.","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-08-20T13:31:08Z","receivedAt":"2013-08-20T13:31:08Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"I didn't look at functions above cmd_repack.\n\nAm 20.08.2013 01:23, schrieb Stefan Beller:\n> +int cmd_repack(int argc, const char **argv, const char *prefix) {\n> +\n> +\tint pack_everything = 0;\n> +\tint pack_everything_but_loose = 0;\n> +\tint delete_redundant = 0;\n> +\tchar *unpack_unreachable = NULL;\n> +\tint window = 0, window_memory = 0;\n> +\tint depth = 0;\n> +\tint max_pack_size = 0;\n> +\tint no_reuse_delta = 0, no_reuse_object = 0;\n> +\tint no_update_server_info = 0;\n> +\tint quiet = 0;\n> +\tint local = 0;\n> +\tchar *packdir, *packtmp;\n> +\tstruct child_process cmd;\n> +\tstruct string_list_item *item;\n> +\tstruct string_list existing_packs = STRING_LIST_INIT_DUP;\n> +\tstruct stat statbuffer;\n> +\tint ext;\n> +\tchar *exts[2] = {\".idx\", \".pack\"};\n> +\n> +\tstruct option builtin_repack_options[] = {\n\nAre the long forms of options your invention?\n\n> +\t\tOPT_BOOL('a', \"all\", &pack_everything,\n> +\t\t\t\tN_(\"pack everything in a single pack\")),\n> +\t\tOPT_BOOL('A', \"all-but-loose\", &pack_everything_but_loose,\n> +\t\t\t\tN_(\"same as -a, and turn unreachable objects loose\")),\n\n--all-but-loose does not express what the help text says. The long form of \n-A is --all --unpack-unreachable, so it is really just a short option for \nconvenience. It does not need its own long form.\n\n> +\t\tOPT_BOOL('d', \"delete-redundant\", &delete_redundant,\n> +\t\t\t\tN_(\"remove redundant packs, and run git-prune-packed\")),\n> +\t\tOPT_BOOL('f', \"no-reuse-delta\", &no_reuse_delta,\n> +\t\t\t\tN_(\"pass --no-reuse-delta to git-pack-objects\")),\n> +\t\tOPT_BOOL('F', \"no-reuse-object\", &no_reuse_object,\n> +\t\t\t\tN_(\"pass --no-reuse-object to git-pack-objects\")),\n\nDo we want to allow --no-no-reuse-delta and --no-no-reuse-object?\n\n> +\t\tOPT_BOOL('n', NULL, &no_update_server_info,\n> +\t\t\t\tN_(\"do not run git-update-server-info\")),\n\nNo long option name?\n\n> +\t\tOPT__QUIET(&quiet, N_(\"be quiet\")),\n> +\t\tOPT_BOOL('l', \"local\", &local,\n> +\t\t\t\tN_(\"pass --local to git-pack-objects\")),\n\nGood.\n\n> +\t\tOPT_STRING(0, \"unpack-unreachable\", &unpack_unreachable, N_(\"approxidate\"),\n> +\t\t\t\tN_(\"with -A, do not loosen objects older than this Packing constraints\")),\n\n\"Packing constraints\" is a section heading, not a continuation of the \nprevious help text.\n\n> +\t\tOPT_INTEGER(0, \"window\", &window,\n> +\t\t\t\tN_(\"size of the window used for delta compression\")),\n\nThis help text is suboptimal as the option is a count, not a \"size\" in the \nnarrow sense. But that can be changed later (as it would affect other \ntools as well, I guess).\n\n> +\t\tOPT_INTEGER(0, \"window-memory\", &window_memory,\n> +\t\t\t\tN_(\"same as the above, but limit memory size instead of entries count\")),\n> +\t\tOPT_INTEGER(0, \"depth\", &depth,\n> +\t\t\t\tN_(\"limits the maximum delta depth\")),\n> +\t\tOPT_INTEGER(0, \"max-pack-size\", &max_pack_size,\n> +\t\t\t\tN_(\"maximum size of each packfile\")),\n> +\t\tOPT_END()\n> +\t};\n\nGood.\n\n> +\n> +\tgit_config(repack_config, NULL);\n> +\n> +\targc = parse_options(argc, argv, prefix, builtin_repack_options,\n> +\t\t\t\tgit_repack_usage, 0);\n> +\n> +\tsigchain_push_common(remove_pack_on_signal);\n\nGood.\n\n> +\tpackdir = mkpathdup(\"%s/pack\", get_object_directory());\n> +\tpacktmp = mkpathdup(\"%s/.tmp-%d-pack\", packdir, getpid());\n\nShould this not be\n\n\tpackdir = xstrdup(git_path(\"pack\"));\n\tpacktmp = xstrdup(git_path(\"pack/.tmp-%d-pack\", getpid()));\n\nPerhaps make packdir and packtmp global so that the strings need not be \nduplicated in get_pack_filenames and remove_temporary_files?\n\n> +\n> +\tremove_temporary_files();\n\nYes, the shell script had this. But is it really necessary?\n\n> +\n> +\tstruct argv_array cmd_args = ARGV_ARRAY_INIT;\n\nDeclaration after statement.\n\n> +\targv_array_push(&cmd_args, \"pack-objects\");\n> +\targv_array_push(&cmd_args, \"--keep-true-parents\");\n> +\targv_array_push(&cmd_args, \"--honor-pack-keep\");\n> +\targv_array_push(&cmd_args, \"--non-empty\");\n> +\targv_array_push(&cmd_args, \"--all\");\n> +\targv_array_push(&cmd_args, \"--reflog\");\n> +\n> +\tif (window)\n> +\t\targv_array_pushf(&cmd_args, \"--window=%u\", window);\n> +\n> +\tif (window_memory)\n> +\t\targv_array_pushf(&cmd_args, \"--window-memory=%u\", window_memory);\n> +\n> +\tif (depth)\n> +\t\targv_array_pushf(&cmd_args, \"--depth=%u\", depth);\n> +\n> +\tif (max_pack_size)\n> +\t\targv_array_pushf(&cmd_args, \"--max_pack_size=%u\", max_pack_size);\n> +\n> +\tif (no_reuse_delta)\n> +\t\targv_array_pushf(&cmd_args, \"--no-reuse-delta\");\n> +\n> +\tif (no_reuse_object)\n> +\t\targv_array_pushf(&cmd_args, \"--no-reuse-object\");\n\nno_reuse_delta and no_reuse_object are mutually exclusive, according to \nthe shell script version.\n\n> +\n> +\tif (pack_everything + pack_everything_but_loose == 0) {\n> +\t\targv_array_push(&cmd_args, \"--unpacked\");\n> +\t\targv_array_push(&cmd_args, \"--incremental\");\n> +\t} else {\n> +\t\tstruct string_list fname_list = STRING_LIST_INIT_DUP;\n> +\t\tget_pack_filenames(packdir, &fname_list);\n> +\t\tfor_each_string_list_item(item, &fname_list) {\n> +\t\t\tchar *fname;\n> +\t\t\tfname = mkpathdup(\"%s/%s.keep\", packdir, item->string);\n> +\t\t\tif (stat(fname, &statbuffer) && S_ISREG(statbuffer.st_mode)) {\n\n\t\t\tif (!stat(fname, &statbuffer) && ...\n\nBut you are using file_exists() later. That should be good enough here as \nwell, no?\n\n> +\t\t\t\t/* when the keep file is there, we're ignoring that pack */\n> +\t\t\t} else {\n> +\t\t\t\tstring_list_append(&existing_packs, item->string);\n> +\t\t\t}\n> +\t\t\tfree(fname);\n> +\t\t}\n> +\n> +\t\tif (existing_packs.nr && delete_redundant) {\n> +\t\t\tif (unpack_unreachable)\n> +\t\t\t\targv_array_pushf(&cmd_args, \"--unpack-unreachable=%s\", unpack_unreachable);\n> +\t\t\telse if (pack_everything_but_loose)\n> +\t\t\t\targv_array_push(&cmd_args, \"--unpack-unreachable\");\n> +\t\t}\n> +\t}\n> +\n> +\tif (local)\n> +\t\targv_array_push(&cmd_args, \"--local\");\n> +\tif (quiet)\n> +\t\targv_array_push(&cmd_args, \"--quiet\");\n> +\tif (delta_base_offset)\n> +\t\targv_array_push(&cmd_args, \"--delta-base-offset\");\n> +\n> +\targv_array_push(&cmd_args, packtmp);\n\nOtherwise, argument setup looks fine.\n\n> +\n> +\tmemset(&cmd, 0, sizeof(cmd));\n> +\tcmd.argv = argv_array_detach(&cmd_args, NULL);\n\nIs it necessary to detach the arguments?\n\n> +\tcmd.git_cmd = 1;\n> +\tcmd.out = -1;\n> +\tcmd.no_stdin = 1;\n> +\n> +\tif (run_command(&cmd))\n> +\t\treturn 1;\n\nYou cannot run_command() and then later read its output! You must split it \ninto start_command(), read stdout, finish_command().\n\n> +\n> +\tstruct string_list names = STRING_LIST_INIT_DUP;\n> +\tstruct string_list rollback = STRING_LIST_INIT_DUP;\n\nDeclaration after statement.\n\n> +\n> +\tchar line[1024];\n> +\tint counter = 0;\n> +\tFILE *out = xfdopen(cmd.out, \"r\");\n> +\twhile (fgets(line, sizeof(line), out)) {\n> +\t\t/* a line consists of 40 hex chars + '\\n' */\n> +\t\tassert(strlen(line) == 41);\n\nYou cannot make assertions about input that you read from an external \ncommand! You can die() if the expectation is not met. But I think that in \nthis case the only necessary expectation is that a line is not empty.\n\nBTW, don't we have strbuf functions to read from an fd linewise?\n\n> +\t\tline[40] = '\\0';\n> +\t\tstring_list_append(&names, line);\n> +\t\tcounter++;\n> +\t}\n> +\tif (!counter)\n> +\t\tprintf(\"Nothing new to pack.\\n\");\n\nThis was 'say Nothing new to pack.'. say obeys --quiet, IIRC.\n\n> +\tfclose(out);\n> +\n> +\tint failed = 0;\n> +\tfor_each_string_list_item(item, &names) {\n> +\t\tfor (ext = 0; ext < 1; ext++) {\n> +\t\t\tchar *fname, *fname_old;\n> +\t\t\tfname = mkpathdup(\"%s/%s%s\", packdir, item->string, exts[ext]);\n> +\t\t\tif (!file_exists(fname)) {\n> +\t\t\t\tfree(fname);\n> +\t\t\t\tcontinue;\n> +\t\t\t}\n> +\n> +\t\t\tfname_old = mkpathdup(\"%s/old-%s%s\", packdir, item->string, exts[ext]);\n\nIf you could use git_path() instead of mkpathdup() in these two cases, we \nwould not need to free() the names.\n\n> +\t\t\tif (file_exists(fname_old))\n> +\t\t\t\tunlink(fname_old);\n> +\n> +\t\t\tif (rename(fname, fname_old)) {\n> +\t\t\t\tfailed = 1;\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t\tfree(fname_old);\n> +\t\t\tstring_list_append_nodup(&rollback, fname);\n\nAh, we would need to allocate here then.\n\n> +\t\t}\n> +\t\tif (failed)\n> +\t\t\t/* set to last element to break for_each loop */\n> +\t\t\titem = names.items + names.nr;\n\nA mere\n\t\t\tbreak;\ndoesn't do it here?\n\n> +\t}\n> +\tif (failed) {\n> +\t\tstruct string_list rollback_failure;\n> +\t\tfor_each_string_list_item(item, &rollback) {\n> +\t\t\tchar *fname, *fname_old;\n> +\t\t\tfname = mkpathdup(\"%s/%s\", packdir, item->string);\n> +\t\t\tfname_old = mkpathdup(\"%s/old-%s\", packdir, item->string);\n\nI think it's possible to attach arbitrary data to each string_list item. \nWe could attach the \"%s/old-%s\" name to the item name, then we wouldn't \nneed to re-construct the names here.\n\n> +\t\t\tif (rename(fname_old, fname))\n> +\t\t\t\tstring_list_append(&rollback_failure, fname);\n> +\t\t\tfree(fname);\n> +\t\t\tfree(fname_old);\n> +\t\t}\n> +\n> +\t\tif (rollback.nr) {\n> +\t\t\tint i;\n> +\t\t\tfprintf(stderr,\n> +\t\t\t\t\"WARNING: Some packs in use have been renamed by\\n\"\n> +\t\t\t\t\"WARNING: prefixing old- to their name, in order to\\n\"\n> +\t\t\t\t\"WARNING: replace them with the new version of the\\n\"\n> +\t\t\t\t\"WARNING: file. But the operation failed, and the\\n\"\n> +\t\t\t\t\"WARNING: attempt to rename them back to their\\n\"\n> +\t\t\t\t\"WARNING: original names also failed.\\n\"\n> +\t\t\t\t\"WARNING: Please rename them in $PACKDIR manually:\\n\");\n> +\t\t\tfor (i = 0; i < rollback.nr; i++)\n> +\t\t\t\tfprintf(stderr, \"WARNING: old-%s -> %s\\n\",\n> +\t\t\t\t\trollback.items[i].string,\n> +\t\t\t\t\trollback.items[i].string);\n> +\t\t}\n> +\t\texit(1);\n> +\t}\n> +\n> +\t/* Now the ones with the same name are out of the way... */\n> +\tfor_each_string_list_item(item, &names) {\n> +\t\tfor (ext = 0; ext < 2; ext++) {\n> +\t\t\tchar *fname, *fname_old;\n> +\t\t\tfname = mkpathdup(\"%s/pack-%s%s\", packdir, item->string, exts[ext]);\n> +\t\t\tfname_old = mkpathdup(\"%s-%s%s\", packtmp, item->string, exts[ext]);\n\nSame here: git_path()?\n\n> +\t\t\tstat(fname_old, &statbuffer);\n\nWe ignore errors during chmod in the shell script. But this doesn't give \nyou license to ignore stat() errors completely: If stat() fails, then \ndon't chmod() below, either.\n\n> +\t\t\tstatbuffer.st_mode &= ~S_IWUSR | ~S_IWGRP | ~S_IWOTH;\n\n\t\t\tstatbuffer.st_mode &= ~(S_IWUSR|S_IWGRP|S_IWOTH);\n\n> +\t\t\tchmod(fname_old, statbuffer.st_mode);\n> +\t\t\tif (rename(fname_old, fname))\n> +\t\t\t\tdie(\"Could not rename packfile: %s -> %s\", fname_old, fname);\n\nUse die_errno() here.\n\n> +\t\t\tfree(fname);\n> +\t\t\tfree(fname_old);\n> +\t\t}\n> +\t}\n> +\n> +\t/* Remove the \"old-\" files */\n> +\tfor_each_string_list_item(item, &names) {\n> +\t\tchar *fname;\n> +\t\tfname = mkpathdup(\"%s/old-pack-%s.idx\", packdir, item->string);\n> +\t\tif (remove_path(fname))\n> +\t\t\tdie(\"Could not remove file: %s\", fname);\n\ndie_errno() makes sense here, too.\n\n> +\t\tfree(fname);\n> +\n> +\t\tfname = mkpathdup(\"%s/old-pack-%s.pack\", packdir, item->string);\n> +\t\tif (remove_path(fname))\n> +\t\t\tdie(\"Could not remove file: %s\", fname);\n\nand here as well.\n\n> +\t\tfree(fname);\n\nAgain git_path?\n\n> +\t}\n> +\n> +\t/* End of pack replacement. */\n\nNit: A blank line should follow this comment.\n\n> +\tif (delete_redundant) {\n> +\t\tsort_string_list(&names);\n> +\t\tfor_each_string_list_item(item, &existing_packs) {\n> +\t\t\tchar *sha1;\n> +\t\t\tsize_t len = strlen(item->string);\n> +\t\t\tif (len < 40)\n> +\t\t\t\tcontinue;\n> +\t\t\tsha1 = item->string + len - 40;\n> +\t\t\tif (!string_list_has_string(&names, sha1))\n> +\t\t\t\tremove_pack(packdir, item->string);\n> +\t\t}\n\nOK.\n\n> +\t\targv_array_clear(&cmd_args);\n> +\t\targv_array_push(&cmd_args, \"prune-packed\");\n> +\t\tif (quiet)\n> +\t\t\targv_array_push(&cmd_args, \"--quiet\");\n> +\n> +\t\tmemset(&cmd, 0, sizeof(cmd));\n> +\t\tcmd.argv = argv_array_detach(&cmd_args, NULL);\n\nAgain: is it necessary to detach?\n\n> +\t\tcmd.git_cmd = 1;\n> +\t\trun_command(&cmd);\n> +\t}\n> +\n> +\tif (!no_update_server_info) {\n> +\t\targv_array_clear(&cmd_args);\n> +\t\targv_array_push(&cmd_args, \"update-server-info\");\n> +\n> +\t\tmemset(&cmd, 0, sizeof(cmd));\n> +\t\tcmd.argv = argv_array_detach(&cmd_args, NULL);\n\nSame here?\n\n> +\t\tcmd.git_cmd = 1;\n> +\t\trun_command(&cmd);\n> +\t}\n> +\treturn 0;\n> +}\n\nIn my opinion, it is good that you keep a large function that resembles \nthe structure of the shell script because it is easier to review. But \nultimately, it should be factored into smaller functions.\n\n-- Hannes\n"},{"id":"225521","messageId":"52138686.1070304@googlemail.com","threadId":"34687","inReplyTo":"52136F9C.6030308@kdbg.org","subject":"Re: [RFC PATCHv4] repack: rewrite the shell script in C.","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-08-20T15:08:54Z","receivedAt":"2013-08-20T15:08:54Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 08/20/2013 03:31 PM, Johannes Sixt wrote:\n> \n> Are the long forms of options your invention?\n\nI tried to keep strong similarity with the shell script for\nease of review. In the shellscript the options where \nput in variables having these names, so for example there was:\n\n\t-f)\tno_reuse=--no-reuse-delta ;;\n\t-F)\tno_reuse=--no-reuse-object ;;\n\nSo I used these variable names as well in here. And as I assumed\nthe variables are meaningful in itself.\n\nIn the shell script they may be meaningful, but with the option\nparser in the C version, I overlooked the possibility for \n--no-