* Re: [PATCH] bisect: test merge base if good rev is not an ancestor of bad rev
From: Junio C Hamano @ 2008-07-10 23:50 UTC (permalink / raw)
To: Christian Couder; +Cc: Johannes Schindelin, Michael Haggerty, Jeff King, git
In-Reply-To: <200807110145.33820.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> Le vendredi 11 juillet 2008, Junio C Hamano a écrit :
>>
>> (2) Good
>> ?---o (maint)
>> /
>> ---x---?---?---?---x (master)
>> Bad Bad
>>
>>
>> If (1), you go ahead with the usual bisection. If (2), you cannot even
>> bisect. Instead, you flip good and bad to find the "fix" in the side
>> branch (the answer has to be either the tip of maint or one previous in
>> the picture) to forward port to, either by merging 'maint' to 'master' or
>> cherry-picking.
>>
>> The idea to check merge-base first is about automating this process (I
>> admit I still haven't looked at Christian's patch text yet).
>
> Well in case (2) my patch does:
>
> -------
> cat >&2 <<EOF
> The merge base $_badmb is bad.
> This means the bug has been fixed between $_badmb and $_g.
> EOF
> exit 3
> -------
>
> but this can be improved upon in some latter patches.
I think such an "improvement" is getting close to being too clever. I
should have worded my description on what you would do in (2) a bit more
carefully.
If (2), you cannot even bisect.
Instead, you may decide to merge 'maint' to 'master' to get that fix.
In which case you do not have to worry about it; you do not do
anything further.
If you cannot afford to merge 'maint' to 'master' but somehow need to
forward port the fix by cherry-picking, you need to flip good and bad
to find the "fix" in the side branch (the answer has to be either the
tip of maint or one previous in the picture).
^ permalink raw reply
* Re: feature request: git-log should accept sth like v2.6.26-rc8-227
From: Johannes Schindelin @ 2008-07-10 23:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jakub Narebski, Toralf Förster, git
In-Reply-To: <7vk5ftnyhp.fsf@gitster.siamese.dyndns.org>
Hi,
On Thu, 10 Jul 2008, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > On Thu, 10 Jul 2008, Junio C Hamano wrote:
> >
> >> Jakub Narebski <jnareb@gmail.com> writes:
> >>
> >> > Besides, it would be nice to have some command (git-rev-parse
> >> > perhaps?) which could take ambiguous commit-ish, and list all commit
> >> > which matches it.
> >>
> >> Have fun writing it and send in a patch.
> >
> > Note that this really could be a patch, but not for rev-parse. Patch
> > revision.c instead to parse the argument into _all_ matching revisions.
>
> As Linus pointed out, that is "all _locally_ matching revisions". It is
> of dubious value in a distributed environment.
Right. Judging from some of the conversations on IRC (and even on this
list), it seems that the word "local" for the reflogs is lost on some, and
there is no reason to expect otherwise for the currently proposed thing.
So strike my suggestions,
Dscho
^ permalink raw reply
* [PATCH/RFC] git-mailinfo: use strbuf's instead of fixed buffers
From: Lukas Sandström @ 2008-07-10 23:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Lukas Sandström, Git Mailing List
In-Reply-To: <48769E40.8030303@etek.chalmers.se>
Signed-off-by: Lukas Sandström <lukass@etek.chalmers.se>
---
builtin-mailinfo.c | 705 +++++++++++++++++++++++++---------------------------
1 files changed, 333 insertions(+), 372 deletions(-)
diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index 2d1520f..254a97c 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -5,14 +5,15 @@
#include "cache.h"
#include "builtin.h"
#include "utf8.h"
+#include "strbuf.h"
static FILE *cmitmsg, *patchfile, *fin, *fout;
static int keep_subject;
static const char *metainfo_charset;
-static char line[1000];
-static char name[1000];
-static char email[1000];
+static struct strbuf line = STRBUF_INIT;
+static struct strbuf name = STRBUF_INIT;
+static struct strbuf email = STRBUF_INIT;
static enum {
TE_DONTCARE, TE_QP, TE_BASE64,
@@ -21,74 +22,74 @@ static enum {
TYPE_TEXT, TYPE_OTHER,
} message_type;
-static char charset[256];
+static struct strbuf charset = STRBUF_INIT;
static int patch_lines;
-static char **p_hdr_data, **s_hdr_data;
+static struct strbuf **p_hdr_data, **s_hdr_data;
#define MAX_HDR_PARSED 10
#define MAX_BOUNDARIES 5
-static char *sanity_check(char *name, char *email)
+static void sanity_check(struct strbuf *out, struct strbuf *name, struct strbuf *email)
{
- int len = strlen(name);
- if (len < 3 || len > 60)
- return email;
- if (strchr(name, '@') || strchr(name, '<') || strchr(name, '>'))
- return email;
- return name;
+ struct strbuf o = STRBUF_INIT;
+ if (name->len < 3 || name->len > 60)
+ strbuf_addbuf(&o, email);
+ if (strchr(name->buf, '@') || strchr(name->buf, '<') ||
+ strchr(name->buf, '>'))
+ strbuf_addbuf(&o, email);
+ strbuf_addbuf(&o, name);
+ strbuf_reset(out);
+ strbuf_addbuf(out, &o);
+ strbuf_release(&o);
}
-static int bogus_from(char *line)
+static int bogus_from(const struct strbuf *line)
{
/* John Doe <johndoe> */
- char *bra, *ket, *dst, *cp;
+ char *bra, *ket;
/* This is fallback, so do not bother if we already have an
* e-mail address.
*/
- if (*email)
+ if (email.len)
return 0;
- bra = strchr(line, '<');
+ bra = strchr(line->buf, '<');
if (!bra)
return 0;
ket = strchr(bra, '>');
if (!ket)
return 0;
- for (dst = email, cp = bra+1; cp < ket; )
- *dst++ = *cp++;
- *dst = 0;
- for (cp = line; isspace(*cp); cp++)
- ;
- for (bra--; isspace(*bra); bra--)
- *bra = 0;
- cp = sanity_check(cp, email);
- strcpy(name, cp);
+ 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);
+ sanity_check(&name, &name, &email);
return 1;
}
-static int handle_from(char *in_line)
+static int handle_from(struct strbuf *from)
{
- char line[1000];
char *at;
- char *dst;
+ size_t el;
- strcpy(line, in_line);
- at = strchr(line, '@');
+ at = strchr(from->buf, '@');
if (!at)
- return bogus_from(line);
+ return bogus_from(from);
/*
* If we already have one email, don't take any confusing lines
*/
- if (*email && strchr(at+1, '@'))
+ if (email.len && strchr(at + 1, '@'))
return 0;
/* Pick up the string around '@', possibly delimited with <>
- * pair; that is the email part. White them out while copying.
+ * pair; that is the email part.
*/
- while (at > line) {
+ while (at > from->buf) {
char c = at[-1];
if (isspace(c))
break;
@@ -98,56 +99,35 @@ static int handle_from(char *in_line)
}
at--;
}
- dst = email;
- for (;;) {
- unsigned char c = *at;
- if (!c || c == '>' || isspace(c)) {
- if (c == '>')
- *at = ' ';
- break;
- }
- *at++ = ' ';
- *dst++ = c;
- }
- *dst++ = 0;
-
+ el = strcspn(at, " \n\t\r\v\f>");
+ strbuf_reset(&email);
+ strbuf_add(&email, at, el);
+ strbuf_remove(from, at - from->buf, el + 1);
/* The remainder is name. It could be "John Doe <john.doe@xz>"
* or "john.doe@xz (John Doe)", but we have whited out the
* email part, so trim from both ends, possibly removing
* the () pair at the end.
*/
- at = line + strlen(line);
- while (at > line) {
- unsigned char c = *--at;
- if (!isspace(c)) {
- at[(c == ')') ? 0 : 1] = 0;
- break;
- }
- }
- at = line;
- for (;;) {
- unsigned char c = *at;
- if (!c || !isspace(c)) {
- if (c == '(')
- at++;
- break;
- }
- at++;
- }
- at = sanity_check(at, email);
- strcpy(name, at);
+ strbuf_trim(from);
+ if (*from->buf == '(')
+ strbuf_remove(&name, 0, 1);
+ if (*(from->buf + from->len - 1) == ')')
+ strbuf_setlen(from, from->len - 1);
+
+ sanity_check(&name, from, &email);
return 1;
}
-static int handle_header(char *line, char *data, int ofs)
+static void handle_header(struct strbuf **out, const struct strbuf *line)
{
- if (!line || !data)
- return 1;
-
- strcpy(data, line+ofs);
+ if (!*out) {
+ *out = xmalloc(sizeof(struct strbuf));
+ strbuf_init(*out, line->len);
+ } else
+ strbuf_reset(*out);
- return 0;
+ strbuf_addbuf(*out, (struct strbuf *)line); /* const warning */
}
/* NOTE NOTE NOTE. We do not claim we do full MIME. We just attempt
@@ -156,13 +136,13 @@ static int handle_header(char *line, char *data, int ofs)
* case insensitively.
*/
-static int slurp_attr(const char *line, const char *name, char *attr)
+static int slurp_attr(const char *line, const char *name, struct strbuf *attr)
{
const char *ends, *ap = strcasestr(line, name);
size_t sz;
if (!ap) {
- *attr = 0;
+ strbuf_setlen(attr, 0);
return 0;
}
ap += strlen(name);
@@ -173,180 +153,176 @@ static int slurp_attr(const char *line, const char *name, char *attr)
else
ends = "; \t";
sz = strcspn(ap, ends);
- memcpy(attr, ap, sz);
- attr[sz] = 0;
+ strbuf_add(attr, ap, sz);
return 1;
}
struct content_type {
- char *boundary;
- int boundary_len;
+ struct strbuf *boundary;
};
static struct content_type content[MAX_BOUNDARIES];
static struct content_type *content_top = content;
-static int handle_content_type(char *line)
+static int handle_content_type(struct strbuf *line)
{
- char boundary[256];
+ struct strbuf *boundary = xmalloc(sizeof(struct strbuf));
+ strbuf_init(boundary, line->len);
- if (strcasestr(line, "text/") == NULL)
+ if (!strcasestr(line->buf, "text/"))
message_type = TYPE_OTHER;
- if (slurp_attr(line, "boundary=", boundary + 2)) {
- memcpy(boundary, "--", 2);
+ if (slurp_attr(line->buf, "boundary=", boundary)) {
+ strbuf_insert(boundary, 0, "--", 2);
if (content_top++ >= &content[MAX_BOUNDARIES]) {
fprintf(stderr, "Too many boundaries to handle\n");
exit(1);
}
- content_top->boundary_len = strlen(boundary);
- content_top->boundary = xmalloc(content_top->boundary_len+1);
- strcpy(content_top->boundary, boundary);
- }
- if (slurp_attr(line, "charset=", charset)) {
- int i, c;
- for (i = 0; (c = charset[i]) != 0; i++)
- charset[i] = tolower(c);
+ content_top->boundary = boundary;
+ } else {
+ strbuf_release(boundary);
+ free(boundary);
}
+ if (slurp_attr(line->buf, "charset=", &charset))
+ strbuf_tolower(&charset);
return 0;
}
-static int handle_content_transfer_encoding(char *line)
+static int handle_content_transfer_encoding(struct strbuf *line)
{
- if (strcasestr(line, "base64"))
+ if (strcasestr(line->buf, "base64"))
transfer_encoding = TE_BASE64;
- else if (strcasestr(line, "quoted-printable"))
+ else if (strcasestr(line->buf, "quoted-printable"))
transfer_encoding = TE_QP;
else
transfer_encoding = TE_DONTCARE;
return 0;
}
-static int is_multipart_boundary(const char *line)
-{
- return (!memcmp(line, content_top->boundary, content_top->boundary_len));
-}
-
-static int eatspace(char *line)
+static int is_multipart_boundary(struct strbuf *line)
{
- int len = strlen(line);
- while (len > 0 && isspace(line[len-1]))
- line[--len] = 0;
- return len;
+ return !strbuf_cmp(line, content_top->boundary);
}
-static char *cleanup_subject(char *subject)
+static void cleanup_subject(struct strbuf *subject)
{
- for (;;) {
- char *p;
- int len, remove;
- switch (*subject) {
+ char *pos;
+ size_t remove;
+ while (subject->len) {
+ switch (*subject->buf) {
case 'r': case 'R':
- if (!memcmp("e:", subject+1, 2)) {
- subject += 3;
+ if (subject->len <= 3)
+ break;
+ if (!memcmp(subject->buf + 1, "e:", 2)) {
+ strbuf_remove(subject, 0, 3);
continue;
}
break;
case ' ': case '\t': case ':':
- subject++;
+ strbuf_remove(subject, 0, 1);
continue;
-
+ break;
case '[':
- p = strchr(subject, ']');
- if (!p) {
- subject++;
- continue;
- }
- len = strlen(p);
- remove = p - subject;
- if (remove <= len *2) {
- subject = p+1;
- continue;
- }
+ if ((pos = strchr(subject->buf, ']'))) {
+ remove = pos - subject->buf + 1;
+ /* Don't remove too much. */
+ if (remove <= (subject->len - remove + 1) * 2) {
+ strbuf_remove(subject, 0, remove);
+ continue;
+ }
+ } else
+ strbuf_remove(subject, 0, 1);
break;
}
- eatspace(subject);
- return subject;
+ strbuf_trim(subject);
+ return;
}
}
-static void cleanup_space(char *buf)
+static void cleanup_space(struct strbuf *sb)
{
- unsigned char c;
- while ((c = *buf) != 0) {
- buf++;
- if (isspace(c)) {
- buf[-1] = ' ';
- c = *buf;
- while (isspace(c)) {
- int len = strlen(buf);
- memmove(buf, buf+1, len);
- c = *buf;
- }
+ 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 decode_header(char *it, unsigned itsize);
+static void decode_header(struct strbuf *line);
static const char *header[MAX_HDR_PARSED] = {
"From","Subject","Date",
};
-static int check_header(char *line, unsigned linesize, char **hdr_data, int overwrite)
+static int cmp_header(const struct strbuf *line, const char *hdr)
{
- int i;
+ int len = strlen(hdr);
+ return !strncasecmp(line->buf, hdr, len) && line->len > len &&
+ line->buf[len] == ':' && isspace(line->buf[len + 1]);
+}
+static int check_header(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) &&
- !strncasecmp(line, header[i], len) &&
- line[len] == ':' && isspace(line[len + 1])) {
+ if ((!hdr_data[i] || overwrite) && cmp_header(line, header[i])) {
/* Unwrap inline B and Q encoding, and optionally
* normalize the meta information to utf8.
*/
- decode_header(line + len + 2, linesize - len - 2);
- hdr_data[i] = xmalloc(1000 * sizeof(char));
- if (! handle_header(line, hdr_data[i], len + 2)) {
- return 1;
- }
+ strbuf_add(&sb, line->buf + len + 2, line->len - len -2);
+ decode_header(&sb);
+ handle_header(&hdr_data[i], &sb);
+ ret = 1;
+ goto check_header_out;
}
}
/* Content stuff */
- if (!strncasecmp(line, "Content-Type", 12) &&
- line[12] == ':' && isspace(line[12 + 1])) {
- decode_header(line + 12 + 2, linesize - 12 - 2);
- if (! handle_content_type(line)) {
- return 1;
- }
- }
- if (!strncasecmp(line, "Content-Transfer-Encoding", 25) &&
- line[25] == ':' && isspace(line[25 + 1])) {
- decode_header(line + 25 + 2, linesize - 25 - 2);
- if (! handle_content_transfer_encoding(line)) {
- return 1;
- }
+ if (cmp_header(line, "Content-Type")) {
+ len = strlen("Content-Type: ");
+ strbuf_reset(&sb);
+ strbuf_add(&sb, line->buf + len, line->len - len);
+ decode_header(&sb);
+ strbuf_insert(&sb, 0, "Content-Type: ", len);
+ if (!handle_content_type(&sb))
+ ret = 1;
+ goto check_header_out;
+ }
+ if (cmp_header(line, "Content-Transfer-Encoding")) {
+ len = strlen("Content-Transfer-Encoding: ");
+ strbuf_reset(&sb);
+ strbuf_add(&sb, line->buf + len, line->len - len);
+ decode_header(&sb);
+ if (!handle_content_transfer_encoding(&sb))
+ ret = 1;
+ goto check_header_out;
}
/* for inbody stuff */
- if (!memcmp(">From", line, 5) && isspace(line[5]))
- return 1;
- if (!memcmp("[PATCH]", line, 7) && isspace(line[7])) {
+ if (!prefixcmp(line->buf, ">From") && isspace(line->buf[5]))
+ ret = 1;
+ goto check_header_out;
+ if (!prefixcmp(line->buf, "[PATCH]") && isspace(line->buf[7])) {
for (i = 0; header[i]; i++) {
if (!memcmp("Subject", header[i], 7)) {
- if (! handle_header(line, hdr_data[i], 0)) {
- return 1;
- }
+ handle_header(&hdr_data[i], line);
+ ret = 1;
+ goto check_header_out;
}
}
}
- /* no match */
- return 0;
+check_header_out:
+ strbuf_release(&sb);
+ return ret;
}
-static int is_rfc2822_header(char *line)
+static int is_rfc2822_header(const struct strbuf *line)
{
/*
* The section that defines the loosest possible
@@ -357,15 +333,15 @@ static int is_rfc2822_header(char *line)
* ftext = %d33-57 / %59-126
*/
int ch;
- char *cp = line;
+ char *cp = line->buf;
/* Count mbox From headers as headers */
- if (!memcmp(line, "From ", 5) || !memcmp(line, ">From ", 6))
+ if (line->len >= 6 && (!memcmp(cp, "From ", 5) || !memcmp(cp, ">From ", 6)))
return 1;
while ((ch = *cp++)) {
if (ch == ':')
- return cp != line;
+ return 1;
if ((33 <= ch && ch <= 57) ||
(59 <= ch && ch <= 126))
continue;
@@ -374,34 +350,20 @@ static int is_rfc2822_header(char *line)
return 0;
}
-/*
- * sz is size of 'line' buffer in bytes. Must be reasonably
- * long enough to hold one physical real-world e-mail line.
- */
-static int read_one_header_line(char *line, int sz, FILE *in)
+static int read_one_header_line(struct strbuf *line, FILE *in)
{
- int len;
-
- /*
- * We will read at most (sz-1) bytes and then potentially
- * re-add NUL after it. Accessing line[sz] after this is safe
- * and we can allow len to grow up to and including sz.
- */
- sz--;
-
/* Get the first part of the line. */
- if (!fgets(line, sz, in))
+ 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")
*/
- len = eatspace(line);
- if (!len || !is_rfc2822_header(line)) {
+ strbuf_rtrim(line);
+ if (!line->len || !is_rfc2822_header(line)) {
/* Re-add the newline */
- line[len] = '\n';
- line[len + 1] = '\0';
+ strbuf_addch(line, '\n');
return 0;
}
@@ -410,65 +372,53 @@ static int read_one_header_line(char *line, int sz, FILE *in)
* Yuck, 2822 header "folding"
*/
for (;;) {
- int peek, addlen;
- static char continuation[1000];
+ int peek;
+ struct strbuf continuation = STRBUF_INIT;
peek = fgetc(in); ungetc(peek, in);
if (peek != ' ' && peek != '\t')
break;
- if (!fgets(continuation, sizeof(continuation), in))
+ if (strbuf_getline(&continuation, in, '\n'))
break;
- addlen = eatspace(continuation);
- if (len < sz - 1) {
- if (addlen >= sz - len)
- addlen = sz - len - 1;
- memcpy(line + len, continuation, addlen);
- line[len] = '\n';
- len += addlen;
- }
+ continuation.buf[0] = '\n';
+ strbuf_rtrim(&continuation);
+ strbuf_addbuf(line, &continuation);
}
- line[len] = 0;
return 1;
}
-static int decode_q_segment(char *in, char *ot, unsigned otsize, char *ep, int rfc2047)
+static struct strbuf *decode_q_segment(const struct strbuf *q_seg, int rfc2047)
{
- char *otbegin = ot;
- char *otend = ot + otsize;
+ const char *in = q_seg->buf;
int c;
- while ((c = *in++) != 0 && (in <= ep)) {
- if (ot == otend) {
- *--ot = '\0';
- return -1;
- }
+ struct strbuf *out = xmalloc(sizeof(struct strbuf));
+ strbuf_init(out, q_seg->len);
+
+ while ((c = *in++) != 0) {
if (c == '=') {
int d = *in++;
if (d == '\n' || !d)
break; /* drop trailing newline */
- *ot++ = ((hexval(d) << 4) | hexval(*in++));
+ strbuf_addch(out, (hexval(d) << 4) | hexval(*in++));
continue;
}
if (rfc2047 && c == '_') /* rfc2047 4.2 (2) */
c = 0x20;
- *ot++ = c;
+ strbuf_addch(out, c);
}
- *ot = 0;
- return (ot - otbegin);
+ return out;
}
-static int decode_b_segment(char *in, char *ot, unsigned otsize, char *ep)
+static struct strbuf *decode_b_segment(const struct strbuf *b_seg)
{
/* Decode in..ep, possibly in-place to ot */
int c, pos = 0, acc = 0;
- char *otbegin = ot;
- char *otend = ot + otsize;
+ const char *in = b_seg->buf;
+ struct strbuf *out = xmalloc(sizeof(struct strbuf));
+ strbuf_init(out, b_seg->len);
- while ((c = *in++) != 0 && (in <= ep)) {
- if (ot == otend) {
- *--ot = '\0';
- return -1;
- }
+ while ((c = *in++) != 0) {
if (c == '+')
c = 62;
else if (c == '/')
@@ -493,21 +443,20 @@ static int decode_b_segment(char *in, char *ot, unsigned otsize, char *ep)
acc = (c << 2);
break;
case 1:
- *ot++ = (acc | (c >> 4));
+ strbuf_addch(out, (acc | (c >> 4)));
acc = (c & 15) << 4;
break;
case 2:
- *ot++ = (acc | (c >> 2));
+ strbuf_addch(out, (acc | (c >> 2)));
acc = (c & 3) << 6;
break;
case 3:
- *ot++ = (acc | c);
+ strbuf_addch(out, (acc | c));
acc = pos = 0;
break;
}
}
- *ot = 0;
- return (ot - otbegin);
+ return out;
}
/*
@@ -521,16 +470,16 @@ static int decode_b_segment(char *in, char *ot, unsigned otsize, char *ep)
* Otherwise, we default to assuming it is Latin1 for historical
* reasons.
*/
-static const char *guess_charset(const char *line, const char *target_charset)
+static const char *guess_charset(const struct strbuf *line, const char *target_charset)
{
if (is_encoding_utf8(target_charset)) {
- if (is_utf8(line))
+ if (is_utf8(line->buf))
return NULL;
}
return "latin1";
}
-static void convert_to_utf8(char *line, unsigned linesize, const char *charset)
+static void convert_to_utf8(struct strbuf *line, const char *charset)
{
char *out;
@@ -542,112 +491,119 @@ static void convert_to_utf8(char *line, unsigned linesize, const char *charset)
if (!strcmp(metainfo_charset, charset))
return;
- out = reencode_string(line, metainfo_charset, charset);
+ out = reencode_string(line->buf, metainfo_charset, charset);
if (!out)
die("cannot convert from %s to %s\n",
charset, metainfo_charset);
- strlcpy(line, out, linesize);
- free(out);
+ strbuf_attach(line, out, strlen(out), strlen(out));
}
-static int decode_header_bq(char *it, unsigned itsize)
+static int decode_header_bq(struct strbuf *it)
{
char *in, *out, *ep, *cp, *sp;
- char outbuf[1000];
+ struct strbuf outbuf = STRBUF_INIT, *dec;
+ struct strbuf charset_q = STRBUF_INIT, piecebuf = STRBUF_INIT;
int rfc2047 = 0;
- in = it;
- out = outbuf;
- while ((ep = strstr(in, "=?")) != NULL) {
- int sz, encoding;
- char charset_q[256], piecebuf[256];
+ in = it->buf;
+ while (in - it->buf <= it->len && (ep = strstr(in, "=?")) != NULL) {
+ int encoding;
+ strbuf_reset(&charset_q);
+ strbuf_reset(&piecebuf);
rfc2047 = 1;
if (in != ep) {
- sz = ep - in;
- memcpy(out, in, sz);
- out += sz;
- in += sz;
+ strbuf_add(&outbuf, in, ep - in);
+ in = ep;
}
/* E.g.
* ep : "=?iso-2022-jp?B?GyR...?= foo"
* ep : "=?ISO-8859-1?Q?Foo=FCbar?= baz"
*/
ep += 2;
- cp = strchr(ep, '?');
- if (!cp)
- return rfc2047; /* no munging */
- for (sp = ep; sp < cp; sp++)
- charset_q[sp - ep] = tolower(*sp);
- charset_q[cp - ep] = 0;
+
+ if (ep - it->buf >= it->len || !(cp = strchr(ep, '?')))
+ goto decode_header_bq_out;
+
+ if (cp + 3 - it->buf > it->len)
+ goto decode_header_bq_out;
+ strbuf_add(&charset_q, ep, cp - ep);
+ strbuf_tolower(&charset_q);
+
encoding = cp[1];
if (!encoding || cp[2] != '?')
- return rfc2047; /* no munging */
+ goto decode_header_bq_out;
ep = strstr(cp + 3, "?=");
if (!ep)
- return rfc2047; /* no munging */
+ goto decode_header_bq_out;
+ strbuf_add(&piecebuf, cp + 3, ep - cp - 3);
switch (tolower(encoding)) {
default:
- return rfc2047; /* no munging */
+ goto decode_header_bq_out;
case 'b':
- sz = decode_b_segment(cp + 3, piecebuf, sizeof(piecebuf), ep);
+ dec = decode_b_segment(&piecebuf);
break;
case 'q':
- sz = decode_q_segment(cp + 3, piecebuf, sizeof(piecebuf), ep, 1);
+ dec = decode_q_segment(&piecebuf, 1);
break;
}
- if (sz < 0)
- return rfc2047;
if (metainfo_charset)
- convert_to_utf8(piecebuf, sizeof(piecebuf), charset_q);
+ convert_to_utf8(dec, charset_q.buf);
- sz = strlen(piecebuf);
- if (outbuf + sizeof(outbuf) <= out + sz)
- return rfc2047; /* no munging */
- strcpy(out, piecebuf);
- out += sz;
+ strbuf_addbuf(&outbuf, dec);
+ strbuf_release(dec);
+ free(dec);
in = ep + 2;
}
- strcpy(out, in);
- strlcpy(it, outbuf, itsize);
+ strbuf_addstr(&outbuf, in);
+ strbuf_reset(it);
+ strbuf_addbuf(it, &outbuf);
+decode_header_bq_out:
+ strbuf_release(&outbuf);
+ strbuf_release(&charset_q);
+ strbuf_release(&piecebuf);
return rfc2047;
}
-static void decode_header(char *it, unsigned itsize)
+static void decode_header(struct strbuf *it)
{
-
- if (decode_header_bq(it, itsize))
+ 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, itsize, "");
+ convert_to_utf8(it, "");
}
-static int decode_transfer_encoding(char *line, unsigned linesize, int inputlen)
+static void decode_transfer_encoding(struct strbuf *line)
{
- char *ep;
+ struct strbuf *ret;
+ int len;
switch (transfer_encoding) {
case TE_QP:
- ep = line + inputlen;
- return decode_q_segment(line, line, linesize, ep, 0);
+ ret = decode_q_segment(line, 0);
+ break;
case TE_BASE64:
- ep = line + inputlen;
- return decode_b_segment(line, line, linesize, ep);
+ ret = decode_b_segment(line);
+ break;
case TE_DONTCARE:
default:
- return inputlen;
+ return;
}
+ strbuf_reset(line);
+ strbuf_addbuf(line, ret);
+ strbuf_release(ret);
+ free(ret);
}
-static int handle_filter(char *line, unsigned linesize, int linelen);
+static int handle_filter(struct strbuf *line);
static int find_boundary(void)
{
- while(fgets(line, sizeof(line), fin) != NULL) {
- if (is_multipart_boundary(line))
+ while(!strbuf_getline(&line, fin, '\n')) {
+ if (is_multipart_boundary(&line))
return 1;
}
return 0;
@@ -655,11 +611,15 @@ static int find_boundary(void)
static int handle_boundary(void)
{
- char newline[]="\n";
+ struct strbuf newline = STRBUF_INIT;
+
+ strbuf_addch(&newline, '\n');
again:
- if (!memcmp(line+content_top->boundary_len, "--", 2)) {
+ if (line.len >= content_top->boundary->len + 2 &&
+ !memcmp(line.buf + content_top->boundary->len, "--", 2)) {
/* we hit an end boundary */
/* pop the current boundary off the stack */
+ strbuf_release(content_top->boundary);
free(content_top->boundary);
/* technically won't happen as is_multipart_boundary()
@@ -670,7 +630,8 @@ again:
"can't recover\n");
exit(1);
}
- handle_filter(newline, sizeof(newline), strlen(newline));
+ handle_filter(&newline);
+ strbuf_release(&newline);
/* skip to the next boundary */
if (!find_boundary())
@@ -680,39 +641,44 @@ again:
/* set some defaults */
transfer_encoding = TE_DONTCARE;
- charset[0] = 0;
+ strbuf_reset(&charset);
message_type = TYPE_TEXT;
/* slurp in this section's info */
- while (read_one_header_line(line, sizeof(line), fin))
- check_header(line, sizeof(line), p_hdr_data, 0);
+ while (read_one_header_line(&line, fin))
+ check_header(&line, p_hdr_data, 0);
+ strbuf_release(&newline);
/* eat the blank line after section info */
- return (fgets(line, sizeof(line), fin) != NULL);
+ return (strbuf_getline(&line, fin, '\n') == 0);
}
-static inline int patchbreak(const char *line)
+static inline int patchbreak(const struct strbuf *line)
{
+ size_t i;
+
/* Beginning of a "diff -" header? */
- if (!memcmp("diff -", line, 6))
+ if (!prefixcmp(line->buf, "diff -"))
return 1;
/* CVS "Index: " line? */
- if (!memcmp("Index: ", line, 7))
+ if (!prefixcmp(line->buf, "Index: "))
return 1;
/*
* "--- <filename>" starts patches without headers
* "---<sp>*" is a manual separator
*/
- if (!memcmp("---", line, 3)) {
- line += 3;
+ if (line->len < 4)
+ return 0;
+
+ if (!prefixcmp(line->buf, "---")) {
/* space followed by a filename? */
- if (line[0] == ' ' && !isspace(line[1]))
+ if (line->buf[3] == ' ' && !isspace(line->buf[4]))
return 1;
/* Just whitespace? */
- for (;;) {
- unsigned char c = *line++;
+ for (i = 3; i < line->len; i++) {
+ unsigned char c = line->buf[i];
if (c == '\n')
return 1;
if (!isspace(c))
@@ -723,32 +689,25 @@ static inline int patchbreak(const char *line)
return 0;
}
-
-static int handle_commit_msg(char *line, unsigned linesize)
+static int handle_commit_msg(struct strbuf *line)
{
static int still_looking = 1;
- char *endline = line + linesize;
+ char *c;
if (!cmitmsg)
return 0;
if (still_looking) {
- char *cp = line;
- if (isspace(*line)) {
- for (cp = line + 1; *cp; cp++) {
- if (!isspace(*cp))
- break;
- }
- if (!*cp)
- return 0;
- }
- if ((still_looking = check_header(cp, endline - cp, s_hdr_data, 0)) != 0)
+ strbuf_ltrim(line);
+ if (!line->len)
+ return 0;
+ if ((still_looking = check_header(line, s_hdr_data, 0)) != 0)
return 0;
}
/* normalize the log message to UTF-8. */
if (metainfo_charset)
- convert_to_utf8(line, endline - line, charset);
+ convert_to_utf8(line, charset.buf);
if (patchbreak(line)) {
fclose(cmitmsg);
@@ -756,18 +715,18 @@ static int handle_commit_msg(char *line, unsigned linesize)
return 1;
}
- fputs(line, cmitmsg);
+ fputs(line->buf, cmitmsg);
return 0;
}
-static int handle_patch(char *line, int len)
+static int handle_patch(const struct strbuf *line)
{
- fwrite(line, 1, len, patchfile);
+ fwrite(line->buf, 1, line->len, patchfile);
patch_lines++;
return 0;
}
-static int handle_filter(char *line, unsigned linesize, int linelen)
+static int handle_filter(struct strbuf *line)
{
static int filter = 0;
@@ -776,11 +735,11 @@ static int handle_filter(char *line, unsigned linesize, int linelen)
*/
switch (filter) {
case 0:
- if (!handle_commit_msg(line, linesize))
+ if (!handle_commit_msg(line))
break;
filter++;
case 1:
- if (!handle_patch(line, linelen))
+ if (!handle_patch(line))
break;
filter++;
default:
@@ -793,101 +752,105 @@ static int handle_filter(char *line, unsigned linesize, int linelen)
static void handle_body(void)
{
int rc = 0;
- static char newline[2000];
- static char *np = newline;
- int len = strlen(line);
+ int len = 0;
+ struct strbuf prev = STRBUF_INIT;
/* Skip up to the first boundary */
if (content_top->boundary) {
if (!find_boundary())
- return;
+ goto handle_body_out;
}
do {
+ strbuf_setlen(&line, line.len + len);
+
/* process any boundary lines */
- if (content_top->boundary && is_multipart_boundary(line)) {
+ if (content_top->boundary && is_multipart_boundary(&line)) {
/* flush any leftover */
- if (np != newline)
- handle_filter(newline, sizeof(newline),
- np - newline);
+ if (line.len)
+ handle_filter(&line);
+
if (!handle_boundary())
- return;
- len = strlen(line);
+ goto handle_body_out;
}
/* Unwrap transfer encoding */
- len = decode_transfer_encoding(line, sizeof(line), len);
- if (len < 0) {
- error("Malformed input line");
- return;
- }
+ decode_transfer_encoding(&line);
switch (transfer_encoding) {
case TE_BASE64:
case TE_QP:
{
- char *op = line;
+ struct strbuf **lines, **it, *sb;
+
+ /* Prepend any previous partial lines */
+ strbuf_insert(&line, 0, prev.buf, prev.len);
+ strbuf_reset(&prev);
/* binary data most likely doesn't have newlines */
if (message_type != TYPE_TEXT) {
- rc = handle_filter(line, sizeof(line), len);
+ rc = handle_filter(&line);
break;
}
-
/*
* This is a decoded line that may contain
* multiple new lines. Pass only one chunk
* at a time to handle_filter()
*/
- do {
- while (op < line + len && *op != '\n')
- *np++ = *op++;
- *np = *op;
- if (*np != 0) {
- /* should be sitting on a new line */
- *(++np) = 0;
- op++;
- rc = handle_filter(newline, sizeof(newline), np - newline);
- np = newline;
- }
- } while (op < line + len);
+ lines = strbuf_split(&line, '\n');
+ strbuf_reset(&line);
+ for (it = lines; (sb = *it); it++) {
+ if (*(it + 1) == NULL) /* The last token */
+ if (sb->buf[sb->len - 1] != '\n') {
+ /* Partial line, save it for later. */
+ strbuf_addbuf(&prev, sb);
+ break;
+ }
+ rc = handle_filter(sb);
+ }
/*
- * The partial chunk is saved in newline and will be
+ * The partial chunk is saved in "prev" and will be
* appended by the next iteration of read_line_with_nul().
*/
+ strbuf_list_free(lines);
break;
}
default:
- rc = handle_filter(line, sizeof(line), len);
+ rc = handle_filter(&line);
+ strbuf_reset(&line);
}
if (rc)
/* nothing left to filter */
break;
- } while ((len = read_line_with_nul(line, sizeof(line), fin)));
+ if (strbuf_avail(&line) < 100)
+ strbuf_grow(&line, 100);
+ } while ((len = read_line_with_nul(line.buf, strbuf_avail(&line), fin)));
+handle_body_out:
+ strbuf_release(&prev);
return;
}
-static void output_header_lines(FILE *fout, const char *hdr, char *data)
+static void output_header_lines(FILE *fout, const char *hdr, const struct strbuf *data)
{
+ char *sp = data->buf;
while (1) {
- char *ep = strchr(data, '\n');
+ char *ep = strchr(sp, '\n');
int len;
if (!ep)
- len = strlen(data);
+ len = strlen(sp);
else
- len = ep - data;
- fprintf(fout, "%s: %.*s\n", hdr, len, data);
+ len = ep - sp;
+ fprintf(fout, "%s: %.*s\n", hdr, len, sp);
if (!ep)
break;
- data = ep + 1;
+ sp = ep + 1;
}
}
static void handle_info(void)
{
- char *sub;
- char *hdr;
+ struct strbuf *hdr;
int i;
for (i = 0; header[i]; i++) {
@@ -901,20 +864,18 @@ static void handle_info(void)
continue;
if (!memcmp(header[i], "Subject", 7)) {
- if (keep_subject)
- sub = hdr;
- else {
- sub = cleanup_subject(hdr);
- cleanup_space(sub);
+ if (!keep_subject) {
+ cleanup_subject(hdr);
+ cleanup_space(hdr);
}
- output_header_lines(fout, "Subject", sub);
+ output_header_lines(fout, "Subject", hdr);
} else if (!memcmp(header[i], "From", 4)) {
handle_from(hdr);
- fprintf(fout, "Author: %s\n", name);
- fprintf(fout, "Email: %s\n", email);
+ fprintf(fout, "Author: %s\n", name.buf);
+ fprintf(fout, "Email: %s\n", email.buf);
} else {
cleanup_space(hdr);
- fprintf(fout, "%s: %s\n", header[i], hdr);
+ fprintf(fout, "%s: %s\n", header[i], hdr->buf);
}
}
fprintf(fout, "\n");
@@ -941,8 +902,8 @@ static int mailinfo(FILE *in, FILE *out, int ks, const char *encoding,
return -1;
}
- p_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(char *));
- s_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(char *));
+ p_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(*p_hdr_data));
+ s_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(*s_hdr_data));
do {
peek = fgetc(in);
@@ -950,8 +911,8 @@ static int mailinfo(FILE *in, FILE *out, int ks, const char *encoding,
ungetc(peek, in);
/* process the email header */
- while (read_one_header_line(line, sizeof(line), fin))
- check_header(line, sizeof(line), p_hdr_data, 1);
+ while (read_one_header_line(&line, fin))
+ check_header(&line, p_hdr_data, 1);
handle_body();
handle_info();
--
1.5.4.5
^ permalink raw reply related
* [PATCH] Add some useful functions for strbuf manipulation.
From: Lukas Sandström @ 2008-07-10 23:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Lukas Sandström, Git Mailing List
In-Reply-To: <7v3amhnwy9.fsf@gitster.siamese.dyndns.org>
Signed-off-by: Lukas Sandström <lukass@etek.chalmers.se>
---
Junio C Hamano wrote:
> Heh, after getting burned by that NUL thingy, I was waiting for somebody
> to step up. Thanks.
Here we go then. Two freshly baked patches.
Note that this is a pretty straight buffers -> strbuf's conversion,
but I think it will be a good start for further hardening of mailinfo.
strbuf.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
strbuf.h | 6 +++++
2 files changed, 76 insertions(+), 0 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index 4aed752..28d6776 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -60,6 +60,18 @@ void strbuf_grow(struct strbuf *sb, size_t extra)
ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);
}
+void strbuf_trim(struct strbuf *sb)
+{
+ char *b = sb->buf;
+ while (sb->len > 0 && isspace((unsigned char)sb->buf[sb->len - 1]))
+ sb->len--;
+ while(sb->len > 0 && isspace(*b)) {
+ b++;
+ sb->len--;
+ }
+ memmove(sb->buf, b, sb->len);
+ sb->buf[sb->len] = '\0';
+}
void strbuf_rtrim(struct strbuf *sb)
{
while (sb->len > 0 && isspace((unsigned char)sb->buf[sb->len - 1]))
@@ -67,6 +79,64 @@ void strbuf_rtrim(struct strbuf *sb)
sb->buf[sb->len] = '\0';
}
+void strbuf_ltrim(struct strbuf *sb)
+{
+ char *b = sb->buf;
+ while(sb->len > 0 && isspace(*b)) {
+ b++;
+ sb->len--;
+ }
+ memmove(sb->buf, b, sb->len);
+ sb->buf[sb->len] = '\0';
+}
+
+void strbuf_tolower(struct strbuf *sb)
+{
+ int i;
+ for (i = 0; i < sb->len; i++)
+ sb->buf[i] = tolower(sb->buf[i]);
+}
+
+struct strbuf ** strbuf_split(struct strbuf *sb, int delim)
+{
+ int alloc = 2, pos = 0;
+ char *n, *p;
+ struct strbuf **ret;
+ struct strbuf *t;
+
+ ret = xcalloc(alloc, sizeof(struct strbuf *));
+ p = n = sb->buf;
+ while (n < sb->buf + sb->len) {
+ int len;
+ n = memchr(n, delim, sb->len - (n - sb->buf));
+ if (pos + 1 >= alloc) {
+ alloc = alloc * 2;
+ ret = xrealloc(ret, sizeof(struct strbuf *) * alloc);
+ }
+ if (!n)
+ n = sb->buf + sb->len - 1;
+ len = n - p + 1;
+ t = xmalloc(sizeof(struct strbuf));
+ strbuf_init(t, len);
+ strbuf_add(t, p, len);
+ ret[pos] = t;
+ ret[++pos] = NULL;
+ p = ++n;
+ }
+ return ret;
+}
+
+void strbuf_list_free(struct strbuf ** sbs)
+{
+ struct strbuf **s = sbs;
+
+ while(*s) {
+ strbuf_release(*s);
+ free(*s++);
+ }
+ free(sbs);
+}
+
int strbuf_cmp(struct strbuf *a, struct strbuf *b)
{
int cmp;
diff --git a/strbuf.h b/strbuf.h
index faec229..577d14e 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -77,8 +77,14 @@ static inline void strbuf_setlen(struct strbuf *sb, size_t len) {
#define strbuf_reset(sb) strbuf_setlen(sb, 0)
/*----- content related -----*/
+extern void strbuf_trim(struct strbuf *);
extern void strbuf_rtrim(struct strbuf *);
+extern void strbuf_ltrim(struct strbuf *);
extern int strbuf_cmp(struct strbuf *, struct strbuf *);
+extern void strbuf_tolower(struct strbuf *);
+
+extern struct strbuf ** strbuf_split(struct strbuf*, int delim);
+extern void strbuf_list_free(struct strbuf **);
/*----- add data in your buffer -----*/
static inline void strbuf_addch(struct strbuf *sb, int c) {
--
1.5.4.5
^ permalink raw reply related
* Re: [PATCH] bisect: test merge base if good rev is not an ancestor of bad rev
From: Christian Couder @ 2008-07-10 23:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Michael Haggerty, Jeff King, git
In-Reply-To: <7v7ibtnx09.fsf@gitster.siamese.dyndns.org>
Le vendredi 11 juillet 2008, Junio C Hamano a écrit :
>
> But in real life, things are more like "Today's master does not work, but
> I am sure it used to work at 1.5.6.2". I happen to always merge all of
> 'maint' to 'master' before pushing them out, so a good topology is
> guaranteed, but not everybody does this (it takes careful planning what
> to apply to 'maint' and where to fork topics from, and maintainers are
> not perfect):
>
> Good
> ?---o (maint)
> /
> ---?---?---?---?---x (master)
> MB Bad
>
> People _will_ face such a topology. If the users Know Better, they will
> test MB=$(merge-base master maint) first to see if it is broken, and then
> the world will have two possibilities:
>
> (1) Good
> ?---o (maint)
> /
> ---o---?---?---?---x (master)
> Good Bad
>
> (2) Good
> ?---o (maint)
> /
> ---x---?---?---?---x (master)
> Bad Bad
>
>
> If (1), you go ahead with the usual bisection. If (2), you cannot even
> bisect. Instead, you flip good and bad to find the "fix" in the side
> branch (the answer has to be either the tip of maint or one previous in
> the picture) to forward port to, either by merging 'maint' to 'master' or
> cherry-picking.
>
> The idea to check merge-base first is about automating this process (I
> admit I still haven't looked at Christian's patch text yet).
Well in case (2) my patch does:
-------
cat >&2 <<EOF
The merge base $_badmb is bad.
This means the bug has been fixed between $_badmb and $_g.
EOF
exit 3
-------
but this can be improved upon in some latter patches.
Thanks,
Christian.
^ permalink raw reply
* Re: [PATCH] Fix backwards-incompatible handling of core.sharedRepository
From: Junio C Hamano @ 2008-07-10 23:39 UTC (permalink / raw)
To: Petr Baudis; +Cc: git, Heikki Orsila
In-Reply-To: <20080710231853.21448.18643.stgit@rover.dkm.cz>
Petr Baudis <pasky@suse.cz> writes:
> The 06cbe8550324e0fd2290839bf3b9a92aa53b70ab core.sharedRepository
> handling extension broke backwards compatibility; before, shared=1 meant
> that Git merely ensured the repository is group-writable, not that it's
> _only_ group-writable, which is the current behaviour.
Donn't our existing tests catch this, and if the answer is no because we
don't have any, could you add some?
> path.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
>
> diff --git a/path.c b/path.c
> index 5983255..75c5915 100644
> --- a/path.c
> +++ b/path.c
> @@ -269,7 +269,7 @@ int adjust_shared_perm(const char *path)
> mode = st.st_mode;
>
> if (shared_repository) {
> - int tweak = shared_repository;
> + int tweak = (mode & 0777) | shared_repository;
> if (!(mode & S_IWUSR))
> tweak &= ~0222;
> mode = (mode & ~0777) | tweak;
I think this change is good. shared_repository has always been about
widening the access and not about limiting.
^ permalink raw reply
* Re: [PATCH] git-mailinfo: Fix getting the subject from the body
From: Junio C Hamano @ 2008-07-10 23:25 UTC (permalink / raw)
To: Lukas Sandström; +Cc: Git Mailing List
In-Reply-To: <48768F30.8070409@etek.chalmers.se>
Lukas Sandström <lukass@etek.chalmers.se> writes:
> I'm currently working on rewriting git-mailinfo to use strbuf's insted
> of the preallocated buffers currently used. Do you want me to send a
> patch allocating hdr_data[i], or can you wait for my strbuf-conversion
> patch? It'll propably be ready for review tonight.
Heh, after getting burned by that NUL thingy, I was waiting for somebody
to step up. Thanks.
^ permalink raw reply
* Re: [PATCH] bisect: test merge base if good rev is not an ancestor of bad rev
From: Junio C Hamano @ 2008-07-10 23:24 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Christian Couder, Michael Haggerty, Jeff King, git
In-Reply-To: <alpine.DEB.1.00.0807110035180.3279@eeepc-johanness>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> You are opening a can of worms here, and I doubt that this is a good idea.
>
> git-bisect as-is has very precise, and _simple_ semantics, and users
> should really know what they are doing (i.e. not marking something as
> "good" which is on a branch containing a fix).
>
> Trying to be too clever here might just make the whole tool rather
> useless.
Have you read the original thread yet? I do not think this is trying to
be clever at all, but trying to be helpful.
As you explained, bisection *requires* that Good is ancestor of Bad.
---o---?---?---?---x
Good Bad
But in real life, things are more like "Today's master does not work, but
I am sure it used to work at 1.5.6.2". I happen to always merge all of
'maint' to 'master' before pushing them out, so a good topology is
guaranteed, but not everybody does this (it takes careful planning what to
apply to 'maint' and where to fork topics from, and maintainers are not
perfect):
Good
?---o (maint)
/
---?---?---?---?---x (master)
MB Bad
People _will_ face such a topology. If the users Know Better, they will
test MB=$(merge-base master maint) first to see if it is broken, and then
the world will have two possibilities:
(1) Good
?---o (maint)
/
---o---?---?---?---x (master)
Good Bad
(2) Good
?---o (maint)
/
---x---?---?---?---x (master)
Bad Bad
If (1), you go ahead with the usual bisection. If (2), you cannot even
bisect. Instead, you flip good and bad to find the "fix" in the side
branch (the answer has to be either the tip of maint or one previous in
the picture) to forward port to, either by merging 'maint' to 'master' or
cherry-picking.
The idea to check merge-base first is about automating this process (I
admit I still haven't looked at Christian's patch text yet).
^ permalink raw reply
* [PATCH] Fix backwards-incompatible handling of core.sharedRepository
From: Petr Baudis @ 2008-07-10 23:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Heikki Orsila
The 06cbe8550324e0fd2290839bf3b9a92aa53b70ab core.sharedRepository
handling extension broke backwards compatibility; before, shared=1 meant
that Git merely ensured the repository is group-writable, not that it's
_only_ group-writable, which is the current behaviour.
Maybe it makes sense to provide the current semantics in some way too,
but that cannot be done at the expense of ditching backwards
compatibility; this bug has just wasted me two hours and broke
repo.or.cz pushing for several hours.
Signed-off-by: Petr Baudis <pasky@rover.dkm.cz>
---
Sorry for the resend, StGIT kind of tricked me to adding two Cc headers and
the first one just got dropped.
path.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/path.c b/path.c
index 5983255..75c5915 100644
--- a/path.c
+++ b/path.c
@@ -269,7 +269,7 @@ int adjust_shared_perm(const char *path)
mode = st.st_mode;
if (shared_repository) {
- int tweak = shared_repository;
+ int tweak = (mode & 0777) | shared_repository;
if (!(mode & S_IWUSR))
tweak &= ~0222;
mode = (mode & ~0777) | tweak;
^ permalink raw reply related
* Re: [CFH] Broken permissions in repo.or.cz repositories
From: Petr Baudis @ 2008-07-10 23:19 UTC (permalink / raw)
To: git; +Cc: Jesper Louis Andersen
In-Reply-To: <20080710230058.GQ10151@machine.or.cz>
Hi,
On Fri, Jul 11, 2008 at 01:00:58AM +0200, Petr Baudis wrote:
> turns out current git has buggy handling of the sharedRepository
> config parameter, resulting in permission problems. Hopefully fixed.
sorry, this was not supposed to be cc'd to the mailing list in the
end, and is not CFH anymore. I'm trying to send a patch to the mailing
list now. :-)
--
Petr "Pasky" Baudis
The last good thing written in C++ was the Pachelbel Canon. -- J. Olson
^ permalink raw reply
* Re: [PATCH] bisect: test merge base if good rev is not an ancestor of bad rev
From: Christian Couder @ 2008-07-10 23:21 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Michael Haggerty, Jeff King, git
In-Reply-To: <alpine.DEB.1.00.0807110035180.3279@eeepc-johanness>
Le vendredi 11 juillet 2008, Johannes Schindelin a écrit :
> Hi,
>
> On Fri, 11 Jul 2008, Christian Couder wrote:
> > Le jeudi 10 juillet 2008, Junio C Hamano a écrit :
> > > - "Test this merge-base before going forward, please" will add
> > > typically only one round of check (if you have more merge bases
> > > between good and bad, you need to test all of them are good to be
> > > sure), so it is not "slower nor more complex".
> >
> > By "slower" I meant that it would need more rounds of check on average.
> > By "more complex" I meant that more code is needed.
> >
> > And I think you are right, all the merge bases need to be tested so I
> > will send a patch on top of the patch discussed here.
>
> Good luck. This will open the scenario where people use a proper
> ancestor as "good" revision. In this case, you test that.
If a proper ancestor is used as good rev, then the merge base between this
good rev and the bad rev is this good rev, and as it is known to be good it
doesn't need to be tested by the user. I think my patch handles this well.
> If it is
> "bad" you report that it is the _first_ one.
>
> You are opening a can of worms here, and I doubt that this is a good
> idea.
>
> git-bisect as-is has very precise, and _simple_ semantics, and users
> should really know what they are doing (i.e. not marking something as
> "good" which is on a branch containing a fix).
I think that users don't and can't always know what they are doing. If there
is a revert in a branch for example, how can they know all the bugs that it
fixes?
> Trying to be too clever here might just make the whole tool rather
> useless.
I don't think so. I think that if user can trust "git bisect" more, they
will use it more, and in more automated ways and everyone will win in the
end.
Regards,
Christian.
^ permalink raw reply
* Re: [PATCH] bisect: test merge base if good rev is not an ancestor of bad rev
From: Junio C Hamano @ 2008-07-10 23:10 UTC (permalink / raw)
To: Christian Couder; +Cc: Johannes Schindelin, Michael Haggerty, Jeff King, git
In-Reply-To: <200807110036.17504.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> Another idea to fix the problem, might be to bisect as usual and at the end
> before saying "X is first bad commit" to check if some of X parents are
> merge bases between the bad rev and a good rev. If that is the case, then
> we could ask the user to check that these parents are all good. On average
> this would probably reduce the number of revs the user must check.
I do not think that would fly well. After spending long bisection cycle,
you will be telling the user that it was a wild goose chase (iow, the user
did an invalid bisection and what we stopped at was not the first
breakage). If the bisection topology is invalid, we should tell the user
before he wastes too much time.
The sad part is that the biesction log from such an initial round would
not be very useful for reusing even if the user then chooses to hunt for
the "fix" on the side branch to forward port, in which case the meaning of
good and bad needs to be swapped from the beginning.
^ permalink raw reply
* Re: feature request: git-log should accept sth like v2.6.26-rc8-227
From: Jakub Narebski @ 2008-07-10 23:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Toralf Förster, git
In-Reply-To: <7vk5ftnyhp.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> On Thu, 10 Jul 2008, Junio C Hamano wrote:
>>> Jakub Narebski <jnareb@gmail.com> writes:
>>>
>>>> Besides, it would be nice to have some command (git-rev-parse
>>>> perhaps?) which could take ambiguous commit-ish, and list all commit
>>>> which matches it.
>>>
>>> Have fun writing it and send in a patch.
>>
>> Note that this really could be a patch, but not for rev-parse. Patch
>> revision.c instead to parse the argument into _all_ matching revisions.
>
> As Linus pointed out, that is "all _locally_ matching revisions". It is
> of dubious value in a distributed environment.
It can be useful for example in situation where shortened sha-1 of
a commit (for example to 6 characters) stops being ambiguous...
--
Jakub Narebski
Poland
^ permalink raw reply
* [CFH] Broken permissions in repo.or.cz repositories
From: Petr Baudis @ 2008-07-10 23:00 UTC (permalink / raw)
To: Jesper Louis Andersen; +Cc: git
In-Reply-To: <56a0a2840807101218n7a280b67gfb3c579ada6b0e02@mail.gmail.com>
Hi,
On Thu, Jul 10, 2008 at 09:18:18PM +0200, Jesper Louis Andersen wrote:
> I have run into a problem with your new fine cloud-changes: whenever I
> push changes to the 'master'
> branch of the etorrent.git repository, the overview page at
> http://repo.or.cz/w/etorrent.git removes
> the shortlog and shows no master branch at all. This problem means that we get:
>
> jlouis@ogre:~/tmp$ git clone git://repo.or.cz/etorrent.git/
> Initialized empty Git repository in /home/jlouis/tmp/etorrent/.git/
> fatal: The remote end hung up unexpectedly
>
> for new checkouts. Now, I've found a fix which is to pull over ssh:
>
> jlouis@ogre:~/tmp$ git clone git+ssh://repo.or.cz/srv/git/etorrent.git
> Initialized empty Git repository in /home/jlouis/tmp/etorrent/.git/
> remote: Counting objects: 7510, done.
> remote: Compressing objReceiving objects: 9% (676/7510), 100.00 KiB
> | 167 KiB/remote: Compressing objects: 69% (1649/238Receiving
> objects: 10% (751/7510), 1remote: Compressing obReceiving objects:
> 12% (902/7510), 100.00 KiB | 167 KiB/sremote: Comremote: pressing
> objects: 100% (2389/2389), done.
> remote: Total 7510 (delta 4432), reused 7510 (delta 4432)
> Receiving objects: 100% (7510/7510), 1.93 MiB | 517 KiB/s, done.
> Resolving deltas: 100% (4432/4432), done.
> jlouis@ogre:~/tmp$ ls
>
> which, as you can see, works. Then one can clone via git://... until I
> make another push and we are
> back to square one. I have not been able to find another repository
> having the same problem which
> makes me wonder if there is something wrong with my repository state
> which becomes fixed when it compresses
> the objects when I use the git+ssh transport?
>
> For the sake of completeness, it is Ubuntu 8.04 here with git --version:
>
> jlouis@ogre:~$ git --version
> git version 1.5.4.3
turns out current git has buggy handling of the sharedRepository
config parameter, resulting in permission problems. Hopefully fixed.
--
Petr "Pasky" Baudis
The last good thing written in C++ was the Pachelbel Canon. -- J. Olson
^ permalink raw reply
* Re: feature request: git-log should accept sth like v2.6.26-rc8-227
From: Junio C Hamano @ 2008-07-10 22:52 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jakub Narebski, Toralf Förster, git
In-Reply-To: <alpine.DEB.1.00.0807110022510.3279@eeepc-johanness>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Thu, 10 Jul 2008, Junio C Hamano wrote:
>
>> Jakub Narebski <jnareb@gmail.com> writes:
>>
>> > Besides, it would be nice to have some command (git-rev-parse
>> > perhaps?) which could take ambiguous commit-ish, and list all commit
>> > which matches it.
>>
>> Have fun writing it and send in a patch.
>
> Note that this really could be a patch, but not for rev-parse. Patch
> revision.c instead to parse the argument into _all_ matching revisions.
As Linus pointed out, that is "all _locally_ matching revisions". It is
of dubious value in a distributed environment.
^ permalink raw reply
* Re: [JGIT PATCH 2/5] Don't display passwords on the console in fetch/push output
From: Robin Rosenberg @ 2008-07-10 22:42 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Shawn O. Pearce, Marek Zawirski, git
In-Reply-To: <alpine.DEB.1.00.0807110024480.3279@eeepc-johanness>
fredagen den 11 juli 2008 00.25.27 skrev Johannes Schindelin:
> Hi,
>
> On Thu, 10 Jul 2008, Robin Rosenberg wrote:
>
> > >From 99c09cf2321f36eb81043aed2fa6834811ee762b Mon Sep 17 00:00:00 2001
> > From: Robin Rosenberg <robin.rosenberg@dewire.com>
> > Date: Thu, 10 Jul 2008 22:16:19 +0200
> > Subject: [PATCH] Avoid password leak from URIIsh
>
> What is this new fashion of sending them headers in the mail body? Robin,
> I thought you knew better.
Laziness, I suppose, and that's not a new fashion. I'll try to have better in the future.
-- robin
^ permalink raw reply
* [EGIT PATCH 0/2] Quickdiff improvement
From: Robin Rosenberg @ 2008-07-10 22:41 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Marek Zawirski, Robin Rosenberg
Connect quickdiff to the repository event mechanism and allow us
to specify what to compare against.
-- robin
Robin Rosenberg (2):
Make quick diff aware of changes in the repository.
Provide the ability to configure the quickdiff baseline
org.spearce.egit.ui/plugin.properties | 2 +-
org.spearce.egit.ui/plugin.xml | 14 +++
.../actions/QuickdiffBaselineOperation.java | 44 ++++++++++
.../actions/ResetQuickdiffBaselineAction.java | 24 +++++
.../actions/SetQuickdiffBaselineAction.java | 26 ++++++
.../egit/ui/internal/decorators/GitDocument.java | 86 +++++++++++++++++++
.../internal/decorators/GitQuickDiffProvider.java | 90 +++++++++-----------
7 files changed, 236 insertions(+), 50 deletions(-)
create mode 100644 org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/QuickdiffBaselineOperation.java
create mode 100644 org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/ResetQuickdiffBaselineAction.java
create mode 100644 org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/SetQuickdiffBaselineAction.java
create mode 100644 org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
^ permalink raw reply
* [EGIT PATCH 2/2] Provide the ability to configure the quickdiff baseline
From: Robin Rosenberg @ 2008-07-10 22:41 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Marek Zawirski, Robin Rosenberg
In-Reply-To: <1215729672-26906-2-git-send-email-robin.rosenberg@dewire.com>
By default quickdiff compares to HEAD, but you can change that
using the context menu in the history graph. The setting is
remembered during a session on a per Git repository basis.
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
org.spearce.egit.ui/plugin.properties | 2 +-
org.spearce.egit.ui/plugin.xml | 14 +++++
.../actions/QuickdiffBaselineOperation.java | 44 +++++++++++++++
.../actions/ResetQuickdiffBaselineAction.java | 24 ++++++++
.../actions/SetQuickdiffBaselineAction.java | 26 +++++++++
.../egit/ui/internal/decorators/GitDocument.java | 57 ++++++-------------
.../internal/decorators/GitQuickDiffProvider.java | 33 +++++++++++-
7 files changed, 158 insertions(+), 42 deletions(-)
create mode 100644 org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/QuickdiffBaselineOperation.java
create mode 100644 org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/ResetQuickdiffBaselineAction.java
create mode 100644 org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/SetQuickdiffBaselineAction.java
diff --git a/org.spearce.egit.ui/plugin.properties b/org.spearce.egit.ui/plugin.properties
index 64a88c3..3240ec0 100644
--- a/org.spearce.egit.ui/plugin.properties
+++ b/org.spearce.egit.ui/plugin.properties
@@ -6,7 +6,7 @@ Git_clone_wizard=Git Repository
Git_clone_description=Clone an existing Git repository.
SharingWizard_name=Git
-GitRemoteQuickDiffProvider_label=Latest Git Revision
+GitRemoteQuickDiffProvider_label=A Git Revision
DisconnectAction_label=Disconnect
DisconnectAction_tooltip=Disconnect the Git team provider.
diff --git a/org.spearce.egit.ui/plugin.xml b/org.spearce.egit.ui/plugin.xml
index cfd4b80..b809300 100644
--- a/org.spearce.egit.ui/plugin.xml
+++ b/org.spearce.egit.ui/plugin.xml
@@ -124,6 +124,20 @@
enablesFor="1"
tooltip="Resets HEAD and index, and working directory (changed in tracked files will be lost)">
</action>
+ <action
+ class="org.spearce.egit.ui.internal.actions.SetQuickdiffBaselineAction"
+ enablesFor="1"
+ id="org.spearce.egit.ui.setquickdiffbaseline"
+ label="Set as quickdiff baseline"
+ menubarPath="additions">
+ </action>
+ <action
+ class="org.spearce.egit.ui.internal.actions.ResetQuickdiffBaselineAction"
+ enablesFor="*"
+ id="org.spearce.egit.ui.resetquickdiffbaseline"
+ label="Reset quickdiff baseline to HEAD"
+ menubarPath="additions">
+ </action>
</objectContribution>
</extension>
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/QuickdiffBaselineOperation.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/QuickdiffBaselineOperation.java
new file mode 100644
index 0000000..990958f
--- /dev/null
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/QuickdiffBaselineOperation.java
@@ -0,0 +1,44 @@
+/*******************************************************************************
+ * Copyright (C) 2008, Robin Rosenberg <robin.rosenberg@dewire.com>
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * See LICENSE for the full license text, also available.
+ *******************************************************************************/
+package org.spearce.egit.ui.internal.actions;
+
+import java.io.IOException;
+
+import org.eclipse.core.runtime.CoreException;
+import org.eclipse.core.runtime.IProgressMonitor;
+import org.spearce.egit.ui.Activator;
+import org.spearce.egit.ui.internal.decorators.GitQuickDiffProvider;
+import org.spearce.jgit.lib.Repository;
+
+/**
+ * UI operation to change the git quickdiff baseline
+ */
+public class QuickdiffBaselineOperation extends AbstractRevObjectOperation {
+
+ private final String baseline;
+
+ /**
+ * Construct a QuickdiffBaselineOperation for changing quickdiff baseline
+ * @param repository
+ *
+ * @param baseline
+ */
+ QuickdiffBaselineOperation(final Repository repository, final String baseline) {
+ super(repository);
+ this.baseline = baseline;
+ }
+
+ public void run(IProgressMonitor monitor) throws CoreException {
+ try {
+ GitQuickDiffProvider.setBaselineReference(repository, baseline);
+ } catch (IOException e) {
+ Activator.logError("Cannot set quickdiff basekine", e);
+ }
+ }
+
+}
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/ResetQuickdiffBaselineAction.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/ResetQuickdiffBaselineAction.java
new file mode 100644
index 0000000..a42635a
--- /dev/null
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/ResetQuickdiffBaselineAction.java
@@ -0,0 +1,24 @@
+/*******************************************************************************
+ * Copyright (C) 2008, Robin Rosenberg <robin.rosenberg@dewire.com>
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * See LICENSE for the full license text, also available.
+ *******************************************************************************/
+package org.spearce.egit.ui.internal.actions;
+
+import java.util.List;
+
+import org.eclipse.core.resources.IWorkspaceRunnable;
+import org.eclipse.jface.action.IAction;
+
+/**
+ * Changes the reference for the quickdiff
+ */
+public class ResetQuickdiffBaselineAction extends AbstractRevObjectAction {
+
+ @Override
+ protected IWorkspaceRunnable createOperation(IAction act, List selection) {
+ return new QuickdiffBaselineOperation(getActiveRepository(), "HEAD");
+ }
+}
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/SetQuickdiffBaselineAction.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/SetQuickdiffBaselineAction.java
new file mode 100644
index 0000000..05686cf
--- /dev/null
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/SetQuickdiffBaselineAction.java
@@ -0,0 +1,26 @@
+/*******************************************************************************
+ * Copyright (C) 2008, Robin Rosenberg <robin.rosenberg@dewire.com>
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * See LICENSE for the full license text, also available.
+ *******************************************************************************/
+package org.spearce.egit.ui.internal.actions;
+
+import java.util.List;
+
+import org.eclipse.core.resources.IWorkspaceRunnable;
+import org.eclipse.jface.action.IAction;
+import org.spearce.jgit.revwalk.RevObject;
+
+/**
+ * Changes the reference for the quickdiff
+ */
+public class SetQuickdiffBaselineAction extends AbstractRevObjectAction {
+
+ @Override
+ protected IWorkspaceRunnable createOperation(IAction act, List selection) {
+ return new QuickdiffBaselineOperation(getActiveRepository(), ((RevObject)selection.get(0)).getId().toString());
+ }
+
+}
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
index 23e06d9..ebed0cf 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
@@ -7,33 +7,26 @@
*******************************************************************************/
package org.spearce.egit.ui.internal.decorators;
-import java.io.BufferedReader;
-import java.io.CharArrayWriter;
import java.io.IOException;
-import java.io.InputStream;
-import java.io.InputStreamReader;
import org.eclipse.core.resources.IProject;
import org.eclipse.core.resources.IResource;
-import org.eclipse.core.resources.IStorage;
-import org.eclipse.core.runtime.CoreException;
import org.eclipse.jface.text.Document;
import org.eclipse.team.core.RepositoryProvider;
-import org.eclipse.team.core.history.IFileHistory;
-import org.eclipse.team.core.history.IFileHistoryProvider;
-import org.eclipse.team.core.history.IFileRevision;
import org.spearce.egit.core.GitProvider;
import org.spearce.egit.core.project.RepositoryMapping;
import org.spearce.egit.ui.Activator;
import org.spearce.jgit.lib.IndexChangedEvent;
+import org.spearce.jgit.lib.ObjectLoader;
import org.spearce.jgit.lib.RefsChangedEvent;
import org.spearce.jgit.lib.Repository;
import org.spearce.jgit.lib.RepositoryListener;
+import org.spearce.jgit.lib.TreeEntry;
class GitDocument extends Document implements RepositoryListener {
private final IResource resource;
- static GitDocument create(IResource resource) throws IOException, CoreException {
+ static GitDocument create(final IResource resource) throws IOException {
GitDocument ret = null;
if (RepositoryProvider.getProvider(resource.getProject()) instanceof GitProvider) {
ret = new GitDocument(resource);
@@ -44,36 +37,24 @@ class GitDocument extends Document implements RepositoryListener {
private GitDocument(IResource resource) {
this.resource = resource;
+ GitQuickDiffProvider.doc2repo.put(this, getRepository());
}
- void populate() throws IOException, CoreException {
+ void populate() throws IOException {
set("");
- IProject project = resource.getProject();
- RepositoryProvider provider = RepositoryProvider.getProvider(project);
- getRepository().addRepositoryChangedListener(this);
- IFileHistoryProvider fileHistoryProvider = provider
- .getFileHistoryProvider();
- IFileHistory fileHistoryFor = fileHistoryProvider.getFileHistoryFor(
- resource, IFileHistoryProvider.SINGLE_REVISION, null);
- IFileRevision[] revisions = fileHistoryFor.getFileRevisions();
- if (revisions != null && revisions.length > 0) {
- IFileRevision revision = revisions[0];
- Activator.trace("(GitQuickDiffProvider) compareTo: "
- + revision.getContentIdentifier());
- IStorage storage = revision.getStorage(null);
- InputStream contents = storage.getContents();
- BufferedReader in = new BufferedReader(new InputStreamReader(
- contents));
- final int DEFAULT_FILE_SIZE = 15 * 1024;
-
- CharArrayWriter caw = new CharArrayWriter(DEFAULT_FILE_SIZE);
- char[] readBuffer = new char[2048];
- int n = in.read(readBuffer);
- while (n > 0) {
- caw.write(readBuffer, 0, n);
- n = in.read(readBuffer);
- }
- String s = caw.toString();
+ final IProject project = resource.getProject();
+ final String gitPath = RepositoryMapping.getMapping(project).getRepoRelativePath(resource);
+ final Repository repository = getRepository();
+ repository.addRepositoryChangedListener(this);
+ String baseline = GitQuickDiffProvider.baseline.get(repository);
+ if (baseline == null)
+ baseline = "HEAD";
+ TreeEntry blobEnry = repository.mapTree(baseline).findBlobMember(gitPath);
+ if (blobEnry != null) {
+ Activator.trace("(GitQuickDiffProvider) compareTo: " + baseline);
+ ObjectLoader loader = repository.openBlob(blobEnry.getId());
+ byte[] bytes = loader.getBytes();
+ String s = new String(bytes); // FIXME Platform default charset. should be Eclipse default
set(s);
Activator.trace("(GitQuickDiffProvider) has reference doc, size=" + s.length() + " bytes");
} else {
@@ -90,8 +71,6 @@ class GitDocument extends Document implements RepositoryListener {
populate();
} catch (IOException e1) {
Activator.logError("Failed to refresh quickdiff", e1);
- } catch (CoreException e1) {
- Activator.logError("Failed to refresh quickdiff", e1);
}
}
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java
index 052552e..88f5ea0 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java
@@ -9,6 +9,8 @@
package org.spearce.egit.ui.internal.decorators;
import java.io.IOException;
+import java.util.Map;
+import java.util.WeakHashMap;
import org.eclipse.core.resources.IResource;
import org.eclipse.core.runtime.CoreException;
@@ -21,6 +23,7 @@ import org.eclipse.ui.texteditor.ITextEditor;
import org.eclipse.ui.texteditor.quickdiff.IQuickDiffReferenceProvider;
import org.spearce.egit.ui.Activator;
import org.spearce.egit.ui.UIText;
+import org.spearce.jgit.lib.Repository;
/**
* This class provides input for the Eclipse Quick Diff feature.
@@ -33,8 +36,19 @@ public class GitQuickDiffProvider implements IQuickDiffReferenceProvider {
private IResource resource;
+ static Map<Repository,String> baseline = new WeakHashMap<Repository,String>();
+ static Map<GitDocument,Repository> doc2repo = new WeakHashMap<GitDocument, Repository>();
+
+ /**
+ * Create the GitQuickDiffProvider instance
+ */
+ public GitQuickDiffProvider() {
+ // Empty
+ }
+
public void dispose() {
Activator.trace("(GitQuickDiffProvider) dispose");
+ doc2repo.remove(document);
if (document != null)
document.dispose();
}
@@ -51,8 +65,6 @@ public class GitQuickDiffProvider implements IQuickDiffReferenceProvider {
if (provider != null) {
try {
document = GitDocument.create(resource);
- } catch (CoreException e) {
- Activator.error(UIText.QuickDiff_failedLoading, e);
} catch (IOException e) {
Activator.error(UIText.QuickDiff_failedLoading, e);
}
@@ -74,4 +86,21 @@ public class GitQuickDiffProvider implements IQuickDiffReferenceProvider {
public void setId(String id) {
this.id = id;
}
+
+ /**
+ * Set a new baseline for quickdiff
+ *
+ * @param repository
+ * @param baseline any commit reference, ref, symref or sha-1
+ * @throws IOException
+ */
+ public static void setBaselineReference(final Repository repository, final String baseline) throws IOException {
+ GitQuickDiffProvider.baseline.put(repository, baseline);
+ for (Map.Entry<GitDocument, Repository> i : doc2repo.entrySet()) {
+ if (i.getValue() == repository) {
+ i.getKey().populate();
+ }
+ }
+ }
+
}
--
1.5.6.2.220.g44701
^ permalink raw reply related
* [EGIT PATCH 1/2] Make quick diff aware of changes in the repository.
From: Robin Rosenberg @ 2008-07-10 22:41 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Marek Zawirski, Robin Rosenberg
In-Reply-To: <1215729672-26906-1-git-send-email-robin.rosenberg@dewire.com>
Currently only refs changes are relevant.
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../egit/ui/internal/decorators/GitDocument.java | 107 ++++++++++++++++++++
.../internal/decorators/GitQuickDiffProvider.java | 57 ++---------
2 files changed, 117 insertions(+), 47 deletions(-)
create mode 100644 org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
new file mode 100644
index 0000000..23e06d9
--- /dev/null
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
@@ -0,0 +1,107 @@
+/*******************************************************************************
+ * Copyright (C) 2008, Robin Rosenberg <robin.rosenberg@dewire.com>
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * See LICENSE for the full license text, also available.
+ *******************************************************************************/
+package org.spearce.egit.ui.internal.decorators;
+
+import java.io.BufferedReader;
+import java.io.CharArrayWriter;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+
+import org.eclipse.core.resources.IProject;
+import org.eclipse.core.resources.IResource;
+import org.eclipse.core.resources.IStorage;
+import org.eclipse.core.runtime.CoreException;
+import org.eclipse.jface.text.Document;
+import org.eclipse.team.core.RepositoryProvider;
+import org.eclipse.team.core.history.IFileHistory;
+import org.eclipse.team.core.history.IFileHistoryProvider;
+import org.eclipse.team.core.history.IFileRevision;
+import org.spearce.egit.core.GitProvider;
+import org.spearce.egit.core.project.RepositoryMapping;
+import org.spearce.egit.ui.Activator;
+import org.spearce.jgit.lib.IndexChangedEvent;
+import org.spearce.jgit.lib.RefsChangedEvent;
+import org.spearce.jgit.lib.Repository;
+import org.spearce.jgit.lib.RepositoryListener;
+
+class GitDocument extends Document implements RepositoryListener {
+ private final IResource resource;
+
+ static GitDocument create(IResource resource) throws IOException, CoreException {
+ GitDocument ret = null;
+ if (RepositoryProvider.getProvider(resource.getProject()) instanceof GitProvider) {
+ ret = new GitDocument(resource);
+ ret.populate();
+ }
+ return ret;
+ }
+
+ private GitDocument(IResource resource) {
+ this.resource = resource;
+ }
+
+ void populate() throws IOException, CoreException {
+ set("");
+ IProject project = resource.getProject();
+ RepositoryProvider provider = RepositoryProvider.getProvider(project);
+ getRepository().addRepositoryChangedListener(this);
+ IFileHistoryProvider fileHistoryProvider = provider
+ .getFileHistoryProvider();
+ IFileHistory fileHistoryFor = fileHistoryProvider.getFileHistoryFor(
+ resource, IFileHistoryProvider.SINGLE_REVISION, null);
+ IFileRevision[] revisions = fileHistoryFor.getFileRevisions();
+ if (revisions != null && revisions.length > 0) {
+ IFileRevision revision = revisions[0];
+ Activator.trace("(GitQuickDiffProvider) compareTo: "
+ + revision.getContentIdentifier());
+ IStorage storage = revision.getStorage(null);
+ InputStream contents = storage.getContents();
+ BufferedReader in = new BufferedReader(new InputStreamReader(
+ contents));
+ final int DEFAULT_FILE_SIZE = 15 * 1024;
+
+ CharArrayWriter caw = new CharArrayWriter(DEFAULT_FILE_SIZE);
+ char[] readBuffer = new char[2048];
+ int n = in.read(readBuffer);
+ while (n > 0) {
+ caw.write(readBuffer, 0, n);
+ n = in.read(readBuffer);
+ }
+ String s = caw.toString();
+ set(s);
+ Activator.trace("(GitQuickDiffProvider) has reference doc, size=" + s.length() + " bytes");
+ } else {
+ Activator.trace("(GitQuickDiffProvider) no revision.");
+ }
+ }
+
+ void dispose() {
+ getRepository().removeRepositoryChangedListener(this);
+ }
+
+ public void refsChanged(final RefsChangedEvent e) {
+ try {
+ populate();
+ } catch (IOException e1) {
+ Activator.logError("Failed to refresh quickdiff", e1);
+ } catch (CoreException e1) {
+ Activator.logError("Failed to refresh quickdiff", e1);
+ }
+ }
+
+ public void indexChanged(final IndexChangedEvent e) {
+ // Index not relevant at this moment
+ }
+
+ private Repository getRepository() {
+ IProject project = resource.getProject();
+ RepositoryMapping mapping = RepositoryMapping.getMapping(project);
+ return mapping.getRepository();
+ }
+}
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java
index 5525914..052552e 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java
@@ -8,22 +8,13 @@
*******************************************************************************/
package org.spearce.egit.ui.internal.decorators;
-import java.io.BufferedReader;
-import java.io.CharArrayWriter;
import java.io.IOException;
-import java.io.InputStream;
-import java.io.InputStreamReader;
-import org.eclipse.core.resources.IFile;
-import org.eclipse.core.resources.IStorage;
+import org.eclipse.core.resources.IResource;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
-import org.eclipse.jface.text.Document;
import org.eclipse.jface.text.IDocument;
import org.eclipse.team.core.RepositoryProvider;
-import org.eclipse.team.core.history.IFileHistory;
-import org.eclipse.team.core.history.IFileHistoryProvider;
-import org.eclipse.team.core.history.IFileRevision;
import org.eclipse.ui.IEditorInput;
import org.eclipse.ui.ide.ResourceUtil;
import org.eclipse.ui.texteditor.ITextEditor;
@@ -38,12 +29,14 @@ public class GitQuickDiffProvider implements IQuickDiffReferenceProvider {
private String id;
- private Document document;
+ private GitDocument document;
- private IFile file;
+ private IResource resource;
public void dispose() {
- // No resources to free
+ Activator.trace("(GitQuickDiffProvider) dispose");
+ if (document != null)
+ document.dispose();
}
public String getId() {
@@ -52,42 +45,12 @@ public class GitQuickDiffProvider implements IQuickDiffReferenceProvider {
public IDocument getReference(IProgressMonitor monitor)
throws CoreException {
- document = new Document();
- Activator.trace("(GitQuickDiffProvider) file: " + file);
-
- RepositoryProvider provider = RepositoryProvider.getProvider(file
+ Activator.trace("(GitQuickDiffProvider) file: " + resource);
+ RepositoryProvider provider = RepositoryProvider.getProvider(resource
.getProject());
if (provider != null) {
try {
- IFileHistoryProvider fileHistoryProvider = provider
- .getFileHistoryProvider();
- IFileHistory fileHistoryFor = fileHistoryProvider
- .getFileHistoryFor(file,
- IFileHistoryProvider.SINGLE_REVISION, null);
- IFileRevision[] revisions = fileHistoryFor.getFileRevisions();
- if (revisions != null && revisions.length > 0) {
- IFileRevision revision = revisions[0];
- Activator.trace("(GitQuickDiffProvider) compareTo: "
- + revision.getContentIdentifier());
- IStorage storage = revision.getStorage(null);
- InputStream contents = storage.getContents();
- BufferedReader in = new BufferedReader(
- new InputStreamReader(contents));
- final int DEFAULT_FILE_SIZE = 15 * 1024;
-
- CharArrayWriter caw = new CharArrayWriter(DEFAULT_FILE_SIZE);
- char[] readBuffer = new char[2048];
- int n = in.read(readBuffer);
- while (n > 0) {
- caw.write(readBuffer, 0, n);
- n = in.read(readBuffer);
- }
- String s = caw.toString();
- document.set(s);
- } else {
- Activator.trace("(GitQuickDiffProvider) no revision.");
- document.set("");
- }
+ document = GitDocument.create(resource);
} catch (CoreException e) {
Activator.error(UIText.QuickDiff_failedLoading, e);
} catch (IOException e) {
@@ -105,7 +68,7 @@ public class GitQuickDiffProvider implements IQuickDiffReferenceProvider {
public void setActiveEditor(ITextEditor editor) {
IEditorInput editorInput = editor.getEditorInput();
- file = ResourceUtil.getFile(editorInput);
+ resource = ResourceUtil.getResource(editorInput);
}
public void setId(String id) {
--
1.5.6.2.220.g44701
^ permalink raw reply related
* [EGIT PATCH 2/3] Avoid errors by allowing phantoms during traversal in the resource decorator
From: Robin Rosenberg @ 2008-07-10 22:41 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Marek Zawirski, Robin Rosenberg
In-Reply-To: <1215729668-26865-2-git-send-email-robin.rosenberg@dewire.com>
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../internal/decorators/GitResourceDecorator.java | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java
index c625ce6..8d4a7c7 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java
@@ -137,7 +137,9 @@ public class GitResourceDecorator extends LabelProvider implements
getActiveDecorator().clearDecorationState(resource);
return true;
}
- });
+ },
+ IResource.DEPTH_INFINITE,
+ true);
} finally {
getJobManager().endRule(markerRule);
}
@@ -164,7 +166,9 @@ public class GitResourceDecorator extends LabelProvider implements
}
return true;
}
- });
+ },
+ true
+ );
} catch (Exception e) {
Activator.logError("Problem during decorations. Stopped", e);
}
--
1.5.6.2.220.g44701
^ permalink raw reply related
* [EGIT PATCH 1/3] Use a job for the resource decorator
From: Robin Rosenberg @ 2008-07-10 22:41 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Marek Zawirski, Robin Rosenberg
In-Reply-To: <1215729668-26865-1-git-send-email-robin.rosenberg@dewire.com>
Scanning directories for changes is slow so we want to use
a background job. We also connect to the repository event mechanism
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../internal/decorators/GitResourceDecorator.java | 162 +++++++++++---------
1 files changed, 92 insertions(+), 70 deletions(-)
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java
index dcb87b7..c625ce6 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java
@@ -11,6 +11,8 @@ package org.spearce.egit.ui.internal.decorators;
import java.io.IOException;
import java.util.HashSet;
+import java.util.Iterator;
+import java.util.LinkedHashSet;
import java.util.Set;
import org.eclipse.core.resources.IContainer;
@@ -25,14 +27,19 @@ import org.eclipse.core.resources.IResourceVisitor;
import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IAdaptable;
+import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.QualifiedName;
+import org.eclipse.core.runtime.Status;
+import org.eclipse.core.runtime.jobs.ISchedulingRule;
+import org.eclipse.core.runtime.jobs.Job;
import org.eclipse.jface.viewers.IDecoration;
import org.eclipse.jface.viewers.ILightweightLabelDecorator;
import org.eclipse.jface.viewers.LabelProvider;
import org.eclipse.jface.viewers.LabelProviderChangedEvent;
import org.eclipse.swt.widgets.Display;
import org.eclipse.team.core.Team;
-import org.eclipse.ui.PlatformUI;
+import org.eclipse.ui.IDecoratorManager;
import org.spearce.egit.core.project.GitProjectData;
import org.spearce.egit.core.project.RepositoryChangeListener;
import org.spearce.egit.core.project.RepositoryMapping;
@@ -65,17 +72,24 @@ import org.spearce.jgit.lib.GitIndex.Entry;
public class GitResourceDecorator extends LabelProvider implements
ILightweightLabelDecorator {
- private static final RCL myrcl = new RCL();
+ static final String decoratorId = "org.spearce.egit.ui.internal.decorators.GitResourceDecorator";
+ static class ResCL extends Job implements IResourceChangeListener, RepositoryChangeListener, RepositoryListener {
- static class RCL implements RepositoryChangeListener, RepositoryListener, Runnable {
- private boolean requested;
+ ResCL() {
+ super("Git resource decorator trigger");
+ }
- public synchronized void run() {
- requested = false;
- PlatformUI.getWorkbench().getDecoratorManager().update(
- GitResourceDecorator.class.getName());
+ GitResourceDecorator getActiveDecorator() {
+ IDecoratorManager decoratorManager = Activator.getDefault()
+ .getWorkbench().getDecoratorManager();
+ if (decoratorManager.getEnabled(decoratorId))
+ return (GitResourceDecorator) decoratorManager
+ .getLightweightLabelDecorator(decoratorId);
+ return null;
}
+ private Set<IResource> resources = new LinkedHashSet<IResource>();
+
public void refsChanged(RefsChangedEvent e) {
repositoryChanged(e);
}
@@ -97,76 +111,87 @@ public class GitResourceDecorator extends LabelProvider implements
}
public void repositoryChanged(final RepositoryMapping which) {
- try {
- which.getContainer().accept(new IResourceVisitor() {
- public boolean visit(IResource resource) throws CoreException {
- if (resource instanceof IContainer)
- clearDecorationState(resource);
- return true;
- }
- });
- } catch (CoreException e) {
- // TODO Auto-generated catch block
- e.printStackTrace();
+ synchronized (resources) {
+ resources.add(which.getContainer());
}
- start();
+ schedule();
}
- synchronized void start() {
- if (requested)
- return;
- final Display d = PlatformUI.getWorkbench().getDisplay();
- if (d.getThread() == Thread.currentThread())
- run();
- else {
- requested = true;
- d.asyncExec(this);
+ @Override
+ protected IStatus run(IProgressMonitor arg0) {
+ try {
+ if (resources.size() > 0) {
+ IResource m;
+ synchronized(resources) {
+ Iterator<IResource> i = resources.iterator();
+ m = i.next();
+ i.remove();
+ if (resources.size() > 0)
+ schedule();
+ }
+ ISchedulingRule markerRule = m.getWorkspace().getRuleFactory().markerRule(m);
+ getJobManager().beginRule(markerRule, arg0);
+ try {
+ m.accept(new IResourceVisitor() {
+ public boolean visit(IResource resource) throws CoreException {
+ getActiveDecorator().clearDecorationState(resource);
+ return true;
+ }
+ });
+ } finally {
+ getJobManager().endRule(markerRule);
+ }
+ }
+ return Status.OK_STATUS;
+ } catch (Exception e) {
+ return new Status(IStatus.ERROR, Activator.getPluginId(), "Failed to trigger resource decoration", e);
}
}
- }
- static class ResCL implements IResourceChangeListener {
public void resourceChanged(IResourceChangeEvent event) {
if (event.getType() != IResourceChangeEvent.POST_CHANGE) {
return;
}
try {
event.getDelta().accept(new IResourceDeltaVisitor() {
-
public boolean visit(IResourceDelta delta)
throws CoreException {
for (IResource r = delta.getResource(); r.getType() != IResource.ROOT; r = r
.getParent()) {
- try {
- clearDecorationState(r);
- } catch (CoreException e) {
- // TODO Auto-generated catch block
- e.printStackTrace();
+ synchronized (resources) {
+ resources.add(r);
}
}
return true;
}
-
});
- } catch (CoreException e2) {
- // TODO Auto-generated catch block
- e2.printStackTrace();
- return;
+ } catch (Exception e) {
+ Activator.logError("Problem during decorations. Stopped", e);
}
- myrcl.start();
+ schedule();
}
- }
- static void clearDecorationState(IResource r) throws CoreException {
+ void force() {
+ for (IProject p : ResourcesPlugin.getWorkspace().getRoot().getProjects()) {
+ synchronized (resources) {
+ resources.add(p);
+ }
+ }
+ schedule();
+ }
+ } // End ResCL
+
+ void clearDecorationState(IResource r) throws CoreException {
if (r.isAccessible())
r.setSessionProperty(GITFOLDERDIRTYSTATEPROPERTY, null);
+ fireLabelProviderChanged(new LabelProviderChangedEvent(this, r));
}
static ResCL myrescl = new ResCL();
static {
- Repository.addAnyRepositoryChangedListener(myrcl);
- GitProjectData.addRepositoryChangeListener(myrcl);
+ Repository.addAnyRepositoryChangedListener(myrescl);
+ GitProjectData.addRepositoryChangeListener(myrescl);
ResourcesPlugin.getWorkspace().addResourceChangeListener(myrescl,
IResourceChangeEvent.POST_CHANGE);
}
@@ -179,7 +204,7 @@ public class GitResourceDecorator extends LabelProvider implements
* </p>
*/
public static void refresh() {
- myrcl.start();
+ myrescl.force();
}
private static IResource toIResource(final Object e) {
@@ -368,36 +393,33 @@ public class GitResourceDecorator extends LabelProvider implements
try {
Integer dirty = (Integer) rsrc.getSessionProperty(GITFOLDERDIRTYSTATEPROPERTY);
+ Runnable runnable = new Runnable() {
+ public void run() {
+ // Async could be called after a
+ // project is closed or a
+ // resource is deleted
+ if (!rsrc.isAccessible())
+ return;
+ fireLabelProviderChanged(new LabelProviderChangedEvent(
+ GitResourceDecorator.this, rsrc));
+ }
+ };
if (dirty == null) {
rsrc.setSessionProperty(GITFOLDERDIRTYSTATEPROPERTY, new Integer(flag));
orState(rsrc.getParent(), flag);
- Display.getDefault().asyncExec(new Runnable() {
- public void run() {
- // Async could be called after a
- // project is closed or a
- // resource is deleted
- if (!rsrc.isAccessible())
- return;
- fireLabelProviderChanged(new LabelProviderChangedEvent(
- GitResourceDecorator.this, rsrc));
- }
- });
+// if (Thread.currentThread() == Display.getDefault().getThread())
+// runnable.run();
+// else
+ Display.getDefault().asyncExec(runnable);
} else {
if ((dirty.intValue() | flag) != dirty.intValue()) {
dirty = new Integer(dirty.intValue() | flag);
rsrc.setSessionProperty(GITFOLDERDIRTYSTATEPROPERTY, dirty);
orState(rsrc.getParent(), dirty.intValue());
- Display.getDefault().asyncExec(new Runnable() {
- public void run() {
- // Async could be called after a
- // project is closed or a
- // resource is deleted
- if (!rsrc.isAccessible())
- return;
- fireLabelProviderChanged(new LabelProviderChangedEvent(
- GitResourceDecorator.this, rsrc));
- }
- });
+// if (Thread.currentThread() == Display.getDefault().getThread())
+// runnable.run();
+// else
+ Display.getDefault().asyncExec(runnable);
}
}
} catch (CoreException e) {
--
1.5.6.2.220.g44701
^ permalink raw reply related
* [EGIT PATCH 3/3] Log decoration problems more silently
From: Robin Rosenberg @ 2008-07-10 22:41 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Marek Zawirski, Robin Rosenberg
In-Reply-To: <1215729668-26865-3-git-send-email-robin.rosenberg@dewire.com>
Seems our logging caused too much alerts in the UI.
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../internal/decorators/GitResourceDecorator.java | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java
index 8d4a7c7..6d2f88e 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java
@@ -146,7 +146,9 @@ public class GitResourceDecorator extends LabelProvider implements
}
return Status.OK_STATUS;
} catch (Exception e) {
- return new Status(IStatus.ERROR, Activator.getPluginId(), "Failed to trigger resource decoration", e);
+ // We must be silent here or the UI will panic with lots of error messages
+ Activator.logError("Failed to trigger resource re-decoration", e);
+ return Status.OK_STATUS;
}
}
--
1.5.6.2.220.g44701
^ permalink raw reply related
* [EGIT PATCH 0/3] Resource decorator improvements
From: Robin Rosenberg @ 2008-07-10 22:41 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Marek Zawirski, Robin Rosenberg
Connect the decorator and fix some problems. The first patch depends
on the repository event mechanism.
-- robin
Robin Rosenberg (3):
Use a job for the resource decorator
Avoid errors by allowing phantoms during traversal in the resource
decorator
Log decoration problems more silently
.../internal/decorators/GitResourceDecorator.java | 168 ++++++++++++--------
1 files changed, 98 insertions(+), 70 deletions(-)
^ permalink raw reply
* [EGIT PATCH 9/9] Attach the resource decorator to the repository change event mechanism
From: Robin Rosenberg @ 2008-07-10 22:40 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Marek Zawirski, Robin Rosenberg
In-Reply-To: <1215729651-26781-9-git-send-email-robin.rosenberg@dewire.com>
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../internal/decorators/GitResourceDecorator.java | 29 +++++++++++++++++++-
1 files changed, 28 insertions(+), 1 deletions(-)
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java
index 0308f6a..dcb87b7 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java
@@ -10,6 +10,8 @@
package org.spearce.egit.ui.internal.decorators;
import java.io.IOException;
+import java.util.HashSet;
+import java.util.Set;
import org.eclipse.core.resources.IContainer;
import org.eclipse.core.resources.IFile;
@@ -38,7 +40,11 @@ import org.spearce.egit.ui.Activator;
import org.spearce.egit.ui.UIIcons;
import org.spearce.egit.ui.UIText;
import org.spearce.jgit.lib.GitIndex;
+import org.spearce.jgit.lib.IndexChangedEvent;
+import org.spearce.jgit.lib.RefsChangedEvent;
import org.spearce.jgit.lib.Repository;
+import org.spearce.jgit.lib.RepositoryChangedEvent;
+import org.spearce.jgit.lib.RepositoryListener;
import org.spearce.jgit.lib.RepositoryState;
import org.spearce.jgit.lib.Tree;
import org.spearce.jgit.lib.TreeEntry;
@@ -61,7 +67,7 @@ public class GitResourceDecorator extends LabelProvider implements
private static final RCL myrcl = new RCL();
- static class RCL implements RepositoryChangeListener, Runnable {
+ static class RCL implements RepositoryChangeListener, RepositoryListener, Runnable {
private boolean requested;
public synchronized void run() {
@@ -70,6 +76,26 @@ public class GitResourceDecorator extends LabelProvider implements
GitResourceDecorator.class.getName());
}
+ public void refsChanged(RefsChangedEvent e) {
+ repositoryChanged(e);
+ }
+
+ public void indexChanged(IndexChangedEvent e) {
+ repositoryChanged(e);
+ }
+
+ private void repositoryChanged(RepositoryChangedEvent e) {
+ Set<RepositoryMapping> ms = new HashSet<RepositoryMapping>();
+ for (IProject p : ResourcesPlugin.getWorkspace().getRoot().getProjects()) {
+ RepositoryMapping mapping = RepositoryMapping.getMapping(p);
+ if (mapping != null && mapping.getRepository() == e.getRepository())
+ ms.add(mapping);
+ }
+ for (RepositoryMapping m : ms) {
+ repositoryChanged(m);
+ }
+ }
+
public void repositoryChanged(final RepositoryMapping which) {
try {
which.getContainer().accept(new IResourceVisitor() {
@@ -139,6 +165,7 @@ public class GitResourceDecorator extends LabelProvider implements
static ResCL myrescl = new ResCL();
static {
+ Repository.addAnyRepositoryChangedListener(myrcl);
GitProjectData.addRepositoryChangeListener(myrcl);
ResourcesPlugin.getWorkspace().addResourceChangeListener(myrescl,
IResourceChangeEvent.POST_CHANGE);
--
1.5.6.2.220.g44701
^ permalink raw reply related
* [EGIT PATCH 7/9] Add a job to refresh projects when the index changes.
From: Robin Rosenberg @ 2008-07-10 22:40 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Marek Zawirski, Robin Rosenberg
In-Reply-To: <1215729651-26781-7-git-send-email-robin.rosenberg@dewire.com>
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../src/org/spearce/egit/ui/Activator.java | 83 +++++++++++++++++++-
1 files changed, 79 insertions(+), 4 deletions(-)
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/Activator.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/Activator.java
index 3e02c44..39d3bc9 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/Activator.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/Activator.java
@@ -11,16 +11,19 @@ package org.spearce.egit.ui;
import java.net.Authenticator;
import java.net.ProxySelector;
import java.util.HashSet;
+import java.util.LinkedHashSet;
import java.util.Set;
import org.eclipse.core.net.proxy.IProxyService;
import org.eclipse.core.resources.IProject;
+import org.eclipse.core.resources.IResource;
import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.Platform;
import org.eclipse.core.runtime.Status;
+import org.eclipse.core.runtime.SubProgressMonitor;
import org.eclipse.core.runtime.jobs.ISchedulingRule;
import org.eclipse.core.runtime.jobs.Job;
import org.eclipse.jsch.core.IJSchService;
@@ -30,7 +33,10 @@ import org.eclipse.ui.themes.ITheme;
import org.osgi.framework.BundleContext;
import org.osgi.framework.ServiceReference;
import org.spearce.egit.core.project.RepositoryMapping;
+import org.spearce.jgit.lib.IndexChangedEvent;
+import org.spearce.jgit.lib.RefsChangedEvent;
import org.spearce.jgit.lib.Repository;
+import org.spearce.jgit.lib.RepositoryListener;
import org.spearce.jgit.transport.SshSessionFactory;
/**
@@ -137,6 +143,7 @@ public class Activator extends AbstractUIPlugin {
private boolean traceVerbose;
private RCS rcs;
+ private RIRefresh refreshJob;
/**
* Constructor for the egit ui plugin singleton
@@ -151,6 +158,70 @@ public class Activator extends AbstractUIPlugin {
setupSSH(context);
setupProxy(context);
setupRepoChangeScanner();
+ setupRepoIndexRefresh();
+ }
+
+ private void setupRepoIndexRefresh() {
+ refreshJob = new RIRefresh();
+ Repository.addAnyRepositoryChangedListener(refreshJob);
+ }
+
+ static class RIRefresh extends Job implements RepositoryListener {
+
+ RIRefresh() {
+ super("Git index refresh Job");
+ }
+
+ private Set<IProject> projectsToScan = new LinkedHashSet<IProject>();
+
+ @Override
+ protected IStatus run(IProgressMonitor monitor) {
+ IProject[] projects = ResourcesPlugin.getWorkspace().getRoot().getProjects();
+ monitor.beginTask("Refreshing git managed projects", projects.length);
+
+ while (projectsToScan.size() > 0) {
+ IProject p;
+ synchronized (projectsToScan) {
+ if (projectsToScan.size() == 0) {
+ }
+ p = projectsToScan.iterator().next();
+ projectsToScan.remove(p);
+ }
+ ISchedulingRule rule = p.getWorkspace().getRuleFactory().refreshRule(p);
+ try {
+ getJobManager().beginRule(rule, monitor);
+ p.refreshLocal(IResource.DEPTH_INFINITE, new SubProgressMonitor(monitor, 1));
+ } catch (CoreException e) {
+ logError("Failed to refresh projects from index changes", e);
+ return new Status(IStatus.ERROR, getPluginId(), e.getMessage());
+ } finally {
+ getJobManager().endRule(rule);
+ }
+ }
+ monitor.done();
+ return Status.OK_STATUS;
+ }
+
+ public void indexChanged(IndexChangedEvent e) {
+ IProject[] projects = ResourcesPlugin.getWorkspace().getRoot().getProjects();
+ Set<IProject> toRefresh= new HashSet<IProject>();
+ for (IProject p : projects) {
+ RepositoryMapping mapping = RepositoryMapping.getMapping(p);
+ if (mapping != null && mapping.getRepository() == e.getRepository()) {
+ toRefresh.add(p);
+ }
+ }
+ synchronized (projectsToScan) {
+ projectsToScan.addAll(toRefresh);
+ }
+ if (projectsToScan.size() > 0)
+ schedule();
+ }
+
+ public void refsChanged(RefsChangedEvent e) {
+ // Do not react here
+ }
+
}
static class RCS extends Job {
@@ -236,10 +307,14 @@ public class Activator extends AbstractUIPlugin {
public void stop(final BundleContext context) throws Exception {
trace("Trying to cancel " + rcs.getName() + " job");
- if (!rcs.cancel()) {
- rcs.join();
- }
- trace("rcs.getName() " + rcs.getName() + " cancelled ok");
+ rcs.cancel();
+ trace("Trying to cancel " + refreshJob.getName() + " job");
+ refreshJob.cancel();
+
+ rcs.join();
+ refreshJob.join();
+
+ trace("Jobs terminated");
super.stop(context);
plugin = null;
}
--
1.5.6.2.220.g44701
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox