Re: [PATCH 1/2] t1401: generalize reference locking

From: Jeff King <peff@peff.net>
Date: Jan 11, 2024, 07:13 UTC
Message-ID: <20240111071329.GC48154@coredump.intra.peff.net>
In-Reply-To: <cb78b549e5e826ffef39c55bd726164e6b7bb755.1704912750.git.gitgitgadget@gmail.com>

On Wed, Jan 10, 2024 at 06:52:29PM +0000, Justin Tobler via GitGitGadget wrote:

Show 7 quoted lines

> From: Justin Tobler <jltobler@gmail.com>
> 
> Some tests set up reference locks by directly creating the lockfile.
> While this works for the files reference backend, reftable reference
> locks operate differently and are incompatible with this approach.
> Refactor the test to use git-update-ref(1) to lock refs instead so that
> the test does not need to be aware of how the ref backend locks refs.

It looks like you re-create this situation in a backend-agnostic way by having two simultaneous updates that conflict on the lock (but don't care how that lock is implemented).

That works, but I think we could keep it simple. This test doesn't care about the exact error condition we create. The point was just to die in create_symref() and make sure the exit code was propagated. So something like this would work:

  $ git symbolic-ref refs/heads refs/heads/foo
  error: unable to write symref for refs/heads: Is a directory

(note that you get a different error message if the refs are packed, since there we can notice the d/f conflict manually).

There may be other ways to stimulate a failure. I thought "symbolic-ref HEAD refs/heads/.invalid" might work, but sadly the refname format check happens earlier.

I think it is worth avoiding the fifo magic if we can. It's complicated, and it means that not all platforms run the test.

-Peff