From patchwork Sat Jun 26 17:05:19 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12346557 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F06C7C49EA5 for ; Sat, 26 Jun 2021 17:05:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BEE5961C2F for ; Sat, 26 Jun 2021 17:05:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230104AbhFZRHs (ORCPT ); Sat, 26 Jun 2021 13:07:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38830 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229916AbhFZRHr (ORCPT ); Sat, 26 Jun 2021 13:07:47 -0400 Received: from mail-wr1-x436.google.com (mail-wr1-x436.google.com [IPv6:2a00:1450:4864:20::436]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 357F1C061766 for ; Sat, 26 Jun 2021 10:05:25 -0700 (PDT) Received: by mail-wr1-x436.google.com with SMTP id a13so14377839wrf.10 for ; Sat, 26 Jun 2021 10:05:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=x2RmhhlrifJJogdSHWJkjQBMbdntxcLjvbsGph67WvQ=; b=JKT6QZklQHmFgIGSjwbwEYglS9VQ5nAiJuygSHBQBVn5i10KptwaKEcathpH+HXNJ5 NEfInQq41y25yQreB7/3tBGHEPhKxYGwfVk/vx2ryQrKx+EOL0QoQnt+WHTZDWoHiyVa bTsVh3eDWtMsdOKseaHzgm4nEJ4OjtVmMNwjfElAQ8edEGLfvPqOG6lPBlpcfbrTdSbh f/hhVsGW7b+l3fIInZfrLfKdcADe4Ga/T58f2yt9RUlalPXj2F78/rOMHUNbl1hvTQjA yS9LZkotuhq2UAK6awqzubuqBKkoHbXiACkzCmCAFYfIQ/1gAiHrjCGz0hhe5hsAjujf f4uw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=x2RmhhlrifJJogdSHWJkjQBMbdntxcLjvbsGph67WvQ=; b=bs/r4OtN8/fNWppozJyJrerqJ/0OfwGTFXGz9qe2RnrqMGlQWacA7WrO97Hq52mA/1 DauDZ0z9i/iP1P3ByF0GJgz4haowDO3fRMvXPiqWJoCUN6smLyGxaRZczC4LWKvIJ42X 0VvP37ejuQ7ZTtXKhpAYpWrRcumrPige40xTI4sL6JVHDYH57BcTMHKXk43AEuUNVQPW NHyC9mFdFS+Zm5jrySv658+RaTPilm1D6XuImpN5s6Knu7IiSGoBmFzH+e5Vzv+V4irU YnoTLP6PW0a5vljYJsd+6MVN6udTVE9o/LFU68hKTchIPKP825xO5fxoXOXWkKhDBlck PD3A== X-Gm-Message-State: AOAM531fSglUB7HQPaJCcTIN/N5MYjCdiqxZuSwyaFLAhm1+gkbgNqrC rNClOckZ1tgRVU/G33VwkqqP/Fu4ltI= X-Google-Smtp-Source: ABdhPJzJmMXdaNgYYJsmz25RU4QdSAHgQY1YBGFx1w/meS6uuK69vQm7fqmTVAoz1gzDHwkRFSYuvQ== X-Received: by 2002:a5d:65cc:: with SMTP id e12mr17726441wrw.354.1624727123783; Sat, 26 Jun 2021 10:05:23 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id g15sm8984138wri.75.2021.06.26.10.05.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 26 Jun 2021 10:05:23 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Sat, 26 Jun 2021 17:05:19 +0000 Subject: [PATCH 1/3] t6423: test directory renames causing rename-to-self Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Elijah Newren , Elijah Newren Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Elijah Newren From: Elijah Newren Directory rename detection can cause transitive renames, e.g. if the two different sides of history each do one half of: A/file -> B/file B/ -> C/ then directory rename detection transitively renames to give us C/file. Since the default for merge.directoryRenames is conflict, this results in an error message saying it is unclear whether the file should be placed at B/file or C/file. What if C/ is A/, though? In such a case, the transitive rename would give us A/file, the original name we started with. Logically, having an error message with B/file vs. A/file should be fine, as should leaving the file where it started. But the logic in both merge-recursive and merge-ort did not handle a case of a filename being renamed to itself correctly; merge-recursive had two bugs, and merge-ort had one. Add some testcases covering such a scenario. Based-on-testcase-by: Anders Kaseorg Signed-off-by: Elijah Newren --- t/t6423-merge-rename-directories.sh | 117 ++++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index be84d22419d..2a2ab907338 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -5024,6 +5024,123 @@ test_expect_failure '12h: renaming a file within a renamed directory' ' ) ' +# Testcase 12i, Directory rename causes rename-to-self +# Commit O: source/{subdir/foo, bar, baz_1} +# Commit A: source/{foo, bar, baz_1} +# Commit B: source/{subdir/{foo, bar}, baz_2} +# Expected: source/{foo, bar, baz_2}, with conflicts on +# source/bar vs. source/subdir/bar + +test_setup_12i () { + test_create_repo 12i && + ( + cd 12i && + + mkdir -p source/subdir && + echo foo >source/subdir/foo && + echo bar >source/bar && + echo baz >source/baz && + git add source && + git commit -m orig && + + git branch O && + git branch A && + git branch B && + + git switch A && + git mv source/subdir/foo source/foo && + git commit -m A && + + git switch B && + git mv source/bar source/subdir/bar && + echo more baz >>source/baz && + git commit -m B + ) +} + +test_expect_merge_algorithm failure failure '12i: Directory rename causes rename-to-self' ' + test_setup_12i && + ( + cd 12i && + + git checkout A^0 && + + test_must_fail git -c merge.directoryRenames=conflict merge -s recursive B^0 && + + test_path_is_missing source/subdir && + test_path_is_file source/bar && + test_path_is_file source/baz && + + git ls-files | uniq >tracked && + test_line_count = 3 tracked && + + git status --porcelain -uno >actual && + cat >expect <<-\EOF && + UU source/bar + M source/baz + EOF + test_cmp expect actual + ) +' + +# Testcase 12j, Directory rename to root causes rename-to-self +# Commit O: {subdir/foo, bar, baz_1} +# Commit A: {foo, bar, baz_1} +# Commit B: {subdir/{foo, bar}, baz_2} +# Expected: {foo, bar, baz_2}, with conflicts on bar vs. subdir/bar + +test_setup_12j () { + test_create_repo 12j && + ( + cd 12j && + + mkdir -p subdir && + echo foo >subdir/foo && + echo bar >bar && + echo baz >baz && + git add . && + git commit -m orig && + + git branch O && + git branch A && + git branch B && + + git switch A && + git mv subdir/foo foo && + git commit -m A && + + git switch B && + git mv bar subdir/bar && + echo more baz >>baz && + git commit -m B + ) +} + +test_expect_merge_algorithm failure failure '12j: Directory rename to root causes rename-to-self' ' + test_setup_12j && + ( + cd 12j && + + git checkout A^0 && + + test_must_fail git -c merge.directoryRenames=conflict merge -s recursive B^0 && + + test_path_is_missing subdir && + test_path_is_file bar && + test_path_is_file baz && + + git ls-files | uniq >tracked && + test_line_count = 3 tracked && + + git status --porcelain -uno >actual && + cat >expect <<-\EOF && + UU bar + M baz + EOF + test_cmp expect actual + ) +' + ########################################################################### # SECTION 13: Checking informational and conflict messages # From patchwork Sat Jun 26 17:05:20 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12346561 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B94D9C49EA5 for ; Sat, 26 Jun 2021 17:05:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9879A61C19 for ; Sat, 26 Jun 2021 17:05:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230180AbhFZRHv (ORCPT ); Sat, 26 Jun 2021 13:07:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38832 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230107AbhFZRHs (ORCPT ); Sat, 26 Jun 2021 13:07:48 -0400 Received: from mail-wr1-x430.google.com (mail-wr1-x430.google.com [IPv6:2a00:1450:4864:20::430]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CAF6CC061766 for ; Sat, 26 Jun 2021 10:05:25 -0700 (PDT) Received: by mail-wr1-x430.google.com with SMTP id l8so5636480wry.13 for ; Sat, 26 Jun 2021 10:05:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=AQ1OdsAPr2gaze4eE18sUokQoCP+whnjXHIa60XXz/U=; b=Idhx+zdq5IE3D30DsGX1TOBv6vcMZe8qJc48pVDCt8XfCdUWorYh3L7UQk9sz4rT4r hP24VrlX1t5lEXW4KmIsP1LoGTBbkY+RVTg+8p7Hjj2GQA/e5XLbMT4kcU690Bp6x45C tFH5KndWkxZNDnRVBt2chm9zhNZlsYNDTwm/fbO7o1hzMrfknYeIiHriMXoik8BQH7Er PUYpXcO1vGUupXwDl6VGMNw2i6fw0nDnJ3NNcFvoABi9XN+FnrhEEa+Kvj4N6rHmDpe8 M/Q8FVhfB4fO8DnhsRm/qfx1sptUg41eHuS58tTTTWWatu64ps+Oe8nsVcy6mqs1xDZX cnYg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=AQ1OdsAPr2gaze4eE18sUokQoCP+whnjXHIa60XXz/U=; b=OiGGxbmjcDUV8snL2PKJmXfStGVV2JF2xk+6Kh/+8IR+C8aDuYdBgjDF+FOaBtnip4 EzT3HvxaxNLEla2P8R4BNd2SK5UeH46tGTeMITiygVURWUKIOPwAhNez6Bg8+QFehWFM ttMf+Ro6zzOvEXq8bmoNLTmZTZz2jrMzK5yGQ8jXMlWKkL6bKgw1S6r6p7C3D5bHXsf7 RCOBk7J+VdyBc6f23Z1GxGyvcy6rZQwDe3xKyZP+sNAi5EP6O8fjzZTu4d+Qe1aHGeuZ 7DqvE+1BoWafS5bu1moK4gn2yF8G2ljkmA25ELrrluAgTHPO5Tf8t+Iqt/6AV5RC/870 rW+Q== X-Gm-Message-State: AOAM533OlmVTRw+bWazILdhv6qMfEmGWrh5WaL0fY0N/iHlvgwephKzH tFGwSHPsB+we5+386Rfcy2Y3Dxmjglg= X-Google-Smtp-Source: ABdhPJxqnAOWoglcNoRBmpX1fs4Ylv3U9ixNg459poxDsRU22KTEQy4QInqR2arqXYQMU8RbYQra0A== X-Received: by 2002:a5d:6d8b:: with SMTP id l11mr17954366wrs.21.1624727124465; Sat, 26 Jun 2021 10:05:24 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id h15sm9121057wrq.88.2021.06.26.10.05.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 26 Jun 2021 10:05:24 -0700 (PDT) Message-Id: <052f40c3c1a6b35f9253faf698f8cbc87f81675e.1624727121.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Sat, 26 Jun 2021 17:05:20 +0000 Subject: [PATCH 2/3] merge-ort: ensure we consult df_conflict and path_conflicts Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Elijah Newren , Elijah Newren Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Elijah Newren From: Elijah Newren Path conflicts (typically rename path conflicts, e.g. rename/rename(1to2) or rename/add/delete), and directory/file conflicts should obviously result in files not being marked as clean in the merge. We had a codepath where we missed consulting the path_conflict and df_conflict flags, based on match_mask. Granted, it requires an unusual setup to trigger this codepath (directory rename causing rename-to-self is the only case I can think of), but we still need to handle it. To make it clear that we have audited the other codepaths that do not explicitly mention these flags, add some assertions that the flags are not set. Reported-by: Anders Kaseorg Signed-off-by: Elijah Newren --- merge-ort.c | 6 +++++- t/t6423-merge-rename-directories.sh | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/merge-ort.c b/merge-ort.c index b954f7184a5..373dbac5079 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -3237,7 +3237,7 @@ static void process_entry(struct merge_options *opt, * above. */ if (ci->match_mask) { - ci->merged.clean = 1; + ci->merged.clean = !ci->df_conflict && !ci->path_conflict; if (ci->match_mask == 6) { /* stages[1] == stages[2] */ ci->merged.result.mode = ci->stages[1].mode; @@ -3249,6 +3249,8 @@ static void process_entry(struct merge_options *opt, ci->merged.result.mode = ci->stages[side].mode; ci->merged.is_null = !ci->merged.result.mode; + if (ci->merged.is_null) + ci->merged.clean = 1; oidcpy(&ci->merged.result.oid, &ci->stages[side].oid); assert(othermask == 2 || othermask == 4); @@ -3421,6 +3423,7 @@ static void process_entry(struct merge_options *opt, path)) { ci->merged.is_null = 1; ci->merged.clean = 1; + assert(!ci->df_conflict && !ci->path_conflict); } else if (ci->path_conflict && oideq(&ci->stages[0].oid, &ci->stages[side].oid)) { /* @@ -3447,6 +3450,7 @@ static void process_entry(struct merge_options *opt, ci->merged.is_null = 1; ci->merged.result.mode = 0; oidcpy(&ci->merged.result.oid, null_oid()); + assert(!ci->df_conflict); ci->merged.clean = !ci->path_conflict; } diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index 2a2ab907338..7480daab46a 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -5058,7 +5058,7 @@ test_setup_12i () { ) } -test_expect_merge_algorithm failure failure '12i: Directory rename causes rename-to-self' ' +test_expect_merge_algorithm failure success '12i: Directory rename causes rename-to-self' ' test_setup_12i && ( cd 12i && @@ -5116,7 +5116,7 @@ test_setup_12j () { ) } -test_expect_merge_algorithm failure failure '12j: Directory rename to root causes rename-to-self' ' +test_expect_merge_algorithm failure success '12j: Directory rename to root causes rename-to-self' ' test_setup_12j && ( cd 12j && From patchwork Sat Jun 26 17:05:21 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12346563 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CBBEFC49EA6 for ; Sat, 26 Jun 2021 17:05:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A759E61C2F for ; Sat, 26 Jun 2021 17:05:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230192AbhFZRHv (ORCPT ); Sat, 26 Jun 2021 13:07:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38836 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230151AbhFZRHt (ORCPT ); Sat, 26 Jun 2021 13:07:49 -0400 Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 59764C061574 for ; Sat, 26 Jun 2021 10:05:26 -0700 (PDT) Received: by mail-wr1-x434.google.com with SMTP id m18so14389734wrv.2 for ; Sat, 26 Jun 2021 10:05:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=oiM9fffUL0nbCGmSppFNqI0tAxYDhzVU8K5Oy6k25AE=; b=MhNYqF5jxxo5O/0uVuN10mPRavx2gVNpXVSsZArxbItedQokJZMXHsZ4kSyf4A6eEx VdPXeupGtVfVKvSO03BisLjHi5s2msXKCZSxr2jGNIQTmD2A7fAZZ/b7l8IEi7K7VTKl 0Gh5q90bd/XQLUwyNSX0sUcyqCOohwPWB5PWY3VxNHPrksrHvvVLsyX/T/P68vo9RO/b 6yakv2JSrodQUzKeDRo5YST1NUl8zJQsqMWoE9O3XkYXCEDH6VWL1g1GZ3xBafiAc4WE /6B/bD0pZJOAWTXTWmzpekQ/C+0b2RQcONzk3Ii4A+WrN5rnGorpONPY57KvIgM4mUej uEDg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=oiM9fffUL0nbCGmSppFNqI0tAxYDhzVU8K5Oy6k25AE=; b=IG9pFbIVloGL2GtfT4HUJugL7H3eL9l3nNt4bXaj+AjbWwroLua5olOALNHQjamNZ6 CeEOJusaElrCg8hZN3gLRrpjbJI1w2C9w7D1QOEZdosgICVWQCl6p/qKiwrU7b+QRqTK hdpqghEjkM+VyJ3hLZZgefg16FJAGykMTtq5QlpkmppvtFJgflvaUNiyv8SmVfiIUNVQ dr/gRMsglRS1UY9peAGOnPnX2sgYTCKyVnMaAIcoM7KVm6aMcvceb9b/SNSlj7ILrT55 dbykktarX7ciwREBvTkUnAwyNRZINtQkfn6K0kB1R2WlGuxGRqC2ghLhA3dG7ClM/PMs 9pbg== X-Gm-Message-State: AOAM532lbHEc8UcWnJPZh8nwJBU6ejlti9ab2EyvgmJw61M66nw+bKS7 8U7pJIdkIC+IAWgfm+0/F3BpGCH5v5w= X-Google-Smtp-Source: ABdhPJz6HKYF6ElYWa+SvNWUm7ypB02p9/CPkwOs2wBg92osL1r/F5rGUp1jgGYz52lOnQ1hLbExAg== X-Received: by 2002:adf:e2cf:: with SMTP id d15mr17754582wrj.48.1624727124979; Sat, 26 Jun 2021 10:05:24 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id z6sm8740551wrh.65.2021.06.26.10.05.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 26 Jun 2021 10:05:24 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Sat, 26 Jun 2021 17:05:21 +0000 Subject: [PATCH 3/3] merge-recursive: handle rename-to-self case Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Elijah Newren , Elijah Newren Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Elijah Newren From: Elijah Newren Directory rename detection can cause transitive renames, e.g. if the two different sides of history each do one half of: A/file -> B/file B/ -> C/ then directory rename detection transitively renames to give us A/file -> C/file However, when C/ == A/, note that this gives us A/file -> A/file. merge-recursive assumed that any rename D -> E would have D != E. While that is almost always true, the above is a special case where it is not. So we cannot do things like delete the rename source, we cannot assume that a file existing at path E implies a rename/add conflict and we have to be careful about what stages end up in the output. This change feels a bit hackish. It took me surprisingly many hours to find, and given merge-recursive's design causing it to attempt to enumerate all combinations of edge and corner cases with special code for each combination, I'm worried there are other similar fixes needed elsewhere if we can just come up with the right special testcase. Perhaps an audit would rule it out, but I have not the energy. merge-recursive deserves to die, and since it is on its way out anyway, fixing this particular bug narrowly will have to be good enough. Reported-by: Anders Kaseorg Signed-off-by: Elijah Newren --- merge-recursive.c | 19 +++++++++++++------ t/t6423-merge-rename-directories.sh | 4 ++-- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index d146bb116f7..c895145a8f5 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -2804,12 +2804,19 @@ static int process_renames(struct merge_options *opt, int renamed_stage = a_renames == renames1 ? 2 : 3; int other_stage = a_renames == renames1 ? 3 : 2; + /* + * Directory renames have a funny corner case... + */ + int renamed_to_self = !strcmp(ren1_src, ren1_dst); + /* BUG: We should only remove ren1_src in the base * stage and in other_stage (think of rename + * add-source case). */ - remove_file(opt, 1, ren1_src, - renamed_stage == 2 || !was_tracked(opt, ren1_src)); + if (!renamed_to_self) + remove_file(opt, 1, ren1_src, + renamed_stage == 2 || + !was_tracked(opt, ren1_src)); oidcpy(&src_other.oid, &ren1->src_entry->stages[other_stage].oid); @@ -2823,6 +2830,9 @@ static int process_renames(struct merge_options *opt, ren1->dir_rename_original_type == 'A') { setup_rename_conflict_info(RENAME_VIA_DIR, opt, ren1, NULL); + } else if (renamed_to_self) { + setup_rename_conflict_info(RENAME_NORMAL, + opt, ren1, NULL); } else if (oideq(&src_other.oid, null_oid())) { setup_rename_conflict_info(RENAME_DELETE, opt, ren1, NULL); @@ -3180,7 +3190,6 @@ static int handle_rename_normal(struct merge_options *opt, struct rename *ren = ci->ren1; struct merge_file_info mfi; int clean; - int side = (ren->branch == opt->branch1 ? 2 : 3); /* Merge the content and write it out */ clean = handle_content_merge(&mfi, opt, path, was_dirty(opt, path), @@ -3190,9 +3199,7 @@ static int handle_rename_normal(struct merge_options *opt, opt->detect_directory_renames == MERGE_DIRECTORY_RENAMES_CONFLICT && ren->dir_rename_original_dest) { if (update_stages(opt, path, - NULL, - side == 2 ? &mfi.blob : NULL, - side == 2 ? NULL : &mfi.blob)) + &mfi.blob, &mfi.blob, &mfi.blob)) return -1; clean = 0; /* not clean, but conflicted */ } diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index 7480daab46a..d67b5c28944 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -5058,7 +5058,7 @@ test_setup_12i () { ) } -test_expect_merge_algorithm failure success '12i: Directory rename causes rename-to-self' ' +test_expect_success '12i: Directory rename causes rename-to-self' ' test_setup_12i && ( cd 12i && @@ -5116,7 +5116,7 @@ test_setup_12j () { ) } -test_expect_merge_algorithm failure success '12j: Directory rename to root causes rename-to-self' ' +test_expect_success '12j: Directory rename to root causes rename-to-self' ' test_setup_12j && ( cd 12j &&