{"thread":{"id":"52232","subject":"[PATCH v3 3/3] bundle-verify: add --quiet","startedAt":"2019-11-10T20:41:52Z","lastAt":"2019-11-12T15:09:43Z","messageCount":13,"participants":["Robin H. Johnson","Junio C Hamano","Jeff King","Johannes Schindelin"],"isPatch":true,"patchVersion":3,"patchTotal":3},"messages":[{"id":"385848","messageId":"20191110204126.30553-3-robbat2@gentoo.org","threadId":"52232","inReplyTo":"20191110204126.30553-1-robbat2@gentoo.org","subject":"[PATCH v3 3/3] bundle-verify: add --quiet","fromName":"Robin H. Johnson","fromEmail":"robbat2@gentoo.org","sentAt":"2019-11-10T20:41:26Z","receivedAt":"2019-11-10T20:41:52Z","isPatch":true,"sender":{"key":"robbat2@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/373898?v=4"},"body":"Add --quiet to git-bundle verify as proposed on the mailing list [1].\n\nReference: https://www.mail-archive.com/git@vger.kernel.org/msg182844.html \nSigned-off-by: Robin H. Johnson \n---\n Documentation/git-bundle.txt | 2 +-\n builtin/bundle.c | 9 ++++++---\n 2 files changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-bundle.txt b/Documentation/git-bundle.txt\nindex 96bb94df7b..ccada80a4a 100644\n--- a/Documentation/git-bundle.txt\n+++ b/Documentation/git-bundle.txt\n@@ -10,7 +10,7 @@ SYNOPSIS\n --------\n [verse]\n 'git bundle' create [-q | --quiet | --progress | --all-progress] [--all-progress-implied] \n-'git bundle' verify \n+'git bundle' verify [-q | --quiet] \n 'git bundle' list-heads [...]\n 'git bundle' unbundle [...]\n \ndiff --git a/builtin/bundle.c b/builtin/bundle.c\nindex 39b3e88d40..f049d27a14 100644\n--- a/builtin/bundle.c\n+++ b/builtin/bundle.c\n@@ -13,7 +13,7 @@\n \n static const char * const builtin_bundle_usage[] = {\n N_(\"git bundle create [] \"),\n- N_(\"git bundle verify \"),\n+ N_(\"git bundle verify [] \"),\n N_(\"git bundle list-heads [...]\"),\n N_(\"git bundle unbundle [...]\"),\n NULL\n@@ -25,7 +25,7 @@ static const char * const builtin_bundle_create_usage[] = {\n };\n \n static const char * const builtin_bundle_verify_usage[] = {\n- N_(\"git bundle verify \"),\n+ N_(\"git bundle verify [] \"),\n NULL\n };\n \n@@ -97,8 +97,11 @@ static int cmd_bundle_create(int argc, const char **argv, const char *prefix) {\n static int cmd_bundle_verify(int argc, const char **argv, const char *prefix) {\n \tstruct bundle_header header;\n \tint bundle_fd = -1;\n+\tint quiet = 0;\n \n \tstruct option options[] = {\n+\t\tOPT_BOOL('q', \"quiet\", &quiet,\n+\t\t\t N_(\"do not show bundle details\")),\n \t\tOPT_END()\n \t};\n \tconst char* bundle_file;\n@@ -111,7 +114,7 @@ static int cmd_bundle_verify(int argc, const char **argv, const char *prefix) {\n \tif ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0)\n \t\treturn 1;\n \tclose(bundle_fd);\n-\tif (verify_bundle(the_repository, &header, 1))\n+\tif (verify_bundle(the_repository, &header, !quiet))\n \t\treturn 1;\n \tfprintf(stderr, _(\"%s is okay\\n\"), bundle_file);\n \treturn 0;\n-- \n2.23.0\n\n"},{"id":"385850","messageId":"20191110204126.30553-1-robbat2@gentoo.org","threadId":"52232","inReplyTo":"1f7f0aa1e8fae54bf967ae83a160be2b30db634f.1573248640.git.gitgitgadget@gmail.com","subject":"[PATCH v3 1/3] bundle: framework for options before bundle file","fromName":"Robin H. Johnson","fromEmail":"robbat2@gentoo.org","sentAt":"2019-11-10T20:41:24Z","receivedAt":"2019-11-10T20:41:52Z","isPatch":true,"sender":{"key":"robbat2@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/373898?v=4"},"body":"Make it possible for any of the git-bundle subcommands to include\noptions:\n- before the sub-command\n- after the sub-command, before the bundle filename\n\nThere is an immediate gain in support for help with all of the\nsub-commands, where 'git bundle list-heads -h' previously returned an\nerror.\n\nDownside here is an increase in code duplication that cannot be\ntrivially avoided short of shared global static options.\n\nSigned-off-by: Robin H. Johnson \n---\n builtin/bundle.c | 190 ++++++++++++++++++++++++++++++++++++-----------\n 1 file changed, 145 insertions(+), 45 deletions(-)\n\nI tried doing this via GitGitGadget initially as a test of that process,\nas well as the CI integration side; however as noted in #git-devel and\nelsewhere on the list, vger seems to swallow the mail to /dev/null\n\ndiff --git a/builtin/bundle.c b/builtin/bundle.c\nindex 1ea4bfdfc1..09b989cfc0 100644\n--- a/builtin/bundle.c\n+++ b/builtin/bundle.c\n@@ -1,4 +1,5 @@\n #include \"builtin.h\"\n+#include \"parse-options.h\"\n #include \"cache.h\"\n #include \"bundle.h\"\n \n@@ -9,59 +10,158 @@\n * bundle supporting \"fetch\", \"pull\", and \"ls-remote\".\n */\n \n-static const char builtin_bundle_usage[] =\n- \"git bundle create \\n\"\n- \" or: git bundle verify \\n\"\n- \" or: git bundle list-heads [...]\\n\"\n- \" or: git bundle unbundle [...]\";\n+static const char * const builtin_bundle_usage[] = {\n+ N_(\"git bundle create \"),\n+ N_(\"git bundle verify \"),\n+ N_(\"git bundle list-heads [...]\"),\n+ N_(\"git bundle unbundle [...]\"),\n+ NULL\n+};\n \n-int cmd_bundle(int argc, const char **argv, const char *prefix)\n-{\n+static const char * const builtin_bundle_create_usage[] = {\n+ N_(\"git bundle create \"),\n+ NULL\n+};\n+\n+static const char * const builtin_bundle_verify_usage[] = {\n+ N_(\"git bundle verify \"),\n+ NULL\n+};\n+\n+static const char * const builtin_bundle_list_heads_usage[] = {\n+ N_(\"git bundle list-heads [...]\"),\n+ NULL\n+};\n+\n+static const char * const builtin_bundle_unbundle_usage[] = {\n+ N_(\"git bundle unbundle [...]\"),\n+ NULL\n+};\n+\n+static int verbose;\n+\n+static int parse_options_cmd_bundle(int argc,\n+\t\tconst char **argv,\n+\t\tconst char* prefix,\n+\t\tconst char * const usagestr[],\n+\t\tconst struct option options[],\n+\t\tconst char **bundle_file) {\n+\tint newargc;\n+\tnewargc = parse_options(argc, argv, NULL, options, usagestr,\n+\t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n+\tif (argc < 1)\n+\t\tusage_with_options(usagestr, options);\n+\t*bundle_file = prefix_filename(prefix, argv[0]);\n+\treturn newargc;\n+}\n+\n+static int cmd_bundle_create(int argc, const char **argv, const char *prefix) {\n+\tstruct option options[] = {\n+\t\tOPT_END()\n+\t};\n+\tconst char* bundle_file;\n+\n+\targc = parse_options_cmd_bundle(argc, argv, prefix,\n+\t\t\tbuiltin_bundle_create_usage, options, &bundle_file);\n+\t/* bundle internals use argv[1] as further parameters */\n+\n+\tif (!startup_info->have_repository)\n+\t\tdie(_(\"Need a repository to create a bundle.\"));\n+\treturn !!create_bundle(the_repository, bundle_file, argc, argv);\n+}\n+\n+static int cmd_bundle_verify(int argc, const char **argv, const char *prefix) {\n \tstruct bundle_header header;\n-\tconst char *cmd, *bundle_file;\n \tint bundle_fd = -1;\n \n-\tif (argc < 3)\n-\t\tusage(builtin_bundle_usage);\n+\tstruct option options[] = {\n+\t\tOPT_END()\n+\t};\n+\tconst char* bundle_file;\n \n-\tcmd = argv[1];\n-\tbundle_file = prefix_filename(prefix, argv[2]);\n-\targc -= 2;\n-\targv += 2;\n+\targc = parse_options_cmd_bundle(argc, argv, prefix,\n+\t\t\tbuiltin_bundle_verify_usage, options, &bundle_file);\n+\t/* bundle internals use argv[1] as further parameters */\n \n \tmemset(&header, 0, sizeof(header));\n-\tif (strcmp(cmd, \"create\") && (bundle_fd =\n-\t\t\t\tread_bundle_header(bundle_file, &header)) < 0)\n+\tif ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0)\n \t\treturn 1;\n+\tclose(bundle_fd);\n+\tif (verify_bundle(the_repository, &header, 1))\n+\t\treturn 1;\n+\tfprintf(stderr, _(\"%s is okay\\n\"), bundle_file);\n+\treturn 0;\n+}\n \n-\tif (!strcmp(cmd, \"verify\")) {\n-\t\tclose(bundle_fd);\n-\t\tif (argc != 1) {\n-\t\t\tusage(builtin_bundle_usage);\n-\t\t\treturn 1;\n-\t\t}\n-\t\tif (verify_bundle(the_repository, &header, 1))\n-\t\t\treturn 1;\n-\t\tfprintf(stderr, _(\"%s is okay\\n\"), bundle_file);\n-\t\treturn 0;\n-\t}\n-\tif (!strcmp(cmd, \"list-heads\")) {\n-\t\tclose(bundle_fd);\n-\t\treturn !!list_bundle_refs(&header, argc, argv);\n+static int cmd_bundle_list_heads(int argc, const char **argv, const char *prefix) {\n+\tstruct bundle_header header;\n+\tint bundle_fd = -1;\n+\n+\tstruct option options[] = {\n+\t\tOPT_END()\n+\t};\n+\tconst char* bundle_file;\n+\n+\targc = parse_options_cmd_bundle(argc, argv, prefix,\n+\t\t\tbuiltin_bundle_list_heads_usage, options, &bundle_file);\n+\t/* bundle internals use argv[1] as further parameters */\n+\n+\tmemset(&header, 0, sizeof(header));\n+\tif ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0)\n+\t\treturn 1;\n+\tclose(bundle_fd);\n+\treturn !!list_bundle_refs(&header, argc, argv);\n+}\n+\n+static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix) {\n+\tstruct bundle_header header;\n+\tint bundle_fd = -1;\n+\n+\tstruct option options[] = {\n+\t\tOPT_END()\n+\t};\n+\tconst char* bundle_file;\n+\n+\targc = parse_options_cmd_bundle(argc, argv, prefix,\n+\t\t\tbuiltin_bundle_unbundle_usage, options, &bundle_file);\n+\t/* bundle internals use argv[1] as further parameters */\n+\n+\tmemset(&header, 0, sizeof(header));\n+\tif ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0)\n+\t\treturn 1;\n+\tif (!startup_info->have_repository)\n+\t\tdie(_(\"Need a repository to unbundle.\"));\n+\treturn !!unbundle(the_repository, &header, bundle_fd, 0) ||\n+\t\tlist_bundle_refs(&header, argc, argv);\n+}\n+\n+int cmd_bundle(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct option options[] = {\n+\t\tOPT__VERBOSE(&verbose, N_(\"be verbose; must be placed before a subcommand\")),\n+\t\tOPT_END()\n+\t};\n+\tint result;\n+\n+\targc = parse_options(argc, argv, prefix, options, builtin_bundle_usage,\n+\t\tPARSE_OPT_STOP_AT_NON_OPTION);\n+\n+\tpacket_trace_identity(\"bundle\");\n+\n+\tif (argc < 2)\n+\t\tusage_with_options(builtin_bundle_usage, options);\n+\n+\telse if (!strcmp(argv[0], \"create\"))\n+\t\tresult = cmd_bundle_create(argc, argv, prefix);\n+\telse if (!strcmp(argv[0], \"verify\"))\n+\t\tresult = cmd_bundle_verify(argc, argv, prefix);\n+\telse if (!strcmp(argv[0], \"list-heads\"))\n+\t\tresult = cmd_bundle_list_heads(argc, argv, prefix);\n+\telse if (!strcmp(argv[0], \"unbundle\"))\n+\t\tresult = cmd_bundle_unbundle(argc, argv, prefix);\n+\telse {\n+\t\terror(_(\"Unknown subcommand: %s\"), argv[0]);\n+\t\tusage_with_options(builtin_bundle_usage, options);\n \t}\n-\tif (!strcmp(cmd, \"create\")) {\n-\t\tif (argc < 2) {\n-\t\t\tusage(builtin_bundle_usage);\n-\t\t\treturn 1;\n-\t\t}\n-\t\tif (!startup_info->have_repository)\n-\t\t\tdie(_(\"Need a repository to create a bundle.\"));\n-\t\treturn !!create_bundle(the_repository, bundle_file, argc, argv);\n-\t} else if (!strcmp(cmd, \"unbundle\")) {\n-\t\tif (!startup_info->have_repository)\n-\t\t\tdie(_(\"Need a repository to unbundle.\"));\n-\t\treturn !!unbundle(the_repository, &header, bundle_fd, 0) ||\n-\t\t\tlist_bundle_refs(&header, argc, argv);\n-\t} else\n-\t\tusage(builtin_bundle_usage);\n+\treturn result ? 1 : 0;\n }\n-- \n2.23.0\n\n"},{"id":"385849","messageId":"20191110204126.30553-2-robbat2@gentoo.org","threadId":"52232","inReplyTo":"20191110204126.30553-1-robbat2@gentoo.org","subject":"[PATCH v3 2/3] bundle-create: progress output control","fromName":"Robin H. Johnson","fromEmail":"robbat2@gentoo.org","sentAt":"2019-11-10T20:41:25Z","receivedAt":"2019-11-10T20:41:53Z","isPatch":true,"sender":{"key":"robbat2@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/373898?v=4"},"body":"Support the progress output options from pack-objects in git-bundle's\ncreate subcommand. Most notably, this provides --quiet as requested on\nthe git mailing list per [1]\n\nReference: https://www.mail-archive.com/git@vger.kernel.org/msg182844.html \nSigned-off-by: Robin H. Johnson \n---\n Documentation/git-bundle.txt | 33 +++++++++++++++++++++++++++++++--\n builtin/bundle.c | 30 +++++++++++++++++++++++++++---\n bundle.c | 9 +++++----\n bundle.h | 3 ++-\n 4 files changed, 65 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-bundle.txt b/Documentation/git-bundle.txt\nindex 7d6c9dcd17..96bb94df7b 100644\n--- a/Documentation/git-bundle.txt\n+++ b/Documentation/git-bundle.txt\n@@ -9,7 +9,7 @@ git-bundle - Move objects and refs by archive\n SYNOPSIS\n --------\n [verse]\n-'git bundle' create \n+'git bundle' create [-q | --quiet | --progress | --all-progress] [--all-progress-implied] \n 'git bundle' verify \n 'git bundle' list-heads [...]\n 'git bundle' unbundle [...]\n@@ -33,9 +33,11 @@ destination repository.\n OPTIONS\n -------\n \n-create ::\n+create [options] ::\n \tUsed to create a bundle named 'file'. This requires the\n \t'git-rev-list-args' arguments to define the bundle contents.\n+\t'options' contains the options specific to the 'git bundle create'\n+\tsubcommand.\n \n verify ::\n \tUsed to check that a bundle file is valid and will apply\n@@ -75,6 +77,33 @@ unbundle ::\n \tnecessarily everything in the pack (in this case, 'git bundle' acts\n \tlike 'git fetch-pack').\n \n+--progress::\n+\tProgress status is reported on the standard error stream\n+\tby default when it is attached to a terminal, unless -q\n+\tis specified. This flag forces progress status even if\n+\tthe standard error stream is not directed to a terminal.\n+\n+--all-progress::\n+\tWhen --stdout is specified then progress report is\n+\tdisplayed during the object count and compression phases\n+\tbut inhibited during the write-out phase. The reason is\n+\tthat in some cases the output stream is directly linked\n+\tto another command which may wish to display progress\n+\tstatus of its own as it processes incoming pack data.\n+\tThis flag is like --progress except that it forces progress\n+\treport for the write-out phase as well even if --stdout is\n+\tused.\n+\n+--all-progress-implied::\n+\tThis is used to imply --all-progress whenever progress display\n+\tis activated. Unlike --all-progress this flag doesn't actually\n+\tforce any progress display by itself.\n+\n+-q::\n+--quiet::\n+\tThis flag makes the command not to report its progress\n+\ton the standard error stream.\n+\n SPECIFYING REFERENCES\n ---------------------\n \ndiff --git a/builtin/bundle.c b/builtin/bundle.c\nindex 09b989cfc0..39b3e88d40 100644\n--- a/builtin/bundle.c\n+++ b/builtin/bundle.c\n@@ -1,4 +1,5 @@\n #include \"builtin.h\"\n+#include \"argv-array.h\"\n #include \"parse-options.h\"\n #include \"cache.h\"\n #include \"bundle.h\"\n@@ -11,7 +12,7 @@\n */\n \n static const char * const builtin_bundle_usage[] = {\n- N_(\"git bundle create \"),\n+ N_(\"git bundle create [] \"),\n N_(\"git bundle verify \"),\n N_(\"git bundle list-heads [...]\"),\n N_(\"git bundle unbundle [...]\"),\n@@ -19,7 +20,7 @@ static const char * const builtin_bundle_usage[] = {\n };\n \n static const char * const builtin_bundle_create_usage[] = {\n- N_(\"git bundle create \"),\n+ N_(\"git bundle create [] \"),\n NULL\n };\n \n@@ -56,7 +57,20 @@ static int parse_options_cmd_bundle(int argc,\n }\n \n static int cmd_bundle_create(int argc, const char **argv, const char *prefix) {\n+\tint all_progress_implied = 0;\n+\tint progress = isatty(STDERR_FILENO);\n+\tstruct argv_array pack_opts;\n+\n \tstruct option options[] = {\n+\t\tOPT_SET_INT('q', \"quiet\", &progress,\n+\t\t\t N_(\"do not show progress meter\"), 0),\n+\t\tOPT_SET_INT(0, \"progress\", &progress,\n+\t\t\t N_(\"show progress meter\"), 1),\n+\t\tOPT_SET_INT(0, \"all-progress\", &progress,\n+\t\t\t N_(\"show progress meter during object writing phase\"), 2),\n+\t\tOPT_BOOL(0, \"all-progress-implied\",\n+\t\t\t &all_progress_implied,\n+\t\t\t N_(\"similar to --all-progress when progress meter is shown\")),\n \t\tOPT_END()\n \t};\n \tconst char* bundle_file;\n@@ -65,9 +79,19 @@ static int cmd_bundle_create(int argc, const char **argv, const char *prefix) {\n \t\t\tbuiltin_bundle_create_usage, options, &bundle_file);\n \t/* bundle internals use argv[1] as further parameters */\n \n+\targv_array_init(&pack_opts);\n+\tif (progress == 0)\n+\t\targv_array_push(&pack_opts, \"--quiet\");\n+\telse if (progress == 1)\n+\t\targv_array_push(&pack_opts, \"--progress\");\n+\telse if (progress == 2)\n+\t\targv_array_push(&pack_opts, \"--all-progress\");\n+\tif (progress && all_progress_implied)\n+\t\targv_array_push(&pack_opts, \"--all-progress-implied\");\n+\n \tif (!startup_info->have_repository)\n \t\tdie(_(\"Need a repository to create a bundle.\"));\n-\treturn !!create_bundle(the_repository, bundle_file, argc, argv);\n+\treturn !!create_bundle(the_repository, bundle_file, argc, argv, &pack_opts);\n }\n \n static int cmd_bundle_verify(int argc, const char **argv, const char *prefix) {\ndiff --git a/bundle.c b/bundle.c\nindex a85ed3f7bc..99439e07a1 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -249,15 +249,16 @@ static int is_tag_in_date_range(struct object *tag, struct rev_info *revs)\n \n \n /* Write the pack data to bundle_fd */\n-static int write_pack_data(int bundle_fd, struct rev_info *revs)\n+static int write_pack_data(int bundle_fd, struct rev_info *revs, struct argv_array *pack_options)\n {\n \tstruct child_process pack_objects = CHILD_PROCESS_INIT;\n \tint i;\n \n \targv_array_pushl(&pack_objects.args,\n-\t\t\t \"pack-objects\", \"--all-progress-implied\",\n+\t\t\t \"pack-objects\",\n \t\t\t \"--stdout\", \"--thin\", \"--delta-base-offset\",\n \t\t\t NULL);\n+\targv_array_pushv(&pack_objects.args, pack_options->argv);\n \tpack_objects.in = -1;\n \tpack_objects.out = bundle_fd;\n \tpack_objects.git_cmd = 1;\n@@ -428,7 +429,7 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs)\n }\n \n int create_bundle(struct repository *r, const char *path,\n-\t\t int argc, const char **argv)\n+\t\t int argc, const char **argv, struct argv_array *pack_options)\n {\n \tstruct lock_file lock = LOCK_INIT;\n \tint bundle_fd = -1;\n@@ -470,7 +471,7 @@ int create_bundle(struct repository *r, const char *path,\n \t\tgoto err;\n \n \t/* write pack */\n-\tif (write_pack_data(bundle_fd, &revs))\n+\tif (write_pack_data(bundle_fd, &revs, pack_options))\n \t\tgoto err;\n \n \tif (!bundle_to_stdout) {\ndiff --git a/bundle.h b/bundle.h\nindex 37c37d7f65..ceab0c7475 100644\n--- a/bundle.h\n+++ b/bundle.h\n@@ -1,6 +1,7 @@\n #ifndef BUNDLE_H\n #define BUNDLE_H\n \n+#include \"argv-array.h\"\n #include \"cache.h\"\n \n struct ref_list {\n@@ -19,7 +20,7 @@ struct bundle_header {\n int is_bundle(const char *path, int quiet);\n int read_bundle_header(const char *path, struct bundle_header *header);\n int create_bundle(struct repository *r, const char *path,\n-\t\t int argc, const char **argv);\n+\t\t int argc, const char **argv, struct argv_array *pack_options);\n int verify_bundle(struct repository *r, struct bundle_header *header, int verbose);\n #define BUNDLE_VERBOSE 1\n int unbundle(struct repository *r, struct bundle_header *header,\n-- \n2.23.0\n\n"},{"id":"385863","messageId":"xmqqlfsni1jh.fsf@gitster-ct.c.googlers.com","threadId":"52232","inReplyTo":"20191110204126.30553-1-robbat2@gentoo.org","subject":"Re: [PATCH v3 1/3] bundle: framework for options before bundle file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-11T02:34:26Z","receivedAt":"2019-11-11T02:34:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Robin H. Johnson\" writes:\n\n> +static int verbose;\n> +\n> +static int parse_options_cmd_bundle(int argc,\n> +\t\tconst char **argv,\n> +\t\tconst char* prefix,\n> +\t\tconst char * const usagestr[],\n> +\t\tconst struct option options[],\n> +\t\tconst char **bundle_file) {\n> +\tint newargc;\n> +\tnewargc = parse_options(argc, argv, NULL, options, usagestr,\n> +\t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n> +\tif (argc < 1)\n> +\t\tusage_with_options(usagestr, options);\n> +\t*bundle_file = prefix_filename(prefix, argv[0]);\n> +\treturn newargc;\n> +}\n\nLooks like a useful helper to be shared among subcommands.\n\n> +static int cmd_bundle_create(int argc, const char **argv, const char *prefix) {\n> +\tstruct option options[] = {\n> +\t\tOPT_END()\n> +\t};\n> +\tconst char* bundle_file;\n> ...\n> +int cmd_bundle(int argc, const char **argv, const char *prefix)\n> +{\n> +\tstruct option options[] = {\n> +\t\tOPT__VERBOSE(&verbose, N_(\"be verbose; must be placed before a subcommand\")),\n> +\t\tOPT_END()\n> +\t};\n> +\tint result;\n> +\n> +\targc = parse_options(argc, argv, prefix, options, builtin_bundle_usage,\n> +\t\tPARSE_OPT_STOP_AT_NON_OPTION);\n\nLooks like a reasonable arrangement for two-level option parser.\n"},{"id":"385867","messageId":"20191111035420.GA6379@sigill.intra.peff.net","threadId":"52232","inReplyTo":"20191110204126.30553-1-robbat2@gentoo.org","subject":"Re: [PATCH v3 1/3] bundle: framework for options before bundle file","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-11T03:54:20Z","receivedAt":"2019-11-11T03:54:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 10, 2019 at 12:41:24PM -0800, Robin H. Johnson wrote:\n\n> Make it possible for any of the git-bundle subcommands to include\n> options:\n> - before the sub-command\n> - after the sub-command, before the bundle filename\n\nOn reading your subject and the start of the commit message, I thought\nyou mean that you meant to allow both:\n\n git bundle --foo create\n\nand\n\n git bundle create --foo\n\nBut looking at the patch, this is about creating two separate sets of\noptions, one for all sub-commands, and one for each individual\nsub-command. That makes sense to me. I don't know if it's worth trying\nto spell that out more explicitly in the commit message.\n\n-Peff\n"},{"id":"385869","messageId":"20191111040750.GB6379@sigill.intra.peff.net","threadId":"52232","inReplyTo":"20191110204126.30553-2-robbat2@gentoo.org","subject":"Re: [PATCH v3 2/3] bundle-create: progress output control","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-11T04:07:50Z","receivedAt":"2019-11-11T04:07:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 10, 2019 at 12:41:25PM -0800, Robin H. Johnson wrote:\n\n> Support the progress output options from pack-objects in git-bundle's\n> create subcommand. Most notably, this provides --quiet as requested on\n> the git mailing list per [1]\n> \n> Reference: https://www.mail-archive.com/git@vger.kernel.org/msg182844.html \n\nI'm glad you included the message-id here, since \"182844\" is useless if\nmail-archive.com ever goes away. We usually just cite public-inbox for\nthat reason, since its URLs just use the message-id anyway:\n\n https://public-inbox.org/git/robbat2-20190806T191156-796782357Z@orbis-terrarum.net\n\n> +--progress::\n> +\tProgress status is reported on the standard error stream\n> +\tby default when it is attached to a terminal, unless -q\n> +\tis specified. This flag forces progress status even if\n> +\tthe standard error stream is not directed to a terminal.\n> +\n> +--all-progress::\n> +\tWhen --stdout is specified then progress report is\n> +\tdisplayed during the object count and compression phases\n> +\tbut inhibited during the write-out phase. The reason is\n> +\tthat in some cases the output stream is directly linked\n> +\tto another command which may wish to display progress\n> +\tstatus of its own as it processes incoming pack data.\n> +\tThis flag is like --progress except that it forces progress\n> +\treport for the write-out phase as well even if --stdout is\n> +\tused.\n> +\n> +--all-progress-implied::\n> +\tThis is used to imply --all-progress whenever progress display\n> +\tis activated. Unlike --all-progress this flag doesn't actually\n> +\tforce any progress display by itself.\n> +\n> +-q::\n> +--quiet::\n> +\tThis flag makes the command not to report its progress\n> +\ton the standard error stream.\n\nDo we need all four of these?\n\nJust saying \"--no-progress\" would do what you want right now. I could\nunderstand the desire for a general \"--quiet\" flag that implies\n\"--no-progress\", and shuts off any other non-progress chatter as well.\nThere isn't any now, but it could be a future proofing thing (plus\nhaving a \"-q\" option is standard). But I think we should document it\nthat way from the outset (though I notice you probably just lifted this\nfrom pack-objects, IMHO it should be more clear, too).\n\nThe \"all-progress\" thing doesn't seem useful at this level. pack-objects\nneeds it so that it can do the right thing when being driven by\nupload-pack versus send-pack. But for a bundle, we're always writing to\na file. We'd always want \"all-progress\" (and that's what the current\ncode does).\n\nLikewise, \"all-progress-implied\" is about setting the \"all-progress\" bit\nbut still letting pack-objects decide whether to show progress based on\nisatty(2). I don't think we'd need that here at all (we check isatty\nourselves, and we'd always want all-progress).\n\nSo could we perhaps simplify this to:\n\n 1. Set show_progress to isatty(2).\n\n 2. Make --progress a parseopt bool, setting show_progress to 1 (or if\n we see \"--no-progress\").\n\n 3. Pass \"--no-progress\" or \"--all-progress\" to pack-objects, based on\n show_progress.\n\n 4. (Optional) Make \"--quiet\" a synonym for \"--no-progress\", with the\n documentation that it may later encompass other messages.\n\n-Peff\n"},{"id":"385870","messageId":"20191111040938.GC6379@sigill.intra.peff.net","threadId":"52232","inReplyTo":"20191110204126.30553-3-robbat2@gentoo.org","subject":"Re: [PATCH v3 3/3] bundle-verify: add --quiet","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-11T04:09:38Z","receivedAt":"2019-11-11T04:09:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 10, 2019 at 12:41:26PM -0800, Robin H. Johnson wrote:\n\n> @@ -97,8 +97,11 @@ static int cmd_bundle_create(int argc, const char **argv, const char *prefix) {\n> static int cmd_bundle_verify(int argc, const char **argv, const char *prefix) {\n> \tstruct bundle_header header;\n> \tint bundle_fd = -1;\n> +\tint quiet = 0;\n> \n> \tstruct option options[] = {\n> +\t\tOPT_BOOL('q', \"quiet\", &quiet,\n> +\t\t\t N_(\"do not show bundle details\")),\n> \t\tOPT_END()\n> \t};\n\nThis --quiet makes much more sense to me (compared to the last patch) as\ndistinct from \"--no-progress\", because it is about quieting non-progress\nchatter.\n\nThere's an OPT__QUIET() macro; should we be using that here?\n\n-Peff\n"},{"id":"385885","messageId":"robbat2-20191111T065646-317702456Z@orbis-terrarum.net","threadId":"52232","inReplyTo":"20191111040750.GB6379@sigill.intra.peff.net","subject":"Re: [PATCH v3 2/3] bundle-create: progress output control","fromName":"Robin H. Johnson","fromEmail":"robbat2@gentoo.org","sentAt":"2019-11-11T07:28:55Z","receivedAt":"2019-11-11T07:29:07Z","isPatch":true,"sender":{"key":"robbat2@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/373898?v=4"},"body":"On Sun, Nov 10, 2019 at 11:07:50PM -0500, Jeff King wrote:\n> On Sun, Nov 10, 2019 at 12:41:25PM -0800, Robin H. Johnson wrote:\n> \n> > Support the progress output options from pack-objects in git-bundle's\n> > create subcommand. Most notably, this provides --quiet as requested on\n> > the git mailing list per [1]\n> > \n> > Reference: https://www.mail-archive.com/git@vger.kernel.org/msg182844.html \n> \n> I'm glad you included the message-id here, since \"182844\" is useless if\n> mail-archive.com ever goes away. We usually just cite public-inbox for\n> that reason, since its URLs just use the message-id anyway:\n> \n> https://public-inbox.org/git/robbat2-20190806T191156-796782357Z@orbis-terrarum.net\n> \n> > +--progress::\n> > +\tProgress status is reported on the standard error stream\n> > +\tby default when it is attached to a terminal, unless -q\n> > +\tis specified. This flag forces progress status even if\n> > +\tthe standard error stream is not directed to a terminal.\n> > +\n> > +--all-progress::\n> > +\tWhen --stdout is specified then progress report is\n> > +\tdisplayed during the object count and compression phases\n> > +\tbut inhibited during the write-out phase. The reason is\n> > +\tthat in some cases the output stream is directly linked\n> > +\tto another command which may wish to display progress\n> > +\tstatus of its own as it processes incoming pack data.\n> > +\tThis flag is like --progress except that it forces progress\n> > +\treport for the write-out phase as well even if --stdout is\n> > +\tused.\n> > +\n> > +--all-progress-implied::\n> > +\tThis is used to imply --all-progress whenever progress display\n> > +\tis activated. Unlike --all-progress this flag doesn't actually\n> > +\tforce any progress display by itself.\n> > +\n> > +-q::\n> > +--quiet::\n> > +\tThis flag makes the command not to report its progress\n> > +\ton the standard error stream.\n> \n> Do we need all four of these?\nI copied the exact set of messages from git-pack-objects, and I do think\nthe same set makes sense specifically to mirror pack-objects for the\nmoment.\n\nstderr is a tty:\nA/(no options) - shorter output\nB/--quiet = no output\nC/--progress - shorter output\nD/--all-progress - longer output\nE/--all-progress-implied - longer output\n\nstderr is not a tty:\nA/(no options) - no output\nB/--quiet = no output\nC/--progress - shorter output\nD/--all-progress - longer output\nE/--all-progress-implied - no output\n\nMapping this to a table for a moment:\n 1 2\nA s n\nB n n\nC s s\nD l l\nE l n\n\n1 = stderr is a tty\n2 = stderr is not a tty\n\ns = short output\nl = long output (includes \"Delta compression...\", \"Writing objects: ..\")\nn = no output\n\nI think there is a lot of room to improve the behavior here, but at the\nrisk of breaking backwards compatibility on the existing options, I\nthink this older set of options should consistent between this and\npack-objects.\n\n--pack-progress-output=[never|short|long] \n--pack-progress-conditional-on-stderr-tty\n(horrible names, but I wanted to convey the intent)\n\n> Just saying \"--no-progress\" would do what you want right now. I could\n> understand the desire for a general \"--quiet\" flag that implies\n> \"--no-progress\", and shuts off any other non-progress chatter as well.\n> There isn't any now, but it could be a future proofing thing (plus\n> having a \"-q\" option is standard). But I think we should document it\n> that way from the outset (though I notice you probably just lifted this\n> from pack-objects, IMHO it should be more clear, too).\nWilling to do later series to add --no-progress to this &\npack-objects as consistency improvement if you'd like for future\nproofing (specifically --quiet would be all output whereas --no-progress\nwould only cut out progress output).\n\n-- \nRobin Hugh Johnson\nGentoo Linux: Dev, Infra Lead, Foundation Treasurer\nE-Mail : robbat2@gentoo.org\nGnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85\nGnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136\n"},{"id":"385886","messageId":"20191111081047.GA17861@sigill.intra.peff.net","threadId":"52232","inReplyTo":"robbat2-20191111T065646-317702456Z@orbis-terrarum.net","subject":"Re: [PATCH v3 2/3] bundle-create: progress output control","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-11T08:10:47Z","receivedAt":"2019-11-11T08:10:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 11, 2019 at 07:28:55AM +0000, Robin H. Johnson wrote:\n\n> > Do we need all four of these?\n> I copied the exact set of messages from git-pack-objects, and I do think\n> the same set makes sense specifically to mirror pack-objects for the\n> moment.\n\nI'm not sure I agree. In what situation would anybody use \"git bundle\ncreate --all-progress-implied\", for example? Literally no other Git\ncommand except pack-objects has \"--all-progress\" or\n\"--all-progress-implied\" (even ones which call pack-objects under the\nhood to print the progress!), and the presence of the latter in\npack-objects is due to a backwards-compatibility thing in the early days\n(where --all-progress did too many things, but we could no longer change\nit). I think it would be a mistake to spread it further.\n\n> I think there is a lot of room to improve the behavior here, but at the\n> risk of breaking backwards compatibility on the existing options, I\n> think this older set of options should consistent between this and\n> pack-objects.\n\nBut now is the moment where we can do what we want without breaking\ncompatibility (since there aren't any progress options for git-bundle at\nall yet).\n\nI guess another way of thinking about it: why is \"pack-objects\" the\nmodel for how its progress options should work, and not \"send-pack\"?\ngit-bundle is much closer to the latter in how users will invoke it.\n\n> > Just saying \"--no-progress\" would do what you want right now. I could\n> > understand the desire for a general \"--quiet\" flag that implies\n> > \"--no-progress\", and shuts off any other non-progress chatter as well.\n> > There isn't any now, but it could be a future proofing thing (plus\n> > having a \"-q\" option is standard). But I think we should document it\n> > that way from the outset (though I notice you probably just lifted this\n> > from pack-objects, IMHO it should be more clear, too).\n> Willing to do later series to add --no-progress to this &\n\nYou already added --no-progress (and it's already there in\npack-objects). It comes for free with OPT_SET_INT(\"progress\").\n\n-Peff\n"},{"id":"385891","messageId":"nycvar.QRO.7.76.6.1911110943240.46@tvgsbejvaqbjf.bet","threadId":"52232","inReplyTo":"20191110204126.30553-1-robbat2@gentoo.org","subject":"Re: [PATCH v3 1/3] bundle: framework for options before bundle file","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-11-11T08:46:58Z","receivedAt":"2019-11-11T08:47:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Robin,\n\nOn Sun, 10 Nov 2019, Robin H. Johnson wrote:\n\n> Make it possible for any of the git-bundle subcommands to include\n> options:\n> - before the sub-command\n> - after the sub-command, before the bundle filename\n>\n> There is an immediate gain in support for help with all of the\n> sub-commands, where 'git bundle list-heads -h' previously returned an\n> error.\n>\n> Downside here is an increase in code duplication that cannot be\n> trivially avoided short of shared global static options.\n>\n> Signed-off-by: Robin H. Johnson \n> ---\n> builtin/bundle.c | 190 ++++++++++++++++++++++++++++++++++++-----------\n> 1 file changed, 145 insertions(+), 45 deletions(-)\n>\n> I tried doing this via GitGitGadget initially as a test of that process,\n> as well as the CI integration side; however as noted in #git-devel and\n> elsewhere on the list, vger seems to swallow the mail to /dev/null\n\nI am very sorry for the woes, and I have to admit that I _still_ have no\nclue what is going wrong there.\n\nThe mail was sent correctly by GMail, at least it reports that, and the\nmbox of the cover letter reads like this (maybe anybody else has a clue\nwhy vger thinks it okay to just drop the mail without further notice?):\n\n-- snipsnap --\nReturn-Path: \nReceived: from [127.0.0.1] ([13.74.141.28])\n by smtp.gmail.com with ESMTPSA id t1sm9512770wrn.81.2019.11.08.13.30.41\n (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n Fri, 08 Nov 2019 13:30:41 -0800 (PST)\nMessage-Id: \nIn-Reply-To: \nReferences: \nFrom: \"\"Robin H. Johnson\" via GitGitGadget\" \nDate: Fri, 08 Nov 2019 21:30:37 +0000\nSubject: [PATCH v2 0/3] git-bundle --quiet support\nFcc: Sent\nContent-Type: text/plain; charset=UTF-8\nContent-Transfer-Encoding: 8bit\nMIME-Version: 1.0\nTo: git@vger.kernel.org\nCc: \"Robin H. Johnson\" ,\n Junio C Hamano \n\nImplement --quiet support for some git-bundle subcommands: create and verify\n\nReference: https://www.mail-archive.com/git@vger.kernel.org/msg182844.html\nSigned-off-by: Robin H. Johnson robbat2@gentoo.org [robbat2@gentoo.org]\n\nRobin H. Johnson (3):\n bundle: framework for options before bundle file\n bundle-create: progress output control\n bundle-verify: add --quiet\n\n Documentation/git-bundle.txt | 35 +++++-\n builtin/bundle.c | 217 +++++++++++++++++++++++++++--------\n bundle.c | 9 +-\n bundle.h | 3 +-\n 4 files changed, 211 insertions(+), 53 deletions(-)\n\n\nbase-commit: 566a1439f6f56c2171b8853ddbca0ad3f5098770\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-435%2Frobbat2%2Fsilent-bundle-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-435/robbat2/silent-bundle-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/435\n\nRange-diff vs v1:\n\n 1: c48ff9adbb = 1: 1f7f0aa1e8 bundle: framework for options before bundle file\n 2: 5678de06f3 ! 2: 468922581b bundle-create: progress output control\n @@ -6,9 +6,7 @@\n create subcommand. Most notably, this provides --quiet as requested on\n the git mailing list per [1]\n\n - [1] \n -\n - Reference: https://www.mail-archive.com/git@vger.kernel.org/msg182844.html\n + Reference: https://www.mail-archive.com/git@vger.kernel.org/msg182844.html \n Signed-off-by: Robin H. Johnson \n\n diff --git a/Documentation/git-bundle.txt b/Documentation/git-bundle.txt\n 3: 01317800c5 ! 3: 26c7b28309 bundle-verify: add --quiet\n @@ -2,11 +2,9 @@\n\n bundle-verify: add --quiet\n\n - Add --quiet to git-bundle verify as proposed per [1]\n + Add --quiet to git-bundle verify as proposed on the mailing list [1].\n\n - [1] \n -\n - Reference: https://www.mail-archive.com/git@vger.kernel.org/msg182844.html\n + Reference: https://www.mail-archive.com/git@vger.kernel.org/msg182844.html \n Signed-off-by: Robin H. Johnson \n\n diff --git a/Documentation/git-bundle.txt b/Documentation/git-bundle.txt\n\n--\ngitgitgadget\n"},{"id":"385893","messageId":"xmqqzhh2hjnx.fsf@gitster-ct.c.googlers.com","threadId":"52232","inReplyTo":"nycvar.QRO.7.76.6.1911110943240.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v3 1/3] bundle: framework for options before bundle file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-11T09:00:34Z","receivedAt":"2019-11-11T09:00:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> The mail was sent correctly by GMail, at least it reports that, and the\n> mbox of the cover letter reads like this (maybe anybody else has a clue\n> why vger thinks it okay to just drop the mail without further notice?):\n\n> From: \"\"Robin H. Johnson\" via GitGitGadget\" \n\nHow does that doubled double quote work?\n"},{"id":"385894","messageId":"xmqqv9rqhjly.fsf@gitster-ct.c.googlers.com","threadId":"52232","inReplyTo":"20191111081047.GA17861@sigill.intra.peff.net","subject":"Re: [PATCH v3 2/3] bundle-create: progress output control","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-11T09:01:45Z","receivedAt":"2019-11-11T09:01:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Nov 11, 2019 at 07:28:55AM +0000, Robin H. Johnson wrote:\n>\n>> > Do we need all four of these?\n>> I copied the exact set of messages from git-pack-objects, and I do think\n>> the same set makes sense specifically to mirror pack-objects for the\n>> moment.\n>\n> I'm not sure I agree. In what situation would anybody use \"git bundle\n> create --all-progress-implied\", for example? Literally no other Git\n> command except pack-objects has \"--all-progress\" or\n> \"--all-progress-implied\" (even ones which call pack-objects under the\n> hood to print the progress!), and the presence of the latter in\n> pack-objects is due to a backwards-compatibility thing in the early days\n> (where --all-progress did too many things, but we could no longer change\n> it). I think it would be a mistake to spread it further.\n\nI am quite cure I agree with your reasoning that we would want to\nlimit the \"--all-progress-implied\" craziness from spreading ;-)\n\nThanks.\n"},{"id":"386019","messageId":"nycvar.QRO.7.76.6.1911111918410.46@tvgsbejvaqbjf.bet","threadId":"52232","inReplyTo":"xmqqzhh2hjnx.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 1/3] bundle: framework for options before bundle file","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-11-12T15:09:01Z","receivedAt":"2019-11-12T15:09:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 11 Nov 2019, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n>\n> > The mail was sent correctly by GMail, at least it reports that, and the\n> > mbox of the cover letter reads like this (maybe anybody else has a clue\n> > why vger thinks it okay to just drop the mail without further notice?):\n>\n> > From: \"\"Robin H. Johnson\" via GitGitGadget\" \n>\n> How does that doubled double quote work?\n\nWow. I looked through the mbox three times, and I managed to overlook\nthis, still. Talk about bias.\n\nThanks,\nDscho\n"}]}