From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: "SZEDER Gábor" <szeder.dev@gmail.com>,
git@vger.kernel.org, "Derrick Stolee" <derrickstolee@github.com>
Subject: Re: [PATCH 2/6] t3404-rebase-interactive: mark a test with REFFILES prereq
Date: Wed, 05 Oct 2022 09:45:43 -0700 [thread overview]
Message-ID: <xmqqwn9eqkw8.fsf@gitster.g> (raw)
In-Reply-To: <220930.86ill4vman.gmgdl@evledraar.gmail.com> ("Ævar Arnfjörð Bjarmason"'s message of "Fri, 30 Sep 2022 18:46:49 +0200")
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> On Fri, Sep 30 2022, SZEDER Gábor wrote:
>
>> The test '--update-refs: check failed ref update' added in b3b1a21d1a
>> (sequencer: rewrite update-refs as user edits todo list, 2022-07-19)
>> directly modifies the contents of a ref file, so mark this test with
>> the REFFILES prereq.
>> ...
> # At this point, the values of first, second, and third are
> # recorded in the update-refs file. We will force-update the
> # "second" ref, but "git branch -f" will not work because of
> - # the lock in the update-refs file.
> - git rev-parse third >.git/refs/heads/second &&
> + # the lock in the update-refs file, so we need to use
> + # "update-ref".
> + git update-ref refs/heads/second third &&
>
> test_must_fail git rebase --continue 2>err &&
> grep "update_ref failed for ref '\''refs/heads/second'\''" err &&
>
> As the comment notes if you try that with "git branch" you'll get an
> error, even with --force, but update-ref works just fine...
Ah, I had exactly the same thought but was stopped from sending the
suggestion to use "git update-ref" right away by "because of the
lock in the update-refs file" that did not immediately click. What
is happening is "git branch" and any Porcelain commands that call
the now slightly misnamed branch_checked_out() function to see if an
operation is allowed to touch a branch would refrain from touching
this ref, but "update-ref" as a plumbing is deliberately designed to
be usable to bypass the check [*]. The --update-refs series added
code to branch_checked_out() to consider any refs being rewritten
as "checked out hence no touch".
Even though being listed in the update-refs file may count as a
moral equivalent to having a "lock in the update-refs file", because
write_update_refs_state() mentions no "lock", the proposed log
message was confusing and I think that was why it did not
"immediately click" at least for me.
If this works with the update-ref plumbing, we should do so instead
of adding REFFILES prerequisite.
[Footnote]
* Allowing more flexibility to the plumbing is OK, but those
scripting around plumbing commands should get the same support as
those writing built-in and making internal calls to functions
like branch_checked_out(), in order for them to be able to write
their Porcelain and offer the same safety to their users. The
function certainly should be exposed to plumbing users, and
possibly many other internal-only features.
The trend in the past ten or so years has been "Porcelain first"
and then "Porcelain only", which sadly made the composability of
Git plumbing commands much less useful than pre-made Porcelain
commands.
next prev parent reply other threads:[~2022-10-05 16:46 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-30 14:09 [PATCH 0/6] rebase --update-refs: smooth out some rough edges SZEDER Gábor
2022-09-30 14:09 ` [PATCH 1/6] t2407-worktree-heads.sh: remove outdated loop SZEDER Gábor
2022-09-30 14:09 ` [PATCH 2/6] t3404-rebase-interactive: mark a test with REFFILES prereq SZEDER Gábor
2022-09-30 16:46 ` Ævar Arnfjörð Bjarmason
2022-10-05 16:45 ` Junio C Hamano [this message]
2022-09-30 14:09 ` [PATCH 3/6] rebase -i: emphasize that 'update-ref' expects a fully-qualified ref SZEDER Gábor
2022-09-30 14:09 ` [PATCH 4/6] sequencer: avoid empty lines after 'update-ref' instructions SZEDER Gábor
2022-09-30 17:18 ` Derrick Stolee
2022-10-05 16:49 ` Junio C Hamano
2022-09-30 14:09 ` [PATCH 5/6] sequencer: duplicate the result of resolve_ref_unsafe() SZEDER Gábor
2022-09-30 16:45 ` Ævar Arnfjörð Bjarmason
2022-09-30 16:51 ` Ævar Arnfjörð Bjarmason
2022-09-30 17:23 ` Derrick Stolee
2022-10-09 17:23 ` SZEDER Gábor
2022-09-30 14:09 ` [PATCH 6/6] sequencer: fail early if invalid ref is given to 'update-ref' instruction SZEDER Gábor
2022-09-30 17:09 ` Ævar Arnfjörð Bjarmason
2022-09-30 17:29 ` [PATCH 0/6] rebase --update-refs: smooth out some rough edges Derrick Stolee
2022-10-01 16:31 ` SZEDER Gábor
2022-10-01 16:53 ` Junio C Hamano
2022-10-05 17:14 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqwn9eqkw8.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=avarab@gmail.com \
--cc=derrickstolee@github.com \
--cc=git@vger.kernel.org \
--cc=szeder.dev@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).