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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 5, 2022, 16:45 UTC
Message-ID: <xmqqwn9eqkw8.fsf@gitster.g>
In-Reply-To: <220930.86ill4vman.gmgdl@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 21 quoted lines

> 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.