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

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2054 bytes --]

On Thu, Jan 11, 2024 at 02:13:29AM -0500, Jeff King wrote:
> On Wed, Jan 10, 2024 at 06:52:29PM +0000, Justin Tobler via GitGitGadget wrote:
> 
> > 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).

If all we care for is the exit code then this would work for the
reftable backend, too:

```
$ git init --ref-format=reftable repo
Initialized empty Git repository in /tmp/repo/.git/
$ cd repo/
$ git commit --allow-empty --message message
[main (root-commit) c2512d3] x
$ git symbolic-ref refs/heads refs/heads/foo
$ echo $?
1
```

A bit unfortunate that there is no proper error message in that case,
but that is a different topic.

Patrick

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]