[PATCH v2 0/6] Avoid the_repository in merge-ort and replay

["Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>]

Changes since v1:

 * Add a preparatory patch removing the_repository check from blob
   prefetching in both merge-ort and diff*; it's no longer necessary
 * Fix casing mismatch
 * Simplify the hammer a bit based on the new first patch, but add some
   simple comments explaining it

Remove explicit uses of the_repository and the_hash_algo from merge-ort, and
since this has now been done multiple times for both merge-ort and replay,
implement a small measure to prevent them from returning to either merge-ort
or replay.

See
https://lore.kernel.org/git/CABPp-BH7E1Bh2g0vR3T4NEsv34DvFQPzMuJSsqtOAaWY-fFCxg@mail.gmail.com/
and
https://lore.kernel.org/git/CABPp-BFuwvqiCTCCpoyT6em9_1-qrgPWHWhrufQ3UuZ+Kfkb6A@mail.gmail.com/
for recent discussions on these.

As noted in the comments on v1, I actually do not know why
prefetch_for_content_merges() needs to use the_repository. When I introduced
it back in 2bff554b23e8 (merge-ort: add prefetching for content merges,
2021-06-22), I was just looking at diffcore_std() and trying to mimic how it
did the prefetch, and it has such a comparison. If anyone knows why
diffcore_std() needs to compare against the_repository, I'd love to hear...

Series overview: Patches 1-3: Mostly mechanical removal of existing uses
Patches 4-5: Simple hammer to prevent the problem from returning

Elijah Newren (6):
  merge,diff: remove the_repository check before prefetching blobs
  merge-ort: pass repository to write_tree()
  merge-ort: replace the_repository with opt->repo
  merge-ort: replace the_hash_algo with opt->repo->hash_algo
  merge-ort: prevent the_repository from coming back
  replay: prevent the_repository from coming back

 diff.c            |  2 +-
 diffcore-break.c  |  2 +-
 diffcore-rename.c |  4 +-
 merge-ort.c       | 94 +++++++++++++++++++++++++----------------------
 replay.c          |  6 +++
 5 files changed, 61 insertions(+), 47 deletions(-)


base-commit: 73fd77805fc6406f31c36212846d9e2541d19321
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2048%2Fnewren%2Favoid_the_repository-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2048/newren/avoid_the_repository-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2048

Range-diff vs v1:

 -:  ---------- > 1:  7155a0da6f merge,diff: remove the_repository check before prefetching blobs
 1:  620c4ea38b = 2:  911cba991b merge-ort: pass repository to write_tree()
 2:  abba4bd762 ! 3:  68af47ed18 merge-ort: replace the_repository with opt->repo
     @@ merge-ort.c: static void prefetch_for_content_merges(struct merge_options *opt,
       	struct string_list_item *e;
       	struct oid_array to_fetch = OID_ARRAY_INIT;
       
     --	if (opt->repo != the_repository || !repo_has_promisor_remote(the_repository))
     -+	if (opt->repo != the_repository || !repo_has_promisor_remote(opt->repo))
     +-	if (!repo_has_promisor_remote(the_repository))
     ++	if (!repo_has_promisor_remote(opt->repo))
       		return;
       
       	for (e = &plist->items[plist->nr-1]; e >= plist->items; --e) {
 3:  36c2713ceb ! 4:  bfa68716af merge-ort: replace the_hash_algo with opt->repo->hash_algo
     @@ Metadata
       ## Commit message ##
          merge-ort: replace the_hash_algo with opt->repo->hash_algo
      
     +    We have a perfectly valid repository available and do not need to use
     +    the_hash_algo (a shorthand for the_repository->hash_algo), so use the
     +    known repository instead.
     +
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## merge-ort.c ##
 4:  46c24e0d05 ! 5:  932d945c9b merge-ort: prevent the_repository from coming back
     @@ Metadata
       ## Commit message ##
          merge-ort: prevent the_repository from coming back
      
     -    There are two things preventing us from removing our usage of
     -    USE_THE_REPOSITORY_VARIABLE: one necessary use of the_repository in
     -    prefetch_for_content_merges(), and the use of DEFAULT_ABBREV.  We have
     -    removed all other uses of the_repository in merge-ort before (multiple
     -    times), but without removing that definition, they keep coming back.
     +    Due to the use of DEFAULT_ABBREV, we cannot get rid of our usage of
     +    USE_THE_REPOSITORY_VARIABLE.  However, we have removed all other uses of
     +    the_repository in merge-ort a few times.  But they keep coming back.
      
          Define the_repository to make it a compilation error so that they don't
     -    come back any more, with a special carve-out for
     -    prefetch_for_content_merges().
     +    come back any more.
      
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
     @@ merge-ort.c
       #include "unpack-trees.h"
       #include "xdiff-interface.h"
       
     ++/*
     ++ * We technically need USE_THE_REPOSITORY_VARIABLE above for DEFAULT_ABBREV,
     ++ * but do not want more uses of the_repository.  Prevent them.
     ++ *
     ++ * opt->repo is available; use it instead.
     ++ */
      +#define the_repository DO_NOT_USE_THE_REPOSITORY
      +
       /*
        * We have many arrays of size 3.  Whenever we have such an array, the
        * indices refer to one of the sides of the three-way merge.  This is so
     -@@ merge-ort.c: static int process_entry(struct merge_options *opt,
     - 	return 0;
     - }
     - 
     -+#undef the_repository
     -+
     - static void prefetch_for_content_merges(struct merge_options *opt,
     - 					struct string_list *plist)
     - {
     -@@ merge-ort.c: static void prefetch_for_content_merges(struct merge_options *opt,
     - 	oid_array_clear(&to_fetch);
     - }
     - 
     -+#define the_repository DO_NOT_USE_the_repository
     -+
     - static int process_entries(struct merge_options *opt,
     - 			   struct object_id *result_oid)
     - {
 5:  d75a71aef9 ! 6:  67db46f34f replay: prevent the_repository from coming back
     @@ replay.c
       #include "strmap.h"
       #include "tree.h"
       
     ++/*
     ++ * We technically need USE_THE_REPOSITORY_VARIABLE for DEFAULT_ABBREV, but
     ++ * do not want to use the_repository.
     ++ */
      +#define the_repository DO_NOT_USE_THE_REPOSITORY
      +
       static const char *short_commit_name(struct repository *repo,

-- 
gitgitgadget