From: Jeff King <peff@peff.net>
To: Kristoffer Haugsbakk <code@khaugsbakk.name>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH 0/3] tag: keep the message file in case ref transaction fails
Date: Wed, 17 May 2023 05:32:14 -0400 [thread overview]
Message-ID: <20230517093214.GA527234@coredump.intra.peff.net> (raw)
In-Reply-To: <cover.1684067644.git.code@khaugsbakk.name>
On Sun, May 14, 2023 at 03:17:57PM +0200, Kristoffer Haugsbakk wrote:
> 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.)
> [...]
>
> So I added only Jeff King based on commit 3927bbe9a4 (tag: delete TAG_EDITMSG
> only on successful tag, 2008-12-06).
It has definitely been a while since that one. :) I agree what you are
doing here is in the same spirit as that commit, though there is one
small difference. 3927bbe9a4 was about saving the message file if we
failed to create the object, since otherwise there is no record of it.
Your patch is about saving it from a failed ref update. But in that case
we _do_ have the data saved in the object. You can find it with
git-fsck:
[after running something like your test script]
$ git fsck
Checking object directories: 100% (256/256), done.
dangling tag b9bde516952a863690027a611575a79d4c786b8d
$ git cat-file tag b9bde516952a863690027a611575a79d4c786b8d
object 8b1e708e32ae3af17825b3c713a3ab317824b932
type commit
tag foo/bar
tagger Jeff King <peff@peff.net> 1684314980 -0400
my message
That said, I'm willing to believe that most users wouldn't figure this
out on their own, and saving TAG_EDITMSG might be more friendly. But one
other alternative might be to mention the hash of that tag object, and
possibly give advice on recovering it.
It's too bad we do not have "git tag -c" to match "git commit -c", which
would allow us to say something like:
A tag object was created, but we failed to update the ref.
After correcting any errors, you can recover the original tag
message with:
git tag -c $oid [other options]
(where we'd replace $oid with the hash of the created tag object). The
best alternatives I could come up with were:
# this is kind of obscure advice to give somebody, plus
# it makes a weird tag if you originally tried to tag "foo/bar"
# but then later switch to some other name. The "tag" field
# in the object won't match the ref.
git update-ref $ref $oid
# this saves just the message but is horribly complicated
git cat-file tag $oid | sed '1,/^$/d' |
git tag -F -
I dunno. There is a certain elegance to telling the user about what
progress we _did_ make, but if there isn't an easy way to turn that into
a retry of their command, it may not be all that useful.
So I'm not really opposed to your patch, but mostly just brainstorming
alternatives.
I gave v3 a brief look, and it seems OK to me. It might be nice to
factor out the duplicated advice message, but since we cannot share any
of the other lines (one spot calls exit() and the other uses die()), I
am OK with it as-is.
-Peff
next prev parent reply other threads:[~2023-05-17 9:32 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-14 13:17 [PATCH 0/3] tag: keep the message file in case ref transaction fails Kristoffer Haugsbakk
2023-05-14 13:17 ` [PATCH 1/3] t/t7004-tag: add regression test for existing behavior Kristoffer Haugsbakk
2023-05-15 6:59 ` Junio C Hamano
2023-05-14 13:17 ` [PATCH 2/3] t/t7004-tag: add failing tag message file test Kristoffer Haugsbakk
2023-05-15 7:00 ` Junio C Hamano
2023-05-15 19:56 ` Kristoffer Haugsbakk
2023-05-15 7:02 ` Junio C Hamano
2023-05-14 13:18 ` [PATCH 3/3] tag: keep the message file in case ref transaction fails Kristoffer Haugsbakk
2023-05-15 6:59 ` Junio C Hamano
2023-05-15 20:29 ` [PATCH v2 0/3] " Kristoffer Haugsbakk
2023-05-15 20:29 ` [PATCH v2 1/3] doc: tag: document `TAG_EDITMSG` Kristoffer Haugsbakk
2023-05-15 21:59 ` Junio C Hamano
2023-05-15 20:29 ` [PATCH v2 2/3] t/t7004-tag: add regression test for successful tag creation Kristoffer Haugsbakk
2023-05-15 20:29 ` [PATCH v2 3/3] tag: keep the message file in case ref transaction fails Kristoffer Haugsbakk
2023-05-15 21:50 ` [PATCH v2 0/3] " Junio C Hamano
2023-05-16 17:55 ` [PATCH v3 " Kristoffer Haugsbakk
2023-05-16 17:55 ` [PATCH v3 1/3] doc: tag: document `TAG_EDITMSG` Kristoffer Haugsbakk
2023-05-16 17:55 ` [PATCH v3 2/3] t/t7004-tag: add regression test for successful tag creation Kristoffer Haugsbakk
2023-05-16 17:55 ` [PATCH v3 3/3] tag: keep the message file in case ref transaction fails Kristoffer Haugsbakk
2023-05-16 18:39 ` [PATCH v3 0/3] " Junio C Hamano
2023-05-17 9:32 ` Jeff King [this message]
2023-05-17 16:00 ` [PATCH " Junio C Hamano
2023-05-17 17:37 ` Jeff King
2023-05-17 17:52 ` Eric Sunshine
2023-05-17 18:02 ` Kristoffer Haugsbakk
2023-05-17 19:58 ` Kristoffer Haugsbakk
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230517093214.GA527234@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=code@khaugsbakk.name \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).