The ref transaction can fail after the user has written their tag
message. In particular, if there exists a tag `foo/bar` and `git tag -a
foo` is said then the command will only fail once it tries to write
`refs/tags/foo`, which is after the file has been unlinked.
Hold on to the message file for a little longer so that it is not
unlinked before the fatal error occurs.
Cc: Jeff King <peff@xxxxxxxx>
Signed-off-by: Kristoffer Haugsbakk <code@xxxxxxxxxxxxxxx>
---
Notes (series):
I duplicated this message (this isn’t obvious in the diff):
fprintf(stderr,
_("The tag message has been left in %s\n"),
path);
Should this be factored into a static function instead?
§ Changes from previous round
Squash (combine) the update to `tag.c` with the test so that the fix and
the regression test is added in one step.
This makes more sense than what I was going for since the test suite
would fail on patch 2/3 of the previous round.
Link: https://lore.kernel.org/git/xmqq4joeaxgw.fsf@gitster.g/T/#u
builtin/tag.c | 24 +++++++++++++++---------
t/t7004-tag.sh | 10 ++++++++++
2 files changed, 25 insertions(+), 9 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index d428c45dc8..7df9c143ac 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -266,11 +266,10 @@ static const char message_advice_nested_tag[] =
static void create_tag(const struct object_id *object, const char *object_ref,
const char *tag,
struct strbuf *buf, struct create_tag_options *opt,
- struct object_id *prev, struct object_id *result)
+ struct object_id *prev, struct object_id *result, char *path)
{
enum object_type type;
struct strbuf header = STRBUF_INIT;
- char *path = NULL;
type = oid_object_info(the_repository, object, NULL);
if (type <= OBJ_NONE)
@@ -294,7 +293,6 @@ static void create_tag(const struct object_id *object, const char *object_ref,
int fd;
/* write the template message before editing: */
- path = git_pathdup("TAG_EDITMSG");
fd = xopen(path, O_CREAT | O_TRUNC | O_WRONLY, 0600);
if (opt->message_given) {
@@ -336,10 +334,6 @@ static void create_tag(const struct object_id *object, const char *object_ref,
path);
exit(128);
}
- if (path) {
- unlink_or_warn(path);
- free(path);
- }
}
static void create_reflog_msg(const struct object_id *oid, struct strbuf *sb)
@@ -487,6 +481,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
};
int ret = 0;
const char *only_in_list = NULL;
+ char *path = NULL;
setup_ref_filter_porcelain_msg();
@@ -621,7 +616,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (create_tag_object) {
if (force_sign_annotate && !annotate)
opt.sign = 1;
- create_tag(&object, object_ref, tag, &buf, &opt, &prev, &object);
+ path = git_pathdup("TAG_EDITMSG");
+ create_tag(&object, object_ref, tag, &buf, &opt, &prev, &object,
+ path);
}
transaction = ref_transaction_begin(&err);
@@ -629,8 +626,17 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
ref_transaction_update(transaction, ref.buf, &object, &prev,
create_reflog ? REF_FORCE_CREATE_REFLOG : 0,
reflog_msg.buf, &err) ||
- ref_transaction_commit(transaction, &err))
+ ref_transaction_commit(transaction, &err)) {
+ if (path)
+ fprintf(stderr,
+ _("The tag message has been left in %s\n"),
+ path);
die("%s", err.buf);
+ }
+ if (path) {
+ unlink_or_warn(path);
+ free(path);
+ }
ref_transaction_free(transaction);
if (force && !is_null_oid(&prev) && !oideq(&prev, &object))
printf(_("Updated tag '%s' (was %s)\n"), tag,
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index cd454acfed..37bfa63fe8 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -2136,4 +2136,14 @@ test_expect_success 'If tag is created then tag message file is unlinked' '
! test_path_exists .git/TAG_EDITMSG
'
+test_expect_success 'If tag cannot be created then tag message file is not unlinked' '
+ test_when_finished "git tag -d foo/bar && rm .git/TAG_EDITMSG" &&
+ write_script fakeeditor <<-\EOF &&
+ echo Message >.git/TAG_EDITMSG
+ EOF
+ git tag foo/bar &&
+ test_must_fail env GIT_EDITOR=./fakeeditor git tag -a foo &&
+ test_path_exists .git/TAG_EDITMSG
+'
+
test_done
--
2.40.1
