Subject: Re: [PATCH 2/6] t3404-rebase-interactive: mark a test with REFFILES prereq

From: Junio C Hamano <[email-masked]>
Date: Wed, 05 Oct 2022 09:45:43 -0700

Ævar Arnfjörð Bjarmason <[email-masked]> 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.