{"thread":{"id":"57109","subject":"[PATCH 0/2] Two small 'git repack' fixes","startedAt":"2021-12-17T16:28:50Z","lastAt":"2021-12-20T19:04:02Z","messageCount":17,"participants":["Derrick Stolee via GitGitGadget","Jeff King","Ævar Arnfjörð Bjarmason","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"444375","messageId":"pull.1098.git.1639758526.gitgitgadget@gmail.com","threadId":"57109","inReplyTo":null,"subject":"[PATCH 0/2] Two small 'git repack' fixes","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-17T16:28:44Z","receivedAt":"2021-12-17T16:28:50Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"I was experimenting with some ideas in 'git repack' and discovered these two\nbugs.\n\nThe first is a \"real\" bug in that it repacks much more data than is\nnecessary when repacking with '--write-midx -b' and there exists a .keep\npack. The fix is simple, which is to change a condition that was added for\nthe '-b' case before '--write-midx' existed.\n\nThe second is a UX bug in that '--quiet' did not disable all progress, at\nleast when stderr was interactive.\n\nThanks, -Stolee\n\nDerrick Stolee (2):\n repack: respect kept objects with '--write-midx -b'\n repack: make '--quiet' disable progress\n\n builtin/repack.c | 8 +++++---\n t/t7700-repack.sh | 15 +++++++++++++++\n 2 files changed, 20 insertions(+), 3 deletions(-)\n\n\nbase-commit: 69a9c10c95e28df457e33b3c7400b16caf2e2962\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1098%2Fderrickstolee%2Frepack-fixes-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1098/derrickstolee/repack-fixes-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1098\n-- \ngitgitgadget\n"},{"id":"444376","messageId":"1ed91f6d255b76bdbdcccea7e1effcebbb263ced.1639758526.git.gitgitgadget@gmail.com","threadId":"57109","inReplyTo":"pull.1098.git.1639758526.gitgitgadget@gmail.com","subject":"[PATCH 1/2] repack: respect kept objects with '--write-midx -b'","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-17T16:28:45Z","receivedAt":"2021-12-17T16:28:52Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nHistorically, we needed a single packfile in order to have reachability\nbitmaps. This introduced logic that when 'git repack' had a '-b' option\nthat we should stop sending the '--honor-pack-keep' option to the 'git\npack-objects' child process, ensuring that we create a packfile\ncontaining all reachable objects.\n\nIn the world of multi-pack-index bitmaps, we no longer need to repack\nall objects into a single pack to have valid bitmaps. Thus, we should\ncontinue sending the '--honor-pack-keep' flag to 'git pack-objects'.\n\nThe fix is very simple: only disable the flag when writing bitmaps but\nalso _not_ writing the multi-pack-index.\n\nThis opens the door to new repacking strategies that might want to keep\nsome historical set of objects in a stable pack-file while only\nrepacking more recent objects.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/repack.c | 2 +-\n t/t7700-repack.sh | 15 +++++++++++++++\n 2 files changed, 16 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex 9b0be6a6ab3..1f128b7c90b 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -693,7 +693,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n \t\twrite_bitmaps = 0;\n \t}\n \tif (pack_kept_objects < 0)\n-\t\tpack_kept_objects = write_bitmaps > 0;\n+\t\tpack_kept_objects = write_bitmaps > 0 && !write_midx;\n \n \tif (write_bitmaps && !(pack_everything & ALL_INTO_ONE) && !write_midx)\n \t\tdie(_(incremental_bitmap_conflict_error));\ndiff --git a/t/t7700-repack.sh b/t/t7700-repack.sh\nindex 0260ad6f0e0..8c4ba6500be 100755\n--- a/t/t7700-repack.sh\n+++ b/t/t7700-repack.sh\n@@ -372,4 +372,19 @@ test_expect_success '--write-midx with preferred bitmap tips' '\n \t)\n '\n \n+test_expect_success '--write-midx -b packs non-kept objects' '\n+\tgit init midx-kept &&\n+\ttest_when_finished \"rm -fr midx-kept\" &&\n+\t(\n+\t\tcd midx-kept &&\n+\t\ttest_commit_bulk 100 &&\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/trace.txt\" \\\n+\t\t\tgit repack --write-midx -a -b &&\n+\t\tcat trace.txt | \\\n+\t\t\tgrep \\\"event\\\":\\\"start\\\" | \\\n+\t\t\tgrep pack-objects | \\\n+\t\t\tgrep \\\"--honor-pack-keep\\\"\n+\t)\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"444377","messageId":"3eff83d9ae14023f3527dfeb419cf8259f6d053d.1639758526.git.gitgitgadget@gmail.com","threadId":"57109","inReplyTo":"pull.1098.git.1639758526.gitgitgadget@gmail.com","subject":"[PATCH 2/2] repack: make '--quiet' disable progress","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-17T16:28:46Z","receivedAt":"2021-12-17T16:28:52Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhile testing some ideas in 'git repack', I ran it with '--quiet' and\ndiscovered that some progress output was still shown. Specifically, the\noutput for writing the multi-pack-index showed the progress.\n\nThe 'show_progress' variable in cmd_repack() is initialized with\nisatty(2) and is not modified at all by the '--quiet' flag. The\n'--quiet' flag modifies the po_args.quiet option which is translated\ninto a '--quiet' flag for the 'git pack-objects' child process. However,\n'show_progress' is used to directly send progress information to the\nmulti-pack-index writing logic which does not use a child process.\n\nThe fix here is to modify 'show_progress' to be false if po_opts.quiet\nis true, and isatty(2) otherwise. This new expectation simplifies a\nlater condition that checks both.\n\nThis is difficult to test because the isatty(2) already prevents the\nprogess indicators from appearing when we redirect stderr to a file.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/repack.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex 1f128b7c90b..c393a5db774 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -612,7 +612,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n \tstruct tempfile *refs_snapshot = NULL;\n \tint i, ext, ret;\n \tFILE *out;\n-\tint show_progress = isatty(2);\n+\tint show_progress;\n \n \t/* variables to be filled by option parsing */\n \tint pack_everything = 0;\n@@ -725,6 +725,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n \n \tprepare_pack_objects(&cmd, &po_args);\n \n+\tshow_progress = !po_args.quiet && isatty(2);\n+\n \tstrvec_push(&cmd.args, \"--keep-true-parents\");\n \tif (!pack_kept_objects)\n \t\tstrvec_push(&cmd.args, \"--honor-pack-keep\");\n@@ -926,7 +928,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n \t\t\t}\n \t\t\tstrbuf_release(&buf);\n \t\t}\n-\t\tif (!po_args.quiet && show_progress)\n+\t\tif (show_progress)\n \t\t\topts |= PRUNE_PACKED_VERBOSE;\n \t\tprune_packed_objects(opts);\n \n-- \ngitgitgadget\n"},{"id":"444382","messageId":"YbzHtsl045XZbJGN@coredump.intra.peff.net","threadId":"57109","inReplyTo":"1ed91f6d255b76bdbdcccea7e1effcebbb263ced.1639758526.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] repack: respect kept objects with '--write-midx -b'","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-12-17T17:24:06Z","receivedAt":"2021-12-17T17:24:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 17, 2021 at 04:28:45PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n> From: Derrick Stolee \n> \n> Historically, we needed a single packfile in order to have reachability\n> bitmaps. This introduced logic that when 'git repack' had a '-b' option\n> that we should stop sending the '--honor-pack-keep' option to the 'git\n> pack-objects' child process, ensuring that we create a packfile\n> containing all reachable objects.\n> \n> In the world of multi-pack-index bitmaps, we no longer need to repack\n> all objects into a single pack to have valid bitmaps. Thus, we should\n> continue sending the '--honor-pack-keep' flag to 'git pack-objects'.\n> \n> The fix is very simple: only disable the flag when writing bitmaps but\n> also _not_ writing the multi-pack-index.\n> \n> This opens the door to new repacking strategies that might want to keep\n> some historical set of objects in a stable pack-file while only\n> repacking more recent objects.\n\nThat all makes sense. Another way of thinking about it: we disable\n--honor-pack-keep so we that keep packs do not interfere with writing a\npack bitmap. But with --write-midx, we skip the pack bitmap entirely.\n\nIn the end it's the same, but I wanted to emphasize that the important\nhing is not so much that we are writing a midx bitmap as that we are\n_not_ writing a pack bitmap. And that is what makes this OK to do (that\nthe repack code already disables the pack bitmap when writing a midx\none).\n\n> diff --git a/builtin/repack.c b/builtin/repack.c\n> index 9b0be6a6ab3..1f128b7c90b 100644\n> --- a/builtin/repack.c\n> +++ b/builtin/repack.c\n> @@ -693,7 +693,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n> \t\twrite_bitmaps = 0;\n> \t}\n> \tif (pack_kept_objects < 0)\n> -\t\tpack_kept_objects = write_bitmaps > 0;\n> +\t\tpack_kept_objects = write_bitmaps > 0 && !write_midx;\n\nSo the code change here looks good.\n\n> diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh\n> index 0260ad6f0e0..8c4ba6500be 100755\n> --- a/t/t7700-repack.sh\n> +++ b/t/t7700-repack.sh\n> @@ -372,4 +372,19 @@ test_expect_success '--write-midx with preferred bitmap tips' '\n> \t)\n> '\n> \n> +test_expect_success '--write-midx -b packs non-kept objects' '\n> +\tgit init midx-kept &&\n> +\ttest_when_finished \"rm -fr midx-kept\" &&\n> +\t(\n> +\t\tcd midx-kept &&\n> +\t\ttest_commit_bulk 100 &&\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/trace.txt\" \\\n> +\t\t\tgit repack --write-midx -a -b &&\n> +\t\tcat trace.txt | \\\n> +\t\t\tgrep \\\"event\\\":\\\"start\\\" | \\\n> +\t\t\tgrep pack-objects | \\\n> +\t\t\tgrep \\\"--honor-pack-keep\\\"\n> +\t)\n> +'\n\nThis looks correct, though:\n\n - do we really need this separate repo directory? The test before it\n uses one, but only because it needs a very specific set of commits.\n There is a long-running \"midx\" directory we could use, though I\n think just the regular test repo would be fine, too.\n\n - likewise, do we need 100 commits? They are not too expensive these\n days with test_commit_bulk, but I think we don't even care about the\n repo contents at all.\n\n - there is no actual .keep file in your test. That's OK, as we are\n just checking the args passed to pack-objects.\n\n - useless use of cat. :) Also, you could probably do it all with one\n grep. This is bikeshedding, of course, but it's nice to keep process\n counts low in the test suite. Also, your middle grep is looser than\n the others (it doesn't check for surrounding quotes).\n\nSo something like this would work:\n\n test_expect_success '--write-midx -b packs non-kept objects' '\n GIT_TRACE2_EVENT=\"$(pwd)/midx-keep-bitmap.trace\" \\\n git -C midx repack --write-midx -a -b &&\n grep \"\\\"event\\\":\\\"start\\\".*\\\"pack-objects\\\".*\\\"--honor-pack-keep\\\"\" \\\n midx-keep-bitmap.trace\n '\n\nOne could perhaps argue that the combined grep is less readable. If\nthat's a concern, I'd suggest wrapping it in a function like:\n\n # usage: check_trace2_arg \n check_trace2_arg () {\n\tgrep \"\\\"event\\\":\\\"start\\\".*\\\"$2\\\".*\\\"$3\\\"\" \"$1\"\n }\n\nAll just suggestions, of course. I'd be happy enough to see the patch go\nin as-is.\n\n-Peff\n"},{"id":"444383","messageId":"YbzSfwQixuonrK/o@coredump.intra.peff.net","threadId":"57109","inReplyTo":"3eff83d9ae14023f3527dfeb419cf8259f6d053d.1639758526.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] repack: make '--quiet' disable progress","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-12-17T18:10:07Z","receivedAt":"2021-12-17T18:10:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 17, 2021 at 04:28:46PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n> From: Derrick Stolee \n> \n> While testing some ideas in 'git repack', I ran it with '--quiet' and\n> discovered that some progress output was still shown. Specifically, the\n> output for writing the multi-pack-index showed the progress.\n> \n> The 'show_progress' variable in cmd_repack() is initialized with\n> isatty(2) and is not modified at all by the '--quiet' flag. The\n> '--quiet' flag modifies the po_args.quiet option which is translated\n> into a '--quiet' flag for the 'git pack-objects' child process. However,\n> 'show_progress' is used to directly send progress information to the\n> multi-pack-index writing logic which does not use a child process.\n> \n> The fix here is to modify 'show_progress' to be false if po_opts.quiet\n> is true, and isatty(2) otherwise. This new expectation simplifies a\n> later condition that checks both.\n\nMakes sense. I wondered if you might have to decide what to do with\n\"--progress --quiet\", but we do not have an explicit progress option for\ngit-repack in the first place.\n\n> This is difficult to test because the isatty(2) already prevents the\n> progess indicators from appearing when we redirect stderr to a file.\n\nYou'd need test_terminal. Something like this:\n\ndiff --git a/t/t7700-repack.sh b/t/t7700-repack.sh\nindex 8c4ba6500b..b673c49650 100755\n--- a/t/t7700-repack.sh\n+++ b/t/t7700-repack.sh\n@@ -5,6 +5,7 @@ test_description='git repack works correctly'\n . ./test-lib.sh\n . \"${TEST_DIRECTORY}/lib-bitmap.sh\"\n . \"${TEST_DIRECTORY}/lib-midx.sh\"\n+. \"${TEST_DIRECTORY}/lib-terminal.sh\"\n \n commit_and_pack () {\n \ttest_commit \"$@\" 1>&2 &&\n@@ -387,4 +388,10 @@ test_expect_success '--write-midx -b packs non-kept objects' '\n \t)\n '\n \n+test_expect_success TTY '--quiet disables progress' '\n+\ttest_terminal env GIT_PROGRESS_DELAY=0 \\\n+\t\tgit -C midx repack -ad --quiet --write-midx 2>stderr &&\n+\ttest_must_be_empty stderr\n+'\n+\n test_done\n\n-Peff\n"},{"id":"444419","messageId":"211218.861r2axmk3.gmgdl@evledraar.gmail.com","threadId":"57109","inReplyTo":"3eff83d9ae14023f3527dfeb419cf8259f6d053d.1639758526.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] repack: make '--quiet' disable progress","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-18T09:55:33Z","receivedAt":"2021-12-18T09:57:37Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Dec 17 2021, Derrick Stolee via GitGitGadget wrote:\n\n> From: Derrick Stolee \n>\n> While testing some ideas in 'git repack', I ran it with '--quiet' and\n> discovered that some progress output was still shown. Specifically, the\n> output for writing the multi-pack-index showed the progress.\n>\n> The 'show_progress' variable in cmd_repack() is initialized with\n> isatty(2) and is not modified at all by the '--quiet' flag. The\n> '--quiet' flag modifies the po_args.quiet option which is translated\n> into a '--quiet' flag for the 'git pack-objects' child process. However,\n> 'show_progress' is used to directly send progress information to the\n> multi-pack-index writing logic which does not use a child process.\n>\n> The fix here is to modify 'show_progress' to be false if po_opts.quiet\n> is true, and isatty(2) otherwise. This new expectation simplifies a\n> later condition that checks both.\n>\n> This is difficult to test because the isatty(2) already prevents the\n> progess indicators from appearing when we redirect stderr to a file.\n>\n> Signed-off-by: Derrick Stolee \n> ---\n> builtin/repack.c | 6 ++++--\n> 1 file changed, 4 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/repack.c b/builtin/repack.c\n> index 1f128b7c90b..c393a5db774 100644\n> --- a/builtin/repack.c\n> +++ b/builtin/repack.c\n> @@ -612,7 +612,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n> \tstruct tempfile *refs_snapshot = NULL;\n> \tint i, ext, ret;\n> \tFILE *out;\n> -\tint show_progress = isatty(2);\n> +\tint show_progress;\n> \n> \t/* variables to be filled by option parsing */\n> \tint pack_everything = 0;\n> @@ -725,6 +725,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n> \n> \tprepare_pack_objects(&cmd, &po_args);\n> \n> +\tshow_progress = !po_args.quiet && isatty(2);\n> +\n> \tstrvec_push(&cmd.args, \"--keep-true-parents\");\n> \tif (!pack_kept_objects)\n> \t\tstrvec_push(&cmd.args, \"--honor-pack-keep\");\n> @@ -926,7 +928,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n> \t\t\t}\n> \t\t\tstrbuf_release(&buf);\n> \t\t}\n> -\t\tif (!po_args.quiet && show_progress)\n> +\t\tif (show_progress)\n> \t\t\topts |= PRUNE_PACKED_VERBOSE;\n> \t\tprune_packed_objects(opts);\n\nThis seems like a good change, but the documentation could really use an\nupdate (also before this change). It just says about -q:\n\n Pass the -q option to git pack-objects. See git-pack-objects(1).\n\nBut do various things in \"git-repack\" itself differently if it's\nsupplied.\n"},{"id":"444420","messageId":"211218.86wnk2w7uq.gmgdl@evledraar.gmail.com","threadId":"57109","inReplyTo":"1ed91f6d255b76bdbdcccea7e1effcebbb263ced.1639758526.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] repack: respect kept objects with '--write-midx -b'","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-18T09:58:01Z","receivedAt":"2021-12-18T10:00:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Dec 17 2021, Derrick Stolee via GitGitGadget wrote:\n\n> +test_expect_success '--write-midx -b packs non-kept objects' '\n> +\tgit init midx-kept &&\n> +\ttest_when_finished \"rm -fr midx-kept\" &&\n\nNit: Better the other way around, so we'll clean up things if \"git init\"\ndies midway, but unlikely to ever happen here.\n\n> +\t(\n> +\t\tcd midx-kept &&\n> +\t\ttest_commit_bulk 100 &&\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/trace.txt\" \\\n> +\t\t\tgit repack --write-midx -a -b &&\n> +\t\tcat trace.txt | \\\n> +\t\t\tgrep \\\"event\\\":\\\"start\\\" | \\\n\nThe \"cat\" here should go in favor of \"grep trace.txt\", and can't\nthis all be on one grep line with \".\" between the relevant parts you're\nextracting out?\n"},{"id":"444459","messageId":"7969c9e5-dd57-705c-b554-67681b9af62f@gmail.com","threadId":"57109","inReplyTo":"YbzSfwQixuonrK/o@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] repack: make '--quiet' disable progress","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-12-20T13:37:52Z","receivedAt":"2021-12-20T13:37:58Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 12/17/2021 1:10 PM, Jeff King wrote:\n> On Fri, Dec 17, 2021 at 04:28:46PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n>> This is difficult to test because the isatty(2) already prevents the\n>> progess indicators from appearing when we redirect stderr to a file.\n> \n> You'd need test_terminal. Something like this:\n> \n> diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh\n> index 8c4ba6500b..b673c49650 100755\n> --- a/t/t7700-repack.sh\n> +++ b/t/t7700-repack.sh\n> @@ -5,6 +5,7 @@ test_description='git repack works correctly'\n> . ./test-lib.sh\n> . \"${TEST_DIRECTORY}/lib-bitmap.sh\"\n> . \"${TEST_DIRECTORY}/lib-midx.sh\"\n> +. \"${TEST_DIRECTORY}/lib-terminal.sh\"\n> \n> commit_and_pack () {\n> \ttest_commit \"$@\" 1>&2 &&\n> @@ -387,4 +388,10 @@ test_expect_success '--write-midx -b packs non-kept objects' '\n> \t)\n> '\n> \n> +test_expect_success TTY '--quiet disables progress' '\n> +\ttest_terminal env GIT_PROGRESS_DELAY=0 \\\n> +\t\tgit -C midx repack -ad --quiet --write-midx 2>stderr &&\n> +\ttest_must_be_empty stderr\n> +'\n> +\n> test_done\n\nThanks. I added this test.\n\nWhen first running the test, it failed because I didn't have the\nIO::Pty Perl module installed. I'm not sure why I don't fail with\nother tests that use test_terminal. If someone knows more about\nwhat is going on, then maybe we need to expand the TTY prereq?\n\nThanks,\n-Stolee\n\n"},{"id":"444460","messageId":"27588da0-b47b-1cd3-8485-e9b8a4131e38@gmail.com","threadId":"57109","inReplyTo":"211218.861r2axmk3.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 2/2] repack: make '--quiet' disable progress","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-12-20T13:38:24Z","receivedAt":"2021-12-20T13:38:30Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 12/18/2021 4:55 AM, Ævar Arnfjörð Bjarmason wrote:\n> This seems like a good change, but the documentation could really use an\n> update (also before this change). It just says about -q:\n> \n> Pass the -q option to git pack-objects. See git-pack-objects(1).\n> \n> But do various things in \"git-repack\" itself differently if it's\n> supplied.\n\nGood point. I will update the docs.\n\nThanks,\n-Stolee\n"},{"id":"444461","messageId":"d10662af-4ce9-6944-aa73-ef344f6bdf67@gmail.com","threadId":"57109","inReplyTo":"YbzHtsl045XZbJGN@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] repack: respect kept objects with '--write-midx -b'","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-12-20T13:40:09Z","receivedAt":"2021-12-20T13:40:18Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 12/17/2021 12:24 PM, Jeff King wrote:\n> On Fri, Dec 17, 2021 at 04:28:45PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n...\n\n>> +test_expect_success '--write-midx -b packs non-kept objects' '\n>> +\tgit init midx-kept &&\n>> +\ttest_when_finished \"rm -fr midx-kept\" &&\n>> +\t(\n>> +\t\tcd midx-kept &&\n>> +\t\ttest_commit_bulk 100 &&\n>> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/trace.txt\" \\\n>> +\t\t\tgit repack --write-midx -a -b &&\n>> +\t\tcat trace.txt | \\\n>> +\t\t\tgrep \\\"event\\\":\\\"start\\\" | \\\n>> +\t\t\tgrep pack-objects | \\\n>> +\t\t\tgrep \\\"--honor-pack-keep\\\"\n>> +\t)\n>> +'\n> \n> This looks correct, though:\n> \n> - do we really need this separate repo directory? The test before it\n> uses one, but only because it needs a very specific set of commits.\n> There is a long-running \"midx\" directory we could use, though I\n> think just the regular test repo would be fine, too.\n> \n> - likewise, do we need 100 commits? They are not too expensive these\n> days with test_commit_bulk, but I think we don't even care about the\n> repo contents at all.\n> \n> - there is no actual .keep file in your test. That's OK, as we are\n> just checking the args passed to pack-objects.\n> \n> - useless use of cat. :) Also, you could probably do it all with one\n> grep. This is bikeshedding, of course, but it's nice to keep process\n> counts low in the test suite. Also, your middle grep is looser than\n> the others (it doesn't check for surrounding quotes).\n> \n> So something like this would work:\n> \n> test_expect_success '--write-midx -b packs non-kept objects' '\n> GIT_TRACE2_EVENT=\"$(pwd)/midx-keep-bitmap.trace\" \\\n> git -C midx repack --write-midx -a -b &&\n> grep \"\\\"event\\\":\\\"start\\\".*\\\"pack-objects\\\".*\\\"--honor-pack-keep\\\"\" \\\n> midx-keep-bitmap.trace\n> '\n> \n> One could perhaps argue that the combined grep is less readable. If\n> that's a concern, I'd suggest wrapping it in a function like:\n> \n> # usage: check_trace2_arg \n> check_trace2_arg () {\n> \tgrep \"\\\"event\\\":\\\"start\\\".*\\\"$2\\\".*\\\"$3\\\"\" \"$1\"\n> }\n> \n> All just suggestions, of course. I'd be happy enough to see the patch go\n> in as-is.\n\nThanks for the suggestions. I decided to adapt the 'test_subcommand'\nhelper into a 'test_subcommand_inexact' helper. The existing helper\nrequires the full argument list in exact order, but the new one only\ncares about the given arguments (in that relative order).\n\nThanks,\n-Stolee\n"},{"id":"444463","messageId":"YcCJ0bU1y4TnS3yp@coredump.intra.peff.net","threadId":"57109","inReplyTo":"7969c9e5-dd57-705c-b554-67681b9af62f@gmail.com","subject":"Re: [PATCH 2/2] repack: make '--quiet' disable progress","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-12-20T13:49:05Z","receivedAt":"2021-12-20T13:49:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 20, 2021 at 08:37:52AM -0500, Derrick Stolee wrote:\n\n> > +test_expect_success TTY '--quiet disables progress' '\n> > +\ttest_terminal env GIT_PROGRESS_DELAY=0 \\\n> > +\t\tgit -C midx repack -ad --quiet --write-midx 2>stderr &&\n> > +\ttest_must_be_empty stderr\n> > +'\n> > +\n> > test_done\n> \n> Thanks. I added this test.\n> \n> When first running the test, it failed because I didn't have the\n> IO::Pty Perl module installed. I'm not sure why I don't fail with\n> other tests that use test_terminal. If someone knows more about\n> what is going on, then maybe we need to expand the TTY prereq?\n\nWeird. I uninstalled IO::Pty, and get:\n\n checking prerequisite: TTY\n \n [...prereq code...]\n\n \n + perl /home/peff/compile/git/t/test-terminal.perl sh -c test -t 1 && test -t 2\n + command /usr/bin/perl /home/peff/compile/git/t/test-terminal.perl sh -c test -t 1 && test -t 2\n Can't locate IO/Pty.pm in @INC (you may need to install the IO::Pty module) (@INC contains: [...etc...]\n BEGIN failed--compilation aborted at /home/peff/compile/git/t/test-terminal.perl line 5.\n prerequisite TTY not satisfied\n ok 25 # skip --quiet disables progress (missing TTY)\n\nWhat does running with \"-x -i\" say for the prereq?\n\n-Peff\n"},{"id":"444464","messageId":"YcCKQEpBbdgbAKrk@coredump.intra.peff.net","threadId":"57109","inReplyTo":"d10662af-4ce9-6944-aa73-ef344f6bdf67@gmail.com","subject":"Re: [PATCH 1/2] repack: respect kept objects with '--write-midx -b'","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-12-20T13:50:56Z","receivedAt":"2021-12-20T13:50:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 20, 2021 at 08:40:09AM -0500, Derrick Stolee wrote:\n\n> > One could perhaps argue that the combined grep is less readable. If\n> > that's a concern, I'd suggest wrapping it in a function like:\n> > \n> > # usage: check_trace2_arg \n> > check_trace2_arg () {\n> > \tgrep \"\\\"event\\\":\\\"start\\\".*\\\"$2\\\".*\\\"$3\\\"\" \"$1\"\n> > }\n> > \n> > All just suggestions, of course. I'd be happy enough to see the patch go\n> > in as-is.\n> \n> Thanks for the suggestions. I decided to adapt the 'test_subcommand'\n> helper into a 'test_subcommand_inexact' helper. The existing helper\n> requires the full argument list in exact order, but the new one only\n> cares about the given arguments (in that relative order).\n\nHeh, I should have looked to see if we had faced this problem before.\nExtending that family of helpers is better still than my suggestion.\n\n-Peff\n"},{"id":"444472","messageId":"6f449f30-abb2-8b11-1669-abb03b73f7d0@gmail.com","threadId":"57109","inReplyTo":"YcCJ0bU1y4TnS3yp@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] repack: make '--quiet' disable progress","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-12-20T14:46:55Z","receivedAt":"2021-12-20T14:54:25Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 12/20/2021 8:49 AM, Jeff King wrote:\n> On Mon, Dec 20, 2021 at 08:37:52AM -0500, Derrick Stolee wrote:\n> \n>>> +test_expect_success TTY '--quiet disables progress' '\n>>> +\ttest_terminal env GIT_PROGRESS_DELAY=0 \\\n>>> +\t\tgit -C midx repack -ad --quiet --write-midx 2>stderr &&\n>>> +\ttest_must_be_empty stderr\n>>> +'\n>>> +\n>>> test_done\n>>\n>> Thanks. I added this test.\n>>\n>> When first running the test, it failed because I didn't have the\n>> IO::Pty Perl module installed. I'm not sure why I don't fail with\n>> other tests that use test_terminal. If someone knows more about\n>> what is going on, then maybe we need to expand the TTY prereq?\n> \n> Weird. I uninstalled IO::Pty, and get:\n> \n> checking prerequisite: TTY\n> \n> [...prereq code...]\n> \n> \n> + perl /home/peff/compile/git/t/test-terminal.perl sh -c test -t 1 && test -t 2\n> + command /usr/bin/perl /home/peff/compile/git/t/test-terminal.perl sh -c test -t 1 && test -t 2\n> Can't locate IO/Pty.pm in @INC (you may need to install the IO::Pty module) (@INC contains: [...etc...]\n> BEGIN failed--compilation aborted at /home/peff/compile/git/t/test-terminal.perl line 5.\n> prerequisite TTY not satisfied\n> ok 25 # skip --quiet disables progress (missing TTY)\n> \n> What does running with \"-x -i\" say for the prereq?\n\nOk, I got this same error and misread it as an error. The prereq\nis working just fine. Thanks for checking.\n\n-Stolee\n"},{"id":"444473","messageId":"pull.1098.v2.git.1640011691.gitgitgadget@gmail.com","threadId":"57109","inReplyTo":"pull.1098.git.1639758526.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] Two small 'git repack' fixes","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-20T14:48:09Z","receivedAt":"2021-12-20T14:56:48Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"I was experimenting with some ideas in 'git repack' and discovered these two\nbugs.\n\nThe first is a \"real\" bug in that it repacks much more data than is\nnecessary when repacking with '--write-midx -b' and there exists a .keep\npack. The fix is simple, which is to change a condition that was added for\nthe '-b' case before '--write-midx' existed.\n\nThe second is a UX bug in that '--quiet' did not disable all progress, at\nleast when stderr was interactive.\n\n\nUpdates in v2\n=============\n\nThanks for the quick review!\n\n * Test for --honor-pack-keep is cleaner with a reusable helper.\n * TTY test is added.\n * Docs are updated.\n\nThanks, -Stolee\n\nDerrick Stolee (2):\n repack: respect kept objects with '--write-midx -b'\n repack: make '--quiet' disable progress\n\n Documentation/git-repack.txt | 5 +++--\n builtin/repack.c | 8 +++++---\n t/t7700-repack.sh | 13 +++++++++++++\n t/test-lib-functions.sh | 34 ++++++++++++++++++++++++++++++++++\n 4 files changed, 55 insertions(+), 5 deletions(-)\n\n\nbase-commit: 69a9c10c95e28df457e33b3c7400b16caf2e2962\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1098%2Fderrickstolee%2Frepack-fixes-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1098/derrickstolee/repack-fixes-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1098\n\nRange-diff vs v1:\n\n 1: 1ed91f6d255 ! 1: 747328a4dd6 repack: respect kept objects with '--write-midx -b'\n @@ Commit message\n some historical set of objects in a stable pack-file while only\n repacking more recent objects.\n \n + To test, create a new 'test_subcommand_inexact' helper that is more\n + flexible than 'test_subcommand'. This allows us to look for the\n + --honor-pack-keep flag without over-indexing on the exact set of\n + arguments.\n +\n Signed-off-by: Derrick Stolee \n \n ## builtin/repack.c ##\n @@ t/t7700-repack.sh: test_expect_success '--write-midx with preferred bitmap tips'\n '\n \n +test_expect_success '--write-midx -b packs non-kept objects' '\n -+\tgit init midx-kept &&\n -+\ttest_when_finished \"rm -fr midx-kept\" &&\n -+\t(\n -+\t\tcd midx-kept &&\n -+\t\ttest_commit_bulk 100 &&\n -+\t\tGIT_TRACE2_EVENT=\"$(pwd)/trace.txt\" \\\n -+\t\t\tgit repack --write-midx -a -b &&\n -+\t\tcat trace.txt | \\\n -+\t\t\tgrep \\\"event\\\":\\\"start\\\" | \\\n -+\t\t\tgrep pack-objects | \\\n -+\t\t\tgrep \\\"--honor-pack-keep\\\"\n -+\t)\n ++\tGIT_TRACE2_EVENT=\"$(pwd)/trace.txt\" \\\n ++\t\tgit repack --write-midx -a -b &&\n ++\ttest_subcommand_inexact git pack-objects --honor-pack-keep ... < \n ++#\n ++# For example, to look for an invocation of \"git pack-objects\"\n ++# with the \"--honor-pack-keep\" argument, use\n ++#\n ++#\tGIT_TRACE2_EVENT=event.log git repack ... &&\n ++#\ttest_subcommand git pack-objects --honor-pack-keep \n Signed-off-by: Derrick Stolee \n \n + ## Documentation/git-repack.txt ##\n +@@ Documentation/git-repack.txt: to the new separate pack will be written.\n + \tlinkgit:git-pack-objects[1].\n + \n + -q::\n +-\tPass the `-q` option to 'git pack-objects'. See\n +-\tlinkgit:git-pack-objects[1].\n ++--quiet::\n ++\tShow no progress over the standard error stream and pass the `-q`\n ++\toption to 'git pack-objects'. See linkgit:git-pack-objects[1].\n + \n + -n::\n + \tDo not update the server information with\n +\n ## builtin/repack.c ##\n @@ builtin/repack.c: int cmd_repack(int argc, const char **argv, const char *prefix)\n \tstruct tempfile *refs_snapshot = NULL;\n @@ builtin/repack.c: int cmd_repack(int argc, const char **argv, const char *prefix\n \t\t\topts |= PRUNE_PACKED_VERBOSE;\n \t\tprune_packed_objects(opts);\n \n +\n + ## t/t7700-repack.sh ##\n +@@ t/t7700-repack.sh: test_description='git repack works correctly'\n + . ./test-lib.sh\n + . \"${TEST_DIRECTORY}/lib-bitmap.sh\"\n + . \"${TEST_DIRECTORY}/lib-midx.sh\"\n ++. \"${TEST_DIRECTORY}/lib-terminal.sh\"\n + \n + commit_and_pack () {\n + \ttest_commit \"$@\" 1>&2 &&\n +@@ t/t7700-repack.sh: test_expect_success '--write-midx -b packs non-kept objects' '\n + \ttest_subcommand_inexact git pack-objects --honor-pack-keep stderr &&\n ++\ttest_must_be_empty stderr\n ++'\n ++\n + test_done\n\n-- \ngitgitgadget\n"},{"id":"444474","messageId":"41260bf0829970eeb27bec2cb41cabeffa54dabb.1640011691.git.gitgitgadget@gmail.com","threadId":"57109","inReplyTo":"pull.1098.v2.git.1640011691.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] repack: make '--quiet' disable progress","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-20T14:48:11Z","receivedAt":"2021-12-20T14:57:27Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhile testing some ideas in 'git repack', I ran it with '--quiet' and\ndiscovered that some progress output was still shown. Specifically, the\noutput for writing the multi-pack-index showed the progress.\n\nThe 'show_progress' variable in cmd_repack() is initialized with\nisatty(2) and is not modified at all by the '--quiet' flag. The\n'--quiet' flag modifies the po_args.quiet option which is translated\ninto a '--quiet' flag for the 'git pack-objects' child process. However,\n'show_progress' is used to directly send progress information to the\nmulti-pack-index writing logic which does not use a child process.\n\nThe fix here is to modify 'show_progress' to be false if po_opts.quiet\nis true, and isatty(2) otherwise. This new expectation simplifies a\nlater condition that checks both.\n\nUpdate the documentation to make it clear that '-q' will disable all\nprogress in addition to ensuring the 'git pack-objects' child process\nwill receive the flag.\n\nUse 'test_terminal' to check that this works to get around the isatty(2)\ncheck.\n\nHelped-by: Jeff King \nSigned-off-by: Derrick Stolee \n---\n Documentation/git-repack.txt | 5 +++--\n builtin/repack.c | 6 ++++--\n t/t7700-repack.sh | 7 +++++++\n 3 files changed, 14 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt\nindex 7183fb498f4..ee30edc178a 100644\n--- a/Documentation/git-repack.txt\n+++ b/Documentation/git-repack.txt\n@@ -76,8 +76,9 @@ to the new separate pack will be written.\n \tlinkgit:git-pack-objects[1].\n \n -q::\n-\tPass the `-q` option to 'git pack-objects'. See\n-\tlinkgit:git-pack-objects[1].\n+--quiet::\n+\tShow no progress over the standard error stream and pass the `-q`\n+\toption to 'git pack-objects'. See linkgit:git-pack-objects[1].\n \n -n::\n \tDo not update the server information with\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex 1f128b7c90b..c393a5db774 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -612,7 +612,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n \tstruct tempfile *refs_snapshot = NULL;\n \tint i, ext, ret;\n \tFILE *out;\n-\tint show_progress = isatty(2);\n+\tint show_progress;\n \n \t/* variables to be filled by option parsing */\n \tint pack_everything = 0;\n@@ -725,6 +725,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n \n \tprepare_pack_objects(&cmd, &po_args);\n \n+\tshow_progress = !po_args.quiet && isatty(2);\n+\n \tstrvec_push(&cmd.args, \"--keep-true-parents\");\n \tif (!pack_kept_objects)\n \t\tstrvec_push(&cmd.args, \"--honor-pack-keep\");\n@@ -926,7 +928,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n \t\t\t}\n \t\t\tstrbuf_release(&buf);\n \t\t}\n-\t\tif (!po_args.quiet && show_progress)\n+\t\tif (show_progress)\n \t\t\topts |= PRUNE_PACKED_VERBOSE;\n \t\tprune_packed_objects(opts);\n \ndiff --git a/t/t7700-repack.sh b/t/t7700-repack.sh\nindex 63c9a247f57..fe2b493d0ee 100755\n--- a/t/t7700-repack.sh\n+++ b/t/t7700-repack.sh\n@@ -5,6 +5,7 @@ test_description='git repack works correctly'\n . ./test-lib.sh\n . \"${TEST_DIRECTORY}/lib-bitmap.sh\"\n . \"${TEST_DIRECTORY}/lib-midx.sh\"\n+. \"${TEST_DIRECTORY}/lib-terminal.sh\"\n \n commit_and_pack () {\n \ttest_commit \"$@\" 1>&2 &&\n@@ -378,4 +379,10 @@ test_expect_success '--write-midx -b packs non-kept objects' '\n \ttest_subcommand_inexact git pack-objects --honor-pack-keep stderr &&\n+\ttest_must_be_empty stderr\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"444475","messageId":"747328a4dd69f2325892b4d89e3e62a0bcfbba59.1640011691.git.gitgitgadget@gmail.com","threadId":"57109","inReplyTo":"pull.1098.v2.git.1640011691.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] repack: respect kept objects with '--write-midx -b'","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-20T14:48:10Z","receivedAt":"2021-12-20T14:57:36Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nHistorically, we needed a single packfile in order to have reachability\nbitmaps. This introduced logic that when 'git repack' had a '-b' option\nthat we should stop sending the '--honor-pack-keep' option to the 'git\npack-objects' child process, ensuring that we create a packfile\ncontaining all reachable objects.\n\nIn the world of multi-pack-index bitmaps, we no longer need to repack\nall objects into a single pack to have valid bitmaps. Thus, we should\ncontinue sending the '--honor-pack-keep' flag to 'git pack-objects'.\n\nThe fix is very simple: only disable the flag when writing bitmaps but\nalso _not_ writing the multi-pack-index.\n\nThis opens the door to new repacking strategies that might want to keep\nsome historical set of objects in a stable pack-file while only\nrepacking more recent objects.\n\nTo test, create a new 'test_subcommand_inexact' helper that is more\nflexible than 'test_subcommand'. This allows us to look for the\n--honor-pack-keep flag without over-indexing on the exact set of\narguments.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/repack.c | 2 +-\n t/t7700-repack.sh | 6 ++++++\n t/test-lib-functions.sh | 34 ++++++++++++++++++++++++++++++++++\n 3 files changed, 41 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex 9b0be6a6ab3..1f128b7c90b 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -693,7 +693,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n \t\twrite_bitmaps = 0;\n \t}\n \tif (pack_kept_objects < 0)\n-\t\tpack_kept_objects = write_bitmaps > 0;\n+\t\tpack_kept_objects = write_bitmaps > 0 && !write_midx;\n \n \tif (write_bitmaps && !(pack_everything & ALL_INTO_ONE) && !write_midx)\n \t\tdie(_(incremental_bitmap_conflict_error));\ndiff --git a/t/t7700-repack.sh b/t/t7700-repack.sh\nindex 0260ad6f0e0..63c9a247f57 100755\n--- a/t/t7700-repack.sh\n+++ b/t/t7700-repack.sh\n@@ -372,4 +372,10 @@ test_expect_success '--write-midx with preferred bitmap tips' '\n \t)\n '\n \n+test_expect_success '--write-midx -b packs non-kept objects' '\n+\tGIT_TRACE2_EVENT=\"$(pwd)/trace.txt\" \\\n+\t\tgit repack --write-midx -a -b &&\n+\ttest_subcommand_inexact git pack-objects --honor-pack-keep ... < \n+#\n+# For example, to look for an invocation of \"git pack-objects\"\n+# with the \"--honor-pack-keep\" argument, use\n+#\n+#\tGIT_TRACE2_EVENT=event.log git repack ... &&\n+#\ttest_subcommand git pack-objects --honor-pack-keep I was experimenting with some ideas in 'git repack' and discovered these two\n> bugs.\n\nThis iteration looks good to me.\n\n> + ## Documentation/git-repack.txt ##\n> +@@ Documentation/git-repack.txt: to the new separate pack will be written.\n> + \tlinkgit:git-pack-objects[1].\n> + \n> + -q::\n> +-\tPass the `-q` option to 'git pack-objects'. See\n> +-\tlinkgit:git-pack-objects[1].\n> ++--quiet::\n> ++\tShow no progress over the standard error stream and pass the `-q`\n> ++\toption to 'git pack-objects'. See linkgit:git-pack-objects[1].\n> + \n> + -n::\n> + \tDo not update the server information with\n> +\n\nNit: I think the addition of --quiet here is good, but also wouldn't\nmind a prep change to add it as a separate step.\n\nFWIW it appears to have been an unintended/hidden effect of a1bbc6c0176\n(repack: rewrite the shell script in C, 2013-09-15), i.e. we started\nusing OPT__QUIET() which added \"--quiet\", but the shellscript just\nsupported -q.\n\nHere we update the docs, but the SYNOPSIS stillj just lists -q.\n\nI think all of that's fine & this v2 is good enough. Just braindumping\nnotes/observations while reading along/checking the v1->v2 diff.\n"}]}