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

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

Changes since v2:

 * In first patch, actually avoid the_repository when attempting to remove
   check against the_repository
 * Fix commit message of patch 3 due to the new patch 1.
 * Slight tweak to commit message of patch 6.

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-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2048/newren/avoid_the_repository-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2048

Range-diff vs v2:

 1:  7155a0da6f ! 1:  e75334a5cf merge,diff: remove the_repository check before prefetching blobs
     @@ diff.c: void diffcore_std(struct diff_options *options)
       	 * decides that it needs inexact rename detection.
       	 */
      -	if (options->repo == the_repository && repo_has_promisor_remote(the_repository) &&
     -+	if (repo_has_promisor_remote(the_repository) &&
     ++	if (repo_has_promisor_remote(options->repo) &&
       	    (options->output_format & output_formats_to_prefetch ||
       	     options->pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))
       		diff_queued_diff_prefetch(options->repo);
     @@ diffcore-break.c: static int should_break(struct repository *r,
       		return 0; /* they are the same */
       
      -	if (r == the_repository && repo_has_promisor_remote(the_repository)) {
     -+	if (repo_has_promisor_remote(the_repository)) {
     ++	if (repo_has_promisor_remote(r)) {
       		options.missing_object_cb = diff_queued_diff_prefetch;
       		options.missing_object_data = r;
       	}
     @@ diffcore-rename.c: static int find_basename_matches(struct diff_options *options
       	}
       
      -	if (options->repo == the_repository && repo_has_promisor_remote(the_repository)) {
     -+	if (repo_has_promisor_remote(the_repository)) {
     ++	if (repo_has_promisor_remote(options->repo)) {
       		dpf_options.missing_object_cb = basename_prefetch;
       		dpf_options.missing_object_data = &prefetch_options;
       	}
     @@ diffcore-rename.c: void diffcore_rename_extended(struct diff_options *options,
       	/* Finish setting up dpf_options */
       	prefetch_options.skip_unmodified = skip_unmodified;
      -	if (options->repo == the_repository && repo_has_promisor_remote(the_repository)) {
     -+	if (repo_has_promisor_remote(the_repository)) {
     ++	if (repo_has_promisor_remote(options->repo)) {
       		dpf_options.missing_object_cb = inexact_prefetch;
       		dpf_options.missing_object_data = &prefetch_options;
       	}
     @@ merge-ort.c: static void prefetch_for_content_merges(struct merge_options *opt,
       	struct oid_array to_fetch = OID_ARRAY_INIT;
       
      -	if (opt->repo != the_repository || !repo_has_promisor_remote(the_repository))
     -+	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) {
 2:  911cba991b = 2:  a9a9d422a3 merge-ort: pass repository to write_tree()
 3:  68af47ed18 ! 3:  4ebfcb08a5 merge-ort: replace the_repository with opt->repo
     @@ Commit message
          merge-ort: replace the_repository with opt->repo
      
          We have a perfectly valid repository available and do not need to use
     -    the_repository, except for one location in
     -    prefetch_for_content_merges().
     +    the_repository.
      
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
     @@ merge-ort.c: static int read_oid_strbuf(struct merge_options *opt,
       	if (!buf) {
       		path_msg(opt, ERROR_OBJECT_READ_FAILED, 0,
       			 path, NULL, NULL, NULL,
     -@@ 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 (!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) {
      @@ merge-ort.c: static int checkout(struct merge_options *opt,
       	unpack_opts.verbose_update = (opt->verbosity > 2);
       	unpack_opts.fn = twoway_merge;
 4:  bfa68716af = 4:  09076d81b6 merge-ort: replace the_hash_algo with opt->repo->hash_algo
 5:  932d945c9b = 5:  42a2576878 merge-ort: prevent the_repository from coming back
 6:  67db46f34f ! 6:  0654d04584 replay: prevent the_repository from coming back
     @@ Commit message
          coming back.
      
          Define the_repository to make it a compilation error so that they don't
     -    come back any more.
     +    come back any more; the repo parameter plumbed through the various
     +    functions can be used instead.
      
          Signed-off-by: Elijah Newren <newren@gmail.com>
      

-- 
gitgitgadget