Thread (12 messages) flat view 12 messages, 2 authors, 2016-06-15

Re: [PATCH] Reuse previous annotation when overwriting a tag

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:46

Hi,

On Sat, 3 Nov 2007, Mike Hommey wrote:
quoted hunk ↗ jump to hunk
diff --git a/builtin-tag.c b/builtin-tag.c
index 66e5a58..cfd8017 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -247,9 +247,42 @@ static int git_tag_config(const char *var, const char *value)
 	return git_default_config(var, value);
 }
 
+static void write_annotation(int fd, const unsigned char *sha1)
Technically, it is the "body".
+{
+	int i;
+	unsigned long size;
+	enum object_type type;
+	char *buf, *sp, *eol;
+	size_t len;
+
+	sp = buf = read_sha1_file(sha1, &type, &size);
+	if (!buf)
+		return;
+	if (!size || (type != OBJ_TAG)) {
Please lose the extra parents.
+		free(buf);
+		return;
+	}
+	/* skip header */
+	while (sp + 1 < buf + size &&
+			!(sp[0] == '\n' && sp[1] == '\n'))
+		sp++;
This can be done much easier with 'sp = strstr(buf, "\n\n");'.  You can 
even do that before the previous if(), to free() && return if there is no 
body.
+	/* strip the signature */
+	for (i = 0, sp += 2; sp < buf + size &&
+			prefixcmp(sp, PGP_SIGNATURE "\n");
+			i++) {
+		eol = memchr(sp, '\n', size - (sp - buf));
+		len = eol ? eol - sp : size - (sp - buf);
+		write_or_die(fd, sp, len + 1);
+		if (!eol)
+			break;
+		sp = eol + 1;
+	}
+	free(buf);
This can be done much easier with 'eob = strstr(sp, "\n" PGP_SIGNATURE 
"\n");'.
+}
+
 static void create_tag(const unsigned char *object, const char *tag,
 		       struct strbuf *buf, int message, int sign,
-			   unsigned char *result)
+			unsigned char *prev, unsigned char *result)
This changes indentation.
quoted hunk ↗ jump to hunk
@@ -282,6 +315,10 @@ static void create_tag(const unsigned char *object, const char *tag,
 		if (fd < 0)
 			die("could not create file '%s': %s",
 						path, strerror(errno));
+
+		if (prev)
+			write_annotation(fd, prev);
+
 		write_or_die(fd, tag_template, strlen(tag_template));
Isn't an "else" missing before the write_or_die() here?
quoted hunk ↗ jump to hunk
@@ -308,7 +345,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 {
 	struct strbuf buf;
 	unsigned char object[20], prev[20];
-	int annotate = 0, sign = 0, force = 0, lines = 0, message = 0;
+	int annotate = 0, sign = 0, force = 0, lines = 0,
+	    message = 0, existed = 0;
 	char ref[PATH_MAX];
 	const char *object_ref, *tag;
 	int i;
@@ -417,9 +455,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		hashclr(prev);
 	else if (!force)
 		die("tag '%s' already exists", tag);
+	else
+		existed = 1;
 
 	if (annotate)
-		create_tag(object, tag, &buf, message, sign, object);
+		create_tag(object, tag, &buf, message, sign,
+			   existed ? prev : NULL, object);
Why not teach write_annotations() (or write_tag_body() like I would prefer 
it to be called) to grok a null_sha1?  It's not like we care for 
performance here, but rather for readability and ease of use.

Ciao,
Dscho
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help