The ref transaction can fail after the message has been written using the
editor. The ref transaction is attempted after the message file (`TAG_EDITMSG`)
has been unlinked, so there is no backup tag message file to retry the
command.[1]
This is unfortunate if someone has written more than e.g. “v1.99.4” in the
editor. (I don’t know if people write long tag messages in practice.)
Hold on to the tag message file until after the ref transaction in order
to preserve the backup.
† 1: On commit 91428f078b (The eighteenth batch, 2023-05-10)
§ Reproduction script
```
cd /tmp
dir=$(mktemp -d)
cd $dir
git init
git commit --allow-empty -mInit
git tag release/v1
# Fails
git tag -a release
```
Error message:
```
fatal: cannot lock ref 'refs/tags/release': 'refs/tags/release/v1' exists; cannot create 'refs/tags/release'
```
Better error message and behavior:
```
The tag message has been left in .git/TAG_EDITMSG
fatal: cannot lock ref 'refs/tags/release': 'refs/tags/release/v1' exists; cannot create 'refs/tags/release'
```
§ Alternatives considered
My first thought was to find a way to “dry run” the ref update before opening
the editor (the edge case of the ref update command succeeding the first time
but not the second *real* time seemed incredibly unlikely to happen by
happenstance, so I saw no reason to consider that). However that seemed like it
would involve more code and conditionals, and I don’t know if the dry-run mode
is even supported.
A benefit of this alternative approach would be to error out immediately instead
of opening the editor. But trying to create a tag which collides with an
existing “namespace” seems very unlikely to happen in practice.[2] Losing a file
is much worse than being inconvenienced to retry the command, so I decided to
just focus on the former problem.
Most importantly though this approach was within my ability to implement.
† 2: Just observe my “Reproduction script”: one tries to create `release` after
someone else made `release/v1`. But what is just “release”? What follows
(next version) that? But why am I arguing against my change…
§ CI
https://github.com/LemmingAvalanche/git/actions/runs/4972149825
§ Carbon copy
The suggested contacts seemed too long:
```
$ ./contrib/contacts/git-contacts v2.40.1..
Junio C Hamano <>
René Scharfe <>
Jonathan Tan <>
Jonathan Nieder <>
Jeff King <>
Stefan Beller <>
Denton Liu <>
Robert Dailey <>
```
So I added only Jeff King based on commit 3927bbe9a4 (tag: delete TAG_EDITMSG
only on successful tag, 2008-12-06).
§ Patches
1. Test the happy case
2. Test the unhappy case (fails without the next patch)
3. The change
—
Cheers.
Kristoffer Haugsbakk (3):
t/t7004-tag: add regression test for existing behavior
t/t7004-tag: add failing tag message file test
tag: keep the message file in case ref transaction fails
builtin/tag.c | 24 +++++++++++++++---------
t/t7004-tag.sh | 19 +++++++++++++++++++
2 files changed, 34 insertions(+), 9 deletions(-)
--
2.40.1
