Re: [PATCH 0/3] tag: keep the message file in case ref transaction fails

From: Kristoffer Haugsbakk <code@khaugsbakk.name>
Date: May 17, 2023, 19:58 UTC
Message-ID: <970f3420-13b8-455c-bee5-284a79893c8a@app.fastmail.com>
In-Reply-To: <20230517093214.GA527234@coredump.intra.peff.net>

On Wed, May 17, 2023, at 11:32, Jeff King wrote:

Show 8 quoted lines

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

Show 16 quoted lines

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

Show 6 quoted lines

> 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