On Thu, Oct 08, 2020 at 03:04:39PM +0000, Derrick Stolee via GitGitGadget wrote:
> Since the root cause for producing commit-graph layers with these
> duplicate commits is currently unknown, it is difficult to create a test
> for this scenario. For now, we must rely on testing the example data
> graciously provided in [1]. My local test successfully merged layers,
> and 'git commit-graph verify' passed.
Yeah, that is unfortunate. We could synthetically create such a graph
file, but I'm not sure if it's worth the trouble.
> @@ -2023,17 +2023,27 @@ static void sort_and_scan_merged_commits(struct write_commit_graph_context *ctx)
>
> if (i && oideq(&ctx->commits.list[i - 1]->object.oid,
> &ctx->commits.list[i]->object.oid)) {
> - die(_("unexpected duplicate commit id %s"),
> - oid_to_hex(&ctx->commits.list[i]->object.oid));
> + /*
> + * Silently ignore duplicates. These were likely
> + * created due to a commit appearing in multiple
> + * layers of the chain, which is unexpected but
> + * not invalid. We should make sure there is a
> + * unique copy in the new layer.
> + */
You mentioned earlier checking tha the metadata for the duplicates was
identical. How hard would that be to do here?
> } else {
> unsigned int num_parents;
>
> + ctx->commits.list[dedup_i] = ctx->commits.list[i];
> + dedup_i++;
> +
This in-place de-duping is much nicer than what was in v1. There's still
a slight cost to the common case when we have no duplicates, but it's
minor (just an extra noop self-assignment of each index).
-Peff
