Re: [PATCH v3 4/5] merge-ort: shuffle the computation and cleanup of potential collisions

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, Jul 01 2022, Elijah Newren via GitGitGadget wrote:

> From: Elijah Newren <newren@gmail.com>
>
> Run compute_collisions() for renames on both sides of history before
> any calls to collect_renames(), and do not free the computed collisions
> until after both calls to collect_renames().  This is just a code
> reorganization at this point that doesn't make sense on its own, but
> will permit us to use the computed collision info from both sides
> within each call to collect_renames() in a subsequent commit.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>

This is much easier to read & review with the prep patches, thanks a
lot.

B.t.w. on the "legacy code" comment wrt merge-{ort,recursive}.c : I
didn't look in that case, but I've seen that you've copied various older
code from merge-recursive.c to merge-ort.c (which makes sense) in the
past, but I didn't check the origin in that case. Sorry :)

> @@ -3106,6 +3105,7 @@ static int detect_and_process_renames(struct merge_options *opt,
>  {
>  	struct diff_queue_struct combined = { 0 };
>  	struct rename_info *renames = &opt->priv->renames;
> +	struct strmap collisions[3];
>  	int need_dir_renames, s, i, clean = 1;
>  	unsigned detection_run = 0;
>  
> @@ -3155,12 +3155,22 @@ static int detect_and_process_renames(struct merge_options *opt,
>  	ALLOC_GROW(combined.queue,
>  		   renames->pairs[1].nr + renames->pairs[2].nr,
>  		   combined.alloc);
> +	for (int i = MERGE_SIDE1; i <= MERGE_SIDE2; i++) {

The "int i" here will need to be pre-declared earlier, per: 6563706568b
(CodingGuidelines: give deadline for "for (int i = 0; ...", 2022-03-30)

I also don't mind us just saying "we've waited enough". Junio?

> +		int other_side = 3 - i;
> +		compute_collisions(&collisions[i],
> +				   &renames->dir_renames[other_side],
> +				   &renames->pairs[i]);
> +	}
>  	clean &= collect_renames(opt, &combined, MERGE_SIDE1,
> +				 collisions,
>  				 &renames->dir_renames[2],
>  				 &renames->dir_renames[1]);
>  	clean &= collect_renames(opt, &combined, MERGE_SIDE2,
> +				 collisions,
>  				 &renames->dir_renames[1],
>  				 &renames->dir_renames[2]);
> +	for (int i = MERGE_SIDE1; i <= MERGE_SIDE2; i++)
> +		free_collisions(&collisions[i]);
>  	STABLE_QSORT(combined.queue, combined.nr, compare_pairs);
>  	trace2_region_leave("merge", "directory renames", opt->repo);