From: "Kristoffer Haugsbakk" <code@khaugsbakk.name>
To: "Jeff King" <peff@peff.net>
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 21:58:00 +0200 [thread overview]
Message-ID: <970f3420-13b8-455c-bee5-284a79893c8a@app.fastmail.com> (raw)
In-Reply-To: <20230517093214.GA527234@coredump.intra.peff.net>
On Wed, May 17, 2023, at 11:32, Jeff King wrote:
> 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:
I didn’t even think of that. :) Too much tunnel-focus on that text file…
>
> [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.
The current error says:
```
fatal: cannot lock ref 'refs/tags/release': 'refs/tags/release/v1' exists; cannot create 'refs/tags/release'
```
As a user, I have no idea if anything (the tag object) was even
created. The error just says that the tag couldn’t be created. (As a
user the ref pointing to the annotated tag object and the object itself
sound like basically the same thing.)
I might be able to guess that it *had* created something tangible like
an object in the database since it got all the way to update the ref to
point to it (just one more step…). But that feels like a stretch.
> But one other alternative might be to mention the hash of that tag
> object, and possibly give advice on recovering it.
That could work. Like:
```
hint: git tag <new name> f73e152cb1560aff1446b208affba4cdcf5bea36
```
So I copy that into my prompt and update the name. All is good.
But I might decide to be just copy the hash, press “Up” in the terminal,
copy the hash at the end, and change the name:
```
git tag -a release-it f73e152cb1560aff1446b208affba4cdcf5bea36
```
Next I’m in my text editor again. Not sure what that is about but okay.
*closes it*
```
hint: You have created a nested tag. The object referred to by your new tag is
hint: already a tag. If you meant to tag the object that it points to, use:
hint:
hint: git tag -f release-it f73e152cb1560aff1446b208affba4cdcf5bea36^{}
hint: Disable this message with "git config advice.nestedTag false"
```
> It's too bad we do not have "git tag -c" to match "git commit -c", which
> would allow us to say something like:
Sure. I appreciate that kind of symmetry. (I guess that’s not the most
precise word in this context.)
> 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.
As a user, the only expensive part (potentially; refer back to footnote
№ 2 of the CV) is writing the tag message. So fudging around a bit with
copying the backup text file into the new message (I would probably just
copy it into the editor) is not a big deal. And retrying the command is
just “Up” and a little line editing.
> So I'm not really opposed to your patch, but mostly just brainstorming
> alternatives.
It’s very much appreciated.
> 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
I’ll make a function for the message in the next round. It’s nice to not
have to duplicate such strings/message ids, I think.
Cheers
--
Kristoffer Haugsbakk
prev parent reply other threads:[~2023-05-17 19:58 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 ` [PATCH " Jeff King
2023-05-17 16:00 ` 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 [this message]
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=970f3420-13b8-455c-bee5-284a79893c8a@app.fastmail.com \
--to=code@khaugsbakk.name \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/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).