[PATCH v10 00/12] merge-index: prepare to rewrite merge drivers in C

From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Taylor Blau" <me@ttaylorr.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Alban Gruin" <alban.gruin@gmail.com>,
	"Phillip Wood" <phillip.wood123@gmail.com>,
	"Elijah Newren" <newren@gmail.com>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v10 00/12] merge-index: prepare to rewrite merge drivers in C
Date: Thu, 15 Dec 2022 09:52:04 +0100	[thread overview]
Message-ID: <cover-v10-00.12-00000000000-20221215T084803Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-v9-00.12-00000000000-20221118T110058Z-avarab@gmail.com>

This is a prep series for a re-roll of Alban Gruin's series to rewrite
various merge drivers from *.sh to *.c, and being able to call those
in-process.

That series was discussed on-list in August[1], and has now been
ejected from "seen" due to staleness. This v10 re-roll is my second
attempt at re-starting this topic, see [2] for v9.

In v8 there were concerns with the later part of this topic, but the
parts that are included here weren't controversial, those will be part
2 (and I think I've addressed those concerns).

Changes since v9:

* Rebase on minor (unrelated) merge-index and
  "USE_THE_INDEX_COMPATIBILITY_MACROS" changes that have since landed.

* Trivial adjustments to error messages, including marking one that
  wasn't marked with _() for translation.

See [3] for my branch for this topic, which includes passing CI.

1. https://lore.kernel.org/git/20220809185429.20098-9-alban.gruin@gmail.com/
2. https://lore.kernel.org/git/cover-v9-00.12-00000000000-20221118T110058Z-avarab@gmail.com/
3. https://github.com/avar/git/tree/ag/merge-strategies-in-c-prep-3

Alban Gruin (4):
  t6060: modify multiple files to expose a possible issue with
    merge-index
  t6060: add tests for removed files
  merge-index: improve die() error messages
  merge-index: libify merge_one_path() and merge_all()

Ævar Arnfjörð Bjarmason (8):
  merge-index doc & -h: fix padding, labels and "()" use
  merge-index tests: add usage tests
  merge-index: migrate to parse_options() API
  merge-index i18n: mark die() messages for translation
  merge-index: stop calling ensure_full_index() twice
  builtin/merge-index.c: don't USE_THE_INDEX_VARIABLE
  merge-index: use "struct strvec" and helper to prepare args
  merge-index: make the argument parsing sensible & simpler

 Documentation/git-merge-index.txt |   2 +-
 Makefile                          |   1 +
 builtin/merge-index.c             | 167 ++++++++++++++----------------
 git.c                             |   2 +-
 merge-strategies.c                |  87 ++++++++++++++++
 merge-strategies.h                |  19 ++++
 t/t0450/txt-help-mismatches       |   1 -
 t/t6060-merge-index.sh            |  65 +++++++++++-
 8 files changed, 249 insertions(+), 95 deletions(-)
 create mode 100644 merge-strategies.c
 create mode 100644 merge-strategies.h

Range-diff against v9:
 1:  660b1242707 !  1:  9240ab10649 merge-index doc & -h: fix padding, labels and "()" use
    @@ builtin/merge-index.c: int cmd_merge_index(int argc, const char **argv, const ch
     -		usage("git merge-index [-o] [-q] <merge-program> (-a | [--] [<filename>...])");
     +		usage("git merge-index [-o] [-q] <merge-program> (-a | ([--] <file>...))");

    - 	read_cache();
    + 	repo_read_index(the_repository);

      ## t/t0450/txt-help-mismatches ##
 2:  caf4a3790c4 =  2:  de36b52286b t6060: modify multiple files to expose a possible issue with merge-index
 3:  d659ac983f8 =  3:  5edc8132329 t6060: add tests for removed files
 4:  7c5b7c36411 =  4:  aa731011e0a merge-index tests: add usage tests
 5:  07f6936011a !  5:  a3f69564ac5 merge-index: migrate to parse_options() API
    @@ Commit message

      ## builtin/merge-index.c ##
     @@
    - #define USE_THE_INDEX_COMPATIBILITY_MACROS
    + #define USE_THE_INDEX_VARIABLE
      #include "builtin.h"
     +#include "parse-options.h"
      #include "run-command.h"
    @@ builtin/merge-index.c: int cmd_merge_index(int argc, const char **argv, const ch
     +		usage_msg_opt(_("'-a' and '<file>...' are mutually exclusive"),
     +			      usage, options);

    - 	read_cache();
    + 	repo_read_index(the_repository);

      	/* TODO: audit for interaction with sparse-index. */
      	ensure_full_index(&the_index);
 6:  8d6cfd4bacc !  6:  324368401a2 merge-index: improve die() error messages
    @@ builtin/merge-index.c
     @@ builtin/merge-index.c: static int merge_entry(int pos, const char *path)
      	struct child_process cmd = CHILD_PROCESS_INIT;

    - 	if (pos >= active_nr)
    + 	if (pos >= the_index.cache_nr)
     -		die("git merge-index: %s not in the cache", path);
     +		die("'%s' is not in the cache", path);
      	found = 0;
      	do {
    - 		const struct cache_entry *ce = active_cache[pos];
    + 		const struct cache_entry *ce = the_index.cache[pos];
     @@ builtin/merge-index.c: static int merge_entry(int pos, const char *path)
      		arguments[stage + 4] = ownbuf[stage];
    - 	} while (++pos < active_nr);
    + 	} while (++pos < the_index.cache_nr);
      	if (!found)
     -		die("git merge-index: %s not in the cache", path);
     +		die("'%s' is not in the cache", path);
 7:  62c5fd4faaa !  7:  de4d11798db merge-index i18n: mark die() messages for translation
    @@ builtin/merge-index.c
     @@ builtin/merge-index.c: static int merge_entry(int pos, const char *path)
      	struct child_process cmd = CHILD_PROCESS_INIT;

    - 	if (pos >= active_nr)
    + 	if (pos >= the_index.cache_nr)
     -		die("'%s' is not in the cache", path);
     +		die(_("'%s' is not in the cache"), path);
      	found = 0;
      	do {
    - 		const struct cache_entry *ce = active_cache[pos];
    + 		const struct cache_entry *ce = the_index.cache[pos];
     @@ builtin/merge-index.c: static int merge_entry(int pos, const char *path)
      		arguments[stage + 4] = ownbuf[stage];
    - 	} while (++pos < active_nr);
    + 	} while (++pos < the_index.cache_nr);
      	if (!found)
     -		die("'%s' is not in the cache", path);
     +		die(_("'%s' is not in the cache"), path);
 8:  e44d58a505a !  8:  45cf7995448 merge-index: stop calling ensure_full_index() twice
    @@ builtin/merge-index.c: static void merge_one_path(const char *path)
     -	/* TODO: audit for interaction with sparse-index. */
     -	ensure_full_index(&the_index);
     +
    - 	for (i = 0; i < active_nr; i++) {
    - 		const struct cache_entry *ce = active_cache[i];
    + 	for (i = 0; i < the_index.cache_nr; i++) {
    + 		const struct cache_entry *ce = the_index.cache[i];
      		if (!ce_stage(ce))
     @@ builtin/merge-index.c: int cmd_merge_index(int argc, const char **argv, const char *prefix)
      	/* TODO: audit for interaction with sparse-index. */
 9:  1f7c941035d !  9:  fc9a05ee034 builtin/merge-index.c: don't USE_THE_INDEX_COMPATIBILITY_MACROS
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

      ## Commit message ##
    -    builtin/merge-index.c: don't USE_THE_INDEX_COMPATIBILITY_MACROS
    +    builtin/merge-index.c: don't USE_THE_INDEX_VARIABLE

    -    Remove "USE_THE_INDEX_COMPATIBILITY_MACROS" and instead pass
    -    "the_index" around between the functions in this file. In a subsequent
    -    commit we'll libify this, and don't want to use
    -    "USE_THE_INDEX_COMPATIBILITY_MACROS" in any more places in the
    -    top-level *.c files. Doing this first makes that diff a lot smaller.
    +    Remove "USE_THE_INDEX_VARIABLE" and instead pass "the_index" around
    +    between the functions in this file. In a subsequent commit we'll
    +    libify this, and don't want to use "USE_THE_INDEX_VARIABLE" in any
    +    more places in the top-level *.c files. Doing this first makes that
    +    diff a lot smaller.

         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

      ## builtin/merge-index.c ##
     @@
    --#define USE_THE_INDEX_COMPATIBILITY_MACROS
    +-#define USE_THE_INDEX_VARIABLE
      #include "builtin.h"
      #include "parse-options.h"
      #include "run-command.h"
    @@ builtin/merge-index.c: static int merge_entry(int pos, const char *path)
      	char ownbuf[4][60];
      	struct child_process cmd = CHILD_PROCESS_INIT;

    --	if (pos >= active_nr)
    +-	if (pos >= the_index.cache_nr)
     +	if (pos >= istate->cache_nr)
      		die(_("'%s' is not in the cache"), path);
      	found = 0;
      	do {
    --		const struct cache_entry *ce = active_cache[pos];
    +-		const struct cache_entry *ce = the_index.cache[pos];
     +		const struct cache_entry *ce = istate->cache[pos];
      		int stage = ce_stage(ce);

    @@ builtin/merge-index.c: static int merge_entry(int pos, const char *path)
      		xsnprintf(ownbuf[stage], sizeof(ownbuf[stage]), "%o", ce->ce_mode);
      		arguments[stage] = hexbuf[stage];
      		arguments[stage + 4] = ownbuf[stage];
    --	} while (++pos < active_nr);
    +-	} while (++pos < the_index.cache_nr);
     +	} while (++pos < istate->cache_nr);
      	if (!found)
      		die(_("'%s' is not in the cache"), path);
    @@ builtin/merge-index.c: static int merge_entry(int pos, const char *path)
     -static void merge_one_path(const char *path)
     +static void merge_one_path(struct index_state *istate, const char *path)
      {
    --	int pos = cache_name_pos(path, strlen(path));
    +-	int pos = index_name_pos(&the_index, path, strlen(path));
     +	int pos = index_name_pos(istate, path, strlen(path));

      	/*
    @@ builtin/merge-index.c: static int merge_entry(int pos, const char *path)
      {
      	int i;

    --	for (i = 0; i < active_nr; i++) {
    --		const struct cache_entry *ce = active_cache[i];
    +-	for (i = 0; i < the_index.cache_nr; i++) {
    +-		const struct cache_entry *ce = the_index.cache[i];
     +	for (i = 0; i < istate->cache_nr; i++) {
     +		const struct cache_entry *ce = istate->cache[i];
      		if (!ce_stage(ce))
    @@ builtin/merge-index.c: static int merge_entry(int pos, const char *path)
      }

     @@ builtin/merge-index.c: int cmd_merge_index(int argc, const char **argv, const char *prefix)
    - 		usage_msg_opt(_("'-a' and '<file>...' are mutually exclusive"),
    - 			      usage, options);
    - 
    --	read_cache();
    -+	repo_read_index(the_repository);
    + 	repo_read_index(the_repository);

      	/* TODO: audit for interaction with sparse-index. */
     -	ensure_full_index(&the_index);
10:  8c43b64dec4 = 10:  0efc5039e46 merge-index: libify merge_one_path() and merge_all()
11:  592db883dad = 11:  748fef4434f merge-index: use "struct strvec" and helper to prepare args
12:  5a2c4dd3acf = 12:  40b6d296f3a merge-index: make the argument parsing sensible & simpler
-- 
2.39.0.rc2.1048.g0e5493b8d5b