From patchwork Wed Jun 19 03:00:13 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 13703359 Received: from mail-wr1-f49.google.com (mail-wr1-f49.google.com [209.85.221.49]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CF0612594 for ; Wed, 19 Jun 2024 03:00:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.49 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718766026; cv=none; b=MDoJYpMGJx7AaNeuQDgLGr2NEuPVXeC/mMPl7z8uXjgr4dCJuJqW5wW6nHVY4ybJmHvy9KD2kwGICTZ40HalYDswULxo9fQzHvQztGG0DM3aaCsHDflLj6Z2XC1U8meXvEuKyuqR6+OGanralNyk7SxlZBKeA0fVoS2aqsyna9I= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718766026; c=relaxed/simple; bh=URc+Xn1Nm1ZI4I4VEvem3okKDvwA1LUDYLqkZk+n/Nw=; h=Message-Id:In-Reply-To:References:From:Date:Subject:Content-Type: MIME-Version:To:Cc; b=qVWZgVRRyUEhhGqpP3xEcUba/KPEH/7D9ouJJf2WOZrJMfmyktwj9XUNgykxOBPAyxNHSBH/S2z0ElXsPafs9673JALFcBN+cPXzcOC2Zo0ynRSrU9PtIhpClzsiGW98whsa1RXKRq5ZRYC0dUtCnniRElO+tiIeCxdDEHtEdm4= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=I5CHYUNs; arc=none smtp.client-ip=209.85.221.49 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="I5CHYUNs" Received: by mail-wr1-f49.google.com with SMTP id ffacd0b85a97d-35f14af40c2so5135043f8f.0 for ; Tue, 18 Jun 2024 20:00:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1718766023; x=1719370823; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=KVdZAMq+IYS2SrXDc0mge0VdZqsAN0YncneL5cyJhk8=; b=I5CHYUNseBg8hnDnZ64b0kDONZirYsYGeF/RwcJ71cGxQav6rs2FSiv+Wv3Vc5Pukz 5tx71UlBGqJvdyAUZAN1ELhOHZmobgbgiDxow0lr74A5who0JV+jbQmbk6TiNpe/d1Qc iXIcd1WM8ZnydVe5Bpy+0Fz3GVRla6naz2Jq9L2gCLZsf/TbwdzED+E94NF7HBl+YYm+ 9mvB5gx48u+lU62CRF1Idu3453by8S93l7qmKw/4AY/9U73+5V6kwhab19NM5SmCPpSd zjJCHHf2IpaDU+9/KhiTSwj85dGiUTQvy2Za2Pm7Oha9YYHdLjBz3EeqTkF50uOumCys mfIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718766023; x=1719370823; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=KVdZAMq+IYS2SrXDc0mge0VdZqsAN0YncneL5cyJhk8=; b=bVrEBPx9i55LD838wWbYoWEsUESPvBUQhbT/I43EfkHZjG7AGUa05neTo07CeAfx0G ZUSoJOIq9+v9L9W+hTFtarg5XS6Mn60n5XjHMvMko+uXdvAvwNzYCt/wZFmZg2HXuSFm 9ndTyvvAmJJf2Mo/OGdfsQMWCok9/AA9hAmaoH2uEKov5fd4AY3oXo67ffoOG6JgB3Uj uruuLVp5kAQFHDl3LEHI5dPEEx5rnLzsUgBog/JzhDPsLge1+gXk+kl3fDe4lx8dzvvM fWLQ7VQtuxJE6FmjXVKTy9U3XsGJIV/EhyvQbY2GanmeR2/jHf789aWUPXsv45KtO4XE YQRw== X-Gm-Message-State: AOJu0Yxd9Q0mzftQ6uxpWtpInqx+1oMOBi9ZmIVPoH/oaKBnEnVyMtO3 tvBURunJPZgPevGYTmbUG8hXGk0aEFI1jBQOoc8uPX6j8tAYb1wSeiYqTQ== X-Google-Smtp-Source: AGHT+IFGJPJnjTdtZ2LRnWDgHuhQUQ/MiWUMCXWnA/7YExRYwWqueNqhu24B4LE2WJRNtWA1hsy/uQ== X-Received: by 2002:adf:f791:0:b0:362:3836:35e7 with SMTP id ffacd0b85a97d-363192cf052mr885475f8f.48.1718766022467; Tue, 18 Jun 2024 20:00:22 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-36075093d41sm15855429f8f.16.2024.06.18.20.00.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Jun 2024 20:00:22 -0700 (PDT) Message-Id: <5c50c0b3aadbefc7973a2e8d77890808c95875d6.1718766019.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Wed, 19 Jun 2024 03:00:13 +0000 Subject: [PATCH v2 1/7] merge-ort: extract handling of priv member into reusable function Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Taylor Blau , Eric Sunshine , Elijah Newren , Elijah Newren From: Elijah Newren From: Elijah Newren In preparation for a subsequent commit which will ensure we do not forget to maintain our invariants for the priv member in error codepaths, extract the necessary functionality out into a separate function. This change is cosmetic at this point, and introduces no changes beyond an extra assertion sanity check. Signed-off-by: Elijah Newren --- merge-ort.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/merge-ort.c b/merge-ort.c index eaede6cead9..700ddfccb90 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -5000,6 +5000,26 @@ static void merge_check_renames_reusable(struct merge_result *result, /*** Function Grouping: merge_incore_*() and their internal variants ***/ +static void move_opt_priv_to_result_priv(struct merge_options *opt, + struct merge_result *result) +{ + /* + * opt->priv and result->priv are a bit weird. opt->priv contains + * information that we can re-use in subsequent merge operations to + * enable our cached renames optimization. The best way to provide + * that to subsequent merges is putting it in result->priv. + * However, putting it directly there would mean retrofitting lots + * of functions in this file to also take a merge_result pointer, + * which is ugly and annoying. So, we just make sure at the end of + * the merge (the outer merge if there are internal recursive ones) + * to move it. + */ + assert(opt->priv && !result->priv); + result->priv = opt->priv; + result->_properly_initialized = RESULT_INITIALIZED; + opt->priv = NULL; +} + /* * Originally from merge_trees_internal(); heavily adapted, though. */ @@ -5060,11 +5080,8 @@ static void merge_ort_nonrecursive_internal(struct merge_options *opt, /* existence of conflicted entries implies unclean */ result->clean &= strmap_empty(&opt->priv->conflicted); } - if (!opt->priv->call_depth) { - result->priv = opt->priv; - result->_properly_initialized = RESULT_INITIALIZED; - opt->priv = NULL; - } + if (!opt->priv->call_depth) + move_opt_priv_to_result_priv(opt, result); } /* From patchwork Wed Jun 19 03:00:14 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 13703361 Received: from mail-wr1-f47.google.com (mail-wr1-f47.google.com [209.85.221.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0457D14265 for ; Wed, 19 Jun 2024 03:00:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.47 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718766027; cv=none; b=Deokr+34yv+N2q2DjHTGBkm34OYgTWUNfXXzOUFngl2qRECAeKdSxUVgZP/sdTml70V85175Bt81vCV3yFJ21ep2MoZky94f/6jvu6Emfd2L6U9CprADv7RJpyl3ABIjn2QSawBBNY2AVFfYCG71idqCzrkjD2vxLS3/jbppo3o= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718766027; c=relaxed/simple; bh=YjclWHVUF13LYDFXYKOF8UjnWqMnijPbrOPwIrPqqis=; h=Message-Id:In-Reply-To:References:From:Date:Subject:Content-Type: MIME-Version:To:Cc; b=f5cCxVQUZUlEYs9mSQMXCZnChEXjxMcNSdb3CM08dSoPw3Fh65UCddjFNTx1mjEXRe20p2XcLFodZ2CIZE8B2pocwc8HSo2nnMNkpe9jjefrH/DRsnB+/nkZxFRVOKZz03LVjNARuZ0qbHtmiAYyZr/fBzeTuVu0+JOzpY5bw2Y= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=fLlYBxVk; arc=none smtp.client-ip=209.85.221.47 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="fLlYBxVk" Received: by mail-wr1-f47.google.com with SMTP id ffacd0b85a97d-363826fbcdeso134173f8f.0 for ; Tue, 18 Jun 2024 20:00:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1718766024; x=1719370824; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=+wAiMfckS8jQMvlxxLrZhJdJEE9l28mGLPDlEX35XnQ=; b=fLlYBxVk26RoA1DUAX0oWFOOzShEGWbOU/n5q4HOE9IMbqeUkbEZRvI08gcm+EFYsj /pW9DvNw15Yj9eb6UNrenL/EXKu06SCv8rKONftV6kMXBVGMz430fKKkPfuEpCzjzQB3 HvGCH0F/+AR7XLrdxu9EI0BF54c3Zz2+YfCR0kBkl6RwB+MFjpJisRzG4Z0+eZ9Q5gOn pf8ZAPkvf+vB5vb2zBInbxshYtz0K4X8c19LbJMUxzzvOdg+HOLH7CODDaS5IRtSn5GA qTx1Nym9W6Wl7ScyTZsaEhZJvmAnvKChA7OOl4EPFOQ+loU2l9iftFElG5MfzTSn+9B3 YqDg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718766024; x=1719370824; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=+wAiMfckS8jQMvlxxLrZhJdJEE9l28mGLPDlEX35XnQ=; b=cQNkJ8KNkcRqrRPDRw6/qWHdfIezoDOCueEtw2P3ATwAPht8hJT9yGUcGZwWQwK9yY RN+laG9c8xacMX8bdAUtvmXSYqqGvlJYl/GGhKenvvsYWAjT5LYRiJerSNOXLHgPGD9u kFEVEOhfnPbuxyfbwyMLPh8ab51O8TSvVAe4O1XBGtOSS7jiGv4sWcH1IrRxEKzZwLLz SY/zx41JRPKLamU2En7xVtp51KU4Ag5w0JaGhZGAvW74ylcoirXbwyL/JY1MhCWDm9O+ L5vOuUKWuyZ8x0XHbP1BYFXSDXgmwxaYm8KUUfoGp9wRrfhL2HkbdGy/drWZ/+U4FPrI 8VQA== X-Gm-Message-State: AOJu0Yw2UZWA93LQKRmjD50/md6tIhUhpZxKS1Lmr5nAE1ijrBt0WK7P vBFdcKW+UkxQn2EzV8rW36oBIwdoYYTe0lN4GCV87h4ieUupMK/wML+S3g== X-Google-Smtp-Source: AGHT+IFGBNNWsfLkPIcwDCWbjwlO+/ZoGjIeab6LTsFW3DTGpC7ZApI/8NTz0Tz10GJ1QFOsS6speg== X-Received: by 2002:a5d:4b46:0:b0:360:8683:c072 with SMTP id ffacd0b85a97d-363192cde9emr731870f8f.50.1718766023911; Tue, 18 Jun 2024 20:00:23 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-422f6127c1esm209783085e9.23.2024.06.18.20.00.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Jun 2024 20:00:23 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Wed, 19 Jun 2024 03:00:14 +0000 Subject: [PATCH v2 2/7] merge-ort: maintain expected invariant for priv member Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Taylor Blau , Eric Sunshine , Elijah Newren , Elijah Newren From: Elijah Newren From: Elijah Newren The calling convention for the merge machinery is One call to init_merge_options() One or more calls to merge_incore_[non]recursive() One call to merge_finalize() (possibly indirectly via merge_switch_to_result()) Both merge_switch_to_result() and merge_finalize() expect opt->priv == NULL && result->priv != NULL which is supposed to be set up by our move_opt_priv_to_result_priv() function. However, two codepaths dealing with error cases did not execute this necessary logic, which could result in assertion failures (or, if assertions were compiled out, could result in segfaults). Fix the oversight and add a test that would have caught one of these problems. While at it, also tighten an existing test for a non-recursive merge to verify that it fails with appropriate status. Most merge tests in the testsuite check either for success or conflicts; those testing for neither are rare and it is good to ensure they support the invariant assumed by builtin/merge.c in this comment: /* * The backend exits with 1 when conflicts are * left to be resolved, with 2 when it does not * handle the given merge at all. */ So, explicitly check for the exit status of 2 in these cases. Reported-by: Matt Cree Signed-off-by: Elijah Newren --- merge-ort.c | 3 ++- t/t6406-merge-attr.sh | 42 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/merge-ort.c b/merge-ort.c index 700ddfccb90..dc62aaf6d11 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -5050,6 +5050,7 @@ static void merge_ort_nonrecursive_internal(struct merge_options *opt, oid_to_hex(&side1->object.oid), oid_to_hex(&side2->object.oid)); result->clean = -1; + move_opt_priv_to_result_priv(opt, result); return; } trace2_region_leave("merge", "collect_merge_info", opt->repo); @@ -5080,7 +5081,7 @@ static void merge_ort_nonrecursive_internal(struct merge_options *opt, /* existence of conflicted entries implies unclean */ result->clean &= strmap_empty(&opt->priv->conflicted); } - if (!opt->priv->call_depth) + if (!opt->priv->call_depth || result->clean < 0) move_opt_priv_to_result_priv(opt, result); } diff --git a/t/t6406-merge-attr.sh b/t/t6406-merge-attr.sh index 156a1efacfe..b6db5c2cc36 100755 --- a/t/t6406-merge-attr.sh +++ b/t/t6406-merge-attr.sh @@ -185,7 +185,7 @@ test_expect_success !WINDOWS 'custom merge driver that is killed with a signal' >./please-abort && echo "* merge=custom" >.gitattributes && - test_must_fail git merge main 2>err && + test_expect_code 2 git merge main 2>err && grep "^error: failed to execute internal merge" err && git ls-files -u >output && git diff --name-only HEAD >>output && @@ -261,4 +261,44 @@ test_expect_success 'binary files with union attribute' ' grep -i "warning.*cannot merge.*HEAD vs. bin-main" output ' +test_expect_success !WINDOWS 'custom merge driver that is killed with a signal on recursive merge' ' + test_when_finished "rm -f output please-abort" && + test_when_finished "git checkout side" && + + git reset --hard anchor && + + git checkout -b base-a main^ && + echo base-a >text && + git commit -m base-a text && + + git checkout -b base-b main^ && + echo base-b >text && + git commit -m base-b text && + + git checkout -b recursive-a base-a && + test_must_fail git merge base-b && + echo recursive-a >text && + git add text && + git commit -m recursive-a && + + git checkout -b recursive-b base-b && + test_must_fail git merge base-a && + echo recursive-b >text && + git add text && + git commit -m recursive-b && + + git config --replace-all \ + merge.custom.driver "./custom-merge %O %A %B 0 %P %S %X %Y" && + git config --replace-all \ + merge.custom.name "custom merge driver for testing" && + + >./please-abort && + echo "* merge=custom" >.gitattributes && + test_expect_code 2 git merge recursive-a 2>err && + grep "^error: failed to execute internal merge" err && + git ls-files -u >output && + git diff --name-only HEAD >>output && + test_must_be_empty output +' + test_done From patchwork Wed Jun 19 03:00:15 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 13703362 Received: from mail-wr1-f46.google.com (mail-wr1-f46.google.com [209.85.221.46]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 751DE53E24 for ; Wed, 19 Jun 2024 03:00:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.46 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718766029; cv=none; b=lSmukI6ic72j/AcJ1bk4rTuBqHlT6LzGSBmHlkc9x0JIbASfJ61YGDpk0Wx6Q/Ru483OJF49JZbquB0iGHYvE7XDIuEEQWFbmSF/F164MXFySEUxW6l0XOrwUXwRc39F64UBbOR7FnyzNjDsJnhScw9wMzDtImHgYPMlE53BZRg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718766029; c=relaxed/simple; bh=hGflWmrr4wNRFZWU59RTxX5tUfS6QxhpFY90w5BxwzA=; h=Message-Id:In-Reply-To:References:From:Date:Subject:Content-Type: MIME-Version:To:Cc; b=cIeWojxsZz8SYDo94WAKjjWpSBBlM4rrfgQY9k78NELqdVQoCAL0q6A72Xn+lHpke/VGMJDY9Vbia0YuiYORUBi2dmTdB7JCq4TfzJT3k7640cKOK925+vf8TOH9ASQSTJfUlFAuRSzyjDzymRlg2r74EyjPT+I3prEB3mBPfRs= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=hMcTCW16; arc=none smtp.client-ip=209.85.221.46 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="hMcTCW16" Received: by mail-wr1-f46.google.com with SMTP id ffacd0b85a97d-35f23f3da44so5524820f8f.0 for ; Tue, 18 Jun 2024 20:00:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1718766025; x=1719370825; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=2B1kBECs0y3/R6p9HHdBMpnqoLUd5fr92TsN7p8Py3A=; b=hMcTCW16NRahU6eGlVpNF0ZD0tfRNgyrvTBej/40I/r3vhs+NGIfjkz3RqEI9/JMrX GbKCiesjJOMSVb4zvvbCiDNqEiZ81CmqtNQ8iBnkq7Q1ah0rlitYZUgwSebDMjNk6Hef y/+A1MaturTg79/etQRk56zBoprTNRxKTOzzsHjibvS7QPLqhAFutIo+s1L6BpCX+ab6 p1kFlYG3lhzfDa1lM+zGysMU2UPfcewv4Y9BWSYey2KT1Fiu3VkTGCHTzqcJXMfwSDip sdd5i1W7jcFbIdYTL8tSqZKy3PEaTqYYsDr9tYo3cp7jfe2xvuXROkJnUWsjJ0/FmOJO VnlQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718766025; x=1719370825; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=2B1kBECs0y3/R6p9HHdBMpnqoLUd5fr92TsN7p8Py3A=; b=F+kMr9lv4RuJCR/b18O8bgr59B8Cu5Wq15Ne4TwbfHPUGWdC9SeuARpiMVzFFa9CKt rZ7k6lDyOLQ5t22HDI4xjmLuSqMj5xmdT5p/Iq8IBDABkK00+uonQHK3WOjmHjablBoF uYdZOJ1oQWPws+yU8dcBYZs0TfOaLOa6bARQlRtMXAWIAShM6B164+PuRbbnkR7T/Ppe ROndQryf8aJ5cRRk/OfWfpcrJKoJk3zAeaamNykib3SEb50pLwPfxoebwtHyJCjgZseC hh7IBOd1FmTcDf+kCue68jeg1m2Aq9D8wkZhIvuSc948bAMonMXf8b9ou6+cHxjWgfAn qxCg== X-Gm-Message-State: AOJu0YynfDEvocLhxR3Y7+Rvw9Dh31R5Iw2skTZhrmSyf8apmteKkzoN 2T+pOI974SyQOe8Wn/pg3ZYKlGV+m0V3iV22RXj1wAz+eFTjmvBR7WMzTA== X-Google-Smtp-Source: AGHT+IGeMiEqt1s/V9iAIo8LjE2U2lQpjO6PsenCoy3V4YGEiu82kwjKm+gz34hrely+rhLsvfKLRA== X-Received: by 2002:a05:6000:235:b0:362:40cd:1bc with SMTP id ffacd0b85a97d-363178982camr859689f8f.24.1718766025134; Tue, 18 Jun 2024 20:00:25 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-36075104b2esm15700914f8f.101.2024.06.18.20.00.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Jun 2024 20:00:24 -0700 (PDT) Message-Id: <034b91db1d2ed78995b52c014de313744972ff40.1718766019.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Wed, 19 Jun 2024 03:00:15 +0000 Subject: [PATCH v2 3/7] merge-ort: fix type of local 'clean' var in handle_content_merge() Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Taylor Blau , Eric Sunshine , Elijah Newren , Elijah Newren From: Elijah Newren From: Elijah Newren handle_content_merge() returns an int. Every caller of handle_content_merge() expects an int. However, we declare a local variable 'clean' that we use for the return value to be unsigned. To make matters worse, we also assign 'clean' the return value of merge_submodule() in one codepath, which is defined to return an int. It seems that the only reason to have 'clean' be unsigned was to allow a cutesy bit manipulation operation to be well-defined. Fix the type of the 'clean' local in handle_content_merge(). Signed-off-by: Elijah Newren --- merge-ort.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/merge-ort.c b/merge-ort.c index dc62aaf6d11..be0f5bc3838 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -2109,7 +2109,7 @@ static int handle_content_merge(struct merge_options *opt, * merges, which happens for example with rename/rename(2to1) and * rename/add conflicts. */ - unsigned clean = 1; + int clean = 1; /* * handle_content_merge() needs both files to be of the same type, i.e. @@ -2184,7 +2184,8 @@ static int handle_content_merge(struct merge_options *opt, free(result_buf.ptr); if (ret) return -1; - clean &= (merge_status == 0); + if (merge_status > 0) + clean = 0; path_msg(opt, INFO_AUTO_MERGING, 1, path, NULL, NULL, NULL, _("Auto-merging %s"), path); } else if (S_ISGITLINK(a->mode)) { From patchwork Wed Jun 19 03:00:16 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 13703363 Received: from mail-wr1-f41.google.com (mail-wr1-f41.google.com [209.85.221.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 910305588E for ; Wed, 19 Jun 2024 03:00:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718766030; cv=none; b=LYCE1L2RP6kr6vLpysKHpak3xeLkfxoBvYDgU5f9fnsOOruNlarQ6b5+wzvRh8t3pQ7ELbb+kLxubMnbVFVDJHRmnezvwjoj6XoVV6vz310q0NdwyGfFkrFHG9+J+Sn+xDZQRX/LT/Tzy9MOwUY3KlGg+jDQ4KaxT13pvgXwNOo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718766030; c=relaxed/simple; bh=p7Gvro1XqzmZgn9Bjfch/A148Q74+QR1RPehkXZCtxw=; h=Message-Id:In-Reply-To:References:From:Date:Subject:Content-Type: MIME-Version:To:Cc; b=LGA/9fF1PtptKl3SUJEW/FDKFKeh4xoVZHoBmv3teorhrI8yWWWoJKNYB1+7nROUeJURMV/ZmmjkqdDofABpIzdYpglsQjfMYIVQ4ovIxbp9zQdkHVFCygDWcbkjNj7L9EwGz0GMmS4W/xwhSvSPYx5nW2knoRw5S7ejg0ow7BY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=bp0IA/iw; arc=none smtp.client-ip=209.85.221.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="bp0IA/iw" Received: by mail-wr1-f41.google.com with SMTP id ffacd0b85a97d-35dc1d8867eso4843744f8f.0 for ; Tue, 18 Jun 2024 20:00:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1718766026; x=1719370826; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=Z5F4rVzGsIYAg6aARuElb2+3B4u4GLFXVeckrXtpT2k=; b=bp0IA/iw+09BSjKqbfIcqxHbo7boxoorEdTaBNQozLHByZXp/z2yblfZykHA8UlrxC My/9cdIcbz7RQXmcnjjNxM1NoKd8YzdnivzTIOA1pCrGyOJw22Rn7iqC+F4ijAmWdfwp MVh+VkVsUZnp+c8gZ06NFSUIKwPQGo9Wqgck+Fd00GTG9VtrQDus9yRyjHPGZDKdmYlX uxPUQY6HNrwvCS6BeyBNUKs56/zCR+pnA3tc3zL/yYK/jLyRhBVxlC7qZa8lX6e5UdWT Ua8q58nrH2xgM1YfeU+dSAGjx8rFnfgB+krUn8pmGLQQDHh1OI733FyRESpiYoQnJvGW +wIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718766026; x=1719370826; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Z5F4rVzGsIYAg6aARuElb2+3B4u4GLFXVeckrXtpT2k=; b=J/RUuKSDJx7Y6nhXkXfwTeorlGuC4XXoJJEPBD0Vi636AMfYn9KPLW2fAwcm+3fbUC gexNse3J+Dea1u7VvIt2/Ap8LUsaEbx2Pv7FPaH/rXlqXNQAWhillzonh+XuXFfNLWQW 5+MBG5SfG+cQageuiSPe+20+MCAeFZRSlDn3dHOxN5LVItMc0zpprF9Fu44RF/fun64B B65kar0aUgx8LWJ9GMgcUqpT6MUfu//a6xnVe6v5408OFzVrW6De2/T0W8Gb4qwbyj0u RvTNdNceDMab9xk8ktyWysAy4Jwo7VCJhuSuKT6mprwxGDL9LuWigIsUzAUHh2+Iff1E sf6g== X-Gm-Message-State: AOJu0YzNe1eu/7LTEYMyFgEWzfpwkn3EWHMwV3YRTDME632r/bKRSOtw 6o4KqUTDpX0lPtCdB8IGZz79ouUBoSwVxban+v59kRdPtihaCfR0Pf4xQA== X-Google-Smtp-Source: AGHT+IGPoKb8jRS39vYewgeKJiHIicI/uHbPwbgRzyQT2t8LjymtMOZgPLJCl0fkNz278/FsTEI6Sg== X-Received: by 2002:a5d:51ce:0:b0:362:f853:603c with SMTP id ffacd0b85a97d-36317c77507mr821350f8f.32.1718766026627; Tue, 18 Jun 2024 20:00:26 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-360750ad216sm15760063f8f.57.2024.06.18.20.00.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Jun 2024 20:00:25 -0700 (PDT) Message-Id: <2813a15b48b70ead7e3fd062d1b49baee665fc9d.1718766019.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Wed, 19 Jun 2024 03:00:16 +0000 Subject: [PATCH v2 4/7] merge-ort: clearer propagation of failure-to-function from merge_submodule Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Taylor Blau , Eric Sunshine , Elijah Newren , Elijah Newren From: Elijah Newren From: Elijah Newren The 'clean' member variable is somewhat of a tri-state (1 = clean, 0 = conflicted, -1 = failure-to-determine), but we often like to think of it as binary (ignoring the possibility of a negative value) and use constructs like '!clean' to reflect this. However, these constructs can make codepaths more difficult to understand, unless we handle the negative case early and return pre-emptively; do that in handle_content_merge() to make the code a bit easier to read. Signed-off-by: Elijah Newren --- merge-ort.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/merge-ort.c b/merge-ort.c index be0f5bc3838..d187c966c6a 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -2193,6 +2193,8 @@ static int handle_content_merge(struct merge_options *opt, clean = merge_submodule(opt, pathnames[0], two_way ? null_oid() : &o->oid, &a->oid, &b->oid, &result->oid); + if (clean < 0) + return -1; if (opt->priv->call_depth && two_way && !clean) { result->mode = o->mode; oidcpy(&result->oid, &o->oid); From patchwork Wed Jun 19 03:00:17 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 13703364 Received: from mail-wr1-f43.google.com (mail-wr1-f43.google.com [209.85.221.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 020E355C29 for ; Wed, 19 Jun 2024 03:00:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.43 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718766031; cv=none; b=S7gH7QMLlGgRUmnVaYuOtBxFJisrN3q/perr9dISwK1MIPU5r8TZ0lVD267A78+IYyFuUaSQbRQS+aYkKoBnon1hXEMlfDzTzPjBut8/1ylVQhXhwoAf/p1xoD3fpkYJOVK2/81QDJqS5OGjRkxkpGX9/nJxkkJ2V52v1RvoyQE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718766031; c=relaxed/simple; bh=UAgyhOQGzBJ/PFokPNFjE6wxsZOfJ8xCvbo17oZqzQI=; h=Message-Id:In-Reply-To:References:From:Date:Subject:Content-Type: MIME-Version:To:Cc; b=DVIlgHAq4G+Gg+Zx+g1kZiP8dpaooJa4c1AxacpMVPeJFv/vNQZWNZufjX6SVBBDxmnFhlhgfz2LM2cWrboBPl/42p0kkM0kFmJqI8jo7UiZ7+77mQiBknJYl6RpinOJgctRzwhmil5rcuUAccauHc5udwdUrqxJ/AiaKtfp1Hk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=QZjxsKZM; arc=none smtp.client-ip=209.85.221.43 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="QZjxsKZM" Received: by mail-wr1-f43.google.com with SMTP id ffacd0b85a97d-354b722fe81so5415643f8f.3 for ; Tue, 18 Jun 2024 20:00:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1718766028; x=1719370828; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=kMgAN7pVzhAnDGXhIE3pMLyn76ebCvOVzTT6wi/4ab4=; b=QZjxsKZMBDwWxb2D+rRUzif3VdOxEK8EWCgv+FD+ZdlKY/TUlXLUD9Y+2gi1RZSGf1 CZQx6wo61WeoYHZuzIJveunN+O22jHUG11FStaZ68biuGQ+49wCzHUwdTeR+N5Kle9+Q hKYrRbHZjEiX+McNArabzGBq5QK/UBAhhTLBtZ6/8vvltrxQzOOC80kQJkXGBthfGDma h3f4qk4zKbKy8mq8XcG+P6rh+vMEVDNQ5SUGPtMsx227P5p6K3xqYO20PFb5FketK6QS YH2eka4eZ/TIAVSPy0lDDEwSXyy6m6izA7ag3C7xy5YB7XzKIhfBHJdRcS4w74620W+z gJuw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718766028; x=1719370828; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=kMgAN7pVzhAnDGXhIE3pMLyn76ebCvOVzTT6wi/4ab4=; b=tZNZxwPNaDgSPc3WZVoA9L1o00gl/fC51FVsD9UhgqePMz2sW1BTfMIpCePtaJdv2u dnLBrKPtjd4GCGZpb/6pAC95NfL7wZdDCbzHg4BkJtrH70F8yNGkQs08q3BrUiY7Q7jy 84WNikonNWaqDShvLcVv6Eet2FwzV9AJTdZG9XtFET147dZ2PMKhaAWkjFh6ZC+LzBpM coRHZlOPzVh6lkZtZnIrQQlHqyZI2nqvlXHdZRGFczDbvNA36M/va0HttDASmWT7/MXL 1na4MwvECnerGsOahQpDOs9YtaTrV9LNz3kDVaK+sm4vxgj06726637GGJeAo0z0w6AD klog== X-Gm-Message-State: AOJu0YzfLjJHRsRqDhxmb3rvL4c4I4zkZEymT85FDeSJOBaM2E+DENXX v1dW1ZqcFnnGtg+DhHJDNFeyCoBIqfDo1fgPhgmDGA2J0JpjPXskQ4pJjg== X-Google-Smtp-Source: AGHT+IGuyKZtUnSbngiWFiobxqORtO/oussdwQopPtifBM9M9HZF7bHS8Pl6AirWjcKTnn9hVrXfkg== X-Received: by 2002:a05:6000:b44:b0:35f:1e30:f69a with SMTP id ffacd0b85a97d-36317d758e1mr982312f8f.42.1718766027799; Tue, 18 Jun 2024 20:00:27 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-362c7c2dffdsm1948917f8f.35.2024.06.18.20.00.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Jun 2024 20:00:27 -0700 (PDT) Message-Id: <750acae4dba009d7a2f2e10b3767fcecf3a4748c.1718766019.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Wed, 19 Jun 2024 03:00:17 +0000 Subject: [PATCH v2 5/7] merge-ort: loosen commented requirements Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Taylor Blau , Eric Sunshine , Elijah Newren , Elijah Newren From: Elijah Newren From: Elijah Newren The comment above type_short_descriptions claimed that the order had to match what was found in the conflict_info_and_types enum. Since type_short_descriptions uses designated initializers, the order should not actually matter; I am guessing that positional initializers may have been under consideration when that comment was added, but the comment was not updated when designated initializers were chosen. Signed-off-by: Elijah Newren --- merge-ort.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/merge-ort.c b/merge-ort.c index d187c966c6a..d0b13463283 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -553,7 +553,7 @@ enum conflict_and_info_types { * Short description of conflict type, relied upon by external tools. * * We can add more entries, but DO NOT change any of these strings. Also, - * Order MUST match conflict_info_and_types. + * please ensure the order matches what is used in conflict_info_and_types. */ static const char *type_short_descriptions[] = { /*** "Simple" conflicts and informational messages ***/ From patchwork Wed Jun 19 03:00:18 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 13703365 Received: from mail-wm1-f45.google.com (mail-wm1-f45.google.com [209.85.128.45]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1250356766 for ; Wed, 19 Jun 2024 03:00:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.45 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718766032; cv=none; b=X+lkNJJLWKeALT7D3uZEB5xjGvc65nBidOlkT/GjMjCyJSJDrMLlN11K577dv75cAfJtODpGqx0MUksYr9jxH4taynLlBS58pfQkcEuoGZlnxArre6PmUe41sHvEsnaDRO88x9tQR3KJmXKSZd8s35ZixhRNGFGGJW26Yo6bMjw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718766032; c=relaxed/simple; bh=g/s4pDKau4zJ3xPBXI+BdJob4svDmH9t1cmpJjEMLX0=; h=Message-Id:In-Reply-To:References:From:Date:Subject:Content-Type: MIME-Version:To:Cc; b=iM/ah8ID4QKhFcRApwFNhRrURla2xEPSNHytd+vGPa9bLdd5d/I82hhyRNXbyTUJMKdBscMIGyDGkRLZrX4oj5RxyCaJgDVKieF43EcKrtq1YS5r5k2oxyHYTi5B27dfRukiUqOTAm3b07peNw4CppgLkkC/WIVtCiVGnqKpKr8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=PvMLNOTR; arc=none smtp.client-ip=209.85.128.45 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="PvMLNOTR" Received: by mail-wm1-f45.google.com with SMTP id 5b1f17b1804b1-4217990f8baso55093415e9.2 for ; Tue, 18 Jun 2024 20:00:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1718766029; x=1719370829; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=gCcoEWlJCrgh53o0wPpgv1dTJAB9+0KhbA69cFsF4GY=; b=PvMLNOTRfwOi+YwBEKDm85XON0UsJwGjBhIxby3PpcNIukmsUaaqPzoDp0/0xhKOn5 gNCtoF8IjagwcPX9lfvdVsW1aj4t4bb8ydnRHQVLSc3AqOw5CoWEuFynnTVIL6eGULhO XJidS3big0OWwWLNC3iClUyVoRR8XsqH4+YWbw7tk9g6Y64aOh7F0cmbfHfbsSY7BRQE 3yklb4CbrUNJ3hmkeViAjh+ajeLREi0t6B/Biu+apIHnSgPkQd7oV72sfBEnLv0mlec+ +oB4u9grzPu1ZPmtXQ5ZrblekEneS42YZ7wRbUPqueTgKYYWd3eblcPO5F4UAi/ZUez9 MwxQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718766029; x=1719370829; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=gCcoEWlJCrgh53o0wPpgv1dTJAB9+0KhbA69cFsF4GY=; b=FZjo0tQW4cJwD9fHpqgVnyzxVFRrpSOnBzoE9CNe8H9p28rB0nMnsqMsp9BVccdxDk rz/bKW5I9SKLgofvv1S5QuVxJRpst5qELAP5l5fj0rYIElST2IN31SywhYNRVwY129l5 KaxyqcMMdnHkDHrttQfejQVMzRzsbz7pWOdUN3JGK3Ls/9Bk5OxPjIhTsRHkQpF/+Yxr YKfTBETHE7BktiiQ7SnIiOoI4JKTtPo4TDKc8gKqDgrFkjzrkKI5b3bWvbwiNLbpwvpw zJ+TJrL+/wAuZnGd5hvHNShSC0P5pYZqbdtcsFQgE26P2M+xA4FZkHP7q2mrq2jjRrx/ nZPA== X-Gm-Message-State: AOJu0YwVkkg5zVgPmr2VA4wFTLNXoZuM+v5cMl8yjTOO0WEBxpLglWFr 0B4+6hYMApNX+IJ49kj4nsCkahYVnX9jD4o8wKpA2Bwthxss9I7stIEFLQ== X-Google-Smtp-Source: AGHT+IEKU6tAfF8RCkijMOpiFIBT7XMzOMEpTLZwWaSqtQTli1QvkW9fg+nrW82IgNfVGbn1VJt1BQ== X-Received: by 2002:a5d:53cd:0:b0:362:5b83:51dd with SMTP id ffacd0b85a97d-363192cdeb4mr979554f8f.49.1718766029205; Tue, 18 Jun 2024 20:00:29 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4247101aac6sm28683705e9.0.2024.06.18.20.00.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Jun 2024 20:00:28 -0700 (PDT) Message-Id: <6756956d0c7e3672a1a3b362b31b9d7e29bc5b9f.1718766019.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Wed, 19 Jun 2024 03:00:18 +0000 Subject: [PATCH v2 6/7] merge-ort: upon merge abort, only show messages causing the abort Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Taylor Blau , Eric Sunshine , Elijah Newren , Elijah Newren From: Elijah Newren From: Elijah Newren When something goes wrong enough that we need to abort early and not even attempt merging the remaining files, it probably does not make sense to report conflicts messages for the subset of files we processed before hitting the fatal error. Instead, only show the messages associated with paths where we hit the fatal error. Also, print these messages to stderr rather than stdout. Signed-off-by: Elijah Newren --- merge-ort.c | 78 ++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 53 insertions(+), 25 deletions(-) diff --git a/merge-ort.c b/merge-ort.c index d0b13463283..b337e4d74ef 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -543,10 +543,24 @@ enum conflict_and_info_types { CONFLICT_SUBMODULE_HISTORY_NOT_AVAILABLE, CONFLICT_SUBMODULE_MAY_HAVE_REWINDS, CONFLICT_SUBMODULE_NULL_MERGE_BASE, - CONFLICT_SUBMODULE_CORRUPT, + + /* INSERT NEW ENTRIES HERE */ + + /* + * Keep this entry after all regular conflict and info types; only + * errors (failures causing immediate abort of the merge) should + * come after this. + */ + NB_REGULAR_CONFLICT_TYPES, + + /* + * Something is seriously wrong; cannot even perform merge; + * Keep this group _last_ other than NB_TOTAL_TYPES + */ + ERROR_SUBMODULE_CORRUPT, /* Keep this entry _last_ in the list */ - NB_CONFLICT_TYPES, + NB_TOTAL_TYPES, }; /* @@ -597,8 +611,10 @@ static const char *type_short_descriptions[] = { "CONFLICT (submodule may have rewinds)", [CONFLICT_SUBMODULE_NULL_MERGE_BASE] = "CONFLICT (submodule lacks merge base)", - [CONFLICT_SUBMODULE_CORRUPT] = - "CONFLICT (submodule corrupt)" + + /* Something is seriously wrong; cannot even perform merge */ + [ERROR_SUBMODULE_CORRUPT] = + "ERROR (submodule corrupt)", }; struct logical_conflict_info { @@ -762,7 +778,8 @@ static void path_msg(struct merge_options *opt, /* Sanity checks */ assert(omittable_hint == - !starts_with(type_short_descriptions[type], "CONFLICT") || + (!starts_with(type_short_descriptions[type], "CONFLICT") && + !starts_with(type_short_descriptions[type], "ERROR")) || type == CONFLICT_DIR_RENAME_SUGGESTED); if (opt->record_conflict_msgs_as_headers && omittable_hint) return; /* Do not record mere hints in headers */ @@ -1817,9 +1834,9 @@ static int merge_submodule(struct merge_options *opt, /* check whether both changes are forward */ ret2 = repo_in_merge_bases(&subrepo, commit_o, commit_a); if (ret2 < 0) { - path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0, + path_msg(opt, ERROR_SUBMODULE_CORRUPT, 0, path, NULL, NULL, NULL, - _("Failed to merge submodule %s " + _("error: failed to merge submodule %s " "(repository corrupt)"), path); ret = -1; @@ -1828,9 +1845,9 @@ static int merge_submodule(struct merge_options *opt, if (ret2 > 0) ret2 = repo_in_merge_bases(&subrepo, commit_o, commit_b); if (ret2 < 0) { - path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0, + path_msg(opt, ERROR_SUBMODULE_CORRUPT, 0, path, NULL, NULL, NULL, - _("Failed to merge submodule %s " + _("error: failed to merge submodule %s " "(repository corrupt)"), path); ret = -1; @@ -1848,9 +1865,9 @@ static int merge_submodule(struct merge_options *opt, /* Case #1: a is contained in b or vice versa */ ret2 = repo_in_merge_bases(&subrepo, commit_a, commit_b); if (ret2 < 0) { - path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0, + path_msg(opt, ERROR_SUBMODULE_CORRUPT, 0, path, NULL, NULL, NULL, - _("Failed to merge submodule %s " + _("error: failed to merge submodule %s " "(repository corrupt)"), path); ret = -1; @@ -1867,9 +1884,9 @@ static int merge_submodule(struct merge_options *opt, } ret2 = repo_in_merge_bases(&subrepo, commit_b, commit_a); if (ret2 < 0) { - path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0, + path_msg(opt, ERROR_SUBMODULE_CORRUPT, 0, path, NULL, NULL, NULL, - _("Failed to merge submodule %s " + _("error: failed to merge submodule %s " "(repository corrupt)"), path); ret = -1; @@ -1901,9 +1918,9 @@ static int merge_submodule(struct merge_options *opt, &merges); switch (parent_count) { case -1: - path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0, + path_msg(opt, ERROR_SUBMODULE_CORRUPT, 0, path, NULL, NULL, NULL, - _("Failed to merge submodule %s " + _("error: failed to merge submodule %s " "(repository corrupt)"), path); ret = -1; @@ -4646,6 +4663,7 @@ void merge_display_update_messages(struct merge_options *opt, struct hashmap_iter iter; struct strmap_entry *e; struct string_list olist = STRING_LIST_INIT_NODUP; + FILE *o = stdout; if (opt->record_conflict_msgs_as_headers) BUG("Either display conflict messages or record them as headers, not both"); @@ -4662,6 +4680,10 @@ void merge_display_update_messages(struct merge_options *opt, } string_list_sort(&olist); + /* Print to stderr if we hit errors rather than just conflicts */ + if (result->clean < 0) + o = stderr; + /* Iterate over the items, printing them */ for (int path_nr = 0; path_nr < olist.nr; ++path_nr) { struct string_list *conflicts = olist.items[path_nr].util; @@ -4669,25 +4691,31 @@ void merge_display_update_messages(struct merge_options *opt, struct logical_conflict_info *info = conflicts->items[i].util; + /* On failure, ignore regular conflict types */ + if (result->clean < 0 && + info->type < NB_REGULAR_CONFLICT_TYPES) + continue; + if (detailed) { - printf("%lu", (unsigned long)info->paths.nr); - putchar('\0'); + fprintf(o, "%lu", (unsigned long)info->paths.nr); + fputc('\0', o); for (int n = 0; n < info->paths.nr; n++) { - fputs(info->paths.v[n], stdout); - putchar('\0'); + fputs(info->paths.v[n], o); + fputc('\0', o); } - fputs(type_short_descriptions[info->type], - stdout); - putchar('\0'); + fputs(type_short_descriptions[info->type], o); + fputc('\0', o); } - puts(conflicts->items[i].string); + fputs(conflicts->items[i].string, o); + fputc('\n', o); if (detailed) - putchar('\0'); + fputc('\0', o); } } string_list_clear(&olist, 0); - print_submodule_conflict_suggestion(&opti->conflicted_submodules); + if (result->clean >= 0) + print_submodule_conflict_suggestion(&opti->conflicted_submodules); /* Also include needed rename limit adjustment now */ diff_warn_rename_limit("merge.renamelimit", From patchwork Wed Jun 19 03:00:19 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 13703366 Received: from mail-wm1-f53.google.com (mail-wm1-f53.google.com [209.85.128.53]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6EB1657CAC for ; Wed, 19 Jun 2024 03:00:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.53 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718766034; cv=none; b=aK5y0Bp7zSJUP4jqa/q4EOiaTmiliRe227P6Iq5KHfxsjTW8CXoIPq8hNcDmAamEHiQJ52mH22DKIwB+4n39HxBMd+5JnBgksJetRFmEUnDZZ/U3H5bG0Xij+XWZAGvGc77tJ7r5SLlWFvc7RQ82JiAEa7USp2hv2hHMPToR8mY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718766034; c=relaxed/simple; bh=KeZ+lF2i85IQ10NuNm8X7oFz8AX/wN4pg6t3AlQch7g=; h=Message-Id:In-Reply-To:References:From:Date:Subject:Content-Type: MIME-Version:To:Cc; b=aMES521LFfibwmirFRL+CGFuJo8u9wiZ1DD1d3MSHThZGuwrxJBi+HAmD2t85adZ4rTaEgxtWoyTym1cE+o/4c6yX+omzXVTVmmIQc4/m/6xy/OZmL/Q1Bjqtl0LJke1gSFfNKym9YMmw76DjN+zuvpc1PBSXY7bZ2k4wfwRw0U= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=ltw6XKQ2; arc=none smtp.client-ip=209.85.128.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ltw6XKQ2" Received: by mail-wm1-f53.google.com with SMTP id 5b1f17b1804b1-424720e73e0so11182735e9.0 for ; Tue, 18 Jun 2024 20:00:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1718766030; x=1719370830; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=7kdoI+8QhUjtqoKbyy/tMgx0gjM9+4DotxjdiXk1cK4=; b=ltw6XKQ2FY0NHyxDn0iaCYoCpB8988KqaPLpA+tBqLiYEIlPQuF3BteOyhS1ufC9XH IIaPiqyPLZ9qZj37OG+xhHaqXoDn8LvJmi4Bo6uZyv+LlIJYbXklUboucBqxFI5Nn45w wViVZ4MT4wbqCxD1ccBBzny6mIg7WWIPQYpFXXVVbY9C8KXayepadgsB46vmVGtKqwpS IfhuXJ8BNXMT4vPctLu8cWh9jeqoq9cDLSjoeZGa2R9QIDI8L1ycXqRGmgG3NuLPVOjf N36/LEqGQQW0dxnLqRCOF3LXU8TeS0pVjPp+lPSVjKM3eFsvKbzEbjxmUXQXEuBAH1L2 792A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718766030; x=1719370830; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=7kdoI+8QhUjtqoKbyy/tMgx0gjM9+4DotxjdiXk1cK4=; b=OMAZLIl3hR8vVXngQXspdomcswzjdDZPTH6cg+5atO4Gsu4q247OQ+HMgfFNxztRSB Bb11VZEBA3qmiNlWk0DvsZZVeMJ7tpkhMyy+9Fsv43PayPpgEo/ftOvjKfcDi0bkdGnC uxl3zPL8POGN9Z3ltfIDThnnWz1ijGYF6fPfP0yCBjJxu4HuLSCfpY03WsjQjtvYnLx1 px80peb0RlO7i6PHJM1AbP1sKEJrUZf/s/zoW+ZHekxDUVX0fhcSM7I9GgogfZaOq1U5 Jw81LRWoQjgPlHKUrpA2a39KfASvkL4HoVQEXfEh8EIkx4dNNbLmHjQDvlA1qP6WbN4w 5e6g== X-Gm-Message-State: AOJu0YwKoeDw/bB3sB8d5HnVCay5wz/h7TSsOaJzSEg9KM0nFsLrp4p7 asIU8yaXuuUlNM44U6f0RMRvtjybL2mYXx2xydUpQVXEU8d6J8yPSeQFRg== X-Google-Smtp-Source: AGHT+IEg5cj+OPVcE39GXrFNA21Uczb9a2SnflWHemVocRyYmoJDlNfyOCgwwSOab4D5eL3Qp6BboQ== X-Received: by 2002:a05:600c:212:b0:41f:b0e7:f299 with SMTP id 5b1f17b1804b1-424751762e5mr7015055e9.9.1718766030425; Tue, 18 Jun 2024 20:00:30 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-3607509c890sm15803755f8f.28.2024.06.18.20.00.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Jun 2024 20:00:29 -0700 (PDT) Message-Id: <500433edf49a4df448b330e4ed9201cfac83cecf.1718766019.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Wed, 19 Jun 2024 03:00:19 +0000 Subject: [PATCH v2 7/7] merge-ort: convert more error() cases to path_msg() Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Taylor Blau , Eric Sunshine , Elijah Newren , Elijah Newren From: Elijah Newren From: Elijah Newren merge_submodule() stores errors using path_msg(), whereas other call sites make use of the error() function. This is inconsistent, and moving towards path_msg() seems more friendly for libification efforts since it will allow the caller to determine whether the error messages need to be printed. Note that this deferred handling of error messages changes the error message in a recursive merge from error: failed to execute internal merge to From inner merge: error: failed to execute internal merge which provides a little more information about the error which may be useful. Since the recursive merge strategy still only shows the older error, we had to adjust the new testcase introduced a few commits ago to just search for the older message somewhere in the output. Signed-off-by: Elijah Newren --- merge-ort.c | 53 +++++++++++++++++++++++++++++++++---------- t/t6406-merge-attr.sh | 2 +- 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/merge-ort.c b/merge-ort.c index b337e4d74ef..8dfe80f1009 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -558,6 +558,10 @@ enum conflict_and_info_types { * Keep this group _last_ other than NB_TOTAL_TYPES */ ERROR_SUBMODULE_CORRUPT, + ERROR_THREEWAY_CONTENT_MERGE_FAILED, + ERROR_OBJECT_WRITE_FAILED, + ERROR_OBJECT_READ_FAILED, + ERROR_OBJECT_NOT_A_BLOB, /* Keep this entry _last_ in the list */ NB_TOTAL_TYPES, @@ -615,6 +619,14 @@ static const char *type_short_descriptions[] = { /* Something is seriously wrong; cannot even perform merge */ [ERROR_SUBMODULE_CORRUPT] = "ERROR (submodule corrupt)", + [ERROR_THREEWAY_CONTENT_MERGE_FAILED] = + "ERROR (three-way content merge failed)", + [ERROR_OBJECT_WRITE_FAILED] = + "ERROR (object write failed)", + [ERROR_OBJECT_READ_FAILED] = + "ERROR (object read failed)", + [ERROR_OBJECT_NOT_A_BLOB] = + "ERROR (object is not a blob)", }; struct logical_conflict_info { @@ -2190,15 +2202,24 @@ static int handle_content_merge(struct merge_options *opt, pathnames, extra_marker_size, &result_buf); - if ((merge_status < 0) || !result_buf.ptr) - ret = error(_("failed to execute internal merge")); + if ((merge_status < 0) || !result_buf.ptr) { + path_msg(opt, ERROR_THREEWAY_CONTENT_MERGE_FAILED, 0, + pathnames[0], pathnames[1], pathnames[2], NULL, + _("error: failed to execute internal merge for %s"), + path); + ret = -1; + } if (!ret && write_object_file(result_buf.ptr, result_buf.size, - OBJ_BLOB, &result->oid)) - ret = error(_("unable to add %s to database"), path); - + OBJ_BLOB, &result->oid)) { + path_msg(opt, ERROR_OBJECT_WRITE_FAILED, 0, + pathnames[0], pathnames[1], pathnames[2], NULL, + _("error: unable to add %s to database"), path); + ret = -1; + } free(result_buf.ptr); + if (ret) return -1; if (merge_status > 0) @@ -3577,18 +3598,26 @@ static int sort_dirs_next_to_their_children(const char *one, const char *two) return c1 - c2; } -static int read_oid_strbuf(const struct object_id *oid, - struct strbuf *dst) +static int read_oid_strbuf(struct merge_options *opt, + const struct object_id *oid, + struct strbuf *dst, + const char *path) { void *buf; enum object_type type; unsigned long size; buf = repo_read_object_file(the_repository, oid, &type, &size); - if (!buf) - return error(_("cannot read object %s"), oid_to_hex(oid)); + if (!buf) { + path_msg(opt, ERROR_OBJECT_READ_FAILED, 0, + path, NULL, NULL, NULL, + _("error: cannot read object %s"), oid_to_hex(oid)); + return -1; + } if (type != OBJ_BLOB) { free(buf); - return error(_("object %s is not a blob"), oid_to_hex(oid)); + path_msg(opt, ERROR_OBJECT_NOT_A_BLOB, 0, + path, NULL, NULL, NULL, + _("error: object %s is not a blob"), oid_to_hex(oid)); } strbuf_attach(dst, buf, size, size + 1); return 0; @@ -3612,8 +3641,8 @@ static int blob_unchanged(struct merge_options *opt, if (oideq(&base->oid, &side->oid)) return 1; - if (read_oid_strbuf(&base->oid, &basebuf) || - read_oid_strbuf(&side->oid, &sidebuf)) + if (read_oid_strbuf(opt, &base->oid, &basebuf, path) || + read_oid_strbuf(opt, &side->oid, &sidebuf, path)) goto error_return; /* * Note: binary | is used so that both renormalizations are diff --git a/t/t6406-merge-attr.sh b/t/t6406-merge-attr.sh index b6db5c2cc36..9bf95249347 100755 --- a/t/t6406-merge-attr.sh +++ b/t/t6406-merge-attr.sh @@ -295,7 +295,7 @@ test_expect_success !WINDOWS 'custom merge driver that is killed with a signal o >./please-abort && echo "* merge=custom" >.gitattributes && test_expect_code 2 git merge recursive-a 2>err && - grep "^error: failed to execute internal merge" err && + grep "error: failed to execute internal merge" err && git ls-files -u >output && git diff --name-only HEAD >>output && test_must_be_empty output