Re: git tag -s: TAG_EDITMSG should not be deleted upon failures
Jeff King <peff@peff.net> writes:
Show 17 quoted lines
> tag: delete TAG_EDITMSG only on successful tag
>
> The user may put some effort into writing an annotated tag
> message. When the tagging process later fails (which can
> happen fairly easily, since it may be dependent on gpg being
> correctly configured and used), there is no record left on
> disk of the tag message.
>
> Instead, let's keep the TAG_EDITMSG file around until we are
> sure the tag has been created successfully. If we die
> because of an error, the user can recover their text from
> that file. Leaving the file in place causes no conflicts;
> it will be silently overwritten by the next annotated tag
> creation.
>
> This matches the behavior of COMMIT_EDITMSG, which stays
> around in case of error.
Thanks. I love patches that addresses bugs during -rc period.
Show 11 quoted lines
> There are two possible improvements I can think of:
>
> - we can be more friendly about helping the user recover. Right now,
> we don't tell them that their message was saved anywhere, and it
> will be silently overwritten if they try another tag. I'm not sure
> what would be the best way to go about that, though.
>
> - the "path" variable became a little less local. It might be worth
> giving it a better name ("editmsg_path" or similar), but keeping it
> made the diff a lot less noisy (and it's still local to a fairly
> simple function).There is another.
- the "path" variable is uninitialized if we do not start editor at
all, so unlink(path) and free(path) have a very high chance of
failing. I think you need [Update #1] below squashed in to fix this.
As to your first potential improvement, I think you could do something like [Update #2] (on top of [Update #1], of course).
[Update #1]
diff --git c/builtin-tag.c w/builtin-tag.c
index ea596d2..8086b3a 100644
--- c/builtin-tag.c
+++ w/builtin-tag.c
@@ -260,7 +260,7 @@ static void create_tag(const unsigned char *object, const char *tag,
enum object_type type;
char header_buf[1024];
int header_len;
- char *path;
+ char *path = NULL;
type = sha1_object_info(object, NULL);
if (type <= OBJ_NONE)
@@ -314,8 +314,10 @@ static void create_tag(const unsigned char *object, const char *tag,
if (write_sha1_file(buf->buf, buf->len, tag_type, result) < 0)
die("unable to write tag file");
- unlink(path);
- free(path);
+ if (path) {
+ unlink(path);
+ free(path);
+ }
}
struct msg_arg {
[Update #2]
diff --git i/builtin-tag.c w/builtin-tag.c
index 8086b3a..20c1c0e 100644
--- i/builtin-tag.c
+++ w/builtin-tag.c
@@ -253,6 +253,15 @@ static void write_tag_body(int fd, const unsigned char *sha1)
free(buf);
}
+static int build_tag_object(struct strbuf *buf, int sign, unsigned char *result)
+{
+ if (sign && do_sign(buf) < 0)
+ return error("unable to sign the tag");
+ if (write_sha1_file(buf->buf, buf->len, tag_type, result) < 0)
+ return error("unable to write tag file");
+ return 0;
+}
+
static void create_tag(const unsigned char *object, const char *tag,
struct strbuf *buf, int message, int sign,
unsigned char *prev, unsigned char *result)
@@ -309,11 +318,12 @@ static void create_tag(const unsigned char *object, const char *tag,
strbuf_insert(buf, 0, header_buf, header_len);
- if (sign && do_sign(buf) < 0)
- die("unable to sign the tag");
- if (write_sha1_file(buf->buf, buf->len, tag_type, result) < 0)
- die("unable to write tag file");
-
+ if (build_tag_object(buf, sign, result) < 0) {
+ if (path)
+ fprintf(stderr, "What you edited in your editor is left in %s",
+ path);
+ exit(128);
+ }
if (path) {
unlink(path);
free(path);
