mbox series

[v9,0/4] Add cat-file --batch-command flag

Message ID pull.1212.v9.git.git.1645045157.gitgitgadget@gmail.com (mailing list archive)
Headers show
Series Add cat-file --batch-command flag | expand

Message

Philippe Blain via GitGitGadget Feb. 16, 2022, 8:59 p.m. UTC
The feature proposal of adding a command interface to cat-file was first
discussed in [A]. In [B], Taylor expressed the need for a fuller proposal
before moving forward with a new flag. An RFC was created [C] and the idea
was discussed more thoroughly, and overall it seemed like it was headed in
the right direction.

This patch series consolidates the feedback from these different threads.

This patch series has four parts:

 1. preparation patch to rename a variable
 2. adding an enum to keep track of batch modes
 3. add a remove_timestamp() helper that takes stdin and removes timestamps
 4. logic to handle --batch-command flag, adding contents, info, flush
    commands

Changes since v8

 * have caller free line through a helper function for the sake of
   separation of concerns

Changes since v7

 * revert back to having caller set nr to 0
 * add comment before dispatch_calls to clarify usage of helper
 * rename prefix->name

Changes since v6 (thanks to Eric's feedback)

 * allow command parsing logic to handle the case of flush as well
 * fixed documentation by adding --batch-command to the synopsis and
   adjusting tick marks
 * set nr=0 within helper function

Changes since v5

 * replaced flush tests that used fifo pipes to using a GIT_TEST_ env
   variable to control whether or not --batch-command flushes on exit.
 * added remove_timestamp helper in tests.
 * added documentation to show format can be used with --batch-command

Changes since v4

 * added Phillip's suggested test for testing flush. This should have
   addressed the flaky test that was hanging. I tested it on my side and
   wasn't able to reproduce the deadlock.
 * plugged some holes in the logic that parsed the command and arguments,
   thanks to Eric's feedback
 * fixed verbiage in commit messages per Christian's feedback
 * clarified places in documentation that should mention --batch-command per
   Eric's feedback

Changes since v3 (thanks to Junio's feedback):

 * added cascading logic in batch_options_callback()
 * free memory for queued call input lines
 * do not throw error when flushing an empty queue
 * renamed cmds array to singular queued_cmd
 * fixed flaky test that failed --stress

Changes since v2:

 * added enum to keep track of which batch mode we are in (thanks to Junio's
   feedback)
 * fixed array allocation logic (thanks to Junio's feedback)
 * added code to flush commands when --batch-commands receives an EOF and
   exits (thanks to Phillip's feedback)
 * fixed docs formatting (thanks to Jonathan's feedback)

Changes since v1:

 * simplified "session" mechanism. "flush" will execute all commands that
   were entered in since the last "flush" when in --buffer mode
 * when not in --buffer mode, each command is executed and flushed each time
 * rename cmdmode to transform_mode instead of just mode
 * simplified command parsing logic
 * changed rename of cmdmode to transform_mode
 * clarified verbiage in commit messages

A. https://lore.kernel.org/git/xmqqk0hitnkc.fsf@gitster.g/ B.
https://lore.kernel.org/git/YehomwNiIs0l83W7@nand.local/ C.
https://lore.kernel.org/git/e75ba9ea-fdda-6e9f-4dd6-24190117d93b@gmail.com/

John Cai (4):
  cat-file: rename cmdmode to transform_mode
  cat-file: introduce batch_mode enum to replace print_contents
  cat-file: add remove_timestamp helper
  cat-file: add --batch-command mode

 Documentation/git-cat-file.txt |  42 +++++++-
 builtin/cat-file.c             | 181 ++++++++++++++++++++++++++++++---
 t/README                       |   3 +
 t/t1006-cat-file.sh            | 122 ++++++++++++++++++++--
 4 files changed, 326 insertions(+), 22 deletions(-)


base-commit: b80121027d1247a0754b3cc46897fee75c050b44
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1212%2Fjohn-cai%2Fjc-cat-file-batch-command-v9
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1212/john-cai/jc-cat-file-batch-command-v9
Pull-Request: https://github.com/git/git/pull/1212

Range-diff vs v8:

 1:  fa6294387ab = 1:  76d6e4fe517 cat-file: rename cmdmode to transform_mode
 2:  1a038097bfc = 2:  12084a335cb cat-file: introduce batch_mode enum to replace print_contents
 3:  486ee847816 = 3:  bf74b6cd75b cat-file: add remove_timestamp helper
 4:  8edf80574b8 ! 4:  dbe194f8a85 cat-file: add --batch-command mode
     @@ builtin/cat-file.c: static int batch_unordered_packed(const struct object_id *oi
      +	batch_one_object(line, output, opt, data);
      +}
      +
     -+/* Loop through each queued_cmd, dispatch the function, free the
     -+ * memory associated with line so the cmd array can be reused.
     -+ * Callers must set nr back to 0 in order to reuse the cmd array.
     -+ */
      +static void dispatch_calls(struct batch_options *opt,
      +		struct strbuf *output,
      +		struct expand_data *data,
     @@ builtin/cat-file.c: static int batch_unordered_packed(const struct object_id *oi
      +	if (!opt->buffer_output)
      +		die(_("flush is only for --buffer mode"));
      +
     -+	for (i = 0; i < nr; i++) {
     ++	for (i = 0; i < nr; i++)
      +		cmd[i].fn(opt, cmd[i].line, output, data);
     -+		FREE_AND_NULL(cmd[i].line);
     -+	}
      +
      +	fflush(stdout);
      +}
      +
     ++static void free_cmds(struct queued_cmd *cmd, int nr)
     ++{
     ++	int i;
     ++
     ++	for (i = 0; i < nr; i++)
     ++		FREE_AND_NULL(cmd[i].line);
     ++}
     ++
     ++
      +static const struct parse_cmd {
      +	const char *name;
      +	parse_cmd_fn_t fn;
     @@ builtin/cat-file.c: static int batch_unordered_packed(const struct object_id *oi
      +
      +		if (!strcmp(cmd->name, "flush")) {
      +			dispatch_calls(opt, output, data, queued_cmd, nr);
     ++			free_cmds(queued_cmd, nr);
      +			nr = 0;
      +			continue;
      +		}
     @@ builtin/cat-file.c: static int batch_unordered_packed(const struct object_id *oi
      +
      +	if (opt->buffer_output &&
      +	    nr &&
     -+	    !git_env_bool("GIT_TEST_CAT_FILE_NO_FLUSH_ON_EXIT", 0))
     ++	    !git_env_bool("GIT_TEST_CAT_FILE_NO_FLUSH_ON_EXIT", 0)) {
      +		dispatch_calls(opt, output, data, queued_cmd, nr);
     ++		free_cmds(queued_cmd, nr);
     ++	}
      +
      +	free(queued_cmd);
      +	strbuf_release(&input);
     @@ builtin/cat-file.c: static int batch_objects(struct batch_options *opt)
      +		batch_objects_command(opt, &output, &data);
      +		goto cleanup;
      +	}
     ++
       	while (strbuf_getline(&input, stdin) != EOF) {
       		if (data.split_on_whitespace) {
       			/*