From patchwork Fri Feb 7 07:34:36 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Karthik Nayak X-Patchwork-Id: 13964458 Received: from mail-pl1-f175.google.com (mail-pl1-f175.google.com [209.85.214.175]) (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 2D8921A2C0E for ; Fri, 7 Feb 2025 07:37:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.175 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738913826; cv=none; b=He6rENwyPGwzC5KzXpxVGYnUdXzFtxwcGevkysafZd8bOYPXY+yWdWByMNy5vpOeSJEpY9j4YrGxy6H2UVum8hOBfbjL1fky+J4Kub0CqEbW6cAr3YMUviG+mBrHe5zEt+PJ7jLlHRs2pXvDKlo1ApY8EET20/Yi7AgjP5rwiwU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738913826; c=relaxed/simple; bh=FPEtb933i/ph+3B1q6fak6jB9o9O6rmyEjnVW9FqYXQ=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=SP9ML4JuKS6y35P/0wlWCRcxZIi60evPf/CAClNo0zZMavjy+CR15r+V/Mju8p+O5Clbc1jK3wPkdvprrRYP6B84FHevJztAioJg96JzjKynhhaqMehEMOSah9YzNWbEJb1m/aDGqAZORkk93zq3RmroFk+V2kGh+VCDvOeBFPo= 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=Adx2v8T0; arc=none smtp.client-ip=209.85.214.175 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="Adx2v8T0" Received: by mail-pl1-f175.google.com with SMTP id d9443c01a7336-21f0c4275a1so26933085ad.2 for ; Thu, 06 Feb 2025 23:37:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738913824; x=1739518624; darn=vger.kernel.org; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=a9dvGWvmJ9Hhk4pAGFZSmGw3UkklUfmgnMIjLCXPLIs=; b=Adx2v8T07IAZ7bc9y0U0876jASDxjEcEq2pTR2KK6nzWI6NG3SZ10KvA3XgqFhEQV7 8e73vFg6s+tT/5/sYO6uckEzs7L9RuTCrIWR70ZHPmp1SHai74jdAIJdvCCx4aJ1lp6h u8G5cnmUlpsMqu86wnoW/McIURLY+ocm7wJgUdY4YTkbuLfupHKWmportkaTsfHe31kg s/GW9z/fDc58/6KV6JXrtmvW2SjJvg6x7pqHfE7G4mNqLXZkiaNRgxxIeEJsU7qTMRUs aX7mCEfK1G7iXjC5BUpCSBYlBLHtK8GgBPvFHGx23DhQwgg1xGqv5lUwKj0PXjhsKj+e iARA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738913824; x=1739518624; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=a9dvGWvmJ9Hhk4pAGFZSmGw3UkklUfmgnMIjLCXPLIs=; b=h15aiuaNuWyB9UPEWtHRgY9jmF7f+GmVGSBD/8jzdiRurF88124VloNNBkXt9uwPse EQOe09OqFL8L7UYnKUEJDkPBdMkRIFSKcuRGMsCKx/UeRxlZdh1pU/BNXgfGdVN7SHi/ d5q/FpiH5Ktxc8EeG8i6SDOweDz7jk0TQ2T8OEPOa65GkAcIWye5RzTa9HsR5YMY75k/ dNcnfPOceWA6CZyd65rr9oLLv1Vvs1MStv5IKyAMDCrs16kn0X0FLkR0NPGs8anzVyQ5 vfeF5rcCAzfXatQpX5HAJIwrZ6WZTjQ7HiXK2B+IuoElwdPdUF6YyWm3pCEiY43tnj/f wrrw== X-Gm-Message-State: AOJu0YzH6ve1olz5Vdwb/uABMQLiM99r+qWK2shdGHYitMqwbgc2VykP 37FDbfgHT9FxmqTUQgy7Po8GI2GvWpCfG8wonRI21aSdlq/fN/wVu5j20Ip3 X-Gm-Gg: ASbGncu51SkuA67ZXdE6XfJcPfFwi5zzcjNkjsI8x87Dh8yH7QCUepf8SsXtKCExppM 72Ms54GXKG/HA/sHRj7/MFmwuAMKCUqI7SxbyAh8KF7Sp/jnEzSuZ7TDkXvrckxHQxtmRCyq8oN GxQC1JG21+E0IMvEtyF1dF21/TlWHaQdHk9+IhBw5cXJpzIafQdm2R9a9KgDaBVQYff550sG68E d+ALG9YmwaBjTgheT4PlbGIk6digQ7IEzwY5gnwO9Bfte2dujqsGAd/aGhfD+T9xRBf5g/TWM91 BGu3jOtLLsRfOJIsp96llA== X-Google-Smtp-Source: AGHT+IE7brL3e0Jafb/WwGom6fHJeNarcoOdHn0/p5HLU88+tjrqtXyfsSpNnDED3sMCgMPgInNPYA== X-Received: by 2002:a05:6a20:9d8f:b0:1e1:b1bb:87a0 with SMTP id adf61e73a8af0-1ee03b12926mr5487509637.34.1738913824077; Thu, 06 Feb 2025 23:37:04 -0800 (PST) Received: from [127.0.0.2] ([2401:4900:33b7:4cb0:4dd3:85f0:5c4b:b677]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-ad51aee79casm2115063a12.44.2025.02.06.23.37.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Feb 2025 23:37:03 -0800 (PST) From: Karthik Nayak Date: Fri, 07 Feb 2025 08:34:36 +0100 Subject: [PATCH 1/6] refs/files: remove duplicate check in `split_symref_update()` Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250207-245-partially-atomic-ref-updates-v1-1-e6a3690ff23a@gmail.com> References: <20250207-245-partially-atomic-ref-updates-v1-0-e6a3690ff23a@gmail.com> In-Reply-To: <20250207-245-partially-atomic-ref-updates-v1-0-e6a3690ff23a@gmail.com> To: git@vger.kernel.org Cc: ps@pks.im, jltobler@gmail.com, Karthik Nayak X-Mailer: b4 0.14.2 X-Developer-Signature: v=1; a=openpgp-sha256; l=3208; i=karthik.188@gmail.com; h=from:subject:message-id; bh=FPEtb933i/ph+3B1q6fak6jB9o9O6rmyEjnVW9FqYXQ=; b=owJ4nAHtARL+kA0DAAoBPtWfJI5GjH8ByyZiAGeluBqKIFiECuMa6u9Yv0S4ELFPekvn0TXk0 zEXEK+M3Dh7NokBswQAAQoAHRYhBFfOTH9jdXEPy2XGBj7VnySORox/BQJnpbgaAAoJED7VnySO Rox/ln0L/0AayNSEXNy2VP8g0B+L/nC3bUvSDohorlOWxACKX5D9m9cs3IMAtAUu4o40nhCoWsG 1eK4pAC+JHlvGYEjyy3mSlHQnRPAfz1ve9ijwgCU9v3cebdF2x+/0LvZHKkHcX2f8RmlYZhKpB9 3tQxe0PDYbXPcsyXImcPa4gle/aPJj4t6qWNyUOgTi8d+zfTTEV1O8cqiJEFft3bdx113xR6CLe +z5QgrtKtjwZep+cKqVd5+vvV9s3+C5TWDy/3QzGlIZP3mPhHWv6FJ1aiDn6OTqe8fsjX84nkSf oAgPADkVO0xI2lght7YcEHrkYgmxmkbh36zwWvTsOWFskzMSVSd0hDKGy7mxAQ9eSKlvTvZ3q+e 1NYkNiHTsNN3HI/OdoXSzVw0kpEV0XBbPpCsvfi7qHB08eAs+oWGt1JkDQP0FaEAwNs7DrBDlya C6egIwbo50QWxNsQ7V4WSnGONl3Ie7Rt6fdZ+mUv3ErnYgGtOJrYrsaazc3p+J/BeF/J6P+O6t7 U0= X-Developer-Key: i=karthik.188@gmail.com; a=openpgp; fpr=57CE4C7F6375710FCB65C6063ED59F248E468C7F In split_symref_update(), there were two redundant checks: - At the start: checking if refname exists in `affected_refnames`. - After adding refname: checking if the item added to `affected_refnames` contains the util field. Remove the second check since the first one already prevents duplicate refnames from being added to the transaction updates. Since this is the only place that utilizes the `item->util` value, avoid setting the value in the first place and cleanup code around it. Signed-off-by: Karthik Nayak --- refs/files-backend.c | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 29f08dced40418eb815072c6335e0c3d1a45c7d8..c6a3f6d6261a894e1c294bb1329fdf8079a39eb4 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2387,7 +2387,6 @@ static int split_head_update(struct ref_update *update, struct string_list *affected_refnames, struct strbuf *err) { - struct string_list_item *item; struct ref_update *new_update; if ((update->flags & REF_LOG_ONLY) || @@ -2426,8 +2425,7 @@ static int split_head_update(struct ref_update *update, */ if (strcmp(new_update->refname, "HEAD")) BUG("%s unexpectedly not 'HEAD'", new_update->refname); - item = string_list_insert(affected_refnames, new_update->refname); - item->util = new_update; + string_list_insert(affected_refnames, new_update->refname); return 0; } @@ -2446,7 +2444,6 @@ static int split_symref_update(struct ref_update *update, struct string_list *affected_refnames, struct strbuf *err) { - struct string_list_item *item; struct ref_update *new_update; unsigned int new_flags; @@ -2501,11 +2498,7 @@ static int split_symref_update(struct ref_update *update, * be valid as long as affected_refnames is in use, and NOT * referent, which might soon be freed by our caller. */ - item = string_list_insert(affected_refnames, new_update->refname); - if (item->util) - BUG("%s unexpectedly found in affected_refnames", - new_update->refname); - item->util = new_update; + string_list_insert(affected_refnames, new_update->refname); return 0; } @@ -2837,7 +2830,6 @@ static int files_transaction_prepare(struct ref_store *ref_store, */ for (i = 0; i < transaction->nr; i++) { struct ref_update *update = transaction->updates[i]; - struct string_list_item *item; if ((update->flags & REF_IS_PRUNING) && !(update->flags & REF_NO_DEREF)) @@ -2846,13 +2838,7 @@ static int files_transaction_prepare(struct ref_store *ref_store, if (update->flags & REF_LOG_ONLY) continue; - item = string_list_append(&affected_refnames, update->refname); - /* - * We store a pointer to update in item->util, but at - * the moment we never use the value of this field - * except to check whether it is non-NULL. - */ - item->util = update; + string_list_append(&affected_refnames, update->refname); } string_list_sort(&affected_refnames); if (ref_update_reject_duplicates(&affected_refnames, err)) { From patchwork Fri Feb 7 07:34:37 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Karthik Nayak X-Patchwork-Id: 13964459 Received: from mail-pl1-f175.google.com (mail-pl1-f175.google.com [209.85.214.175]) (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 E64CE23314B for ; Fri, 7 Feb 2025 07:37:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.175 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738913830; cv=none; b=Y6R23h6IOsGX516ZpWSqKhVYjbPsAuv4QqSvjdbYIXnAhId/pVfmiOOWkIvouoUfaJ9M/LY5UcUvNn35SrPYiVf142FiuJkF07gc3hucjsGTI0ww/4uJMFLvGWefUP6+XE46Bl+094t8YZ5acYaVxMO6/r+5tL+Ru/5pQnVJSCI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738913830; c=relaxed/simple; bh=ZIn7CMRwDia1Tkipj9WqcC9yYE0tbSEAKUBDbq3a61c=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=AEnMUZf+r/3wQ44bgqsMdLOCEiUJw++EZroaP728Mg6cNYiMBUB7u2scSOjaG6CX3Q5X+58flyvg4Oz3FflcLyE912kXYbmXPYtCRYANlsV2+e1ISYOwcZnOb9qSRo58cd1FniMpJhS0lHP8uaqKH7s2BFSW+SopEIsP+xDN+lQ= 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=LAiAlWoO; arc=none smtp.client-ip=209.85.214.175 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="LAiAlWoO" Received: by mail-pl1-f175.google.com with SMTP id d9443c01a7336-21f0c4275a1so26933655ad.2 for ; Thu, 06 Feb 2025 23:37:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738913827; x=1739518627; darn=vger.kernel.org; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=IFM+6CxvGV+3T3C+TJ8HHpHfPfbBXHVjHURFNp3xsAY=; b=LAiAlWoOWtW8AoltGTHLHf4aV3iue/nPqWvjbenl5WIlMyyQYvls1s7a6Xqmt6UUSn IPQEK0Z57EzpcbW3mn7SihIJTqWXF0JTpMU94MtH3Qjg6/LEgyF5PU+NwNSPhOkIYmWO SbW/Mqs73rLGzgpRuL8lJrqogzrmfVriMtFclpW3+Fq+J5mQySwNJ+pHmrWW3AW+TOTa fRsPGbq2mY5Nfu4zcn2VvhzcyGBBJ4okqr38Zwjcq6/7Wc205WpvDVdFZHeimG4Hm9ps WEAo/1E2pEZNyy9v7Uzh5S+SzQIZ2aLqqtJk8H+/A7R9xU3FlTYiCKDw+rsiqDaIO36N Waxg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738913827; x=1739518627; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=IFM+6CxvGV+3T3C+TJ8HHpHfPfbBXHVjHURFNp3xsAY=; b=U96hcf7a9gPeHzDk1KlyE1AVhLki8WpYCOg8i9XuX8GUIMF4HHbfiLhBz0Al5b3W8J O+CzmMaT4OyzoPdl6uoIMPCYQMwd/FKwzoSGICIiNMc2OCtv67pRUTBk1AyDYhx+yuqr POFNpmownstP8MM2sDtcjT7qW2cEILv2/SwFMSrt7918MsuRV3tjRQOib5kFbQgBI6y3 sV3e4q8hMuSUJCoznMWTNQlHhBFBoNNK+uaMO3KGAjr6rQYWAMrSfr7Dk6OWqcevsA0U SSK/49rPzPBvRtJ01EX6+l7MncFTzCHCBk0N8Aw6OXlmFqWOb5wqMM7+hzgItKLZbGa3 e3pw== X-Gm-Message-State: AOJu0Yx3QWykY/NkrMZHbhH6ItP8E4awvTclJM5GW90vXyCrnNeDDMOs fVPJ/MHOjB9luqXWegVLg/rZ20PY7Kze0YD0CUi2Hv8yDT0qT7ly+6jPixfj X-Gm-Gg: ASbGncubEL5gdxjgNjR6E9kkghjEbocKNkkIGmdpg+n/amulOZLUBmQaHCCpJotIFpJ Fg1GVUkevAYnfX22ZA/2ayVr6GyjxWqcn8zFuY35bZ+/7bb4vhIpIBugSPhUY9BqrpzXWZdiTKq CyguhTVsxacJkLjSyCw1bctt9hZTNO3q6xqTcKXQqCik71c66rtpbEFwxJPad0jsClsK98OTu2f XyJPQDnZgzc9+8soT65OjUdN9QtgToRNcJ0RypQpeHxq+yMzPEw2cw2GvyxsB2wogtNn45kGC4j LHtGd5e9cAbAY54u2mbVKg== X-Google-Smtp-Source: AGHT+IEvc+2DpKfvxJnjZTYbGcoTMMzfI0XPfMfRfAFoJX8gAwrx98dRNpA/nKaYAMoWWCmyokBjgg== X-Received: by 2002:a17:903:240f:b0:216:69ca:7714 with SMTP id d9443c01a7336-21f4e6a106emr40330535ad.11.1738913826516; Thu, 06 Feb 2025 23:37:06 -0800 (PST) Received: from [127.0.0.2] ([2401:4900:33b7:4cb0:4dd3:85f0:5c4b:b677]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-ad51aee79casm2115063a12.44.2025.02.06.23.37.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Feb 2025 23:37:06 -0800 (PST) From: Karthik Nayak Date: Fri, 07 Feb 2025 08:34:37 +0100 Subject: [PATCH 2/6] refs: move duplicate refname update check to generic layer Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250207-245-partially-atomic-ref-updates-v1-2-e6a3690ff23a@gmail.com> References: <20250207-245-partially-atomic-ref-updates-v1-0-e6a3690ff23a@gmail.com> In-Reply-To: <20250207-245-partially-atomic-ref-updates-v1-0-e6a3690ff23a@gmail.com> To: git@vger.kernel.org Cc: ps@pks.im, jltobler@gmail.com, Karthik Nayak X-Mailer: b4 0.14.2 X-Developer-Signature: v=1; a=openpgp-sha256; l=19275; i=karthik.188@gmail.com; h=from:subject:message-id; bh=ZIn7CMRwDia1Tkipj9WqcC9yYE0tbSEAKUBDbq3a61c=; b=owJ4nAHtARL+kA0DAAoBPtWfJI5GjH8ByyZiAGeluBq5L72fE0kuwEz9mXT6TmjjfqsGObL5+ ycMFjmIZDve54kBswQAAQoAHRYhBFfOTH9jdXEPy2XGBj7VnySORox/BQJnpbgaAAoJED7VnySO Rox/o7UMAIs3XdvETmpJ8FCvkrx3JBtWrhHikFk1P3cnc9ZsyD1vl0WCYMmtTJGHcghjjsA6+MM p1go2C/2kK3THSLapBDosY27o1XN/8cFAvdhI+5MxRL23OBLUHypGvJmvcaa3B00W3Q4G1ZdIFG UE/SWFNNrWQ57mEVUUjuAqmaPPUBjpFiZtxqSrO43xp5tHEcBTgUz60O9tgD2iQydV0508KTA1i WRmzRZncBXR/q6M7wXV3BglAkghwQPVqtvU66R4tPgQwYU79g+qL9iBVqgTMeipXik4RT/pz+gr TYxsP93r4KLMBPGQslVrbEtJGjhVcCRIADHyTnvFi03R1XkDc0Gw2zPe4Jw8SNEzqzM2gPWzn+4 tfBi1oSWQ9coswEYE4PL5CugukHzWARUF/uMzuWfM9bx3SLHbYiKyUHgaVp+RUBu8y36p3ltcx7 90F8XKBHRnoLHHxS4RIhiVDKKf4E/NeZMOwEH0e8v+Oo9aBeOTQ85zivKZlMo8UMtA9B5DRTKI5 Yo= X-Developer-Key: i=karthik.188@gmail.com; a=openpgp; fpr=57CE4C7F6375710FCB65C6063ED59F248E468C7F Move the tracking of refnames in `affected_refnames` from individual backends into the generic layer in 'refs.c'. This centralizes the duplicate refname detection that was previously handled separately by each backend. Make some changes to accommodate this move: - Add a `string_list` field `refnames` to `ref_transaction` to contain all the references in a transaction. This field is updated whenever a new update is added. - Modify the backends to use this field internally as needed. The backends need to check if an update for refname already exists when splitting symrefs or adding an update for 'HEAD'. - In the reftable backend, in `reftable_be_transaction_prepare()`, move the instance of `string_list_has_string()` above `ref_transaction_add_update()` to check before the reference is added. This helps reduce duplication of functionality between the backends and makes it easier to make changes in a more centralized manner. Signed-off-by: Karthik Nayak --- refs.c | 17 ++++++++++++ refs/files-backend.c | 69 ++++++++++--------------------------------------- refs/packed-backend.c | 25 +----------------- refs/refs-internal.h | 2 ++ refs/reftable-backend.c | 53 ++++++++++++------------------------- 5 files changed, 50 insertions(+), 116 deletions(-) diff --git a/refs.c b/refs.c index f4094a326a9f88f979654b668cc9c3d27d83cb5d..4c9b706461977995be1d55e7667f7fb708fbbb76 100644 --- a/refs.c +++ b/refs.c @@ -1175,6 +1175,7 @@ struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs, CALLOC_ARRAY(tr, 1); tr->ref_store = refs; tr->flags = flags; + string_list_init_dup(&tr->refnames); return tr; } @@ -1205,6 +1206,7 @@ void ref_transaction_free(struct ref_transaction *transaction) free((char *)transaction->updates[i]->old_target); free(transaction->updates[i]); } + string_list_clear(&transaction->refnames, 0); free(transaction->updates); free(transaction); } @@ -1218,6 +1220,7 @@ struct ref_update *ref_transaction_add_update( const char *committer_info, const char *msg) { + struct string_list_item *item; struct ref_update *update; if (transaction->state != REF_TRANSACTION_OPEN) @@ -1245,6 +1248,16 @@ struct ref_update *ref_transaction_add_update( update->msg = normalize_reflog_message(msg); } + /* + * This list is generally used by the backends to avoid duplicates. + * But we do support multiple log updates for a given refname within + * a single transaction. + */ + if (!(update->flags & REF_LOG_ONLY)) { + item = string_list_append(&transaction->refnames, refname); + item->util = update; + } + return update; } @@ -2397,6 +2410,10 @@ int ref_transaction_prepare(struct ref_transaction *transaction, return -1; } + string_list_sort(&transaction->refnames); + if (ref_update_reject_duplicates(&transaction->refnames, err)) + return TRANSACTION_GENERIC_ERROR; + ret = refs->be->transaction_prepare(refs, transaction, err); if (ret) return ret; diff --git a/refs/files-backend.c b/refs/files-backend.c index c6a3f6d6261a894e1c294bb1329fdf8079a39eb4..18da30c3f37dc5c09f7d81a9083d6b41d0463bd5 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2383,9 +2383,7 @@ static struct ref_iterator *files_reflog_iterator_begin(struct ref_store *ref_st */ static int split_head_update(struct ref_update *update, struct ref_transaction *transaction, - const char *head_ref, - struct string_list *affected_refnames, - struct strbuf *err) + const char *head_ref, struct strbuf *err) { struct ref_update *new_update; @@ -2403,7 +2401,7 @@ static int split_head_update(struct ref_update *update, * transaction. This check is O(lg N) in the transaction * size, but it happens at most once per transaction. */ - if (string_list_has_string(affected_refnames, "HEAD")) { + if (string_list_has_string(&transaction->refnames, "HEAD")) { /* An entry already existed */ strbuf_addf(err, "multiple updates for 'HEAD' (including one " @@ -2425,7 +2423,6 @@ static int split_head_update(struct ref_update *update, */ if (strcmp(new_update->refname, "HEAD")) BUG("%s unexpectedly not 'HEAD'", new_update->refname); - string_list_insert(affected_refnames, new_update->refname); return 0; } @@ -2441,7 +2438,6 @@ static int split_head_update(struct ref_update *update, static int split_symref_update(struct ref_update *update, const char *referent, struct ref_transaction *transaction, - struct string_list *affected_refnames, struct strbuf *err) { struct ref_update *new_update; @@ -2453,7 +2449,7 @@ static int split_symref_update(struct ref_update *update, * size, but it happens at most once per symref in a * transaction. */ - if (string_list_has_string(affected_refnames, referent)) { + if (string_list_has_string(&transaction->refnames, referent)) { /* An entry already exists */ strbuf_addf(err, "multiple updates for '%s' (including one " @@ -2491,15 +2487,6 @@ static int split_symref_update(struct ref_update *update, update->flags |= REF_LOG_ONLY | REF_NO_DEREF; update->flags &= ~REF_HAVE_OLD; - /* - * Add the referent. This insertion is O(N) in the transaction - * size, but it happens at most once per symref in a - * transaction. Make sure to add new_update->refname, which will - * be valid as long as affected_refnames is in use, and NOT - * referent, which might soon be freed by our caller. - */ - string_list_insert(affected_refnames, new_update->refname); - return 0; } @@ -2561,9 +2548,7 @@ struct files_transaction_backend_data { static int lock_ref_for_update(struct files_ref_store *refs, struct ref_update *update, struct ref_transaction *transaction, - const char *head_ref, - struct string_list *affected_refnames, - struct strbuf *err) + const char *head_ref, struct strbuf *err) { struct strbuf referent = STRBUF_INIT; int mustexist = ref_update_expects_existing_old_ref(update); @@ -2579,8 +2564,7 @@ static int lock_ref_for_update(struct files_ref_store *refs, update->flags |= REF_DELETING; if (head_ref) { - ret = split_head_update(update, transaction, head_ref, - affected_refnames, err); + ret = split_head_update(update, transaction, head_ref, err); if (ret) goto out; } @@ -2590,7 +2574,7 @@ static int lock_ref_for_update(struct files_ref_store *refs, lock->count++; } else { ret = lock_raw_ref(refs, update->refname, mustexist, - affected_refnames, + &transaction->refnames, &lock, &referent, &update->type, err); if (ret) { @@ -2646,9 +2630,8 @@ static int lock_ref_for_update(struct files_ref_store *refs, * of processing the split-off update, so we * don't have to do it here. */ - ret = split_symref_update(update, - referent.buf, transaction, - affected_refnames, err); + ret = split_symref_update(update, referent.buf, + transaction, err); if (ret) goto out; } @@ -2803,7 +2786,6 @@ static int files_transaction_prepare(struct ref_store *ref_store, "ref_transaction_prepare"); size_t i; int ret = 0; - struct string_list affected_refnames = STRING_LIST_INIT_NODUP; char *head_ref = NULL; int head_type; struct files_transaction_backend_data *backend_data; @@ -2821,12 +2803,7 @@ static int files_transaction_prepare(struct ref_store *ref_store, transaction->backend_data = backend_data; /* - * Fail if a refname appears more than once in the - * transaction. (If we end up splitting up any updates using - * split_symref_update() or split_head_update(), those - * functions will check that the new updates don't have the - * same refname as any existing ones.) Also fail if any of the - * updates use REF_IS_PRUNING without REF_NO_DEREF. + * Fail if any of the updates use REF_IS_PRUNING without REF_NO_DEREF. */ for (i = 0; i < transaction->nr; i++) { struct ref_update *update = transaction->updates[i]; @@ -2834,16 +2811,6 @@ static int files_transaction_prepare(struct ref_store *ref_store, if ((update->flags & REF_IS_PRUNING) && !(update->flags & REF_NO_DEREF)) BUG("REF_IS_PRUNING set without REF_NO_DEREF"); - - if (update->flags & REF_LOG_ONLY) - continue; - - string_list_append(&affected_refnames, update->refname); - } - string_list_sort(&affected_refnames); - if (ref_update_reject_duplicates(&affected_refnames, err)) { - ret = TRANSACTION_GENERIC_ERROR; - goto cleanup; } /* @@ -2884,7 +2851,7 @@ static int files_transaction_prepare(struct ref_store *ref_store, struct ref_update *update = transaction->updates[i]; ret = lock_ref_for_update(refs, update, transaction, - head_ref, &affected_refnames, err); + head_ref, err); if (ret) goto cleanup; @@ -2957,7 +2924,6 @@ static int files_transaction_prepare(struct ref_store *ref_store, cleanup: free(head_ref); - string_list_clear(&affected_refnames, 0); if (ret) files_transaction_cleanup(refs, transaction); @@ -3021,7 +2987,6 @@ static int files_transaction_finish_initial(struct files_ref_store *refs, { size_t i; int ret = 0; - struct string_list affected_refnames = STRING_LIST_INIT_NODUP; struct ref_transaction *packed_transaction = NULL; struct ref_transaction *loose_transaction = NULL; @@ -3030,13 +2995,8 @@ static int files_transaction_finish_initial(struct files_ref_store *refs, if (transaction->state != REF_TRANSACTION_PREPARED) BUG("commit called for transaction that is not prepared"); - /* Fail if a refname appears more than once in the transaction: */ - for (i = 0; i < transaction->nr; i++) - if (!(transaction->updates[i]->flags & REF_LOG_ONLY)) - string_list_append(&affected_refnames, - transaction->updates[i]->refname); - string_list_sort(&affected_refnames); - if (ref_update_reject_duplicates(&affected_refnames, err)) { + string_list_sort(&transaction->refnames); + if (ref_update_reject_duplicates(&transaction->refnames, err)) { ret = TRANSACTION_GENERIC_ERROR; goto cleanup; } @@ -3054,7 +3014,7 @@ static int files_transaction_finish_initial(struct files_ref_store *refs, * that we are creating already exists. */ if (refs_for_each_rawref(&refs->base, ref_present, - &affected_refnames)) + &transaction->refnames)) BUG("initial ref transaction called with existing refs"); packed_transaction = ref_store_transaction_begin(refs->packed_ref_store, @@ -3072,7 +3032,7 @@ static int files_transaction_finish_initial(struct files_ref_store *refs, BUG("initial ref transaction with old_sha1 set"); if (refs_verify_refname_available(&refs->base, update->refname, - &affected_refnames, NULL, 1, err)) { + &transaction->refnames, NULL, 1, err)) { ret = TRANSACTION_NAME_CONFLICT; goto cleanup; } @@ -3132,7 +3092,6 @@ static int files_transaction_finish_initial(struct files_ref_store *refs, if (packed_transaction) ref_transaction_free(packed_transaction); transaction->state = REF_TRANSACTION_CLOSED; - string_list_clear(&affected_refnames, 0); return ret; } diff --git a/refs/packed-backend.c b/refs/packed-backend.c index a7b6f74b6e35f897f619c540cbc600bbd888bc67..6e7acb077e81435715a1ca3cc928550147c8c56a 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1604,8 +1604,6 @@ int is_packed_transaction_needed(struct ref_store *ref_store, struct packed_transaction_backend_data { /* True iff the transaction owns the packed-refs lock. */ int own_lock; - - struct string_list updates; }; static void packed_transaction_cleanup(struct packed_ref_store *refs, @@ -1614,8 +1612,6 @@ static void packed_transaction_cleanup(struct packed_ref_store *refs, struct packed_transaction_backend_data *data = transaction->backend_data; if (data) { - string_list_clear(&data->updates, 0); - if (is_tempfile_active(refs->tempfile)) delete_tempfile(&refs->tempfile); @@ -1640,7 +1636,6 @@ static int packed_transaction_prepare(struct ref_store *ref_store, REF_STORE_READ | REF_STORE_WRITE | REF_STORE_ODB, "ref_transaction_prepare"); struct packed_transaction_backend_data *data; - size_t i; int ret = TRANSACTION_GENERIC_ERROR; /* @@ -1653,34 +1648,16 @@ static int packed_transaction_prepare(struct ref_store *ref_store, */ CALLOC_ARRAY(data, 1); - string_list_init_nodup(&data->updates); transaction->backend_data = data; - /* - * Stick the updates in a string list by refname so that we - * can sort them: - */ - for (i = 0; i < transaction->nr; i++) { - struct ref_update *update = transaction->updates[i]; - struct string_list_item *item = - string_list_append(&data->updates, update->refname); - - /* Store a pointer to update in item->util: */ - item->util = update; - } - string_list_sort(&data->updates); - - if (ref_update_reject_duplicates(&data->updates, err)) - goto failure; - if (!is_lock_file_locked(&refs->lock)) { if (packed_refs_lock(ref_store, 0, err)) goto failure; data->own_lock = 1; } - if (write_with_updates(refs, &data->updates, err)) + if (write_with_updates(refs, &transaction->refnames, err)) goto failure; transaction->state = REF_TRANSACTION_PREPARED; diff --git a/refs/refs-internal.h b/refs/refs-internal.h index aaab711bb96844755dfa600d37efdb25b30a0765..f43766e1f00443eb689685cf4df0fa0b80018a03 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -3,6 +3,7 @@ #include "refs.h" #include "iterator.h" +#include "string-list.h" struct fsck_options; struct ref_transaction; @@ -198,6 +199,7 @@ enum ref_transaction_state { struct ref_transaction { struct ref_store *ref_store; struct ref_update **updates; + struct string_list refnames; size_t alloc; size_t nr; enum ref_transaction_state state; diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index d39a14c5a469d7d219362e9eae4f578784d65a5b..dd2099d94948a4f23fd9f7ddc06bf3d741229eba 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1068,7 +1068,6 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, struct reftable_ref_store *refs = reftable_be_downcast(ref_store, REF_STORE_WRITE|REF_STORE_MAIN, "ref_transaction_prepare"); struct strbuf referent = STRBUF_INIT, head_referent = STRBUF_INIT; - struct string_list affected_refnames = STRING_LIST_INIT_NODUP; struct reftable_transaction_data *tx_data = NULL; struct reftable_backend *be; struct object_id head_oid; @@ -1092,10 +1091,6 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, transaction->updates[i], err); if (ret) goto done; - - if (!(transaction->updates[i]->flags & REF_LOG_ONLY)) - string_list_append(&affected_refnames, - transaction->updates[i]->refname); } /* @@ -1107,17 +1102,6 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, tx_data->args[i].updates_alloc = tx_data->args[i].updates_expected; } - /* - * Fail if a refname appears more than once in the transaction. - * This code is taken from the files backend and is a good candidate to - * be moved into the generic layer. - */ - string_list_sort(&affected_refnames); - if (ref_update_reject_duplicates(&affected_refnames, err)) { - ret = TRANSACTION_GENERIC_ERROR; - goto done; - } - /* * TODO: it's dubious whether we should reload the stack that "HEAD" * belongs to or not. In theory, it may happen that we only modify @@ -1185,14 +1169,12 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, !(u->flags & REF_LOG_ONLY) && !(u->flags & REF_UPDATE_VIA_HEAD) && !strcmp(rewritten_ref, head_referent.buf)) { - struct ref_update *new_update; - /* * First make sure that HEAD is not already in the * transaction. This check is O(lg N) in the transaction * size, but it happens at most once per transaction. */ - if (string_list_has_string(&affected_refnames, "HEAD")) { + if (string_list_has_string(&transaction->refnames, "HEAD")) { /* An entry already existed */ strbuf_addf(err, _("multiple updates for 'HEAD' (including one " @@ -1202,12 +1184,11 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, goto done; } - new_update = ref_transaction_add_update( - transaction, "HEAD", - u->flags | REF_LOG_ONLY | REF_NO_DEREF, - &u->new_oid, &u->old_oid, NULL, NULL, NULL, - u->msg); - string_list_insert(&affected_refnames, new_update->refname); + ref_transaction_add_update( + transaction, "HEAD", + u->flags | REF_LOG_ONLY | REF_NO_DEREF, + &u->new_oid, &u->old_oid, NULL, NULL, NULL, + u->msg); } ret = reftable_backend_read_ref(be, rewritten_ref, @@ -1225,7 +1206,7 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, * at a later point. */ ret = refs_verify_refname_available(ref_store, u->refname, - &affected_refnames, NULL, + &transaction->refnames, NULL, transaction->flags & REF_TRANSACTION_FLAG_INITIAL, err); if (ret < 0) @@ -1277,6 +1258,15 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, if (!strcmp(rewritten_ref, "HEAD")) new_flags |= REF_UPDATE_VIA_HEAD; + if (string_list_has_string(&transaction->refnames, referent.buf)) { + strbuf_addf(err, + _("multiple updates for '%s' (including one " + "via symref '%s') are not allowed"), + referent.buf, u->refname); + ret = TRANSACTION_NAME_CONFLICT; + goto done; + } + /* * If we are updating a symref (eg. HEAD), we should also * update the branch that the symref points to. @@ -1301,16 +1291,6 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, */ u->flags |= REF_LOG_ONLY | REF_NO_DEREF; u->flags &= ~REF_HAVE_OLD; - - if (string_list_has_string(&affected_refnames, new_update->refname)) { - strbuf_addf(err, - _("multiple updates for '%s' (including one " - "via symref '%s') are not allowed"), - referent.buf, u->refname); - ret = TRANSACTION_NAME_CONFLICT; - goto done; - } - string_list_insert(&affected_refnames, new_update->refname); } } @@ -1391,7 +1371,6 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, strbuf_addf(err, _("reftable: transaction prepare: %s"), reftable_error_str(ret)); } - string_list_clear(&affected_refnames, 0); strbuf_release(&referent); strbuf_release(&head_referent); From patchwork Fri Feb 7 07:34:38 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Karthik Nayak X-Patchwork-Id: 13964460 Received: from mail-pl1-f173.google.com (mail-pl1-f173.google.com [209.85.214.173]) (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 53336233D98 for ; Fri, 7 Feb 2025 07:37:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.173 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738913831; cv=none; b=oq5hfOtfJhDxJqU9ZX/0xbAdr3/baX8jJ1PpbzD/U4tvBzfigLH++5UaN12CLtXbkjLIxCbOIkrF53UNX1cGtOn8DII8DyDHLMUdB3bAvmuu1Dc4I9fYQd0pz/JJvBnArr0UbtHk9FdEmVMVyhBW+km6K33cO/C9FNFnRyly3Ps= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738913831; c=relaxed/simple; bh=/93ilwoAPUAPhaigNwnzxuvu5ZqOVZO0gIj2Cz+Tvyw=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=oiC3/5UlZDM2bfghTqvmAmTM4MlkKvxjRWP/qwny6iGICKhj4FhRJ3e/SdR0+hJxI9UqhGHJYJaSUs3blSL6Yt7U1pi9jEwWBDcknUGgWzJHvd6uzDpr4gPbciYrJz3TOsGaEw1Xxqu1iQxde0Z31BtRE090JhNngevdFVjbzEc= 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=Dx8H3Pa2; arc=none smtp.client-ip=209.85.214.173 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="Dx8H3Pa2" Received: by mail-pl1-f173.google.com with SMTP id d9443c01a7336-21c2f1b610dso43496015ad.0 for ; Thu, 06 Feb 2025 23:37:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738913829; x=1739518629; darn=vger.kernel.org; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=E8mVxehKNFPgAh/+uKFSdFCJxS5RQi97Hzg+MSLee/s=; b=Dx8H3Pa2P2XOG++M9NKdBAS9tivAdGu3Vz21jaLODabrciKs9ZbTahGnfBsW/nAbvJ mSnv9jaX9d8YSd5L1p5Ochen8DuRWFuhHk5zoq4fRc2y3uo6ZJcuvYBhFY/WRGe/2roA CfjRafd/V/EjV3Ra9kbBHYUrJwIDpgZXHy9U9Xh3mLaq7gYi8M9l+eVdfyEWGUu0OEi7 AdiTQwks7BJtxf+FSblRxbAazfDwM/ZR3t+WP8haUTGTFkjjg6jLbjSPQdrXHGMc2bdU fI16q+47vRwLzuphdeNdSZ3j69dgist5h6HWMt1F4v8tKlEAiQhzHtMamRtTo609V2ZY Q6MQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738913829; x=1739518629; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=E8mVxehKNFPgAh/+uKFSdFCJxS5RQi97Hzg+MSLee/s=; b=G3EJaic3gXaV5ldK7Bg6HAzhVzWx5cNCwZKAjIIkITEdRJb9NNCqlrImyDL4JBM5b6 +nCkClpsxGyVADfe36wWbjmiPQlsNTXCHcF1Rhm28DVOzN1lGCyeKAESWWbb0t9YdUF3 5U76bwhdu/7Bm8yo84nYpobLsqs3Uft2WdVivaaIdNtsreMsXjgARbNUiRhPmyMlMy46 4P/IYpZrg1oVPScDk2aAtDIdvNSMvvygw8FNa6oqXKW+9ZJQ3aA+GHKQVXHOfxxSLA5w Hy1ydRHNJyBFAb/1YGjjzQScHu/xc4kbmiOyoDkZOg45u5E7xAD/tQKMqjd/9R2YUmKS FcYA== X-Gm-Message-State: AOJu0Yzox0+QL9aeHRvICBnyIsLoSdzwFUdO4DNeTTfaHgFp64ErH3SO gkj8twxmBho6CVcPaoF6OdDRvCOz61EbCUCpQ5QDzTXCzcMYMGd67IFYCfje X-Gm-Gg: ASbGncuCvPPkjGhjLXMN7J/jC2ybhSUftnK67lZ8y7rUpBk2delGjGrSxZLfqQOicud 2PspnfzPvgMMsep+yvlOpMQ6XTf6Nqi0X/irqgg4nVs9xJsbsS3hm9gcc3jheOb6brVOM0sVPkn QgJZqFB9iTISA/JRIgTl4C5iPIppkwEgqsKHvR1DvLT9coSv4QPb7iMIQt008S3+Igjz7FkW8LT bX/EyZS5ZRIgr2t7i/av4MXb1xVonixjJ+JulMCnS64bYFZG/iXxWGO07+ZmxB3/8WRfux5hF/F Yk7JGOPIXG4pZYSeBKkDLw== X-Google-Smtp-Source: AGHT+IEafYOdRCNVHdjzte0WL03dGO6Nvfb6yJtjZCtvlJBkRyejsfU95Z086we5rK9AAQqmbIeamg== X-Received: by 2002:a05:6a21:100c:b0:1db:eead:c588 with SMTP id adf61e73a8af0-1ee03b20a2bmr4510835637.29.1738913829297; Thu, 06 Feb 2025 23:37:09 -0800 (PST) Received: from [127.0.0.2] ([2401:4900:33b7:4cb0:4dd3:85f0:5c4b:b677]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-ad51aee79casm2115063a12.44.2025.02.06.23.37.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Feb 2025 23:37:09 -0800 (PST) From: Karthik Nayak Date: Fri, 07 Feb 2025 08:34:38 +0100 Subject: [PATCH 3/6] refs/files: remove duplicate duplicates check Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250207-245-partially-atomic-ref-updates-v1-3-e6a3690ff23a@gmail.com> References: <20250207-245-partially-atomic-ref-updates-v1-0-e6a3690ff23a@gmail.com> In-Reply-To: <20250207-245-partially-atomic-ref-updates-v1-0-e6a3690ff23a@gmail.com> To: git@vger.kernel.org Cc: ps@pks.im, jltobler@gmail.com, Karthik Nayak X-Mailer: b4 0.14.2 X-Developer-Signature: v=1; a=openpgp-sha256; l=3178; i=karthik.188@gmail.com; h=from:subject:message-id; bh=/93ilwoAPUAPhaigNwnzxuvu5ZqOVZO0gIj2Cz+Tvyw=; b=owJ4nAHtARL+kA0DAAoBPtWfJI5GjH8ByyZiAGeluBqZxXX/tH5tGKdpnGwNgJNy1psoip+av hl7q9UDpnVkpokBswQAAQoAHRYhBFfOTH9jdXEPy2XGBj7VnySORox/BQJnpbgaAAoJED7VnySO Rox/TXsL/iAr3ru0YXw0KF3QgslNjPm6o8quulcvAg2XugzKYJrbI9vDuj1CnAjHvsHMvAhQhJK 8CcjmLtSRPr9zewxg3MZr3i5ijBWm3P7OPQCixROAJy9TdGemmBZgqayXrM1FdX78KiMI60xXlH UYBmVlD6lWHJrLDBRocj+0j4kXKogPe1/EihF8OAFGxgMDPmRHpERMC4TzD0/QPUkzlWJArjY91 NOaWoOLIp9EIq+2Bn6Eoi3glGipXcEPrJFC5JnSDPzcAVWzl4jNetgE1VIGGW3chNK6SaaOP3r3 TJIHJo3zUN9agDJiX2Uc6gNCX5Yw3aNIk8rHXDFTLSXCuiuy3qHYnoQtfgmgOERTSEpIhhOt/En u3eXuyHNwAICGsFQZvU9OUvNGhEZ9qbDVQfICHuJDFOU0kF235c9+Zc7i9c3Bx2o7JkSLY8C3VJ YLpFrNcq9Qj6YUq9v4PLbeog056el3PXhgY0ngP1M8I40qkbylVIdsnsUwJ3pTIzua+9iHu55U4 cc= X-Developer-Key: i=karthik.188@gmail.com; a=openpgp; fpr=57CE4C7F6375710FCB65C6063ED59F248E468C7F Within the files reference backend's transaction's 'finish' phase, a verification step is currently performed wherein the refnames list is sorted and examined for multiple updates targeting the same refname. It has been observed that this verification is redundant, as an identical check is already executed during the transaction's 'prepare' stage. Since the refnames list remains unmodified following the 'prepare' stage, this secondary verification can be safely eliminated. The duplicate check has been removed accordingly, and the `ref_update_reject_duplicates()` function has been marked as static, as its usage is now confined to 'refs.c'. Signed-off-by: Karthik Nayak --- refs.c | 9 +++++++-- refs/files-backend.c | 6 ------ refs/refs-internal.h | 8 -------- 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/refs.c b/refs.c index 4c9b706461977995be1d55e7667f7fb708fbbb76..b420a120102b3793168598b885bba68e4f5f5f03 100644 --- a/refs.c +++ b/refs.c @@ -2295,8 +2295,13 @@ int refs_update_symref_extended(struct ref_store *refs, const char *ref, return ret; } -int ref_update_reject_duplicates(struct string_list *refnames, - struct strbuf *err) +/* + * Write an error to `err` and return a nonzero value iff the same + * refname appears multiple times in `refnames`. `refnames` must be + * sorted on entry to this function. + */ +static int ref_update_reject_duplicates(struct string_list *refnames, + struct strbuf *err) { size_t i, n = refnames->nr; diff --git a/refs/files-backend.c b/refs/files-backend.c index 18da30c3f37dc5c09f7d81a9083d6b41d0463bd5..9fc5454678340dd7c72539bfa0f15ee7eb24b1ff 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2995,12 +2995,6 @@ static int files_transaction_finish_initial(struct files_ref_store *refs, if (transaction->state != REF_TRANSACTION_PREPARED) BUG("commit called for transaction that is not prepared"); - string_list_sort(&transaction->refnames); - if (ref_update_reject_duplicates(&transaction->refnames, err)) { - ret = TRANSACTION_GENERIC_ERROR; - goto cleanup; - } - /* * It's really undefined to call this function in an active * repository or when there are existing references: we are diff --git a/refs/refs-internal.h b/refs/refs-internal.h index f43766e1f00443eb689685cf4df0fa0b80018a03..434362b6099a35f92906a04ddd65365140147572 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -142,14 +142,6 @@ int refs_read_raw_ref(struct ref_store *ref_store, const char *refname, struct object_id *oid, struct strbuf *referent, unsigned int *type, int *failure_errno); -/* - * Write an error to `err` and return a nonzero value iff the same - * refname appears multiple times in `refnames`. `refnames` must be - * sorted on entry to this function. - */ -int ref_update_reject_duplicates(struct string_list *refnames, - struct strbuf *err); - /* * Add a ref_update with the specified properties to transaction, and * return a pointer to the new object. This function does not verify From patchwork Fri Feb 7 07:34:39 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Karthik Nayak X-Patchwork-Id: 13964461 Received: from mail-pl1-f176.google.com (mail-pl1-f176.google.com [209.85.214.176]) (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 0DE102343B3 for ; Fri, 7 Feb 2025 07:37:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.176 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738913835; cv=none; b=h6F5TJH3qIrsyg20Ou+05nl6p/AYz1zK9bXDRH5WRBw54huniUtdxFXnWDrWbp9bgaeRmryWadiVoaPCrm6N2HPJa7zXKGFfRGrSTS3Ia0YJ3Dh8z6oKwnw5k7QrPq0eXydLstm4MYLgQSYOdSzcW6KuImMaeJQm6Sn/l0uD72k= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738913835; c=relaxed/simple; bh=gmDuid9pZZ+vsVgXtewSbqqp3CYMorAr0xWDnqVtqHw=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=inQ6X1n0Oh6aDVUxblhrFJL70UDBPL5u0ijJI6zFT0P272QYdB1i8LEAQmIzBUJuM7CqC4iYW0TGag5E9qDYkbtKwVmtxBtCG8JV/kBga1eWYS3rL2lR86pwgC3J8y78UgJAM9Iv5NodtWkYk4yfgm2030nQZFgQ2PZpve1413Q= 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=TlsKu87M; arc=none smtp.client-ip=209.85.214.176 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="TlsKu87M" Received: by mail-pl1-f176.google.com with SMTP id d9443c01a7336-2166360285dso32001475ad.1 for ; Thu, 06 Feb 2025 23:37:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738913832; x=1739518632; darn=vger.kernel.org; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=Z9ptp5AUmnHfbqn1TSEj+23gY/vjbG1oL+MNziJusaY=; b=TlsKu87Md4Mm2Ef9peaWNncn282iuNBEvdV8S13PVmSsqH2RUi+MDHcdTFRFMWl7E/ YBztzERp9exHQx1ClOYwogI7DSmontvKfUdpZyFbl2vdqLku9sjlDi4prSAEJsE1SiI4 C2B2KtI/8MIyaOVT/S8nCZC8to9vmsofk7cdMkNqRl7rvXIE0tMe0B70xYt/mNOPl9IE Hn+volLPZ8RncJV38qCd6Gvhp2uOVmllMak0oN9IPlqtbbxwgI2x3eICBZ6sOI/mKYPd OYQ5neC4BEiwsq0NRi2IjdDF0NqenhY5/YK3OUN8XVNIv6bZIyUsi5/7I/SBILBvzaQi tqrQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738913832; x=1739518632; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Z9ptp5AUmnHfbqn1TSEj+23gY/vjbG1oL+MNziJusaY=; b=NA5yb6DCVA13bfwL/sJgoUYV6z02YPPPsjwWZLqtQdKryOcTACCqE/EYp6IHbieMc3 xTiA3z2FpNJEETCiX2KHPijcZLZ/lBPzzjxVYJTizPNaBVe5Ns1CxcIz5lCqK56zooZw KRA5LH7kwL7XEFB0S424SN1EJKqWcSo+uaaAgZ9Rlw/3S/PG42mdq2l5pBCPRc7mIoIr 6zvWdG8xq066UyfzZHx7TPWkPqmAm2hKcN+GoxjJDqdF/iJezBsT2UP0l7UKNeqXoU/D Pbrtgki+OK5JJhj5lzzMWF00ttTZJJxqB4rZCni19JsPVBRbB2Quctjza/dqybtCUEV4 YxUw== X-Gm-Message-State: AOJu0Yz+BskRhE8afojsqmGsiZsQiqsQpTcsEZASitLbt57ciIZvsZpS hi190RBdXb5aHJoV77cmXvmYrNA0uT3lfInZE4EaPCZ9uVfLkGc5OuokYLZw X-Gm-Gg: ASbGncuhs9vKowmLyMALQpHZPhivbMIwREPPYCjVQis74CvSvC4KpB12mn+8qHzXFVd KHvkez5mJ3Pxo+R9WjgGErikDA5jMwgHB1GdkBY1d41PnUTlM+XzfexPGcMLRBIV1nmGsrhFhoz efk9n9cdkADv4o9iUU5+nl/TOVFEBoIXg2Etx1FUhdf5KZL6PEH2JTyhXutg7wf3zJab9X0WWGq m1dcWp1OIzaLONj2d6lWLfs2y/DJM52WzTq2AfOPdTGFfXrTABjccpuMHuv+66mSLeK1R57SjLQ 3qiqz8m8aBlzdAGbcFoo7g== X-Google-Smtp-Source: AGHT+IGasue6tbgXGvrIufjPt7lVV8OUrfN+pwdaMAWtgDlGIHPie3J6LqGgmM5fz9Jmg0wEq0SGjA== X-Received: by 2002:a05:6a21:170f:b0:1ea:eca7:10ee with SMTP id adf61e73a8af0-1ee03a21d5cmr4542732637.6.1738913831799; Thu, 06 Feb 2025 23:37:11 -0800 (PST) Received: from [127.0.0.2] ([2401:4900:33b7:4cb0:4dd3:85f0:5c4b:b677]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-ad51aee79casm2115063a12.44.2025.02.06.23.37.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Feb 2025 23:37:11 -0800 (PST) From: Karthik Nayak Date: Fri, 07 Feb 2025 08:34:39 +0100 Subject: [PATCH 4/6] refs/reftable: extract code from the transaction preparation Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250207-245-partially-atomic-ref-updates-v1-4-e6a3690ff23a@gmail.com> References: <20250207-245-partially-atomic-ref-updates-v1-0-e6a3690ff23a@gmail.com> In-Reply-To: <20250207-245-partially-atomic-ref-updates-v1-0-e6a3690ff23a@gmail.com> To: git@vger.kernel.org Cc: ps@pks.im, jltobler@gmail.com, Karthik Nayak X-Mailer: b4 0.14.2 X-Developer-Signature: v=1; a=openpgp-sha256; l=17774; i=karthik.188@gmail.com; h=from:subject:message-id; bh=gmDuid9pZZ+vsVgXtewSbqqp3CYMorAr0xWDnqVtqHw=; b=owJ4nAHtARL+kA0DAAoBPtWfJI5GjH8ByyZiAGeluBtm6fk8Ey9Y7k94ZIglAEXruQIb1zzTj ykhppIxxHqYYIkBswQAAQoAHRYhBFfOTH9jdXEPy2XGBj7VnySORox/BQJnpbgbAAoJED7VnySO Rox/BKQL/0HB4HHGdLCFkl0AYTtJgQy2gAxRvInb/9EtpkLg8jHVI6RjKcq0kFJqz7HP47BCVOO RR8/wJOnrSQbp4YaJrlDGRuQfJFB8S+MRaZG4oWbz79BSzlLDVOQoOTbGOHIydc9sXWayUehwe9 7UINH4exfLIwrqMqvBJySb3qhWnBePr14gbo1vQBYyGAup92X3iUf6cv47dWDfIq43W9I7bKQJX rgRqxHhMCruOaKt7h6oC9OilJhSgdy3EVBGvht13JyElSeCw/0a/iT1q9esapXQwJ4K+wFDB3yh EfguD/syoeZkL/3w03PSPoTDp2ARE1ZeqIuOomdAniUkiNUR5GxA3bIgZfjxJwtwbpadNFlzaxx Jig6cXv2ILm3mK1Qx9/uBjGYlzd8a6iue1/KO262VjTe3coZMMEqLWpGFMGuJxhgjnjTD9Q16y9 Mq/yKU/26iWoMM2dhHNCZ2qg/KWIwpbh6+VPFDid7JhowLbInZGSsXArUeL9EbySUZ/IBhOjXa5 mQ= X-Developer-Key: i=karthik.188@gmail.com; a=openpgp; fpr=57CE4C7F6375710FCB65C6063ED59F248E468C7F Extract the core logic for preparing individual reference updates from `reftable_be_transaction_prepare()` into `prepare_single_update()`. This dedicated function now handles all validation and preparation steps for each reference update in the transaction, including object ID verification, HEAD reference handling, and symref processing. The refactoring consolidates all reference update validation into a single logical block, which improves code maintainability and readability. More importantly, this restructuring lays the groundwork for implementing partial transaction support in the reftable backend, which will be introduced in the following commit. No functional changes are included in this commit - it is purely a code reorganization to support future enhancements. Signed-off-by: Karthik Nayak --- refs/reftable-backend.c | 471 ++++++++++++++++++++++++------------------------ 1 file changed, 240 insertions(+), 231 deletions(-) diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index dd2099d94948a4f23fd9f7ddc06bf3d741229eba..5533acfaf9027765d5a270abfce96225e42cc823 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1061,6 +1061,242 @@ static int queue_transaction_update(struct reftable_ref_store *refs, return 0; } +static int prepare_single_update(struct ref_store *ref_store, + struct reftable_ref_store *refs, + struct reftable_transaction_data *tx_data, + struct ref_transaction *transaction, + struct reftable_backend *be, + struct ref_update *u, + unsigned int head_type, + struct strbuf *head_referent, + struct strbuf *referent, + struct strbuf *err) +{ + struct object_id current_oid = {0}; + const char *rewritten_ref; + int ret = 0; + + /* + * There is no need to reload the respective backends here as + * we have already reloaded them when preparing the transaction + * update. And given that the stacks have been locked there + * shouldn't have been any concurrent modifications of the + * stack. + */ + ret = backend_for(&be, refs, u->refname, &rewritten_ref, 0); + if (ret) + return ret; + + /* Verify that the new object ID is valid. */ + if ((u->flags & REF_HAVE_NEW) && !is_null_oid(&u->new_oid) && + !(u->flags & REF_SKIP_OID_VERIFICATION) && + !(u->flags & REF_LOG_ONLY)) { + struct object *o = parse_object(refs->base.repo, &u->new_oid); + if (!o) { + strbuf_addf(err, + _("trying to write ref '%s' with nonexistent object %s"), + u->refname, oid_to_hex(&u->new_oid)); + return -1; + } + + if (o->type != OBJ_COMMIT && is_branch(u->refname)) { + strbuf_addf(err, _("trying to write non-commit object %s to branch '%s'"), + oid_to_hex(&u->new_oid), u->refname); + return -1; + } + } + + /* + * When we update the reference that HEAD points to we enqueue + * a second log-only update for HEAD so that its reflog is + * updated accordingly. + */ + if (head_type == REF_ISSYMREF && + !(u->flags & REF_LOG_ONLY) && + !(u->flags & REF_UPDATE_VIA_HEAD) && + !strcmp(rewritten_ref, head_referent->buf)) { + /* + * First make sure that HEAD is not already in the + * transaction. This check is O(lg N) in the transaction + * size, but it happens at most once per transaction. + */ + if (string_list_has_string(&transaction->refnames, "HEAD")) { + /* An entry already existed */ + strbuf_addf(err, + _("multiple updates for 'HEAD' (including one " + "via its referent '%s') are not allowed"), + u->refname); + return TRANSACTION_NAME_CONFLICT; + } + + ref_transaction_add_update( + transaction, "HEAD", + u->flags | REF_LOG_ONLY | REF_NO_DEREF, + &u->new_oid, &u->old_oid, NULL, NULL, NULL, + u->msg); + } + + ret = reftable_backend_read_ref(be, rewritten_ref, + ¤t_oid, referent, &u->type); + if (ret < 0) + return ret; + if (ret > 0 && !ref_update_expects_existing_old_ref(u)) { + /* + * The reference does not exist, and we either have no + * old object ID or expect the reference to not exist. + * We can thus skip below safety checks as well as the + * symref splitting. But we do want to verify that + * there is no conflicting reference here so that we + * can output a proper error message instead of failing + * at a later point. + */ + ret = refs_verify_refname_available(ref_store, u->refname, + &transaction->refnames, NULL, + transaction->flags & REF_TRANSACTION_FLAG_INITIAL, + err); + if (ret < 0) + return ret; + + /* + * There is no need to write the reference deletion + * when the reference in question doesn't exist. + */ + if ((u->flags & REF_HAVE_NEW) && !ref_update_has_null_new_value(u)) { + ret = queue_transaction_update(refs, tx_data, u, + ¤t_oid, err); + if (ret) + return ret; + } + + return 0; + } + if (ret > 0) { + /* The reference does not exist, but we expected it to. */ + strbuf_addf(err, _("cannot lock ref '%s': " + "unable to resolve reference '%s'"), + ref_update_original_update_refname(u), u->refname); + return -1; + } + + if (u->type & REF_ISSYMREF) { + /* + * The reftable stack is locked at this point already, + * so it is safe to call `refs_resolve_ref_unsafe()` + * here without causing races. + */ + const char *resolved = refs_resolve_ref_unsafe(&refs->base, u->refname, 0, + ¤t_oid, NULL); + + if (u->flags & REF_NO_DEREF) { + if (u->flags & REF_HAVE_OLD && !resolved) { + strbuf_addf(err, _("cannot lock ref '%s': " + "error reading reference"), u->refname); + return -1; + } + } else { + struct ref_update *new_update; + int new_flags; + + new_flags = u->flags; + if (!strcmp(rewritten_ref, "HEAD")) + new_flags |= REF_UPDATE_VIA_HEAD; + + if (string_list_has_string(&transaction->refnames, referent->buf)) { + strbuf_addf(err, + _("multiple updates for '%s' (including one " + "via symref '%s') are not allowed"), + referent->buf, u->refname); + return TRANSACTION_NAME_CONFLICT; + } + + /* + * If we are updating a symref (eg. HEAD), we should also + * update the branch that the symref points to. + * + * This is generic functionality, and would be better + * done in refs.c, but the current implementation is + * intertwined with the locking in files-backend.c. + */ + new_update = ref_transaction_add_update( + transaction, referent->buf, new_flags, + u->new_target ? NULL : &u->new_oid, + u->old_target ? NULL : &u->old_oid, + u->new_target, u->old_target, + u->committer_info, u->msg); + + new_update->parent_update = u; + + /* + * Change the symbolic ref update to log only. Also, it + * doesn't need to check its old OID value, as that will be + * done when new_update is processed. + */ + u->flags |= REF_LOG_ONLY | REF_NO_DEREF; + u->flags &= ~REF_HAVE_OLD; + } + } + + /* + * Verify that the old object matches our expectations. Note + * that the error messages here do not make a lot of sense in + * the context of the reftable backend as we never lock + * individual refs. But the error messages match what the files + * backend returns, which keeps our tests happy. + */ + if (u->old_target) { + if (!(u->type & REF_ISSYMREF)) { + strbuf_addf(err, _("cannot lock ref '%s': " + "expected symref with target '%s': " + "but is a regular ref"), + ref_update_original_update_refname(u), + u->old_target); + return -1; + } + + if (ref_update_check_old_target(referent->buf, u, err)) { + return -1; + } + } else if ((u->flags & REF_HAVE_OLD) && !oideq(¤t_oid, &u->old_oid)) { + if (is_null_oid(&u->old_oid)) { + strbuf_addf(err, _("cannot lock ref '%s': " + "reference already exists"), + ref_update_original_update_refname(u)); + return TRANSACTION_CREATE_EXISTS; + } + else if (is_null_oid(¤t_oid)) + strbuf_addf(err, _("cannot lock ref '%s': " + "reference is missing but expected %s"), + ref_update_original_update_refname(u), + oid_to_hex(&u->old_oid)); + else + strbuf_addf(err, _("cannot lock ref '%s': " + "is at %s but expected %s"), + ref_update_original_update_refname(u), + oid_to_hex(¤t_oid), + oid_to_hex(&u->old_oid)); + return TRANSACTION_NAME_CONFLICT; + } + + /* + * If all of the following conditions are true: + * + * - We're not about to write a symref. + * - We're not about to write a log-only entry. + * - Old and new object ID are different. + * + * Then we're essentially doing a no-op update that can be + * skipped. This is not only for the sake of efficiency, but + * also skips writing unneeded reflog entries. + */ + if ((u->type & REF_ISSYMREF) || + (u->flags & REF_LOG_ONLY) || + (u->flags & REF_HAVE_NEW && !oideq(¤t_oid, &u->new_oid))) + return queue_transaction_update(refs, tx_data, u, + ¤t_oid, err); + + return 0; +} + static int reftable_be_transaction_prepare(struct ref_store *ref_store, struct ref_transaction *transaction, struct strbuf *err) @@ -1124,239 +1360,12 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, ret = 0; for (i = 0; i < transaction->nr; i++) { - struct ref_update *u = transaction->updates[i]; - struct object_id current_oid = {0}; - const char *rewritten_ref; - - /* - * There is no need to reload the respective backends here as - * we have already reloaded them when preparing the transaction - * update. And given that the stacks have been locked there - * shouldn't have been any concurrent modifications of the - * stack. - */ - ret = backend_for(&be, refs, u->refname, &rewritten_ref, 0); + ret = prepare_single_update(ref_store, refs, tx_data, + transaction, be, + transaction->updates[i], head_type, + &head_referent, &referent, err); if (ret) goto done; - - /* Verify that the new object ID is valid. */ - if ((u->flags & REF_HAVE_NEW) && !is_null_oid(&u->new_oid) && - !(u->flags & REF_SKIP_OID_VERIFICATION) && - !(u->flags & REF_LOG_ONLY)) { - struct object *o = parse_object(refs->base.repo, &u->new_oid); - if (!o) { - strbuf_addf(err, - _("trying to write ref '%s' with nonexistent object %s"), - u->refname, oid_to_hex(&u->new_oid)); - ret = -1; - goto done; - } - - if (o->type != OBJ_COMMIT && is_branch(u->refname)) { - strbuf_addf(err, _("trying to write non-commit object %s to branch '%s'"), - oid_to_hex(&u->new_oid), u->refname); - ret = -1; - goto done; - } - } - - /* - * When we update the reference that HEAD points to we enqueue - * a second log-only update for HEAD so that its reflog is - * updated accordingly. - */ - if (head_type == REF_ISSYMREF && - !(u->flags & REF_LOG_ONLY) && - !(u->flags & REF_UPDATE_VIA_HEAD) && - !strcmp(rewritten_ref, head_referent.buf)) { - /* - * First make sure that HEAD is not already in the - * transaction. This check is O(lg N) in the transaction - * size, but it happens at most once per transaction. - */ - if (string_list_has_string(&transaction->refnames, "HEAD")) { - /* An entry already existed */ - strbuf_addf(err, - _("multiple updates for 'HEAD' (including one " - "via its referent '%s') are not allowed"), - u->refname); - ret = TRANSACTION_NAME_CONFLICT; - goto done; - } - - ref_transaction_add_update( - transaction, "HEAD", - u->flags | REF_LOG_ONLY | REF_NO_DEREF, - &u->new_oid, &u->old_oid, NULL, NULL, NULL, - u->msg); - } - - ret = reftable_backend_read_ref(be, rewritten_ref, - ¤t_oid, &referent, &u->type); - if (ret < 0) - goto done; - if (ret > 0 && !ref_update_expects_existing_old_ref(u)) { - /* - * The reference does not exist, and we either have no - * old object ID or expect the reference to not exist. - * We can thus skip below safety checks as well as the - * symref splitting. But we do want to verify that - * there is no conflicting reference here so that we - * can output a proper error message instead of failing - * at a later point. - */ - ret = refs_verify_refname_available(ref_store, u->refname, - &transaction->refnames, NULL, - transaction->flags & REF_TRANSACTION_FLAG_INITIAL, - err); - if (ret < 0) - goto done; - - /* - * There is no need to write the reference deletion - * when the reference in question doesn't exist. - */ - if ((u->flags & REF_HAVE_NEW) && !ref_update_has_null_new_value(u)) { - ret = queue_transaction_update(refs, tx_data, u, - ¤t_oid, err); - if (ret) - goto done; - } - - continue; - } - if (ret > 0) { - /* The reference does not exist, but we expected it to. */ - strbuf_addf(err, _("cannot lock ref '%s': " - "unable to resolve reference '%s'"), - ref_update_original_update_refname(u), u->refname); - ret = -1; - goto done; - } - - if (u->type & REF_ISSYMREF) { - /* - * The reftable stack is locked at this point already, - * so it is safe to call `refs_resolve_ref_unsafe()` - * here without causing races. - */ - const char *resolved = refs_resolve_ref_unsafe(&refs->base, u->refname, 0, - ¤t_oid, NULL); - - if (u->flags & REF_NO_DEREF) { - if (u->flags & REF_HAVE_OLD && !resolved) { - strbuf_addf(err, _("cannot lock ref '%s': " - "error reading reference"), u->refname); - ret = -1; - goto done; - } - } else { - struct ref_update *new_update; - int new_flags; - - new_flags = u->flags; - if (!strcmp(rewritten_ref, "HEAD")) - new_flags |= REF_UPDATE_VIA_HEAD; - - if (string_list_has_string(&transaction->refnames, referent.buf)) { - strbuf_addf(err, - _("multiple updates for '%s' (including one " - "via symref '%s') are not allowed"), - referent.buf, u->refname); - ret = TRANSACTION_NAME_CONFLICT; - goto done; - } - - /* - * If we are updating a symref (eg. HEAD), we should also - * update the branch that the symref points to. - * - * This is generic functionality, and would be better - * done in refs.c, but the current implementation is - * intertwined with the locking in files-backend.c. - */ - new_update = ref_transaction_add_update( - transaction, referent.buf, new_flags, - u->new_target ? NULL : &u->new_oid, - u->old_target ? NULL : &u->old_oid, - u->new_target, u->old_target, - u->committer_info, u->msg); - - new_update->parent_update = u; - - /* - * Change the symbolic ref update to log only. Also, it - * doesn't need to check its old OID value, as that will be - * done when new_update is processed. - */ - u->flags |= REF_LOG_ONLY | REF_NO_DEREF; - u->flags &= ~REF_HAVE_OLD; - } - } - - /* - * Verify that the old object matches our expectations. Note - * that the error messages here do not make a lot of sense in - * the context of the reftable backend as we never lock - * individual refs. But the error messages match what the files - * backend returns, which keeps our tests happy. - */ - if (u->old_target) { - if (!(u->type & REF_ISSYMREF)) { - strbuf_addf(err, _("cannot lock ref '%s': " - "expected symref with target '%s': " - "but is a regular ref"), - ref_update_original_update_refname(u), - u->old_target); - ret = -1; - goto done; - } - - if (ref_update_check_old_target(referent.buf, u, err)) { - ret = -1; - goto done; - } - } else if ((u->flags & REF_HAVE_OLD) && !oideq(¤t_oid, &u->old_oid)) { - ret = TRANSACTION_NAME_CONFLICT; - if (is_null_oid(&u->old_oid)) { - strbuf_addf(err, _("cannot lock ref '%s': " - "reference already exists"), - ref_update_original_update_refname(u)); - ret = TRANSACTION_CREATE_EXISTS; - } - else if (is_null_oid(¤t_oid)) - strbuf_addf(err, _("cannot lock ref '%s': " - "reference is missing but expected %s"), - ref_update_original_update_refname(u), - oid_to_hex(&u->old_oid)); - else - strbuf_addf(err, _("cannot lock ref '%s': " - "is at %s but expected %s"), - ref_update_original_update_refname(u), - oid_to_hex(¤t_oid), - oid_to_hex(&u->old_oid)); - goto done; - } - - /* - * If all of the following conditions are true: - * - * - We're not about to write a symref. - * - We're not about to write a log-only entry. - * - Old and new object ID are different. - * - * Then we're essentially doing a no-op update that can be - * skipped. This is not only for the sake of efficiency, but - * also skips writing unneeded reflog entries. - */ - if ((u->type & REF_ISSYMREF) || - (u->flags & REF_LOG_ONLY) || - (u->flags & REF_HAVE_NEW && !oideq(¤t_oid, &u->new_oid))) { - ret = queue_transaction_update(refs, tx_data, u, - ¤t_oid, err); - if (ret) - goto done; - } } transaction->backend_data = tx_data; From patchwork Fri Feb 7 07:34:40 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Karthik Nayak X-Patchwork-Id: 13964462 Received: from mail-pl1-f178.google.com (mail-pl1-f178.google.com [209.85.214.178]) (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 E45C91A76DA for ; Fri, 7 Feb 2025 07:37:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.178 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738913837; cv=none; b=bErd420RMIDRYtR5NzD9212Ch7o15JYSSKEBBXWRWw4Upteb30SfsxqMcYXTocO4UbaYe34qN36wv1h+C3L+tF4g+mvTl6tHPkB5PaqOC4DR8tZQx1VT8nVsY6m0hCA3/coQ7leDCUyuyEoKREkKNOWSbB3e3tL97PdptC0ngRo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738913837; c=relaxed/simple; bh=8KO6JLLgnSt4/pEIUCAwehohejZEpruazEGuJAKD4cg=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=XR9audvZl1R+FYq1k43yF67ZzrHrMSfAqtiy23SOeVJ2wI7D9vHcCewrK6v2Qxk4O2K+iZpQpo0Io1OXnvTDQXyqw8g0qNF0yGOhU84Lnm11jqmLGKUST6m6284K4lZURzq9XInKCWJBcrxQsyPKFWUVqlplkbaXnlCQIOHsOj0= 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=CqGNaNWW; arc=none smtp.client-ip=209.85.214.178 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="CqGNaNWW" Received: by mail-pl1-f178.google.com with SMTP id d9443c01a7336-21634338cfdso43131015ad.2 for ; Thu, 06 Feb 2025 23:37:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738913835; x=1739518635; darn=vger.kernel.org; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=QYM49T1CDiI0W8ah27wSJFisbd8NjikRgzTr+rQqP3k=; b=CqGNaNWW6ZL71v11+saoZCbDvnmQHbk0myQSWdtc/jLxodpOXADPe8qv7BuRy3Ng+V Onu33HmTVuxKlqF1rZyAWlFhB7oZ1I6Ia+rCojiBMaTd9ApUPDO3SSBVTlstowfn3aYT PbrSni84xm21KOmdA0hvvIUo5fBt7L+9mgEoOcWAxJ/YPFCHHxXGNK1GSCym9Ao+Y0+e iQ/D6U1PhB/xsCe/itzj8Pmxj+NPZ7gB2Gxy3RfOAMsjCX3o9Vi0nZTiQEvJycxZ5WM5 BPZu8ckAkZLCaNFGn7yMRxjnk08Fos/SHkLxYjTzjFgNA+maKuX+Ll9H3nIo+WS3inyC Irog== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738913835; x=1739518635; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=QYM49T1CDiI0W8ah27wSJFisbd8NjikRgzTr+rQqP3k=; b=r//U/V5WOks3Pqu1s3kPBzzRK4enRjuLp9Di6eroYNr755tUYqR4iWAum4xCQ1g43A SSV+XtAl4SHsou4x3d7m2vsLQHBzdja1DcgKZfYUtPmoaXm1/zYkX/5Ei9wB7qh2ZqQE do4Kk+9VxVwZPydnCD3XVMHSwyEnGASq1ysH91ld10sFgpMAYeBCJGmaaPgsRJZin9Es MqVDj519PLcvHyghyAQpruYkWv0Z/RVnQIsc+XcuIYW8VJ5Nw4Plbd4C6aZ8aDnhmTpJ PrirHLhjyubG6Yd97nK0ZXolQugt4+w/CqDLLtF0HT/42+75CDrqmqMkTpvlvlTmU+SC zG2A== X-Gm-Message-State: AOJu0YyRy3BkhOuBBI6JzcPia5et7U4a0/qIDGolNjcD+CsMfa1egjJx WIDaT04iBRc3aMKoq+EN0Tx5U5+/30JO50w9+SwnwIm7jPpq5FlkGxjVoulZ X-Gm-Gg: ASbGncuWit/d/AjRsUe79x4kaPaV47J8UmjOduNab6XaAC31aCjVSvm/MVwT/JJQKMd 3zbu+RcQ3upqtyFN3o3cwfcLiFSiLqtYo/TCG5KQD/y74mnwP6HsWyIWD4YpxYhSIly4t+fyJSv z5EtjHZ9cgWiQEJ0A9t9dna8W1TsCS0/u3nzXUH9s1rxM1hDHayiAWv8052trGyYP4jROrbwvOI dB5yJ3uu+WJNvlhRQFKc74WfaAXXgsjSxQzYnMkSz+4bAAXFcguyQwl84AV7gSNhWLLVDpot7pd KSrngmO91AuAy2jVP9Bebw== X-Google-Smtp-Source: AGHT+IEuK7XpAF7G0BzB9zau/IVhn18iFUPQmu1QsGqct1unSf4/jeW3zxHSWtnTD2GouvE7HgdPJA== X-Received: by 2002:a05:6a00:807:b0:725:f282:1f04 with SMTP id d2e1a72fcca58-7305d4eff53mr4226404b3a.18.1738913834718; Thu, 06 Feb 2025 23:37:14 -0800 (PST) Received: from [127.0.0.2] ([2401:4900:33b7:4cb0:4dd3:85f0:5c4b:b677]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-ad51aee79casm2115063a12.44.2025.02.06.23.37.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Feb 2025 23:37:14 -0800 (PST) From: Karthik Nayak Date: Fri, 07 Feb 2025 08:34:40 +0100 Subject: [PATCH 5/6] refs: implement partial reference transaction support Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250207-245-partially-atomic-ref-updates-v1-5-e6a3690ff23a@gmail.com> References: <20250207-245-partially-atomic-ref-updates-v1-0-e6a3690ff23a@gmail.com> In-Reply-To: <20250207-245-partially-atomic-ref-updates-v1-0-e6a3690ff23a@gmail.com> To: git@vger.kernel.org Cc: ps@pks.im, jltobler@gmail.com, Karthik Nayak X-Mailer: b4 0.14.2 X-Developer-Signature: v=1; a=openpgp-sha256; l=10840; i=karthik.188@gmail.com; h=from:subject:message-id; bh=8KO6JLLgnSt4/pEIUCAwehohejZEpruazEGuJAKD4cg=; b=owJ4nAHtARL+kA0DAAoBPtWfJI5GjH8ByyZiAGeluBu7beVH660nOoWKktK4/05yoldUMiBuN EWYliKdJaWg+okBswQAAQoAHRYhBFfOTH9jdXEPy2XGBj7VnySORox/BQJnpbgbAAoJED7VnySO Rox/iQML/2MGog05l1TnSMHm5X5KYAWuP8Bvp7o3xbHbfvNtjcCz1SuwSdJ68DMvEJjVE/x1Uyr /8+cW8qaJPrO6mtivFEeAt+AzTmjWew2IRBlBzY88V8oPk9n37xAmOVrASVZ74CApp8M7t1qUgD WH/5oTd1EVQ90X01jaAfoQ+e8lXkGDBiEQ0YCfqfuPjlisnIpmxnZtYtpYZzcRAm7qiA2Mts1qy R3zPEjBQ1Xjs7KJXd3q/s6NGfEwhwGfX1RvYxFwsF525s+asXRifAdRYGU5mtSqDfcxaahwVRYU JYm2hhQqftKHlYB3H5lPASxmFTeIoMXzTr+8QmE3MXlEUujABESZVpiVC58ExK5HnNLLzSpBUJC 8mcSiRPzYV5p6geDa+e3g0633IF99XRoaq0SK+5q2xtPjmbcSl0qzYdynQXTyDkhIURo9/jGfc9 fkldFy2JBxYEHd/MbNGH4OP0WrCDeVx9yALjyZWOBsXJuTFhyjsJs3WybCJJgFGgpcD6AZ5X8l5 wQ= X-Developer-Key: i=karthik.188@gmail.com; a=openpgp; fpr=57CE4C7F6375710FCB65C6063ED59F248E468C7F Git's reference transactions are all-or-nothing: either all updates succeed, or none do. While this atomic behavior is generally desirable, it can be suboptimal when using the reftable backend, where batching multiple reference updates into a single transaction is more efficient than performing them sequentially. Introduce partial transaction support through a new flag `REF_TRANSACTION_ALLOW_PARTIAL`. When this flag is set, individual reference updates that would normally fail the entire transaction are instead marked as rejected while allowing other updates to proceed. This provides more flexibility while maintaining transactional integrity where needed. The implementation introduces several key components: - Add 'rejected' and 'rejection_err' fields to struct `ref_update` to track failed updates and their failure reasons. - Modify reference backends (files, packed, reftable) to handle partial transactions by using `ref_transaction_add_rejection()` instead of failing the entire transaction when `REF_TRANSACTION_ALLOW_PARTIAL` is set. - Add `ref_transaction_for_each_rejected_update()` to let callers examine which updates were rejected and why. This foundational change enables partial transaction support throughout the reference subsystem. The next commit will expose this capability to users by adding a `--allow-partial` flag to 'git-update-ref(1)', providing both a user-facing feature and a testable implementation. Signed-off-by: Karthik Nayak --- refs.c | 32 ++++++++++++++++++++++++++++++++ refs.h | 22 ++++++++++++++++++++++ refs/files-backend.c | 12 +++++++++++- refs/packed-backend.c | 26 ++++++++++++++++++++++++-- refs/refs-internal.h | 15 +++++++++++++++ refs/reftable-backend.c | 12 +++++++++++- 6 files changed, 115 insertions(+), 4 deletions(-) diff --git a/refs.c b/refs.c index b420a120102b3793168598b885bba68e4f5f5f03..75dbd84acbc41658d4b8b6b5e7763c04e78d0061 100644 --- a/refs.c +++ b/refs.c @@ -1204,6 +1204,7 @@ void ref_transaction_free(struct ref_transaction *transaction) free(transaction->updates[i]->committer_info); free((char *)transaction->updates[i]->new_target); free((char *)transaction->updates[i]->old_target); + strbuf_release(&transaction->updates[i]->rejection_err); free(transaction->updates[i]); } string_list_clear(&transaction->refnames, 0); @@ -1211,6 +1212,14 @@ void ref_transaction_free(struct ref_transaction *transaction) free(transaction); } +void ref_transaction_add_rejection(struct ref_transaction *transaction, + size_t update_idx, struct strbuf *err) +{ + struct ref_update *update = transaction->updates[update_idx]; + update->rejected = 1; + strbuf_addbuf(&update->rejection_err, err); +} + struct ref_update *ref_transaction_add_update( struct ref_transaction *transaction, const char *refname, unsigned int flags, @@ -1237,6 +1246,8 @@ struct ref_update *ref_transaction_add_update( update->flags = flags; + strbuf_init(&update->rejection_err, 0); + update->new_target = xstrdup_or_null(new_target); update->old_target = xstrdup_or_null(old_target); if ((flags & REF_HAVE_NEW) && new_oid) @@ -2676,6 +2687,27 @@ void ref_transaction_for_each_queued_update(struct ref_transaction *transaction, } } +void ref_transaction_for_each_rejected_update(struct ref_transaction *transaction, + ref_transaction_for_each_rejected_update_fn cb, + void *cb_data) +{ + if (!(transaction->flags & REF_TRANSACTION_ALLOW_PARTIAL)) + return; + + for (size_t i = 0; i < transaction->nr; i++) { + struct ref_update *update = transaction->updates[i]; + + if (!update->rejected) + continue; + + cb(update->refname, + (update->flags & REF_HAVE_OLD) ? &update->old_oid : NULL, + (update->flags & REF_HAVE_NEW) ? &update->new_oid : NULL, + update->old_target, update->new_target, + &update->rejection_err, cb_data); + } +} + int refs_delete_refs(struct ref_store *refs, const char *logmsg, struct string_list *refnames, unsigned int flags) { diff --git a/refs.h b/refs.h index a0cdd99250e8286b55808b697b0a94afac5d8319..a0f15fdea024527fcfdb478f78cbf6fd6568a25b 100644 --- a/refs.h +++ b/refs.h @@ -638,6 +638,13 @@ enum ref_transaction_flag { * either be absent or null_oid. */ REF_TRANSACTION_FLAG_INITIAL = (1 << 0), + + /* + * The transaction mechanism by default fails all updates if any conflict + * is detected. This flag allows transactions to partially apply updates + * while rejecting updates which do not match the expected state. + */ + REF_TRANSACTION_ALLOW_PARTIAL = (1 << 1), }; /* @@ -889,6 +896,21 @@ void ref_transaction_for_each_queued_update(struct ref_transaction *transaction, ref_transaction_for_each_queued_update_fn cb, void *cb_data); +/* + * Execute the given callback function for each of the reference updates which + * have been rejected in the given transaction. + */ +typedef void ref_transaction_for_each_rejected_update_fn(const char *refname, + const struct object_id *old_oid, + const struct object_id *new_oid, + const char *old_target, + const char *new_target, + const struct strbuf *reason, + void *cb_data); +void ref_transaction_for_each_rejected_update(struct ref_transaction *transaction, + ref_transaction_for_each_rejected_update_fn cb, + void *cb_data); + /* * Free `*transaction` and all associated data. */ diff --git a/refs/files-backend.c b/refs/files-backend.c index 9fc5454678340dd7c72539bfa0f15ee7eb24b1ff..99ec29164fbd30635125cc2325aab3d300cf906c 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2852,8 +2852,18 @@ static int files_transaction_prepare(struct ref_store *ref_store, ret = lock_ref_for_update(refs, update, transaction, head_ref, err); - if (ret) + if (ret) { + if (transaction->flags & REF_TRANSACTION_ALLOW_PARTIAL) { + ref_transaction_add_rejection(transaction, i, err); + + strbuf_setlen(err, 0); + ret = 0; + + continue; + } goto cleanup; + } + if (update->flags & REF_DELETING && !(update->flags & REF_LOG_ONLY) && diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 6e7acb077e81435715a1ca3cc928550147c8c56a..cb9b6f0a620eaa59941f6fbc653600304f2bae8c 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1313,9 +1313,10 @@ static int packed_ref_store_remove_on_disk(struct ref_store *ref_store, * remain locked when it is done. */ static int write_with_updates(struct packed_ref_store *refs, - struct string_list *updates, + struct ref_transaction *transaction, struct strbuf *err) { + struct string_list *updates = &transaction->refnames; struct ref_iterator *iter = NULL; size_t i; int ok; @@ -1393,6 +1394,13 @@ static int write_with_updates(struct packed_ref_store *refs, strbuf_addf(err, "cannot update ref '%s': " "reference already exists", update->refname); + + if (transaction->flags & REF_TRANSACTION_ALLOW_PARTIAL) { + ref_transaction_add_rejection(transaction, i, err); + strbuf_setlen(err, 0); + continue; + } + goto error; } else if (!oideq(&update->old_oid, iter->oid)) { strbuf_addf(err, "cannot update ref '%s': " @@ -1400,6 +1408,13 @@ static int write_with_updates(struct packed_ref_store *refs, update->refname, oid_to_hex(iter->oid), oid_to_hex(&update->old_oid)); + + if (transaction->flags & REF_TRANSACTION_ALLOW_PARTIAL) { + ref_transaction_add_rejection(transaction, i, err); + strbuf_setlen(err, 0); + continue; + } + goto error; } } @@ -1434,6 +1449,13 @@ static int write_with_updates(struct packed_ref_store *refs, "reference is missing but expected %s", update->refname, oid_to_hex(&update->old_oid)); + + if (transaction->flags & REF_TRANSACTION_ALLOW_PARTIAL) { + ref_transaction_add_rejection(transaction, i, err); + strbuf_setlen(err, 0); + continue; + } + goto error; } } @@ -1657,7 +1679,7 @@ static int packed_transaction_prepare(struct ref_store *ref_store, data->own_lock = 1; } - if (write_with_updates(refs, &transaction->refnames, err)) + if (write_with_updates(refs, transaction, err)) goto failure; transaction->state = REF_TRANSACTION_PREPARED; diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 434362b6099a35f92906a04ddd65365140147572..6b8f5b2bd83baa22480083e1002daba9300f1b70 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -3,6 +3,7 @@ #include "refs.h" #include "iterator.h" +#include "strbuf.h" #include "string-list.h" struct fsck_options; @@ -123,6 +124,13 @@ struct ref_update { */ unsigned int index; + /* + * Used in partial transactions to mark a given update as rejected, + * with rejection reason. + */ + unsigned int rejected; + struct strbuf rejection_err; + /* * If this ref_update was split off of a symref update via * split_symref_update(), then this member points at that @@ -142,6 +150,13 @@ int refs_read_raw_ref(struct ref_store *ref_store, const char *refname, struct object_id *oid, struct strbuf *referent, unsigned int *type, int *failure_errno); +/* + * Mark a given update as rejected with a given reason. To be used in conjuction + * with the `REF_TRANSACTION_ALLOW_PARTIAL` flag to allow partial transactions. + */ +void ref_transaction_add_rejection(struct ref_transaction *transaction, + size_t update_idx, struct strbuf *err); + /* * Add a ref_update with the specified properties to transaction, and * return a pointer to the new object. This function does not verify diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 5533acfaf9027765d5a270abfce96225e42cc823..a2d86d1c5098b30bd212fc12a3708d2c0a60c677 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1364,8 +1364,18 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, transaction, be, transaction->updates[i], head_type, &head_referent, &referent, err); - if (ret) + + if (ret) { + if (transaction->flags & REF_TRANSACTION_ALLOW_PARTIAL) { + ref_transaction_add_rejection(transaction, i, err); + + strbuf_setlen(err, 0); + ret = 0; + + continue; + } goto done; + } } transaction->backend_data = tx_data; From patchwork Fri Feb 7 07:34:41 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Karthik Nayak X-Patchwork-Id: 13964463 Received: from mail-pl1-f177.google.com (mail-pl1-f177.google.com [209.85.214.177]) (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 5B5462343BD for ; Fri, 7 Feb 2025 07:37:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.177 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738913840; cv=none; b=lsBGOuxDRFgS12UeDHwuevT8KNlD7g9zbzvfCBWTITwF9OQAMj6RVt1AZoz9gpsH72fGm6VRF7fq3qJZSk5+4s+V1lAvnmZ480czOJDDDS7TFyjJzJZaCJ8//fSzI5GBiQMdajw8MukjHDJ1dQGb1JGcTKEPIkLLRu8RPpIDlbI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738913840; c=relaxed/simple; bh=Rsph4tSVjiPiv8HHMM8Oyy3vTEC8cV89ownDrzSQkrE=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=R4P2km3gpeGK6Rv2YymnuuNSTSx9kYG8iEhHXrJCB1ZpIGWnsmyzY8nyly3loKTPCCOu0klDwdSdIRMZNHqU7bKxNnJOE8q+NOxNIulZ2hHLhMByyGaBkmb8yjgFpk5p6Ryq/mp3FPZSwIQw8bZSxmZeTTVrTkek9wlcIJj/vNQ= 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=mBY32bhq; arc=none smtp.client-ip=209.85.214.177 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="mBY32bhq" Received: by mail-pl1-f177.google.com with SMTP id d9443c01a7336-2166360285dso32002465ad.1 for ; Thu, 06 Feb 2025 23:37:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738913837; x=1739518637; darn=vger.kernel.org; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=C+lPadWeLrvjhfYUEVhJHPQqcpr8sjrf+fJ2NVgwUAU=; b=mBY32bhqh2cRTMbj5Kgm81vv/JdU7mQWBJVoJJFrAQnkS+fiF2nldE+tX2Tu7UBD4t 0zvN95xlrVk0gpYsBmvYwc3GmscYLUVKIkNQcCxImp/JyiCA4i+0MLI3ufVMJkfb7Ilg 9BI7outI7D4yyCNLJRKpYbOna0LEg0g5D89H0di0K5XrzIytFclSksxHQ3LpS47kmhlr 5Z/M1aAD38QHDYj+DTYACMFwAXUKug7VU8OVRVsJAIuTCHBWJYxoeIN+iJJQF7JOe2hZ 4tAQMZc2NbM6rrReUyku+heaw39nf1TIj0Upkz+Qa9Ietr5urFvcKGUN8y3fLFK3+yrF 9Sfw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738913837; x=1739518637; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=C+lPadWeLrvjhfYUEVhJHPQqcpr8sjrf+fJ2NVgwUAU=; b=WllkfNS8ysOPCvxMpWFqgQXcEHPhGdoDeZqTyijBCkl6Jp/FNrV8SzH3DRT3MhIgP6 nQ2c7g4Ny9UcasFoaU3UyeG7AlIJA44RDgRGJim3Xf5XJm3Y8/2MVN6bqp4GFZ8xgo4i 1ORHPd9QVBDlbpaCLMf7Tje+IrTCG6/Ng8nApvRkmaHTLixDbq0+D5p09zftejGH8dL4 UmKCdfWFf6MipqqMp3c385h0LlGHx+EgsKow/qbJnUJNrRwP60TKrjVF5PFIcUQPq5BK kljGv1J2mIdjOcjb7jT82qwTen9EXjXIkoRgpGBRJADDOEG8La6t1lWbuzEmEnA8qRt5 rM6g== X-Gm-Message-State: AOJu0YxKZPdRQKN1yAFQ0B2JwEc1v40pTNlgsvPXPJ6/e70a7hIsARYf fiSQ+E4zCSptMSxOFWXg0y9U2JiGoLo/eEFd9U6MuUbL/08kH+kkXVT/GY6V X-Gm-Gg: ASbGncvcroJFNL/0JvCxhpiREMugFCAsTeXY09IFbw4licBnQR8jGOzjGp3BeWOm6uF mW2a+MTNDrEEhdxa+LR217DNZqs5SDnRFeyn/tO3QPcE3w87cWveMBw+6SkLukbkkN9XzLTDhMk dF8TwXoPHW0cleFR9kivHPG553xYoCkOyJzWt9WsuZTQCNx+Zf4A19ZjdGChrHRmFE+vKFEFUJx pOuDtReUiSzemmp27R2QY0tUfoihMZaYqIMHMvENwjeHr0a0+pkMLHi2anvu/i699fadx78foj8 hbZl+6KnMABdebgc0BhgQQ== X-Google-Smtp-Source: AGHT+IEzXBpPGfMOzuSoh1vAKKYkqgQXvt2jLjKH3C5bQ7fGi/EA3ElGMkHueKWTJWHnDS7lx6KFKg== X-Received: by 2002:a05:6a21:2d05:b0:1e1:bf3d:a190 with SMTP id adf61e73a8af0-1ee03b0c3c9mr5481533637.30.1738913837222; Thu, 06 Feb 2025 23:37:17 -0800 (PST) Received: from [127.0.0.2] ([2401:4900:33b7:4cb0:4dd3:85f0:5c4b:b677]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-ad51aee79casm2115063a12.44.2025.02.06.23.37.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Feb 2025 23:37:16 -0800 (PST) From: Karthik Nayak Date: Fri, 07 Feb 2025 08:34:41 +0100 Subject: [PATCH 6/6] update-ref: add --allow-partial flag for stdin mode Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250207-245-partially-atomic-ref-updates-v1-6-e6a3690ff23a@gmail.com> References: <20250207-245-partially-atomic-ref-updates-v1-0-e6a3690ff23a@gmail.com> In-Reply-To: <20250207-245-partially-atomic-ref-updates-v1-0-e6a3690ff23a@gmail.com> To: git@vger.kernel.org Cc: ps@pks.im, jltobler@gmail.com, Karthik Nayak X-Mailer: b4 0.14.2 X-Developer-Signature: v=1; a=openpgp-sha256; l=15257; i=karthik.188@gmail.com; h=from:subject:message-id; bh=Rsph4tSVjiPiv8HHMM8Oyy3vTEC8cV89ownDrzSQkrE=; b=owJ4nAHtARL+kA0DAAoBPtWfJI5GjH8ByyZiAGeluBvV+lXY4+Gs4b8NKMGRx6IOP2CpMpo4N 0kS8U4T4aJYLYkBswQAAQoAHRYhBFfOTH9jdXEPy2XGBj7VnySORox/BQJnpbgbAAoJED7VnySO Rox/+YIL/3y417+f2BKwjPygNU8aTuGrRMjv4pS9YlHqfqLeWmgXweuzZfbh+giQiCUn+JTrcUw RFTBdjrioKaQ/yICGLC1cYSMDGk92n4AI9YNJCaB0tRiXgOOVEu8Z6tL7WttRvMPn8BIi/Qd6ZT gV2trJC3uJNlGMZC9GjGZKwQEaCy3gelDr5n4mzX1bIv2+b8/B1lCdrbdDDeiyCuEXy0o2d8O7S sfNN2fUbcR6TRv7ngK4kTjsT6u1Ue+rGUhE8CR5tO6f8sP3DzKZljAp8ZwVl2YZdDFOotUtwTod +RRtuKRMYjzncOyXYBZlSD6MLFLi/b2+T+npFHRlmVxk7mK9ycS6XxEXF+NYrsvwXyg4zP0pLJA vpyxuwaaw5wZx0yqQkXRzGmY/BJFW+cnRvJoF7hO0DkZu3sHHuaMyr8hYIOQjNyPqV/ukZci0vq 9FE78tlYaE40JZ9pV2urjYHcIhh0WoUmrlP2BE7jbl1pgOwAT3PB++HoJm1V0XY3ZXezdsjcei6 6I= X-Developer-Key: i=karthik.188@gmail.com; a=openpgp; fpr=57CE4C7F6375710FCB65C6063ED59F248E468C7F When updating multiple references through stdin, Git's update-ref command normally aborts the entire transaction if any single update fails. While this atomic behavior prevents partial updates by default, there are cases where applying successful updates while reporting failures is desirable. Add a new `--allow-partial` flag that allows the transaction to continue even when individual reference updates fail. This flag can only be used in `--stdin` mode and builds upon the partial transaction support added to the refs subsystem. When enabled, failed updates are reported in the following format: rejected SP ( | ) SP ( | ) SP LF or with `-z`: rejected NUL ( | ) NUL ( | ) NUL NUL Update the documentation to reflect this change and also tests to cover different scenarios where an update could be rejected. Signed-off-by: Karthik Nayak --- Documentation/git-update-ref.txt | 12 ++- builtin/update-ref.c | 53 +++++++++-- t/t1400-update-ref.sh | 191 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 248 insertions(+), 8 deletions(-) diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt index 9e6935d38d031b4890135e0cce36fffcc349ac1d..529d3c15404cdc13216219fba6f56dde91f4909c 100644 --- a/Documentation/git-update-ref.txt +++ b/Documentation/git-update-ref.txt @@ -8,7 +8,7 @@ git-update-ref - Update the object name stored in a ref safely SYNOPSIS -------- [verse] -'git update-ref' [-m ] [--no-deref] (-d [] | [--create-reflog] [] | --stdin [-z]) +'git update-ref' [-m ] [--no-deref] (-d [] | [--create-reflog] [] | --stdin [-z] [--allow-partial]) DESCRIPTION ----------- @@ -57,6 +57,12 @@ performs all modifications together. Specify commands of the form: With `--create-reflog`, update-ref will create a reflog for each ref even if one would not ordinarily be created. +With `--allow-partial`, update-ref will process the transaction even if +some of the updates fail, allowing remaining updates to be applied. +Failed updates will be printed in the following format: + + rejected SP ( | ) SP ( | ) SP LF + Quote fields containing whitespace as if they were strings in C source code; i.e., surrounded by double-quotes and with backslash escapes. Use 40 "0" characters or the empty string to specify a zero value. To @@ -82,6 +88,10 @@ quoting: In this format, use 40 "0" to specify a zero value, and use the empty string to specify a missing value. +With `-z`, `--allow-partial` will print rejections in the following form: + + rejected NUL ( | ) NUL ( | ) NUL NUL + In either format, values can be specified in any form that Git recognizes as an object name. Commands in any other format or a repeated produce an error. Command meanings are: diff --git a/builtin/update-ref.c b/builtin/update-ref.c index 4d35bdc4b4b57937112e6c4c9740420b1f1771e5..83dcb7d8d73f423226c36b61374c86c6b29ec756 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -5,6 +5,7 @@ #include "config.h" #include "gettext.h" #include "hash.h" +#include "hex.h" #include "refs.h" #include "object-name.h" #include "parse-options.h" @@ -13,7 +14,7 @@ static const char * const git_update_ref_usage[] = { N_("git update-ref [] -d []"), N_("git update-ref [] []"), - N_("git update-ref [] --stdin [-z]"), + N_("git update-ref [] --stdin [-z] [--allow-partial]"), NULL }; @@ -562,6 +563,30 @@ static void parse_cmd_abort(struct ref_transaction *transaction, report_ok("abort"); } +static void print_rejected_refs(const char *refname, + const struct object_id *old_oid, + const struct object_id *new_oid, + const char *old_target, + const char *new_target, + const struct strbuf *reason, + void *cb_data UNUSED) +{ + struct strbuf sb = STRBUF_INIT; + char space = ' '; + + if (!line_termination) + space = line_termination; + + strbuf_addf(&sb, "rejected%c%s%c%s%c%c%s%c%s%c", space, + refname, space, new_oid ? oid_to_hex(new_oid) : new_target, + space, space, old_oid ? oid_to_hex(old_oid) : old_target, + space, reason->buf, line_termination); + + fwrite(sb.buf, sb.len, 1, stdout); + strbuf_release(&sb); + fflush(stdout); +} + static void parse_cmd_commit(struct ref_transaction *transaction, const char *next, const char *end UNUSED) { @@ -570,6 +595,10 @@ static void parse_cmd_commit(struct ref_transaction *transaction, die("commit: extra input: %s", next); if (ref_transaction_commit(transaction, &error)) die("commit: %s", error.buf); + + ref_transaction_for_each_rejected_update(transaction, + print_rejected_refs, NULL); + report_ok("commit"); ref_transaction_free(transaction); } @@ -606,7 +635,7 @@ static const struct parse_cmd { { "commit", parse_cmd_commit, 0, UPDATE_REFS_CLOSED }, }; -static void update_refs_stdin(void) +static void update_refs_stdin(unsigned int flags) { struct strbuf input = STRBUF_INIT, err = STRBUF_INIT; enum update_refs_state state = UPDATE_REFS_OPEN; @@ -614,7 +643,7 @@ static void update_refs_stdin(void) int i, j; transaction = ref_store_transaction_begin(get_main_ref_store(the_repository), - 0, &err); + flags, &err); if (!transaction) die("%s", err.buf); @@ -682,7 +711,7 @@ static void update_refs_stdin(void) */ state = cmd->state; transaction = ref_store_transaction_begin(get_main_ref_store(the_repository), - 0, &err); + flags, &err); if (!transaction) die("%s", err.buf); @@ -698,6 +727,8 @@ static void update_refs_stdin(void) /* Commit by default if no transaction was requested. */ if (ref_transaction_commit(transaction, &err)) die("%s", err.buf); + ref_transaction_for_each_rejected_update(transaction, + print_rejected_refs, NULL); ref_transaction_free(transaction); break; case UPDATE_REFS_STARTED: @@ -723,7 +754,8 @@ int cmd_update_ref(int argc, const char *refname, *oldval; struct object_id oid, oldoid; int delete = 0, no_deref = 0, read_stdin = 0, end_null = 0; - int create_reflog = 0; + int create_reflog = 0, allow_partial = 0; + struct option options[] = { OPT_STRING( 'm', NULL, &msg, N_("reason"), N_("reason of the update")), OPT_BOOL('d', NULL, &delete, N_("delete the reference")), @@ -732,6 +764,7 @@ int cmd_update_ref(int argc, OPT_BOOL('z', NULL, &end_null, N_("stdin has NUL-terminated arguments")), OPT_BOOL( 0 , "stdin", &read_stdin, N_("read updates from stdin")), OPT_BOOL( 0 , "create-reflog", &create_reflog, N_("create a reflog")), + OPT_BOOL('0', "allow-partial", &allow_partial, N_("allow partial transactions")), OPT_END(), }; @@ -749,13 +782,19 @@ int cmd_update_ref(int argc, } if (read_stdin) { + unsigned int flags = 0; + + if (allow_partial) + flags |= REF_TRANSACTION_ALLOW_PARTIAL; + if (delete || argc > 0) usage_with_options(git_update_ref_usage, options); if (end_null) line_termination = '\0'; - update_refs_stdin(); + update_refs_stdin(flags); return 0; - } + } else if (allow_partial) + die("--allow-partial can only be used with --stdin"); if (end_null) usage_with_options(git_update_ref_usage, options); diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index 29045aad43906fce3f64fb82ee98fb5f80d4796b..4f02f1974de4164442507a2eaec258edf6574f1f 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -2066,6 +2066,197 @@ do grep "$(git rev-parse $a) $(git rev-parse $a)" actual ' + test_expect_success "stdin $type allow-partial" ' + git init repo && + test_when_finished "rm -fr repo" && + ( + cd repo && + test_commit commit && + head=$(git rev-parse HEAD) && + + format_command $type "update refs/heads/ref1" "$head" "$Z" >stdin && + format_command $type "update refs/heads/ref2" "$head" "$Z" >>stdin && + git update-ref $type --stdin --allow-partial expect && + git rev-parse refs/heads/ref1 >actual && + test_cmp expect actual && + git rev-parse refs/heads/ref2 >actual && + test_cmp expect actual + ) + ' + + test_expect_success "stdin $type allow-partial with invalid new_oid" ' + git init repo && + test_when_finished "rm -fr repo" && + ( + cd repo && + test_commit one && + old_head=$(git rev-parse HEAD) && + test_commit two && + head=$(git rev-parse HEAD) && + git update-ref refs/heads/ref1 $head && + git update-ref refs/heads/ref2 $head && + + format_command $type "update refs/heads/ref1" "$old_head" "$head" >stdin && + format_command $type "update refs/heads/ref2" "$(test_oid 001)" "$head" >>stdin && + git update-ref $type --stdin --allow-partial stdout && + echo $old_head >expect && + git rev-parse refs/heads/ref1 >actual && + test_cmp expect actual && + echo $head >expect && + git rev-parse refs/heads/ref2 >actual && + test_cmp expect actual && + test_grep -q "trying to write ref ${SQ}refs/heads/ref2${SQ} with nonexistent object" stdout + ) + ' + + test_expect_success "stdin $type allow-partial with non-commit new_oid" ' + git init repo && + test_when_finished "rm -fr repo" && + ( + cd repo && + test_commit one && + old_head=$(git rev-parse HEAD) && + test_commit two && + head=$(git rev-parse HEAD) && + head_tree=$(git rev-parse HEAD^{tree}) && + git update-ref refs/heads/ref1 $head && + git update-ref refs/heads/ref2 $head && + + format_command $type "update refs/heads/ref1" "$old_head" "$head" >stdin && + format_command $type "update refs/heads/ref2" "$head_tree" "$head" >>stdin && + git update-ref $type --stdin --allow-partial stdout && + echo $old_head >expect && + git rev-parse refs/heads/ref1 >actual && + test_cmp expect actual && + echo $head >expect && + git rev-parse refs/heads/ref2 >actual && + test_cmp expect actual && + test_grep -q "trying to write non-commit object $head_tree to branch ${SQ}refs/heads/ref2${SQ}" stdout + ) + ' + + test_expect_success "stdin $type allow-partial with non-existent ref" ' + git init repo && + test_when_finished "rm -fr repo" && + ( + cd repo && + test_commit one && + old_head=$(git rev-parse HEAD) && + test_commit two && + head=$(git rev-parse HEAD) && + git update-ref refs/heads/ref1 $head && + + format_command $type "update refs/heads/ref1" "$old_head" "$head" >stdin && + format_command $type "update refs/heads/ref2" "$old_head" "$head" >>stdin && + git update-ref $type --stdin --allow-partial stdout && + echo $old_head >expect && + git rev-parse refs/heads/ref1 >actual && + test_cmp expect actual && + test_must_fail git rev-parse refs/heads/ref2 && + test_grep -q "unable to resolve reference" stdout + ) + ' + + test_expect_success "stdin $type allow-partial with dangling symref" ' + git init repo && + test_when_finished "rm -fr repo" && + ( + cd repo && + test_commit one && + old_head=$(git rev-parse HEAD) && + test_commit two && + head=$(git rev-parse HEAD) && + git update-ref refs/heads/ref1 $head && + git symbolic-ref refs/heads/ref2 refs/heads/nonexistent && + + format_command $type "update refs/heads/ref1" "$old_head" "$head" >stdin && + format_command $type "update refs/heads/ref2" "$old_head" "$head" >>stdin && + git update-ref $type --no-deref --stdin --allow-partial stdout && + echo $old_head >expect && + git rev-parse refs/heads/ref1 >actual && + test_cmp expect actual && + echo $head >expect && + test_must_fail git rev-parse refs/heads/ref2 && + test_grep -q "reference is missing but expected $head" stdout + ) + ' + + test_expect_success "stdin $type allow-partial with regular ref as symref" ' + git init repo && + test_when_finished "rm -fr repo" && + ( + cd repo && + test_commit one && + old_head=$(git rev-parse HEAD) && + test_commit two && + head=$(git rev-parse HEAD) && + git update-ref refs/heads/ref1 $head && + git update-ref refs/heads/ref2 $head && + + format_command $type "update refs/heads/ref1" "$old_head" "$head" >stdin && + format_command $type "symref-update refs/heads/ref2" "$old_head" "ref" "refs/heads/nonexistent" >>stdin && + git update-ref $type --no-deref --stdin --allow-partial stdout && + echo $old_head >expect && + git rev-parse refs/heads/ref1 >actual && + test_cmp expect actual && + echo $head >expect && + echo $head >expect && + git rev-parse refs/heads/ref2 >actual && + test_cmp expect actual && + test_grep -q "expected symref with target ${SQ}refs/heads/nonexistent${SQ}: but is a regular ref" stdout + ) + ' + + test_expect_success "stdin $type allow-partial with invalid old_oid" ' + git init repo && + test_when_finished "rm -fr repo" && + ( + cd repo && + test_commit one && + old_head=$(git rev-parse HEAD) && + test_commit two && + head=$(git rev-parse HEAD) && + git update-ref refs/heads/ref1 $head && + git update-ref refs/heads/ref2 $head && + + format_command $type "update refs/heads/ref1" "$old_head" "$head" >stdin && + format_command $type "update refs/heads/ref2" "$old_head" "$Z" >>stdin && + git update-ref $type --stdin --allow-partial stdout && + echo $old_head >expect && + git rev-parse refs/heads/ref1 >actual && + test_cmp expect actual && + echo $head >expect && + git rev-parse refs/heads/ref2 >actual && + test_cmp expect actual && + test_grep -q "reference already exists" stdout + ) + ' + + test_expect_success "stdin $type allow-partial with incorrect old oid" ' + git init repo && + test_when_finished "rm -fr repo" && + ( + cd repo && + test_commit one && + old_head=$(git rev-parse HEAD) && + test_commit two && + head=$(git rev-parse HEAD) && + git update-ref refs/heads/ref1 $head && + git update-ref refs/heads/ref2 $head && + + format_command $type "update refs/heads/ref1" "$old_head" "$head" >stdin && + format_command $type "update refs/heads/ref2" "$head" "$old_head" >>stdin && + git update-ref $type --stdin --allow-partial stdout && + echo $old_head >expect && + git rev-parse refs/heads/ref1 >actual && + test_cmp expect actual && + echo $head >expect && + git rev-parse refs/heads/ref2 >actual && + test_cmp expect actual && + test_grep -q "${SQ}refs/heads/ref2${SQ}: is at $head but expected $old_head" stdout + ) + ' done test_expect_success 'update-ref should also create reflog for HEAD' '