[PATCH v3 09/34] mailinfo: do not let find_boundary() touch global "line" directly

[Junio C Hamano]

With the previous two commits, we established that the local
variable "line" in handle_body() and handle_boundary() functions
always refer to the global "line" that is used as the common and
shared "current line from the input".  They are the only callers of
the last function that refers to the global line directly, i.e.
find_boundary().  Pass "line" as a parameter to this leaf function
to complete the clean-up.  Now the only function that directly refers
to the global "line" is the caller of handle_body() at the very
beginning of this whole callchain.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index cab1235..12d1eda 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -788,10 +788,10 @@ static void handle_filter(struct strbuf *line, int 
*filter_stage, int *header_st
}
 }
 
-static int find_boundary(void)
+static int find_boundary(struct strbuf *line)
 {
-   while (!strbuf_getline(, fin, '\n')) {
-   if (*content_top && is_multipart_boundary())
+   while (!strbuf_getline(line, fin, '\n')) {
+   if (*content_top && is_multipart_boundary(line))
return 1;
}
return 0;
@@ -823,7 +823,7 @@ again:
strbuf_release();
 
/* skip to the next boundary */
-   if (!find_boundary())
+   if (!find_boundary(line))
return 0;
goto again;
}
@@ -852,7 +852,7 @@ static void handle_body(struct strbuf *line)
 
/* Skip up to the first boundary */
if (*content_top) {
-   if (!find_boundary())
+   if (!find_boundary(line))
goto handle_body_out;
}
 
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 02/34] mailinfo: fold decode_header_bq() into decode_header()

[Junio C Hamano]

In olden days we might have wanted to behave differently in
decode_header() if the header line was encoded with RFC2047, but we
apparently do not do so, hence this helper function can go, together
with its return value.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 23 +++
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 5a4ed75..addc0e0 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -525,19 +525,17 @@ static void convert_to_utf8(struct strbuf *line, const 
char *charset)
strbuf_attach(line, out, strlen(out), strlen(out));
 }
 
-static int decode_header_bq(struct strbuf *it)
+static void decode_header(struct strbuf *it)
 {
char *in, *ep, *cp;
struct strbuf outbuf = STRBUF_INIT, *dec;
struct strbuf charset_q = STRBUF_INIT, piecebuf = STRBUF_INIT;
-   int rfc2047 = 0;
 
in = it->buf;
while (in - it->buf <= it->len && (ep = strstr(in, "=?")) != NULL) {
int encoding;
strbuf_reset(_q);
strbuf_reset();
-   rfc2047 = 1;
 
if (in != ep) {
/*
@@ -567,22 +565,22 @@ static int decode_header_bq(struct strbuf *it)
ep += 2;
 
if (ep - it->buf >= it->len || !(cp = strchr(ep, '?')))
-   goto decode_header_bq_out;
+   goto release_return;
 
if (cp + 3 - it->buf > it->len)
-   goto decode_header_bq_out;
+   goto release_return;
strbuf_add(_q, ep, cp - ep);
 
encoding = cp[1];
if (!encoding || cp[2] != '?')
-   goto decode_header_bq_out;
+   goto release_return;
ep = strstr(cp + 3, "?=");
if (!ep)
-   goto decode_header_bq_out;
+   goto release_return;
strbuf_add(, cp + 3, ep - cp - 3);
switch (tolower(encoding)) {
default:
-   goto decode_header_bq_out;
+   goto release_return;
case 'b':
dec = decode_b_segment();
break;
@@ -601,17 +599,10 @@ static int decode_header_bq(struct strbuf *it)
strbuf_addstr(, in);
strbuf_reset(it);
strbuf_addbuf(it, );
-decode_header_bq_out:
+release_return:
strbuf_release();
strbuf_release(_q);
strbuf_release();
-   return rfc2047;
-}
-
-static void decode_header(struct strbuf *it)
-{
-   if (decode_header_bq(it))
-   return;
 }
 
 static void decode_transfer_encoding(struct strbuf *line)
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 11/34] mailinfo: introduce "struct mailinfo" to hold globals

[Junio C Hamano]

In this first step, move only 'email' and 'name' fields in there and
remove the corresponding globals.  In subsequent patches, more
globals will be moved to this and the structure will be passed
around as a new parameter to more functions.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 71 --
 1 file changed, 47 insertions(+), 24 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index c8dc73f..e3c0c31 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -12,8 +12,11 @@ static FILE *cmitmsg, *patchfile, *fin, *fout;
 static int keep_subject;
 static int keep_non_patch_brackets_in_subject;
 static const char *metainfo_charset;
-static struct strbuf name = STRBUF_INIT;
-static struct strbuf email = STRBUF_INIT;
+
+struct mailinfo {
+   struct strbuf name;
+   struct strbuf email;
+};
 static char *message_id;
 
 static enum  {
@@ -45,7 +48,7 @@ static void get_sane_name(struct strbuf *out, struct strbuf 
*name, struct strbuf
strbuf_addbuf(out, src);
 }
 
-static void parse_bogus_from(const struct strbuf *line)
+static void parse_bogus_from(struct mailinfo *mi, const struct strbuf *line)
 {
/* John Doe  */
 
@@ -53,7 +56,7 @@ static void parse_bogus_from(const struct strbuf *line)
/* This is fallback, so do not bother if we already have an
 * e-mail address.
 */
-   if (email.len)
+   if (mi->email.len)
return;
 
bra = strchr(line->buf, '<');
@@ -63,16 +66,16 @@ static void parse_bogus_from(const struct strbuf *line)
if (!ket)
return;
 
-   strbuf_reset();
-   strbuf_add(, bra + 1, ket - bra - 1);
+   strbuf_reset(>email);
+   strbuf_add(>email, bra + 1, ket - bra - 1);
 
-   strbuf_reset();
-   strbuf_add(, line->buf, bra - line->buf);
-   strbuf_trim();
-   get_sane_name(, , );
+   strbuf_reset(>name);
+   strbuf_add(>name, line->buf, bra - line->buf);
+   strbuf_trim(>name);
+   get_sane_name(>name, >name, >email);
 }
 
-static void handle_from(const struct strbuf *from)
+static void handle_from(struct mailinfo *mi, const struct strbuf *from)
 {
char *at;
size_t el;
@@ -83,14 +86,14 @@ static void handle_from(const struct strbuf *from)
 
at = strchr(f.buf, '@');
if (!at) {
-   parse_bogus_from(from);
+   parse_bogus_from(mi, from);
return;
}
 
/*
 * If we already have one email, don't take any confusing lines
 */
-   if (email.len && strchr(at + 1, '@')) {
+   if (mi->email.len && strchr(at + 1, '@')) {
strbuf_release();
return;
}
@@ -109,8 +112,8 @@ static void handle_from(const struct strbuf *from)
at--;
}
el = strcspn(at, " \n\t\r\v\f>");
-   strbuf_reset();
-   strbuf_add(, at, el);
+   strbuf_reset(>email);
+   strbuf_add(>email, at, el);
strbuf_remove(, at - f.buf, el + (at[el] ? 1 : 0));
 
/* The remainder is name.  It could be
@@ -132,7 +135,7 @@ static void handle_from(const struct strbuf *from)
strbuf_setlen(, f.len - 1);
}
 
-   get_sane_name(, , );
+   get_sane_name(>name, , >email);
strbuf_release();
 }
 
@@ -929,7 +932,7 @@ static void output_header_lines(FILE *fout, const char 
*hdr, const struct strbuf
}
 }
 
-static void handle_info(void)
+static void handle_info(struct mailinfo *mi)
 {
struct strbuf *hdr;
int i;
@@ -951,9 +954,9 @@ static void handle_info(void)
output_header_lines(fout, "Subject", hdr);
} else if (!strcmp(header[i], "From")) {
cleanup_space(hdr);
-   handle_from(hdr);
-   fprintf(fout, "Author: %s\n", name.buf);
-   fprintf(fout, "Email: %s\n", email.buf);
+   handle_from(mi, hdr);
+   fprintf(fout, "Author: %s\n", mi->name.buf);
+   fprintf(fout, "Email: %s\n", mi->email.buf);
} else {
cleanup_space(hdr);
fprintf(fout, "%s: %s\n", header[i], hdr->buf);
@@ -962,7 +965,8 @@ static void handle_info(void)
fprintf(fout, "\n");
 }
 
-static int mailinfo(FILE *in, FILE *out, const char *msg, const char *patch)
+static int mailinfo(struct mailinfo *mi,
+   FILE *in, FILE *out, const char *msg, const char *patch)
 {
int peek;
struct strbuf line = STRBUF_INIT;
@@ -997,7 +1001,7 @@ static int mailinfo(FILE *in, FILE *out, const char *msg, 
const char *patch)
handle_body();
fclose(patchfile);
 
-   handle_info();
+   handle_info(mi);
 
return 0;
 }
@@ -1014,17 +1018,33 @@ static int git_mailinfo_config(const char *var, const 
char *value, 

[PATCH v3 10/34] mailinfo: move global "line" into mailinfo() function

[Junio C Hamano]

With the previous steps, it becomes clear that the mailinfo()
function is the only one that wants the "line" to be directly
touchable.  Move it to the function scope of this function.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 12d1eda..c8dc73f 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -12,7 +12,6 @@ static FILE *cmitmsg, *patchfile, *fin, *fout;
 static int keep_subject;
 static int keep_non_patch_brackets_in_subject;
 static const char *metainfo_charset;
-static struct strbuf line = STRBUF_INIT;
 static struct strbuf name = STRBUF_INIT;
 static struct strbuf email = STRBUF_INIT;
 static char *message_id;
@@ -966,6 +965,8 @@ static void handle_info(void)
 static int mailinfo(FILE *in, FILE *out, const char *msg, const char *patch)
 {
int peek;
+   struct strbuf line = STRBUF_INIT;
+
fin = in;
fout = out;
 
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add ls-files --eol-staged, --eol-worktree

[Junio C Hamano]

Torsten Bögershausen  writes:

> I like this idea:
>
> binary
> text
> crlf
> mixed
> lf

If you really like it, it would mean that my attempt to use Socratic
method to enlighten you why the above is not a good idea failed X-<.

> 
> $ git ls-files --eol-staged -s
>  [snip]
>  100644 981f810e80008d878d6a5af1331c89dc093c5927 0   txt-lf worktree.c

Does it even make sense to give --eol-worktree in this case?

> My understanding is that the eol options work togther with the existing 
> option,
> as long as it makes sense (but see below)
>
> "git check-attr" will even report attributes for a file, that doesn't even 
> exist.

Both "ls-files -o/-i" talk about untracked paths, so that is not a
very useful and valid objection, is it?

> "git ls-files is a command which by default operates on the staged
> area, unless I mis-understand it.

It is even worse than that.  It is true that "ls-files [-s]" is
about "--cached" and there is no equivalent to show the working tree
version.  But "-t", "-d", etc. are not about the state in the index
nor the state in the working tree.  They are about the relationship
between these two states.

What the new operation wants to do, if I understand correctly, is
either check the blob contents in the working tree or in the index,
which is not a good fit with what the rest of "ls-files" does for
exactly that reason.  The inability to mix -s with --eol-worktree
is another natural consequence of this.

> I was thinking about adding "git check-eol", but didn't want to
> introduce just another command,

Between adding a new command that does one thing well and whose user
interaction is coherent with the rest of the system, and adding a
new operation mode to an existing command and makes the user
interaction of that existing command more incoherent by introducing
two variants --foo and --foo-worktree when there is no existing
option that has similar variant pair, I'd say we prefer to see a new
command.

The -z output, and --stdin input are what we would want to have for
the new command, but I do not think we want it to know -o, -x or -X.
You would instead pipe output from ls-files with these options to
the new command run with the --stdin option.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 01/34] mailinfo: remove a no-op call convert_to_utf8(it, "")

[Junio C Hamano]

The called function checks if the second parameter is either a NULL
or an empty string at the very beginning and returns without doing
anything.  Remove the useless call.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 999a525..5a4ed75 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -612,11 +612,6 @@ static void decode_header(struct strbuf *it)
 {
if (decode_header_bq(it))
return;
-   /* otherwise "it" is a straight copy of the input.
-* This can be binary guck but there is no charset specified.
-*/
-   if (metainfo_charset)
-   convert_to_utf8(it, "");
 }
 
 static void decode_transfer_encoding(struct strbuf *line)
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 28/34] mailinfo: move check_header() after the helpers it uses

[Junio C Hamano]

This way, we can lose a forward decl for decode_header().

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 139 ++---
 1 file changed, 69 insertions(+), 70 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 5a21d93..6fc6aa8 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -293,7 +293,6 @@ static void cleanup_space(struct strbuf *sb)
}
 }
 
-static void decode_header(struct mailinfo *mi, struct strbuf *line);
 static const char *header[MAX_HDR_PARSED] = {
"From","Subject","Date",
 };
@@ -321,75 +320,6 @@ static int is_format_patch_separator(const char *line, int 
len)
return !memcmp(SAMPLE + (cp - line), cp, strlen(SAMPLE) - (cp - line));
 }
 
-static int check_header(struct mailinfo *mi,
-   const struct strbuf *line,
-   struct strbuf *hdr_data[], int overwrite)
-{
-   int i, ret = 0, len;
-   struct strbuf sb = STRBUF_INIT;
-
-   /* search for the interesting parts */
-   for (i = 0; header[i]; i++) {
-   int len = strlen(header[i]);
-   if ((!hdr_data[i] || overwrite) && cmp_header(line, header[i])) 
{
-   /* Unwrap inline B and Q encoding, and optionally
-* normalize the meta information to utf8.
-*/
-   strbuf_add(, line->buf + len + 2, line->len - len - 
2);
-   decode_header(mi, );
-   handle_header(_data[i], );
-   ret = 1;
-   goto check_header_out;
-   }
-   }
-
-   /* Content stuff */
-   if (cmp_header(line, "Content-Type")) {
-   len = strlen("Content-Type: ");
-   strbuf_add(, line->buf + len, line->len - len);
-   decode_header(mi, );
-   strbuf_insert(, 0, "Content-Type: ", len);
-   handle_content_type(mi, );
-   ret = 1;
-   goto check_header_out;
-   }
-   if (cmp_header(line, "Content-Transfer-Encoding")) {
-   len = strlen("Content-Transfer-Encoding: ");
-   strbuf_add(, line->buf + len, line->len - len);
-   decode_header(mi, );
-   handle_content_transfer_encoding(mi, );
-   ret = 1;
-   goto check_header_out;
-   }
-   if (cmp_header(line, "Message-Id")) {
-   len = strlen("Message-Id: ");
-   strbuf_add(, line->buf + len, line->len - len);
-   decode_header(mi, );
-   handle_message_id(mi, );
-   ret = 1;
-   goto check_header_out;
-   }
-
-   /* for inbody stuff */
-   if (starts_with(line->buf, ">From") && isspace(line->buf[5])) {
-   ret = is_format_patch_separator(line->buf + 1, line->len - 1);
-   goto check_header_out;
-   }
-   if (starts_with(line->buf, "[PATCH]") && isspace(line->buf[7])) {
-   for (i = 0; header[i]; i++) {
-   if (!strcmp("Subject", header[i])) {
-   handle_header(_data[i], line);
-   ret = 1;
-   goto check_header_out;
-   }
-   }
-   }
-
-check_header_out:
-   strbuf_release();
-   return ret;
-}
-
 static struct strbuf *decode_q_segment(const struct strbuf *q_seg, int rfc2047)
 {
const char *in = q_seg->buf;
@@ -550,6 +480,75 @@ release_return:
strbuf_release();
 }
 
+static int check_header(struct mailinfo *mi,
+   const struct strbuf *line,
+   struct strbuf *hdr_data[], int overwrite)
+{
+   int i, ret = 0, len;
+   struct strbuf sb = STRBUF_INIT;
+
+   /* search for the interesting parts */
+   for (i = 0; header[i]; i++) {
+   int len = strlen(header[i]);
+   if ((!hdr_data[i] || overwrite) && cmp_header(line, header[i])) 
{
+   /* Unwrap inline B and Q encoding, and optionally
+* normalize the meta information to utf8.
+*/
+   strbuf_add(, line->buf + len + 2, line->len - len - 
2);
+   decode_header(mi, );
+   handle_header(_data[i], );
+   ret = 1;
+   goto check_header_out;
+   }
+   }
+
+   /* Content stuff */
+   if (cmp_header(line, "Content-Type")) {
+   len = strlen("Content-Type: ");
+   strbuf_add(, line->buf + len, line->len - len);
+   decode_header(mi, );
+   strbuf_insert(, 0, "Content-Type: ", len);
+   handle_content_type(mi, );
+   ret = 1;
+   goto check_header_out;
+   }
+   if (cmp_header(line, 

[PATCH v3 12/34] mailinfo: move keep_subject & keep_non_patch_bracket to struct mailinfo

[Junio C Hamano]

These two are the only easy ones that do not require passing the
structure around to deep corners of the callchain.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index e3c0c31..08c67f5 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -9,13 +9,13 @@
 
 static FILE *cmitmsg, *patchfile, *fin, *fout;
 
-static int keep_subject;
-static int keep_non_patch_brackets_in_subject;
 static const char *metainfo_charset;
 
 struct mailinfo {
struct strbuf name;
struct strbuf email;
+   int keep_subject;
+   int keep_non_patch_brackets_in_subject;
 };
 static char *message_id;
 
@@ -224,7 +224,7 @@ static int is_multipart_boundary(const struct strbuf *line)
!memcmp(line->buf, (*content_top)->buf, (*content_top)->len));
 }
 
-static void cleanup_subject(struct strbuf *subject)
+static void cleanup_subject(struct mailinfo *mi, struct strbuf *subject)
 {
size_t at = 0;
 
@@ -252,7 +252,7 @@ static void cleanup_subject(struct strbuf *subject)
if (!pos)
break;
remove = pos - subject->buf + at + 1;
-   if (!keep_non_patch_brackets_in_subject ||
+   if (!mi->keep_non_patch_brackets_in_subject ||
(7 <= remove &&
 memmem(subject->buf + at, remove, "PATCH", 5)))
strbuf_remove(subject, at, remove);
@@ -947,8 +947,8 @@ static void handle_info(struct mailinfo *mi)
continue;
 
if (!strcmp(header[i], "Subject")) {
-   if (!keep_subject) {
-   cleanup_subject(hdr);
+   if (!mi->keep_subject) {
+   cleanup_subject(mi, hdr);
cleanup_space(hdr);
}
output_header_lines(fout, "Subject", hdr);
@@ -1051,9 +1051,9 @@ int cmd_mailinfo(int argc, const char **argv, const char 
*prefix)
 
while (1 < argc && argv[1][0] == '-') {
if (!strcmp(argv[1], "-k"))
-   keep_subject = 1;
+   mi.keep_subject = 1;
else if (!strcmp(argv[1], "-b"))
-   keep_non_patch_brackets_in_subject = 1;
+   mi.keep_non_patch_brackets_in_subject = 1;
else if (!strcmp(argv[1], "-m") || !strcmp(argv[1], 
"--message-id"))
add_message_id = 1;
else if (!strcmp(argv[1], "-u"))
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 03/34] mailinfo: fix an off-by-one error in the boundary stack

[Junio C Hamano]

We pre-increment the pointer that we will use to store something at,
so the pointer is already beyond the end of the array if it points
at content[MAX_BOUNDARIES].

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index addc0e0..1566c19 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -185,7 +185,7 @@ static void handle_content_type(struct strbuf *line)
 
if (slurp_attr(line->buf, "boundary=", boundary)) {
strbuf_insert(boundary, 0, "--", 2);
-   if (++content_top > [MAX_BOUNDARIES]) {
+   if (++content_top >= [MAX_BOUNDARIES]) {
fprintf(stderr, "Too many boundaries to handle\n");
exit(1);
}
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 19/34] mailinfo: move check for metainfo_charset to convert_to_utf8()

[Junio C Hamano]

All callers of this function refrains from calling it when
mi->metainfo_charset is NULL; move the check to the callee,
as it already has a few conditions at its beginning to turn
it into a no-op.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 26fd9b1..737d0fc 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -524,7 +524,7 @@ static void convert_to_utf8(struct mailinfo *mi,
 {
char *out;
 
-   if (!charset || !*charset)
+   if (!mi->metainfo_charset || !charset || !*charset)
return;
 
if (same_encoding(mi->metainfo_charset, charset))
@@ -599,8 +599,7 @@ static void decode_header(struct mailinfo *mi, struct 
strbuf *it)
dec = decode_q_segment(, 1);
break;
}
-   if (mi->metainfo_charset)
-   convert_to_utf8(mi, dec, charset_q.buf);
+   convert_to_utf8(mi, dec, charset_q.buf);
 
strbuf_addbuf(, dec);
strbuf_release(dec);
@@ -745,8 +744,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
mi->header_stage = 0;
 
/* normalize the log message to UTF-8. */
-   if (mi->metainfo_charset)
-   convert_to_utf8(mi, line, charset.buf);
+   convert_to_utf8(mi, line, charset.buf);
 
if (mi->use_scissors && is_scissors_line(line)) {
int i;
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 13/34] mailinfo: move global "FILE *fin, *fout" to struct mailinfo

[Junio C Hamano]

This requires us to pass "struct mailinfo" to more functions
throughout the codepath that read input lines.  Incidentally,
later steps are helped by this patch passing the struct to
more callchains.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 54 --
 1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 08c67f5..b150936 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -7,11 +7,14 @@
 #include "utf8.h"
 #include "strbuf.h"
 
-static FILE *cmitmsg, *patchfile, *fin, *fout;
+static FILE *cmitmsg, *patchfile;
 
 static const char *metainfo_charset;
 
 struct mailinfo {
+   FILE *input;
+   FILE *output;
+
struct strbuf name;
struct strbuf email;
int keep_subject;
@@ -790,16 +793,17 @@ static void handle_filter(struct strbuf *line, int 
*filter_stage, int *header_st
}
 }
 
-static int find_boundary(struct strbuf *line)
+static int find_boundary(struct mailinfo *mi, struct strbuf *line)
 {
-   while (!strbuf_getline(line, fin, '\n')) {
+   while (!strbuf_getline(line, mi->input, '\n')) {
if (*content_top && is_multipart_boundary(line))
return 1;
}
return 0;
 }
 
-static int handle_boundary(struct strbuf *line, int *filter_stage, int 
*header_stage)
+static int handle_boundary(struct mailinfo *mi, struct strbuf *line,
+  int *filter_stage, int *header_stage)
 {
struct strbuf newline = STRBUF_INIT;
 
@@ -825,7 +829,7 @@ again:
strbuf_release();
 
/* skip to the next boundary */
-   if (!find_boundary(line))
+   if (!find_boundary(mi, line))
return 0;
goto again;
}
@@ -835,18 +839,18 @@ again:
strbuf_reset();
 
/* slurp in this section's info */
-   while (read_one_header_line(line, fin))
+   while (read_one_header_line(line, mi->input))
check_header(line, p_hdr_data, 0);
 
strbuf_release();
/* replenish line */
-   if (strbuf_getline(line, fin, '\n'))
+   if (strbuf_getline(line, mi->input, '\n'))
return 0;
strbuf_addch(line, '\n');
return 1;
 }
 
-static void handle_body(struct strbuf *line)
+static void handle_body(struct mailinfo *mi, struct strbuf *line)
 {
struct strbuf prev = STRBUF_INIT;
int filter_stage = 0;
@@ -854,7 +858,7 @@ static void handle_body(struct strbuf *line)
 
/* Skip up to the first boundary */
if (*content_top) {
-   if (!find_boundary(line))
+   if (!find_boundary(mi, line))
goto handle_body_out;
}
 
@@ -866,7 +870,7 @@ static void handle_body(struct strbuf *line)
handle_filter(, _stage, 
_stage);
strbuf_reset();
}
-   if (!handle_boundary(line, _stage, 
_stage))
+   if (!handle_boundary(mi, line, _stage, 
_stage))
goto handle_body_out;
}
 
@@ -909,7 +913,7 @@ static void handle_body(struct strbuf *line)
handle_filter(line, _stage, _stage);
}
 
-   } while (!strbuf_getwholeline(line, fin, '\n'));
+   } while (!strbuf_getwholeline(line, mi->input, '\n'));
 
 handle_body_out:
strbuf_release();
@@ -951,29 +955,25 @@ static void handle_info(struct mailinfo *mi)
cleanup_subject(mi, hdr);
cleanup_space(hdr);
}
-   output_header_lines(fout, "Subject", hdr);
+   output_header_lines(mi->output, "Subject", hdr);
} else if (!strcmp(header[i], "From")) {
cleanup_space(hdr);
handle_from(mi, hdr);
-   fprintf(fout, "Author: %s\n", mi->name.buf);
-   fprintf(fout, "Email: %s\n", mi->email.buf);
+   fprintf(mi->output, "Author: %s\n", mi->name.buf);
+   fprintf(mi->output, "Email: %s\n", mi->email.buf);
} else {
cleanup_space(hdr);
-   fprintf(fout, "%s: %s\n", header[i], hdr->buf);
+   fprintf(mi->output, "%s: %s\n", header[i], hdr->buf);
}
}
-   fprintf(fout, "\n");
+   fprintf(mi->output, "\n");
 }
 
-static int mailinfo(struct mailinfo *mi,
-   FILE *in, FILE *out, const char *msg, const char *patch)
+static int mailinfo(struct mailinfo *mi, const char *msg, const char *patch)
 {
int peek;
struct strbuf line = STRBUF_INIT;
 
-   fin = in;
-   fout = out;
-
cmitmsg = fopen(msg, "w");
   

[PATCH v3 16/34] mailinfo: move add_message_id and message_id to struct mailinfo

[Junio C Hamano]

This requires us to pass the structure into check_header() codepath.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 31 +--
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 8bd76c6..c0522f2 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -19,12 +19,13 @@ struct mailinfo {
struct strbuf email;
int keep_subject;
int keep_non_patch_brackets_in_subject;
+   int add_message_id;
 
+   char *message_id;
int patch_lines;
int filter_stage; /* still reading log or are we copying patch? */
int header_stage; /* still checking in-body headers? */
 };
-static char *message_id;
 
 static enum  {
TE_DONTCARE, TE_QP, TE_BASE64
@@ -34,7 +35,6 @@ static struct strbuf charset = STRBUF_INIT;
 
 static struct strbuf **p_hdr_data, **s_hdr_data;
 static int use_scissors;
-static int add_message_id;
 static int use_inbody_headers = 1;
 
 #define MAX_HDR_PARSED 10
@@ -209,10 +209,10 @@ static void handle_content_type(struct strbuf *line)
}
 }
 
-static void handle_message_id(const struct strbuf *line)
+static void handle_message_id(struct mailinfo *mi, const struct strbuf *line)
 {
-   if (add_message_id)
-   message_id = strdup(line->buf);
+   if (mi->add_message_id)
+   mi->message_id = strdup(line->buf);
 }
 
 static void handle_content_transfer_encoding(const struct strbuf *line)
@@ -321,11 +321,13 @@ static int is_format_patch_separator(const char *line, 
int len)
return !memcmp(SAMPLE + (cp - line), cp, strlen(SAMPLE) - (cp - line));
 }
 
-static int check_header(const struct strbuf *line,
-   struct strbuf *hdr_data[], int overwrite)
+static int check_header(struct mailinfo *mi,
+   const struct strbuf *line,
+   struct strbuf *hdr_data[], int overwrite)
 {
int i, ret = 0, len;
struct strbuf sb = STRBUF_INIT;
+
/* search for the interesting parts */
for (i = 0; header[i]; i++) {
int len = strlen(header[i]);
@@ -363,7 +365,7 @@ static int check_header(const struct strbuf *line,
len = strlen("Message-Id: ");
strbuf_add(, line->buf + len, line->len - len);
decode_header();
-   handle_message_id();
+   handle_message_id(mi, );
ret = 1;
goto check_header_out;
}
@@ -733,7 +735,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
}
 
if (use_inbody_headers && mi->header_stage) {
-   mi->header_stage = check_header(line, s_hdr_data, 0);
+   mi->header_stage = check_header(mi, line, s_hdr_data, 0);
if (mi->header_stage)
return 0;
} else
@@ -767,8 +769,8 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
}
 
if (patchbreak(line)) {
-   if (message_id)
-   fprintf(cmitmsg, "Message-Id: %s\n", message_id);
+   if (mi->message_id)
+   fprintf(cmitmsg, "Message-Id: %s\n", mi->message_id);
fclose(cmitmsg);
cmitmsg = NULL;
return 1;
@@ -843,7 +845,7 @@ again:
 
/* slurp in this section's info */
while (read_one_header_line(line, mi->input))
-   check_header(line, p_hdr_data, 0);
+   check_header(mi, line, p_hdr_data, 0);
 
strbuf_release();
/* replenish line */
@@ -997,7 +999,7 @@ static int mailinfo(struct mailinfo *mi, const char *msg, 
const char *patch)
 
/* process the email header */
while (read_one_header_line(, mi->input))
-   check_header(, p_hdr_data, 1);
+   check_header(mi, , p_hdr_data, 1);
 
handle_body(mi, );
fclose(patchfile);
@@ -1032,6 +1034,7 @@ static void clear_mailinfo(struct mailinfo *mi)
 {
strbuf_release(>name);
strbuf_release(>email);
+   free(mi->message_id);
 }
 
 static const char mailinfo_usage[] =
@@ -1057,7 +1060,7 @@ int cmd_mailinfo(int argc, const char **argv, const char 
*prefix)
else if (!strcmp(argv[1], "-b"))
mi.keep_non_patch_brackets_in_subject = 1;
else if (!strcmp(argv[1], "-m") || !strcmp(argv[1], 
"--message-id"))
-   add_message_id = 1;
+   mi.add_message_id = 1;
else if (!strcmp(argv[1], "-u"))
metainfo_charset = def_charset;
else if (!strcmp(argv[1], "-n"))
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 25/34] mailinfo: handle_commit_msg() shouldn't be called after finding patchbreak

[Junio C Hamano]

There is a strange "if (!mi->cmitmsg) return 0" at the very beginning
of handle_commit_msg(), but the condition should never trigger, because:

 * The only place cmitmsg is set to NULL is after this function sees
   a patch break, closes the FILE * to write the commit log message
   and returns 1.  This function returns non-zero only from that
   codepath.

 * The caller of this function, upon seeing a non-zero return,
   increments filter_stage, starts treating the input as patch text
   and will never call handle_commit_msg() again.

Replace it with an assert(!mi->filter_stage) to ensure the above
observation will stay to be true.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index ec65805..286eda0 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -725,8 +725,7 @@ static int is_scissors_line(const struct strbuf *line)
 
 static int handle_commit_msg(struct mailinfo *mi, struct strbuf *line)
 {
-   if (!mi->cmitmsg)
-   return 0;
+   assert(!mi->filter_stage);
 
if (mi->header_stage) {
if (!line->len || (line->len == 1 && line->buf[0] == '\n'))
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 05/34] mailinfo: move handle_boundary() lower

[Junio C Hamano]

This function wants to call find_boundary() and is called only from
one place without any recursing, so it becomes easier to read if it
appears after the called function.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 114 ++---
 1 file changed, 56 insertions(+), 58 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 73be47c..2b7f97b 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -626,64 +626,6 @@ static void decode_transfer_encoding(struct strbuf *line)
free(ret);
 }
 
-static void handle_filter(struct strbuf *line);
-
-static int find_boundary(void)
-{
-   while (!strbuf_getline(, fin, '\n')) {
-   if (*content_top && is_multipart_boundary())
-   return 1;
-   }
-   return 0;
-}
-
-static int handle_boundary(void)
-{
-   struct strbuf newline = STRBUF_INIT;
-
-   strbuf_addch(, '\n');
-again:
-   if (line.len >= (*content_top)->len + 2 &&
-   !memcmp(line.buf + (*content_top)->len, "--", 2)) {
-   /* we hit an end boundary */
-   /* pop the current boundary off the stack */
-   strbuf_release(*content_top);
-   free(*content_top);
-   *content_top = NULL;
-
-   /* technically won't happen as is_multipart_boundary()
-  will fail first.  But just in case..
-*/
-   if (--content_top < content) {
-   fprintf(stderr, "Detected mismatched boundaries, "
-   "can't recover\n");
-   exit(1);
-   }
-   handle_filter();
-   strbuf_release();
-
-   /* skip to the next boundary */
-   if (!find_boundary())
-   return 0;
-   goto again;
-   }
-
-   /* set some defaults */
-   transfer_encoding = TE_DONTCARE;
-   strbuf_reset();
-
-   /* slurp in this section's info */
-   while (read_one_header_line(, fin))
-   check_header(, p_hdr_data, 0);
-
-   strbuf_release();
-   /* replenish line */
-   if (strbuf_getline(, fin, '\n'))
-   return 0;
-   strbuf_addch(, '\n');
-   return 1;
-}
-
 static inline int patchbreak(const struct strbuf *line)
 {
size_t i;
@@ -851,6 +793,62 @@ static void handle_filter(struct strbuf *line)
}
 }
 
+static int find_boundary(void)
+{
+   while (!strbuf_getline(, fin, '\n')) {
+   if (*content_top && is_multipart_boundary())
+   return 1;
+   }
+   return 0;
+}
+
+static int handle_boundary(void)
+{
+   struct strbuf newline = STRBUF_INIT;
+
+   strbuf_addch(, '\n');
+again:
+   if (line.len >= (*content_top)->len + 2 &&
+   !memcmp(line.buf + (*content_top)->len, "--", 2)) {
+   /* we hit an end boundary */
+   /* pop the current boundary off the stack */
+   strbuf_release(*content_top);
+   free(*content_top);
+   *content_top = NULL;
+
+   /* technically won't happen as is_multipart_boundary()
+  will fail first.  But just in case..
+*/
+   if (--content_top < content) {
+   fprintf(stderr, "Detected mismatched boundaries, "
+   "can't recover\n");
+   exit(1);
+   }
+   handle_filter();
+   strbuf_release();
+
+   /* skip to the next boundary */
+   if (!find_boundary())
+   return 0;
+   goto again;
+   }
+
+   /* set some defaults */
+   transfer_encoding = TE_DONTCARE;
+   strbuf_reset();
+
+   /* slurp in this section's info */
+   while (read_one_header_line(, fin))
+   check_header(, p_hdr_data, 0);
+
+   strbuf_release();
+   /* replenish line */
+   if (strbuf_getline(, fin, '\n'))
+   return 0;
+   strbuf_addch(, '\n');
+   return 1;
+}
+
 static void handle_body(void)
 {
struct strbuf prev = STRBUF_INIT;
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 04/34] mailinfo: explicitly close file handle to the patch output

[Junio C Hamano]

This does not make a difference within the context of "git mailinfo"
that runs once and exits, as flushing and closing would happen upon
process termination.  It however will matter when we eventually make
it callable as an API function.

Besides, cleaning after yourself once you are done is a good hygiene.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 1566c19..73be47c 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -999,6 +999,8 @@ static int mailinfo(FILE *in, FILE *out, const char *msg, 
const char *patch)
check_header(, p_hdr_data, 1);
 
handle_body();
+   fclose(patchfile);
+
handle_info();
 
return 0;
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 06/34] mailinfo: get rid of function-local static states

[Junio C Hamano]

Two helper functions use "static int" in their scope to keep track
of the state while repeatedly getting called once for each input
line.  Move these state variables to their ultimate caller and pass
down pointers to them along the callchain, as a small step in
preparation for making this entire callchain more reentrant.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 41 +++--
 1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 2b7f97b..1518708 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -713,27 +713,25 @@ static int is_scissors_line(const struct strbuf *line)
gap * 2 < perforation);
 }
 
-static int handle_commit_msg(struct strbuf *line)
+static int handle_commit_msg(struct strbuf *line, int *still_looking)
 {
-   static int still_looking = 1;
-
if (!cmitmsg)
return 0;
 
-   if (still_looking) {
+   if (*still_looking) {
if (!line->len || (line->len == 1 && line->buf[0] == '\n'))
return 0;
}
 
-   if (use_inbody_headers && still_looking) {
-   still_looking = check_header(line, s_hdr_data, 0);
-   if (still_looking)
+   if (use_inbody_headers && *still_looking) {
+   *still_looking = check_header(line, s_hdr_data, 0);
+   if (*still_looking)
return 0;
} else
/* Only trim the first (blank) line of the commit message
 * when ignoring in-body headers.
 */
-   still_looking = 0;
+   *still_looking = 0;
 
/* normalize the log message to UTF-8. */
if (metainfo_charset)
@@ -745,7 +743,7 @@ static int handle_commit_msg(struct strbuf *line)
die_errno("Could not rewind output message file");
if (ftruncate(fileno(cmitmsg), 0))
die_errno("Could not truncate output message file at 
scissors");
-   still_looking = 1;
+   *still_looking = 1;
 
/*
 * We may have already read "secondary headers"; purge
@@ -777,16 +775,13 @@ static void handle_patch(const struct strbuf *line)
patch_lines++;
 }
 
-static void handle_filter(struct strbuf *line)
+static void handle_filter(struct strbuf *line, int *filter_stage, int 
*header_stage)
 {
-   static int filter = 0;
-
-   /* filter tells us which part we left off on */
-   switch (filter) {
+   switch (*filter_stage) {
case 0:
-   if (!handle_commit_msg(line))
+   if (!handle_commit_msg(line, header_stage))
break;
-   filter++;
+   (*filter_stage)++;
case 1:
handle_patch(line);
break;
@@ -802,7 +797,7 @@ static int find_boundary(void)
return 0;
 }
 
-static int handle_boundary(void)
+static int handle_boundary(int *filter_stage, int *header_stage)
 {
struct strbuf newline = STRBUF_INIT;
 
@@ -824,7 +819,7 @@ again:
"can't recover\n");
exit(1);
}
-   handle_filter();
+   handle_filter(, filter_stage, header_stage);
strbuf_release();
 
/* skip to the next boundary */
@@ -852,6 +847,8 @@ again:
 static void handle_body(void)
 {
struct strbuf prev = STRBUF_INIT;
+   int filter_stage = 0;
+   int header_stage = 1;
 
/* Skip up to the first boundary */
if (*content_top) {
@@ -864,10 +861,10 @@ static void handle_body(void)
if (*content_top && is_multipart_boundary()) {
/* flush any leftover */
if (prev.len) {
-   handle_filter();
+   handle_filter(, _stage, 
_stage);
strbuf_reset();
}
-   if (!handle_boundary())
+   if (!handle_boundary(_stage, _stage))
goto handle_body_out;
}
 
@@ -897,7 +894,7 @@ static void handle_body(void)
strbuf_addbuf(, sb);
break;
}
-   handle_filter(sb);
+   handle_filter(sb, _stage, _stage);
}
/*
 * The partial chunk is saved in "prev" and will be
@@ -907,7 +904,7 @@ static void handle_body(void)
break;
}
default:
-   handle_filter();
+   handle_filter(, _stage, _stage);
}
 
} 

[PATCH v3 00/34] libify mailinfo and call it directly from am

[Junio C Hamano]

During the discussion on the recent "git am" regression, I noticed
that the command reimplemented in C spawns one "mailsplit" and then
spawns "mailinfo" followed by "apply --index" to commit the changes
described in each message.  As there are platforms where spawning
subprocess via run_command() interface is heavy-weight, something
that is conceptually very simple like "mailinfo" is better called
directly inside the process---something that is lightweight and
frequently used is where the overhead of run_command() would be felt
most.

I think this round is ready for 'next'.  Relative to the previous
round, the changes are:

 * Editorial fixes on log messages.

 * The previous round leaked some fields in struct mailinfo upon
   completion (of course, inherited from the original that let the
   system clean them up upon process termination).  clear_mailinfo()
   has been enhanced to clear them.

 * The step to remove the global "line" variable has been split into
   multiple steps.

 * The step to move metainfo_charset to the struct has been split
   into two.

And here are the patches.

  mailinfo: remove a no-op call convert_to_utf8(it, "")
  mailinfo: fold decode_header_bq() into decode_header()
  mailinfo: fix an off-by-one error in the boundary stack
  mailinfo: explicitly close file handle to the patch output
  mailinfo: move handle_boundary() lower
  mailinfo: get rid of function-local static states

Mostly unchanged other than editorial fixes on their log messages.

  mailinfo: do not let handle_body() touch global "line" directly
  mailinfo: do not let handle_boundary() touch global "line" directly
  mailinfo: do not let find_boundary() touch global "line" directly
  mailinfo: move global "line" into mailinfo() function

After Stefan's review comments, I wanted to be really sure that this
conversion was correct.  Blindingly replacing the reference to the
global with a pointer passed from the callern is not sufficient to
ensure that the change is a no-op; the patches needed to show that
the pointer passed from the caller is always the global "line".  For
that, the patch [v2 7/31] needed to be split further into 3 patches
to convert three functions, each of which always is called with the
pointer that points at the global "line", in separate steps.

  mailinfo: introduce "struct mailinfo" to hold globals

The previous round was lazy and did not introduce clear_mailinfo()
until later, leaking the two strbuf moved into the struct.  The
original was leaking the global anyway, so it is not a big deal, but
this round adds corresponding clean-up as the patches move global
variables to the struct, which should make it harder to miss
forgotten clean-up.

  mailinfo: move keep_subject & keep_non_patch_bracket to struct mailinfo
  mailinfo: move global "FILE *fin, *fout" to struct mailinfo
  mailinfo: move filter/header stage to struct mailinfo
  mailinfo: move patch_lines to struct mailinfo
  mailinfo: move add_message_id and message_id to struct mailinfo
  mailinfo: move use_scissors and use_inbody_headers to struct mailinfo
  mailinfo: move metainfo_charset to struct mailinfo

Mostly unchanged, except for the clean-up in clear_mailinfo().

  mailinfo: move check for metainfo_charset to convert_to_utf8()

Eric's review noticed that the previous round conflated this step
in the previous step.  Separated the change to its own commit.

  mailinfo: move transfer_encoding to struct mailinfo
  mailinfo: move charset to struct mailinfo
  mailinfo: move cmitmsg and patchfile to struct mailinfo
  mailinfo: move [ps]_hdr_data to struct mailinfo
  mailinfo: move content/content_top to struct mailinfo

Mostly unchanged, except for the clean-up in clear_mailinfo().

  mailinfo: handle_commit_msg() shouldn't be called after finding patchbreak
  mailinfo: keep the parsed log message in a strbuf

These two were placed earlier in the series in the previous round,
but are not about moving globals to struct.  This round finishes
moving the globals to struct mailinfo before doing these two steps.

  mailinfo: move read_one_header_line() closer to its callers
  mailinfo: move check_header() after the helpers it uses
  mailinfo: move cleanup_space() before its users
  mailinfo: move definition of MAX_HDR_PARSED closer to its use

Mostly unchanged.  These are pure shuffling of functions and
variables to lose forward declarations and to allow easier reading
by grouping related things together.

  mailinfo: libify

Mostly pure code movement that is unchanged since v2.

  mailinfo: handle charset conversion errors in the caller
  mailinfo: remove calls to exit() and die() deep in the callchain
  am: make direct call to mailinfo

 Makefile |1 +
 builtin/am.c |   42 +-
 builtin/mailinfo.c   | 1137 ++
 builtin/mailinfo.c => mailinfo.c |  799 +--
 mailinfo.h   |   41 ++
 5 files changed, 505 

Re: [GIT PULL] l10n updates for 2.6 maint branch

[Junio C Hamano]

Jiang Xin  writes:

> Hi Junio,
>
> Please pull the following into the maint branch.  It includes l10n
> updates in Russian which missed the update window for 2.6.
>
> The following changes since commit 8d530c4d64ffcc853889f7b385f554d53db375ed:
>
>   Git 2.6-rc3 (2015-09-21 13:26:13 -0700)
>
> are available in the git repository at:
>
>   git://github.com/git-l10n/git-po maint
>
> for you to fetch changes up to 82aa9b751fe96c5e55c36819aedea3d47e98bb57:
>
>   l10n: ru.po: update Russian translation (2015-09-30 18:01:23 +0300)
>
> 
> Dimitriy Ryazantcev (1):
>   l10n: ru.po: update Russian translation
>
>  po/ru.po | 3550 
> ++
>  1 file changed, 1967 insertions(+), 1583 deletions(-)

Thanks, pulled directly to 'maint'.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] worktree: usage: denote as optional with 'add'

[Junio C Hamano]

Sidhant Sharma  writes:

> Although 1eb07d8 (worktree: add: auto-vivify new branch when
>  is omitted, 2015-07-06) updated the documentation when
>  became optional, it neglected to update the in-code
> usage message. Fix this oversight.
>
> Reported-by: ch3co...@gmail.com
> Signed-off-by: Sidhant Sharma 
> ---

Thanks.

I'll add "Helped-by: Eric Sunshine " and
queue.

>  builtin/worktree.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 71bb770..33d2d37 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -10,7 +10,7 @@
>  #include "refs.h"
>
>  static const char * const worktree_usage[] = {
> - N_("git worktree add []  "),
> + N_("git worktree add []  []"),
>   N_("git worktree prune []"),
>   NULL
>  };
> --
> 2.6.2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 20/34] mailinfo: move transfer_encoding to struct mailinfo

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 27 ++-
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 737d0fc..18781b7 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -23,14 +23,14 @@ struct mailinfo {
const char *metainfo_charset;
 
char *message_id;
+   enum  {
+   TE_DONTCARE, TE_QP, TE_BASE64
+   } transfer_encoding;
int patch_lines;
int filter_stage; /* still reading log or are we copying patch? */
int header_stage; /* still checking in-body headers? */
 };
 
-static enum  {
-   TE_DONTCARE, TE_QP, TE_BASE64
-} transfer_encoding;
 
 static struct strbuf charset = STRBUF_INIT;
 
@@ -214,14 +214,15 @@ static void handle_message_id(struct mailinfo *mi, const 
struct strbuf *line)
mi->message_id = strdup(line->buf);
 }
 
-static void handle_content_transfer_encoding(const struct strbuf *line)
+static void handle_content_transfer_encoding(struct mailinfo *mi,
+const struct strbuf *line)
 {
if (strcasestr(line->buf, "base64"))
-   transfer_encoding = TE_BASE64;
+   mi->transfer_encoding = TE_BASE64;
else if (strcasestr(line->buf, "quoted-printable"))
-   transfer_encoding = TE_QP;
+   mi->transfer_encoding = TE_QP;
else
-   transfer_encoding = TE_DONTCARE;
+   mi->transfer_encoding = TE_DONTCARE;
 }
 
 static int is_multipart_boundary(const struct strbuf *line)
@@ -356,7 +357,7 @@ static int check_header(struct mailinfo *mi,
len = strlen("Content-Transfer-Encoding: ");
strbuf_add(, line->buf + len, line->len - len);
decode_header(mi, );
-   handle_content_transfer_encoding();
+   handle_content_transfer_encoding(mi, );
ret = 1;
goto check_header_out;
}
@@ -615,11 +616,11 @@ release_return:
strbuf_release();
 }
 
-static void decode_transfer_encoding(struct strbuf *line)
+static void decode_transfer_encoding(struct mailinfo *mi, struct strbuf *line)
 {
struct strbuf *ret;
 
-   switch (transfer_encoding) {
+   switch (mi->transfer_encoding) {
case TE_QP:
ret = decode_q_segment(line, 0);
break;
@@ -838,7 +839,7 @@ again:
}
 
/* set some defaults */
-   transfer_encoding = TE_DONTCARE;
+   mi->transfer_encoding = TE_DONTCARE;
strbuf_reset();
 
/* slurp in this section's info */
@@ -876,9 +877,9 @@ static void handle_body(struct mailinfo *mi, struct strbuf 
*line)
}
 
/* Unwrap transfer encoding */
-   decode_transfer_encoding(line);
+   decode_transfer_encoding(mi, line);
 
-   switch (transfer_encoding) {
+   switch (mi->transfer_encoding) {
case TE_BASE64:
case TE_QP:
{
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 29/34] mailinfo: move cleanup_space() before its users

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 25 +++--
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 6fc6aa8..104d3d9 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -41,8 +41,17 @@ struct mailinfo {
 
 #define MAX_HDR_PARSED 10
 
-static void cleanup_space(struct strbuf *sb);
-
+static void cleanup_space(struct strbuf *sb)
+{
+   size_t pos, cnt;
+   for (pos = 0; pos < sb->len; pos++) {
+   if (isspace(sb->buf[pos])) {
+   sb->buf[pos] = ' ';
+   for (cnt = 0; isspace(sb->buf[pos + cnt + 1]); cnt++);
+   strbuf_remove(sb, pos + 1, cnt);
+   }
+   }
+}
 
 static void get_sane_name(struct strbuf *out, struct strbuf *name, struct 
strbuf *email)
 {
@@ -281,18 +290,6 @@ static void cleanup_subject(struct mailinfo *mi, struct 
strbuf *subject)
strbuf_trim(subject);
 }
 
-static void cleanup_space(struct strbuf *sb)
-{
-   size_t pos, cnt;
-   for (pos = 0; pos < sb->len; pos++) {
-   if (isspace(sb->buf[pos])) {
-   sb->buf[pos] = ' ';
-   for (cnt = 0; isspace(sb->buf[pos + cnt + 1]); cnt++);
-   strbuf_remove(sb, pos + 1, cnt);
-   }
-   }
-}
-
 static const char *header[MAX_HDR_PARSED] = {
"From","Subject","Date",
 };
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 18/34] mailinfo: move metainfo_charset to struct mailinfo

[Junio C Hamano]

This requires us to pass the struct down to decode_header() and
convert_to_utf8() callchain.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 40 
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 2c8f249..26fd9b1 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -9,8 +9,6 @@
 
 static FILE *cmitmsg, *patchfile;
 
-static const char *metainfo_charset;
-
 struct mailinfo {
FILE *input;
FILE *output;
@@ -22,6 +20,7 @@ struct mailinfo {
int add_message_id;
int use_scissors;
int use_inbody_headers; /* defaults to 1 */
+   const char *metainfo_charset;
 
char *message_id;
int patch_lines;
@@ -293,7 +292,7 @@ static void cleanup_space(struct strbuf *sb)
}
 }
 
-static void decode_header(struct strbuf *line);
+static void decode_header(struct mailinfo *mi, struct strbuf *line);
 static const char *header[MAX_HDR_PARSED] = {
"From","Subject","Date",
 };
@@ -336,7 +335,7 @@ static int check_header(struct mailinfo *mi,
 * normalize the meta information to utf8.
 */
strbuf_add(, line->buf + len + 2, line->len - len - 
2);
-   decode_header();
+   decode_header(mi, );
handle_header(_data[i], );
ret = 1;
goto check_header_out;
@@ -347,7 +346,7 @@ static int check_header(struct mailinfo *mi,
if (cmp_header(line, "Content-Type")) {
len = strlen("Content-Type: ");
strbuf_add(, line->buf + len, line->len - len);
-   decode_header();
+   decode_header(mi, );
strbuf_insert(, 0, "Content-Type: ", len);
handle_content_type();
ret = 1;
@@ -356,7 +355,7 @@ static int check_header(struct mailinfo *mi,
if (cmp_header(line, "Content-Transfer-Encoding")) {
len = strlen("Content-Transfer-Encoding: ");
strbuf_add(, line->buf + len, line->len - len);
-   decode_header();
+   decode_header(mi, );
handle_content_transfer_encoding();
ret = 1;
goto check_header_out;
@@ -364,7 +363,7 @@ static int check_header(struct mailinfo *mi,
if (cmp_header(line, "Message-Id")) {
len = strlen("Message-Id: ");
strbuf_add(, line->buf + len, line->len - len);
-   decode_header();
+   decode_header(mi, );
handle_message_id(mi, );
ret = 1;
goto check_header_out;
@@ -520,23 +519,24 @@ static struct strbuf *decode_b_segment(const struct 
strbuf *b_seg)
return out;
 }
 
-static void convert_to_utf8(struct strbuf *line, const char *charset)
+static void convert_to_utf8(struct mailinfo *mi,
+   struct strbuf *line, const char *charset)
 {
char *out;
 
if (!charset || !*charset)
return;
 
-   if (same_encoding(metainfo_charset, charset))
+   if (same_encoding(mi->metainfo_charset, charset))
return;
-   out = reencode_string(line->buf, metainfo_charset, charset);
+   out = reencode_string(line->buf, mi->metainfo_charset, charset);
if (!out)
die("cannot convert from %s to %s",
-   charset, metainfo_charset);
+   charset, mi->metainfo_charset);
strbuf_attach(line, out, strlen(out), strlen(out));
 }
 
-static void decode_header(struct strbuf *it)
+static void decode_header(struct mailinfo *mi, struct strbuf *it)
 {
char *in, *ep, *cp;
struct strbuf outbuf = STRBUF_INIT, *dec;
@@ -599,8 +599,8 @@ static void decode_header(struct strbuf *it)
dec = decode_q_segment(, 1);
break;
}
-   if (metainfo_charset)
-   convert_to_utf8(dec, charset_q.buf);
+   if (mi->metainfo_charset)
+   convert_to_utf8(mi, dec, charset_q.buf);
 
strbuf_addbuf(, dec);
strbuf_release(dec);
@@ -745,8 +745,8 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
mi->header_stage = 0;
 
/* normalize the log message to UTF-8. */
-   if (metainfo_charset)
-   convert_to_utf8(line, charset.buf);
+   if (mi->metainfo_charset)
+   convert_to_utf8(mi, line, charset.buf);
 
if (mi->use_scissors && is_scissors_line(line)) {
int i;
@@ -1055,7 +1055,7 @@ int cmd_mailinfo(int argc, const char **argv, const char 
*prefix)
setup_mailinfo();
 
def_charset = get_commit_output_encoding();
-   metainfo_charset = def_charset;
+   

[PATCH v3 21/34] mailinfo: move charset to struct mailinfo

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 18781b7..810d132 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -22,6 +22,7 @@ struct mailinfo {
int use_inbody_headers; /* defaults to 1 */
const char *metainfo_charset;
 
+   struct strbuf charset;
char *message_id;
enum  {
TE_DONTCARE, TE_QP, TE_BASE64
@@ -31,9 +32,6 @@ struct mailinfo {
int header_stage; /* still checking in-body headers? */
 };
 
-
-static struct strbuf charset = STRBUF_INIT;
-
 static struct strbuf **p_hdr_data, **s_hdr_data;
 
 #define MAX_HDR_PARSED 10
@@ -186,7 +184,7 @@ static struct strbuf *content[MAX_BOUNDARIES];
 
 static struct strbuf **content_top = content;
 
-static void handle_content_type(struct strbuf *line)
+static void handle_content_type(struct mailinfo *mi, struct strbuf *line)
 {
struct strbuf *boundary = xmalloc(sizeof(struct strbuf));
strbuf_init(boundary, line->len);
@@ -200,7 +198,7 @@ static void handle_content_type(struct strbuf *line)
*content_top = boundary;
boundary = NULL;
}
-   slurp_attr(line->buf, "charset=", );
+   slurp_attr(line->buf, "charset=", >charset);
 
if (boundary) {
strbuf_release(boundary);
@@ -349,7 +347,7 @@ static int check_header(struct mailinfo *mi,
strbuf_add(, line->buf + len, line->len - len);
decode_header(mi, );
strbuf_insert(, 0, "Content-Type: ", len);
-   handle_content_type();
+   handle_content_type(mi, );
ret = 1;
goto check_header_out;
}
@@ -745,7 +743,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
mi->header_stage = 0;
 
/* normalize the log message to UTF-8. */
-   convert_to_utf8(mi, line, charset.buf);
+   convert_to_utf8(mi, line, mi->charset.buf);
 
if (mi->use_scissors && is_scissors_line(line)) {
int i;
@@ -840,7 +838,7 @@ again:
 
/* set some defaults */
mi->transfer_encoding = TE_DONTCARE;
-   strbuf_reset();
+   strbuf_reset(>charset);
 
/* slurp in this section's info */
while (read_one_header_line(line, mi->input))
@@ -1027,6 +1025,7 @@ static void setup_mailinfo(struct mailinfo *mi)
memset(mi, 0, sizeof(*mi));
strbuf_init(>name, 0);
strbuf_init(>email, 0);
+   strbuf_init(>charset, 0);
mi->header_stage = 1;
mi->use_inbody_headers = 1;
git_config(git_mailinfo_config, );
@@ -1036,6 +1035,7 @@ static void clear_mailinfo(struct mailinfo *mi)
 {
strbuf_release(>name);
strbuf_release(>email);
+   strbuf_release(>charset);
free(mi->message_id);
 }
 
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 31/34] mailinfo: libify

[Junio C Hamano]

Move the bulk of the code from builtin/mailinfo.c to mailinfo.c
so that new callers can start calling mailinfo() directly.

Note that a few calls to exit() and die() need to be cleaned up
for the API to be truly useful, which will come in later steps.

Signed-off-by: Junio C Hamano 
---
 Makefile |1 +
 builtin/mailinfo.c   | 1167 ++
 builtin/mailinfo.c => mailinfo.c |   96 +---
 mailinfo.h   |   40 ++
 4 files changed, 106 insertions(+), 1198 deletions(-)
 rewrite builtin/mailinfo.c (93%)
 copy builtin/mailinfo.c => mailinfo.c (90%)
 create mode 100644 mailinfo.h

diff --git a/Makefile b/Makefile
index 8d5df7e..7dd3bff 100644
--- a/Makefile
+++ b/Makefile
@@ -726,6 +726,7 @@ LIB_OBJS += list-objects.o
 LIB_OBJS += ll-merge.o
 LIB_OBJS += lockfile.o
 LIB_OBJS += log-tree.o
+LIB_OBJS += mailinfo.o
 LIB_OBJS += mailmap.o
 LIB_OBJS += match-trees.o
 LIB_OBJS += merge.o
diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
dissimilarity index 93%
index 4eabc11..f6df274 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -1,1106 +1,61 @@
-/*
- * Another stupid program, this one parsing the headers of an
- * email to figure out authorship and subject
- */
-#include "cache.h"
-#include "builtin.h"
-#include "utf8.h"
-#include "strbuf.h"
-
-#define MAX_BOUNDARIES 5
-
-struct mailinfo {
-   FILE *input;
-   FILE *output;
-   FILE *patchfile;
-
-   struct strbuf name;
-   struct strbuf email;
-   int keep_subject;
-   int keep_non_patch_brackets_in_subject;
-   int add_message_id;
-   int use_scissors;
-   int use_inbody_headers; /* defaults to 1 */
-   const char *metainfo_charset;
-
-   struct strbuf *content[MAX_BOUNDARIES];
-   struct strbuf **content_top;
-   struct strbuf charset;
-   char *message_id;
-   enum  {
-   TE_DONTCARE, TE_QP, TE_BASE64
-   } transfer_encoding;
-   int patch_lines;
-   int filter_stage; /* still reading log or are we copying patch? */
-   int header_stage; /* still checking in-body headers? */
-   struct strbuf **p_hdr_data;
-   struct strbuf **s_hdr_data;
-
-   struct strbuf log_message;
-};
-
-static void cleanup_space(struct strbuf *sb)
-{
-   size_t pos, cnt;
-   for (pos = 0; pos < sb->len; pos++) {
-   if (isspace(sb->buf[pos])) {
-   sb->buf[pos] = ' ';
-   for (cnt = 0; isspace(sb->buf[pos + cnt + 1]); cnt++);
-   strbuf_remove(sb, pos + 1, cnt);
-   }
-   }
-}
-
-static void get_sane_name(struct strbuf *out, struct strbuf *name, struct 
strbuf *email)
-{
-   struct strbuf *src = name;
-   if (name->len < 3 || 60 < name->len || strchr(name->buf, '@') ||
-   strchr(name->buf, '<') || strchr(name->buf, '>'))
-   src = email;
-   else if (name == out)
-   return;
-   strbuf_reset(out);
-   strbuf_addbuf(out, src);
-}
-
-static void parse_bogus_from(struct mailinfo *mi, const struct strbuf *line)
-{
-   /* John Doe  */
-
-   char *bra, *ket;
-   /* This is fallback, so do not bother if we already have an
-* e-mail address.
-*/
-   if (mi->email.len)
-   return;
-
-   bra = strchr(line->buf, '<');
-   if (!bra)
-   return;
-   ket = strchr(bra, '>');
-   if (!ket)
-   return;
-
-   strbuf_reset(>email);
-   strbuf_add(>email, bra + 1, ket - bra - 1);
-
-   strbuf_reset(>name);
-   strbuf_add(>name, line->buf, bra - line->buf);
-   strbuf_trim(>name);
-   get_sane_name(>name, >name, >email);
-}
-
-static void handle_from(struct mailinfo *mi, const struct strbuf *from)
-{
-   char *at;
-   size_t el;
-   struct strbuf f;
-
-   strbuf_init(, from->len);
-   strbuf_addbuf(, from);
-
-   at = strchr(f.buf, '@');
-   if (!at) {
-   parse_bogus_from(mi, from);
-   return;
-   }
-
-   /*
-* If we already have one email, don't take any confusing lines
-*/
-   if (mi->email.len && strchr(at + 1, '@')) {
-   strbuf_release();
-   return;
-   }
-
-   /* Pick up the string around '@', possibly delimited with <>
-* pair; that is the email part.
-*/
-   while (at > f.buf) {
-   char c = at[-1];
-   if (isspace(c))
-   break;
-   if (c == '<') {
-   at[-1] = ' ';
-   break;
-   }
-   at--;
-   }
-   el = strcspn(at, " \n\t\r\v\f>");
-   strbuf_reset(>email);
-   strbuf_add(>email, at, el);
-   strbuf_remove(, at - f.buf, el + (at[el] ? 1 : 0));
-
-   /* The remainder is name.  It could be
-*
-* - "John Doe " 

[PATCH v3 32/34] mailinfo: handle charset conversion errors in the caller

[Junio C Hamano]

Instead of dying in convert_to_utf8(), just report an error and let
the callers handle it.  Between the two callers:

 - decode_header() silently punts when it cannot parse a broken
   RFC2047 encoded text (e.g. when it sees anything other than B or
   Q after it sees "=?") by jumping to release_return,
   returning the string it successfully parsed out so far, to the
   caller.  A piece of string that convert_to_utf8() cannot handle
   can be treated the same way.

 - handle_commit_msg() doesn't cope with a malformed line well, so
   die there for now.  We'll lift this even higher in later changes
   in this series.

Signed-off-by: Junio C Hamano 
---
 mailinfo.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/mailinfo.c b/mailinfo.c
index df6cfef..4fbf38f 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -344,21 +344,22 @@ static struct strbuf *decode_b_segment(const struct 
strbuf *b_seg)
return out;
 }
 
-static void convert_to_utf8(struct mailinfo *mi,
-   struct strbuf *line, const char *charset)
+static int convert_to_utf8(struct mailinfo *mi,
+  struct strbuf *line, const char *charset)
 {
char *out;
 
if (!mi->metainfo_charset || !charset || !*charset)
-   return;
+   return 0;
 
if (same_encoding(mi->metainfo_charset, charset))
-   return;
+   return 0;
out = reencode_string(line->buf, mi->metainfo_charset, charset);
if (!out)
-   die("cannot convert from %s to %s",
-   charset, mi->metainfo_charset);
+   return error("cannot convert from %s to %s",
+charset, mi->metainfo_charset);
strbuf_attach(line, out, strlen(out), strlen(out));
+   return 0;
 }
 
 static void decode_header(struct mailinfo *mi, struct strbuf *it)
@@ -424,7 +425,8 @@ static void decode_header(struct mailinfo *mi, struct 
strbuf *it)
dec = decode_q_segment(, 1);
break;
}
-   convert_to_utf8(mi, dec, charset_q.buf);
+   if (convert_to_utf8(mi, dec, charset_q.buf))
+   goto release_return;
 
strbuf_addbuf(, dec);
strbuf_release(dec);
@@ -637,7 +639,8 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
mi->header_stage = 0;
 
/* normalize the log message to UTF-8. */
-   convert_to_utf8(mi, line, mi->charset.buf);
+   if (convert_to_utf8(mi, line, mi->charset.buf))
+   exit(128);
 
if (mi->use_scissors && is_scissors_line(line)) {
int i;
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 23/34] mailinfo: move [ps]_hdr_data to struct mailinfo

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 37 +++--
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index b8b94d6..2c194da 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -30,10 +30,10 @@ struct mailinfo {
int patch_lines;
int filter_stage; /* still reading log or are we copying patch? */
int header_stage; /* still checking in-body headers? */
+   struct strbuf **p_hdr_data;
+   struct strbuf **s_hdr_data;
 };
 
-static struct strbuf **p_hdr_data, **s_hdr_data;
-
 #define MAX_HDR_PARSED 10
 #define MAX_BOUNDARIES 5
 
@@ -733,7 +733,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
}
 
if (mi->use_inbody_headers && mi->header_stage) {
-   mi->header_stage = check_header(mi, line, s_hdr_data, 0);
+   mi->header_stage = check_header(mi, line, mi->s_hdr_data, 0);
if (mi->header_stage)
return 0;
} else
@@ -758,9 +758,9 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
 * them to give ourselves a clean restart.
 */
for (i = 0; header[i]; i++) {
-   if (s_hdr_data[i])
-   strbuf_release(s_hdr_data[i]);
-   s_hdr_data[i] = NULL;
+   if (mi->s_hdr_data[i])
+   strbuf_release(mi->s_hdr_data[i]);
+   mi->s_hdr_data[i] = NULL;
}
return 0;
}
@@ -842,7 +842,7 @@ again:
 
/* slurp in this section's info */
while (read_one_header_line(line, mi->input))
-   check_header(mi, line, p_hdr_data, 0);
+   check_header(mi, line, mi->p_hdr_data, 0);
 
strbuf_release();
/* replenish line */
@@ -943,10 +943,10 @@ static void handle_info(struct mailinfo *mi)
 
for (i = 0; header[i]; i++) {
/* only print inbody headers if we output a patch file */
-   if (mi->patch_lines && s_hdr_data[i])
-   hdr = s_hdr_data[i];
-   else if (p_hdr_data[i])
-   hdr = p_hdr_data[i];
+   if (mi->patch_lines && mi->s_hdr_data[i])
+   hdr = mi->s_hdr_data[i];
+   else if (mi->p_hdr_data[i])
+   hdr = mi->p_hdr_data[i];
else
continue;
 
@@ -986,8 +986,8 @@ static int mailinfo(struct mailinfo *mi, const char *msg, 
const char *patch)
return -1;
}
 
-   p_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(*p_hdr_data));
-   s_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(*s_hdr_data));
+   mi->p_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(*(mi->p_hdr_data)));
+   mi->s_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(*(mi->s_hdr_data)));
 
do {
peek = fgetc(mi->input);
@@ -996,7 +996,7 @@ static int mailinfo(struct mailinfo *mi, const char *msg, 
const char *patch)
 
/* process the email header */
while (read_one_header_line(, mi->input))
-   check_header(mi, , p_hdr_data, 1);
+   check_header(mi, , mi->p_hdr_data, 1);
 
handle_body(mi, );
fclose(mi->patchfile);
@@ -1033,10 +1033,19 @@ static void setup_mailinfo(struct mailinfo *mi)
 
 static void clear_mailinfo(struct mailinfo *mi)
 {
+   int i;
+
strbuf_release(>name);
strbuf_release(>email);
strbuf_release(>charset);
free(mi->message_id);
+
+   for (i = 0; mi->p_hdr_data[i]; i++)
+   strbuf_release(mi->p_hdr_data[i]);
+   free(mi->p_hdr_data);
+   for (i = 0; mi->s_hdr_data[i]; i++)
+   strbuf_release(mi->s_hdr_data[i]);
+   free(mi->s_hdr_data);
 }
 
 static const char mailinfo_usage[] =
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 17/34] mailinfo: move use_scissors and use_inbody_headers to struct mailinfo

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 23 +--
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index c0522f2..2c8f249 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -20,6 +20,8 @@ struct mailinfo {
int keep_subject;
int keep_non_patch_brackets_in_subject;
int add_message_id;
+   int use_scissors;
+   int use_inbody_headers; /* defaults to 1 */
 
char *message_id;
int patch_lines;
@@ -34,8 +36,6 @@ static enum  {
 static struct strbuf charset = STRBUF_INIT;
 
 static struct strbuf **p_hdr_data, **s_hdr_data;
-static int use_scissors;
-static int use_inbody_headers = 1;
 
 #define MAX_HDR_PARSED 10
 #define MAX_BOUNDARIES 5
@@ -734,7 +734,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
return 0;
}
 
-   if (use_inbody_headers && mi->header_stage) {
+   if (mi->use_inbody_headers && mi->header_stage) {
mi->header_stage = check_header(mi, line, s_hdr_data, 0);
if (mi->header_stage)
return 0;
@@ -748,7 +748,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
if (metainfo_charset)
convert_to_utf8(line, charset.buf);
 
-   if (use_scissors && is_scissors_line(line)) {
+   if (mi->use_scissors && is_scissors_line(line)) {
int i;
if (fseek(cmitmsg, 0L, SEEK_SET))
die_errno("Could not rewind output message file");
@@ -1009,12 +1009,14 @@ static int mailinfo(struct mailinfo *mi, const char 
*msg, const char *patch)
return 0;
 }
 
-static int git_mailinfo_config(const char *var, const char *value, void 
*unused)
+static int git_mailinfo_config(const char *var, const char *value, void *mi_)
 {
+   struct mailinfo *mi = mi_;
+
if (!starts_with(var, "mailinfo."))
-   return git_default_config(var, value, unused);
+   return git_default_config(var, value, NULL);
if (!strcmp(var, "mailinfo.scissors")) {
-   use_scissors = git_config_bool(var, value);
+   mi->use_scissors = git_config_bool(var, value);
return 0;
}
/* perhaps others here */
@@ -1027,6 +1029,7 @@ static void setup_mailinfo(struct mailinfo *mi)
strbuf_init(>name, 0);
strbuf_init(>email, 0);
mi->header_stage = 1;
+   mi->use_inbody_headers = 1;
git_config(git_mailinfo_config, );
 }
 
@@ -1068,11 +1071,11 @@ int cmd_mailinfo(int argc, const char **argv, const 
char *prefix)
else if (starts_with(argv[1], "--encoding="))
metainfo_charset = argv[1] + 11;
else if (!strcmp(argv[1], "--scissors"))
-   use_scissors = 1;
+   mi.use_scissors = 1;
else if (!strcmp(argv[1], "--no-scissors"))
-   use_scissors = 0;
+   mi.use_scissors = 0;
else if (!strcmp(argv[1], "--no-inbody-headers"))
-   use_inbody_headers = 0;
+   mi.use_inbody_headers = 0;
else
usage(mailinfo_usage);
argc--; argv++;
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 26/34] mailinfo: keep the parsed log message in a strbuf

[Junio C Hamano]

When mailinfo() is eventually libified, the calling "git am" still
will have to write out the log message in the "msg" file for hooks
and other users of the information, but it does not have to reopen
and reread what it wrote earlier if the function kept it in a strbuf.

This also removes the need for seeking and truncating the output
file when we see a scissors mark in the input, which in turn allows
us to lose two callsites of die_errno().

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 28 
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 286eda0..81bb3c7 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -12,7 +12,6 @@
 struct mailinfo {
FILE *input;
FILE *output;
-   FILE *cmitmsg;
FILE *patchfile;
 
struct strbuf name;
@@ -36,6 +35,8 @@ struct mailinfo {
int header_stage; /* still checking in-body headers? */
struct strbuf **p_hdr_data;
struct strbuf **s_hdr_data;
+
+   struct strbuf log_message;
 };
 
 #define MAX_HDR_PARSED 10
@@ -747,10 +748,8 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
 
if (mi->use_scissors && is_scissors_line(line)) {
int i;
-   if (fseek(mi->cmitmsg, 0L, SEEK_SET))
-   die_errno("Could not rewind output message file");
-   if (ftruncate(fileno(mi->cmitmsg), 0))
-   die_errno("Could not truncate output message file at 
scissors");
+
+   strbuf_setlen(>log_message, 0);
mi->header_stage = 1;
 
/*
@@ -767,13 +766,12 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
 
if (patchbreak(line)) {
if (mi->message_id)
-   fprintf(mi->cmitmsg, "Message-Id: %s\n", 
mi->message_id);
-   fclose(mi->cmitmsg);
-   mi->cmitmsg = NULL;
+   strbuf_addf(>log_message,
+   "Message-Id: %s\n", mi->message_id);
return 1;
}
 
-   fputs(line->buf, mi->cmitmsg);
+   strbuf_addbuf(>log_message, line);
return 0;
 }
 
@@ -971,18 +969,19 @@ static void handle_info(struct mailinfo *mi)
 
 static int mailinfo(struct mailinfo *mi, const char *msg, const char *patch)
 {
+   FILE *cmitmsg;
int peek;
struct strbuf line = STRBUF_INIT;
 
-   mi->cmitmsg = fopen(msg, "w");
-   if (!mi->cmitmsg) {
+   cmitmsg = fopen(msg, "w");
+   if (!cmitmsg) {
perror(msg);
return -1;
}
mi->patchfile = fopen(patch, "w");
if (!mi->patchfile) {
perror(patch);
-   fclose(mi->cmitmsg);
+   fclose(cmitmsg);
return -1;
}
 
@@ -999,6 +998,8 @@ static int mailinfo(struct mailinfo *mi, const char *msg, 
const char *patch)
check_header(mi, , mi->p_hdr_data, 1);
 
handle_body(mi, );
+   fwrite(mi->log_message.buf, 1, mi->log_message.len, cmitmsg);
+   fclose(cmitmsg);
fclose(mi->patchfile);
 
handle_info(mi);
@@ -1026,6 +1027,7 @@ static void setup_mailinfo(struct mailinfo *mi)
strbuf_init(>name, 0);
strbuf_init(>email, 0);
strbuf_init(>charset, 0);
+   strbuf_init(>log_message, 0);
mi->header_stage = 1;
mi->use_inbody_headers = 1;
mi->content_top = mi->content;
@@ -1052,6 +1054,8 @@ static void clear_mailinfo(struct mailinfo *mi)
free(*(mi->content_top));
mi->content_top--;
}
+
+   strbuf_release(>log_message);
 }
 
 static const char mailinfo_usage[] =
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 15/34] mailinfo: move patch_lines to struct mailinfo

[Junio C Hamano]

This one is trivial thanks to previous steps that started passing
the structure throughout the input codepaths.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 5823ae5..8bd76c6 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -20,6 +20,7 @@ struct mailinfo {
int keep_subject;
int keep_non_patch_brackets_in_subject;
 
+   int patch_lines;
int filter_stage; /* still reading log or are we copying patch? */
int header_stage; /* still checking in-body headers? */
 };
@@ -30,7 +31,6 @@ static enum  {
 } transfer_encoding;
 
 static struct strbuf charset = STRBUF_INIT;
-static int patch_lines;
 
 static struct strbuf **p_hdr_data, **s_hdr_data;
 static int use_scissors;
@@ -778,10 +778,10 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
return 0;
 }
 
-static void handle_patch(const struct strbuf *line)
+static void handle_patch(struct mailinfo *mi, const struct strbuf *line)
 {
fwrite(line->buf, 1, line->len, patchfile);
-   patch_lines++;
+   mi->patch_lines++;
 }
 
 static void handle_filter(struct mailinfo *mi, struct strbuf *line)
@@ -792,7 +792,7 @@ static void handle_filter(struct mailinfo *mi, struct 
strbuf *line)
break;
mi->filter_stage++;
case 1:
-   handle_patch(line);
+   handle_patch(mi, line);
break;
}
 }
@@ -944,7 +944,7 @@ static void handle_info(struct mailinfo *mi)
 
for (i = 0; header[i]; i++) {
/* only print inbody headers if we output a patch file */
-   if (patch_lines && s_hdr_data[i])
+   if (mi->patch_lines && s_hdr_data[i])
hdr = s_hdr_data[i];
else if (p_hdr_data[i])
hdr = p_hdr_data[i];
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 33/34] mailinfo: remove calls to exit() and die() deep in the callchain

[Junio C Hamano]

The top-level mailinfo() would instead punt when the code in the
deeper part of the callchain detects an unrecoverable error in the
input.

Signed-off-by: Junio C Hamano 
---
 mailinfo.c | 30 ++
 mailinfo.h |  1 +
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/mailinfo.c b/mailinfo.c
index 4fbf38f..9d009fa 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -163,8 +163,10 @@ static void handle_content_type(struct mailinfo *mi, 
struct strbuf *line)
if (slurp_attr(line->buf, "boundary=", boundary)) {
strbuf_insert(boundary, 0, "--", 2);
if (++mi->content_top >= >content[MAX_BOUNDARIES]) {
-   fprintf(stderr, "Too many boundaries to handle\n");
-   exit(1);
+   error("Too many boundaries to handle");
+   mi->input_error = -1;
+   mi->content_top = >content[MAX_BOUNDARIES] - 1;
+   return;
}
*(mi->content_top) = boundary;
boundary = NULL;
@@ -355,9 +357,11 @@ static int convert_to_utf8(struct mailinfo *mi,
if (same_encoding(mi->metainfo_charset, charset))
return 0;
out = reencode_string(line->buf, mi->metainfo_charset, charset);
-   if (!out)
+   if (!out) {
+   mi->input_error = -1;
return error("cannot convert from %s to %s",
 charset, mi->metainfo_charset);
+   }
strbuf_attach(line, out, strlen(out), strlen(out));
return 0;
 }
@@ -367,6 +371,7 @@ static void decode_header(struct mailinfo *mi, struct 
strbuf *it)
char *in, *ep, *cp;
struct strbuf outbuf = STRBUF_INIT, *dec;
struct strbuf charset_q = STRBUF_INIT, piecebuf = STRBUF_INIT;
+   int found_error = 1; /* pessimism */
 
in = it->buf;
while (in - it->buf <= it->len && (ep = strstr(in, "=?")) != NULL) {
@@ -436,10 +441,14 @@ static void decode_header(struct mailinfo *mi, struct 
strbuf *it)
strbuf_addstr(, in);
strbuf_reset(it);
strbuf_addbuf(it, );
+   found_error = 0;
 release_return:
strbuf_release();
strbuf_release(_q);
strbuf_release();
+
+   if (found_error)
+   mi->input_error = -1;
 }
 
 static int check_header(struct mailinfo *mi,
@@ -640,7 +649,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
 
/* normalize the log message to UTF-8. */
if (convert_to_utf8(mi, line, mi->charset.buf))
-   exit(128);
+   return 0; /* mi->input_error already set */
 
if (mi->use_scissors && is_scissors_line(line)) {
int i;
@@ -783,12 +792,15 @@ again:
   will fail first.  But just in case..
 */
if (--mi->content_top < mi->content) {
-   fprintf(stderr, "Detected mismatched boundaries, "
-   "can't recover\n");
-   exit(1);
+   error("Detected mismatched boundaries, can't recover");
+   mi->input_error = -1;
+   mi->content_top = mi->content;
+   return 0;
}
handle_filter(mi, );
strbuf_release();
+   if (mi->input_error)
+   return 0;
 
/* skip to the next boundary */
if (!find_boundary(mi, line))
@@ -873,6 +885,8 @@ static void handle_body(struct mailinfo *mi, struct strbuf 
*line)
handle_filter(mi, line);
}
 
+   if (mi->input_error)
+   break;
} while (!strbuf_getwholeline(line, mi->input, '\n'));
 
 handle_body_out:
@@ -966,7 +980,7 @@ int mailinfo(struct mailinfo *mi, const char *msg, const 
char *patch)
 
handle_info(mi);
 
-   return 0;
+   return mi->input_error;
 }
 
 static int git_mailinfo_config(const char *var, const char *value, void *mi_)
diff --git a/mailinfo.h b/mailinfo.h
index 27841be..5bf257d 100644
--- a/mailinfo.h
+++ b/mailinfo.h
@@ -31,6 +31,7 @@ struct mailinfo {
struct strbuf **s_hdr_data;
 
struct strbuf log_message;
+   int input_error;
 };
 
 extern void setup_mailinfo(struct mailinfo *);
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 22/34] mailinfo: move cmitmsg and patchfile to struct mailinfo

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 32 
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 810d132..b8b94d6 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -7,11 +7,11 @@
 #include "utf8.h"
 #include "strbuf.h"
 
-static FILE *cmitmsg, *patchfile;
-
 struct mailinfo {
FILE *input;
FILE *output;
+   FILE *cmitmsg;
+   FILE *patchfile;
 
struct strbuf name;
struct strbuf email;
@@ -724,7 +724,7 @@ static int is_scissors_line(const struct strbuf *line)
 
 static int handle_commit_msg(struct mailinfo *mi, struct strbuf *line)
 {
-   if (!cmitmsg)
+   if (!mi->cmitmsg)
return 0;
 
if (mi->header_stage) {
@@ -747,9 +747,9 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
 
if (mi->use_scissors && is_scissors_line(line)) {
int i;
-   if (fseek(cmitmsg, 0L, SEEK_SET))
+   if (fseek(mi->cmitmsg, 0L, SEEK_SET))
die_errno("Could not rewind output message file");
-   if (ftruncate(fileno(cmitmsg), 0))
+   if (ftruncate(fileno(mi->cmitmsg), 0))
die_errno("Could not truncate output message file at 
scissors");
mi->header_stage = 1;
 
@@ -767,19 +767,19 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
 
if (patchbreak(line)) {
if (mi->message_id)
-   fprintf(cmitmsg, "Message-Id: %s\n", mi->message_id);
-   fclose(cmitmsg);
-   cmitmsg = NULL;
+   fprintf(mi->cmitmsg, "Message-Id: %s\n", 
mi->message_id);
+   fclose(mi->cmitmsg);
+   mi->cmitmsg = NULL;
return 1;
}
 
-   fputs(line->buf, cmitmsg);
+   fputs(line->buf, mi->cmitmsg);
return 0;
 }
 
 static void handle_patch(struct mailinfo *mi, const struct strbuf *line)
 {
-   fwrite(line->buf, 1, line->len, patchfile);
+   fwrite(line->buf, 1, line->len, mi->patchfile);
mi->patch_lines++;
 }
 
@@ -974,15 +974,15 @@ static int mailinfo(struct mailinfo *mi, const char *msg, 
const char *patch)
int peek;
struct strbuf line = STRBUF_INIT;
 
-   cmitmsg = fopen(msg, "w");
-   if (!cmitmsg) {
+   mi->cmitmsg = fopen(msg, "w");
+   if (!mi->cmitmsg) {
perror(msg);
return -1;
}
-   patchfile = fopen(patch, "w");
-   if (!patchfile) {
+   mi->patchfile = fopen(patch, "w");
+   if (!mi->patchfile) {
perror(patch);
-   fclose(cmitmsg);
+   fclose(mi->cmitmsg);
return -1;
}
 
@@ -999,7 +999,7 @@ static int mailinfo(struct mailinfo *mi, const char *msg, 
const char *patch)
check_header(mi, , p_hdr_data, 1);
 
handle_body(mi, );
-   fclose(patchfile);
+   fclose(mi->patchfile);
 
handle_info(mi);
 
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 27/34] mailinfo: move read_one_header_line() closer to its callers

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 132 ++---
 1 file changed, 66 insertions(+), 66 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 81bb3c7..5a21d93 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -390,72 +390,6 @@ check_header_out:
return ret;
 }
 
-static int is_rfc2822_header(const struct strbuf *line)
-{
-   /*
-* The section that defines the loosest possible
-* field name is "3.6.8 Optional fields".
-*
-* optional-field = field-name ":" unstructured CRLF
-* field-name = 1*ftext
-* ftext = %d33-57 / %59-126
-*/
-   int ch;
-   char *cp = line->buf;
-
-   /* Count mbox From headers as headers */
-   if (starts_with(cp, "From ") || starts_with(cp, ">From "))
-   return 1;
-
-   while ((ch = *cp++)) {
-   if (ch == ':')
-   return 1;
-   if ((33 <= ch && ch <= 57) ||
-   (59 <= ch && ch <= 126))
-   continue;
-   break;
-   }
-   return 0;
-}
-
-static int read_one_header_line(struct strbuf *line, FILE *in)
-{
-   /* Get the first part of the line. */
-   if (strbuf_getline(line, in, '\n'))
-   return 0;
-
-   /*
-* Is it an empty line or not a valid rfc2822 header?
-* If so, stop here, and return false ("not a header")
-*/
-   strbuf_rtrim(line);
-   if (!line->len || !is_rfc2822_header(line)) {
-   /* Re-add the newline */
-   strbuf_addch(line, '\n');
-   return 0;
-   }
-
-   /*
-* Now we need to eat all the continuation lines..
-* Yuck, 2822 header "folding"
-*/
-   for (;;) {
-   int peek;
-   struct strbuf continuation = STRBUF_INIT;
-
-   peek = fgetc(in); ungetc(peek, in);
-   if (peek != ' ' && peek != '\t')
-   break;
-   if (strbuf_getline(, in, '\n'))
-   break;
-   continuation.buf[0] = ' ';
-   strbuf_rtrim();
-   strbuf_addbuf(line, );
-   }
-
-   return 1;
-}
-
 static struct strbuf *decode_q_segment(const struct strbuf *q_seg, int rfc2047)
 {
const char *in = q_seg->buf;
@@ -794,6 +728,72 @@ static void handle_filter(struct mailinfo *mi, struct 
strbuf *line)
}
 }
 
+static int is_rfc2822_header(const struct strbuf *line)
+{
+   /*
+* The section that defines the loosest possible
+* field name is "3.6.8 Optional fields".
+*
+* optional-field = field-name ":" unstructured CRLF
+* field-name = 1*ftext
+* ftext = %d33-57 / %59-126
+*/
+   int ch;
+   char *cp = line->buf;
+
+   /* Count mbox From headers as headers */
+   if (starts_with(cp, "From ") || starts_with(cp, ">From "))
+   return 1;
+
+   while ((ch = *cp++)) {
+   if (ch == ':')
+   return 1;
+   if ((33 <= ch && ch <= 57) ||
+   (59 <= ch && ch <= 126))
+   continue;
+   break;
+   }
+   return 0;
+}
+
+static int read_one_header_line(struct strbuf *line, FILE *in)
+{
+   /* Get the first part of the line. */
+   if (strbuf_getline(line, in, '\n'))
+   return 0;
+
+   /*
+* Is it an empty line or not a valid rfc2822 header?
+* If so, stop here, and return false ("not a header")
+*/
+   strbuf_rtrim(line);
+   if (!line->len || !is_rfc2822_header(line)) {
+   /* Re-add the newline */
+   strbuf_addch(line, '\n');
+   return 0;
+   }
+
+   /*
+* Now we need to eat all the continuation lines..
+* Yuck, 2822 header "folding"
+*/
+   for (;;) {
+   int peek;
+   struct strbuf continuation = STRBUF_INIT;
+
+   peek = fgetc(in); ungetc(peek, in);
+   if (peek != ' ' && peek != '\t')
+   break;
+   if (strbuf_getline(, in, '\n'))
+   break;
+   continuation.buf[0] = ' ';
+   strbuf_rtrim();
+   strbuf_addbuf(line, );
+   }
+
+   return 1;
+}
+
 static int find_boundary(struct mailinfo *mi, struct strbuf *line)
 {
while (!strbuf_getline(line, mi->input, '\n')) {
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 24/34] mailinfo: move content/content_top to struct mailinfo

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 45 ++---
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 2c194da..ec65805 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -7,6 +7,8 @@
 #include "utf8.h"
 #include "strbuf.h"
 
+#define MAX_BOUNDARIES 5
+
 struct mailinfo {
FILE *input;
FILE *output;
@@ -22,6 +24,8 @@ struct mailinfo {
int use_inbody_headers; /* defaults to 1 */
const char *metainfo_charset;
 
+   struct strbuf *content[MAX_BOUNDARIES];
+   struct strbuf **content_top;
struct strbuf charset;
char *message_id;
enum  {
@@ -35,7 +39,6 @@ struct mailinfo {
 };
 
 #define MAX_HDR_PARSED 10
-#define MAX_BOUNDARIES 5
 
 static void cleanup_space(struct strbuf *sb);
 
@@ -180,10 +183,6 @@ static int slurp_attr(const char *line, const char *name, 
struct strbuf *attr)
return 1;
 }
 
-static struct strbuf *content[MAX_BOUNDARIES];
-
-static struct strbuf **content_top = content;
-
 static void handle_content_type(struct mailinfo *mi, struct strbuf *line)
 {
struct strbuf *boundary = xmalloc(sizeof(struct strbuf));
@@ -191,11 +190,11 @@ static void handle_content_type(struct mailinfo *mi, 
struct strbuf *line)
 
if (slurp_attr(line->buf, "boundary=", boundary)) {
strbuf_insert(boundary, 0, "--", 2);
-   if (++content_top >= [MAX_BOUNDARIES]) {
+   if (++mi->content_top >= >content[MAX_BOUNDARIES]) {
fprintf(stderr, "Too many boundaries to handle\n");
exit(1);
}
-   *content_top = boundary;
+   *(mi->content_top) = boundary;
boundary = NULL;
}
slurp_attr(line->buf, "charset=", >charset);
@@ -223,10 +222,12 @@ static void handle_content_transfer_encoding(struct 
mailinfo *mi,
mi->transfer_encoding = TE_DONTCARE;
 }
 
-static int is_multipart_boundary(const struct strbuf *line)
+static int is_multipart_boundary(struct mailinfo *mi, const struct strbuf 
*line)
 {
-   return (((*content_top)->len <= line->len) &&
-   !memcmp(line->buf, (*content_top)->buf, (*content_top)->len));
+   struct strbuf *content_top = *(mi->content_top);
+
+   return ((content_top->len <= line->len) &&
+   !memcmp(line->buf, content_top->buf, content_top->len));
 }
 
 static void cleanup_subject(struct mailinfo *mi, struct strbuf *subject)
@@ -799,7 +800,7 @@ static void handle_filter(struct mailinfo *mi, struct 
strbuf *line)
 static int find_boundary(struct mailinfo *mi, struct strbuf *line)
 {
while (!strbuf_getline(line, mi->input, '\n')) {
-   if (*content_top && is_multipart_boundary(line))
+   if (*(mi->content_top) && is_multipart_boundary(mi, line))
return 1;
}
return 0;
@@ -811,18 +812,18 @@ static int handle_boundary(struct mailinfo *mi, struct 
strbuf *line)
 
strbuf_addch(, '\n');
 again:
-   if (line->len >= (*content_top)->len + 2 &&
-   !memcmp(line->buf + (*content_top)->len, "--", 2)) {
+   if (line->len >= (*(mi->content_top))->len + 2 &&
+   !memcmp(line->buf + (*(mi->content_top))->len, "--", 2)) {
/* we hit an end boundary */
/* pop the current boundary off the stack */
-   strbuf_release(*content_top);
-   free(*content_top);
-   *content_top = NULL;
+   strbuf_release(*(mi->content_top));
+   free(*(mi->content_top));
+   *(mi->content_top) = NULL;
 
/* technically won't happen as is_multipart_boundary()
   will fail first.  But just in case..
 */
-   if (--content_top < content) {
+   if (--mi->content_top < mi->content) {
fprintf(stderr, "Detected mismatched boundaries, "
"can't recover\n");
exit(1);
@@ -857,14 +858,14 @@ static void handle_body(struct mailinfo *mi, struct 
strbuf *line)
struct strbuf prev = STRBUF_INIT;
 
/* Skip up to the first boundary */
-   if (*content_top) {
+   if (*(mi->content_top)) {
if (!find_boundary(mi, line))
goto handle_body_out;
}
 
do {
/* process any boundary lines */
-   if (*content_top && is_multipart_boundary(line)) {
+   if (*(mi->content_top) && is_multipart_boundary(mi, line)) {
/* flush any leftover */
if (prev.len) {
handle_filter(mi, );
@@ -1028,6 +1029,7 @@ static void setup_mailinfo(struct mailinfo *mi)
strbuf_init(>charset, 0);
mi->header_stage = 1;
 

[PATCH v3 30/34] mailinfo: move definition of MAX_HDR_PARSED closer to its use

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 104d3d9..4eabc11 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -39,8 +39,6 @@ struct mailinfo {
struct strbuf log_message;
 };
 
-#define MAX_HDR_PARSED 10
-
 static void cleanup_space(struct strbuf *sb)
 {
size_t pos, cnt;
@@ -290,6 +288,7 @@ static void cleanup_subject(struct mailinfo *mi, struct 
strbuf *subject)
strbuf_trim(subject);
 }
 
+#define MAX_HDR_PARSED 10
 static const char *header[MAX_HDR_PARSED] = {
"From","Subject","Date",
 };
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 08/34] mailinfo: do not let handle_boundary() touch global "line" directly

[Junio C Hamano]

This function has a single caller, and called with the global "line"
holding the multi-part boundary line the caller saw while processing
the e-mail body.  The function then goes into a loop to process each
line of the input, and fills the same global "line" variable from
the input as it needs to read more lines to process the multi-part
headers.

Let the caller explicitly pass a pointer to this global "line"
variable as an argument, and have the function itself use that
strbuf throughout, instead of referring to the global "line" itself.

There still is a helper function that this function calls that still
touches the global directly; it will be updated as the series progresses.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 52be7db..cab1235 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -797,14 +797,14 @@ static int find_boundary(void)
return 0;
 }
 
-static int handle_boundary(int *filter_stage, int *header_stage)
+static int handle_boundary(struct strbuf *line, int *filter_stage, int 
*header_stage)
 {
struct strbuf newline = STRBUF_INIT;
 
strbuf_addch(, '\n');
 again:
-   if (line.len >= (*content_top)->len + 2 &&
-   !memcmp(line.buf + (*content_top)->len, "--", 2)) {
+   if (line->len >= (*content_top)->len + 2 &&
+   !memcmp(line->buf + (*content_top)->len, "--", 2)) {
/* we hit an end boundary */
/* pop the current boundary off the stack */
strbuf_release(*content_top);
@@ -833,14 +833,14 @@ again:
strbuf_reset();
 
/* slurp in this section's info */
-   while (read_one_header_line(, fin))
-   check_header(, p_hdr_data, 0);
+   while (read_one_header_line(line, fin))
+   check_header(line, p_hdr_data, 0);
 
strbuf_release();
/* replenish line */
-   if (strbuf_getline(, fin, '\n'))
+   if (strbuf_getline(line, fin, '\n'))
return 0;
-   strbuf_addch(, '\n');
+   strbuf_addch(line, '\n');
return 1;
 }
 
@@ -864,7 +864,7 @@ static void handle_body(struct strbuf *line)
handle_filter(, _stage, 
_stage);
strbuf_reset();
}
-   if (!handle_boundary(_stage, _stage))
+   if (!handle_boundary(line, _stage, 
_stage))
goto handle_body_out;
}
 
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 14/34] mailinfo: move filter/header stage to struct mailinfo

[Junio C Hamano]

Earlier we got rid of two function-scope static variables that kept
track of the states of helper functions by making them extra arguments
that are passed throughout the callchain.  Now we have a convenient
place to store and pass them around in the form of "struct mailinfo",
change them into two fields in the struct.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 42 ++
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index b150936..5823ae5 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -19,6 +19,9 @@ struct mailinfo {
struct strbuf email;
int keep_subject;
int keep_non_patch_brackets_in_subject;
+
+   int filter_stage; /* still reading log or are we copying patch? */
+   int header_stage; /* still checking in-body headers? */
 };
 static char *message_id;
 
@@ -28,6 +31,7 @@ static enum  {
 
 static struct strbuf charset = STRBUF_INIT;
 static int patch_lines;
+
 static struct strbuf **p_hdr_data, **s_hdr_data;
 static int use_scissors;
 static int add_message_id;
@@ -718,25 +722,25 @@ static int is_scissors_line(const struct strbuf *line)
gap * 2 < perforation);
 }
 
-static int handle_commit_msg(struct strbuf *line, int *still_looking)
+static int handle_commit_msg(struct mailinfo *mi, struct strbuf *line)
 {
if (!cmitmsg)
return 0;
 
-   if (*still_looking) {
+   if (mi->header_stage) {
if (!line->len || (line->len == 1 && line->buf[0] == '\n'))
return 0;
}
 
-   if (use_inbody_headers && *still_looking) {
-   *still_looking = check_header(line, s_hdr_data, 0);
-   if (*still_looking)
+   if (use_inbody_headers && mi->header_stage) {
+   mi->header_stage = check_header(line, s_hdr_data, 0);
+   if (mi->header_stage)
return 0;
} else
/* Only trim the first (blank) line of the commit message
 * when ignoring in-body headers.
 */
-   *still_looking = 0;
+   mi->header_stage = 0;
 
/* normalize the log message to UTF-8. */
if (metainfo_charset)
@@ -748,7 +752,7 @@ static int handle_commit_msg(struct strbuf *line, int 
*still_looking)
die_errno("Could not rewind output message file");
if (ftruncate(fileno(cmitmsg), 0))
die_errno("Could not truncate output message file at 
scissors");
-   *still_looking = 1;
+   mi->header_stage = 1;
 
/*
 * We may have already read "secondary headers"; purge
@@ -780,13 +784,13 @@ static void handle_patch(const struct strbuf *line)
patch_lines++;
 }
 
-static void handle_filter(struct strbuf *line, int *filter_stage, int 
*header_stage)
+static void handle_filter(struct mailinfo *mi, struct strbuf *line)
 {
-   switch (*filter_stage) {
+   switch (mi->filter_stage) {
case 0:
-   if (!handle_commit_msg(line, header_stage))
+   if (!handle_commit_msg(mi, line))
break;
-   (*filter_stage)++;
+   mi->filter_stage++;
case 1:
handle_patch(line);
break;
@@ -802,8 +806,7 @@ static int find_boundary(struct mailinfo *mi, struct strbuf 
*line)
return 0;
 }
 
-static int handle_boundary(struct mailinfo *mi, struct strbuf *line,
-  int *filter_stage, int *header_stage)
+static int handle_boundary(struct mailinfo *mi, struct strbuf *line)
 {
struct strbuf newline = STRBUF_INIT;
 
@@ -825,7 +828,7 @@ again:
"can't recover\n");
exit(1);
}
-   handle_filter(, filter_stage, header_stage);
+   handle_filter(mi, );
strbuf_release();
 
/* skip to the next boundary */
@@ -853,8 +856,6 @@ again:
 static void handle_body(struct mailinfo *mi, struct strbuf *line)
 {
struct strbuf prev = STRBUF_INIT;
-   int filter_stage = 0;
-   int header_stage = 1;
 
/* Skip up to the first boundary */
if (*content_top) {
@@ -867,10 +868,10 @@ static void handle_body(struct mailinfo *mi, struct 
strbuf *line)
if (*content_top && is_multipart_boundary(line)) {
/* flush any leftover */
if (prev.len) {
-   handle_filter(, _stage, 
_stage);
+   handle_filter(mi, );
strbuf_reset();
}
-   if (!handle_boundary(mi, line, _stage, 
_stage))
+   if (!handle_boundary(mi, line))
goto 

[PATCH v3 07/34] mailinfo: do not let handle_body() touch global "line" directly

[Junio C Hamano]

This function has a single caller, and called with the global "line"
holding the first line of the e-mail body after the caller finished
processing the e-mail headers.  The function then goes into a loop
to process each line of the input, starting from what was given by
its caller, and fills the same global "line" variable from the input
as it needs to process more lines.

Let the caller explicitly pass a pointer to this global "line"
variable as an argument, and have the function itself use that
strbuf throughout, instead of referring to the global "line" itself.

There are helper functions that this function calls that still touch
the global directly; they will be updated as the series progresses.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 1518708..52be7db 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -844,7 +844,7 @@ again:
return 1;
 }
 
-static void handle_body(void)
+static void handle_body(struct strbuf *line)
 {
struct strbuf prev = STRBUF_INIT;
int filter_stage = 0;
@@ -858,7 +858,7 @@ static void handle_body(void)
 
do {
/* process any boundary lines */
-   if (*content_top && is_multipart_boundary()) {
+   if (*content_top && is_multipart_boundary(line)) {
/* flush any leftover */
if (prev.len) {
handle_filter(, _stage, 
_stage);
@@ -869,7 +869,7 @@ static void handle_body(void)
}
 
/* Unwrap transfer encoding */
-   decode_transfer_encoding();
+   decode_transfer_encoding(line);
 
switch (transfer_encoding) {
case TE_BASE64:
@@ -878,7 +878,7 @@ static void handle_body(void)
struct strbuf **lines, **it, *sb;
 
/* Prepend any previous partial lines */
-   strbuf_insert(, 0, prev.buf, prev.len);
+   strbuf_insert(line, 0, prev.buf, prev.len);
strbuf_reset();
 
/*
@@ -886,7 +886,7 @@ static void handle_body(void)
 * multiple new lines.  Pass only one chunk
 * at a time to handle_filter()
 */
-   lines = strbuf_split(, '\n');
+   lines = strbuf_split(line, '\n');
for (it = lines; (sb = *it); it++) {
if (*(it + 1) == NULL) /* The last line */
if (sb->buf[sb->len - 1] != '\n') {
@@ -904,10 +904,10 @@ static void handle_body(void)
break;
}
default:
-   handle_filter(, _stage, _stage);
+   handle_filter(line, _stage, _stage);
}
 
-   } while (!strbuf_getwholeline(, fin, '\n'));
+   } while (!strbuf_getwholeline(line, fin, '\n'));
 
 handle_body_out:
strbuf_release();
@@ -993,7 +993,7 @@ static int mailinfo(FILE *in, FILE *out, const char *msg, 
const char *patch)
while (read_one_header_line(, fin))
check_header(, p_hdr_data, 1);
 
-   handle_body();
+   handle_body();
fclose(patchfile);
 
handle_info();
-- 
2.6.2-383-g144b2e6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options

[Matthieu Moy]

Junio C Hamano  writes:

> I personally would suggest whichever order you feel more comfortable
> and less error-prone.

This is a good summary, and I fully agree with it.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Feature Request: Branch Description updating support for git fetch/pull/push

[Stephen Connolly]

Branch descriptions are great. Even more so if you set
merge.branchdesc=true (while we are at it could there be a CLI option
to git merge that allowed overriding merge.branchdesc for that merge)

But the bit that pains me is that my description is local only.

Could we add support for syncing the branch description with the remote?

When I think about how such sync could work:

* I would see the sync as an option that needs to be enabled rather
than on by default (at least as long as `merge.branchdesc` is an
option). Overriding by the CLI would be great, e.g.
`--description-sync / --no-description-sync`
* When that sync is enabled, if the destination side is missing a
description then the description is sync'd. Otherwise if the
destination description differs from the source description the user
must explicitly specify one of `--description-sync /
--no-description-sync` or else the operation will stop before updating
references (I think that `git fetch` could be an exception to this
rule as updating the remote branch descriptions by overwriting is
exactly what you want, but for `git pull` / `git push` I think they
need the CLI confirmation if the user has opted into synchronizing
branch descriptions)

WDYT?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/4] stripspace: Implement --count-lines option

[Tobias Klauser]

On 2015-10-18 at 19:18:53 +0200, Junio C Hamano  wrote:
> Eric Sunshine  writes:
> 
> > Is there any application beyond git-rebase--interactive where a
> > --count-lines options is expected to be useful? It's not obvious from
> > the commit message that this change is necessarily a win for later
> > porting of git-rebase--interactive to C since the amount of extra code
> > and support material added by this patch probably outweighs the amount
> > of code a C version of git-rebase--interactive would need to count the
> > lines itself.
> >
> > Stated differently, are the two or three instances of piping through
> > 'wc' in git-rebase--interactive sufficient justification for
> > introducing extra complexity into git-stripspace and its documentation
> > and tests?
> 
> Interesting thought.  When somebody rewrites "rebase -i" in C,
> nobody needs to count lines in "stripspace" output.  The rewritten
> "rebase -i" would internally run strbuf_stripspace() and the question
> becomes what is the best way to let that code find out how many lines
> the result contains.
> 
> When viewed from that angle, I agree that "stripspace --count" does
> not add anything to further the goal of helping "rebase -i" to move
> to C.  Adding strbuf_count_lines() that counts the number of lines
> in the given strbuf (if there is no such helper yet; I didn't check),
> though.

I check before implementing this series and didn't find any helper. I
also didn't find any other uses of line counting in the code.

After considering your and Eric's reply, I'll drop these patches for
now and only resubmit patches 1/4 and 2/4 for v3 (also see my reply to
Eric).

> >> +test_expect_success '--count-lines with newline only' '
> >> +   printf "0\n" >expect &&
> >> +   printf "\n" | git stripspace --count-lines >actual &&
> >> +   test_cmp expect actual
> >> +'
> >
> > What is the expected behavior when the input is an empty file, a file
> > with content but no newline, a file with one or more lines but lacking
> > a newline on the final line? Should these cases be tested, as well?
> 
> Good point here, too.  If we were to add strbuf_count_lines()
> helper, whoever adds that function needs to take a possible
> incomplete line at the end into account.

Yes, makes more sense like this (even though it doesn't correspond to
what 'wc -l' does).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/4] stripspace: Implement --count-lines option

[Tobias Klauser]

On 2015-10-18 at 01:57:57 +0200, Eric Sunshine  wrote:
> On Fri, Oct 16, 2015 at 11:16 AM, Tobias Klauser  wrote:
> > Implement the --count-lines options for git stripspace [...]
> >
> > This will make it easier to port git-rebase--interactive.sh to C later
> > on.
> 
> Is there any application beyond git-rebase--interactive where a
> --count-lines options is expected to be useful? It's not obvious from
> the commit message that this change is necessarily a win for later
> porting of git-rebase--interactive to C since the amount of extra code
> and support material added by this patch probably outweighs the amount
> of code a C version of git-rebase--interactive would need to count the
> lines itself.

Agreed, it doesn't make much sense anymore in the current form. An
strbuf helper function implementing the line counting would probably be
the better way. But I guess this should only be introduced once someone
decides to write a C version of git-rebase--interactive (or any other
use for line counting appears).

> Stated differently, are the two or three instances of piping through
> 'wc' in git-rebase--interactive sufficient justification for
> introducing extra complexity into git-stripspace and its documentation
> and tests?

IMO it doesn't add a lot of complexity, but on the other hand it also
doesn't provide a large benefit apart from getting rid of a few
calls to an external program in a code path which is not performance
critical.

So I suggest I'll drop patches 3/4 and 4/4 for v3. Once someone really
needs the line counting functionality in C, an strbuf helper can still
be added.

> More below.
> 
> > Furthermore, add the corresponding documentation and tests.
> >
> > Signed-off-by: Tobias Klauser 
> > ---
> > diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
> > index 29e91d8..9c00cb9 100755
> > --- a/t/t0030-stripspace.sh
> > +++ b/t/t0030-stripspace.sh
> > @@ -438,4 +438,40 @@ test_expect_success 'avoid SP-HT sequence in commented 
> > line' '
> > test_cmp expect actual
> >  '
> >
> > +test_expect_success '--count-lines with newline only' '
> > +   printf "0\n" >expect &&
> > +   printf "\n" | git stripspace --count-lines >actual &&
> > +   test_cmp expect actual
> > +'
> 
> What is the expected behavior when the input is an empty file, a file
> with content but no newline, a file with one or more lines but lacking
> a newline on the final line? Should these cases be tested, as well?

Not really sure. For the implementation I followed the behavior of 'wc
-l' which doesn't consider the final line if it lacks a newline. Should
this be different for git's purposes? In any case, I agree that these
cases should excplicitely be tested/documented.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Your Inheritance

Hello,

I wish to notify you that late Gianni Agnelli, had included you as beneficiary 
of his Will. He left the sum of sixty Million, Five Hundred Thousand Dollars 
(US$60, 500,000.00) to you in the Codicil and last testament to his Will. This 
may sound strange and unbelievable to you; you are advised to contact Hollis 
Grey Chambers via our personal email address:  holli...@foxmail.com

Regards,
Hollis Grey Chambers.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: problem with modification time of packfiles

[Andreas Amann]

"brian m. carlson"  writes:
> On Sun, Oct 18, 2015 at 10:37:55PM +0100, Andreas Amann wrote:
>> git (2.6.1) sometimes updates the modification time of a packfile, even if it
>> has not changed at all.
>> 
>> On my system this triggers quite expensive an d unnecessary backup
>> operations, which I would prefer to avoid.  Is there a simple way to
>> keep the mtime of packfiles fixed, once they are created?
>> 
>> Apparently the undesired mtime update is done in
>> sha1_file.c:freshen_file() which is called (indirectly) by
>> write_sha1_file().  However I did not understand, why this is done.
>> 
>> Any clarification and pointers, how mtime can be kept constant would be
>> appreciated.
>
> This is required to avoid deleting items that might still be needed.
> The commit message for the commit that introduced that function is as
> follows:
>
>   write_sha1_file: freshen existing objects
>   
>   When we try to write a loose object file, we first check
>   whether that object already exists. If so, we skip the
>   write as an optimization. However, this can interfere with
>   prune's strategy of using mtimes to mark files in progress.
>   

Thank you for your answer.  However, this reasoning only applies to loose
objects and not packfiles.

My understanding is that "git prune" will not prune any pack files
(except those starting with tmp_).  Only "git repack" should do that.
Repack seems to be however mtime agnostic and therefore it does not seem
to be necessary to freshen packfiles.

It therefore seems that git freshens packfiles unnecessarily, which can
lead to expensive and unnecessary backup operations. 

Given this, would a trivial patch to remove the freshening of packfiles
be acceptable?

Alternatively, maybe it would be preferrable to use ctime instead of
mtime to mark recently used packfiles and loose objects?  This might be
more natural, as mtime is usually associated with a "modification" of
the file itself, which does not happen here.  ctime on the other hand
indicates attribute changes.  (Instead of utime() in this case chmod()
could be used to update ctime.)

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Announcing Git for Windows 2.6.2

[Johannes Schindelin]

Hi all,

it is my pleasure to announce that Git for Windows 2.6.2 is available.
Please find it at https://git-for-windows.github.io/

Changes since Git for Windows v2.6.1 (October 5th 2015)

New Features

  • Comes with Git v2.6.2
  • Users who are part of a Windows domain now have sensible default
values for user.name and user.email.

Bug Fixes

  • We no longer run out of page file space when git fetching large
repositories.
  • The description of Windows' default console is accurate now (the
console became more powerful in Windows 10).
  • Git GUI now respects the terminal emulation chosen at install time
when running the Git Bash.

Ciao,
Johannes

[PATCH v1] git-p4: Add option to ignore empty commits

[larsxschneider]

From: Lars Schneider 

A changelist that contains only excluded files (e.g. via client spec or
branch prefix) will be imported as empty commit. Add option
"git-p4.ignoreEmptyCommits" to ignore these commits.

Signed-off-by: Lars Schneider 
---
 Documentation/git-p4.txt   |   5 ++
 git-p4.py  |  41 -
 t/t9826-git-p4-ignore-empty-commits.sh | 103 +
 3 files changed, 133 insertions(+), 16 deletions(-)
 create mode 100755 t/t9826-git-p4-ignore-empty-commits.sh

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 82aa5d6..f096a6a 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -510,6 +510,11 @@ git-p4.useClientSpec::
option '--use-client-spec'.  See the "CLIENT SPEC" section above.
This variable is a boolean, not the name of a p4 client.
 
+git-p4.ignoreEmptyCommits::
+   A changelist that contains only excluded files will be imported
+   as empty commit. To ignore these commits set this boolean option
+   to 'true'.
+
 Submit variables
 
 git-p4.detectRenames::
diff --git a/git-p4.py b/git-p4.py
index 0093fa3..6c50c74 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2288,12 +2288,6 @@ class P4Sync(Command, P4UserMap):
 filesToDelete = []
 
 for f in files:
-# if using a client spec, only add the files that have
-# a path in the client
-if self.clientSpecDirs:
-if self.clientSpecDirs.map_in_client(f['path']) == "":
-continue
-
 filesForCommit.append(f)
 if f['action'] in self.delete_actions:
 filesToDelete.append(f)
@@ -2368,18 +2362,33 @@ class P4Sync(Command, P4UserMap):
 if self.verbose:
 print "commit into %s" % branch
 
-# start with reading files; if that fails, we should not
-# create a commit.
-new_files = []
-for f in files:
-if [p for p in self.branchPrefixes if p4PathStartsWith(f['path'], 
p)]:
-new_files.append (f)
-else:
-sys.stderr.write("Ignoring file outside of prefix: %s\n" % 
f['path'])
-
 if self.clientSpecDirs:
 self.clientSpecDirs.update_client_spec_path_cache(files)
 
+def inClientSpec(path):
+if not self.clientSpecDirs:
+return True
+inClientSpec = self.clientSpecDirs.map_in_client(path)
+if not inClientSpec and self.verbose:
+print '\n  Ignoring file outside of client spec' % path
+return inClientSpec
+
+def hasBranchPrefix(path):
+if not self.branchPrefixes:
+return True
+hasPrefix = [p for p in self.branchPrefixes
+if p4PathStartsWith(path, p)]
+if hasPrefix and self.verbose:
+print '\n  Ignoring file outside of prefix: %s' % path
+return hasPrefix
+
+files = [f for f in files
+if inClientSpec(f['path']) and hasBranchPrefix(f['path'])]
+
+if not files and gitConfigBool('git-p4.ignoreEmptyCommits'):
+print '\n  Ignoring change %s as it would produce an empty commit.'
+return
+
 self.gitStream.write("commit %s\n" % branch)
 #gitStream.write("mark :%s\n" % details["change"])
 self.committedChanges.add(int(details["change"]))
@@ -2403,7 +2412,7 @@ class P4Sync(Command, P4UserMap):
 print "parent %s" % parent
 self.gitStream.write("from %s\n" % parent)
 
-self.streamP4Files(new_files)
+self.streamP4Files(files)
 self.gitStream.write("\n")
 
 change = int(details["change"])
diff --git a/t/t9826-git-p4-ignore-empty-commits.sh 
b/t/t9826-git-p4-ignore-empty-commits.sh
new file mode 100755
index 000..5ddccde
--- /dev/null
+++ b/t/t9826-git-p4-ignore-empty-commits.sh
@@ -0,0 +1,103 @@
+#!/bin/sh
+
+test_description='Clone repositories and ignore empty commits'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+   start_p4d
+'
+
+test_expect_success 'Create a repo' '
+   client_view "//depot/... //client/..." &&
+   (
+   cd "$cli" &&
+
+   mkdir -p subdir &&
+
+   >subdir/file1.txt &&
+   p4 add subdir/file1.txt &&
+   p4 submit -d "Add file 1" &&
+
+   >file2.txt &&
+   p4 add file2.txt &&
+   p4 submit -d "Add file 2" &&
+
+   >subdir/file3.txt &&
+   p4 add subdir/file3.txt &&
+   p4 submit -d "Add file 3"
+   )
+'
+
+test_expect_success 'Clone repo root path with all history' '
+   client_view "//depot/... //client/..." &&
+   test_when_finished cleanup_git &&
+   (
+   cd "$git" &&
+   git init . &&

Re: [PATCH v2 3/4] stripspace: Implement --count-lines option

[Matthieu Moy]

Eric Sunshine  writes:

> With this in mind, my
> question was also indirectly asking whether there was sufficient
> justification of the long-term cost of a --count-lines option. The
> argument that --count-lines would help test a proposed
> strbuf_count_lines() likely does not outweigh that cost.

I agree. If we expect users to call --count-lines outside
rebase-interactive.sh and our own tests, then the actual use should be
justified in the commit message.

If not, then at least the --count-lines option should be hidden and not
documented in the public doc. But I agree that introducing test-strbuf
would be even better.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/4] stripspace: Implement --count-lines option

[Eric Sunshine]

On Mon, Oct 19, 2015 at 1:03 PM, Christian Couder
 wrote:
> On Mon, Oct 19, 2015 at 3:46 PM, Tobias Klauser  wrote:
>> On 2015-10-18 at 19:18:53 +0200, Junio C Hamano  wrote:
>>> Eric Sunshine  writes:
>>> > Is there any application beyond git-rebase--interactive where a
>>> > --count-lines options is expected to be useful? It's not obvious from
>>> > the commit message that this change is necessarily a win for later
>>> > porting of git-rebase--interactive to C since the amount of extra code
>>> > and support material added by this patch probably outweighs the amount
>>> > of code a C version of git-rebase--interactive would need to count the
>>> > lines itself.
>>> >
>>> > Stated differently, are the two or three instances of piping through
>>> > 'wc' in git-rebase--interactive sufficient justification for
>>> > introducing extra complexity into git-stripspace and its documentation
>>> > and tests?
>>>
>>> Interesting thought.  When somebody rewrites "rebase -i" in C,
>>> nobody needs to count lines in "stripspace" output.  The rewritten
>>> "rebase -i" would internally run strbuf_stripspace() and the question
>>> becomes what is the best way to let that code find out how many lines
>>> the result contains.
>>>
>>> When viewed from that angle, I agree that "stripspace --count" does
>>> not add anything to further the goal of helping "rebase -i" to move
>>> to C.  Adding strbuf_count_lines() that counts the number of lines
>>> in the given strbuf (if there is no such helper yet; I didn't check),
>>> though.
>>
>> I check before implementing this series and didn't find any helper. I
>> also didn't find any other uses of line counting in the code.
>
> This shows that implementing "git stripspace --count-lines" could
> indirectly help porting "git rebase -i" to C as you could implement
> strbuf_count_lines() for the former and it could then be reused in the
> latter.

In this project, where all user-facing functionality must be supported
for the life of the project, each new command, command-line option,
configuration setting, and environment variable exacts additional
costs beyond the initial implementation cost. With this in mind, my
question was also indirectly asking whether there was sufficient
justification of the long-term cost of a --count-lines option. The
argument that --count-lines would help test a proposed
strbuf_count_lines() likely does not outweigh that cost.

>>> >> +test_expect_success '--count-lines with newline only' '
>>> >> +   printf "0\n" >expect &&
>>> >> +   printf "\n" | git stripspace --count-lines >actual &&
>>> >> +   test_cmp expect actual
>>> >> +'
>>> >
>>> > What is the expected behavior when the input is an empty file, a file
>>> > with content but no newline, a file with one or more lines but lacking
>>> > a newline on the final line? Should these cases be tested, as well?
>>>
>>> Good point here, too.  If we were to add strbuf_count_lines()
>>> helper, whoever adds that function needs to take a possible
>>> incomplete line at the end into account.
>>
>> Yes, makes more sense like this (even though it doesn't correspond to
>> what 'wc -l' does).
>
> Tests for "git stripspace --count-lines" would test
> strbuf_count_lines() which would also help when porting git rebase -i
> to C.

Rather than saddling the project with the cost of a new user-facing,
but otherwise unneeded option, a more direct way to test the proposed
strbuf_count_lines() would be to add a test-strbuf program, akin to
test-config, test-string-list, etc. This has the added benefit of
providing a home for strbuf-based tests beyond line counting.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pull: add angle brackets to usage string

[Alex Henrie]

2015-10-16 11:42 GMT-06:00 Junio C Hamano :
> Alex Henrie  writes:
>
>> 2015-10-16 10:36 GMT-06:00 Junio C Hamano :
>>> Makes sense, as all the other  in the usage string are
>>> bracketted.
>>>
>>> Does it make sense to do this for contrib/examples, which is the
>>> historical record, though?
>>
>> I didn't know that contrib/examples was a historical record. The last
>> patch I submitted, b7447679e84ed973430ab19fce87f56857b83068, also
>> modified contrib/examples.
>
> Yes, but that fixes historical "mistake", no?
>
> With this, you are breaking historical practice by changing only one
> instance to deviate from the then-current practice of saying
> 'options' without brackets.  It is based on the point of view that
> considers anything inside  and a fixed string 'options' are
> meant to be replaced by intelligent readers, which is as valid as
> the more recent practice to consider only things inside 
> are placeholders.

OK, I see. You're saying that it's OK to fix typos and grammatical
errors in contrib/examples, but it's not okay to modernize the
scripts' designs. That's fine; standardizing placeholder syntax is
primarily for the benefit of translators, and contrib/ is not
translated, so it's not causing a problem.

-Alex
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] fix flaky untracked-cache test

[David Turner]

Dirty the test worktree's root directory, as the test expects.

When testing the untracked-cache, we previously assumed that checking
out master would be sufficient to mark the mtime of the worktree's
root directory as racily-dirty.  But sometimes, the checkout would
happen at 12345.999 seconds and the status at 12346.001 seconds,
meaning that the worktree's root directory would not be racily-dirty.
And since it was not truly dirty, occasionally the test would fail.
By making the root truly dirty, the test will always succeed.

Tested by running a few hundred times.

Signed-off-by: David Turner 
---
 t/t7063-status-untracked-cache.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/t/t7063-status-untracked-cache.sh 
b/t/t7063-status-untracked-cache.sh
index 37a24c1..0e8d0d4 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -412,7 +412,9 @@ test_expect_success 'create/modify files, some of which are 
gitignored' '
echo two bis >done/two &&
echo three >done/three && # three is gitignored
echo four >done/four && # four is gitignored at a higher level
-   echo five >done/five # five is not gitignored
+   echo five >done/five && # five is not gitignored
+   echo test >base && #we need to ensure that the root dir is touched
+   rm base
 '
 
 test_expect_success 'test sparse status with untracked cache' '
-- 
2.4.2.644.g97b850b-twtrsrc

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Urgent Reply

[Sahara]

ATTN:
Do you need a convenient and direct loan? We offer personal loan, car loan, 
small business loan and Investment financing with an affordable interest rate, 
fast and convenient. To apply email: enquir...@saharafc.com
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/5] run-command: Fix early shutdown

[Stefan Beller]

The return value of `pp_collect_finished` indicates if we want to shutdown
the parallel processing early. Both returns from that function should
return any accumulated results.

Signed-off-by: Stefan Beller 
---
 run-command.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/run-command.c b/run-command.c
index ef3da27..8f47c6e 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1077,7 +1077,7 @@ static int pp_collect_finished(struct parallel_processes 
*pp)
while (pp->nr_processes > 0) {
pid = waitpid(-1, _status, WNOHANG);
if (pid == 0)
-   return 0;
+   return result;
 
if (pid < 0)
die_errno("wait");
-- 
2.5.0.285.g8fe9b61.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/5] Fixes for the parallel processing

[Stefan Beller]

I noticed a problem with the gracefully abortion in the parallel processing,
that is fixed in patch 1.

Patch 2 makes the API more maintainable/usable (the caller may forget to
call child_process_init and only fill in fields which the callback is
interested in)

Patch 3 is another fixup. It actually initializes the shutdown flag properly.
(Apparently it had the right value so far)

Patches 4 and 5 add more tests both for the gracefully aborting case as well
as the standard use case.

This applies on top of sb/submodule-parallel-fetch which is already in next.
Junio, do you want me to reroll that series with these patches squashed in
appropriately, or just put it on top of that series ?

Thanks,
Stefan

Stefan Beller (5):
  run-command: Fix early shutdown
  run-command: Call get_next_task with a clean child process.
  run-command: Initialize the shutdown flag
  test-run-command: test for gracefully aborting
  test-run-command: Increase test coverage

 run-command.c  |  5 -
 t/t0061-run-command.sh | 28 ++--
 test-run-command.c | 22 --
 3 files changed, 50 insertions(+), 5 deletions(-)

-- 
2.5.0.285.g8fe9b61.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC] URL rewrite in .gitmodules

[Lars Schneider]

Hi,

I have a closed source Git repo which references an Open Source Git repo as 
Submodule. The Open Source Git repo references yet another Open Source repo as 
submodule. In order to avoid failing builds due to external services I mirrored 
the Open Source repos in my company network. That works great with the first 
level of Submodules. Unfortunately it does not work with the second level 
because the first level still references the "outside of company" repos. I know 
I can rewrite Git URLs with the git config "url..insteadOf" option. 
However, git configs are client specific. I would prefer a solution that works 
without setup on any client. I also know that I could update the .gitmodules 
file in the Open Source repo on the first level. I also would prefer not to do 
this as I want to use the very same hashes as defined by the "upstream" Open 
Source repos.

Is there yet another way in Git to change URLs of Submodules in the way I want 
it?

If not, what do you think about a patch that adds a "url" section similar to 
the one in git config to a .gitmodules file?

Example:
--
[submodule "git"]
path = git
url=git://github.com/larsxschneider/git.git

[url "mycompany.com"]
insteadOf = outside.com
--

Thanks,
Lars--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/5] test-run-command: Increase test coverage

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 t/t0061-run-command.sh | 16 +---
 test-run-command.c | 12 
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index 0af77cd..f27ada7 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -62,8 +62,18 @@ Hello
 World
 EOF
 
-test_expect_success 'run_command runs in parallel' '
-   test-run-command run-command-parallel-4 sh -c "printf \"%s\n%s\n\" 
Hello World" 2>actual &&
+test_expect_success 'run_command runs in parallel with more jobs available 
than tasks' '
+   test-run-command run-command-parallel 5 sh -c "printf \"%s\n%s\n\" 
Hello World" 2>actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'run_command runs in parallel with as many jobs as tasks' '
+   test-run-command run-command-parallel 4 sh -c "printf \"%s\n%s\n\" 
Hello World" 2>actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'run_command runs in parallel with more tasks than jobs 
available' '
+   test-run-command run-command-parallel 3 sh -c "printf \"%s\n%s\n\" 
Hello World" 2>actual &&
test_cmp expect actual
 '
 
@@ -77,7 +87,7 @@ asking for a quick stop
 EOF
 
 test_expect_success 'run_command is asked to abort gracefully' '
-   test-run-command run-command-abort-3 false 2>actual &&
+   test-run-command run-command-abort 3 false 2>actual &&
test_cmp expect actual
 '
 
diff --git a/test-run-command.c b/test-run-command.c
index 4b59482..44710c3 100644
--- a/test-run-command.c
+++ b/test-run-command.c
@@ -47,6 +47,7 @@ static int task_finished(int result,
 int main(int argc, char **argv)
 {
struct child_process proc = CHILD_PROCESS_INIT;
+   int jobs;
 
if (argc < 3)
return 1;
@@ -61,12 +62,15 @@ int main(int argc, char **argv)
if (!strcmp(argv[1], "run-command"))
exit(run_command());
 
-   if (!strcmp(argv[1], "run-command-parallel-4"))
-   exit(run_processes_parallel(4, parallel_next,
+   jobs = atoi(argv[2]);
+   proc.argv = (const char **)argv+3;
+
+   if (!strcmp(argv[1], "run-command-parallel"))
+   exit(run_processes_parallel(jobs, parallel_next,
NULL, NULL, ));
 
-   if (!strcmp(argv[1], "run-command-abort-3"))
-   exit(run_processes_parallel(3, parallel_next,
+   if (!strcmp(argv[1], "run-command-abort"))
+   exit(run_processes_parallel(jobs, parallel_next,
NULL, task_finished, ));
 
fprintf(stderr, "check usage\n");
-- 
2.5.0.285.g8fe9b61.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/5] test-run-command: test for gracefully aborting

[Stefan Beller]

We reuse the get_next_task callback which would stop after invoking the
test program 4 times. However as we have only 3 parallel processes started
(We pass in 3 as max parallel processes and 3 is smaller than the spawn
cap in run-command, so we will start the 3 processes in the first run
deterministically), then the abort logic from the new task_finished
callback kicks in and prevents the parallel processing engine to start
any more processes.

As the task_finished is unconditional, we will see its output 3 times, too.

Signed-off-by: Stefan Beller 
---
 t/t0061-run-command.sh | 14 ++
 test-run-command.c | 14 ++
 2 files changed, 28 insertions(+)

diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index 49aa3db..0af77cd 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -67,4 +67,18 @@ test_expect_success 'run_command runs in parallel' '
test_cmp expect actual
 '
 
+cat >expect <<-EOF
+preloaded output of a child
+asking for a quick stop
+preloaded output of a child
+asking for a quick stop
+preloaded output of a child
+asking for a quick stop
+EOF
+
+test_expect_success 'run_command is asked to abort gracefully' '
+   test-run-command run-command-abort-3 false 2>actual &&
+   test_cmp expect actual
+'
+
 test_done
diff --git a/test-run-command.c b/test-run-command.c
index 699d9e9..4b59482 100644
--- a/test-run-command.c
+++ b/test-run-command.c
@@ -34,6 +34,16 @@ static int parallel_next(void** task_cb,
return 1;
 }
 
+static int task_finished(int result,
+struct child_process *cp,
+struct strbuf *err,
+void *pp_cb,
+void *pp_task_cb)
+{
+   strbuf_addf(err, "asking for a quick stop\n");
+   return 1;
+}
+
 int main(int argc, char **argv)
 {
struct child_process proc = CHILD_PROCESS_INIT;
@@ -55,6 +65,10 @@ int main(int argc, char **argv)
exit(run_processes_parallel(4, parallel_next,
NULL, NULL, ));
 
+   if (!strcmp(argv[1], "run-command-abort-3"))
+   exit(run_processes_parallel(3, parallel_next,
+   NULL, task_finished, ));
+
fprintf(stderr, "check usage\n");
return 1;
 }
-- 
2.5.0.285.g8fe9b61.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/5] run-command: Call get_next_task with a clean child process.

[Stefan Beller]

If the `get_next_task` did not explicitly called child_process_init
and only filled in some fields, there may have been some stale data
in the child process. This is hard to debug and also adds a review
burden for each new user of that API. To improve the situation, we
pass only cleanly initialized child structs to the get_next_task.

Signed-off-by: Stefan Beller 
---
 run-command.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/run-command.c b/run-command.c
index 8f47c6e..b8b5747 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1010,6 +1010,8 @@ static int pp_start_one(struct parallel_processes *pp)
if (i == pp->max_processes)
die("BUG: bookkeeping is hard");
 
+   child_process_init(>children[i].process);
+
if (!pp->get_next_task(>children[i].data,
   >children[i].process,
   >children[i].err,
-- 
2.5.0.285.g8fe9b61.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/5] run-command: Initialize the shutdown flag

[Stefan Beller]

It did work out without initializing the flag so far, but to make it
future proof, we want to explicitly initialize the flag.

Signed-off-by: Stefan Beller 
---
 run-command.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/run-command.c b/run-command.c
index b8b5747..108b930 100644
--- a/run-command.c
+++ b/run-command.c
@@ -956,6 +956,7 @@ static struct parallel_processes *pp_init(int n,
 
pp->nr_processes = 0;
pp->output_owner = 0;
+   pp->shutdown = 0;
pp->children = xcalloc(n, sizeof(*pp->children));
pp->pfd = xcalloc(n, sizeof(*pp->pfd));
strbuf_init(>buffered_output, 0);
-- 
2.5.0.285.g8fe9b61.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] URL rewrite in .gitmodules

[Stefan Beller]

On Mon, Oct 19, 2015 at 12:28 PM, Lars Schneider
 wrote:
> Hi,
>
> I have a closed source Git repo which references an Open Source Git repo as 
> Submodule. The Open Source Git repo references yet another Open Source repo 
> as submodule. In order to avoid failing builds due to external services I 
> mirrored the Open Source repos in my company network. That works great with 
> the first level of Submodules. Unfortunately it does not work with the second 
> level because the first level still references the "outside of company" 
> repos. I know I can rewrite Git URLs with the git config 
> "url..insteadOf" option. However, git configs are client specific.

I feel like this is working as intended. You only want to improve your
one client (say the buildbot) to not goto the open source site, while
the developer may do want to fetch from external sources ("Hey shiny
new code!";)


> I would prefer a solution that works without setup on any client. I also know 
> that I could update the .gitmodules file in the Open Source repo on the first 
> level. I also would prefer not to do this as I want to use the very same 
> hashes as defined by the "upstream" Open Source repos.

You could carry a patch on top of the tip of the first submodule
re-pointing the nested submodule. This requires good workflows
available to deal with submodules though. (Fetch and merge or rebase,
git submodule update should be able to do that?)

>
> Is there yet another way in Git to change URLs of Submodules in the way I 
> want it?
>
> If not, what do you think about a patch that adds a "url" section similar to 
> the one in git config to a .gitmodules file?
>

So we have different kinds of git configs. within one repository, in
the home director (global to the one machine),
maybe you would want to have one "global" config on a network share,
such that every box in your company
reads that "company-wide" global config and acts upon that?

> Example:
> --
> [submodule "git"]
> path = git
> url=git://github.com/larsxschneider/git.git
>
> [url "mycompany.com"]
> insteadOf = outside.com

Wouldn't that be better put into say a global git config instead of
repeating it for every submodule?

In case of the nested submodule you would need to carry the last lines
as an extra patch anyway
if this was done in the .gitmodules files? Or do you expect this to be
applied recursively (i.e. nested
submodules all the way down also substitute outside.com)


> --
>

Am I missing your point?

> Thanks,
> Lars--
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] merge: fix cache_entry use-after-free

[David Turner]

On Fri, 2015-10-16 at 09:04 -0700, Junio C Hamano wrote:
> David Turner  writes:
> 
> > We also do dozens or hundreds of merges per day and only saw this quite
> > rarely.  Interestingly, we could only trigger it with an alternate
> > hashing function for git's hashmap implementation, and only once a
> > certain bug in that implementation was *fixed*.  But that was just a
> > trigger; it was certainly not the cause.  The bug would, in general,
> > have caused more hash collisions due to worse mixing, but I believe it
> > is possible that some particular collision would have been present in
> > the non-bugged version of the code but not in the bugged version.
> >
> > It may have also had something to do with a case-insensitive filesystem;
> > we never saw anyone reproduce it on anything but OS X, and even there it
> > was quite difficult to reproduce.
> >
> > In short, I don't think we know why the bug was not seen widely.
> 
> It has been a long time since I looked at name-hash.c and I am fuzzy
> on what the four functions (cache|index)_(file|dir)_exists are meant
> for, but I have this feeling that the original premise of the patch,
> "we may free a ce because we no longer use it in the index, but it
> may still need to keep a reference to it in name-hash" may be wrong
> in the first place.  The proposed "fix" conceptually feels wrong.
>
> The whole point of the name-hash is so that we can detect collisions
> in two names, one of which wants to have a file in one place while
> the other wants to have a directory, at the same path in the index.
> The pathnames and cache entries registered in the name-hash have to
> be the ones that are relevant to the index in quesition.  If a new
> ce will be added to the index, the name-hash will have to know about
> its path (and that is what CE_HASHED bit is about).  On the other
> hand, if you are going to remove an existing ce from the index, its
> sub-paths should no longer prevent other cache entries to be there.
> 
> E.g. if you have "a/b", it must prevent "A" from entering the index
> and the name-hash helps you to do so; when you remove "a/b", then
> name-hash must now allow "A" to enter the index.  So "a/b" must be
> removed from the name-hash by calling remove_name_hash() and normal
> codepaths indeed do so.
>
> I do not doubt the existence of "use-after-free bug" you observed,
> but I am not convinced that refcounting is "fixing" the problem; it
> smells like papering over a different bug that is the root cause of
> the use-after-free.
> 
> For example, if we forget to "unhash" a ce from name-hash when we
> remove a ce from the index (or after we "hashed" it, expecting to
> add it to the index, but in the end decided not to add to the index,
> perhaps), we would see a now-freed ce still in the name-hash.
> Checking a path against the name-hash in such a state would have to
> use the ce->name from that stale ce, which is a use-after-free bug.
> 
> In such a situation, isn't the real cause of the bug the fact that
> the stale ce that is no longer relevant to the true index contents
> still in name-hash?  The refcounting does not fix the fact that a
> ce->name of a stale ce that is no longer relevant being used for D/F
> collision checking.
> 
> I am not saying that I found such a codepath that forgets to unhash,
> but from the overall design and purpose of the name-hash subsystem,
> anything that deliberately _allows_ a stale ce that does not exist
> in the index in there smells like a workaround going in a wrong
> direction.

I've spent some time looking into this, but I don't quite have a repro.
I do have some comments which might be interesting.

The problem is not the name_hash, but with the dir_hash.  The dir_hash
is only even populated on case-insensitive filesystems (which is why you
and Linus don't see this bug).  The dir_hash is not designed to catch
d/f conflicts, but rather case conflicts (one side of a merge has
foo/bar; the other has FOO/bar).  For each directory, it maintains a
pointer to a cache_entry.  Then add_to_index uses that cache_entry to 
rewrite incoming entries' parent directories to the expected case.
Weirdly, that part of add_to_index is, so far as I can tell, never
called during a merge.  So where's are we using the freed value?
Probably in dir_entry_cmp, while adding another entry to the hashmap;
that's where our crash seems to have happened.  And our failure depended
on the details of the hash function as well; if a certain collision did
not happen, we would not see it.

There is, I think, another way to handle this: we could *copy* the path
into the struct dir_entry instead of pointing to a cache_entry.  But
this seems like a bunch of extra work; the refcounting is simpler.  

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: problem with modification time of packfiles

[brian m. carlson]

On Mon, Oct 19, 2015 at 08:59:15PM +0100, Andreas Amann wrote:
> Thank you for your answer.  However, this reasoning only applies to loose
> objects and not packfiles.
> 
> My understanding is that "git prune" will not prune any pack files
> (except those starting with tmp_).  Only "git repack" should do that.
> Repack seems to be however mtime agnostic and therefore it does not seem
> to be necessary to freshen packfiles.
> 
> It therefore seems that git freshens packfiles unnecessarily, which can
> lead to expensive and unnecessary backup operations.
> 
> Given this, would a trivial patch to remove the freshening of packfiles
> be acceptable?

I'm not familiar enough with the code to say for certain, but it looks
like you're right.  Peff, Junio, do you think this is safe, or is there
something we're missing?
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: PGP signature

Re: [PATCH v3 10/34] mailinfo: move global "line" into mailinfo() function

[Eric Sunshine]

On Mon, Oct 19, 2015 at 3:28 AM, Junio C Hamano  wrote:
> With the previous steps, it becomes clear that the mailinfo()
> function is the only one that wants the "line" to be directly
> touchable.  Move it to the function scope of this function.
>
> Signed-off-by: Junio C Hamano 
> ---
> diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
> index 12d1eda..c8dc73f 100644
> --- a/builtin/mailinfo.c
> +++ b/builtin/mailinfo.c
> @@ -12,7 +12,6 @@ static FILE *cmitmsg, *patchfile, *fin, *fout;
>  static int keep_subject;
>  static int keep_non_patch_brackets_in_subject;
>  static const char *metainfo_charset;
> -static struct strbuf line = STRBUF_INIT;
>  static struct strbuf name = STRBUF_INIT;
>  static struct strbuf email = STRBUF_INIT;
>  static char *message_id;
> @@ -966,6 +965,8 @@ static void handle_info(void)
>  static int mailinfo(FILE *in, FILE *out, const char *msg, const char *patch)
>  {
> int peek;
> +   struct strbuf line = STRBUF_INIT;

Does there need to be a corresponding strbuf_release() at the end
of the function?

> fin = in;
> fout = out;
>
> --
> 2.6.2-383-g144b2e6
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 19/34] mailinfo: move check for metainfo_charset to convert_to_utf8()

[Eric Sunshine]

On Mon, Oct 19, 2015 at 3:28 AM, Junio C Hamano  wrote:
> All callers of this function refrains from calling it when

s/refrains/refrain/

> mi->metainfo_charset is NULL; move the check to the callee,
> as it already has a few conditions at its beginning to turn
> it into a no-op.
>
> Signed-off-by: Junio C Hamano 
> ---
> diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
> index 26fd9b1..737d0fc 100644
> --- a/builtin/mailinfo.c
> +++ b/builtin/mailinfo.c
> @@ -524,7 +524,7 @@ static void convert_to_utf8(struct mailinfo *mi,
>  {
> char *out;
>
> -   if (!charset || !*charset)
> +   if (!mi->metainfo_charset || !charset || !*charset)
> return;
>
> if (same_encoding(mi->metainfo_charset, charset))
> @@ -599,8 +599,7 @@ static void decode_header(struct mailinfo *mi, struct 
> strbuf *it)
> dec = decode_q_segment(, 1);
> break;
> }
> -   if (mi->metainfo_charset)
> -   convert_to_utf8(mi, dec, charset_q.buf);
> +   convert_to_utf8(mi, dec, charset_q.buf);
>
> strbuf_addbuf(, dec);
> strbuf_release(dec);
> @@ -745,8 +744,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
> strbuf *line)
> mi->header_stage = 0;
>
> /* normalize the log message to UTF-8. */
> -   if (mi->metainfo_charset)
> -   convert_to_utf8(mi, line, charset.buf);
> +   convert_to_utf8(mi, line, charset.buf);
>
> if (mi->use_scissors && is_scissors_line(line)) {
> int i;
> --
> 2.6.2-383-g144b2e6
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 02/26] refs: make repack_without_refs and is_branch public

[David Turner]

On Fri, 2015-10-16 at 08:34 +0200, Michael Haggerty wrote:
> On 10/15/2015 09:46 PM, David Turner wrote:
> > is_branch was already non-static, but this patch declares it in the
> > header.
> 
> The commit message no longer reflects the patch.
> 
> > Signed-off-by: Ronnie Sahlberg 
> > Signed-off-by: David Turner 
> > Signed-off-by: Junio C Hamano 
> > ---
> >  refs.c | 5 +++--
> >  refs.h | 2 ++
> >  2 files changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/refs.c b/refs.c
> > index fe71ea0..84abc82 100644
> > --- a/refs.c
> > +++ b/refs.c
> > @@ -2816,8 +2816,9 @@ int pack_refs(unsigned int flags)
> >  
> >  /*
> >   * Rewrite the packed-refs file, omitting any refs listed in
> > - * 'refnames'. On error, leave packed-refs unchanged, write an error
> > - * message to 'err', and return a nonzero value.
> > + * 'refnames'. On error, packed-refs will be unchanged, the return
> > + * value is nonzero, and a message about the error is written to the
> > + * 'err' strbuf.
> 
> ^^^ ?
> 
> It is preferable for docstrings to be written in imperative form, so in
> my opinion this is a step backwards...
> 
> ...literally. Your "new" version comes from an older version of Git; it
> was changed in
> 
> 79e4d8a9b8 repack_without_refs(): make function private (2015-06-22)
> 
> to the imperative form.
> 
> Assuming you are using `git-format-patch` to prepare your patches, it is
> always a good idea to read over the prepared email files before sending
> them to the ML, to check for bloopers like this.

Sorry about that one.  It's hard to keep track of what all of these
patches do -- especially the ones that were rebases of Ronnie's.  I've
fixed that and 03/26 as well.

Do you have comments on any of the rest before I re-roll?  

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: problem with modification time of packfiles

[Jeff King]

On Mon, Oct 19, 2015 at 11:09:19PM +, brian m. carlson wrote:

> On Mon, Oct 19, 2015 at 08:59:15PM +0100, Andreas Amann wrote:
> > Thank you for your answer.  However, this reasoning only applies to loose
> > objects and not packfiles.
> > 
> > My understanding is that "git prune" will not prune any pack files
> > (except those starting with tmp_).  Only "git repack" should do that.
> > Repack seems to be however mtime agnostic and therefore it does not seem
> > to be necessary to freshen packfiles.
> > 
> > It therefore seems that git freshens packfiles unnecessarily, which can
> > lead to expensive and unnecessary backup operations.
> > 
> > Given this, would a trivial patch to remove the freshening of packfiles
> > be acceptable?
> 
> I'm not familiar enough with the code to say for certain, but it looks
> like you're right.  Peff, Junio, do you think this is safe, or is there
> something we're missing?

No, it's not safe. When doing a full repack, we pack only reachable
objects. Unreachable ones are either loosened and given the mtime of the
packfile (from which they can then be pruned), or discarded if the pack
mtime is already old (as an optimization to avoid writing and then
immediately pruning).

See builtin/pack-objects.c:loosen_unused_packed_objects.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pull: add angle brackets to usage string

[Junio C Hamano]

Alex Henrie  writes:

> 2015-10-16 11:42 GMT-06:00 Junio C Hamano :
>>
>> Yes, but that fixes historical "mistake", no?
>>
>> With this, you are breaking historical practice by changing only one
>> instance to deviate from the then-current practice of saying
>> 'options' without brackets.  It is based on the point of view that
>> considers anything inside  and a fixed string 'options' are
>> meant to be replaced by intelligent readers, which is as valid as
>> the more recent practice to consider only things inside 
>> are placeholders.
>
> OK, I see. You're saying that it's OK to fix typos and grammatical
> errors in contrib/examples, but it's not okay to modernize the
> scripts' designs.

Please read it again, look at contrib/examples and realize that that
is not what I said at all.

This is not about modern vs old-school.  The reason why the part of
the patch to contrib/ under discussion is wrong is because of
(in)consistency.

Look at the output from "git grep option contrib/examples/" and
notice that in the old days, these scripted Porcelains consistently
said "[options]" without "".

It would have been a different matter if the patch _were_ to update
all "[options]" to "[]" in contrib/examples/ consistently,
and such a patch might have even been an improvement, especially if
the modern style were clearly superiour than the old-school style
(which is not, by they way [*1*]).

But that is not what the patch did.  It turned only one of them into
"[]", making the single instance inconsistent from all the
others around it.  That is why it was wrong.


[Footnote]

*1* The "modern" style is not necessarily an improvement, by the
way.  The way we specify that a "thing" in the help text is a
placeholder and that there may be more instances of the same
"thing" is to say "[...]", but in your "modernized" form,
unlike all the other usual "things", possibly multiple options
are spelled "[]" without having ellipses at the end,
which is an oddball.  If we are to treat options specially like
that anyway, intelligent readers can read an "old-school"
description "[options]" and understand that that token stands
for possibly multiple options just fine, and all we have gained
by going to the "modernized" form is to waste two characters for
.

I am not saying that we should not apply the other half of the
patch that makes builtin/pull.c say "[]".  These days,
many other commands nearby (i.e. the "modern" ones) do use that
form consistently, so it is an improvement.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] merge: fix cache_entry use-after-free

[Junio C Hamano]

David Turner  writes:

> The problem is not the name_hash, but with the dir_hash.  The dir_hash
> is only even populated on case-insensitive filesystems (which is why you
> and Linus don't see this bug).  The dir_hash is not designed to catch
> d/f conflicts, but rather case conflicts (one side of a merge has
> foo/bar; the other has FOO/bar).  For each directory, it maintains a
> pointer to a cache_entry.  Then add_to_index uses that cache_entry to 
> rewrite incoming entries' parent directories to the expected case.
> Weirdly, that part of add_to_index is, so far as I can tell, never
> called during a merge.  So where's are we using the freed value?
> Probably in dir_entry_cmp, while adding another entry to the hashmap;
> that's where our crash seems to have happened.  And our failure depended
> on the details of the hash function as well; if a certain collision did
> not happen, we would not see it.
>
> There is, I think, another way to handle this: we could *copy* the path
> into the struct dir_entry instead of pointing to a cache_entry.  But
> this seems like a bunch of extra work; the refcounting is simpler.  

This codepath is a mess.  Whoever wrote hash_dir_entry() seems to
have already known that the code is buggy by leaving this comment
there:

 * Note that the cache_entry stored with the dir_entry merely
 * supplies the name of the directory (up to dir_entry.namelen). We
 * track the number of 'active' files in a directory in dir_entry.nr,
 * so we can tell if the directory is still relevant, e.g. for git
 * status. However, if cache_entries are removed, we cannot pinpoint
 * an exact cache_entry that's still active. It is very possible that
 * multiple dir_entries point to the same cache_entry.

If you add "a/frotz" to the index, the code will somehow create a
dir_entry to represent that the canonical case for directory "a/" is
such, so that when you later try to add "A/nitfol", the code can
grab the canonical path "a/" from dir_entry hash and turn it into
"a/nitfol".  That is what happens in add_to_index in read-cache.c

But what happens if, after adding "a/frotz" and "a/nitfol" to the
index like that, you remove "a/frotz"?  I do not see any code that
notices the poitner to "a/frotz" will become stale and replace it
with a pointer to "a/nitfol" that is still in the system.  The next
time you try to add "a/xyzzy", find_dir_entry() will try to see if
there is an existing entry that matches "a/xyzzy"'s directory, and
one of the dir_entries has a stale pointer to ce for "a/frotz" that
is already gone.

I think the root cause of the bug is the external interface to the
index_dir_exists() function.  For the above processing, there is no
reason for dir_entry() to hold a pointer to ce for "a/frotz" or
"a/nitfol".  All it needs to have is an embedded string in the
structure itself, i.e.

struct dir_entry {
struct hashmap_entry ent;
struct dir_entry *parent;
int nr;
unsigned int namelen;
char name[FLEX_ARRAY];
};

The caller of index_dir_exists() in add_to_index() can be adjusted
to work with a new function in name-hash.c that does this part:

const char *startPtr = ce->name;
const char *ptr = startPtr;
while (*ptr) {
while (*ptr && *ptr != '/')
++ptr;
if (*ptr == '/') {
struct cache_entry *foundce;
++ptr;
foundce = index_dir_exists(istate, ce->name, ptr - 
ce->name - 1);
if (foundce) {
memcpy((void *)startPtr, foundce->name + 
(startPtr - ce->name), ptr - startPtr);
startPtr = ptr;
}
}
}

Perhaps name it "adjust_dirname_case(istate, ce->name)" and have it
do the whole while loop above, all inside name-hash.c.  That would
make this caller a lot easier to read and understand what is going
on.

The other one, directory_exists_in_index_icase() in dir.c, expects
that a ce is returned, but the way it uses the returned value does
not really require the function to return a ce.  It does look at the
ce->ce_mode but that is only because the way index_dir_exists()
tells its caller if there is a directory (hence some files inside
it) or if there is a submodule (hence there is nothing below it) by
making a fallback call internally to index_file_exists() and returns
the ce for gitlink only when (1) there is no directory in dir_hash
and (2) file_hash as a submodule in that path.

A more cleaner helper function to give what this caller really wants
to know that name-hash.c can provide would be a function that takes
pathname and len and returns an enum: "there is nothing there",
"there is a directory", or "there is a submodule".  For completeness
of the API, even though this sole caller does not need it, we could

Re:My CV

[yanira perico]

Hi there! A friend of mine gave me your email adress.
He was confident you would be very happy to read my CV.

After looking through your homepage and seeing what it is you guys do, I am 
also quite sure
you will not be disappointed. I'm attaching my CV for you to read.

I'm truly looking forward to hearing back from you.
Best regards.
yanira perico


MyCV_83112.doc
Description: MS-Word document

Re: [PATCH v2 3/4] stripspace: Implement --count-lines option

[Christian Couder]

On Mon, Oct 19, 2015 at 3:46 PM, Tobias Klauser  wrote:
> On 2015-10-18 at 19:18:53 +0200, Junio C Hamano  wrote:
>> Eric Sunshine  writes:
>>
>> > Is there any application beyond git-rebase--interactive where a
>> > --count-lines options is expected to be useful? It's not obvious from
>> > the commit message that this change is necessarily a win for later
>> > porting of git-rebase--interactive to C since the amount of extra code
>> > and support material added by this patch probably outweighs the amount
>> > of code a C version of git-rebase--interactive would need to count the
>> > lines itself.
>> >
>> > Stated differently, are the two or three instances of piping through
>> > 'wc' in git-rebase--interactive sufficient justification for
>> > introducing extra complexity into git-stripspace and its documentation
>> > and tests?
>>
>> Interesting thought.  When somebody rewrites "rebase -i" in C,
>> nobody needs to count lines in "stripspace" output.  The rewritten
>> "rebase -i" would internally run strbuf_stripspace() and the question
>> becomes what is the best way to let that code find out how many lines
>> the result contains.
>>
>> When viewed from that angle, I agree that "stripspace --count" does
>> not add anything to further the goal of helping "rebase -i" to move
>> to C.  Adding strbuf_count_lines() that counts the number of lines
>> in the given strbuf (if there is no such helper yet; I didn't check),
>> though.
>
> I check before implementing this series and didn't find any helper. I
> also didn't find any other uses of line counting in the code.

This shows that implementing "git stripspace --count-lines" could
indirectly help porting "git rebase -i" to C as you could implement
strbuf_count_lines() for the former and it could then be reused in the
latter.

> After considering your and Eric's reply, I'll drop these patches for
> now and only resubmit patches 1/4 and 2/4 for v3 (also see my reply to
> Eric).

It would be sad in my opinion.

>> >> +test_expect_success '--count-lines with newline only' '
>> >> +   printf "0\n" >expect &&
>> >> +   printf "\n" | git stripspace --count-lines >actual &&
>> >> +   test_cmp expect actual
>> >> +'
>> >
>> > What is the expected behavior when the input is an empty file, a file
>> > with content but no newline, a file with one or more lines but lacking
>> > a newline on the final line? Should these cases be tested, as well?
>>
>> Good point here, too.  If we were to add strbuf_count_lines()
>> helper, whoever adds that function needs to take a possible
>> incomplete line at the end into account.
>
> Yes, makes more sense like this (even though it doesn't correspond to
> what 'wc -l' does).

Tests for "git stripspace --count-lines" would test
strbuf_count_lines() which would also help when porting git rebase -i
to C.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 10/34] mailinfo: move global "line" into mailinfo() function

[Junio C Hamano]

Eric Sunshine  writes:

> On Mon, Oct 19, 2015 at 3:28 AM, Junio C Hamano  wrote:
>> With the previous steps, it becomes clear that the mailinfo()
>> function is the only one that wants the "line" to be directly
>> touchable.  Move it to the function scope of this function.
>>
>> Signed-off-by: Junio C Hamano 
>> ---
>> diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
>> index 12d1eda..c8dc73f 100644
>> --- a/builtin/mailinfo.c
>> +++ b/builtin/mailinfo.c
>> @@ -12,7 +12,6 @@ static FILE *cmitmsg, *patchfile, *fin, *fout;
>>  static int keep_subject;
>>  static int keep_non_patch_brackets_in_subject;
>>  static const char *metainfo_charset;
>> -static struct strbuf line = STRBUF_INIT;
>>  static struct strbuf name = STRBUF_INIT;
>>  static struct strbuf email = STRBUF_INIT;
>>  static char *message_id;
>> @@ -966,6 +965,8 @@ static void handle_info(void)
>>  static int mailinfo(FILE *in, FILE *out, const char *msg, const char *patch)
>>  {
>> int peek;
>> +   struct strbuf line = STRBUF_INIT;
>
> Does there need to be a corresponding strbuf_release() at the end
> of the function?

Indeed.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git bug report: git doesn't change working directory

[Johannes Schindelin]

Hi Khải,

On Sat, 17 Oct 2015, Khải wrote:

> I'm using Windows 10. Before I install git 2.6.1.windows.1, I have
> installed git 1.9.5.github.0 (by installing GitHub Desktop), it works
> just fine.

Good.

> But when I installed git 2.6.1.windows.1 (from git-scm.com), I'm not
> able to use git anymore:
>
>  - The powershell console displayed [(unknown)] instead of [master], even 
> when I changed working directory to my project, it still display 
> [(unknown)]
>
>  - When I "git add" (or "git commit", "git push"), It told me an error: 
> "fatal: Not a git repository: 'C:\Program Files\Git'"

Unfortunately, this is not enough information. Which options did you
choose during installation? How do you modify the PATH to be able to run
Git from Powershell? What code do you use to display `[(unknown)]`
(Powershell has *no* Git integration by default)?

Ciao,
Johannes