{"thread":{"id":"51284","subject":"[PATCH 00/11] Create 'expire' and 'repack' verbs for git-multi-pack-index","startedAt":"2019-06-10T23:35:25Z","lastAt":"2019-07-01T19:24:57Z","messageCount":29,"participants":["Derrick Stolee via GitGitGadget","Junio C Hamano","Johannes Sixt","Eric Sunshine","Jeff King","SZEDER Gábor","Derrick Stolee","Johannes Schindelin","Andreas Schwab"],"isPatch":true,"patchVersion":1,"patchTotal":11},"messages":[{"id":"376976","messageId":"pull.261.git.gitgitgadget@gmail.com","threadId":"51284","inReplyTo":null,"subject":"[PATCH 00/11] Create 'expire' and 'repack' verbs for git-multi-pack-index","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-06-10T23:35:21Z","receivedAt":"2019-06-10T23:35:25Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"The multi-pack-index provides a fast way to find an object among a large\nlist of pack-files. It stores a single pack-reference for each object id, so\nduplicate objects are ignored. Among a list of pack-files storing the same\nobject, the most-recently modified one is used.\n\nCreate new subcommands for the multi-pack-index builtin.\n\n * 'git multi-pack-index expire': If we have a pack-file indexed by the\n multi-pack-index, but all objects in that pack are duplicated in\n more-recently modified packs, then delete that pack (and any others like\n it). Delete the reference to that pack in the multi-pack-index.\n \n \n * 'git multi-pack-index repack --batch-size=': Starting from the oldest\n pack-files covered by the multi-pack-index, find those whose \"expected\n size\" is below the batch size until we have a collection of packs whose\n expected sizes add up to the batch size. We compute the expected size by\n multiplying the number of referenced objects by the pack-size and\n dividing by the total number of objects in the pack. If the batch-size is\n zero, then select all packs. Create a new pack containing all objects\n that the multi-pack-index references to those packs.\n \n \n\nThis allows us to create a new pattern for repacking objects: run 'repack'.\nAfter enough time has passed that all Git commands that started before the\nlast 'repack' are finished, run 'expire' again. This approach has some\nadvantages over the existing \"repack everything\" model:\n\n 1. Incremental. We can repack a small batch of objects at a time, instead\n of repacking all reachable objects. We can also limit ourselves to the\n objects that do not appear in newer pack-files.\n \n \n 2. Highly Available. By adding a new pack-file (and not deleting the old\n pack-files) we do not interrupt concurrent Git commands, and do not\n suffer performance degradation. By expiring only pack-files that have no\n referenced objects, we know that Git commands that are doing normal\n object lookups* will not be interrupted.\n \n \n\n * Note: if someone concurrently runs a Git command that uses\n get_all_packs(), then that command could try to read the pack-files and\n pack-indexes that we are deleting during an expire command. Such commands\n are usually related to object maintenance (i.e. fsck, gc, pack-objects)\n or are related to less-often-used features (i.e. fast-import,\n http-backend, server-info).\n\nWe are using this approach in VFS for Git to do background maintenance of\nthe \"shared object cache\" which is a Git alternate directory filled with\npackfiles containing commits and trees. We currently download pack-files on\nan hourly basis to keep up-to-date with the central server. The cache\nservers supply packs on an hourly and daily basis, so most of the hourly\npacks become useless after a new daily pack is downloaded. The 'expire'\ncommand would clear out most of those packs, but many will still remain with\nfewer than 100 objects remaining. The 'repack' command (with a batch size of\n1-3gb, probably) can condense the remaining packs in commands that run for\n1-3 min at a time. Since the daily packs range from 100-250mb, we will also\ncombine and condense those packs.\n\nThis series is the same as v6 of an earlier series [1].\n\nThanks, -Stolee\n\n[1] https://public-inbox.org/git/pull.92.git.gitgitgadget@gmail.com/T/#u\n\nDerrick Stolee (11):\n repack: refactor pack deletion for future use\n Docs: rearrange subcommands for multi-pack-index\n multi-pack-index: prepare for 'expire' subcommand\n midx: simplify computation of pack name lengths\n midx: refactor permutation logic and pack sorting\n multi-pack-index: implement 'expire' subcommand\n multi-pack-index: prepare 'repack' subcommand\n midx: implement midx_repack()\n multi-pack-index: test expire while adding packs\n midx: add test that 'expire' respects .keep files\n t5319-multi-pack-index.sh: test batch size zero\n\n Documentation/git-multi-pack-index.txt | 32 +-\n builtin/multi-pack-index.c | 14 +-\n builtin/repack.c | 14 +-\n midx.c | 440 +++++++++++++++++++------\n midx.h | 2 +\n packfile.c | 28 ++\n packfile.h | 7 +\n t/t5319-multi-pack-index.sh | 184 +++++++++++\n 8 files changed, 602 insertions(+), 119 deletions(-)\n\n\nbase-commit: af96fe3392fb078cb5447bcb94f2ed8d79d0a4a8\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-261%2Fderrickstolee%2Fmidx-expire%2Fupstream-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-261/derrickstolee/midx-expire/upstream-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/261\n-- \ngitgitgadget\n"},{"id":"376978","messageId":"3b424f7c2acafb400284089b218d2fb57f3192f2.1560209720.git.gitgitgadget@gmail.com","threadId":"51284","inReplyTo":"pull.261.git.gitgitgadget@gmail.com","subject":"[PATCH 01/11] repack: refactor pack deletion for future use","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-06-10T23:35:22Z","receivedAt":"2019-06-10T23:35:26Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe repack builtin deletes redundant pack-files and their\nassociated .idx, .promisor, .bitmap, and .keep files. We will want\nto re-use this logic in the future for other types of repack, so\npull the logic into 'unlink_pack_path()' in packfile.c.\n\nThe 'ignore_keep' parameter is enabled for the use in repack, but\nwill be important for a future caller.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/repack.c | 14 ++------------\n packfile.c | 28 ++++++++++++++++++++++++++++\n packfile.h | 7 +++++++\n 3 files changed, 37 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex 67f8978043..1f9e6fad1b 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -129,19 +129,9 @@ static void get_non_kept_pack_filenames(struct string_list *fname_list,\n \n static void remove_redundant_pack(const char *dir_name, const char *base_name)\n {\n-\tconst char *exts[] = {\".pack\", \".idx\", \".keep\", \".bitmap\", \".promisor\"};\n-\tint i;\n \tstruct strbuf buf = STRBUF_INIT;\n-\tsize_t plen;\n-\n-\tstrbuf_addf(&buf, \"%s/%s\", dir_name, base_name);\n-\tplen = buf.len;\n-\n-\tfor (i = 0; i < ARRAY_SIZE(exts); i++) {\n-\t\tstrbuf_setlen(&buf, plen);\n-\t\tstrbuf_addstr(&buf, exts[i]);\n-\t\tunlink(buf.buf);\n-\t}\n+\tstrbuf_addf(&buf, \"%s/%s.pack\", dir_name, base_name);\n+\tunlink_pack_path(buf.buf, 1);\n \tstrbuf_release(&buf);\n }\n \ndiff --git a/packfile.c b/packfile.c\nindex 060de420d1..683ce5674c 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -352,6 +352,34 @@ void close_all_packs(struct raw_object_store *o)\n \t}\n }\n \n+void unlink_pack_path(const char *pack_name, int force_delete)\n+{\n+\tstatic const char *exts[] = {\".pack\", \".idx\", \".keep\", \".bitmap\", \".promisor\"};\n+\tint i;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tsize_t plen;\n+\n+\tstrbuf_addstr(&buf, pack_name);\n+\tstrip_suffix_mem(buf.buf, &buf.len, \".pack\");\n+\tplen = buf.len;\n+\n+\tif (!force_delete) {\n+\t\tstrbuf_addstr(&buf, \".keep\");\n+\t\tif (!access(buf.buf, F_OK)) {\n+\t\t\tstrbuf_release(&buf);\n+\t\t\treturn;\n+\t\t}\n+\t}\n+\n+\tfor (i = 0; i < ARRAY_SIZE(exts); i++) {\n+\t\tstrbuf_setlen(&buf, plen);\n+\t\tstrbuf_addstr(&buf, exts[i]);\n+\t\tunlink(buf.buf);\n+\t}\n+\n+\tstrbuf_release(&buf);\n+}\n+\n /*\n * The LRU pack is the one with the oldest MRU window, preferring packs\n * with no used windows, or the oldest mtime if it has no windows allocated.\ndiff --git a/packfile.h b/packfile.h\nindex 12baa6118a..09f5222113 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -94,6 +94,13 @@ void unuse_pack(struct pack_window **);\n void clear_delta_base_cache(void);\n struct packed_git *add_packed_git(const char *path, size_t path_len, int local);\n \n+/*\n+ * Unlink the .pack and associated extension files.\n+ * Does not unlink if 'force_delete' is false and the pack-file is\n+ * marked as \".keep\".\n+ */\n+extern void unlink_pack_path(const char *pack_name, int force_delete);\n+\n /*\n * Make sure that a pointer access into an mmap'd index file is within bounds,\n * and can provide at least 8 bytes of data.\n-- \ngitgitgadget\n\n"},{"id":"376977","messageId":"fe047db57064a2d8dafe5eb1268768cf5c441945.1560209720.git.gitgitgadget@gmail.com","threadId":"51284","inReplyTo":"pull.261.git.gitgitgadget@gmail.com","subject":"[PATCH 02/11] Docs: rearrange subcommands for multi-pack-index","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-06-10T23:35:22Z","receivedAt":"2019-06-10T23:35:27Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWe will add new subcommands to the multi-pack-index, and that will\nmake the documentation a bit messier. Clean up the 'verb'\ndescriptions by renaming the concept to 'subcommand' and removing\nthe reference to the object directory.\n\nHelped-by: Stefan Beller \nHelped-by: Szeder Gábor \nSigned-off-by: Derrick Stolee \n---\n Documentation/git-multi-pack-index.txt | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt\nindex f7778a2c85..1af406aca2 100644\n--- a/Documentation/git-multi-pack-index.txt\n+++ b/Documentation/git-multi-pack-index.txt\n@@ -9,7 +9,7 @@ git-multi-pack-index - Write and verify multi-pack-indexes\n SYNOPSIS\n --------\n [verse]\n-'git multi-pack-index' [--object-dir=] \n+'git multi-pack-index' [--object-dir=] \n \n DESCRIPTION\n -----------\n@@ -23,13 +23,13 @@ OPTIONS\n \t`/packs/multi-pack-index` for the current MIDX file, and\n \t`/packs` for the pack-files to index.\n \n+The following subcommands are available:\n+\n write::\n-\tWhen given as the verb, write a new MIDX file to\n-\t`/packs/multi-pack-index`.\n+\tWrite a new MIDX file.\n \n verify::\n-\tWhen given as the verb, verify the contents of the MIDX file\n-\tat `/packs/multi-pack-index`.\n+\tVerify the contents of the MIDX file.\n \n \n EXAMPLES\n-- \ngitgitgadget\n\n"},{"id":"376979","messageId":"b78967a0521340ca87b60a4700d9b7b72e40b128.1560209720.git.gitgitgadget@gmail.com","threadId":"51284","inReplyTo":"pull.261.git.gitgitgadget@gmail.com","subject":"[PATCH 03/11] multi-pack-index: prepare for 'expire' subcommand","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-06-10T23:35:23Z","receivedAt":"2019-06-10T23:35:29Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe multi-pack-index tracks objects in a collection of pack-files.\nOnly one copy of each object is indexed, using the modified time\nof the pack-files to determine tie-breakers. It is possible to\nhave a pack-file with no referenced objects because all objects\nhave a duplicate in a newer pack-file.\n\nIntroduce a new 'expire' subcommand to the multi-pack-index builtin.\nThis subcommand will delete these unused pack-files and rewrite the\nmulti-pack-index to no longer refer to those files. More details\nabout the specifics will follow as the method is implemented.\n\nAdd a test that verifies the 'expire' subcommand is correctly wired,\nbut will still be valid when the verb is implemented. Specifically,\ncreate a set of packs that should all have referenced objects and\nshould not be removed during an 'expire' operation. The packs are\ncreated carefully to ensure they have a specific order when sorted\nby size. This will be important in a later test.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/git-multi-pack-index.txt | 5 +++\n builtin/multi-pack-index.c | 4 ++-\n midx.c | 5 +++\n midx.h | 1 +\n t/t5319-multi-pack-index.sh | 49 ++++++++++++++++++++++++++\n 5 files changed, 63 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt\nindex 1af406aca2..6186c4c936 100644\n--- a/Documentation/git-multi-pack-index.txt\n+++ b/Documentation/git-multi-pack-index.txt\n@@ -31,6 +31,11 @@ write::\n verify::\n \tVerify the contents of the MIDX file.\n \n+expire::\n+\tDelete the pack-files that are tracked \tby the MIDX file, but\n+\thave no objects referenced by the MIDX. Rewrite the MIDX file\n+\tafterward to remove all references to these pack-files.\n+\n \n EXAMPLES\n --------\ndiff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c\nindex 72dfd3dadc..ad10d40512 100644\n--- a/builtin/multi-pack-index.c\n+++ b/builtin/multi-pack-index.c\n@@ -6,7 +6,7 @@\n #include \"trace2.h\"\n \n static char const * const builtin_multi_pack_index_usage[] = {\n-\tN_(\"git multi-pack-index [--object-dir=] (write|verify)\"),\n+\tN_(\"git multi-pack-index [--object-dir=] (write|verify|expire)\"),\n \tNULL\n };\n \n@@ -47,6 +47,8 @@ int cmd_multi_pack_index(int argc, const char **argv,\n \t\treturn write_midx_file(opts.object_dir);\n \tif (!strcmp(argv[0], \"verify\"))\n \t\treturn verify_midx_file(the_repository, opts.object_dir);\n+\tif (!strcmp(argv[0], \"expire\"))\n+\t\treturn expire_midx_packs(the_repository, opts.object_dir);\n \n \tdie(_(\"unrecognized verb: %s\"), argv[0]);\n }\ndiff --git a/midx.c b/midx.c\nindex e7e1fe4d65..3b7da1a360 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -1140,3 +1140,8 @@ int verify_midx_file(struct repository *r, const char *object_dir)\n \n \treturn verify_midx_error;\n }\n+\n+int expire_midx_packs(struct repository *r, const char *object_dir)\n+{\n+\treturn 0;\n+}\ndiff --git a/midx.h b/midx.h\nindex 3eb29731f2..505f1431b7 100644\n--- a/midx.h\n+++ b/midx.h\n@@ -50,6 +50,7 @@ int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, i\n int write_midx_file(const char *object_dir);\n void clear_midx_file(struct repository *r);\n int verify_midx_file(struct repository *r, const char *object_dir);\n+int expire_midx_packs(struct repository *r, const char *object_dir);\n \n void close_midx(struct multi_pack_index *m);\n \ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 1ebf19ec3c..1b2d32f475 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -363,4 +363,53 @@ test_expect_success 'verify incorrect 64-bit offset' '\n \t\t\"incorrect object offset\"\n '\n \n+test_expect_success 'setup expire tests' '\n+\tmkdir dup &&\n+\t(\n+\t\tcd dup &&\n+\t\tgit init &&\n+\t\ttest-tool genrandom \"data\" 4096 >large_file.txt &&\n+\t\tgit update-index --add large_file.txt &&\n+\t\tfor i in $(test_seq 1 20)\n+\t\tdo\n+\t\t\ttest_commit $i\n+\t\tdone &&\n+\t\tgit branch A HEAD &&\n+\t\tgit branch B HEAD~8 &&\n+\t\tgit branch C HEAD~13 &&\n+\t\tgit branch D HEAD~16 &&\n+\t\tgit branch E HEAD~18 &&\n+\t\tgit pack-objects --revs .git/objects/pack/pack-A <<-EOF &&\n+\t\trefs/heads/A\n+\t\t^refs/heads/B\n+\t\tEOF\n+\t\tgit pack-objects --revs .git/objects/pack/pack-B <<-EOF &&\n+\t\trefs/heads/B\n+\t\t^refs/heads/C\n+\t\tEOF\n+\t\tgit pack-objects --revs .git/objects/pack/pack-C <<-EOF &&\n+\t\trefs/heads/C\n+\t\t^refs/heads/D\n+\t\tEOF\n+\t\tgit pack-objects --revs .git/objects/pack/pack-D <<-EOF &&\n+\t\trefs/heads/D\n+\t\t^refs/heads/E\n+\t\tEOF\n+\t\tgit pack-objects --revs .git/objects/pack/pack-E <<-EOF &&\n+\t\trefs/heads/E\n+\t\tEOF\n+\t\tgit multi-pack-index write\n+\t)\n+'\n+\n+test_expect_success 'expire does not remove any packs' '\n+\t(\n+\t\tcd dup &&\n+\t\tls .git/objects/pack >expect &&\n+\t\tgit multi-pack-index expire &&\n+\t\tls .git/objects/pack >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"376980","messageId":"989d49d0b284980fc1b46bd8c309448ff538fd78.1560209720.git.gitgitgadget@gmail.com","threadId":"51284","inReplyTo":"pull.261.git.gitgitgadget@gmail.com","subject":"[PATCH 05/11] midx: refactor permutation logic and pack sorting","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-06-10T23:35:24Z","receivedAt":"2019-06-10T23:35:31Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nIn anticipation of the expire subcommand, refactor the way we sort\nthe packfiles by name. This will greatly simplify our approach to\ndropping expired packs from the list.\n\nFirst, create 'struct pack_info' to replace 'struct pack_pair'.\nThis struct contains the necessary information about a pack,\nincluding its name, a pointer to its packfile struct (if not\nalready in the multi-pack-index), and the original pack-int-id.\n\nSecond, track the pack information using an array of pack_info\nstructs in the pack_list struct. This simplifies the logic around\nthe multiple arrays we were tracking in that struct.\n\nFinally, update get_sorted_entries() to not permute the pack-int-id\nand instead supply the permutation to write_midx_object_offsets().\nThis requires sorting the packs after get_sorted_entries().\n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 156 +++++++++++++++++++++++++--------------------------------\n 1 file changed, 69 insertions(+), 87 deletions(-)\n\ndiff --git a/midx.c b/midx.c\nindex 62404620ad..6d4b84e243 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -427,12 +427,23 @@ static size_t write_midx_header(struct hashfile *f,\n \treturn MIDX_HEADER_SIZE;\n }\n \n+struct pack_info {\n+\tuint32_t orig_pack_int_id;\n+\tchar *pack_name;\n+\tstruct packed_git *p;\n+};\n+\n+static int pack_info_compare(const void *_a, const void *_b)\n+{\n+\tstruct pack_info *a = (struct pack_info *)_a;\n+\tstruct pack_info *b = (struct pack_info *)_b;\n+\treturn strcmp(a->pack_name, b->pack_name);\n+}\n+\n struct pack_list {\n-\tstruct packed_git **list;\n-\tchar **names;\n+\tstruct pack_info *info;\n \tuint32_t nr;\n-\tuint32_t alloc_list;\n-\tuint32_t alloc_names;\n+\tuint32_t alloc;\n \tstruct multi_pack_index *m;\n };\n \n@@ -445,66 +456,32 @@ static void add_pack_to_midx(const char *full_path, size_t full_path_len,\n \t\tif (packs->m && midx_contains_pack(packs->m, file_name))\n \t\t\treturn;\n \n-\t\tALLOC_GROW(packs->list, packs->nr + 1, packs->alloc_list);\n-\t\tALLOC_GROW(packs->names, packs->nr + 1, packs->alloc_names);\n+\t\tALLOC_GROW(packs->info, packs->nr + 1, packs->alloc);\n \n-\t\tpacks->list[packs->nr] = add_packed_git(full_path,\n-\t\t\t\t\t\t\tfull_path_len,\n-\t\t\t\t\t\t\t0);\n+\t\tpacks->info[packs->nr].p = add_packed_git(full_path,\n+\t\t\t\t\t\t\t full_path_len,\n+\t\t\t\t\t\t\t 0);\n \n-\t\tif (!packs->list[packs->nr]) {\n+\t\tif (!packs->info[packs->nr].p) {\n \t\t\twarning(_(\"failed to add packfile '%s'\"),\n \t\t\t\tfull_path);\n \t\t\treturn;\n \t\t}\n \n-\t\tif (open_pack_index(packs->list[packs->nr])) {\n+\t\tif (open_pack_index(packs->info[packs->nr].p)) {\n \t\t\twarning(_(\"failed to open pack-index '%s'\"),\n \t\t\t\tfull_path);\n-\t\t\tclose_pack(packs->list[packs->nr]);\n-\t\t\tFREE_AND_NULL(packs->list[packs->nr]);\n+\t\t\tclose_pack(packs->info[packs->nr].p);\n+\t\t\tFREE_AND_NULL(packs->info[packs->nr].p);\n \t\t\treturn;\n \t\t}\n \n-\t\tpacks->names[packs->nr] = xstrdup(file_name);\n+\t\tpacks->info[packs->nr].pack_name = xstrdup(file_name);\n+\t\tpacks->info[packs->nr].orig_pack_int_id = packs->nr;\n \t\tpacks->nr++;\n \t}\n }\n \n-struct pack_pair {\n-\tuint32_t pack_int_id;\n-\tchar *pack_name;\n-};\n-\n-static int pack_pair_compare(const void *_a, const void *_b)\n-{\n-\tstruct pack_pair *a = (struct pack_pair *)_a;\n-\tstruct pack_pair *b = (struct pack_pair *)_b;\n-\treturn strcmp(a->pack_name, b->pack_name);\n-}\n-\n-static void sort_packs_by_name(char **pack_names, uint32_t nr_packs, uint32_t *perm)\n-{\n-\tuint32_t i;\n-\tstruct pack_pair *pairs;\n-\n-\tALLOC_ARRAY(pairs, nr_packs);\n-\n-\tfor (i = 0; i < nr_packs; i++) {\n-\t\tpairs[i].pack_int_id = i;\n-\t\tpairs[i].pack_name = pack_names[i];\n-\t}\n-\n-\tQSORT(pairs, nr_packs, pack_pair_compare);\n-\n-\tfor (i = 0; i < nr_packs; i++) {\n-\t\tpack_names[i] = pairs[i].pack_name;\n-\t\tperm[pairs[i].pack_int_id] = i;\n-\t}\n-\n-\tfree(pairs);\n-}\n-\n struct pack_midx_entry {\n \tstruct object_id oid;\n \tuint32_t pack_int_id;\n@@ -530,7 +507,6 @@ static int midx_oid_compare(const void *_a, const void *_b)\n }\n \n static int nth_midxed_pack_midx_entry(struct multi_pack_index *m,\n-\t\t\t\t uint32_t *pack_perm,\n \t\t\t\t struct pack_midx_entry *e,\n \t\t\t\t uint32_t pos)\n {\n@@ -538,7 +514,7 @@ static int nth_midxed_pack_midx_entry(struct multi_pack_index *m,\n \t\treturn 1;\n \n \tnth_midxed_object_oid(&e->oid, m, pos);\n-\te->pack_int_id = pack_perm[nth_midxed_pack_int_id(m, pos)];\n+\te->pack_int_id = nth_midxed_pack_int_id(m, pos);\n \te->offset = nth_midxed_offset(m, pos);\n \n \t/* consider objects in midx to be from \"old\" packs */\n@@ -572,8 +548,7 @@ static void fill_pack_entry(uint32_t pack_int_id,\n * of a packfile containing the object).\n */\n static struct pack_midx_entry *get_sorted_entries(struct multi_pack_index *m,\n-\t\t\t\t\t\t struct packed_git **p,\n-\t\t\t\t\t\t uint32_t *perm,\n+\t\t\t\t\t\t struct pack_info *info,\n \t\t\t\t\t\t uint32_t nr_packs,\n \t\t\t\t\t\t uint32_t *nr_objects)\n {\n@@ -584,7 +559,7 @@ static struct pack_midx_entry *get_sorted_entries(struct multi_pack_index *m,\n \tuint32_t start_pack = m ? m->num_packs : 0;\n \n \tfor (cur_pack = start_pack; cur_pack < nr_packs; cur_pack++)\n-\t\ttotal_objects += p[cur_pack]->num_objects;\n+\t\ttotal_objects += info[cur_pack].p->num_objects;\n \n \t/*\n \t * As we de-duplicate by fanout value, we expect the fanout\n@@ -609,7 +584,7 @@ static struct pack_midx_entry *get_sorted_entries(struct multi_pack_index *m,\n \n \t\t\tfor (cur_object = start; cur_object < end; cur_object++) {\n \t\t\t\tALLOC_GROW(entries_by_fanout, nr_fanout + 1, alloc_fanout);\n-\t\t\t\tnth_midxed_pack_midx_entry(m, perm,\n+\t\t\t\tnth_midxed_pack_midx_entry(m,\n \t\t\t\t\t\t\t &entries_by_fanout[nr_fanout],\n \t\t\t\t\t\t\t cur_object);\n \t\t\t\tnr_fanout++;\n@@ -620,12 +595,12 @@ static struct pack_midx_entry *get_sorted_entries(struct multi_pack_index *m,\n \t\t\tuint32_t start = 0, end;\n \n \t\t\tif (cur_fanout)\n-\t\t\t\tstart = get_pack_fanout(p[cur_pack], cur_fanout - 1);\n-\t\t\tend = get_pack_fanout(p[cur_pack], cur_fanout);\n+\t\t\t\tstart = get_pack_fanout(info[cur_pack].p, cur_fanout - 1);\n+\t\t\tend = get_pack_fanout(info[cur_pack].p, cur_fanout);\n \n \t\t\tfor (cur_object = start; cur_object < end; cur_object++) {\n \t\t\t\tALLOC_GROW(entries_by_fanout, nr_fanout + 1, alloc_fanout);\n-\t\t\t\tfill_pack_entry(perm[cur_pack], p[cur_pack], cur_object, &entries_by_fanout[nr_fanout]);\n+\t\t\t\tfill_pack_entry(cur_pack, info[cur_pack].p, cur_object, &entries_by_fanout[nr_fanout]);\n \t\t\t\tnr_fanout++;\n \t\t\t}\n \t\t}\n@@ -654,7 +629,7 @@ static struct pack_midx_entry *get_sorted_entries(struct multi_pack_index *m,\n }\n \n static size_t write_midx_pack_names(struct hashfile *f,\n-\t\t\t\t char **pack_names,\n+\t\t\t\t struct pack_info *info,\n \t\t\t\t uint32_t num_packs)\n {\n \tuint32_t i;\n@@ -662,14 +637,14 @@ static size_t write_midx_pack_names(struct hashfile *f,\n \tsize_t written = 0;\n \n \tfor (i = 0; i < num_packs; i++) {\n-\t\tsize_t writelen = strlen(pack_names[i]) + 1;\n+\t\tsize_t writelen = strlen(info[i].pack_name) + 1;\n \n-\t\tif (i && strcmp(pack_names[i], pack_names[i - 1]) <= 0)\n+\t\tif (i && strcmp(info[i].pack_name, info[i - 1].pack_name) <= 0)\n \t\t\tBUG(\"incorrect pack-file order: %s before %s\",\n-\t\t\t pack_names[i - 1],\n-\t\t\t pack_names[i]);\n+\t\t\t info[i - 1].pack_name,\n+\t\t\t info[i].pack_name);\n \n-\t\thashwrite(f, pack_names[i], writelen);\n+\t\thashwrite(f, info[i].pack_name, writelen);\n \t\twritten += writelen;\n \t}\n \n@@ -740,6 +715,7 @@ static size_t write_midx_oid_lookup(struct hashfile *f, unsigned char hash_len,\n }\n \n static size_t write_midx_object_offsets(struct hashfile *f, int large_offset_needed,\n+\t\t\t\t\tuint32_t *perm,\n \t\t\t\t\tstruct pack_midx_entry *objects, uint32_t nr_objects)\n {\n \tstruct pack_midx_entry *list = objects;\n@@ -749,7 +725,7 @@ static size_t write_midx_object_offsets(struct hashfile *f, int large_offset_nee\n \tfor (i = 0; i < nr_objects; i++) {\n \t\tstruct pack_midx_entry *obj = list++;\n \n-\t\thashwrite_be32(f, obj->pack_int_id);\n+\t\thashwrite_be32(f, perm[obj->pack_int_id]);\n \n \t\tif (large_offset_needed && obj->offset >> 31)\n \t\t\thashwrite_be32(f, MIDX_LARGE_OFFSET_NEEDED | nr_large_offset++);\n@@ -822,20 +798,17 @@ int write_midx_file(const char *object_dir)\n \tpacks.m = load_multi_pack_index(object_dir, 1);\n \n \tpacks.nr = 0;\n-\tpacks.alloc_list = packs.m ? packs.m->num_packs : 16;\n-\tpacks.alloc_names = packs.alloc_list;\n-\tpacks.list = NULL;\n-\tpacks.names = NULL;\n-\tALLOC_ARRAY(packs.list, packs.alloc_list);\n-\tALLOC_ARRAY(packs.names, packs.alloc_names);\n+\tpacks.alloc = packs.m ? packs.m->num_packs : 16;\n+\tpacks.info = NULL;\n+\tALLOC_ARRAY(packs.info, packs.alloc);\n \n \tif (packs.m) {\n \t\tfor (i = 0; i < packs.m->num_packs; i++) {\n-\t\t\tALLOC_GROW(packs.list, packs.nr + 1, packs.alloc_list);\n-\t\t\tALLOC_GROW(packs.names, packs.nr + 1, packs.alloc_names);\n+\t\t\tALLOC_GROW(packs.info, packs.nr + 1, packs.alloc);\n \n-\t\t\tpacks.list[packs.nr] = NULL;\n-\t\t\tpacks.names[packs.nr] = xstrdup(packs.m->pack_names[i]);\n+\t\t\tpacks.info[packs.nr].orig_pack_int_id = i;\n+\t\t\tpacks.info[packs.nr].pack_name = xstrdup(packs.m->pack_names[i]);\n+\t\t\tpacks.info[packs.nr].p = NULL;\n \t\t\tpacks.nr++;\n \t\t}\n \t}\n@@ -845,10 +818,7 @@ int write_midx_file(const char *object_dir)\n \tif (packs.m && packs.nr == packs.m->num_packs)\n \t\tgoto cleanup;\n \n-\tALLOC_ARRAY(pack_perm, packs.nr);\n-\tsort_packs_by_name(packs.names, packs.nr, pack_perm);\n-\n-\tentries = get_sorted_entries(packs.m, packs.list, pack_perm, packs.nr, &nr_entries);\n+\tentries = get_sorted_entries(packs.m, packs.info, packs.nr, &nr_entries);\n \n \tfor (i = 0; i < nr_entries; i++) {\n \t\tif (entries[i].offset > 0x7fffffff)\n@@ -857,8 +827,21 @@ int write_midx_file(const char *object_dir)\n \t\t\tlarge_offsets_needed = 1;\n \t}\n \n+\tQSORT(packs.info, packs.nr, pack_info_compare);\n+\n+\t/*\n+\t * pack_perm stores a permutation between pack-int-ids from the\n+\t * previous multi-pack-index to the new one we are writing:\n+\t *\n+\t * pack_perm[old_id] = new_id\n+\t */\n+\tALLOC_ARRAY(pack_perm, packs.nr);\n+\tfor (i = 0; i < packs.nr; i++) {\n+\t\tpack_perm[packs.info[i].orig_pack_int_id] = i;\n+\t}\n+\n \tfor (i = 0; i < packs.nr; i++)\n-\t\tpack_name_concat_len += strlen(packs.names[i]) + 1;\n+\t\tpack_name_concat_len += strlen(packs.info[i].pack_name) + 1;\n \n \tif (pack_name_concat_len % MIDX_CHUNK_ALIGNMENT)\n \t\tpack_name_concat_len += MIDX_CHUNK_ALIGNMENT -\n@@ -929,7 +912,7 @@ int write_midx_file(const char *object_dir)\n \n \t\tswitch (chunk_ids[i]) {\n \t\t\tcase MIDX_CHUNKID_PACKNAMES:\n-\t\t\t\twritten += write_midx_pack_names(f, packs.names, packs.nr);\n+\t\t\t\twritten += write_midx_pack_names(f, packs.info, packs.nr);\n \t\t\t\tbreak;\n \n \t\t\tcase MIDX_CHUNKID_OIDFANOUT:\n@@ -941,7 +924,7 @@ int write_midx_file(const char *object_dir)\n \t\t\t\tbreak;\n \n \t\t\tcase MIDX_CHUNKID_OBJECTOFFSETS:\n-\t\t\t\twritten += write_midx_object_offsets(f, large_offsets_needed, entries, nr_entries);\n+\t\t\t\twritten += write_midx_object_offsets(f, large_offsets_needed, pack_perm, entries, nr_entries);\n \t\t\t\tbreak;\n \n \t\t\tcase MIDX_CHUNKID_LARGEOFFSETS:\n@@ -964,15 +947,14 @@ int write_midx_file(const char *object_dir)\n \n cleanup:\n \tfor (i = 0; i < packs.nr; i++) {\n-\t\tif (packs.list[i]) {\n-\t\t\tclose_pack(packs.list[i]);\n-\t\t\tfree(packs.list[i]);\n+\t\tif (packs.info[i].p) {\n+\t\t\tclose_pack(packs.info[i].p);\n+\t\t\tfree(packs.info[i].p);\n \t\t}\n-\t\tfree(packs.names[i]);\n+\t\tfree(packs.info[i].pack_name);\n \t}\n \n-\tfree(packs.list);\n-\tfree(packs.names);\n+\tfree(packs.info);\n \tfree(entries);\n \tfree(pack_perm);\n \tfree(midx_name);\n-- \ngitgitgadget\n\n"},{"id":"376981","messageId":"8e243939efc9131dd4f765e7931e4ad55f7e6b29.1560209720.git.gitgitgadget@gmail.com","threadId":"51284","inReplyTo":"pull.261.git.gitgitgadget@gmail.com","subject":"[PATCH 10/11] midx: add test that 'expire' respects .keep files","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-06-10T23:35:28Z","receivedAt":"2019-06-10T23:35:32Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe 'git multi-pack-index expire' subcommand may delete packs that\nare not needed from the perspective of the multi-pack-index. If\na pack has a .keep file, then we should not delete that pack. Add\na test that ensures we preserve a pack that would otherwise be\nexpired. First, create a new pack that contains every object in\nthe repo, then add it to the multi-pack-index. Then create a .keep\nfile for a pack starting with \"a-pack\" that was added in the\nprevious test. Finally, expire and verify that the pack remains\nand the other packs were expired.\n\nSigned-off-by: Derrick Stolee \n---\n t/t5319-multi-pack-index.sh | 18 ++++++++++++++++++\n 1 file changed, 18 insertions(+)\n\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 8e04ce2821..c288901401 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -510,4 +510,22 @@ test_expect_success 'expire works when adding new packs' '\n \t)\n '\n \n+test_expect_success 'expire respects .keep files' '\n+\t(\n+\t\tcd dup &&\n+\t\tgit pack-objects --revs .git/objects/pack/pack-all <<-EOF &&\n+\t\trefs/heads/A\n+\t\tEOF\n+\t\tgit multi-pack-index write &&\n+\t\tPACKA=$(ls .git/objects/pack/a-pack*\\.pack | sed s/\\.pack\\$//) &&\n+\t\ttouch $PACKA.keep &&\n+\t\tgit multi-pack-index expire &&\n+\t\tls -S .git/objects/pack/a-pack* | grep $PACKA >a-pack-files &&\n+\t\ttest_line_count = 3 a-pack-files &&\n+\t\ttest-tool read-midx .git/objects | grep idx >midx-list &&\n+\t\ttest_line_count = 2 midx-list\n+\t)\n+'\n+\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"376982","messageId":"3ed388f0a8295c504da5d0a39887f246ec532aaa.1560209720.git.gitgitgadget@gmail.com","threadId":"51284","inReplyTo":"pull.261.git.gitgitgadget@gmail.com","subject":"[PATCH 11/11] t5319-multi-pack-index.sh: test batch size zero","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-06-10T23:35:29Z","receivedAt":"2019-06-10T23:35:33Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe 'git multi-pack-index repack' command can take a batch size of\nzero, which creates a new pack-file containing all objects in the\nmulti-pack-index. The first 'repack' command will create one new\npack-file, and an 'expire' command after that will delete the old\npack-files, as they no longer contain any referenced objects in the\nmulti-pack-index.\n\nWe must remove the .keep file that was added in the previous test\nin order to expire that pack-file.\n\nAlso test that a 'repack' will do nothing if there is only one\npack-file.\n\nSigned-off-by: Derrick Stolee \n---\n t/t5319-multi-pack-index.sh | 19 +++++++++++++++++++\n 1 file changed, 19 insertions(+)\n\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex c288901401..79bfaeafa9 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -527,5 +527,24 @@ test_expect_success 'expire respects .keep files' '\n \t)\n '\n \n+test_expect_success 'repack --batch-size=0 repacks everything' '\n+\t(\n+\t\tcd dup &&\n+\t\trm .git/objects/pack/*.keep &&\n+\t\tls .git/objects/pack/*idx >idx-list &&\n+\t\ttest_line_count = 2 idx-list &&\n+\t\tgit multi-pack-index repack --batch-size=0 &&\n+\t\tls .git/objects/pack/*idx >idx-list &&\n+\t\ttest_line_count = 3 idx-list &&\n+\t\ttest-tool read-midx .git/objects | grep idx >midx-list &&\n+\t\ttest_line_count = 3 midx-list &&\n+\t\tgit multi-pack-index expire &&\n+\t\tls -al .git/objects/pack/*idx >idx-list &&\n+\t\ttest_line_count = 1 idx-list &&\n+\t\tgit multi-pack-index repack --batch-size=0 &&\n+\t\tls -al .git/objects/pack/*idx >new-idx-list &&\n+\t\ttest_cmp idx-list new-idx-list\n+\t)\n+'\n \n test_done\n-- \ngitgitgadget\n"},{"id":"376983","messageId":"ab77ce8afe186bee4c94463b832528fd2695996f.1560209720.git.gitgitgadget@gmail.com","threadId":"51284","inReplyTo":"pull.261.git.gitgitgadget@gmail.com","subject":"[PATCH 08/11] midx: implement midx_repack()","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-06-10T23:35:27Z","receivedAt":"2019-06-10T23:35:34Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nTo repack with a non-zero batch-size, first sort all pack-files by\ntheir modified time. Second, walk those pack-files from oldest\nto newest, compute their expected size, and add the packs to a list\nif they are smaller than the given batch-size. Stop when the total\nexpected size is at least the batch size.\n\nIf the batch size is zero, select all packs in the multi-pack-index.\n\nFinally, collect the objects from the multi-pack-index that are in\nthe selected packs and send them to 'git pack-objects'. Write a new\nmulti-pack-index that includes the new pack.\n\nUsing a batch size of zero is very similar to a standard 'git repack'\ncommand, except that we do not delete the old packs and instead rely\non the new multi-pack-index to prevent new processes from reading the\nold packs. This does not disrupt other Git processes that are currently\nreading the old packs based on the old multi-pack-index.\n\nWhile first designing a 'git multi-pack-index repack' operation, I\nstarted by collecting the batches based on the actual size of the\nobjects instead of the size of the pack-files. This allows repacking\na large pack-file that has very few referencd objects. However, this\ncame at a significant cost of parsing pack-files instead of simply\nreading the multi-pack-index and getting the file information for\nthe pack-files. The \"expected size\" version provides similar\nbehavior, but could skip a pack-file if the average object size is\nmuch larger than the actual size of the referenced objects, or\ncan create a large pack if the actual size of the referenced objects\nis larger than the expected size.\n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 151 +++++++++++++++++++++++++++++++++++-\n t/t5319-multi-pack-index.sh | 28 +++++++\n 2 files changed, 178 insertions(+), 1 deletion(-)\n\ndiff --git a/midx.c b/midx.c\nindex fbed8a8adb..d649644420 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -9,6 +9,7 @@\n #include \"midx.h\"\n #include \"progress.h\"\n #include \"trace2.h\"\n+#include \"run-command.h\"\n \n #define MIDX_SIGNATURE 0x4d494458 /* \"MIDX\" */\n #define MIDX_VERSION 1\n@@ -1227,7 +1228,155 @@ int expire_midx_packs(struct repository *r, const char *object_dir)\n \treturn result;\n }\n \n-int midx_repack(struct repository *r, const char *object_dir, size_t batch_size)\n+struct repack_info {\n+\ttimestamp_t mtime;\n+\tuint32_t referenced_objects;\n+\tuint32_t pack_int_id;\n+};\n+\n+static int compare_by_mtime(const void *a_, const void *b_)\n {\n+\tconst struct repack_info *a, *b;\n+\n+\ta = (const struct repack_info *)a_;\n+\tb = (const struct repack_info *)b_;\n+\n+\tif (a->mtime < b->mtime)\n+\t\treturn -1;\n+\tif (a->mtime > b->mtime)\n+\t\treturn 1;\n+\treturn 0;\n+}\n+\n+static int fill_included_packs_all(struct multi_pack_index *m,\n+\t\t\t\t unsigned char *include_pack)\n+{\n+\tuint32_t i;\n+\n+\tfor (i = 0; i < m->num_packs; i++)\n+\t\tinclude_pack[i] = 1;\n+\n+\treturn m->num_packs < 2;\n+}\n+\n+static int fill_included_packs_batch(struct repository *r,\n+\t\t\t\t struct multi_pack_index *m,\n+\t\t\t\t unsigned char *include_pack,\n+\t\t\t\t size_t batch_size)\n+{\n+\tuint32_t i, packs_to_repack;\n+\tsize_t total_size;\n+\tstruct repack_info *pack_info = xcalloc(m->num_packs, sizeof(struct repack_info));\n+\n+\tfor (i = 0; i < m->num_packs; i++) {\n+\t\tpack_info[i].pack_int_id = i;\n+\n+\t\tif (prepare_midx_pack(r, m, i))\n+\t\t\tcontinue;\n+\n+\t\tpack_info[i].mtime = m->packs[i]->mtime;\n+\t}\n+\n+\tfor (i = 0; batch_size && i < m->num_objects; i++) {\n+\t\tuint32_t pack_int_id = nth_midxed_pack_int_id(m, i);\n+\t\tpack_info[pack_int_id].referenced_objects++;\n+\t}\n+\n+\tQSORT(pack_info, m->num_packs, compare_by_mtime);\n+\n+\ttotal_size = 0;\n+\tpacks_to_repack = 0;\n+\tfor (i = 0; total_size < batch_size && i < m->num_packs; i++) {\n+\t\tint pack_int_id = pack_info[i].pack_int_id;\n+\t\tstruct packed_git *p = m->packs[pack_int_id];\n+\t\tsize_t expected_size;\n+\n+\t\tif (!p)\n+\t\t\tcontinue;\n+\t\tif (open_pack_index(p) || !p->num_objects)\n+\t\t\tcontinue;\n+\n+\t\texpected_size = (size_t)(p->pack_size\n+\t\t\t\t\t * pack_info[i].referenced_objects);\n+\t\texpected_size /= p->num_objects;\n+\n+\t\tif (expected_size >= batch_size)\n+\t\t\tcontinue;\n+\n+\t\tpacks_to_repack++;\n+\t\ttotal_size += expected_size;\n+\t\tinclude_pack[pack_int_id] = 1;\n+\t}\n+\n+\tfree(pack_info);\n+\n+\tif (total_size < batch_size || packs_to_repack < 2)\n+\t\treturn 1;\n+\n \treturn 0;\n }\n+\n+int midx_repack(struct repository *r, const char *object_dir, size_t batch_size)\n+{\n+\tint result = 0;\n+\tuint32_t i;\n+\tunsigned char *include_pack;\n+\tstruct child_process cmd = CHILD_PROCESS_INIT;\n+\tstruct strbuf base_name = STRBUF_INIT;\n+\tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n+\n+\tif (!m)\n+\t\treturn 0;\n+\n+\tinclude_pack = xcalloc(m->num_packs, sizeof(unsigned char));\n+\n+\tif (batch_size) {\n+\t\tif (fill_included_packs_batch(r, m, include_pack, batch_size))\n+\t\t\tgoto cleanup;\n+\t} else if (fill_included_packs_all(m, include_pack))\n+\t\tgoto cleanup;\n+\n+\targv_array_push(&cmd.args, \"pack-objects\");\n+\n+\tstrbuf_addstr(&base_name, object_dir);\n+\tstrbuf_addstr(&base_name, \"/pack/pack\");\n+\targv_array_push(&cmd.args, base_name.buf);\n+\tstrbuf_release(&base_name);\n+\n+\tcmd.git_cmd = 1;\n+\tcmd.in = cmd.out = -1;\n+\n+\tif (start_command(&cmd)) {\n+\t\terror(_(\"could not start pack-objects\"));\n+\t\tresult = 1;\n+\t\tgoto cleanup;\n+\t}\n+\n+\tfor (i = 0; i < m->num_objects; i++) {\n+\t\tstruct object_id oid;\n+\t\tuint32_t pack_int_id = nth_midxed_pack_int_id(m, i);\n+\n+\t\tif (!include_pack[pack_int_id])\n+\t\t\tcontinue;\n+\n+\t\tnth_midxed_object_oid(&oid, m, i);\n+\t\txwrite(cmd.in, oid_to_hex(&oid), the_hash_algo->hexsz);\n+\t\txwrite(cmd.in, \"\\n\", 1);\n+\t}\n+\tclose(cmd.in);\n+\n+\tif (finish_command(&cmd)) {\n+\t\terror(_(\"could not finish pack-objects\"));\n+\t\tresult = 1;\n+\t\tgoto cleanup;\n+\t}\n+\n+\tresult = write_midx_internal(object_dir, m, NULL);\n+\tm = NULL;\n+\n+cleanup:\n+\tif (m)\n+\t\tclose_midx(m);\n+\tfree(include_pack);\n+\treturn result;\n+}\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 133d5b7068..6e47e5d0b2 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -450,4 +450,32 @@ test_expect_success 'repack with minimum size does not alter existing packs' '\n \t)\n '\n \n+test_expect_success 'repack creates a new pack' '\n+\t(\n+\t\tcd dup &&\n+\t\tls .git/objects/pack/*idx >idx-list &&\n+\t\ttest_line_count = 5 idx-list &&\n+\t\tTHIRD_SMALLEST_SIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 3 | tail -n 1) &&\n+\t\tBATCH_SIZE=$(($THIRD_SMALLEST_SIZE + 1)) &&\n+\t\tgit multi-pack-index repack --batch-size=$BATCH_SIZE &&\n+\t\tls .git/objects/pack/*idx >idx-list &&\n+\t\ttest_line_count = 6 idx-list &&\n+\t\ttest-tool read-midx .git/objects | grep idx >midx-list &&\n+\t\ttest_line_count = 6 midx-list\n+\t)\n+'\n+\n+test_expect_success 'expire removes repacked packs' '\n+\t(\n+\t\tcd dup &&\n+\t\tls -al .git/objects/pack/*pack &&\n+\t\tls -S .git/objects/pack/*pack | head -n 4 >expect &&\n+\t\tgit multi-pack-index expire &&\n+\t\tls -S .git/objects/pack/*pack >actual &&\n+\t\ttest_cmp expect actual &&\n+\t\ttest-tool read-midx .git/objects | grep idx >midx-list &&\n+\t\ttest_line_count = 4 midx-list\n+\t)\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"376984","messageId":"dec7f384eea97511224ae5a77a82ed9a4330872c.1560209720.git.gitgitgadget@gmail.com","threadId":"51284","inReplyTo":"pull.261.git.gitgitgadget@gmail.com","subject":"[PATCH 04/11] midx: simplify computation of pack name lengths","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-06-10T23:35:24Z","receivedAt":"2019-06-10T23:35:35Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nBefore writing the multi-pack-index, we compute the length of the\npack-index names concatenated together. This forms the data in the\npack name chunk, and we precompute it to compute chunk offsets.\nThe value is also modified to fit alignment needs.\n\nPreviously, this computation was coupled with adding packs from\nthe existing multi-pack-index and the remaining packs in the object\ndir not already covered by the multi-pack-index.\n\nIn anticipation of this becoming more complicated with the 'expire'\nsubcommand, simplify the computation by centralizing it to a single\nloop before writing the file.\n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/midx.c b/midx.c\nindex 3b7da1a360..62404620ad 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -433,7 +433,6 @@ struct pack_list {\n \tuint32_t nr;\n \tuint32_t alloc_list;\n \tuint32_t alloc_names;\n-\tsize_t pack_name_concat_len;\n \tstruct multi_pack_index *m;\n };\n \n@@ -468,7 +467,6 @@ static void add_pack_to_midx(const char *full_path, size_t full_path_len,\n \t\t}\n \n \t\tpacks->names[packs->nr] = xstrdup(file_name);\n-\t\tpacks->pack_name_concat_len += strlen(file_name) + 1;\n \t\tpacks->nr++;\n \t}\n }\n@@ -812,6 +810,7 @@ int write_midx_file(const char *object_dir)\n \tuint32_t nr_entries, num_large_offsets = 0;\n \tstruct pack_midx_entry *entries = NULL;\n \tint large_offsets_needed = 0;\n+\tint pack_name_concat_len = 0;\n \n \tmidx_name = get_midx_filename(object_dir);\n \tif (safe_create_leading_directories(midx_name)) {\n@@ -827,7 +826,6 @@ int write_midx_file(const char *object_dir)\n \tpacks.alloc_names = packs.alloc_list;\n \tpacks.list = NULL;\n \tpacks.names = NULL;\n-\tpacks.pack_name_concat_len = 0;\n \tALLOC_ARRAY(packs.list, packs.alloc_list);\n \tALLOC_ARRAY(packs.names, packs.alloc_names);\n \n@@ -838,7 +836,6 @@ int write_midx_file(const char *object_dir)\n \n \t\t\tpacks.list[packs.nr] = NULL;\n \t\t\tpacks.names[packs.nr] = xstrdup(packs.m->pack_names[i]);\n-\t\t\tpacks.pack_name_concat_len += strlen(packs.names[packs.nr]) + 1;\n \t\t\tpacks.nr++;\n \t\t}\n \t}\n@@ -848,10 +845,6 @@ int write_midx_file(const char *object_dir)\n \tif (packs.m && packs.nr == packs.m->num_packs)\n \t\tgoto cleanup;\n \n-\tif (packs.pack_name_concat_len % MIDX_CHUNK_ALIGNMENT)\n-\t\tpacks.pack_name_concat_len += MIDX_CHUNK_ALIGNMENT -\n-\t\t\t\t\t (packs.pack_name_concat_len % MIDX_CHUNK_ALIGNMENT);\n-\n \tALLOC_ARRAY(pack_perm, packs.nr);\n \tsort_packs_by_name(packs.names, packs.nr, pack_perm);\n \n@@ -864,6 +857,13 @@ int write_midx_file(const char *object_dir)\n \t\t\tlarge_offsets_needed = 1;\n \t}\n \n+\tfor (i = 0; i < packs.nr; i++)\n+\t\tpack_name_concat_len += strlen(packs.names[i]) + 1;\n+\n+\tif (pack_name_concat_len % MIDX_CHUNK_ALIGNMENT)\n+\t\tpack_name_concat_len += MIDX_CHUNK_ALIGNMENT -\n+\t\t\t\t\t(pack_name_concat_len % MIDX_CHUNK_ALIGNMENT);\n+\n \thold_lock_file_for_update(&lk, midx_name, LOCK_DIE_ON_ERROR);\n \tf = hashfd(lk.tempfile->fd, lk.tempfile->filename.buf);\n \tFREE_AND_NULL(midx_name);\n@@ -881,7 +881,7 @@ int write_midx_file(const char *object_dir)\n \n \tcur_chunk++;\n \tchunk_ids[cur_chunk] = MIDX_CHUNKID_OIDFANOUT;\n-\tchunk_offsets[cur_chunk] = chunk_offsets[cur_chunk - 1] + packs.pack_name_concat_len;\n+\tchunk_offsets[cur_chunk] = chunk_offsets[cur_chunk - 1] + pack_name_concat_len;\n \n \tcur_chunk++;\n \tchunk_ids[cur_chunk] = MIDX_CHUNKID_OIDLOOKUP;\n-- \ngitgitgadget\n\n"},{"id":"376985","messageId":"1776e36f1952cb6ed7fea73e90167134285c331b.1560209720.git.gitgitgadget@gmail.com","threadId":"51284","inReplyTo":"pull.261.git.gitgitgadget@gmail.com","subject":"[PATCH 07/11] multi-pack-index: prepare 'repack' subcommand","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-06-10T23:35:26Z","receivedAt":"2019-06-10T23:35:35Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nIn an environment where the multi-pack-index is useful, it is due\nto many pack-files and an inability to repack the object store\ninto a single pack-file. However, it is likely that many of these\npack-files are rather small, and could be repacked into a slightly\nlarger pack-file without too much effort. It may also be important\nto ensure the object store is highly available and the repack\noperation does not interrupt concurrent git commands.\n\nIntroduce a 'repack' subcommand to 'git multi-pack-index' that\ntakes a '--batch-size' option. The subcommand will inspect the\nmulti-pack-index for referenced pack-files whose size is smaller\nthan the batch size, until collecting a list of pack-files whose\nsizes sum to larger than the batch size. Then, a new pack-file\nwill be created containing the objects from those pack-files that\nare referenced by the multi-pack-index. The resulting pack is\nlikely to actually be smaller than the batch size due to\ncompression and the fact that there may be objects in the pack-\nfiles that have duplicate copies in other pack-files.\n\nThe current change introduces the command-line arguments, and we\nadd a test that ensures we parse these options properly. Since\nwe specify a small batch size, we will guarantee that future\nimplementations do not change the list of pack-files.\n\nIn addition, we hard-code the modified times of the packs in\nthe pack directory to ensure the list of packs sorted by modified\ntime matches the order if sorted by size (ascending). This will\nbe important in a future test.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/git-multi-pack-index.txt | 17 +++++++++++++++++\n builtin/multi-pack-index.c | 12 ++++++++++--\n midx.c | 5 +++++\n midx.h | 1 +\n t/t5319-multi-pack-index.sh | 20 +++++++++++++++++++-\n 5 files changed, 52 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt\nindex 6186c4c936..233b2b7862 100644\n--- a/Documentation/git-multi-pack-index.txt\n+++ b/Documentation/git-multi-pack-index.txt\n@@ -36,6 +36,23 @@ expire::\n \thave no objects referenced by the MIDX. Rewrite the MIDX file\n \tafterward to remove all references to these pack-files.\n \n+repack::\n+\tCreate a new pack-file containing objects in small pack-files\n+\treferenced by the multi-pack-index. If the size given by the\n+\t`--batch-size=` argument is zero, then create a pack\n+\tcontaining all objects referenced by the multi-pack-index. For\n+\ta non-zero batch size, Select the pack-files by examining packs\n+\tfrom oldest-to-newest, computing the \"expected size\" by counting\n+\tthe number of objects in the pack referenced by the\n+\tmulti-pack-index, then divide by the total number of objects in\n+\tthe pack and multiply by the pack size. We select packs with\n+\texpected size below the batch size until the set of packs have\n+\ttotal expected size at least the batch size. If the total size\n+\tdoes not reach the batch size, then do nothing. If a new pack-\n+\tfile is created, rewrite the multi-pack-index to reference the\n+\tnew pack-file. A later run of 'git multi-pack-index expire' will\n+\tdelete the pack-files that were part of this batch.\n+\n \n EXAMPLES\n --------\ndiff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c\nindex ad10d40512..b1ea1a6aa1 100644\n--- a/builtin/multi-pack-index.c\n+++ b/builtin/multi-pack-index.c\n@@ -6,12 +6,13 @@\n #include \"trace2.h\"\n \n static char const * const builtin_multi_pack_index_usage[] = {\n-\tN_(\"git multi-pack-index [--object-dir=] (write|verify|expire)\"),\n+\tN_(\"git multi-pack-index [--object-dir=] (write|verify|expire|repack --batch-size=)\"),\n \tNULL\n };\n \n static struct opts_multi_pack_index {\n \tconst char *object_dir;\n+\tunsigned long batch_size;\n } opts;\n \n int cmd_multi_pack_index(int argc, const char **argv,\n@@ -20,6 +21,8 @@ int cmd_multi_pack_index(int argc, const char **argv,\n \tstatic struct option builtin_multi_pack_index_options[] = {\n \t\tOPT_FILENAME(0, \"object-dir\", &opts.object_dir,\n \t\t N_(\"object directory containing set of packfile and pack-index pairs\")),\n+\t\tOPT_MAGNITUDE(0, \"batch-size\", &opts.batch_size,\n+\t\t N_(\"during repack, collect pack-files of smaller size into a batch that is larger than this size\")),\n \t\tOPT_END(),\n \t};\n \n@@ -43,6 +46,11 @@ int cmd_multi_pack_index(int argc, const char **argv,\n \n \ttrace2_cmd_mode(argv[0]);\n \n+\tif (!strcmp(argv[0], \"repack\"))\n+\t\treturn midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size);\n+\tif (opts.batch_size)\n+\t\tdie(_(\"--batch-size option is only for 'repack' subcommand\"));\n+\n \tif (!strcmp(argv[0], \"write\"))\n \t\treturn write_midx_file(opts.object_dir);\n \tif (!strcmp(argv[0], \"verify\"))\n@@ -50,5 +58,5 @@ int cmd_multi_pack_index(int argc, const char **argv,\n \tif (!strcmp(argv[0], \"expire\"))\n \t\treturn expire_midx_packs(the_repository, opts.object_dir);\n \n-\tdie(_(\"unrecognized verb: %s\"), argv[0]);\n+\tdie(_(\"unrecognized subcommand: %s\"), argv[0]);\n }\ndiff --git a/midx.c b/midx.c\nindex 9b0b4c1520..fbed8a8adb 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -1226,3 +1226,8 @@ int expire_midx_packs(struct repository *r, const char *object_dir)\n \tstring_list_clear(&packs_to_drop, 0);\n \treturn result;\n }\n+\n+int midx_repack(struct repository *r, const char *object_dir, size_t batch_size)\n+{\n+\treturn 0;\n+}\ndiff --git a/midx.h b/midx.h\nindex 505f1431b7..f0ae656b5d 100644\n--- a/midx.h\n+++ b/midx.h\n@@ -51,6 +51,7 @@ int write_midx_file(const char *object_dir);\n void clear_midx_file(struct repository *r);\n int verify_midx_file(struct repository *r, const char *object_dir);\n int expire_midx_packs(struct repository *r, const char *object_dir);\n+int midx_repack(struct repository *r, const char *object_dir, size_t batch_size);\n \n void close_midx(struct multi_pack_index *m);\n \ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 12570fe7ac..133d5b7068 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -398,7 +398,8 @@ test_expect_success 'setup expire tests' '\n \t\tgit pack-objects --revs .git/objects/pack/pack-E <<-EOF &&\n \t\trefs/heads/E\n \t\tEOF\n-\t\tgit multi-pack-index write\n+\t\tgit multi-pack-index write &&\n+\t\tcp -r .git/objects/pack .git/objects/pack-backup\n \t)\n '\n \n@@ -432,4 +433,21 @@ test_expect_success 'expire removes unreferenced packs' '\n \t)\n '\n \n+test_expect_success 'repack with minimum size does not alter existing packs' '\n+\t(\n+\t\tcd dup &&\n+\t\trm -rf .git/objects/pack &&\n+\t\tmv .git/objects/pack-backup .git/objects/pack &&\n+\t\ttouch -m -t 201901010000 .git/objects/pack/pack-D* &&\n+\t\ttouch -m -t 201901010001 .git/objects/pack/pack-C* &&\n+\t\ttouch -m -t 201901010002 .git/objects/pack/pack-B* &&\n+\t\ttouch -m -t 201901010003 .git/objects/pack/pack-A* &&\n+\t\tls .git/objects/pack >expect &&\n+\t\tMINSIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 1) &&\n+\t\tgit multi-pack-index repack --batch-size=$MINSIZE &&\n+\t\tls .git/objects/pack >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"376987","messageId":"80c7d2e5810cf851af11154dbe1e5abaa852a224.1560209720.git.gitgitgadget@gmail.com","threadId":"51284","inReplyTo":"pull.261.git.gitgitgadget@gmail.com","subject":"[PATCH 09/11] multi-pack-index: test expire while adding packs","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-06-10T23:35:27Z","receivedAt":"2019-06-10T23:35:37Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nDuring development of the multi-pack-index expire subcommand, a\nversion went out that improperly computed the pack order if a new\npack was introduced while other packs were being removed. Part of\nthe subtlety of the bug involved the new pack being placed before\nother packs that already existed in the multi-pack-index.\n\nAdd a test to t5319-multi-pack-index.sh that catches this issue.\nThe test adds new packs that cause another pack to be expired, and\ncreates new packs that are lexicographically sorted before and\nafter the existing packs.\n\nSigned-off-by: Derrick Stolee \n---\n t/t5319-multi-pack-index.sh | 32 ++++++++++++++++++++++++++++++++\n 1 file changed, 32 insertions(+)\n\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 6e47e5d0b2..8e04ce2821 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -478,4 +478,36 @@ test_expect_success 'expire removes repacked packs' '\n \t)\n '\n \n+test_expect_success 'expire works when adding new packs' '\n+\t(\n+\t\tcd dup &&\n+\t\tgit pack-objects --revs .git/objects/pack/pack-combined <<-EOF &&\n+\t\trefs/heads/A\n+\t\t^refs/heads/B\n+\t\tEOF\n+\t\tgit pack-objects --revs .git/objects/pack/pack-combined <<-EOF &&\n+\t\trefs/heads/B\n+\t\t^refs/heads/C\n+\t\tEOF\n+\t\tgit pack-objects --revs .git/objects/pack/pack-combined <<-EOF &&\n+\t\trefs/heads/C\n+\t\t^refs/heads/D\n+\t\tEOF\n+\t\tgit multi-pack-index write &&\n+\t\tgit pack-objects --revs .git/objects/pack/a-pack <<-EOF &&\n+\t\trefs/heads/D\n+\t\t^refs/heads/E\n+\t\tEOF\n+\t\tgit multi-pack-index write &&\n+\t\tgit pack-objects --revs .git/objects/pack/z-pack <<-EOF &&\n+\t\trefs/heads/E\n+\t\tEOF\n+\t\tgit multi-pack-index expire &&\n+\t\tls .git/objects/pack/ | grep idx >expect &&\n+\t\ttest-tool read-midx .git/objects | grep idx >actual &&\n+\t\ttest_cmp expect actual &&\n+\t\tgit multi-pack-index verify\n+\t)\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"376986","messageId":"8213541052a7d040aad4352ee981c0de01cc6516.1560209720.git.gitgitgadget@gmail.com","threadId":"51284","inReplyTo":"pull.261.git.gitgitgadget@gmail.com","subject":"[PATCH 06/11] multi-pack-index: implement 'expire' subcommand","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-06-10T23:35:25Z","receivedAt":"2019-06-10T23:35:38Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe 'git multi-pack-index expire' subcommand looks at the existing\nmult-pack-index, counts the number of objects referenced in each\npack-file, deletes the pack-fils with no referenced objects, and\nrewrites the multi-pack-index to no longer reference those packs.\n\nRefactor the write_midx_file() method to call write_midx_internal()\nwhich now takes an existing 'struct multi_pack_index' and a list\nof pack-files to drop (as specified by the names of their pack-\nindexes). As we write the new multi-pack-index, we drop those\nfile names from the list of known pack-files.\n\nThe expire_midx_packs() method removes the unreferenced pack-files\nafter carefully closing the packs to avoid open handles.\n\nTest that a new pack-file that covers the contents of two other\npack-files leads to those pack-files being deleted during the\nexpire subcommand. Be sure to read the multi-pack-index to ensure\nit no longer references those packs.\n\nSigned-off-by: Derrick Stolee \n---\n midx.c | 119 +++++++++++++++++++++++++++++++++---\n t/t5319-multi-pack-index.sh | 20 ++++++\n 2 files changed, 129 insertions(+), 10 deletions(-)\n\ndiff --git a/midx.c b/midx.c\nindex 6d4b84e243..9b0b4c1520 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -34,6 +34,8 @@\n #define MIDX_CHUNK_LARGE_OFFSET_WIDTH (sizeof(uint64_t))\n #define MIDX_LARGE_OFFSET_NEEDED 0x80000000\n \n+#define PACK_EXPIRED UINT_MAX\n+\n static char *get_midx_filename(const char *object_dir)\n {\n \treturn xstrfmt(\"%s/pack/multi-pack-index\", object_dir);\n@@ -431,6 +433,7 @@ struct pack_info {\n \tuint32_t orig_pack_int_id;\n \tchar *pack_name;\n \tstruct packed_git *p;\n+\tunsigned expired : 1;\n };\n \n static int pack_info_compare(const void *_a, const void *_b)\n@@ -478,6 +481,7 @@ static void add_pack_to_midx(const char *full_path, size_t full_path_len,\n \n \t\tpacks->info[packs->nr].pack_name = xstrdup(file_name);\n \t\tpacks->info[packs->nr].orig_pack_int_id = packs->nr;\n+\t\tpacks->info[packs->nr].expired = 0;\n \t\tpacks->nr++;\n \t}\n }\n@@ -637,13 +641,17 @@ static size_t write_midx_pack_names(struct hashfile *f,\n \tsize_t written = 0;\n \n \tfor (i = 0; i < num_packs; i++) {\n-\t\tsize_t writelen = strlen(info[i].pack_name) + 1;\n+\t\tsize_t writelen;\n+\n+\t\tif (info[i].expired)\n+\t\t\tcontinue;\n \n \t\tif (i && strcmp(info[i].pack_name, info[i - 1].pack_name) <= 0)\n \t\t\tBUG(\"incorrect pack-file order: %s before %s\",\n \t\t\t info[i - 1].pack_name,\n \t\t\t info[i].pack_name);\n \n+\t\twritelen = strlen(info[i].pack_name) + 1;\n \t\thashwrite(f, info[i].pack_name, writelen);\n \t\twritten += writelen;\n \t}\n@@ -725,6 +733,11 @@ static size_t write_midx_object_offsets(struct hashfile *f, int large_offset_nee\n \tfor (i = 0; i < nr_objects; i++) {\n \t\tstruct pack_midx_entry *obj = list++;\n \n+\t\tif (perm[obj->pack_int_id] == PACK_EXPIRED)\n+\t\t\tBUG(\"object %s is in an expired pack with int-id %d\",\n+\t\t\t oid_to_hex(&obj->oid),\n+\t\t\t obj->pack_int_id);\n+\n \t\thashwrite_be32(f, perm[obj->pack_int_id]);\n \n \t\tif (large_offset_needed && obj->offset >> 31)\n@@ -771,7 +784,8 @@ static size_t write_midx_large_offsets(struct hashfile *f, uint32_t nr_large_off\n \treturn written;\n }\n \n-int write_midx_file(const char *object_dir)\n+static int write_midx_internal(const char *object_dir, struct multi_pack_index *m,\n+\t\t\t struct string_list *packs_to_drop)\n {\n \tunsigned char cur_chunk, num_chunks = 0;\n \tchar *midx_name;\n@@ -787,6 +801,8 @@ int write_midx_file(const char *object_dir)\n \tstruct pack_midx_entry *entries = NULL;\n \tint large_offsets_needed = 0;\n \tint pack_name_concat_len = 0;\n+\tint dropped_packs = 0;\n+\tint result = 0;\n \n \tmidx_name = get_midx_filename(object_dir);\n \tif (safe_create_leading_directories(midx_name)) {\n@@ -795,7 +811,10 @@ int write_midx_file(const char *object_dir)\n \t\t\t midx_name);\n \t}\n \n-\tpacks.m = load_multi_pack_index(object_dir, 1);\n+\tif (m)\n+\t\tpacks.m = m;\n+\telse\n+\t\tpacks.m = load_multi_pack_index(object_dir, 1);\n \n \tpacks.nr = 0;\n \tpacks.alloc = packs.m ? packs.m->num_packs : 16;\n@@ -809,13 +828,14 @@ int write_midx_file(const char *object_dir)\n \t\t\tpacks.info[packs.nr].orig_pack_int_id = i;\n \t\t\tpacks.info[packs.nr].pack_name = xstrdup(packs.m->pack_names[i]);\n \t\t\tpacks.info[packs.nr].p = NULL;\n+\t\t\tpacks.info[packs.nr].expired = 0;\n \t\t\tpacks.nr++;\n \t\t}\n \t}\n \n \tfor_each_file_in_pack_dir(object_dir, add_pack_to_midx, &packs);\n \n-\tif (packs.m && packs.nr == packs.m->num_packs)\n+\tif (packs.m && packs.nr == packs.m->num_packs && !packs_to_drop)\n \t\tgoto cleanup;\n \n \tentries = get_sorted_entries(packs.m, packs.info, packs.nr, &nr_entries);\n@@ -829,6 +849,34 @@ int write_midx_file(const char *object_dir)\n \n \tQSORT(packs.info, packs.nr, pack_info_compare);\n \n+\tif (packs_to_drop && packs_to_drop->nr) {\n+\t\tint drop_index = 0;\n+\t\tint missing_drops = 0;\n+\n+\t\tfor (i = 0; i < packs.nr && drop_index < packs_to_drop->nr; i++) {\n+\t\t\tint cmp = strcmp(packs.info[i].pack_name,\n+\t\t\t\t\t packs_to_drop->items[drop_index].string);\n+\n+\t\t\tif (!cmp) {\n+\t\t\t\tdrop_index++;\n+\t\t\t\tpacks.info[i].expired = 1;\n+\t\t\t} else if (cmp > 0) {\n+\t\t\t\terror(_(\"did not see pack-file %s to drop\"),\n+\t\t\t\t packs_to_drop->items[drop_index].string);\n+\t\t\t\tdrop_index++;\n+\t\t\t\tmissing_drops++;\n+\t\t\t\ti--;\n+\t\t\t} else {\n+\t\t\t\tpacks.info[i].expired = 0;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (missing_drops) {\n+\t\t\tresult = 1;\n+\t\t\tgoto cleanup;\n+\t\t}\n+\t}\n+\n \t/*\n \t * pack_perm stores a permutation between pack-int-ids from the\n \t * previous multi-pack-index to the new one we are writing:\n@@ -837,11 +885,18 @@ int write_midx_file(const char *object_dir)\n \t */\n \tALLOC_ARRAY(pack_perm, packs.nr);\n \tfor (i = 0; i < packs.nr; i++) {\n-\t\tpack_perm[packs.info[i].orig_pack_int_id] = i;\n+\t\tif (packs.info[i].expired) {\n+\t\t\tdropped_packs++;\n+\t\t\tpack_perm[packs.info[i].orig_pack_int_id] = PACK_EXPIRED;\n+\t\t} else {\n+\t\t\tpack_perm[packs.info[i].orig_pack_int_id] = i - dropped_packs;\n+\t\t}\n \t}\n \n-\tfor (i = 0; i < packs.nr; i++)\n-\t\tpack_name_concat_len += strlen(packs.info[i].pack_name) + 1;\n+\tfor (i = 0; i < packs.nr; i++) {\n+\t\tif (!packs.info[i].expired)\n+\t\t\tpack_name_concat_len += strlen(packs.info[i].pack_name) + 1;\n+\t}\n \n \tif (pack_name_concat_len % MIDX_CHUNK_ALIGNMENT)\n \t\tpack_name_concat_len += MIDX_CHUNK_ALIGNMENT -\n@@ -857,7 +912,7 @@ int write_midx_file(const char *object_dir)\n \tcur_chunk = 0;\n \tnum_chunks = large_offsets_needed ? 5 : 4;\n \n-\twritten = write_midx_header(f, num_chunks, packs.nr);\n+\twritten = write_midx_header(f, num_chunks, packs.nr - dropped_packs);\n \n \tchunk_ids[cur_chunk] = MIDX_CHUNKID_PACKNAMES;\n \tchunk_offsets[cur_chunk] = written + (num_chunks + 1) * MIDX_CHUNKLOOKUP_WIDTH;\n@@ -958,7 +1013,12 @@ int write_midx_file(const char *object_dir)\n \tfree(entries);\n \tfree(pack_perm);\n \tfree(midx_name);\n-\treturn 0;\n+\treturn result;\n+}\n+\n+int write_midx_file(const char *object_dir)\n+{\n+\treturn write_midx_internal(object_dir, NULL, NULL);\n }\n \n void clear_midx_file(struct repository *r)\n@@ -1125,5 +1185,44 @@ int verify_midx_file(struct repository *r, const char *object_dir)\n \n int expire_midx_packs(struct repository *r, const char *object_dir)\n {\n-\treturn 0;\n+\tuint32_t i, *count, result = 0;\n+\tstruct string_list packs_to_drop = STRING_LIST_INIT_DUP;\n+\tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n+\n+\tif (!m)\n+\t\treturn 0;\n+\n+\tcount = xcalloc(m->num_packs, sizeof(uint32_t));\n+\tfor (i = 0; i < m->num_objects; i++) {\n+\t\tint pack_int_id = nth_midxed_pack_int_id(m, i);\n+\t\tcount[pack_int_id]++;\n+\t}\n+\n+\tfor (i = 0; i < m->num_packs; i++) {\n+\t\tchar *pack_name;\n+\n+\t\tif (count[i])\n+\t\t\tcontinue;\n+\n+\t\tif (prepare_midx_pack(r, m, i))\n+\t\t\tcontinue;\n+\n+\t\tif (m->packs[i]->pack_keep)\n+\t\t\tcontinue;\n+\n+\t\tpack_name = xstrdup(m->packs[i]->pack_name);\n+\t\tclose_pack(m->packs[i]);\n+\n+\t\tstring_list_insert(&packs_to_drop, m->pack_names[i]);\n+\t\tunlink_pack_path(pack_name, 0);\n+\t\tfree(pack_name);\n+\t}\n+\n+\tfree(count);\n+\n+\tif (packs_to_drop.nr)\n+\t\tresult = write_midx_internal(object_dir, m, &packs_to_drop);\n+\n+\tstring_list_clear(&packs_to_drop, 0);\n+\treturn result;\n }\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 1b2d32f475..12570fe7ac 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -412,4 +412,24 @@ test_expect_success 'expire does not remove any packs' '\n \t)\n '\n \n+test_expect_success 'expire removes unreferenced packs' '\n+\t(\n+\t\tcd dup &&\n+\t\tgit pack-objects --revs .git/objects/pack/pack-combined <<-EOF &&\n+\t\trefs/heads/A\n+\t\t^refs/heads/C\n+\t\tEOF\n+\t\tgit multi-pack-index write &&\n+\t\tls .git/objects/pack | grep -v -e pack-[AB] >expect &&\n+\t\tgit multi-pack-index expire &&\n+\t\tls .git/objects/pack >actual &&\n+\t\ttest_cmp expect actual &&\n+\t\tls .git/objects/pack/ | grep idx >expect-idx &&\n+\t\ttest-tool read-midx .git/objects | grep idx >actual-midx &&\n+\t\ttest_cmp expect-idx actual-midx &&\n+\t\tgit multi-pack-index verify &&\n+\t\tgit fsck\n+\t)\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"377019","messageId":"xmqqo934klah.fsf@gitster-ct.c.googlers.com","threadId":"51284","inReplyTo":"8213541052a7d040aad4352ee981c0de01cc6516.1560209720.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 06/11] multi-pack-index: implement 'expire' subcommand","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-06-11T18:51:34Z","receivedAt":"2019-06-11T18:51:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> From: Derrick Stolee \n>\n> The 'git multi-pack-index expire' subcommand looks at the existing\n> mult-pack-index, counts the number of objects referenced in each\n\ns/mult/&i/\n\n> pack-file, deletes the pack-fils with no referenced objects, and\n\ns/fils/files/\n\n> rewrites the multi-pack-index to no longer reference those packs.\n\nAs we do *not* want to expire a packfile that was created after the\nlast run of \"git multi-pack-index write\", \"... in each pack-file\" in\nthe above paragraph is a bit tricky. It cannot literally be \"in\neach packfile\", but a subset of all existing packfiles that are\nknown to the midx we are looking at.\n\nAn obvious alternative is to actually update midx file (so it no\nlonger \"looks at the existing multi-pack-index\", but \"makes sure we\nhave up-to-date view of the multi-pack-index that covers all packs).\n\nAnd I actually suspect that you implemented that alternative here.\nIOW, \"looks at the existing multi-pack-index\" in the first paragraph\nis misleading.\n\nI wondered if the error behaviour of add_pack_to_midx() that ignores\na pack that cannot be added to midx for whatever reason can cause\nissues, but such a pack would not become part of m->packs[] so it\nmay not have anything count[]ed from it, but it will not be eligible\nfor expiration, either. So we'd be safe there, I think.\n\n> Test that a new pack-file that covers the contents of two other\n> pack-files leads to those pack-files being deleted during the\n> expire subcommand. Be sure to read the multi-pack-index to ensure\n> it no longer references those packs.\n\nI am curious to learn how we guarantee that the midx logic chooses\nto record the copy of an object in the new single packfile over the\nother copy of the same object in one of the two older and presumably\nsmaller packfiles for all objects in the latter two packfiles,\nbecause they cannot be expired if even one object were used from\nthere.\n"},{"id":"378349","messageId":"d4a3f56b-5eaa-1325-f8c1-be6797a9ac03@kdbg.org","threadId":"51284","inReplyTo":"pull.261.git.gitgitgadget@gmail.com","subject":"[PATCH] t5319: don't trip over a user name with whitespace","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-06-30T18:57:23Z","receivedAt":"2019-06-30T18:59:56Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On my Windows system where the POSIX commands are provided by MSYS2,\nI observe this output:\n\n$ ls -l Makefile\n-rw-r--r-- 1 Johannes Sixt 197121 101780 Jun 30 09:33 Makefile\n\nNotice the blank in the user name. Obviously, extracting the size\nof a file by counting columns won't work. But two tests in t5319\ndo that. Use the stat command to print just the file size.\n\nSigned-off-by: Johannes Sixt \n---\nNotice further that the group ID is presented numerically. The\nactual output used in the tests is:\n\n-r--r--r-- 1 Johannes Sixt 197121 2239 Jan 1 01:03 .git/objects/pack/pack-A-40289997979ee9f2d7198a5b4ac9fe14e27cbe4b.pack\n-r--r--r-- 1 Johannes Sixt 197121 1385 Jan 1 01:02 .git/objects/pack/pack-B-926e20b0e641003b2f6bcddd6ccf53e7741c8642.pack\n-r--r--r-- 1 Johannes Sixt 197121 838 Jan 1 01:01 .git/objects/pack/pack-C-b33c048d7202659249bb0cea7bbc997ce448c75d.pack\n-r--r--r-- 1 Johannes Sixt 197121 560 Jan 1 01:00 .git/objects/pack/pack-D-49ca36507a52bbba6d1768e28666eb0f4e5f7d95.pack\n-r--r--r-- 1 Johannes Sixt 197121 4587 Jun 30 16:17 .git/objects/pack/pack-E-31ea0966bb15206fbfd1b4c2f5be4b24a03050f5.pack\n\nIn both tests, this group ID was passed as --batch-size, but actually\nthe first of the two tests does not fail.\n\nNow I have to wonder: Why that? The test carefully attempts to extract\nthe smallest pack size to pass as --batch-size argument. But when it\npasses some large number, the test still passes. Is this the intent?\n\nThat is, both a small and a large --batch-size satisfy 'does not alter\nexisting packs', but some medium sized --batch-size satisfy the next\ntest 'repack creates a new pack'. That is very curious.\n\nDisclaimer: I've no clue about this multi-pack-index thing.\n\n t/t5319-multi-pack-index.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 79bfaeafa9..4b4d06a1c8 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' '\n \t\ttouch -m -t 201901010002 .git/objects/pack/pack-B* &&\n \t\ttouch -m -t 201901010003 .git/objects/pack/pack-A* &&\n \t\tls .git/objects/pack >expect &&\n-\t\tMINSIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 1) &&\n+\t\tMINSIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 1) &&\n \t\tgit multi-pack-index repack --batch-size=$MINSIZE &&\n \t\tls .git/objects/pack >actual &&\n \t\ttest_cmp expect actual\n@@ -455,7 +455,7 @@ test_expect_success 'repack creates a new pack' '\n \t\tcd dup &&\n \t\tls .git/objects/pack/*idx >idx-list &&\n \t\ttest_line_count = 5 idx-list &&\n-\t\tTHIRD_SMALLEST_SIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 3 | tail -n 1) &&\n+\t\tTHIRD_SMALLEST_SIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 3 | tail -n 1) &&\n \t\tBATCH_SIZE=$(($THIRD_SMALLEST_SIZE + 1)) &&\n \t\tgit multi-pack-index repack --batch-size=$BATCH_SIZE &&\n \t\tls .git/objects/pack/*idx >idx-list &&\n-- \n2.21.0.285.gc38d92e052\n\n"},{"id":"378350","messageId":"CAPig+cRpOHRgtCXXiqAZtmEtkWnMy7=4GsTVv0XB9R3fF0-66A@mail.gmail.com","threadId":"51284","inReplyTo":"d4a3f56b-5eaa-1325-f8c1-be6797a9ac03@kdbg.org","subject":"Re: [PATCH] t5319: don't trip over a user name with whitespace","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-06-30T19:48:57Z","receivedAt":"2019-06-30T19:55:16Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jun 30, 2019 at 2:57 PM Johannes Sixt wrote:\n> On my Windows system where the POSIX commands are provided by MSYS2,\n> I observe this output:\n>\n> $ ls -l Makefile\n> -rw-r--r-- 1 Johannes Sixt 197121 101780 Jun 30 09:33 Makefile\n>\n> Notice the blank in the user name. Obviously, extracting the size\n> of a file by counting columns won't work. But two tests in t5319\n> do that. Use the stat command to print just the file size.\n>\n> Signed-off-by: Johannes Sixt \n> ---\n> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\n> @@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' '\n> - MINSIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 1) &&\n> + MINSIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 1) &&\n\nUnfortunately, this is not portable. While \"stat -c %s\" works on Linux\nand MSYS2, neither that option nor the format directive are recognized\non BSD-like platforms (I tested Mac OS and FreeBSD), which instead\nneed \"stat -f %z\".\n"},{"id":"378351","messageId":"1a5bb5f9-e090-7f42-ec3f-cd74bcad831f@kdbg.org","threadId":"51284","inReplyTo":"CAPig+cRpOHRgtCXXiqAZtmEtkWnMy7=4GsTVv0XB9R3fF0-66A@mail.gmail.com","subject":"Re: [PATCH] t5319: don't trip over a user name with whitespace","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-06-30T20:59:34Z","receivedAt":"2019-06-30T20:59:39Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 30.06.19 um 21:48 schrieb Eric Sunshine:\n> On Sun, Jun 30, 2019 at 2:57 PM Johannes Sixt wrote:\n>> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\n>> @@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' '\n>> - MINSIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 1) &&\n>> + MINSIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 1) &&\n> \n> Unfortunately, this is not portable. While \"stat -c %s\" works on Linux\n> and MSYS2, neither that option nor the format directive are recognized\n> on BSD-like platforms (I tested Mac OS and FreeBSD), which instead\n> need \"stat -f %z\".\n\nOuch! I did notice that stat(1) is not in POSIX, but hoped that it was\nsufficiently portable. I need a new idea...\n\n-- Hannes\n"},{"id":"378352","messageId":"20190630222537.GA21696@sigill.intra.peff.net","threadId":"51284","inReplyTo":"1a5bb5f9-e090-7f42-ec3f-cd74bcad831f@kdbg.org","subject":"Re: [PATCH] t5319: don't trip over a user name with whitespace","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-06-30T22:25:37Z","receivedAt":"2019-06-30T22:25:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 30, 2019 at 10:59:34PM +0200, Johannes Sixt wrote:\n\n> Am 30.06.19 um 21:48 schrieb Eric Sunshine:\n> > On Sun, Jun 30, 2019 at 2:57 PM Johannes Sixt wrote:\n> >> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\n> >> @@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' '\n> >> - MINSIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 1) &&\n> >> + MINSIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 1) &&\n> > \n> > Unfortunately, this is not portable. While \"stat -c %s\" works on Linux\n> > and MSYS2, neither that option nor the format directive are recognized\n> > on BSD-like platforms (I tested Mac OS and FreeBSD), which instead\n> > need \"stat -f %z\".\n> \n> Ouch! I did notice that stat(1) is not in POSIX, but hoped that it was\n> sufficiently portable. I need a new idea...\n\nIf we are OK relying on rudimentary perl[1], then:\n\n perl -le \"print((stat)[7]) for @ARGV\"\n\nworks. If you want it more readable, then maybe:\n\n perl -MFile::stat -le \"print stat(\\$_)->size for @ARGV\"\n\nBoth of those should work with even antique perl versions.\n\n-Peff\n\n[1] I'd also be fine with implementing a test-tool helper in C that\n behaves like stat(1). Or we could punt on that until somebody feels\n like trying to eradicate perl (because this is far from the first\n perl one-liner in the test suite).\n"},{"id":"378357","messageId":"3e5c0286-7514-9e00-fbc5-5b0ad9403d30@kdbg.org","threadId":"51284","inReplyTo":"20190630222537.GA21696@sigill.intra.peff.net","subject":"Re: [PATCH] t5319: don't trip over a user name with whitespace","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-07-01T06:33:18Z","receivedAt":"2019-07-01T06:33:23Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 01.07.19 um 00:25 schrieb Jeff King:\n> On Sun, Jun 30, 2019 at 10:59:34PM +0200, Johannes Sixt wrote:\n> \n>> Am 30.06.19 um 21:48 schrieb Eric Sunshine:\n>>> On Sun, Jun 30, 2019 at 2:57 PM Johannes Sixt wrote:\n>>>> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\n>>>> @@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' '\n>>>> - MINSIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 1) &&\n>>>> + MINSIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 1) &&\n>>>\n>>> Unfortunately, this is not portable. While \"stat -c %s\" works on Linux\n>>> and MSYS2, neither that option nor the format directive are recognized\n>>> on BSD-like platforms (I tested Mac OS and FreeBSD), which instead\n>>> need \"stat -f %z\".\n>>\n>> Ouch! I did notice that stat(1) is not in POSIX, but hoped that it was\n>> sufficiently portable. I need a new idea...\n> \n> If we are OK relying on rudimentary perl[1], then:\n> \n> perl -le \"print((stat)[7]) for @ARGV\"\n\nI'm fine with that. But then we should do the sort + head -n 1 at the\nsame time. I can do that with a small script, but I'm sure it's possible\nin a one-liner...\n\n-- Hannes\n"},{"id":"378358","messageId":"20190701083601.GJ21574@szeder.dev","threadId":"51284","inReplyTo":"1a5bb5f9-e090-7f42-ec3f-cd74bcad831f@kdbg.org","subject":"Re: [PATCH] t5319: don't trip over a user name with whitespace","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-07-01T08:36:01Z","receivedAt":"2019-07-01T08:36:08Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sun, Jun 30, 2019 at 10:59:34PM +0200, Johannes Sixt wrote:\n> Am 30.06.19 um 21:48 schrieb Eric Sunshine:\n> > On Sun, Jun 30, 2019 at 2:57 PM Johannes Sixt wrote:\n> >> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\n> >> @@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' '\n> >> - MINSIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 1) &&\n> >> + MINSIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 1) &&\n> > \n> > Unfortunately, this is not portable. While \"stat -c %s\" works on Linux\n> > and MSYS2, neither that option nor the format directive are recognized\n> > on BSD-like platforms (I tested Mac OS and FreeBSD), which instead\n> > need \"stat -f %z\".\n> \n> Ouch! I did notice that stat(1) is not in POSIX, but hoped that it was\n> sufficiently portable. I need a new idea...\n\n'wc -c' perhaps? We already use it in a couple of places in the test\nsuite to get file size.\n\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 79bfaeafa9..ddba862114 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' '\n \t\ttouch -m -t 201901010002 .git/objects/pack/pack-B* &&\n \t\ttouch -m -t 201901010003 .git/objects/pack/pack-A* &&\n \t\tls .git/objects/pack >expect &&\n-\t\tMINSIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 1) &&\n+\t\tMINSIZE=$(wc -c .git/objects/pack/*pack | sort -n | sed -n -e \"s/^ *//\" -e \"1 s/[^ ]*$//p\") &&\n \t\tgit multi-pack-index repack --batch-size=$MINSIZE &&\n \t\tls .git/objects/pack >actual &&\n \t\ttest_cmp expect actual\n@@ -455,7 +455,7 @@ test_expect_success 'repack creates a new pack' '\n \t\tcd dup &&\n \t\tls .git/objects/pack/*idx >idx-list &&\n \t\ttest_line_count = 5 idx-list &&\n-\t\tTHIRD_SMALLEST_SIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 3 | tail -n 1) &&\n+\t\tTHIRD_SMALLEST_SIZE=$(wc -c .git/objects/pack/*pack | sort -n | sed -n -e \"s/^ *//\" -e \"3 s/[^ ]*$//p\") &&\n \t\tBATCH_SIZE=$(($THIRD_SMALLEST_SIZE + 1)) &&\n \t\tgit multi-pack-index repack --batch-size=$BATCH_SIZE &&\n \t\tls .git/objects/pack/*idx >idx-list &&\n\nNote that 'wc -c' prints the numbers in its output left padded, hence\nthe 'sed \"s/^ *//\"' above, and once 'sed' is already up and running,\nit might as well take over the role of that 'head -n 3 | tail -n 1'.\n\n"},{"id":"378359","messageId":"20190701091602.GA24791@sigill.intra.peff.net","threadId":"51284","inReplyTo":"3e5c0286-7514-9e00-fbc5-5b0ad9403d30@kdbg.org","subject":"Re: [PATCH] t5319: don't trip over a user name with whitespace","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-07-01T09:16:02Z","receivedAt":"2019-07-01T09:16:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 01, 2019 at 08:33:18AM +0200, Johannes Sixt wrote:\n\n> > If we are OK relying on rudimentary perl[1], then:\n> > \n> > perl -le \"print((stat)[7]) for @ARGV\"\n> \n> I'm fine with that. But then we should do the sort + head -n 1 at the\n> same time. I can do that with a small script, but I'm sure it's possible\n> in a one-liner...\n\nSomething like:\n\n perl -le 'print((sort { $a <=> $b } map { (stat)[7] } @ARGV)[0])'\n\nbut that is getting pretty unreadable. If we rely on List::Util, it's a\nbit nicer:\n\n perl -MList::Util=min -e 'print min(map { (stat)[7] } @ARGV)'\n\nbut that implies perl v5.7.3. Which is from 2002, and older than our\nusual minimum-perl version, but we've typically been very conservative\nwith these one-liners, since they do not respect NO_PERL.\n\nProbably writing it out like:\n\n perl -le '\n my @sizes = map { (stat)[7] } @ARGV;\n @sizes = sort { $a <=> $b } @sizes;\n print $size[0];\n '\n\nwould be better (and makes \"3rd-smallest\" a trivial change).\n\nI see Gábor suggested using \"wc -c\" elsewhere in the thread. That would\nbe fine with me, too, though I think the required sed there may be\ngetting pretty unreadable, too. :)\n\n-Peff\n"},{"id":"378360","messageId":"20190701113304.GK21574@szeder.dev","threadId":"51284","inReplyTo":"20190701091602.GA24791@sigill.intra.peff.net","subject":"Re: [PATCH] t5319: don't trip over a user name with whitespace","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-07-01T11:33:04Z","receivedAt":"2019-07-01T11:33:10Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Jul 01, 2019 at 05:16:02AM -0400, Jeff King wrote:\n> I see Gábor suggested using \"wc -c\" elsewhere in the thread. That would\n> be fine with me, too, though I think the required sed there may be\n> getting pretty unreadable, too. :)\n\nIt could be done even without 'sed', though at the expense of running\na coupe more 'wc -c's in a loop:\n\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 79bfaeafa9..bacec5e2e4 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -443,7 +443,12 @@ test_expect_success 'repack with minimum size does not alter existing packs' '\n \t\ttouch -m -t 201901010002 .git/objects/pack/pack-B* &&\n \t\ttouch -m -t 201901010003 .git/objects/pack/pack-A* &&\n \t\tls .git/objects/pack >expect &&\n-\t\tMINSIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 1) &&\n+\t\tMINSIZE=$(\n+\t\t\tfor pack in .git/objects/pack/*pack\n+\t\t\tdo\n+\t\t\t\twc -c <\"$pack\"\n+\t\t\tdone | sort -n | head -n 1\n+\t\t) &&\n \t\tgit multi-pack-index repack --batch-size=$MINSIZE &&\n \t\tls .git/objects/pack >actual &&\n \t\ttest_cmp expect actual\n@@ -455,7 +460,12 @@ test_expect_success 'repack creates a new pack' '\n \t\tcd dup &&\n \t\tls .git/objects/pack/*idx >idx-list &&\n \t\ttest_line_count = 5 idx-list &&\n-\t\tTHIRD_SMALLEST_SIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 3 | tail -n 1) &&\n+\t\tTHIRD_SMALLEST_SIZE=$(\n+\t\t\tfor pack in .git/objects/pack/*pack\n+\t\t\tdo\n+\t\t\t\twc -c <\"$pack\"\n+\t\t\tdone | sort -n | head -n 3 | tail -n 1\n+\t\t) &&\n \t\tBATCH_SIZE=$(($THIRD_SMALLEST_SIZE + 1)) &&\n \t\tgit multi-pack-index repack --batch-size=$BATCH_SIZE &&\n \t\tls .git/objects/pack/*idx >idx-list &&\n\nIs it really better? Dunno, but at least there is no subtlety with\nthe leading padding spaces.\n\n"},{"id":"378366","messageId":"100c5c85-59b6-f68c-7da6-b05637534858@gmail.com","threadId":"51284","inReplyTo":"20190701113304.GK21574@szeder.dev","subject":"Re: [PATCH] t5319: don't trip over a user name with whitespace","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-07-01T12:03:15Z","receivedAt":"2019-07-01T12:03:19Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"Sorry I'm late to the thread. Thanks for reporting this issue, Johannes.\n\nOn 7/1/2019 7:33 AM, SZEDER Gábor wrote:\n> On Mon, Jul 01, 2019 at 05:16:02AM -0400, Jeff King wrote:\n>> I see Gábor suggested using \"wc -c\" elsewhere in the thread. That would\n>> be fine with me, too, though I think the required sed there may be\n>> getting pretty unreadable, too. :)\n> \n> It could be done even without 'sed', though at the expense of running\n> a coupe more 'wc -c's in a loop:>\n> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\n> index 79bfaeafa9..bacec5e2e4 100755\n> --- a/t/t5319-multi-pack-index.sh\n> +++ b/t/t5319-multi-pack-index.sh\n> @@ -443,7 +443,12 @@ test_expect_success 'repack with minimum size does not alter existing packs' '\n> \t\ttouch -m -t 201901010002 .git/objects/pack/pack-B* &&\n> \t\ttouch -m -t 201901010003 .git/objects/pack/pack-A* &&\n> \t\tls .git/objects/pack >expect &&\n> -\t\tMINSIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 1) &&\n> +\t\tMINSIZE=$(\n> +\t\t\tfor pack in .git/objects/pack/*pack\n> +\t\t\tdo\n> +\t\t\t\twc -c <\"$pack\"\n> +\t\t\tdone | sort -n | head -n 1\n> +\t\t) &&\n> \t\tgit multi-pack-index repack --batch-size=$MINSIZE &&\n> \t\tls .git/objects/pack >actual &&\n> \t\ttest_cmp expect actual\n> @@ -455,7 +460,12 @@ test_expect_success 'repack creates a new pack' '\n> \t\tcd dup &&\n> \t\tls .git/objects/pack/*idx >idx-list &&\n> \t\ttest_line_count = 5 idx-list &&\n> -\t\tTHIRD_SMALLEST_SIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 3 | tail -n 1) &&\n> +\t\tTHIRD_SMALLEST_SIZE=$(\n> +\t\t\tfor pack in .git/objects/pack/*pack\n> +\t\t\tdo\n> +\t\t\t\twc -c <\"$pack\"\n> +\t\t\tdone | sort -n | head -n 3 | tail -n 1\n> +\t\t) &&\n> \t\tBATCH_SIZE=$(($THIRD_SMALLEST_SIZE + 1)) &&\n> \t\tgit multi-pack-index repack --batch-size=$BATCH_SIZE &&\n> \t\tls .git/objects/pack/*idx >idx-list &&\n> \n> Is it really better? Dunno, but at least there is no subtlety with\n> the leading padding spaces.\n\nYour change is certainly more straight-forward, and avoids all issues\naround Perl compatibility.\n\nIt does have the issue of more 'wc' processes, which is a downside. The\ncount here is not too bad, but if we need to duplicate this pattern elsewhere\nwe may be better off creating a stat(1) replacement in test-lib.sh.\n\nThanks,\n-Stolee\n"},{"id":"378367","messageId":"nycvar.QRO.7.76.6.1907011409420.44@tvgsbejvaqbjf.bet","threadId":"51284","inReplyTo":"20190630222537.GA21696@sigill.intra.peff.net","subject":"Re: [PATCH] t5319: don't trip over a user name with whitespace","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-07-01T12:11:43Z","receivedAt":"2019-07-01T12:11:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Sun, 30 Jun 2019, Jeff King wrote:\n\n> On Sun, Jun 30, 2019 at 10:59:34PM +0200, Johannes Sixt wrote:\n>\n> > Am 30.06.19 um 21:48 schrieb Eric Sunshine:\n> > > On Sun, Jun 30, 2019 at 2:57 PM Johannes Sixt wrote:\n> > >> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\n> > >> @@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' '\n> > >> - MINSIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 1) &&\n> > >> + MINSIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 1) &&\n> > >\n> > > Unfortunately, this is not portable. While \"stat -c %s\" works on Linux\n> > > and MSYS2, neither that option nor the format directive are recognized\n> > > on BSD-like platforms (I tested Mac OS and FreeBSD), which instead\n> > > need \"stat -f %z\".\n> >\n> > Ouch! I did notice that stat(1) is not in POSIX, but hoped that it was\n> > sufficiently portable. I need a new idea...\n>\n> If we are OK relying on rudimentary perl[1], then:\n>\n> perl -le \"print((stat)[7]) for @ARGV\"\n>\n> works. If you want it more readable, then maybe:\n>\n> perl -MFile::stat -le \"print stat(\\$_)->size for @ARGV\"\n\nOr we stop introducing new Perl calls, and use the perfectly fine\n`test-tool path-utils file-size` command:\n\nhttps://github.com/git/git/blob/v2.22.0/t/helper/test-path-utils.c#L302-L312\n\nThis solves not only portability problems but also avoids yet another\nobstacle into making a `NO_PERL` test suite run really work without Perl.\n\nCiao,\nDscho\n"},{"id":"378372","messageId":"b1f6a752-6edd-d86c-7d69-7624a2c5407f@gmail.com","threadId":"51284","inReplyTo":"nycvar.QRO.7.76.6.1907011409420.44@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] t5319: don't trip over a user name with whitespace","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-07-01T12:30:56Z","receivedAt":"2019-07-01T12:31:00Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/1/2019 8:11 AM, Johannes Schindelin wrote:\n> Hi Peff,\n> \n> On Sun, 30 Jun 2019, Jeff King wrote:\n> \n>> On Sun, Jun 30, 2019 at 10:59:34PM +0200, Johannes Sixt wrote:\n>>\n>>> Am 30.06.19 um 21:48 schrieb Eric Sunshine:\n>>>> On Sun, Jun 30, 2019 at 2:57 PM Johannes Sixt wrote:\n>>>>> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\n>>>>> @@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' '\n>>>>> - MINSIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 1) &&\n>>>>> + MINSIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 1) &&\n>>>>\n>>>> Unfortunately, this is not portable. While \"stat -c %s\" works on Linux\n>>>> and MSYS2, neither that option nor the format directive are recognized\n>>>> on BSD-like platforms (I tested Mac OS and FreeBSD), which instead\n>>>> need \"stat -f %z\".\n>>>\n>>> Ouch! I did notice that stat(1) is not in POSIX, but hoped that it was\n>>> sufficiently portable. I need a new idea...\n>>\n>> If we are OK relying on rudimentary perl[1], then:\n>>\n>> perl -le \"print((stat)[7]) for @ARGV\"\n>>\n>> works. If you want it more readable, then maybe:\n>>\n>> perl -MFile::stat -le \"print stat(\\$_)->size for @ARGV\"\n> \n> Or we stop introducing new Perl calls, and use the perfectly fine\n> `test-tool path-utils file-size` command:\n> \n> https://github.com/git/git/blob/v2.22.0/t/helper/test-path-utils.c#L302-L312\n> \n> This solves not only portability problems but also avoids yet another\n> obstacle into making a `NO_PERL` test suite run really work without Perl.\nThanks! This does seem like the best option. Thanks for bringing this to our\nattention. Here is a diff, and I'll prepare a full patch:\n\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex dd6083e61a2..5379e59168a 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -447,7 +447,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' '\n touch -m -t 201901010002 .git/objects/pack/pack-B* &&\n touch -m -t 201901010003 .git/objects/pack/pack-A* &&\n ls .git/objects/pack >expect &&\n- MINSIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 1) &&\n+ MINSIZE=$(test-tool path-utils file-size .git/objects/pack/*pack | sort -n | head -n 1) &&\n git multi-pack-index repack --batch-size=$MINSIZE &&\n ls .git/objects/pack >actual &&\n test_cmp expect actual\n@@ -459,7 +459,7 @@ test_expect_success 'repack creates a new pack' '\n cd dup &&\n ls .git/objects/pack/*idx >idx-list &&\n test_line_count = 5 idx-list &&\n- THIRD_SMALLEST_SIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 3 | tail -n 1) &&\n+ THIRD_SMALLEST_SIZE=$(test-tool path-utils file-size .git/objects/pack/*pack | sort -n | head -n 3 | tail -n 1) &&\n BATCH_SIZE=$(($THIRD_SMALLEST_SIZE + 1)) &&\n git multi-pack-index repack --batch-size=$BATCH_SIZE &&\n ls .git/objects/pack/*idx >idx-list &&\n\nThanks,\n-Stolee\n"},{"id":"378375","messageId":"20190701125337.GA4704@sigill.intra.peff.net","threadId":"51284","inReplyTo":"nycvar.QRO.7.76.6.1907011409420.44@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] t5319: don't trip over a user name with whitespace","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-07-01T12:53:37Z","receivedAt":"2019-07-01T12:53:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 01, 2019 at 02:11:43PM +0200, Johannes Schindelin wrote:\n\n> Or we stop introducing new Perl calls, and use the perfectly fine\n> `test-tool path-utils file-size` command:\n> \n> https://github.com/git/git/blob/v2.22.0/t/helper/test-path-utils.c#L302-L312\n\nAh, thanks, I missed that we had already added it. Certainly that seems\nlike the right solution then.\n\n-Peff\n"},{"id":"378415","messageId":"87muhxr7xv.fsf@igel.home","threadId":"51284","inReplyTo":"d4a3f56b-5eaa-1325-f8c1-be6797a9ac03@kdbg.org","subject":"Re: [PATCH] t5319: don't trip over a user name with whitespace","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2019-07-01T17:17:00Z","receivedAt":"2019-07-01T17:17:06Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"On Jun 30 2019, Johannes Sixt wrote:\n\n> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\n> index 79bfaeafa9..4b4d06a1c8 100755\n> --- a/t/t5319-multi-pack-index.sh\n> +++ b/t/t5319-multi-pack-index.sh\n> @@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' '\n> \t\ttouch -m -t 201901010002 .git/objects/pack/pack-B* &&\n> \t\ttouch -m -t 201901010003 .git/objects/pack/pack-A* &&\n> \t\tls .git/objects/pack >expect &&\n> -\t\tMINSIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 1) &&\n> +\t\tMINSIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 1) &&\n\nYou could also use ls -n.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1\n\"And now for something completely different.\"\n"},{"id":"378421","messageId":"3645e8cf-3538-7a4e-a54a-f58dc39510e1@kdbg.org","threadId":"51284","inReplyTo":"b1f6a752-6edd-d86c-7d69-7624a2c5407f@gmail.com","subject":"Re: [PATCH] t5319: don't trip over a user name with whitespace","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-07-01T18:22:46Z","receivedAt":"2019-07-01T18:22:50Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 01.07.19 um 14:30 schrieb Derrick Stolee:\n> On 7/1/2019 8:11 AM, Johannes Schindelin wrote:\n>> Or we stop introducing new Perl calls, and use the perfectly fine\n>> `test-tool path-utils file-size` command:\n>>\n>> https://github.com/git/git/blob/v2.22.0/t/helper/test-path-utils.c#L302-L312\n>>\n>> This solves not only portability problems but also avoids yet another\n>> obstacle into making a `NO_PERL` test suite run really work without Perl.\n> Thanks! This does seem like the best option. Thanks for bringing this to our\n> attention. Here is a diff, and I'll prepare a full patch:\n\nThanks. Please also explain why the first of the two tests does not fail\nwith a large --batch-size (unless it is obvious for people who know a\nbit about multi-pack-index, of course).\n\n-- Hannes\n"},{"id":"378423","messageId":"f42b441e-b6ff-464a-6351-739993472ca6@gmail.com","threadId":"51284","inReplyTo":"3645e8cf-3538-7a4e-a54a-f58dc39510e1@kdbg.org","subject":"Re: [PATCH] t5319: don't trip over a user name with whitespace","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-07-01T18:47:37Z","receivedAt":"2019-07-01T18:47:41Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/1/2019 2:22 PM, Johannes Sixt wrote:\n> Am 01.07.19 um 14:30 schrieb Derrick Stolee:\n>> On 7/1/2019 8:11 AM, Johannes Schindelin wrote:\n>>> Or we stop introducing new Perl calls, and use the perfectly fine\n>>> `test-tool path-utils file-size` command:\n>>>\n>>> https://github.com/git/git/blob/v2.22.0/t/helper/test-path-utils.c#L302-L312\n>>>\n>>> This solves not only portability problems but also avoids yet another\n>>> obstacle into making a `NO_PERL` test suite run really work without Perl.\n>> Thanks! This does seem like the best option. Thanks for bringing this to our\n>> attention. Here is a diff, and I'll prepare a full patch:\n> \n> Thanks. Please also explain why the first of the two tests does not fail\n> with a large --batch-size (unless it is obvious for people who know a\n> bit about multi-pack-index, of course).\n\nSorry about missing that. Here is an explanation:\n\nThe --batch-size=X option determines how much data we want to move in\na single operation. Based on this value, we select packs greedily as\nfollows:\n\n 1. If a pack has size at most X, include it.\n 2. If our total pack size is greater than X, stop.\n\nAt the end of this process, if we have zero or one packs we do nothing.\nThat is why a small size does nothing.\n\nAlso, if our total pack size is smaller than X, we do nothing. Our\nbatch is not \"full\" enough to merit the work. This is why a large size\ndoes nothing.\n\nThanks,\n-Stolee\n"},{"id":"378427","messageId":"xmqqtvc55zi3.fsf@gitster-ct.c.googlers.com","threadId":"51284","inReplyTo":"87muhxr7xv.fsf@igel.home","subject":"Re: [PATCH] t5319: don't trip over a user name with whitespace","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-07-01T19:24:52Z","receivedAt":"2019-07-01T19:24:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Schwab writes:\n\n> On Jun 30 2019, Johannes Sixt wrote:\n>\n>> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\n>> index 79bfaeafa9..4b4d06a1c8 100755\n>> --- a/t/t5319-multi-pack-index.sh\n>> +++ b/t/t5319-multi-pack-index.sh\n>> @@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' '\n>> \t\ttouch -m -t 201901010002 .git/objects/pack/pack-B* &&\n>> \t\ttouch -m -t 201901010003 .git/objects/pack/pack-A* &&\n>> \t\tls .git/objects/pack >expect &&\n>> -\t\tMINSIZE=$(ls -l .git/objects/pack/*pack | awk \"{print \\$5;}\" | sort -n | head -n 1) &&\n>> +\t\tMINSIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 1) &&\n>\n> You could also use ls -n.\n\nNice ;-)\n"}]}