From patchwork Wed Jun 5 10:29:54 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Karthik Nayak X-Patchwork-Id: 13686573 Received: from mail-ej1-f48.google.com (mail-ej1-f48.google.com [209.85.218.48]) (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 CD0FD19148D for ; Wed, 5 Jun 2024 10:30:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.48 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717583408; cv=none; b=O5coYQ/iRGuM71PDfaupcppZVT52nXbzEDuGGaU6QbEbOxBIlgsBSetK4FT6YjC9XIaMk1vG4Jc+MRthp+dkKwN26W6aMzGu6PnjyT7JDbC0qpZg3IMC3D4a25dib6zG2fi/LOA7SY2CK1hZeA6hPudAhN4tvoo98t+SyRpnm14= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717583408; c=relaxed/simple; bh=DulOYP/O2rDjF+Kq+d881KvIYFo+QkDB93eOxpKGvSM=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=ABV4qFFuOUrrAqaSX7ORW0VNijtMsa9rwaQ76J5JY2cUfdZUwVtX+MwLdbUozbypEQFeWi8fqnQWWonV7Kjnfo89PL05oeDic+PwOBh67hzZwP5yK3bXZb2Rx2rIG4pnkUoiVF5VVnr7dympHvgZAg5y5J7YcHi6Jd/tCqDYXhU= 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=N3zbuPgb; arc=none smtp.client-ip=209.85.218.48 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="N3zbuPgb" Received: by mail-ej1-f48.google.com with SMTP id a640c23a62f3a-a68ee841138so412050966b.1 for ; Wed, 05 Jun 2024 03:30:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1717583405; x=1718188205; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=rJvj8oLK2adoo0Kr1iYI2blFmO78bImN1wU6pM5d93A=; b=N3zbuPgbEZjGMfUAJk+eHBXrdhfxJWYcEJ/PSz08AbGhN7tMCf0UT0/goLRhaDeWOd 2mHAj0HenYFjM5mducZ9aCTLArfWAMI4aMy6vmdhWCkuaaOA+rGTNirMgz3TCPWI+99r PafXub6DH2f3UF/YHXLfyEby+gH7vXJtAhBxBj+BxQrwT0E6YY+Y03wuEN9cQtqkMmhM phuZJ77EUrQcX6oQcXkHgHkielCOC+imgRfqC0pjzpKd9BohxnI5uMBNEbEJRwkRu+W6 QXBxN3D1sQbwXpxceNqTLZ1L08kcN2SVUeq5aFbAEMfsKvE8fG/jiOdei+4hDGTw4ZZ1 H0hA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717583405; x=1718188205; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=rJvj8oLK2adoo0Kr1iYI2blFmO78bImN1wU6pM5d93A=; b=wq5nWpPc4E8V9aalUcdat0bpnunIhRzb/GU+UCjth8S1CsOzKNHNnEh5sYz4HR8aRf 0fBKSo+cABXE+A690X8x5hpFW2P+wUCbZbIU3YSm9nNI5Bmse/GpMXhiuW+QbPltzW8f C/58W0utZo1V082V1EHQX4YGQokdT0cluVyrL1rk8w2dpgsmBNcFremDasyzE0Tlh24V 0NMnq3pgRkuZpUzRg9886MKeyDrJdvDdxj8zJqvjtunL8aWDJ8EH3jRXzNa45gaClq0y y1WWY7A5xzAiEWfZ9mNNeNW6++2HLs7KnzsoFlk1xwWTOURSzCoUxi+X5IQjvUAzCIVD rXMQ== X-Gm-Message-State: AOJu0YxYSzq4c5o2virKLftnCr6SGl/ARhxLhzvW+NYS3OLTrfEvwT6m PoJqXJ2/4ND0cwVhsrquiIfXd2E72IvkJOpOqM7uSI/uvOUJD4WPpe1PqGV9 X-Google-Smtp-Source: AGHT+IGPX9LqlaDJRhwL9gIvwh/zMCtJ2OlN2EVwW8hmlYGKX/h7vuJANRpV1kZFPlrRcZPsOWGj1Q== X-Received: by 2002:a17:906:594e:b0:a69:2011:7119 with SMTP id a640c23a62f3a-a699fac5565mr120906966b.41.1717583404698; Wed, 05 Jun 2024 03:30:04 -0700 (PDT) Received: from localhost.localdomain ([185.223.147.210]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a68f0e03cfdsm510453166b.195.2024.06.05.03.30.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 Jun 2024 03:30:04 -0700 (PDT) From: Karthik Nayak X-Google-Original-From: Karthik Nayak To: karthik.188@gmail.com Cc: git@vger.kernel.org, gitster@pobox.com, ps@pks.im Subject: [PATCH v4 3/7] update-ref: add support for 'symref-verify' command Date: Wed, 5 Jun 2024 12:29:54 +0200 Message-ID: <20240605102958.716432-4-knayak@gitlab.com> X-Mailer: git-send-email 2.44.1 In-Reply-To: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Karthik Nayak The 'symref-verify' command allows users to verify if a provided contains the provided without changing the . If is not provided, the command will verify that the doesn't exist. The command allows users to verify symbolic refs within a transaction, and this means users can perform a set of changes in a transaction only when the verification holds good. Since we're checking for symbolic refs, this command will only work with the 'no-deref' mode. This is because any dereferenced symbolic ref will point to an object and not a ref and the regular 'verify' command can be used in such situations. Add required tests for symref support in 'verify'. Since we're here, also add reflog checks for the pre-existing 'verify' tests, there is no divergence from behavior, but we never tested to ensure that reflog wasn't affected by the 'verify' command. Helped-by: Patrick Steinhardt Signed-off-by: Karthik Nayak --- Documentation/git-update-ref.txt | 7 +++ builtin/update-ref.c | 80 +++++++++++++++++++++++---- refs.c | 11 +++- refs.h | 1 + t/t1400-update-ref.sh | 94 +++++++++++++++++++++++++++++++- t/t1416-ref-transaction-hooks.sh | 30 ++++++++++ 6 files changed, 208 insertions(+), 15 deletions(-) diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt index 374a2ebd2b..9fe78b3501 100644 --- a/Documentation/git-update-ref.txt +++ b/Documentation/git-update-ref.txt @@ -65,6 +65,7 @@ performs all modifications together. Specify commands of the form: create SP SP LF delete SP [SP ] LF verify SP [SP ] LF + symref-verify SP [SP ] LF option SP LF start LF prepare LF @@ -86,6 +87,7 @@ quoting: create SP NUL NUL delete SP NUL [] NUL verify SP NUL [] NUL + symref-verify SP [NUL ] NUL option SP NUL start NUL prepare NUL @@ -117,6 +119,11 @@ verify:: Verify against but do not change it. If is zero or missing, the ref must not exist. +symref-verify:: + Verify symbolic against but do not change it. + If is missing, the ref must not exist. Can only be + used in `no-deref` mode. + option:: Modify the behavior of the next command naming a . The only valid option is `no-deref` to avoid dereferencing diff --git a/builtin/update-ref.c b/builtin/update-ref.c index 6cda1c08aa..50f5472160 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -76,6 +76,29 @@ static char *parse_refname(const char **next) return strbuf_detach(&ref, NULL); } +/* + * Wrapper around parse_refname which skips the next delimiter. + */ +static char *parse_next_refname(const char **next) +{ + if (line_termination) { + /* Without -z, consume SP and use next argument */ + if (!**next || **next == line_termination) + return NULL; + if (**next != ' ') + die("expected SP but got: %s", *next); + } else { + /* With -z, read the next NUL-terminated line */ + if (**next) + return NULL; + } + /* Skip the delimiter */ + (*next)++; + + return parse_refname(next); +} + + /* * The value being parsed is (as opposed to ; the * difference affects which error messages are generated): @@ -297,11 +320,47 @@ static void parse_cmd_verify(struct ref_transaction *transaction, die("verify %s: extra input: %s", refname, next); if (ref_transaction_verify(transaction, refname, &old_oid, - update_flags, &err)) + NULL, update_flags, &err)) + die("%s", err.buf); + + update_flags = default_flags; + free(refname); + strbuf_release(&err); +} + +static void parse_cmd_symref_verify(struct ref_transaction *transaction, + const char *next, const char *end) +{ + struct strbuf err = STRBUF_INIT; + struct object_id old_oid; + char *refname, *old_target; + + if (!(update_flags & REF_NO_DEREF)) + die("symref-verify: cannot operate with deref mode"); + + refname = parse_refname(&next); + if (!refname) + die("symref-verify: missing "); + + /* + * old_ref is optional, if not provided, we need to ensure that the + * ref doesn't exist. + */ + old_target = parse_next_refname(&next); + if (!old_target) + oidcpy(&old_oid, null_oid()); + + if (*next != line_termination) + die("symref-verify %s: extra input: %s", refname, next); + + if (ref_transaction_verify(transaction, refname, + old_target ? NULL : &old_oid, + old_target, update_flags, &err)) die("%s", err.buf); update_flags = default_flags; free(refname); + free(old_target); strbuf_release(&err); } @@ -380,15 +439,16 @@ static const struct parse_cmd { unsigned args; enum update_refs_state state; } command[] = { - { "update", parse_cmd_update, 3, UPDATE_REFS_OPEN }, - { "create", parse_cmd_create, 2, UPDATE_REFS_OPEN }, - { "delete", parse_cmd_delete, 2, UPDATE_REFS_OPEN }, - { "verify", parse_cmd_verify, 2, UPDATE_REFS_OPEN }, - { "option", parse_cmd_option, 1, UPDATE_REFS_OPEN }, - { "start", parse_cmd_start, 0, UPDATE_REFS_STARTED }, - { "prepare", parse_cmd_prepare, 0, UPDATE_REFS_PREPARED }, - { "abort", parse_cmd_abort, 0, UPDATE_REFS_CLOSED }, - { "commit", parse_cmd_commit, 0, UPDATE_REFS_CLOSED }, + { "update", parse_cmd_update, 3, UPDATE_REFS_OPEN }, + { "create", parse_cmd_create, 2, UPDATE_REFS_OPEN }, + { "delete", parse_cmd_delete, 2, UPDATE_REFS_OPEN }, + { "verify", parse_cmd_verify, 2, UPDATE_REFS_OPEN }, + { "symref-verify", parse_cmd_symref_verify, 2, UPDATE_REFS_OPEN }, + { "option", parse_cmd_option, 1, UPDATE_REFS_OPEN }, + { "start", parse_cmd_start, 0, UPDATE_REFS_STARTED }, + { "prepare", parse_cmd_prepare, 0, UPDATE_REFS_PREPARED }, + { "abort", parse_cmd_abort, 0, UPDATE_REFS_CLOSED }, + { "commit", parse_cmd_commit, 0, UPDATE_REFS_CLOSED }, }; static void update_refs_stdin(void) diff --git a/refs.c b/refs.c index 50d8d7d777..cdc4d25557 100644 --- a/refs.c +++ b/refs.c @@ -1297,14 +1297,19 @@ int ref_transaction_delete(struct ref_transaction *transaction, int ref_transaction_verify(struct ref_transaction *transaction, const char *refname, const struct object_id *old_oid, + const char *old_target, unsigned int flags, struct strbuf *err) { - if (!old_oid) - BUG("verify called with old_oid set to NULL"); + if (!old_target && !old_oid) + BUG("verify called with old_oid and old_target set to NULL"); + if (old_oid && old_target) + BUG("verify called with both old_oid and old_target set"); + if (old_target && !(flags & REF_NO_DEREF)) + BUG("verify cannot operate on symrefs with deref mode"); return ref_transaction_update(transaction, refname, NULL, old_oid, - NULL, NULL, + NULL, old_target, flags, NULL, err); } diff --git a/refs.h b/refs.h index 34568ee1fb..906299351c 100644 --- a/refs.h +++ b/refs.h @@ -736,6 +736,7 @@ int ref_transaction_delete(struct ref_transaction *transaction, int ref_transaction_verify(struct ref_transaction *transaction, const char *refname, const struct object_id *old_oid, + const char *old_target, unsigned int flags, struct strbuf *err); diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index bbee2783ab..52801be07d 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -892,17 +892,23 @@ test_expect_success 'stdin update/create/verify combination works' ' ' test_expect_success 'stdin verify succeeds for correct value' ' + test-tool ref-store main for-each-reflog-ent $m >before && git rev-parse $m >expect && echo "verify $m $m" >stdin && git update-ref --stdin actual && - test_cmp expect actual + test_cmp expect actual && + test-tool ref-store main for-each-reflog-ent $m >after && + test_cmp before after ' test_expect_success 'stdin verify succeeds for missing reference' ' + test-tool ref-store main for-each-reflog-ent $m >before && echo "verify refs/heads/missing $Z" >stdin && git update-ref --stdin after && + test_cmp before after ' test_expect_success 'stdin verify treats no value as missing' ' @@ -1643,4 +1649,88 @@ test_expect_success PIPE 'transaction flushes status updates' ' test_cmp expected actual ' +format_command () { + if test "$1" = "-z" + then + shift + printf "$F" "$@" + else + echo "$@" + fi +} + +for type in "" "-z" +do + + test_expect_success "stdin $type symref-verify fails without --no-deref" ' + git symbolic-ref refs/heads/symref $a && + format_command $type "symref-verify refs/heads/symref" "$a" >stdin && + test_must_fail git update-ref --stdin $type err && + grep "fatal: symref-verify: cannot operate with deref mode" err + ' + + test_expect_success "stdin $type symref-verify fails with too many arguments" ' + format_command $type "symref-verify refs/heads/symref" "$a" "$a" >stdin && + test_must_fail git update-ref --stdin $type --no-deref err && + if test "$type" = "-z" + then + grep "fatal: unknown command: $a" err + else + grep "fatal: symref-verify refs/heads/symref: extra input: $a" err + fi + ' + + test_expect_success "stdin $type symref-verify succeeds for correct value" ' + git symbolic-ref refs/heads/symref >expect && + test-tool ref-store main for-each-reflog-ent refs/heads/symref >before && + format_command $type "symref-verify refs/heads/symref" "$a" >stdin && + git update-ref --stdin $type --no-deref actual && + test_cmp expect actual && + test-tool ref-store main for-each-reflog-ent refs/heads/symref >after && + test_cmp before after + ' + + test_expect_success "stdin $type symref-verify fails with no value" ' + git symbolic-ref refs/heads/symref >expect && + format_command $type "symref-verify refs/heads/symref" "" >stdin && + test_must_fail git update-ref --stdin $type --no-deref stdin && + git update-ref --stdin $type --no-deref before && + format_command $type "symref-verify refs/heads/missing" "refs/heads/unknown" >stdin && + test_must_fail git update-ref --stdin $type --no-deref err && + grep "fatal: cannot lock ref ${SQ}refs/heads/missing${SQ}: unable to resolve reference ${SQ}refs/heads/missing${SQ}" err && + test_must_fail git rev-parse --verify -q refs/heads/missing && + test-tool ref-store main for-each-reflog-ent refs/heads/symref >after && + test_cmp before after + ' + + test_expect_success "stdin $type symref-verify fails for wrong value" ' + git symbolic-ref refs/heads/symref >expect && + format_command $type "symref-verify refs/heads/symref" "$b" >stdin && + test_must_fail git update-ref --stdin $type --no-deref actual && + test_cmp expect actual + ' + + test_expect_success "stdin $type symref-verify fails for mistaken null value" ' + git symbolic-ref refs/heads/symref >expect && + format_command $type "symref-verify refs/heads/symref" "$Z" >stdin && + test_must_fail git update-ref --stdin $type --no-deref actual && + test_cmp expect actual + ' + +done + test_done diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh index 067fd57290..fd58b902f4 100755 --- a/t/t1416-ref-transaction-hooks.sh +++ b/t/t1416-ref-transaction-hooks.sh @@ -157,4 +157,34 @@ test_expect_success 'hook captures git-symbolic-ref updates' ' test_cmp expect actual ' +test_expect_success 'hook gets all queued symref updates' ' + test_when_finished "rm actual" && + + git update-ref refs/heads/branch $POST_OID && + git symbolic-ref refs/heads/symref refs/heads/main && + + test_hook reference-transaction <<-\EOF && + echo "$*" >>actual + while read -r line + do + printf "%s\n" "$line" + done >>actual + EOF + + cat >expect <<-EOF && + prepared + ref:refs/heads/main $ZERO_OID refs/heads/symref + committed + ref:refs/heads/main $ZERO_OID refs/heads/symref + EOF + + git update-ref --no-deref --stdin <<-EOF && + start + symref-verify refs/heads/symref refs/heads/main + prepare + commit + EOF + test_cmp expect actual +' + test_done