Re: [PATCH v2 08/20] merge-ort: compute a few more useful fields for collect_merge_info

From: Jonathan Tan <jonathantanmy@google.com>
Date: Nov 6, 2020, 22:52 UTC
Message-ID: <20201106225211.772384-1-jonathantanmy@google.com>
In-Reply-To: <20201102204344.342633-9-newren@gmail.com>

Show 7 quoted lines

> +	/*
> +	 * Note: We only label files with df_conflict, not directories.
> +	 * Since directories stay where they are, and files move out of the
> +	 * way to make room for a directory, we don't care if there was a
> +	 * directory/file conflict for a parent directory of the current path.
> +	 */
> +	unsigned df_conflict = (filemask != 0) && (dirmask != 0);

Suppose you have:

 [ours]
  foo/
    bar/
      baz
    quux
 [theirs]
  foo

By "we only label files with df_conflict, not directories", are you referring to not labelling "foo/" in [ours], or to "bar/", "baz", and "quux" (so, the files and directories within a directory)? At first I thought you were referring to the former, but perhaps you are referring to the latter.

Show 11 quoted lines

> @@ -161,6 +179,13 @@ static int collect_merge_info_callback(int n,
>  		newinfo.name = p->path;
>  		newinfo.namelen = p->pathlen;
>  		newinfo.pathlen = st_add3(newinfo.pathlen, p->pathlen, 1);
> +		/*
> +		 * If we did care about parent directories having a D/F
> +		 * conflict, then we'd include
> +		 *    newinfo.df_conflicts |= (mask & ~dirmask);
> +		 * here.  But we don't.  (See comment near setting of local
> +		 * df_conflict variable near the beginning of this function).
> +		 */

I'm not sure how "mask" and "dirmask" contains information about parent directories. "mask" represents the available entries, and "dirmask" represents which of them are directories, as far as I know. So we can notice when something is missing, but I don't see how this distinguishes between the case that something is missing because it was in a parent directory that got deleted, vs something is missing because it itself got deleted.