Re: [PATCH v2 5/7] merge-ort: defer recursing into directories when merge base is matched

From: Derrick Stolee <stolee@gmail.com>
Date: Jul 15, 2021, 14:43 UTC
Message-ID: <24a7bea5-6325-289d-8d49-619b5e7ec65b@gmail.com>
In-Reply-To: <317553eadb68ce162b5ebea24045a9ca75342e91.1626204784.git.gitgitgadget@gmail.com>

On 7/13/2021 3:33 PM, Elijah Newren via GitGitGadget wrote:

> From: Elijah Newren <newren@gmail.com>

...

Show 15 quoted lines

> +		/*
> +		 * Check for whether we can avoid recursing due to one side
> +		 * matching the merge base.  The side that does NOT match is
> +		 * the one that might have a rename destination we need.
> +		 */
> +		assert(!side1_matches_mbase || !side2_matches_mbase);
> +		side = side1_matches_mbase ? MERGE_SIDE2 :
> +			side2_matches_mbase ? MERGE_SIDE1 : MERGE_BASE;
> +		if (filemask == 0 && (dirmask == 2 || dirmask == 4)) {
> +			/*
> +			 * Also defer recursing into new directories; set up a
> +			 * few variables to let us do so.
> +			 */
> +			ci->match_mask = (7 - dirmask);
> +			side = dirmask / 2;

Clever.

> +		}
> +		if (renames->dir_rename_mask != 0x07 &&
> +		    (side != MERGE_BASE) &&

nit: these parens are not necessary?

> +		    renames->trivial_merges_okay[side] &&
> +		    !strset_contains(&renames->target_dirs[side], pi.string)) {

Does this last condition mean "this directory is not already a parent of a chosen rename target"?

Show 8 quoted lines

> +			strintmap_set(&renames->possible_trivial_merges[side],
> +				      pi.string, renames->dir_rename_mask);
> +			renames->dir_rename_mask = prev_dir_rename_mask;
> +			return mask;
> +		}
> +
> +		/* We need to recurse */
>  		ci->match_mask &= filemask;

Thanks, -Stolee