[PATCH v2 00/14] Remove unused code from imap-send.c
This is a re-roll, incorporating the feedback of Jonathan Nieder (thanks!). Differences from v1: * Added comments to get_cmd_result() at the place where the NAMESPACE response is skipped over. * Added some comments to lf_to_crlf(), simplified the code a bit further, and expanded the commit message. * Replaced erroneously-deleted space in APPEND command in imap_store_msg(). I also moved the patch rewriting lf_to_crlf() to the end of the series, because it is not just dead-code elimination like the others. Michael Haggerty (14): imap-send.c: remove msg_data::flags, which was always zero imap-send.c: remove struct msg_data iamp-send.c: remove unused struct imap_store_conf imap-send.c: remove struct store_conf imap-send.c: remove struct message imap-send.c: remove some unused fields from struct store imap-send.c: inline imap_parse_list() in imap_list() imap-send.c: remove struct imap argument to parse_imap_list_l() imap-send.c: remove namespace fields from struct imap imap-send.c: remove unused field imap_store::trashnc imap-send.c: use struct imap_store instead of struct store imap-send.c: remove unused field imap_store::uidvalidity imap-send.c: fold struct store into struct imap_store imap-send.c: simplify logic in lf_to_crlf() imap-send.c | 308 +++- 1 file changed, 55 insertions(+), 253 deletions(-) -- 1.8.0.3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 03/14] iamp-send.c: remove unused struct imap_store_conf
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
imap-send.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index 29c10a4..dbe0546 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -130,11 +130,6 @@ static struct imap_server_conf server = {
NULL, /* auth_method */
};
-struct imap_store_conf {
- struct store_conf gen;
- struct imap_server_conf *server;
-};
-
#define NIL(void *)0x1
#define LIST (void *)0x2
--
1.8.0.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 05/14] imap-send.c: remove struct message
It was never used.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
imap-send.c | 26 --
1 file changed, 26 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index b8a7ff9..9e181e0 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -33,23 +33,10 @@ typedef void *SSL;
#include openssl/hmac.h
#endif
-/* For message-status */
-#define M_RECENT (10) /* unsyncable flag; maildir_* depend on this
being 10 */
-#define M_DEAD (11) /* expunged */
-#define M_FLAGS(12) /* flags fetched */
-
-struct message {
- struct message *next;
- size_t size; /* zero implies not fetched */
- int uid;
- unsigned char flags, status;
-};
-
struct store {
/* currently open mailbox */
const char *name; /* foreign! maybe preset? */
char *path; /* own */
- struct message *msgs; /* own */
int uidvalidity;
unsigned char opts; /* maybe preset? */
/* note that the following do _not_ reflect stats from msgs, but
mailbox totals */
@@ -74,8 +61,6 @@ static void imap_warn(const char *, ...);
static char *next_arg(char **);
-static void free_generic_messages(struct message *);
-
__attribute__((format (printf, 3, 4)))
static int nfsnprintf(char *buf, int blen, const char *fmt, ...);
@@ -447,16 +432,6 @@ static char *next_arg(char **s)
return ret;
}
-static void free_generic_messages(struct message *msgs)
-{
- struct message *tmsg;
-
- for (; msgs; msgs = tmsg) {
- tmsg = msgs-next;
- free(msgs);
- }
-}
-
static int nfsnprintf(char *buf, int blen, const char *fmt, ...)
{
int ret;
@@ -914,7 +889,6 @@ static void imap_close_server(struct imap_store *ictx)
static void imap_close_store(struct store *ctx)
{
imap_close_server((struct imap_store *)ctx);
- free_generic_messages(ctx-msgs);
free(ctx);
}
--
1.8.0.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 08/14] imap-send.c: remove struct imap argument to parse_imap_list_l()
It was always set to NULL.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
imap-send.c | 39 +++
1 file changed, 3 insertions(+), 36 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index cbbf845..29e4037 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -578,11 +578,10 @@ static void free_list(struct imap_list *list)
}
}
-static int parse_imap_list_l(struct imap *imap, char **sp, struct imap_list
**curp, int level)
+static int parse_imap_list_l(char **sp, struct imap_list **curp, int level)
{
struct imap_list *cur;
char *s = *sp, *p;
- int n, bytes;
for (;;) {
while (isspace((unsigned char)*s))
@@ -598,39 +597,7 @@ static int parse_imap_list_l(struct imap *imap, char **sp,
struct imap_list **cu
/* sublist */
s++;
cur-val = LIST;
- if (parse_imap_list_l(imap, s, cur-child, level + 1))
- goto bail;
- } else if (imap *s == '{') {
- /* literal */
- bytes = cur-len = strtol(s + 1, s, 10);
- if (*s != '}')
- goto bail;
-
- s = cur-val = xmalloc(cur-len);
-
- /* dump whats left over in the input buffer */
- n = imap-buf.bytes - imap-buf.offset;
-
- if (n bytes)
- /* the entire message fit in the buffer */
- n = bytes;
-
- memcpy(s, imap-buf.buf + imap-buf.offset, n);
- s += n;
- bytes -= n;
-
- /* mark that we used part of the buffer */
- imap-buf.offset += n;
-
- /* now read the rest of the message */
- while (bytes 0) {
- if ((n = socket_read(imap-buf.sock, s,
bytes)) = 0)
- goto bail;
- s += n;
- bytes -= n;
- }
-
- if (buffer_gets(imap-buf, s))
+ if (parse_imap_list_l(s, cur-child, level + 1))
goto bail;
} else if (*s == '') {
/* quoted string */
@@ -673,7 +640,7 @@ static struct imap_list *parse_list(char **sp)
{
struct imap_list *head;
- if (!parse_imap_list_l(NULL, sp, head, 0))
+ if (!parse_imap_list_l(sp, head, 0))
return head;
free_list(head);
return NULL;
--
1.8.0.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 09/14] imap-send.c: remove namespace fields from struct imap
They are unused, and their removal means that a bunch of list-related
infrastructure can be disposed of.
It might be that the NAMESPACE response that is now skipped over in
get_cmd_result() should never be sent by the server. But somebody
would have to check the IMAP protocol and how we interact with the
server to be sure. So for now I am leaving that branch of the if
statement there.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
imap-send.c | 75 -
1 file changed, 9 insertions(+), 66 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index 29e4037..ff44013 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -99,15 +99,6 @@ static struct imap_server_conf server = {
NULL, /* auth_method */
};
-#define NIL(void *)0x1
-#define LIST (void *)0x2
-
-struct imap_list {
- struct imap_list *next, *child;
- char *val;
- int len;
-};
-
struct imap_socket {
int fd[2];
SSL *ssl;
@@ -124,7 +115,6 @@ struct imap_cmd;
struct imap {
int uidnext; /* from SELECT responses */
- struct imap_list *ns_personal, *ns_other, *ns_shared; /* NAMESPACE info
*/
unsigned caps, rcaps; /* CAPABILITY results */
/* command queue */
int nexttag, num_in_progress, literal_pending;
@@ -554,34 +544,9 @@ static int imap_exec_m(struct imap_store *ctx, struct
imap_cmd_cb *cb,
}
}
-static int is_atom(struct imap_list *list)
-{
- return list list-val list-val != NIL list-val != LIST;
-}
-
-static int is_list(struct imap_list *list)
-{
- return list list-val == LIST;
-}
-
-static void free_list(struct imap_list *list)
-{
- struct imap_list *tmp;
-
- for (; list; list = tmp) {
- tmp = list-next;
- if (is_list(list))
- free_list(list-child);
- else if (is_atom(list))
- free(list-val);
- free(list);
- }
-}
-
-static int parse_imap_list_l(char **sp, struct imap_list **curp, int level)
+static int skip_imap_list_l(char **sp, int level)
{
- struct imap_list *cur;
- char *s = *sp, *p;
+ char *s = *sp;
for (;;) {
while (isspace((unsigned char)*s))
@@ -590,36 +555,23 @@ static int parse_imap_list_l(char **sp, struct imap_list
**curp, int level)
s++;
break;
}
- *curp = cur = xmalloc(sizeof(*cur));
- curp = cur-next;
- cur-val = NULL; /* for clean bail */
if (*s == '(') {
/* sublist */
s++;
- cur-val = LIST;
- if (parse_imap_list_l(s, cur-child, level + 1))
+ if (skip_imap_list_l(s, level + 1))
goto bail;
} else if (*s == '') {
/* quoted string */
s++;
- p = s;
for (; *s != ''; s++)
if (!*s)
goto bail;
- cur-len = s - p;
s++;
- cur-val = xmemdupz(p, cur-len);
} else {
/* atom */
- p = s;
for (; *s !isspace((unsigned char)*s); s++)
if (level *s == ')')
break;
- cur-len = s - p;
- if (cur-len == 3 !memcmp(NIL, p, 3))
- cur-val = NIL;
- else
- cur-val = xmemdupz(p, cur-len);
}
if (!level)
@@ -628,22 +580,15 @@ static int parse_imap_list_l(char **sp, struct imap_list
**curp, int level)
goto bail;
}
*sp = s;
- *curp = NULL;
return 0;
bail:
- *curp = NULL;
return -1;
}
-static struct imap_list *parse_list(char **sp)
+static void skip_list(char **sp)
{
- struct imap_list *head;
-
- if (!parse_imap_list_l(sp, head, 0))
- return head;
- free_list(head);
- return NULL;
+ skip_imap_list_l(sp, 0);
}
static void parse_capability(struct imap *imap, char *cmd)
@@ -722,9 +667,10 @@ static int get_cmd_result(struct imap_store *ctx, struct
imap_cmd *tcmd)
}
if (!strcmp(NAMESPACE, arg)) {
- imap-ns_personal = parse_list(cmd);
- imap-ns_other = parse_list(cmd);
- imap-ns_shared = parse_list(cmd);
+ /* rfc2342 NAMESPACE response. */
+ skip_list(cmd); /* Personal mailboxes */
+
[PATCH v2 10/14] imap-send.c: remove unused field imap_store::trashnc
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
imap-send.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index ff44013..909e4db 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -127,7 +127,6 @@ struct imap_store {
int uidvalidity;
struct imap *imap;
const char *prefix;
- unsigned /*currentnc:1,*/ trashnc:1;
};
struct imap_cmd_cb {
@@ -1080,7 +1079,6 @@ static struct store *imap_open_store(struct
imap_server_conf *srvc)
} /* !preauth */
ctx-prefix = ;
- ctx-trashnc = 1;
return (struct store *)ctx;
bail:
--
1.8.0.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 13/14] imap-send.c: fold struct store into struct imap_store
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
imap-send.c | 18 +++---
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index a0f42bb..f2933e9 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -33,12 +33,6 @@ typedef void *SSL;
#include openssl/hmac.h
#endif
-struct store {
- /* currently open mailbox */
- const char *name; /* foreign! maybe preset? */
- int uidvalidity;
-};
-
static const char imap_send_usage[] = git imap-send mbox;
#undef DRV_OK
@@ -123,7 +117,9 @@ struct imap {
};
struct imap_store {
- struct store gen;
+ /* currently open mailbox */
+ const char *name; /* foreign! maybe preset? */
+ int uidvalidity;
struct imap *imap;
const char *prefix;
};
@@ -618,7 +614,7 @@ static int parse_response_code(struct imap_store *ctx,
struct imap_cmd_cb *cb,
*p++ = 0;
arg = next_arg(s);
if (!strcmp(UIDVALIDITY, arg)) {
- if (!(arg = next_arg(s)) || !(ctx-gen.uidvalidity =
atoi(arg))) {
+ if (!(arg = next_arg(s)) || !(ctx-uidvalidity = atoi(arg))) {
fprintf(stderr, IMAP error: malformed UIDVALIDITY
status\n);
return RESP_BAD;
}
@@ -636,7 +632,7 @@ static int parse_response_code(struct imap_store *ctx,
struct imap_cmd_cb *cb,
for (; isspace((unsigned char)*p); p++);
fprintf(stderr, *** IMAP ALERT *** %s\n, p);
} else if (cb cb-ctx !strcmp(APPENDUID, arg)) {
- if (!(arg = next_arg(s)) || !(ctx-gen.uidvalidity =
atoi(arg)) ||
+ if (!(arg = next_arg(s)) || !(ctx-uidvalidity = atoi(arg)) ||
!(arg = next_arg(s)) || !(*(int *)cb-ctx = atoi(arg))) {
fprintf(stderr, IMAP error: malformed APPENDUID
status\n);
return RESP_BAD;
@@ -1140,7 +1136,7 @@ static int imap_store_msg(struct imap_store *ctx, struct
strbuf *msg)
cb.dlen = msg-len;
cb.data = strbuf_detach(msg, NULL);
- box = ctx-gen.name;
+ box = ctx-name;
prefix = !strcmp(box, INBOX) ? : ctx-prefix;
cb.create = 0;
ret = imap_exec_m(ctx, cb, APPEND \%s%s\ , prefix, box);
@@ -1352,7 +1348,7 @@ int main(int argc, char **argv)
}
fprintf(stderr, sending %d message%s\n, total, (total != 1) ? s :
);
- ctx-gen.name = imap_folder;
+ ctx-name = imap_folder;
while (1) {
unsigned percent = n * 100 / total;
--
1.8.0.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 12/14] imap-send.c: remove unused field imap_store::uidvalidity
I suspect that the existence of both imap_store::uidvalidity and
store::uidvalidity was an accident.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
imap-send.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/imap-send.c b/imap-send.c
index 48c646c..a0f42bb 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -124,7 +124,6 @@ struct imap {
struct imap_store {
struct store gen;
- int uidvalidity;
struct imap *imap;
const char *prefix;
};
--
1.8.0.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 04/14] imap-send.c: remove struct store_conf
It was never used.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
imap-send.c | 11 ---
1 file changed, 11 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index dbe0546..b8a7ff9 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -33,15 +33,6 @@ typedef void *SSL;
#include openssl/hmac.h
#endif
-struct store_conf {
- char *name;
- const char *path; /* should this be here? its interpretation is
driver-specific */
- char *map_inbox;
- char *trash;
- unsigned max_size; /* off_t is overkill */
- unsigned trash_remote_new:1, trash_only_new:1;
-};
-
/* For message-status */
#define M_RECENT (10) /* unsyncable flag; maildir_* depend on this
being 10 */
#define M_DEAD (11) /* expunged */
@@ -55,8 +46,6 @@ struct message {
};
struct store {
- struct store_conf *conf; /* foreign */
-
/* currently open mailbox */
const char *name; /* foreign! maybe preset? */
char *path; /* own */
--
1.8.0.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] Make git selectively and conditionally ignore certain stat fields
Robin Rosenberg robin.rosenb...@dewire.com writes: I'd say a simplistic ignore if zero is stored or even ignore this as one of the systems that shares this file writes crap in it may be sufficient, and if this is a jGit specific issue, it might even make sense to introduce a single configuration variable with string jgit somewhere in its name and bypass the stat field comparison for known-problematic fields, instead of having the user know and list what stat fields need special attention. My first patch was something like that, just not using the word jgit. As for what fields to ignore, it's something that can be configured by EGit and documented on the EGit/JGit wiki. That configurability is a slipperly slope to drag us into giving users more complexity that does not help them very much, I suspect. Earlier somebody mentioned size and mtime is often enough, so I think a single option core.looseStatInfo (substitute loose with short, minimum or whatever adjective that is more appropriate---I am not good at picking phrases, it sounds to me a way to more loosely define stat info cleanliness than we usually do) that makes us ignore all fields (regardless of their zero-ness) other than those two fields might not be a bad way to go. I do not offhand know if such a loose mode is too simple and make it excessively risky, though. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 01/14] imap-send.c: remove msg_data::flags, which was always zero
This removes the need for function imap_make_flags(), so delete it,
too.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
imap-send.c | 40 +++-
1 file changed, 3 insertions(+), 37 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index e521e2f..f1c8f5a 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -70,7 +70,6 @@ struct store {
struct msg_data {
struct strbuf data;
- unsigned char flags;
};
static const char imap_send_usage[] = git imap-send mbox;
@@ -225,14 +224,6 @@ static const char *cap_list[] = {
static int get_cmd_result(struct imap_store *ctx, struct imap_cmd *tcmd);
-static const char *Flags[] = {
- Draft,
- Flagged,
- Answered,
- Seen,
- Deleted,
-};
-
#ifndef NO_OPENSSL
static void ssl_socket_perror(const char *func)
{
@@ -1246,23 +1237,6 @@ bail:
return NULL;
}
-static int imap_make_flags(int flags, char *buf)
-{
- const char *s;
- unsigned i, d;
-
- for (i = d = 0; i ARRAY_SIZE(Flags); i++)
- if (flags (1 i)) {
- buf[d++] = ' ';
- buf[d++] = '\\';
- for (s = Flags[i]; *s; s++)
- buf[d++] = *s;
- }
- buf[0] = '(';
- buf[d++] = ')';
- return d;
-}
-
static void lf_to_crlf(struct strbuf *msg)
{
size_t new_len;
@@ -1311,8 +1285,7 @@ static int imap_store_msg(struct store *gctx, struct
msg_data *msg)
struct imap *imap = ctx-imap;
struct imap_cmd_cb cb;
const char *prefix, *box;
- int ret, d;
- char flagstr[128];
+ int ret;
lf_to_crlf(msg-data);
memset(cb, 0, sizeof(cb));
@@ -1320,17 +1293,10 @@ static int imap_store_msg(struct store *gctx, struct
msg_data *msg)
cb.dlen = msg-data.len;
cb.data = strbuf_detach(msg-data, NULL);
- d = 0;
- if (msg-flags) {
- d = imap_make_flags(msg-flags, flagstr);
- flagstr[d++] = ' ';
- }
- flagstr[d] = 0;
-
box = gctx-name;
prefix = !strcmp(box, INBOX) ? : ctx-prefix;
cb.create = 0;
- ret = imap_exec_m(ctx, cb, APPEND \%s%s\ %s, prefix, box, flagstr);
+ ret = imap_exec_m(ctx, cb, APPEND \%s%s\ , prefix, box);
imap-caps = imap-rcaps;
if (ret != DRV_OK)
return ret;
@@ -1483,7 +1449,7 @@ static int git_imap_config(const char *key, const char
*val, void *cb)
int main(int argc, char **argv)
{
struct strbuf all_msgs = STRBUF_INIT;
- struct msg_data msg = {STRBUF_INIT, 0};
+ struct msg_data msg = {STRBUF_INIT};
struct store *ctx = NULL;
int ofs = 0;
int r;
--
1.8.0.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 07/14] imap-send.c: inline imap_parse_list() in imap_list()
The function is only called from here.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
imap-send.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index f193211..cbbf845 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -669,21 +669,16 @@ bail:
return -1;
}
-static struct imap_list *parse_imap_list(struct imap *imap, char **sp)
+static struct imap_list *parse_list(char **sp)
{
struct imap_list *head;
- if (!parse_imap_list_l(imap, sp, head, 0))
+ if (!parse_imap_list_l(NULL, sp, head, 0))
return head;
free_list(head);
return NULL;
}
-static struct imap_list *parse_list(char **sp)
-{
- return parse_imap_list(NULL, sp);
-}
-
static void parse_capability(struct imap *imap, char *cmd)
{
char *arg;
--
1.8.0.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 02/14] imap-send.c: remove struct msg_data
Now that its flags member has been deleted, all that is left is a
strbuf. So use a strbuf directly.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
imap-send.c | 18 +++---
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index f1c8f5a..29c10a4 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -68,10 +68,6 @@ struct store {
int recent; /* # of recent messages - don't trust this beyond the
initial read */
};
-struct msg_data {
- struct strbuf data;
-};
-
static const char imap_send_usage[] = git imap-send mbox;
#undef DRV_OK
@@ -1279,7 +1275,7 @@ static void lf_to_crlf(struct strbuf *msg)
* Store msg to IMAP. Also detach and free the data from msg-data,
* leaving msg-data empty.
*/
-static int imap_store_msg(struct store *gctx, struct msg_data *msg)
+static int imap_store_msg(struct store *gctx, struct strbuf *msg)
{
struct imap_store *ctx = (struct imap_store *)gctx;
struct imap *imap = ctx-imap;
@@ -1287,11 +1283,11 @@ static int imap_store_msg(struct store *gctx, struct
msg_data *msg)
const char *prefix, *box;
int ret;
- lf_to_crlf(msg-data);
+ lf_to_crlf(msg);
memset(cb, 0, sizeof(cb));
- cb.dlen = msg-data.len;
- cb.data = strbuf_detach(msg-data, NULL);
+ cb.dlen = msg-len;
+ cb.data = strbuf_detach(msg, NULL);
box = gctx-name;
prefix = !strcmp(box, INBOX) ? : ctx-prefix;
@@ -1449,7 +1445,7 @@ static int git_imap_config(const char *key, const char
*val, void *cb)
int main(int argc, char **argv)
{
struct strbuf all_msgs = STRBUF_INIT;
- struct msg_data msg = {STRBUF_INIT};
+ struct strbuf msg = STRBUF_INIT;
struct store *ctx = NULL;
int ofs = 0;
int r;
@@ -1511,10 +1507,10 @@ int main(int argc, char **argv)
unsigned percent = n * 100 / total;
fprintf(stderr, %4u%% (%d/%d) done\r, percent, n, total);
- if (!split_msg(all_msgs, msg.data, ofs))
+ if (!split_msg(all_msgs, msg, ofs))
break;
if (server.use_html)
- wrap_in_html(msg.data);
+ wrap_in_html(msg);
r = imap_store_msg(ctx, msg);
if (r != DRV_OK)
break;
--
1.8.0.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 11/14] imap-send.c: use struct imap_store instead of struct store
In fact, all struct store instances are upcasts of struct imap_store
anyway, so stop making the distinction.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
imap-send.c | 19 +--
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index 909e4db..48c646c 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -782,9 +782,9 @@ static void imap_close_server(struct imap_store *ictx)
free(imap);
}
-static void imap_close_store(struct store *ctx)
+static void imap_close_store(struct imap_store *ctx)
{
- imap_close_server((struct imap_store *)ctx);
+ imap_close_server(ctx);
free(ctx);
}
@@ -869,7 +869,7 @@ static int auth_cram_md5(struct imap_store *ctx, struct
imap_cmd *cmd, const cha
return 0;
}
-static struct store *imap_open_store(struct imap_server_conf *srvc)
+static struct imap_store *imap_open_store(struct imap_server_conf *srvc)
{
struct imap_store *ctx;
struct imap *imap;
@@ -1079,10 +1079,10 @@ static struct store *imap_open_store(struct
imap_server_conf *srvc)
} /* !preauth */
ctx-prefix = ;
- return (struct store *)ctx;
+ return ctx;
bail:
- imap_close_store(ctx-gen);
+ imap_close_store(ctx);
return NULL;
}
@@ -1128,9 +1128,8 @@ static void lf_to_crlf(struct strbuf *msg)
* Store msg to IMAP. Also detach and free the data from msg-data,
* leaving msg-data empty.
*/
-static int imap_store_msg(struct store *gctx, struct strbuf *msg)
+static int imap_store_msg(struct imap_store *ctx, struct strbuf *msg)
{
- struct imap_store *ctx = (struct imap_store *)gctx;
struct imap *imap = ctx-imap;
struct imap_cmd_cb cb;
const char *prefix, *box;
@@ -1142,7 +1141,7 @@ static int imap_store_msg(struct store *gctx, struct
strbuf *msg)
cb.dlen = msg-len;
cb.data = strbuf_detach(msg, NULL);
- box = gctx-name;
+ box = ctx-gen.name;
prefix = !strcmp(box, INBOX) ? : ctx-prefix;
cb.create = 0;
ret = imap_exec_m(ctx, cb, APPEND \%s%s\ , prefix, box);
@@ -1298,7 +1297,7 @@ int main(int argc, char **argv)
{
struct strbuf all_msgs = STRBUF_INIT;
struct strbuf msg = STRBUF_INIT;
- struct store *ctx = NULL;
+ struct imap_store *ctx = NULL;
int ofs = 0;
int r;
int total, n = 0;
@@ -1354,7 +1353,7 @@ int main(int argc, char **argv)
}
fprintf(stderr, sending %d message%s\n, total, (total != 1) ? s :
);
- ctx-name = imap_folder;
+ ctx-gen.name = imap_folder;
while (1) {
unsigned percent = n * 100 / total;
--
1.8.0.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 14/14] imap-send.c: simplify logic in lf_to_crlf()
* The first character in the string used to be special-cased to get
around the fact that msg-buf[i - 1] is not defined for i == 0.
Instead, keep track of the previous character in a separate
variable, lastc, initialized in such a way to let the loop handle
i == 0 correctly.
* Make the two loops over the string look as similar as possible to
make it more obvious that the count computed in the first pass
agrees with the true length of the new string written in the second
pass. As a side effect, this makes it possible to use the j
counter in place of lfnum and new_len.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
imap-send.c | 52 +++-
1 file changed, 23 insertions(+), 29 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index f2933e9..1d40207 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1081,42 +1081,36 @@ bail:
return NULL;
}
+/*
+ * Insert CR characters as necessary in *msg to ensure that every LF
+ * character in *msg is preceded by a CR.
+ */
static void lf_to_crlf(struct strbuf *msg)
{
- size_t new_len;
char *new;
- int i, j, lfnum = 0;
-
- if (msg-buf[0] == '\n')
- lfnum++;
- for (i = 1; i msg-len; i++) {
- if (msg-buf[i - 1] != '\r' msg-buf[i] == '\n')
- lfnum++;
+ size_t i, j;
+ char lastc;
+
+ /* First pass: tally, in j, the size of the new string: */
+ for (i = j = 0, lastc = '\0'; i msg-len; i++) {
+ if (msg-buf[i] == '\n' lastc != '\r')
+ j++; /* a CR will need to be added here */
+ lastc = msg-buf[i];
+ j++;
}
- new_len = msg-len + lfnum;
- new = xmalloc(new_len + 1);
- if (msg-buf[0] == '\n') {
- new[0] = '\r';
- new[1] = '\n';
- i = 1;
- j = 2;
- } else {
- new[0] = msg-buf[0];
- i = 1;
- j = 1;
- }
- for ( ; i msg-len; i++) {
- if (msg-buf[i] != '\n') {
- new[j++] = msg-buf[i];
- continue;
- }
- if (msg-buf[i - 1] != '\r')
+ new = xmalloc(j + 1);
+
+ /*
+* Second pass: write the new string. Note that this loop is
+* otherwise identical to the first pass.
+*/
+ for (i = j = 0, lastc = '\0'; i msg-len; i++) {
+ if (msg-buf[i] == '\n' lastc != '\r')
new[j++] = '\r';
- /* otherwise it already had CR before */
- new[j++] = '\n';
+ lastc = new[j++] = msg-buf[i];
}
- strbuf_attach(msg, new, new_len, new_len + 1);
+ strbuf_attach(msg, new, j, j + 1);
}
/*
--
1.8.0.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
most proper presents thomas sabo charm bracelet
No matter whether Christmas or Evening of romance or any other wedding that you would need to celebrate, thomas sabo uk reward may be just appropriate for just a celebration. Some individuals see an exceptional demonstrates to uncover for themselves. That one could purchase unique occasion at the previous instant, you are positive to buy another thing very good at Thomas sabo UK. Thomas sabo presents ordinarily are certainly not limited to plainly your spouse, you may want to decide on most proper presents thomas sabo charm bracelet for pretty considerably anyone, together together with your buddies and interaction. The range is quite important and for a great deal of wide assortment that you simply are definate to discover a issue for every single individual. Warmth along with anticipate the best small bit of special place ends together with the wide array sold at Thomas sabo. Collection with the Thomas Sabo Charm Club Collection is at present contains more than 500 troubles hanging necklaces, earrings, bracelets, and so on, and continue to obtain new additions. One of your issues introduced by Thomas Sabo preferred is the fact that of thomas sabo bracelet of Barbie. We all realize that Barbie may be the most well-known girl right now. This collection brings amazing gifts for his daughter, a very good pal, that their loved ones. Thomas Sabo Charms make us content and really feel great. They say that charms symbolize our obsessions and passions. They truly are an excellent approach to make someone really feel superb and stand out inside the crowd. One unique place of charms is known as the kids club. These are for young kids and tweens. Your children can produce their very own person style at the same time. You will discover specific bracelets and necklaces that have been created to match the smaller attributes of kids. They may get essentially the most out of their charms. There is an virtually endless array of cross and symbol charms. You could stand up for the beliefs or express your feelings with thomas sabo bracelets uk and symbol charms. You could also say it out loud with glitter letter charms, silver letter, or the hearts and love section. It is possible to also get stone pendants and tags. It truly is difficult not to express your self with all the options that Thomas Sabo charms must offer. http://www.thomassabobraceletsshop.co.uk/ -- View this message in context: http://git.661346.n2.nabble.com/most-proper-presents-thomas-sabo-charm-bracelet-tp7575136.html Sent from the git mailing list archive at Nabble.com. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
That Explains Why Everybody Is Preaching About thomas sabo
When i received these jewelry (combined with matching necklace around your neck) for Christmas. This graphic would not carry out them rights. The product quality can be great and perhaps they are seriously attractive! thomas sabo bracelet http://www.thomassaboukjewellery.co.uk/ have a seriously eye-catching form and perhaps they are simply the right dimensions. They're apparent devoid of being gaudy, and perhaps they are hence light which you seriously can not possibly convey to you are putting on them. I love them! Thing was shipped soon and also was grouped together perfectly. This jewelry are great! They're gleaming, tiny, and also simply what exactly I needed. Various other reviewers stated how modest they're, thomas sabo chains http://www.thomassaboukjewellery.co.uk/ can be what exactly advised my family for you to order them. With my own age, We are definitely not trying to find large nice jewelry, although We are definitely not way too old to understand a fantastic looking set of two jewelry. Many people match my own armoire, specially tight pants or skirts as well as other informal wear. These jewelry are enjoyable for you to wear rather than serious in the least. I'm keen jewelry that is definitely unique and also these fit the bill. I do think I'm visiting verify in the event there is certainly a new matching necklace around your neck. I got trying to find a couple jewelry which have been fashionable and can likely be worn out with an every single day time frame. http: //www. thomassaboukjewellery. co. uk/ http://www.thomassaboukjewellery.co.uk/ set of two hits this indicate. This jewelry are sound, obviously with high class. They could be recycled way too large rather than way too modest. This gold can be nice gleaming, and in addition they look good with. I will find putting on these using numerous wardrobe. Complete I'm very proud of this order, and also simply bought this matching pendant. -- View this message in context: http://git.661346.n2.nabble.com/That-Explains-Why-Everybody-Is-Preaching-About-thomas-sabo-tp7575158.html Sent from the git mailing list archive at Nabble.com. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
The Very Best Stratagems For thomas sabo
I adore thomas sabo jewellery http://www.thomassaboonlineau.com/ necklace around your neck, the only dilemma which i possess is always that getting older reside instantly seeing that proven around the graphic mainly because there are actually simply no hooks to keep this this way. This typically keeps with one of many aspect. Usually, however, this converts upside down because underside can be more substantial. It is a exclusively dilemma I have from it normally I adore this! When i bought thomas sabo bracelet http://www.thomassaboonlineau.com/ for my own princess for your ex earliest Mother's Time. When your lover adores this and also sports this quite a lot, she has made remarks it can be little bit major serious hence much of the time this pendant can be upside down. Rather than this... it turned out a fantastic treasure! Complete it is pefect for what exactly this thomas sabo au http://www.thomassaboonlineau.com/ ... a new adorable bit of necklace around your neck. When i don't give this numerous personalities for education and learning and also strength mainly because I'm rather than sure these have just about any educational price rather than understanding how to address long run jewelry and also most of us never have seriously subjected to testing this strength rather than to place this with and also go on it away. This sequence can be very thin, then one was twisted upon best, although I got in a position to untangle and also there have been simply no injuries (simply questioning how this wound up draped with the cardiovascular system lots of occasions, and also was however mounted). Many people can be found in a new catch the attention of stringed blue http: //www. thomassaboonlineau. com/ http://www.thomassaboonlineau.com/ , after which from a bit of common box, that is definitely in that case grouped together throughout a different common box. So they really receive packaging this jewelry very significant. -- View this message in context: http://git.661346.n2.nabble.com/The-Very-Best-Stratagems-For-thomas-sabo-tp7575159.html Sent from the git mailing list archive at Nabble.com. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] Possible bug in `remote set-url --add --push`
Junio C Hamano venit, vidit, dixit 15.01.2013 07:39: Junio C Hamano gits...@pobox.com writes: Jardel Weyrich jweyr...@gmail.com writes: If you allow me, I'd like you to forget about the concepts for a minute, and focus on the user experience. Imagine a simple hypothetical scenario in which the user wants to push to 2 distinct repositories. He already has cloned the repo from the 1st repository, thus (theoretically) all he needs to do, is to add a new repository for push. He then uses `remote set-url --add --push 2nd-repo` (which I personally thought would suffice). However, if he tries to push a new commit to this remote, it would be pushed _only_ to the 2nd-repo. The primary reason behind push-url was that (1) usually you push to and fetch from the same, so no pushUrl is ever needed, just a single Url will do (this is often true for cvs/svn style shared repository workflow); and (2) sometimes you want to fetch from one place and push to another (this is often true for fetch from upstream, push to my own and ask upstream to pull from it workflow), and in that case you want pushUrl in addition to Url. Most importantly, in this case, you do *NOT* want to push to Url. You only push to pushUrl. Setting *one* pushURL is a way to say That URL I fetch from is *not* the place I want to push (I may not even be able to push there); when I say 'push', push there instead. Your proposed semantics will make it impossible to arrange such an asymmetric setting. Now I think I finally see where that misunderstanding comes from. It is remote -v that is misdesigned. $ git clone ../there here $ cd here $ git remote -v origin /var/tmp/there (fetch) origin /var/tmp/there (push) This is totally bogus. It should report something like this: $ git remote -v origin /var/tmp/there (fetch/push) Then after running git remote set-url --push origin ../another we should see $ git remote -v origin /var/tmp/there (fetch) origin /var/tmp/another (push) which would make it clear that the original fetch/push came from the (1) usuall you push and fetch from the same place so there is only one setting, and the two lines came from the (2) sometimes you need a separate places to fetch from and push to. Yes, that is one big source of misunderstanding. Cleaning up remote -v would help, along with the man page. Also there is a conceptual confusion: pushurl is meant to push to the same repo using a different url, e.g. something authenticated (https/ssh) for push and something faster/easier for fetch. It never was meant to push to several repos. That is what remotes are for, and it would help if we could push to a remote group (which is difficult because of refspecs etc.) easily. That being said, I don't mind changing the behaviour of set-url. At this point, if you say set-url --push origin ../third, then another will disappear and gets replaced by third; if you instead say set-url --add --push origin ../third, then we will see two (push) lines, in addition to one (fetch), making it clear that you are still in (2) above, fetching from and pushing to different places, and having two places to push to. I misread your response From: Jardel Weyrich jweyr...@gmail.com Subject: Re: [BUG] Possible bug in `remote set-url --add --push` Date: Sat, 12 Jan 2013 06:09:35 -0200 Message-ID: can8taovp_hx6bek86ayox-kvqwdmsbyptxtt2nk+fx+ut1t...@mail.gmail.com where you showed that there was only remote.origin.url (and no pushurl) in the first step, and somehow thought you had a remote.origin.pushurl in the first place. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-hg: store converted URL
On 14.01.2013, at 19:14, Junio C Hamano wrote: Max Horn m...@quendi.de writes: From: Felipe Contreras felipe.contre...@gmail.com Mercurial might convert the URL to something more appropriate, like an absolute path. What it is converted *TO* is fairly clear with , like an ..., but from the first reading it was unclear to me what it is converted *FROM* and *WHEN* the conversion happens. Do you mean that the user gives git clone an URL ../hg-repo via the command line (e.g. the argument to git clone is spelled something like hg::../hg-repo), and that ../hg-repo is rewritten to something else (an absolute path, e.g. /srv/project/hg-repo)? Yes, that was meant. Lets store that instead of the original URL, which won't work from a different working directory if it's relative. What is lacking from this description is why it even needs to work from a different working directory. I am guessing that remote-hg later creates a hidden Hg repository or something in a different place and still tries to use the URL to interact with the upstream, and that is what breaks, but with only the above description without looking at your original report, people who will read the git log output and find this change will not be able to tell why this was needed, I am afraid. Of course, the above guess of mine may even be wrong, but then that is yet another reason that the log needs to explain the change better. Fully agreed. How about this commit message: -- 8 -- remote-hg: store converted URL of hg repo in git config When remote-hg is invoked, read the remote repository URL from the git config, give Mercurial a chance to expand it, and if changed, store it back into the git config. This fixes the following problem: Suppose you clone a local hg repository using a relative path, e.g. git clone hg::hgrepo gitrepo This stores hg::hgrepo in gitrepo/.git/config. However, no information about the PWD is stored, making it impossible to correctly interpret the relative path later on. Thus when latter attempting to, say, git pull from inside gitrepo, remote-hg cannot resolve the relative path correctly, and the user sees an unexpected error. With this commit, the URL hg::hgrepo gets expanded (during cloning, but also during any other remote operation) and the resulting absolute URL (e.g. hg::/abspath/hgrepo) is stored in gitrepo/.git/config. Thus the git clone of hgrepo becomes usable. -- 8 -- Suggested-by: Max Horn m...@quendi.de Signed-off-by: Felipe Contreras felipe.contre...@gmail.com Signed-off-by: Max Horn m...@quendi.de --- For a discussion of the problem, see also http://article.gmane.org/gmane.comp.version-control.git/210250 I do not see any discussion; only your problem report. Aha, an english language issue on my side I guess: For me, a single person can discuss a problem (often, a research paper is said to be discussing a problem). Sorry for that. Anyway, the reason I gave that link was because it attempts explains the problem and one solution (which this patch ended up implementing), but also express that I feel a bit uncomfortable with this. Which I still do. Relying on the remote helper to invoke git config feels like a hack and I was wondering whether this is deemed an acceptable solution -- or whether one should instead extend the remote-helper protocol, allowing the remote helper to signal a rewritten remote URL (perhaps only directly after a clone?). As it is, the remote helper seems (?) to have no way to distinguish whether it is being called during a clone or a pull; hence it has to expand and rewrite the URL every time it is called, just in case. Anyway, as long as this particular command works somehow, I am fine: git clone hg::../relative/path/to/hg-repo git-repo Was this work done outside the list? I just want to make sure this patch is not something Felipe did not want to sign off for whatever reason but you are passing it to the list as a patch signed off by him. The work was done by Felipe's and published in his github repository: https://github.com/felipec/git/commit/605bad5b52d2fcf3d8f5fd782a87d7c97d1b040a See also the discussion (yeah, this time a real one ;-) leading to this: https://github.com/felipec/git/issues/2 I took his sign-off from there and interpreted it as saying that Felipe was OK with this being pushed to git.git. But perhaps this is not what I should have done? In that case I am very sorry :-(. It's just that I feel this patch is quite useful and important for daily use (which is why I suggested it in the first place ;-), so I was/am quite eager to see it in. Cheers, Max PS: recently, yet another tool has (re)emerged for using hg repos from inside git: https://github.com/buchuki/gitifyhg This is partially based on Felipe's work, but has several bug fixes atop that. It is also seems to be a priority for its author, so it os more actively developed... anyway, that's now, what, solution #5 or #6? I
[PATCH] test-lib.sh: unfilter GIT_PERF_*
These variables are user parameters to control how to run the perf tests. Allow users to do so. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- t/test-lib.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/t/test-lib.sh b/t/test-lib.sh index f50f834..b8d35d1 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -86,6 +86,9 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $($PERL_PATH -e ' PROVE VALGRIND PERF_AGGREGATING_LATER + PERF_LARGE_REPO + PERF_REPEAT_COUNT + PERF_REPO )); my @vars = grep(/^GIT_/ !/^GIT_($ok)/o, @env); print join(\n, @vars); -- 1.8.0.rc2.23.g1fb49df -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] remote-hg: fix handling of file perms when pushing
Previously, when changing and committing an executable file, the file
would loose its executable bit on the hg side. Likewise, symlinks ended
up as normal files. This was not immediately apparent on the git side
unless one did a fresh clone.
---
contrib/remote-helpers/git-remote-hg | 2 +-
contrib/remote-helpers/test-hg-hg-git.sh | 68
2 files changed, 69 insertions(+), 1 deletion(-)
diff --git a/contrib/remote-helpers/git-remote-hg
b/contrib/remote-helpers/git-remote-hg
index 7c74d8b..328c2dc 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -53,7 +53,7 @@ def gittz(tz):
return '%+03d%02d' % (-tz / 3600, -tz % 3600 / 60)
def hgmode(mode):
-m = { '0100755': 'x', '012': 'l' }
+m = { '100755': 'x', '12': 'l' }
return m.get(mode, '')
def get_config(config):
diff --git a/contrib/remote-helpers/test-hg-hg-git.sh
b/contrib/remote-helpers/test-hg-hg-git.sh
index 3e76d9f..7e3967f 100755
--- a/contrib/remote-helpers/test-hg-hg-git.sh
+++ b/contrib/remote-helpers/test-hg-hg-git.sh
@@ -109,6 +109,74 @@ setup () {
setup
+test_expect_success 'executable bit' '
+ mkdir -p tmp cd tmp
+ test_when_finished cd .. rm -rf tmp
+
+ (
+ git init -q gitrepo
+ cd gitrepo
+ echo alpha alpha
+ chmod 0644 alpha
+ git add alpha
+ git commit -m add alpha
+ chmod 0755 alpha
+ git add alpha
+ git commit -m set executable bit
+ chmod 0644 alpha
+ git add alpha
+ git commit -m clear executable bit
+ )
+
+ for x in hg git; do
+ (
+ hg_clone_$x gitrepo hgrepo-$x
+ cd hgrepo-$x
+ hg_log .
+ hg manifest -r 1 -v
+ hg manifest -v
+ ) output-$x
+
+ git_clone_$x hgrepo-$x gitrepo2-$x
+ git_log gitrepo2-$x log-$x
+ done
+ cp -r log-* output-* /tmp/foo/
+
+ test_cmp output-hg output-git
+ test_cmp log-hg log-git
+'
+
+test_expect_success 'symlink' '
+ mkdir -p tmp cd tmp
+ test_when_finished cd .. rm -rf tmp
+
+ (
+ git init -q gitrepo
+ cd gitrepo
+ echo alpha alpha
+ git add alpha
+ git commit -m add alpha
+ ln -s alpha beta
+ git add beta
+ git commit -m add beta
+ )
+
+ for x in hg git; do
+ (
+ hg_clone_$x gitrepo hgrepo-$x
+ cd hgrepo-$x
+ hg_log .
+ hg manifest -v
+ ) output-$x
+
+ git_clone_$x hgrepo-$x gitrepo2-$x
+ git_log gitrepo2-$x log-$x
+ done
+
+ test_cmp output-hg output-git
+ test_cmp log-hg log-git
+'
+
test_expect_success 'merge conflict 1' '
mkdir -p tmp cd tmp
test_when_finished cd .. rm -rf tmp
--
1.8.1.448.g79c577a.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-hg: fix handling of file perms when pushing
On 15.01.2013, at 14:02, Max Horn wrote: Previously, when changing and committing an executable file, the file would loose its executable bit on the hg side. Likewise, symlinks ended up as normal files. This was not immediately apparent on the git side unless one did a fresh clone. Sorry, forgot to sign off, please add: Signed-off-by: Max Horn m...@quendi.de Max -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] attr: fix off-by-one directory component length calculation
94bc671 (Add directory pattern matching to attributes - 2012-12-08)
uses find_basename() to calculate the length of directory part in
prepare_attr_stack. This function expects the directory without the
trailing slash (as origin field in match_attr struct is without the
trailing slash). find_basename() includes the trailing slash and
confuses push/pop algorithm.
Consider path = abc/def and the push down code:
while (1) {
len = strlen(attr_stack-origin);
if (dirlen = len)
break;
cp = memchr(path + len + 1, '/', dirlen - len - 1);
if (!cp)
cp = path + dirlen;
dirlen is 4, not 3, without this patch. So when attr_stack-origin is
abc, it'll miss the exit condition because 4 = 3 is wrong. It'll
then try to push abc/ down the attr stack (because cp would be
NULL). So we have both abc and abc/ in the stack.
Next time when abc/ghi is checked, abc/ is popped out because of
the off-by-one dirlen, only to be pushed back in again by the above
code. This repeats for all files in the same directory. Which means
at least one failed open syscall per file, or more if .gitattributes
exists.
This is the perf result with 10 runs on git.git:
Test 94bc671^ 94bc671
HEAD
--
7810.1: grep worktree, cheap regex 0.02(0.01+0.04) 0.05(0.03+0.05)
+150.0% 0.02(0.01+0.04) +0.0%
7810.2: grep worktree, expensive regex 0.25(0.94+0.01) 0.26(0.94+0.02)
+4.0% 0.25(0.93+0.02) +0.0%
7810.3: grep --cached, cheap regex 0.11(0.10+0.00) 0.12(0.10+0.02)
+9.1% 0.10(0.10+0.00) -9.1%
7810.4: grep --cached, expensive regex 0.61(0.60+0.01) 0.62(0.61+0.01)
+1.6% 0.61(0.60+0.00) +0.0%
Reported-by: Ross Lagerwall rosslagerw...@gmail.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
This may be an indication that our perf framework is never actively used :-(
attr.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/attr.c b/attr.c
index 466c93f..bb9a470 100644
--- a/attr.c
+++ b/attr.c
@@ -584,6 +584,13 @@ static void prepare_attr_stack(const char *path)
dirlen = find_basename(path) - path;
/*
+* find_basename() includes the trailing slash, but we do
+* _not_ want it.
+*/
+ if (dirlen)
+ dirlen--;
+
+ /*
* At the bottom of the attribute stack is the built-in
* set of attribute definitions, followed by the contents
* of $(prefix)/etc/gitattributes and a file specified by
--
1.8.0.rc2.23.g1fb49df
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test-lib.sh: unfilter GIT_PERF_*
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
These variables are user parameters to control how to run the perf
tests. Allow users to do so.
[...]
@@ -86,6 +86,9 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $($PERL_PATH -e '
PROVE
VALGRIND
PERF_AGGREGATING_LATER
+ PERF_LARGE_REPO
+ PERF_REPEAT_COUNT
+ PERF_REPO
));
Wouldn't it be more futureproof to put simply PERF as an entry, and rely
on the leading-match logic
my @vars = grep(/^GIT_/ !/^GIT_($ok)/o, @env);
to allow all GIT_PERF variables?
Other than that, Ack. I never noticed because I set mine through
config.mak, which goes to GIT-BUILD-OPTIONS. Those options are not
exported, which means perl does not pick them up. (That just took me
far too long to realize.)
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test-lib.sh: unfilter GIT_PERF_*
On Tue, Jan 15, 2013 at 8:43 PM, Thomas Rast tr...@student.ethz.ch wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: These variables are user parameters to control how to run the perf tests. Allow users to do so. [...] @@ -86,6 +86,9 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $($PERL_PATH -e ' PROVE VALGRIND PERF_AGGREGATING_LATER + PERF_LARGE_REPO + PERF_REPEAT_COUNT + PERF_REPO )); Wouldn't it be more futureproof to put simply PERF as an entry, and rely on the leading-match logic my @vars = grep(/^GIT_/ !/^GIT_($ok)/o, @env); to allow all GIT_PERF variables? Yeah. Other than that, Ack. I never noticed because I set mine through config.mak, which goes to GIT-BUILD-OPTIONS. Those options are not exported, which means perl does not pick them up. (That just took me far too long to realize.) By the way is there an option to skip the first few runs (too lazy to check out the source code, apparently)? I tried linux-2.6 as the large repo and I think the first (cold cache) run ruins the numbers. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] test-lib.sh: unfilter GIT_PERF_*
These variables are user parameters to control how to run the perf tests. Allow users to do so. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- t/test-lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index f50f834..e1c8c85 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -85,7 +85,7 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $($PERL_PATH -e ' .*_TEST PROVE VALGRIND - PERF_AGGREGATING_LATER + PERF )); my @vars = grep(/^GIT_/ !/^GIT_($ok)/o, @env); print join(\n, @vars); -- 1.8.0.rc2.23.g1fb49df -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/14] Remove unused code from imap-send.c
On Tue, Jan 15, 2013 at 09:06:18AM +0100, Michael Haggerty wrote: This is a re-roll, incorporating the feedback of Jonathan Nieder (thanks!). Thanks, I don't see anything wrong with this from a cursory reading. * Added some comments to lf_to_crlf(), simplified the code a bit further, and expanded the commit message. I found this version pretty easy to read (the comments helped a lot). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] ignore memcmp() overreading in bsearch() callback
On Mon, Jan 14, 2013 at 03:36:21PM -0800, Junio C Hamano wrote:
It appears that memcmp() uses the usual one word at a time
comparison and triggers valgrind in a callback of bsearch() used in
the refname search. I can easily trigger problems in any script
with test_commit (e.g. sh t0101-at-syntax.sh --valgrind -i -v)
without this suppression.
Out of curiosity, what platform do you see this on? I can't reproduce on
glibc.
diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp
index 0a6724f..032332f 100644
--- a/t/valgrind/default.supp
+++ b/t/valgrind/default.supp
@@ -49,3 +49,11 @@
Memcheck:Addr4
fun:copy_ref
}
+
+{
+ ignore-memcmp-reading-too-much-in-bsearch-callback
+ Memcheck:Addr4
+ fun:ref_entry_cmp_sslice
+ fun:bsearch
+ fun:search_ref_dir
+}
Given that it is valgrind-clean on my platform, and reading the code I
don't see any problems, I think it probably is a false positive, and
this suppression makes sense.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] ignore memcmp() overreading in bsearch() callback
Am 15.01.2013 00:36, schrieb Junio C Hamano:
It appears that memcmp() uses the usual one word at a time
comparison and triggers valgrind in a callback of bsearch() used in
the refname search. I can easily trigger problems in any script
with test_commit (e.g. sh t0101-at-syntax.sh --valgrind -i -v)
without this suppression.
I can't reproduce it on Debian, but can we perhaps do without the
suppression with a patch like this instead? I would expect it to
be slightly faster because we lose the strlen() call, but didn't
check. It's also simpler, perhaps with the exception of the last
line. Does it help in your case?
René
---
refs.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index 541fec2..1a0e049 100644
--- a/refs.c
+++ b/refs.c
@@ -335,12 +335,10 @@ static int ref_entry_cmp_sslice(const void *key_, const
void *ent_)
{
struct string_slice *key = (struct string_slice *)key_;
struct ref_entry *ent = *(struct ref_entry **)ent_;
- int entlen = strlen(ent-name);
- int cmplen = key-len entlen ? key-len : entlen;
- int cmp = memcmp(key-str, ent-name, cmplen);
+ int cmp = strncmp(key-str, ent-name, key-len);
if (cmp)
return cmp;
- return key-len - entlen;
+ return '\0' - ent-name[key-len];
}
/*
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] am: invoke perl's strftime in C locale
On Tue, Jan 15, 2013 at 12:59:33AM +0400, Dmitry V. Levin wrote:
diff --git a/git-am.sh b/git-am.sh
index c682d34..64b88e4 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -334,7 +334,7 @@ split_patches () {
# Since we cannot guarantee that the commit message is
in
# git-friendly format, we put no Subject: line and just
consume
# all of the message as the body
- perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
+ LC_ALL=C perl -M'POSIX qw(strftime)' -ne 'BEGIN {
$subject = 0 }
if ($subject) { print ; }
elsif (/^\# User /) { s/\# User/From:/ ; print
; }
elsif (/^\# Date /) {
This puts all of perl into the C locale, which would mean error messages
from perl would be in English rather than the user's language. It
probably isn't a big deal, because that snippet of perl is short and not
likely to produce problems, but I wonder how hard it would be to set the
locale just for the strftime call.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] config: add helper function for parsing key names
On Mon, Jan 14, 2013 at 10:08:47AM -0800, Junio C Hamano wrote: +extern int match_config_key(const char *var, +const char *section, +const char **subsection, int *subsection_len, +const char **key); + I agree with Jonathan about the naming s/match/parse/. I see this is marked for re-roll in WC. I'm happy to re-roll it with the suggestions from Jonathan, but before I do, did you have any comment on the struct config_key alternative I sent as a follow-up? You said: After looking at the callers in your later patches, I think the counted interface to subsection is probably fine. The caller can check !subsection to see if it is a two- or three- level name, and if (parse_config_key(var, submodule, name, namelen, key) 0 || !name) return 0; is very easy to follow (that is the result of your 5th step). but I wasn't sure if that was it is not worth the trouble of the other one or I did not yet read the other one. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] attr: fix off-by-one directory component length calculation
Good spotting and a nicely explained patch. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] am: invoke perl's strftime in C locale
This puts all of perl into the C locale, which would mean error messages from perl would be in English rather than the user's language. It probably isn't a big deal, because that snippet of perl is short and not likely to produce problems, but I wonder how hard it would be to set the locale just for the strftime call. Maybe just setting LC_TIME to C would do ... From locale(7) man page: LC_TIME changes the behavior of the strftime(3) function to display the current time in a locally acceptable form; for example, most of Europe uses a 24-hour clock versus the 12-hour clock used in the United States. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-hg: store converted URL
Junio C Hamano gits...@pobox.com writes: Max Horn m...@quendi.de writes: ... See also the discussion (yeah, this time a real one ;-) leading to this: https://github.com/felipec/git/issues/2 ... If I understand correctly, the $backend::$opaqueToken is a contract between the remote-helper and the remote-$backend that says When user wants to interact with the same (foreign) repository, we agreed to let her use 'origin' nickname. The remote-helper looks up this opaque token that corresponds to 'origin' and gives it to the remote-$backend, and whatever is in the opaque token should be sufficient for the remote-$backend to figure out how to proceed from there. But in this hg::../over/there case, it seems that string is not sufficient for remote-hg to do so and the contract is broken. When git clone $backend::$opaqueToken repo is run in /dir/ecto/ry, and then subsequent git fetch origin will be run in (a subdirectory of) /dir/ecto/ry/repo, but anything relative to /dir/ecto/ry will not work once you go inside /dir/ecto/ry/repo. The create a new repository here argument could even be an absolute path to a totally different place, so if the remote-$backend wants to use $opaqueToken as anything relative to the $(cwd) when git clone was invoked, that original location needs to be available somehow. Would a new helper protocol message be necessary, so that the backend can rewrite the $opaqueToken at clone time and tell the helper what to store as URL instead of the original? I do not think that is much different from remote-$backend updating the value of the remote.origin.URL using git config. An alternative approach may be for somebody (either the git clone or the remote-$backend) to store a base directory when git clone was invoked in remote.origin.dirAtCloneTime variable, so that the next time remote-$backend runs, it can read that directory and interpret the $opaqueToken as a relative path to that directory if it wants to. That way, nobody needs to rewrite $opaqueToken. How do other remote helpers solve this, I have to wonder, though. By not allowing relative paths to a directory? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] ignore memcmp() overreading in bsearch() callback
Jeff King p...@peff.net writes:
On Mon, Jan 14, 2013 at 03:36:21PM -0800, Junio C Hamano wrote:
It appears that memcmp() uses the usual one word at a time
comparison and triggers valgrind in a callback of bsearch() used in
the refname search. I can easily trigger problems in any script
with test_commit (e.g. sh t0101-at-syntax.sh --valgrind -i -v)
without this suppression.
Out of curiosity, what platform do you see this on? I can't reproduce on
glibc.
Debian GNU/Linux 6.0.6 (squeeze), on Linux 2.6.32-5-amd64.
libc-bin 2.11.3-4
valgrind-3.6.0.SVN-Debian
gcc 4:4.4.5-1
diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp
index 0a6724f..032332f 100644
--- a/t/valgrind/default.supp
+++ b/t/valgrind/default.supp
@@ -49,3 +49,11 @@
Memcheck:Addr4
fun:copy_ref
}
+
+{
+ignore-memcmp-reading-too-much-in-bsearch-callback
+Memcheck:Addr4
+fun:ref_entry_cmp_sslice
+fun:bsearch
+fun:search_ref_dir
+}
Given that it is valgrind-clean on my platform, and reading the code I
don't see any problems, I think it probably is a false positive, and
this suppression makes sense.
Thanks for a sanity check.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] config: Introduce diff.algorithm variable
On Mon, Jan 14, 2013 at 01:05:27PM -0800, Junio C Hamano wrote:
Michal Privoznik mpriv...@redhat.com writes:
+static long parse_algorithm_value(const char *value)
+{
+ if (!value || !strcasecmp(value, myers))
+ return 0;
[diff]
algorithm
should probably error out.
Definitely.
Also it is rather unusual to parse the keyword values case insensitively.
Is it? git grep strcasecmp shows that we already do so in many cases
(e.g., any bool option, core.autocrlf, receive.deny*, etc). Is there a
reason to reject Myers here?
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] config: add helper function for parsing key names
Jeff King p...@peff.net writes: ... did you have any comment on the struct config_key alternative I sent as a follow-up? I did read it but I cannot say I did so very carefully. My gut reaction was that the take the variable name and section name, return the subsection name pointer and length, if there is any, and the key made it readable enough. The proposed interface to make and lend a copy to the caller does make it more readble, but I do not know if that is worth doing. Neutral-to-slightly-in-favor, I would say. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] config: Introduce diff.algorithm variable
Jeff King p...@peff.net writes: Also it is rather unusual to parse the keyword values case insensitively. Is it? git grep strcasecmp shows that we already do so in many cases (e.g., any bool option, core.autocrlf, receive.deny*, etc). Yeah, I did the same grep after I wrote the message. Thanks for saving me the trouble of reporting the findings ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] ignore memcmp() overreading in bsearch() callback
On Tue, Jan 15, 2013 at 08:55:32AM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: On Mon, Jan 14, 2013 at 03:36:21PM -0800, Junio C Hamano wrote: It appears that memcmp() uses the usual one word at a time comparison and triggers valgrind in a callback of bsearch() used in the refname search. I can easily trigger problems in any script with test_commit (e.g. sh t0101-at-syntax.sh --valgrind -i -v) without this suppression. Out of curiosity, what platform do you see this on? I can't reproduce on glibc. Debian GNU/Linux 6.0.6 (squeeze), on Linux 2.6.32-5-amd64. libc-bin 2.11.3-4 valgrind-3.6.0.SVN-Debian gcc 4:4.4.5-1 Interesting. I can reproduce easily on my squeeze machine, but not my wheezy. So presumably it is a false positive fixed either in libc (I have 2.13-38 on the good box) or valgrind (1:3.8.1-1). However, the error that valgrind reports is on the call to strlen(ent-name), not memcmp (but it has suffered from the same SSE issues in the past). So I feel pretty confident that it really is a false positive; you may want to double-check the offending call for the commit message (though I would not be surprised if it is triggerable from both). I think it also means that René's suggestion to use strncmp cannot be relied on to silence the warning (though I am not opposed to doing it anyway if we think it is more clear). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] am: invoke perl's strftime in C locale
On Tue, Jan 15, 2013 at 08:50:59AM -0800, Jeff King wrote:
On Tue, Jan 15, 2013 at 05:42:12PM +0100, Antoine Pelisse wrote:
This puts all of perl into the C locale, which would mean error messages
from perl would be in English rather than the user's language. It
probably isn't a big deal, because that snippet of perl is short and not
likely to produce problems, but I wonder how hard it would be to set the
locale just for the strftime call.
Maybe just setting LC_TIME to C would do ...
Yeah, that is a nice simple solution. Dmitry, does just setting LC_TIME
fix the problem for you?
Just setting LC_TIME environment variable instead of LC_ALL would end up
with unreliable solution because LC_ALL has the highest priority.
If keeping error messages from perl has the utmost importance, it could be
achieved by
- perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
+ perl -M'POSIX qw(strftime :locale_h)' -ne '
+ BEGIN { setlocale(LC_TIME, C); $subject = 0 }
but the little perl helper script we are talking about hardly worths so
much efforts.
--
ldv
pgp6E64QU8Sck.pgp
Description: PGP signature
Re: [PATCH] remote-hg: store converted URL
On 15.01.2013, at 17:05, Junio C Hamano wrote: Max Horn m...@quendi.de writes: On 14.01.2013, at 19:14, Junio C Hamano wrote: What is lacking from this description is why it even needs to work from a different working directory In your rewrite below, this is still lacking, I think. Hm, I thought I made it clear: It has to change because relative paths only make sense when you know the reference point they are relative with. Typically. This is the pwd. But when I git clone repo newrepo cd newrepo I just changed the PWD. The clone command was given the relative path repo. If git were to use that, it would suddenly refer to a directory inside newrepo, not next to it. Bang. Hence, git expands the relative path to an absolute one in the above example. But git cannot do that for URLs in the form HELPER::PATH, because such a string is necessarily opaque to git. snip Thus when latter attempting to, say, git pull from inside gitrepo, remote-hg cannot resolve the relative path correctly, and the user sees an unexpected error. ... cannot resolve the relative path correctly above sound like a bug in remote-hg. Something like: Cloning a local hg repository using a relative path, e.g. git clone hg::hgrepo gitrepo stores hg::hgrepo in gitrepo/.git/config as its URL. When remote-hg is invoked by git fetch, it chdirs to X (which is different from the gitrepo directory) and uses the URL (which is not correct, as it is a relative path but the cwd is different when it is used) to interact with the original hgrepo, which will fail. is needed, but you didn't explain what that X is. Perhaps it is a temporary directory. Perhaps it is a hidden Hg repository somewhere in gitrepo/.git directory. Or something else. None of the above. Nor does the remote helper chdir anywhere. It is the user who has done the chdir: Away from the location he invoked git clone at, and into the new repository directory that previously did not even exist. Cheers, Max-- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-hg: store converted URL
On 15.01.2013, at 17:51, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: Max Horn m...@quendi.de writes: ... See also the discussion (yeah, this time a real one ;-) leading to this: https://github.com/felipec/git/issues/2 ... If I understand correctly, the $backend::$opaqueToken is a contract between the remote-helper and the remote-$backend Just to clarify: What is the remote-helper ? So far, I thought of this being a contract between git (or some part of git), and remote-$backend (i.e. remote-hg in this case). that says When user wants to interact with the same (foreign) repository, we agreed to let her use 'origin' nickname. The remote-helper looks up this opaque token that corresponds to 'origin' and gives it to the remote-$backend, and whatever is in the opaque token should be sufficient for the remote-$backend to figure out how to proceed from there. Supposing that I interpret remote-helper correctly, that sounds about right to me. But in this hg::../over/there case, it seems that string is not sufficient for remote-hg to do so and the contract is broken. One could put it that way. When git clone $backend::$opaqueToken repo is run in /dir/ecto/ry, and then subsequent git fetch origin will be run in (a subdirectory of) /dir/ecto/ry/repo, but anything relative to /dir/ecto/ry will not work once you go inside /dir/ecto/ry/repo. The create a new repository here argument could even be an absolute path to a totally different place, so if the remote-$backend wants to use $opaqueToken as anything relative to the $(cwd) when git clone was invoked, that original location needs to be available somehow. That would be one option. Another is to do what the proposed patch does, and what git itself does: Change the relative path into an absolute one. This requires remote-$backend to be able to modify the opaque token supplied by the user. Yet another would be to be more strict in remote-$backend as to which opaque tokens to accept: When it contains a relative path, simply always refuse to work, even if the PWD happens to be set the right way. Of course this would be quite undesirable from a user's perspective. Would a new helper protocol message be necessary, so that the backend can rewrite the $opaqueToken at clone time and tell the helper what to store as URL instead of the original? I do not think that is much different from remote-$backend updating the value of the remote.origin.URL using git config. An alternative approach may be for somebody (either the git clone or the remote-$backend) to store a base directory when git clone was invoked in remote.origin.dirAtCloneTime variable, so that the next time remote-$backend runs, it can read that directory and interpret the $opaqueToken as a relative path to that directory if it wants to. That way, nobody needs to rewrite $opaqueToken. As I said above, this would be an option. However, I would prefer rewriting the $opaqueToken, as that would be closer to what git does for native tokens passed to git clone Specifically, I am talking about get_repo_path() in builtin/clone.c which is called by cmd_clone. What this does is in my eyes essentially the equivalent of what the patch discussed here is doing. Anyway, at the end of the day, I mainly care about relative paths working, somehow :-). But I think it would be important to make the issue easy to resolve for all remote-$backend authors, as many of them are affected (see below). How do other remote helpers solve this, I have to wonder, though. By not allowing relative paths to a directory? So far, all I look at do not deal with this at all. Any attempts to deal with it should be pretty easy to recognize: The remote-$backend would have to store something into the git config, or else, verify the opaque token and refuse to work with it under certain conditions (e.g. when it contains a relative path). But they don't. E.g. git-remote-testgit has the exact same problem. Doing git init repo cd repo echo a a git add a git ci -m a a git clone testgit::repo clone cd clone results in a .git/config file containing [remote origin] url = testgit::repo fetch = +refs/heads/*:refs/remotes/origin/* Trying to do a git push from within the clone then gives this: fatal: 'repo/.git' does not appear to be a git repository fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. Traceback (most recent call last): File /usr/local/libexec/git-core/git-remote-testgit, line 272, in module sys.exit(main(sys.argv)) File /usr/local/libexec/git-core/git-remote-testgit, line 261, in main repo = get_repo(alias, url) File /usr/local/libexec/git-core/git-remote-testgit, line 39, in get_repo repo.get_revs() File /usr/local/lib/python2.7/site-packages/git_remote_helpers/git/repo.py, line 59, in get_revs check_call(args,
Re: [PATCH v2 06/19] reset.c: remove unnecessary variable 'i'
I suppose this was meant for everyone. Adding back the others. On Tue, Jan 15, 2013 at 10:27 AM, Holding, Lawrence (NZ) lawrence.hold...@cubic.com wrote: Maybe use *argv instead of argv[0]? Sure. Everywhere? Also in the lines added in patch 17/19 that refer to both argv[0] and argv[1], such as argv[1] !get_sha1_treeish(argv[0], unused)? Or is this just a sign that I'm making the code _more_ confusing to those who are more familiar with C? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Allow custom comment char
From: Junio C Hamano gits...@pobox.com
Some users do want to write a line that begin with a pound sign, #,
in their commit log message. Many tracking system recognise
a token of #bugid form, for example.
The support we offer these use cases is not very friendly to the end
users. They have a choice between
- Don't do it. Avoid such a line by rewrapping or indenting; and
- Use --cleanup=whitespace but remove all the hint lines we add.
Give them a way to set a custom comment char, e.g.
$ git -c core.commentchar=% commit
so that they do not have to do either of the two workarounds.
Signed-off-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Ralf Thielow ralf.thie...@gmail.com
---
Documentation/config.txt | 6
Documentation/technical/api-strbuf.txt | 10 +++
builtin/branch.c | 10 +++
builtin/commit.c | 12
builtin/fmt-merge-msg.c| 2 +-
builtin/merge.c| 5 ++--
builtin/notes.c| 34 ++-
builtin/stripspace.c | 2 +-
builtin/tag.c | 35
cache.h| 6
config.c | 8 ++
environment.c | 6
git-submodule.sh | 14 +++---
strbuf.c | 38 ++
strbuf.h | 4 +++
t/t7502-commit.sh | 7 +
t/t7508-status.sh | 50 ++
wt-status.c| 10 ---
18 files changed, 198 insertions(+), 61 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index d5809e0..e99b9f2 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -528,6 +528,12 @@ core.editor::
variable when it is set, and the environment variable
`GIT_EDITOR` is not set. See linkgit:git-var[1].
+core.commentchar::
+ Commands such as `commit` and `tag` that lets you edit
+ messages consider a line that begins with this character
+ commented, and removes them after the editor returns
+ (default '#').
+
sequence.editor::
Text editor used by `git rebase -i` for editing the rebase insn file.
The value is meant to be interpreted by the shell when it is used.
diff --git a/Documentation/technical/api-strbuf.txt
b/Documentation/technical/api-strbuf.txt
index 84686b5..6767102 100644
--- a/Documentation/technical/api-strbuf.txt
+++ b/Documentation/technical/api-strbuf.txt
@@ -156,6 +156,11 @@ then they will free() it.
Remove the bytes between `pos..pos+len` and replace it with the given
data.
+`strbuf_commented_addstr`::
+
+ Add a NUL-terminated string prepended by a comment character and a blank
+ to the buffer.
+
`strbuf_add`::
Add data of given length to the buffer.
@@ -229,6 +234,11 @@ which can be used by the programmer of the callback as she
sees fit.
Add a formatted string to the buffer.
+`strbuf_commented_addf`::
+
+ Add a formatted string prepended by a comment character and a
+ blank to the buffer.
+
`strbuf_fread`::
Read a given size of data from a FILE* pointer to the buffer.
diff --git a/builtin/branch.c b/builtin/branch.c
index 873f624..3548271 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -706,11 +706,11 @@ static int edit_branch_description(const char
*branch_name)
read_branch_desc(buf, branch_name);
if (!buf.len || buf.buf[buf.len-1] != '\n')
strbuf_addch(buf, '\n');
- strbuf_addf(buf,
- # Please edit the description for the branch\n
- # %s\n
- # Lines starting with '#' will be stripped.\n,
- branch_name);
+ strbuf_commented_addf(buf,
+ Please edit the description for the branch\n
+ %s\n
+ Lines starting with '%c' will be stripped.\n,
+ branch_name, comment_line_char);
fp = fopen(git_path(edit_description), w);
if ((fwrite(buf.buf, 1, buf.len, fp) buf.len) || fclose(fp)) {
strbuf_release(buf);
diff --git a/builtin/commit.c b/builtin/commit.c
index d6dd3df..f95f64a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -733,15 +733,15 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
if (cleanup_mode == CLEANUP_ALL)
status_printf(s, GIT_COLOR_NORMAL,
_(Please enter the commit message for your
changes.
-Lines starting\nwith '#' will be ignored, and
an empty
-message aborts the commit.\n));
+ Lines
Re: Version 1.8.1 does not compile on Cygwin 1.7.14
Mark Levedahl wrote: On 01/11/2013 03:17 PM, Alex Riesen wrote: On Fri, Jan 11, 2013 at 9:08 PM, Alex Riesen raa.l...@gmail.com wrote: This short discussion on GitHub (file git-compat-util.h) might be relevant: https://github.com/msysgit/git/commit/435bdf8c7ffa493f8f6f2e8f329f8cc22db16ce6#commitcomment-2407194 The change suggested there (to remove an inclusion of windows.h in git-compat-util.h) might simplify the solution a little. Might even remove the need for auto-configuration in Makefile (worked for me). Just to be clear, the change is this: diff --git a/git-compat-util.h b/git-compat-util.h index 4a1979f..780a919 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -85,12 +85,6 @@ #define _NETBSD_SOURCE 1 #define _SGI_SOURCE 1 -#ifdef WIN32 /* Both MinGW and MSVC */ -#define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */ -#include winsock2.h -#include windows.h -#endif - #include unistd.h #include stdio.h #include sys/stat.h That change alone seems fine, no apparent change building on current cygwin. However, with that change the build still fails if CYGWIN_V15_WIN32API is defined, so unless someone can show the compilation works on cygwin1.5 WITHOUT defining CYGWIN_V15_WIN32API this change does not help. I do not have an older installation available, so cannot test. Frankly, assuming you can compile with that macro defined, I would suggest leaving well enough alone - an unsupported configuration is unsupported :^) I haven't been following this thread too closely, so I may have misunderstood what you would like to test but, since I use cygwin 1.5, I tried the patch given below. I only had time to compile test this patch (ie I have *not* run any of the tests - it takes over 3 hours for me), but it seems to work to that extent. (I also tried a few simple commands: status, diff, branch; seems to work OK.) If you would like me to test something else, just let me know. HTH ATB, Ramsay Jones -- 8 -- diff --git a/Makefile b/Makefile index 1b30d7b..1c84f68 100644 --- a/Makefile +++ b/Makefile @@ -281,10 +281,6 @@ all:: # # Define NO_REGEX if you have no or inferior regex support in your C library. # -# Define CYGWIN_V15_WIN32API if you are using Cygwin v1.7.x but are not -# using the current w32api packages. The recommended approach, however, -# is to update your installation if compilation errors occur. -# # Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the # user. # @@ -1402,9 +1398,6 @@ ifdef NO_REGEX COMPAT_CFLAGS += -Icompat/regex COMPAT_OBJS += compat/regex/regex.o endif -ifdef CYGWIN_V15_WIN32API - COMPAT_CFLAGS += -DCYGWIN_V15_WIN32API -endif ifdef USE_NED_ALLOCATOR COMPAT_CFLAGS += -Icompat/nedmalloc diff --git a/compat/cygwin.c b/compat/cygwin.c index 5428858..0a9aa6d 100644 --- a/compat/cygwin.c +++ b/compat/cygwin.c @@ -1,13 +1,8 @@ #define WIN32_LEAN_AND_MEAN -#ifdef CYGWIN_V15_WIN32API -#include ../git-compat-util.h -#include win32.h -#else #include sys/stat.h #include sys/errno.h #include win32.h #include ../git-compat-util.h -#endif #include ../cache.h /* to read configuration */ static inline void filetime_to_timespec(const FILETIME *ft, struct timespec *ts) diff --git a/config.mak.uname b/config.mak.uname index bea34f0..5e493c9 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -158,7 +158,6 @@ ifeq ($(uname_O),Cygwin) NO_SYMLINK_HEAD = YesPlease NO_IPV6 = YesPlease OLD_ICONV = UnfortunatelyYes - CYGWIN_V15_WIN32API = YesPlease endif NO_THREAD_SAFE_PREAD = YesPlease NEEDS_LIBICONV = YesPlease diff --git a/git-compat-util.h b/git-compat-util.h index e5a4b74..3186e55 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -85,12 +85,6 @@ #define _NETBSD_SOURCE 1 #define _SGI_SOURCE 1 -#ifdef WIN32 /* Both MinGW and MSVC */ -#define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */ -#include winsock2.h -#include windows.h -#endif - #include unistd.h #include stdio.h #include sys/stat.h -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 07/14] imap-send.c: inline imap_parse_list() in imap_list()
On Tue, Jan 15, 2013 at 09:06:25AM +0100, Michael Haggerty wrote: -static struct imap_list *parse_imap_list(struct imap *imap, char **sp) +static struct imap_list *parse_list(char **sp) The commit subject refers to imap_parse_list and imap_list whereas the code refers to parse_imap_list and parse_list. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] am: invoke perl's strftime in C locale
This fixes hg patch format support for locales other than C and en_*.
Before the change, git-am was making Date: line from hg changeset
metadata according to the current locale, and this line was rejected
later with invalid date format diagnostics because localized date
strings are not supported.
Reported-by: Gleb Fotengauer-Malinovskiy gle...@altlinux.org
Signed-off-by: Dmitry V. Levin l...@altlinux.org
---
v3: alternative implementation using setlocale(LC_TIME, C)
git-am.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/git-am.sh b/git-am.sh
index c682d34..8677d8c 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -334,7 +334,8 @@ split_patches () {
# Since we cannot guarantee that the commit message is
in
# git-friendly format, we put no Subject: line and just
consume
# all of the message as the body
- perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
+ perl -M'POSIX qw(strftime :locale_h)' -ne '
+ BEGIN { setlocale(LC_TIME, C); $subject = 0 }
if ($subject) { print ; }
elsif (/^\# User /) { s/\# User/From:/ ; print
; }
elsif (/^\# Date /) {
--
ldv
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Allow custom comment char
Ralf Thielow ralf.thie...@gmail.com writes:
From: Junio C Hamano gits...@pobox.com
Some users do want to write a line that begin with a pound sign, #,
in their commit log message. Many tracking system recognise
a token of #bugid form, for example.
The support we offer these use cases is not very friendly to the end
users. They have a choice between
- Don't do it. Avoid such a line by rewrapping or indenting; and
- Use --cleanup=whitespace but remove all the hint lines we add.
Give them a way to set a custom comment char, e.g.
$ git -c core.commentchar=% commit
so that they do not have to do either of the two workarounds.
Signed-off-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Ralf Thielow ralf.thie...@gmail.com
---
It would have helped if you said you finished the NEEDSWORK: in
builtin/branch.c in the earlier draft with strbuf_commented_*
functions ;-)
Looks like a good progress overall, except for nits here and there.
diff --git a/builtin/notes.c b/builtin/notes.c
index 453457a..5e84e35 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -92,10 +92,7 @@ static const char * const git_notes_get_ref_usage[] = {
};
static const char note_template[] =
- \n
- #\n
- # Write/edit the notes for the following object:\n
- #\n;
+ Write/edit the notes for the following object:;
I think this (and its use site that manually adds \n#\n) is a
symptom of strbuf_commented_add*() function not designed right.
When it iterates over lines and adds each of them in a commented out
form, it could check if the line is an empty one and refrain from
adding a trailing SP if that is the case. Then this can become
\nWrite/edit the notes...\n\n;
You have to create the \n blank line at the beginning manually,
but that is logically outside the commented out block, so it is not
a problem.
@@ -181,11 +172,16 @@ static void create_note(const unsigned char *object,
struct msg_arg *msg,
write_or_die(fd, msg-buf.buf, msg-buf.len);
else if (prev !append_only)
write_note_data(fd, prev);
- write_or_die(fd, note_template, strlen(note_template));
+
+ strbuf_addf(buf, \n%c\n, comment_line_char);
+ strbuf_commented_addstr(buf, note_template);
+ strbuf_addf(buf, \n%c\n, comment_line_char);
+ write_or_die(fd, buf.buf, buf.len);
diff --git a/builtin/tag.c b/builtin/tag.c
index 9c3e067..e1b72be 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -246,19 +246,13 @@ static int do_sign(struct strbuf *buffer)
}
static const char tag_template[] =
- N_(\n
- #\n
- # Write a tag message\n
- # Lines starting with '#' will be ignored.\n
- #\n);
+ N_(Write a tag message\n
+ Lines starting with '%c' will be ignored.);
...
+ else
+ strbuf_commented_addf(buf,
_(tag_template_nocleanup), comment_line_char);
+ strbuf_addf(buf, \n%c\n, comment_line_char);
+ write_or_die(fd, buf.buf, buf.len);
Same here.
diff --git a/git-submodule.sh b/git-submodule.sh
index 22ec5b6..1b8d95f 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -975,13 +975,19 @@ cmd_summary() {
echo
done |
if test -n $for_status; then
+ comment_char=`git config core.commentchar`
+ if [ ! -n $comment_char ]; then
+ comment_char='#'
+ elif [ ${#comment_char} -gt 1 ]; then
Not portable, I think.
+ echo $comment_char
+ sed -e s|^|$comment_char | -e s|^$comment_char
$|$comment_char|
Can $comment_char be a '|'?
diff --git a/strbuf.c b/strbuf.c
index 9a373be..8af4b4f 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -204,6 +204,44 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
va_end(ap);
}
+void strbuf_commented_addstr(struct strbuf *sb, const char *s)
+{
+ struct strbuf buf = STRBUF_INIT;
+ struct strbuf prefix = STRBUF_INIT;
+
+ strbuf_addf(prefix, %c , comment_line_char);
+ strbuf_addstr(buf, s);
+ strbuf_add_lines(sb, prefix.buf, buf.buf, buf.len);
+
+ // remove additional '\n' added by strbuf_add_lines()
No C++ comments.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] am: invoke perl's strftime in C locale
Dmitry V. Levin l...@altlinux.org writes:
On Tue, Jan 15, 2013 at 08:50:59AM -0800, Jeff King wrote:
On Tue, Jan 15, 2013 at 05:42:12PM +0100, Antoine Pelisse wrote:
This puts all of perl into the C locale, which would mean error messages
from perl would be in English rather than the user's language. It
probably isn't a big deal, because that snippet of perl is short and not
likely to produce problems, but I wonder how hard it would be to set the
locale just for the strftime call.
Maybe just setting LC_TIME to C would do ...
Yeah, that is a nice simple solution. Dmitry, does just setting LC_TIME
fix the problem for you?
Just setting LC_TIME environment variable instead of LC_ALL would end up
with unreliable solution because LC_ALL has the highest priority.
If keeping error messages from perl has the utmost importance, it could be
achieved by
- perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
+ perl -M'POSIX qw(strftime :locale_h)' -ne '
+ BEGIN { setlocale(LC_TIME, C); $subject = 0 }
but the little perl helper script we are talking about hardly worths so
much efforts.
Yeah I agree that this is not worth it, I would think.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] attr: fix off-by-one directory component length calculation
Thank you for the explanation. I did not monitor the system calls when writing that patch. Where is the perf framework? As the mistake is located in the find_basename function, I would propose a fix directly into it so that the output fits what the other functions expect. Something in the line of: diff --git a/attr.c b/attr.c index d6d7190..b6e72f3 100644 --- a/attr.c +++ b/attr.c @@ -572,7 +572,7 @@ static const char *find_basename(const char *path) if (*cp == '/' cp[1]) last_slash = cp; } - return last_slash ? last_slash + 1 : path; + return last_slash ? last_slash : path; } static void prepare_attr_stack(const char *path) @@ -770,6 +770,10 @@ static void collect_all_attrs(const char *path) check_all_attr[i].value = ATTR__UNKNOWN; basename = find_basename(path); + /* the slash is included in the basename + so that it can be matched by a directory pattern */ + if (basename != path) + basename++; pathlen = strlen(path); rem = attr_nr; for (stk = attr_stack; 0 rem stk; stk = stk-prev) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] attr: fix off-by-one directory component length calculation
Jean-Noël AVILA avila...@gmail.com writes: Thank you for the explanation. I did not monitor the system calls when writing that patch. Where is the perf framework? As the mistake is located in the find_basename function, I would propose a fix directly into it so that the output fits what the other functions expect. Isn't that a crazy semantics for the function, though? I would expect find_basename(/a/path/to/file) to return file, not /file. Something in the line of: diff --git a/attr.c b/attr.c index d6d7190..b6e72f3 100644 --- a/attr.c +++ b/attr.c @@ -572,7 +572,7 @@ static const char *find_basename(const char *path) if (*cp == '/' cp[1]) last_slash = cp; } - return last_slash ? last_slash + 1 : path; + return last_slash ? last_slash : path; } static void prepare_attr_stack(const char *path) @@ -770,6 +770,10 @@ static void collect_all_attrs(const char *path) check_all_attr[i].value = ATTR__UNKNOWN; basename = find_basename(path); + /* the slash is included in the basename + so that it can be matched by a directory pattern */ + if (basename != path) + basename++; pathlen = strlen(path); rem = attr_nr; for (stk = attr_stack; 0 rem stk; stk = stk-prev) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] test-lib.sh: unfilter GIT_PERF_*
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: These variables are user parameters to control how to run the perf tests. Allow users to do so. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- I think the Subject makes more sense, so I'd suggest replacing the current one with PERF_, not with PERF. t/test-lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index f50f834..e1c8c85 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -85,7 +85,7 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $($PERL_PATH -e ' .*_TEST PROVE VALGRIND - PERF_AGGREGATING_LATER + PERF )); my @vars = grep(/^GIT_/ !/^GIT_($ok)/o, @env); print join(\n, @vars); -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 2/8 v2] git_remote_helpers: fix input when running under Python 3
Although 2to3 will fix most issues in Python 2 code to make it run under
Python 3, it does not handle the new strict separation between byte
strings and unicode strings. There is one instance in
git_remote_helpers where we are caught by this, which is when reading
refs from git for-each-ref.
While we could fix this by explicitly handling refs as byte strings,
this is merely punting the problem to users of the library since the
same problem will be encountered as soon you want to display the ref
name to a user.
Instead of doing this, explicit decode the incoming byte string into a
unicode string. Following the lead of pygit2 (the Python bindings for
libgit2 - see [1] and [2]), use the filesystem encoding by default,
providing a way for callers to override this if necessary.
[1]
https://github.com/libgit2/pygit2/blob/e34911b63e5d2266f9f72a4e3f32e27b13190feb/src/pygit2/reference.c#L261
[2]
https://github.com/libgit2/pygit2/blob/e34911b63e5d2266f9f72a4e3f32e27b13190feb/include/pygit2/utils.h#L55
Signed-off-by: John Keeping j...@keeping.me.uk
---
I think this is in fact the best way to handle this, and I hope the
above description clarified why I don't think we want to treat refs as
byte strings in Python 3.
My only remaining question is whether it would be better to set the
error mode when decoding to replace instead of strict (the default).
strict will cause a UnicodeError if the string cannot be decoded
whereas replace will use U+FFFD (the replacement character). [3]
I think it's better to use strict and let the user know that
something has gone wrong rather than silently change the string, but I'd
welcome other opinions.
[3] http://docs.python.org/2/library/codecs.html#codec-base-classes
git_remote_helpers/git/importer.py | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/git_remote_helpers/git/importer.py
b/git_remote_helpers/git/importer.py
index e28cc8f..5bc16a4 100644
--- a/git_remote_helpers/git/importer.py
+++ b/git_remote_helpers/git/importer.py
@@ -1,5 +1,6 @@
import os
import subprocess
+import sys
from git_remote_helpers.util import check_call, check_output
@@ -10,17 +11,26 @@ class GitImporter(object):
This importer simply delegates to git fast-import.
-def __init__(self, repo):
+def __init__(self, repo, ref_encoding=None):
Creates a new importer for the specified repo.
+
+If ref_encoding is specified that refs are decoded using that
+encoding. Otherwise the system filesystem encoding is used.
self.repo = repo
+self.ref_encoding = ref_encoding
def get_refs(self, gitdir):
Returns a dictionary with refs.
args = [git, --git-dir= + gitdir, for-each-ref, refs/heads]
-lines = check_output(args).strip().split('\n')
+encoding = self.ref_encoding
+if encoding is None:
+encoding = sys.getfilesystemencoding()
+if encoding is None:
+encoding = sys.getdefaultencoding()
+lines = check_output(args).decode(encoding).strip().split('\n')
refs = {}
for line in lines:
value, name = line.split(' ')
--
1.8.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] attr: fix off-by-one directory component length calculation
Le mardi 15 janvier 2013 20:29:16, Junio C Hamano a écrit : Jean-Noël AVILA avila...@gmail.com writes: Thank you for the explanation. I did not monitor the system calls when writing that patch. Where is the perf framework? As the mistake is located in the find_basename function, I would propose a fix directly into it so that the output fits what the other functions expect. Isn't that a crazy semantics for the function, though? I would expect find_basename(/a/path/to/file) to return file, not /file. Yes, it is. I was wrong on the meaning of basename. Btw, the test 10 to t9902 is failing on my Debian testing. Is it a known issue? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-hg: store converted URL
Max Horn m...@quendi.de writes: So far, all I look at do not deal with this at all. Any attempts to deal with it should be pretty easy to recognize: The remote-$backend would have to store something into the git config, or else, verify the opaque token and refuse to work with it under certain conditions (e.g. when it contains a relative path). But they don't. E.g. git-remote-testgit has the exact same problem. Thanks for confirming what I suspected. I think the way Felipe's patch makes remote-hg take responsibility of how $opaqueToken should look like for future invocations is the simplest and makes the most sense. We could try to go fancier and end up over-engineering, but I'd rather have a simple fix in the tree first. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tests: turn on test-lint-shell-syntax by default
On 13.01.13 23:38, Junio C Hamano wrote:
Jonathan Nieder jrnie...@gmail.com writes:
Hi,
Torsten Bögershausen wrote:
- /^\s*[^#]\s*which\s/ and err 'which is not portable (please use type)';
+ /^\s*[^#]\s*which\s+[-a-zA-Z0-9]+$/ and err 'which is not portable
(please use type)';
Hmm. Neither the old version nor the new one matches what seem to
be typical uses of 'which', based on a quick code search:
if which sl /dev/null 21
then
sl -l
...
fi
or
if test -x $(which sl 2/dev/null)
then
sl -l
...
fi
Yes, these two misuses are what we want it to trigger on, so the
test is very easy to trigger and produce a false positive, but does
not trigger on what we really want to catch.
That does not sound like a good benefit/cost ratio to me.
Thanks for comments, I think writing a regexp for which is difficult.
What do we think about something like this for fishing for which:
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -644,6 +644,10 @@ yes () {
:
done
}
+which () {
+ echo 2 which is not portable (please use type)
+ exit 1
+}
This will happen in runtime, which might be good enough ?
@Matt:
The [^#] appears to ensure that there's at least one character
before the which and that it's not a pound sign. Why is this done?
This is simply wrong.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 06/14] imap-send.c: remove some unused fields from struct store
Michael Haggerty wrote:
- else if ((arg1 = next_arg(cmd))) {
- if (!strcmp(EXISTS, arg1))
- ctx-gen.count = atoi(arg);
- else if (!strcmp(RECENT, arg1))
- ctx-gen.recent = atoi(arg);
+ } else if ((arg1 = next_arg(cmd))) {
+ /* unused */
The above is just the right thing to do to ensure no behavior change.
Let's take a look at the resulting code, though:
if (... various reasonable things ...) {
...
} else if ((arg1 = next_arg(cmd))) {
/* unused */
} else {
fprintf(stderr, IMAP error: unable to parse
untagged response\n);
return RESP_BAD;
Anyone forced by some bug to examine this /* unused */ case is going
to have no clue what's going on. In that respect, the old code was
much better, since it at least made it clear that one case where this
code gets hit is handling num EXISTS and num RECENT untagged
responses.
I suspect that original code did not have an implicit and intended
missing
else
; /* negligible response; ignore it */
but the intent was rather
else {
fprintf(stderr, IMAP error: I can't
parse this\n);
return RESP_BAD;
}
Since actually fixing that is probably too aggressive for this patch,
how about a FIXME comment like the following?
/*
* Unhandled response-data with at least two words.
* Ignore it.
*
* NEEDSWORK: Previously this case handled 'num EXISTS'
* and 'num RECENT' but as a probably-unintended side
* effect it ignores other unrecognized two-word
* responses. imap-send doesn't ever try to read
* messages or mailboxes these days, so consider
* eliminating this case.
*/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tests: turn on test-lint-shell-syntax by default
Torsten Bögershausen tbo...@web.de writes:
What do we think about something like this for fishing for which:
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -644,6 +644,10 @@ yes () {
:
done
}
+which () {
+ echo 2 which is not portable (please use type)
+ exit 1
+}
This will happen in runtime, which might be good enough ?
if (
which frotz
test $(frobonitz --version -le 2.0
)
then
test_set_prereq FROTZ_FROBONITZ
else
echo 2 suitable Frotz/Frobonitz combo not available;
echo 2 some tests may be skipped
fi
I somehow think this is a lost cause.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] attr: fix off-by-one directory component length calculation
Jean-Noël AVILA avila...@gmail.com writes: Btw, the test 10 to t9902 is failing on my Debian testing. Is it a known issue? Which branch? If you mean 'master' with the patch in this discussion applied, I didn't even have a chance to start today's integration cycle, so I don't know (it is not known to me). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/14] Remove unused code from imap-send.c
Michael Haggerty wrote: imap-send.c | 308 +++- 1 file changed, 55 insertions(+), 253 deletions(-) Patch 14 is lovely. Except for patch 6, for what it's worth these are all Reviewed-by: Jonathan Nieder jrnie...@gmail.com Nicely done. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/8 v2] git_remote_helpers: fix input when running under Python 3
John Keeping j...@keeping.me.uk writes: Although 2to3 will fix most issues in Python 2 code to make it run under Python 3, it does not handle the new strict separation between byte strings and unicode strings. There is one instance in git_remote_helpers where we are caught by this, which is when reading refs from git for-each-ref. While we could fix this by explicitly handling refs as byte strings, this is merely punting the problem to users of the library since the same problem will be encountered as soon you want to display the ref name to a user. Instead of doing this, explicit decode the incoming byte string into a unicode string. That really feels wrong. Displaying is a separate issue and it is the _right_ thing to punt the problem at the lower-level machinery level. Following the lead of pygit2 (the Python bindings for libgit2 - see [1] and [2]),... I do not think other people getting it wrong is not an excuse to repeat the same mistake. Is it really so cumbersome to handle byte strings as byte strings in Python? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] config.txt: Document help.htmlpath config parameter
Signed-off-by: Sebastian Staudt korak...@gmail.com --- Documentation/config.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index bf8f911..e452ff8 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1351,6 +1351,12 @@ help.autocorrect:: value is 0 - the command will be just shown but not executed. This is the default. +help.htmlpath:: + Specify the path where the HTML documentation resides. File system paths + and URLs are supported. HTML pages will be prefixed with this path when + help is displayed in the 'web' format. This defaults to the documentation + path of your Git installation. + http.proxy:: Override the HTTP proxy, normally configured using the 'http_proxy', 'https_proxy', and 'all_proxy' environment variables (see -- 1.8.1.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] config.txt: Document help.htmlpath config parameter
Thanks. This will eventually go to the maintenance track as well. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/8 v2] git_remote_helpers: fix input when running under Python 3
On Tue, Jan 15, 2013 at 12:51:13PM -0800, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: Although 2to3 will fix most issues in Python 2 code to make it run under Python 3, it does not handle the new strict separation between byte strings and unicode strings. There is one instance in git_remote_helpers where we are caught by this, which is when reading refs from git for-each-ref. While we could fix this by explicitly handling refs as byte strings, this is merely punting the problem to users of the library since the same problem will be encountered as soon you want to display the ref name to a user. Instead of doing this, explicit decode the incoming byte string into a unicode string. That really feels wrong. Displaying is a separate issue and it is the _right_ thing to punt the problem at the lower-level machinery level. But the display will require decoding the ref name to a Unicode string, which depends on the encoding of the underlying ref name, so it feels like it should be decoded where it's read (see [1]). Following the lead of pygit2 (the Python bindings for libgit2 - see [1] and [2]),... I do not think other people getting it wrong is not an excuse to repeat the same mistake. Is it really so cumbersome to handle byte strings as byte strings in Python? As [1] says, there is a potential for bugs whenever people attempt to combine Unicode and byte strings. I think it also violates the principle of least surprise if a ref name (a string) doesn't behave like a normal string. [1] http://docs.python.org/3.3/howto/unicode.html#tips-for-writing-unicode-aware-programs John -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/8 v2] git_remote_helpers: fix input when running under Python 3
John Keeping j...@keeping.me.uk writes: That really feels wrong. Displaying is a separate issue and it is the _right_ thing to punt the problem at the lower-level machinery level. But the display will require decoding the ref name to a Unicode string, which depends on the encoding of the underlying ref name, so it feels like it should be decoded where it's read (see [1]). If you botch the decoding in a way you cannot recover the original byte string, you cannot create a ref whose name is the original byte string, no? Keeping the original byte string internally (this includes where you use it to create new refs or update existing refs), and attempting to convert it to Unicode when you choose to show that string as a part of a message to the user (and falling back to replacing some bytes to '?' if you cannot, but do so only in the message), you won't have that problem. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 06/14] imap-send.c: remove some unused fields from struct store
Jonathan Nieder jrnie...@gmail.com writes:
Michael Haggerty wrote:
-else if ((arg1 = next_arg(cmd))) {
-if (!strcmp(EXISTS, arg1))
-ctx-gen.count = atoi(arg);
-else if (!strcmp(RECENT, arg1))
-ctx-gen.recent = atoi(arg);
+} else if ((arg1 = next_arg(cmd))) {
+/* unused */
The above is just the right thing to do to ensure no behavior change.
Let's take a look at the resulting code, though:
if (... various reasonable things ...) {
...
} else if ((arg1 = next_arg(cmd))) {
/* unused */
} else {
fprintf(stderr, IMAP error: unable to parse
untagged response\n);
return RESP_BAD;
Anyone forced by some bug to examine this /* unused */ case is going
to have no clue what's going on. In that respect, the old code was
much better, since it at least made it clear that one case where this
code gets hit is handling num EXISTS and num RECENT untagged
responses.
I suspect that original code did not have an implicit and intended
missing
else
; /* negligible response; ignore it */
but the intent was rather
else {
fprintf(stderr, IMAP error: I can't
parse this\n);
return RESP_BAD;
}
Since actually fixing that is probably too aggressive for this patch,
how about a FIXME comment like the following?
/*
* Unhandled response-data with at least two words.
* Ignore it.
*
* NEEDSWORK: Previously this case handled 'num EXISTS'
* and 'num RECENT' but as a probably-unintended side
* effect it ignores other unrecognized two-word
* responses. imap-send doesn't ever try to read
* messages or mailboxes these days, so consider
* eliminating this case.
*/
Hmph; it seems that it is not worth rerolling the whole thing only
for this, so let me squash this in, replacing the /* unused */ with
the large comment, and then merge the result to 'next'.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 2/8 v3] git_remote_helpers: fix input when running under Python 3
Although 2to3 will fix most issues in Python 2 code to make it run under
Python 3, it does not handle the new strict separation between byte
strings and unicode strings. There is one instance in
git_remote_helpers where we are caught by this, which is when reading
refs from git for-each-ref.
Fix this by operating on the returned string as a byte string rather
than a unicode string. As this method is currently only used internally
by the class this does not affect code anywhere else.
Note that we cannot use byte strings in the source as the 'b' prefix is
not supported before Python 2.7 so in order to maintain compatibility
with the maximum range of Python versions we use an explicit call to
encode().
Signed-off-by: John Keeping j...@keeping.me.uk
---
On Tue, Jan 15, 2013 at 02:04:29PM -0800, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
That really feels wrong. Displaying is a separate issue and it is
the _right_ thing to punt the problem at the lower-level machinery
level.
But the display will require decoding the ref name to a Unicode string,
which depends on the encoding of the underlying ref name, so it feels
like it should be decoded where it's read (see [1]).
If you botch the decoding in a way you cannot recover the original
byte string, you cannot create a ref whose name is the original byte
string, no? Keeping the original byte string internally (this
includes where you use it to create new refs or update existing
refs), and attempting to convert it to Unicode when you choose to
show that string as a part of a message to the user (and falling
back to replacing some bytes to '?' if you cannot, but do so only in
the message), you won't have that problem.
Actually, this method is currently only used internally so I don't think
my argument holds.
This is what keeping the refs as byte strings looks like.
git_remote_helpers/git/importer.py | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/git_remote_helpers/git/importer.py
b/git_remote_helpers/git/importer.py
index e28cc8f..c54846c 100644
--- a/git_remote_helpers/git/importer.py
+++ b/git_remote_helpers/git/importer.py
@@ -18,13 +18,16 @@ class GitImporter(object):
def get_refs(self, gitdir):
Returns a dictionary with refs.
+
+Note that the keys in the returned dictionary are byte strings as
+read from git.
args = [git, --git-dir= + gitdir, for-each-ref, refs/heads]
-lines = check_output(args).strip().split('\n')
+lines = check_output(args).strip().split('\n'.encode('utf-8'))
refs = {}
for line in lines:
-value, name = line.split(' ')
-name = name.strip('commit\t')
+value, name = line.split(' '.encode('utf-8'))
+name = name.strip('commit\t'.encode('utf-8'))
refs[name] = value
return refs
--
1.8.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/8] git_remote_helpers: Force rebuild if python version changes
On Sun, Jan 13, 2013 at 05:52:38PM +, John Keeping wrote:
On Sun, Jan 13, 2013 at 12:14:02PM -0500, Pete Wyckoff wrote:
j...@keeping.me.uk wrote on Sun, 13 Jan 2013 16:26 +:
On Sat, Jan 12, 2013 at 06:30:44PM -0500, Pete Wyckoff wrote:
j...@keeping.me.uk wrote on Sat, 12 Jan 2013 19:23 +:
When different version of python are used to build via distutils, the
behaviour can change. Detect changes in version and pass --force in
this case.
[..]
diff --git a/git_remote_helpers/Makefile b/git_remote_helpers/Makefile
[..]
+py_version=$(shell $(PYTHON_PATH) -c \
+'import sys; print(%i.%i % sys.version_info[:2])')
+
all: $(pysetupfile)
-$(QUIET)$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build
+$(QUIET)test $$(cat GIT-PYTHON_VERSION 2/dev/null) =
$(py_version) || \
+flags=--force; \
+$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build $$flags
+$(QUIET)echo $(py_version) GIT-PYTHON_VERSION
Can you depend on ../GIT-PYTHON-VARS instead? It comes from
96a4647 (Makefile: detect when PYTHON_PATH changes, 2012-12-18).
It doesn't check version, just path, but hopefully that's good
enough. I'm imagining a rule that would do clean if
../GIT-PYTHON-VARS changed, then build without --force.
I was trying to keep the git_remote_helpers directory self contained. I
can't see how to depend on ../GIT-PYTHON-VARS in a way that is as simple
as this and keeps make -C git_remote_helpers working in a clean tree.
Am I missing something obvious here?
Not if it wants to stay self-contained; you're right.
I'm not thrilled with how git_remote_helpers/Makefile always
runs setup.py, and always generates PYLIBDIR, and now always
invokes python a third time to see if its version changed.
I don't think PYLIBDIR will be calculated unless it's used ('=' not
':=' means its a deferred variable).
I wonder if the version check should move into setup.py - it would be
just as easy to check the file there and massage sys.args, although
possibly not as neat.
For reference, putting the version check in setup.py looks like this:
-- 8 --
diff --git a/git_remote_helpers/setup.py b/git_remote_helpers/setup.py
index 6de41de..2c21eb5 100644
--- a/git_remote_helpers/setup.py
+++ b/git_remote_helpers/setup.py
@@ -3,6 +3,7 @@
Distutils build/install script for the git_remote_helpers package.
from distutils.core import setup
+import sys
# If building under Python3 we need to run 2to3 on the code, do this by
# trying to import distutils' 2to3 builder, which is only available in
@@ -13,6 +14,24 @@ except ImportError:
# 2.x
from distutils.command.build_py import build_py
+
+current_version = '%d.%d' % sys.version_info[:2]
+try:
+f = open('GIT-PYTHON_VERSION', 'r')
+latest_version = f.read().strip()
+f.close()
+
+if latest_version != current_version:
+if not '--force' in sys.argv:
+sys.argv.insert(0, '--force')
+except IOError:
+pass
+
+f = open('GIT-PYTHON_VERSION', 'w')
+f.write(current_version)
+f.close()
+
+
setup(
name = 'git_remote_helpers',
version = '0.1.0',
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 06/14] imap-send.c: remove some unused fields from struct store
Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: Since actually fixing that is probably too aggressive for this patch, how about a FIXME comment like the following? [...] Hmph; it seems that it is not worth rerolling the whole thing only for this, so let me squash this in, replacing the /* unused */ with the large comment, and then merge the result to 'next'. Sounds good to me. Next time, I'll include a 'fixup!' patch instead of making you do the copy/pasting. Thanks, both. Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] Move Git::SVN::get_tz to Git::get_tz_offset
This function has utility outside of the SVN module for any routine
that needs the equivalent of GNU strftime's %z formatting option.
Move it to the top-level Git.pm so that non-SVN modules don't need to
import the SVN module to use it.
The rename makes the purpose of the function clearer.
Signed-off-by: Ben Walton bdwal...@gmail.com
---
perl/Git.pm | 23 +++
perl/Git/SVN.pm | 12 ++--
perl/Git/SVN/Log.pm |8 ++--
3 files changed, 31 insertions(+), 12 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 931047c..5649bcc 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -59,6 +59,7 @@ require Exporter;
command_bidi_pipe command_close_bidi_pipe
version exec_path html_path hash_object git_cmd_try
remote_refs prompt
+get_tz_offset
temp_acquire temp_release temp_reset temp_path);
@@ -102,6 +103,7 @@ use Error qw(:try);
use Cwd qw(abs_path cwd);
use IPC::Open2 qw(open2);
use Fcntl qw(SEEK_SET SEEK_CUR);
+use Time::Local qw(timelocal);
}
@@ -511,6 +513,27 @@ Cgit --html-path). Useful mostly only internally.
sub html_path { command_oneline('--html-path') }
+
+=item get_tz_offset ( TIME )
+
+Return the time zone offset from GMT in the form +/-HHMM where HH is
+the number of hours from GMT and MM is the number of minutes. This is
+the equivalent of what strftime(%z, ...) would provide on a GNU
+platform.
+
+If TIME is not supplied, the current local time is used.
+
+=cut
+
+sub get_tz_offset {
+ # some systmes don't handle or mishandle %z, so be creative.
+ my $t = shift || time;
+ my $gm = timelocal(gmtime($t));
+ my $sign = qw( + + - )[ $t = $gm ];
+ return sprintf(%s%02d%02d, $sign, (gmtime(abs($t - $gm)))[2,1]);
+}
+
+
=item prompt ( PROMPT , ISPASSWORD )
Query user CPROMPT and return answer from user.
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 59215fa..8c84560 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -11,7 +11,6 @@ use Carp qw/croak/;
use File::Path qw/mkpath/;
use File::Copy qw/copy/;
use IPC::Open3;
-use Time::Local;
use Memoize; # core since 5.8.0, Jul 2002
use Memoize::Storable;
use POSIX qw(:signal_h);
@@ -22,6 +21,7 @@ use Git qw(
command_noisy
command_output_pipe
command_close_pipe
+get_tz_offset
);
use Git::SVN::Utils qw(
fatal
@@ -1311,14 +1311,6 @@ sub get_untracked {
\@out;
}
-sub get_tz {
- # some systmes don't handle or mishandle %z, so be creative.
- my $t = shift || time;
- my $gm = timelocal(gmtime($t));
- my $sign = qw( + + - )[ $t = $gm ];
- return sprintf(%s%02d%02d, $sign, (gmtime(abs($t - $gm)))[2,1]);
-}
-
# parse_svn_date(DATE)
#
# Given a date (in UTC) from Subversion, return a string in the format
@@ -1351,7 +1343,7 @@ sub parse_svn_date {
delete $ENV{TZ};
}
- my $our_TZ = get_tz();
+ my $our_TZ = get_tz_offset();
# This converts $epoch_in_UTC into our local timezone.
my ($sec, $min, $hour, $mday, $mon, $year,
diff --git a/perl/Git/SVN/Log.pm b/perl/Git/SVN/Log.pm
index 3cc1c6f..3f8350a 100644
--- a/perl/Git/SVN/Log.pm
+++ b/perl/Git/SVN/Log.pm
@@ -2,7 +2,11 @@ package Git::SVN::Log;
use strict;
use warnings;
use Git::SVN::Utils qw(fatal);
-use Git qw(command command_oneline command_output_pipe command_close_pipe);
+use Git qw(command
+ command_oneline
+ command_output_pipe
+ command_close_pipe
+ get_tz_offset);
use POSIX qw/strftime/;
use constant commit_log_separator = ('-' x 72) . \n;
use vars qw/$TZ $limit $color $pager $non_recursive $verbose $oneline
@@ -119,7 +123,7 @@ sub run_pager {
sub format_svn_date {
my $t = shift || time;
require Git::SVN;
- my $gmoff = Git::SVN::get_tz($t);
+ my $gmoff = get_tz_offset($t);
return strftime(%Y-%m-%d %H:%M:%S $gmoff (%a, %d %b %Y),
localtime($t));
}
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] Avoid non-portable strftime format specifiers in git-cvsimport
Neither %s or %z are portable strftime format specifiers. There is no
need for %s in git-cvsimport as the supplied time is already in
seconds since the epoch. For %z, use the function get_tz_offset
provided by Git.pm instead.
Signed-off-by: Ben Walton bdwal...@gmail.com
---
git-cvsimport.perl |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 0a31ebd..344f120 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -26,6 +26,7 @@ use IO::Socket;
use IO::Pipe;
use POSIX qw(strftime tzset dup2 ENOENT);
use IPC::Open2;
+use Git qw(get_tz_offset);
$SIG{'PIPE'}=IGNORE;
set_timezone('UTC');
@@ -864,7 +865,9 @@ sub commit {
}
set_timezone($author_tz);
- my $commit_date = strftime(%s %z, localtime($date));
+ # $date is in the seconds since epoch format
+ my $tz_offset = get_tz_offset($date);
+ my $commit_date = $date $tz_offset;
set_timezone('UTC');
$ENV{GIT_AUTHOR_NAME} = $author_name;
$ENV{GIT_AUTHOR_EMAIL} = $author_email;
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] Allow Git::get_tz_offset to properly handle DST boundary times
The Git::get_tz_offset is meant to provide a workalike replacement for
the GNU strftime %z format specifier. The algorithm used failed to
properly handle DST boundary cases.
For example, the unix time 1162105199 in CST6CDT saw set_tz_offset
improperly return -0600 instead of -0500.
TZ=CST6CDT date -d @1162105199 +%c %z
Sun 29 Oct 2006 01:59:59 AM CDT -0500
$ zdump -v /usr/share/zoneinfo/CST6CDT | grep 2006
/usr/share/zoneinfo/CST6CDT Sun Apr 2 07:59:59 2006 UTC = Sun Apr 2
01:59:59 2006 CST isdst=0 gmtoff=-21600
/usr/share/zoneinfo/CST6CDT Sun Apr 2 08:00:00 2006 UTC = Sun Apr 2
03:00:00 2006 CDT isdst=1 gmtoff=-18000
/usr/share/zoneinfo/CST6CDT Sun Oct 29 06:59:59 2006 UTC = Sun Oct 29
01:59:59 2006 CDT isdst=1 gmtoff=-18000
/usr/share/zoneinfo/CST6CDT Sun Oct 29 07:00:00 2006 UTC = Sun Oct 29
01:00:00 2006 CST isdst=0 gmtoff=-21600
To determine how many hours/minutes away from GMT a particular time
was, we calculated the gmtime() of the requested time value and then
used Time::Local's timelocal() function to turn the GMT-based time
back into a scalar value representing seconds from the epoch. Because
GMT has no daylight savings time, timelocal() cannot handle the
ambiguous times that occur at DST boundaries since there are two
possible correct results.
To work around the ambiguity at these boundaries, we must take into
account the pre and post conversion values for is_dst as provided by
both the original time value and the value that has been run through
timelocal(). If the is_dst field of the two times disagree then we
must modify the value returned from timelocal() by an hour in the
correct direction.
Signed-off-by: Ben Walton bdwal...@gmail.com
---
perl/Git.pm | 20
1 file changed, 20 insertions(+)
diff --git a/perl/Git.pm b/perl/Git.pm
index 5649bcc..788b9b4 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -528,7 +528,27 @@ If TIME is not supplied, the current local time is used.
sub get_tz_offset {
# some systmes don't handle or mishandle %z, so be creative.
my $t = shift || time;
+ # timelocal() has a problem when it comes to DST ambiguity so
+ # times that are on a DST boundary cannot be properly converted
+ # using it. we will possibly adjust its result depending on whehter
+ # pre and post conversions agree on DST
my $gm = timelocal(gmtime($t));
+
+ # we need to know whether we were originally in DST or not
+ my $orig_dst = (localtime($t))[8];
+ # and also whether timelocal thinks we're in DST
+ my $conv_dst = (localtime($gm))[8];
+
+ # re-adjust $gm based on the DST value for the two times we're
+ # handling.
+ if ($orig_dst != $conv_dst) {
+ if ($orig_dst == 1) {
+ $gm -= 3600;
+ } else {
+ $gm += 3600;
+ }
+ }
+
my $sign = qw( + + - )[ $t = $gm ];
return sprintf(%s%02d%02d, $sign, (gmtime(abs($t - $gm)))[2,1]);
}
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] Fix a portability issue with git-cvsimport
This patch series started as a quick fix for the use of %s and %z in git-cvsimport but grew slightly when I realized that the get_tz (get_tz_offset after this series) function used by Git::SVN didn't properly handle DST boundary conditions. I realize that Eric Raymond is working to deprecate the current iteration of git-cvsimport so this series may be only partially worthwhile. (If the cvsps 2 vs 3 issue does require a fallback git-cvsimport script then maybe the whole series is still valid?) I'm not attached to the current git-cvsimport so if the third patch isn't useful then maybe the only the second patch is worthwhile (modified to correct the function in its current location). Currently, the DST boundary functionality is exercised by the git-cvsimport tests. If those go away as part of Eric's work then another test that monitors this condition should be added. I can do that as part of this series if it seems the right way to go. Ben Walton (3): Move Git::SVN::get_tz to Git::get_tz_offset Allow Git::get_tz_offset to properly handle DST boundary times Avoid non-portable strftime format specifiers in git-cvsimport git-cvsimport.perl |5 - perl/Git.pm | 43 +++ perl/Git/SVN.pm | 12 ++-- perl/Git/SVN/Log.pm |8 ++-- 4 files changed, 55 insertions(+), 13 deletions(-) -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--simplify-merges returns too many references
I thought I understood the intent of the various history simplification switches, but maybe I am still confused. In git.git, I see three commits which touch stripspace.c: $ git log --oneline -- builtin/stripspace.c 497215d Update documentation for stripspace c2857fb stripspace: fix outdated comment 81b50f3 Move 'builtin-*' into a 'builtin/' subdirectory With --full-history and also with --dense, I see the same three commits: $ git log --full-history --oneline -- builtin/stripspace.c 497215d Update documentation for stripspace c2857fb stripspace: fix outdated comment 81b50f3 Move 'builtin-*' into a 'builtin/' subdirectory $ git log --dense --oneline -- builtin/stripspace.c 497215d Update documentation for stripspace c2857fb stripspace: fix outdated comment 81b50f3 Move 'builtin-*' into a 'builtin/' subdirectory But with --simplify-merges, I see _more_ commits. $ git log --simplify-merges --oneline -- builtin/stripspace.c 634392b Add 'contrib/subtree/' from commit ... 497215d Update documentation for stripspace c2857fb stripspace: fix outdated comment 81b50f3 Move 'builtin-*' into a 'builtin/' subdirectory 610f043 Import branch 'git-p4' of git://repo.or.cz/fast-export b4d2b04 Merge git-gui 0a8f4f0 Merge git://git.kernel.org/pub/scm/git/gitweb 98e031f Merge git-tools repository under tools subdirectory 5569bf9 Do a cross-project merge of Paul Mackerras' gitk visualizer None of the new commits touches this file. The man page suggests that simplify-merges should result in fewer commits than full-history. --simplify-merges Additional option to --full-history to remove some needless merges from the resulting history, as there are no selected commits contributing to this merge. Am I confused or is git? Phil -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] attr: fix off-by-one directory component length calculation
On Tue, Jan 15, 2013 at 12:49:05PM -0800, Junio C Hamano wrote: Jean-Noël AVILA avila...@gmail.com writes: Btw, the test 10 to t9902 is failing on my Debian testing. Is it a known issue? Which branch? t9902.10 is overly sensitive to extra git commands in your PATH, as well as cruft in your build dir (especially if you have been building 'pu', which has git-check-ignore). Try make clean make test. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Fix a portability issue with git-cvsimport
Ben Walton bdwal...@gmail.com writes: This patch series started as a quick fix for the use of %s and %z in git-cvsimport but grew slightly when I realized that the get_tz (get_tz_offset after this series) function used by Git::SVN didn't properly handle DST boundary conditions. I realize that Eric Raymond is working to deprecate the current iteration of git-cvsimport so this series may be only partially worthwhile. (If the cvsps 2 vs 3 issue does require a fallback git-cvsimport script then maybe the whole series is still valid?) There is my reroll of Eric's patch [*1*], that is in 'pu'. The topic ends at 12b3541 (t9600: adjust for new cvsimport, 2013-01-13). I think the folks on the traditional Git side prefer the approach taken by it to keep the old one under cvsimport-2 while adding Eric's as cvsimport-3 and have a separate version switcher wrapper [*2*, *3*]. Also Chris Rorvick, a contributor to cvsps-3 new cvsimport combo, who already has patches to Eric's version, agrees that it is a foundation we can build on together [*4*]. Eric hasn't spoken on the topic yet, but I think what the rest of us agreed may be a reasonable starting point. I think I can apply your patches on top of 12b3541 with am -3 and have it automatically update git-cvsimport-2.perl ;-) [References] *1* http://thread.gmane.org/gmane.comp.version-control.git/213170/focus=213460 *2* http://thread.gmane.org/gmane.comp.version-control.git/213170/focus=213432 *3* http://thread.gmane.org/gmane.comp.version-control.git/213170/focus=213466 *4* http://thread.gmane.org/gmane.comp.version-control.git/213537/focus=213595 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: --simplify-merges returns too many references
Phil Hord phil.h...@gmail.com writes: But with --simplify-merges, I see _more_ commits. $ git log --simplify-merges --oneline -- builtin/stripspace.c 634392b Add 'contrib/subtree/' from commit ... 497215d Update documentation for stripspace c2857fb stripspace: fix outdated comment 81b50f3 Move 'builtin-*' into a 'builtin/' subdirectory 610f043 Import branch 'git-p4' of git://repo.or.cz/fast-export b4d2b04 Merge git-gui 0a8f4f0 Merge git://git.kernel.org/pub/scm/git/gitweb 98e031f Merge git-tools repository under tools subdirectory 5569bf9 Do a cross-project merge of Paul Mackerras' gitk visualizer After indenting away what should be shown, I notice all these extra are merges without any common ancestors. Just an observation; I didn't think things through. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-hg: fix handling of file perms when pushing
Max Horn m...@quendi.de writes: On 15.01.2013, at 14:02, Max Horn wrote: Previously, when changing and committing an executable file, the file would loose its executable bit on the hg side. Likewise, symlinks ended up as normal files. This was not immediately apparent on the git side unless one did a fresh clone. Sorry, forgot to sign off, please add: Signed-off-by: Max Horn m...@quendi.de Max Thanks; merged together with the other patch from Felipe to 'next'. Unfortunately I noticed the loose typo (I think you meant lose) after I pushed out the results X-. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/8 v3] git_remote_helpers: fix input when running under Python 3
j...@keeping.me.uk wrote on Tue, 15 Jan 2013 22:40 +:
This is what keeping the refs as byte strings looks like.
As John knows, it is not possible to interpret text from a byte
string without talking about the character encoding.
Git is (largely) a C program and uses the character set defined
in the C standard, which is a subset of ASCII. But git does
math on strings, like this snippet that takes something from
argv[] and prepends refs/heads/:
strcpy(refname, refs/heads/);
strcpy(refname + strlen(refs/heads/), ret-name);
The result doesn't talk about what character set it is using,
but because it combines a prefix from ASCII with its input,
git makes the assumption that the input is ASCII-compatible.
If you feed a UTF-16 string in argv, e.g.
$ echo master | iconv -f ascii -t utf16 | xargs git branch
xargs: Warning: a NUL character occurred in the input. It cannot be passed
through in the argument list. Did you mean to use the --null option?
fatal: Not a valid object name: ''.
you get an error about NUL, and not the branch you hoped for.
Git assumes that the input character set contains roughly ASCII
in byte positions 0..127.
That's one small reason why the useful character encodings put
ASCII in the 0..127 range, including utf-8, big5 and shift-jis.
ASCII is indeed special due to its legacy, and both C and Python
recognize this.
diff --git a/git_remote_helpers/git/importer.py
b/git_remote_helpers/git/importer.py
@@ -18,13 +18,16 @@ class GitImporter(object):
def get_refs(self, gitdir):
Returns a dictionary with refs.
+
+Note that the keys in the returned dictionary are byte strings as
+read from git.
args = [git, --git-dir= + gitdir, for-each-ref, refs/heads]
-lines = check_output(args).strip().split('\n')
+lines = check_output(args).strip().split('\n'.encode('utf-8'))
refs = {}
for line in lines:
-value, name = line.split(' ')
-name = name.strip('commit\t')
+value, name = line.split(' '.encode('utf-8'))
+name = name.strip('commit\t'.encode('utf-8'))
refs[name] = value
return refs
I'd suggest for this Python conundrum using byte-string literals, e.g.:
lines = check_output(args).strip().split(b'\n')
value, name = line.split(b' ')
name = name.strip(b'commit\t')
Essentially identical to what you have, but avoids naming utf-8 as
the encoding. It instead relies on Python's interpretation of
ASCII characters in string context, which is exactly what C does.
-- Pete
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] attr: fix off-by-one directory component length calculation
On Wed, Jan 16, 2013 at 2:14 AM, Jean-Noël AVILA avila...@gmail.com wrote: I did not monitor the system calls when writing that patch. Where is the perf framework? It's in t/perf. I think you can do: ./run HEAD . to run and compare performance of HEAD and working directory (assume you haven't commit yet). Check out the README file. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] ignore memcmp() overreading in bsearch() callback
Am 15.01.2013 21:27, schrieb Andreas Schwab:
René Scharfe rene.scha...@lsrfire.ath.cx writes:
+return '\0' - ent-name[key-len];
You need to cast to unsigned char first to make it consistent with
memcmp and strcmp.
Thanks for catching this!
-- 8 --
Subject: [PATCH] refs: use strncmp() instead of strlen() and memcmp()
Simplify ref_entry_cmp_sslice() by using strncmp() to compare the
length-limited key and a NUL-terminated entry. While we're at it,
retain the const attribute of the input pointers.
Signed-off-by: Rene Scharfe rene.scha...@lsrfire.ath.cx
---
refs.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index 541fec2..5129da0 100644
--- a/refs.c
+++ b/refs.c
@@ -333,14 +333,12 @@ struct string_slice {
static int ref_entry_cmp_sslice(const void *key_, const void *ent_)
{
- struct string_slice *key = (struct string_slice *)key_;
- struct ref_entry *ent = *(struct ref_entry **)ent_;
- int entlen = strlen(ent-name);
- int cmplen = key-len entlen ? key-len : entlen;
- int cmp = memcmp(key-str, ent-name, cmplen);
+ const struct string_slice *key = key_;
+ const struct ref_entry *ent = *(const struct ref_entry * const *)ent_;
+ int cmp = strncmp(key-str, ent-name, key-len);
if (cmp)
return cmp;
- return key-len - entlen;
+ return '\0' - (unsigned char)ent-name[key-len];
}
/*
--
1.8.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] attr: fix off-by-one directory component length calculation
On Wed, Jan 16, 2013 at 2:29 AM, Junio C Hamano gits...@pobox.com wrote: Jean-Noël AVILA avila...@gmail.com writes: Thank you for the explanation. I did not monitor the system calls when writing that patch. Where is the perf framework? As the mistake is located in the find_basename function, I would propose a fix directly into it so that the output fits what the other functions expect. Isn't that a crazy semantics for the function, though? I would expect find_basename(/a/path/to/file) to return file, not Actually I'd like to remove that function. The function is called twice: - collect_all_attrs + prepare_attr_stack * find_basename + find_basename which could be reordered to - collect_all_attrs + find_basename + prepare_attr_stack (modified to take dirlen from caller) and because that'll be the only place find_basename is used, we could just inline the code there. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jan 2013, #06; Mon, 14)
On Mon, Jan 14, 2013 at 10:23 PM, Junio C Hamano gits...@pobox.com wrote: * as/check-ignore (2013-01-10) 12 commits (merged to 'next' on 2013-01-14 at 9df2afc) + t0008: avoid brace expansion + add git-check-ignore sub-command + setup.c: document get_pathspec() + add.c: extract new die_if_path_beyond_symlink() for reuse + add.c: extract check_path_for_gitlink() from treat_gitlinks() for reuse + pathspec.c: rename newly public functions for clarity + add.c: move pathspec matchers into new pathspec.c for reuse + add.c: remove unused argument from validate_pathspec() + dir.c: improve docs for match_pathspec() and match_pathspec_depth() + dir.c: provide clear_directory() for reclaiming dir_struct memory + dir.c: keep track of where patterns came from + dir.c: use a single struct exclude_list per source of excludes Add a new command git check-ignore for debugging .gitignore files. The above is v4 plus the t0008: avoid brace expansion fix. v4 is slightly outdated and not quite the right version to merge to 'next'. I'll post a v5 re-roll as per: http://thread.gmane.org/gmane.comp.version-control.git/212184/focus=212856 in the next 24 hours or so. I think the t0008: avoid brace expansion fix at the tip should probably be squashed into its parent. I've amended the commit message accordingly in my github fork. Thanks, Adam -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] Avoid non-portable strftime format specifiers in git-cvsimport
On Tue, Jan 15, 2013 at 5:10 PM, Ben Walton bdwal...@gmail.com wrote: Neither %s or %z are portable strftime format specifiers. There is no need for %s in git-cvsimport as the supplied time is already in seconds since the epoch. For %z, use the function get_tz_offset provided by Git.pm instead. Out of curiosity, which platforms are affected? Assuming DST is a 1 hour shift (patch 2/3) is not necessarily portable either, though this currently appears to only affect a small island off of the coast of Australia. :-) Chris -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] attr: fix off-by-one directory component length calculation
On Wed, Jan 16, 2013 at 08:08:03AM +0700, Duy Nguyen wrote:
Actually I'd like to remove that function.
This is what I had in mind:
-- 8 --
Subject: [PATCH] attr: avoid calling find_basename() twice per path
find_basename() is only used inside collect_all_attrs(), called once
in prepare_attr_stack, then again after prepare_attr_stack()
returns. Both calls return exact same value. Reorder the code to do it
once.
While at it, make use of pathlen to stop searching early if
possible.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
attr.c | 46 +++---
1 file changed, 19 insertions(+), 27 deletions(-)
diff --git a/attr.c b/attr.c
index cfc6748..04cb9a0 100644
--- a/attr.c
+++ b/attr.c
@@ -564,32 +564,12 @@ static void bootstrap_attr_stack(void)
attr_stack = elem;
}
-static const char *find_basename(const char *path)
-{
- const char *cp, *last_slash = NULL;
-
- for (cp = path; *cp; cp++) {
- if (*cp == '/' cp[1])
- last_slash = cp;
- }
- return last_slash ? last_slash + 1 : path;
-}
-
-static void prepare_attr_stack(const char *path)
+static void prepare_attr_stack(const char *path, int dirlen)
{
struct attr_stack *elem, *info;
- int dirlen, len;
+ int len;
const char *cp;
- dirlen = find_basename(path) - path;
-
- /*
-* find_basename() includes the trailing slash, but we do
-* _not_ want it.
-*/
- if (dirlen)
- dirlen--;
-
/*
* At the bottom of the attribute stack is the built-in
* set of attribute definitions, followed by the contents
@@ -769,15 +749,27 @@ static int macroexpand_one(int attr_nr, int rem)
static void collect_all_attrs(const char *path)
{
struct attr_stack *stk;
- int i, pathlen, rem;
- const char *basename;
+ int i, pathlen, rem, dirlen = 0;
+ const char *basename = path, *cp;
- prepare_attr_stack(path);
+ pathlen = strlen(path);
+
+ /*
+* This loop is similar to strrchr(path, '/') except that the
+* trailing slash is skipped.
+*/
+ for (cp = path + pathlen - 2; cp = path; cp--) {
+ if (*cp == '/') {
+ basename = cp + 1;
+ dirlen = cp - path;
+ break;
+ }
+ }
+
+ prepare_attr_stack(path, dirlen);
for (i = 0; i attr_nr; i++)
check_all_attr[i].value = ATTR__UNKNOWN;
- basename = find_basename(path);
- pathlen = strlen(path);
rem = attr_nr;
for (stk = attr_stack; 0 rem stk; stk = stk-prev)
rem = fill(path, pathlen, basename, stk, rem);
--
1.8.0.rc3.18.g0d9b108
-- 8 --
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] Update HTTPD/daemon tests for new push.default
I saw a string of these commits already, but found a few more when running the test suite. Brian Gernhardt (3): t5550: do not assume the matching push is the default t5551: do not assume the matching push is the default t5570: do not assume the matching push is the default t/t5550-http-fetch.sh | 1 + t/t5551-http-fetch.sh | 1 + t/t5570-git-daemon.sh | 1 + 3 files changed, 3 insertions(+) -- 1.8.1.rc1.222.gec797b3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] t5551: do not assume the matching push is the default
Signed-off-by: Brian Gernhardt br...@gernhardtsoftware.com
---
t/t5551-http-fetch.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
index c5cd2e3..1b55086 100755
--- a/t/t5551-http-fetch.sh
+++ b/t/t5551-http-fetch.sh
@@ -13,6 +13,7 @@ LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5551'}
start_httpd
test_expect_success 'setup repository' '
+ git config push.default matching
echo content file
git add file
git commit -m one
--
1.8.1.rc1.222.gec797b3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] t5550: do not assume the matching push is the default
Signed-off-by: Brian Gernhardt br...@gernhardtsoftware.com
---
t/t5550-http-fetch.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh
index 80d20c8..f7d0f14 100755
--- a/t/t5550-http-fetch.sh
+++ b/t/t5550-http-fetch.sh
@@ -13,6 +13,7 @@ LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5550'}
start_httpd
test_expect_success 'setup repository' '
+ git config push.default matching
echo content1 file
git add file
git commit -m one
--
1.8.1.rc1.222.gec797b3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] t5570: do not assume the matching push is the default
Signed-off-by: Brian Gernhardt br...@gernhardtsoftware.com
---
t/t5570-git-daemon.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh
index a3a4e47..f01edff 100755
--- a/t/t5570-git-daemon.sh
+++ b/t/t5570-git-daemon.sh
@@ -8,6 +8,7 @@ LIB_GIT_DAEMON_PORT=${LIB_GIT_DAEMON_PORT-5570}
start_git_daemon
test_expect_success 'setup repository' '
+ git config push.default matching
echo content file
git add file
git commit -m one
--
1.8.1.rc1.222.gec797b3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/7] guilt patches, including git 1.8 support
Hi Jeff and other guilty parties, I collected all the guilt patches I could find on-list and added one of my own. Completely untested, except for running the regression tests. These are also available via git protocol from git://repo.or.cz/guilt/mob.git mob Thoughts? Jonathan Nieder (1): Drop unneeded git version check. Per Cederqvist (6): get rid of cat: write error: Broken pipe error message The tests should not fail if log.date or log.decorate are set. Testsuite: get rid of Broken pipe errors from yes. Handle empty patches and patches with only a header. Fix fatal guilt graph error in sha1sum invocation. Change git branch when patches are applied. Documentation/guilt.7 | 4 + guilt | 73 +--- guilt-branch | 12 +- guilt-commit | 7 + guilt-import-commit | 4 +- guilt-repair | 7 +- os.Darwin | 7 +- os.Linux | 7 +- os.SunOS | 7 +- regression/scaffold | 7 +- regression/t-029.sh | 6 +- regression/t-052.out | 24 +-- regression/t-052.sh | 7 +- regression/t-061.out | 468 ++ regression/t-061.sh | 148 15 files changed, 743 insertions(+), 45 deletions(-) create mode 100644 regression/t-061.out create mode 100755 regression/t-061.sh -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT] [PATCH 1/7] get rid of cat: write error: Broken pipe error message
From: Per Cederqvist ced...@opera.com
Date: Tue, 13 Mar 2012 14:39:44 +0100
In some circumstances (like running guilt in a M-x shell buffer in
Emacs) cat may give the above error message when the reader of the
output from cat exits without reading all input from cat. (In other
circumstances cat is just silently terminated with SIGPIPE.)
Get rid of the error by removing the useless use of cat in
do_get_header and do_get_full_header.
Signed-off-by: Per Cederqvist ced...@opera.com
Acked-by: Jeff Sipek jef...@josefsipek.net
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
guilt | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/guilt b/guilt
index d1e17d4..7f6806e 100755
--- a/guilt
+++ b/guilt
@@ -359,7 +359,7 @@ do_get_header()
# 4th line skips any empty lines thereafter.
# 5th line turns off empty line skip upon seeing a non-empty line.
# 6th line terminates execution when we encounter the diff
- cat $1 | awk '
+ awk '
BEGIN{body=0; subj=0}
/^Subject:/ (body == 0 subj == 0){subj=1; print substr($0, 10) \n;
next}
/^(Subject:|From:|Author:|Date:|commit)/ (body == 0){next}
@@ -369,7 +369,7 @@ BEGIN{body=0; subj=0}
/^(diff |---$|--- )/{exit}
{print $0}
END{}
-'
+' $1
}
# usage: do_get_full_header patchfile
@@ -377,12 +377,12 @@ do_get_full_header()
{
# 2nd line checks for the begining of a patch
# 3rd line outputs the line if it didn't get pruned by the above rules
- cat $1 | awk '
+ awk '
BEGIN{}
/^(diff |---$|--- )/{exit}
{print $0}
END{}
-'
+' $1
}
# usage: assert_head_check
--
1.8.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT] [PATCH 2/7] The tests should not fail if log.date or log.decorate are set.
From: Per Cederqvist ced...@opera.com Date: Mon, 30 Apr 2012 12:25:49 +0200 Explicitly set log.date and log.decorate to their Git default values, so that git produces the expected output even if log.date and log.decorate are set by the user in his .gitconfig. Signed-off-by: Per Cederqvist ced...@opera.com Acked-by: Jeff Sipek jef...@josefsipek.net Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- regression/scaffold | 4 1 file changed, 4 insertions(+) diff --git a/regression/scaffold b/regression/scaffold index 8769058..9db79a9 100644 --- a/regression/scaffold +++ b/regression/scaffold @@ -82,6 +82,10 @@ function setup_git_repo git add def git commit -s -m initial 2 /dev/null /dev/null # the commit should be d4850419ccc1146c7169f500725ce504b9774ed0 + + # Explicitly set config that the tests rely on. + git config log.date default + git config log.decorate no } function setup_guilt_repo -- 1.8.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT] [PATCH 3/7] Testsuite: get rid of Broken pipe errors from yes.
From: Per Cederqvist ced...@opera.com Date: Mon, 30 Apr 2012 12:27:21 +0200 Signed-off-by: Per Cederqvist ced...@opera.com Acked-by: Jeff Sipek jef...@josefsipek.net Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- regression/t-029.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/regression/t-029.sh b/regression/t-029.sh index 83e1d2b..09450c6 100755 --- a/regression/t-029.sh +++ b/regression/t-029.sh @@ -21,11 +21,11 @@ echo | shouldfail guilt repair --full cmd list_files -yes n | shouldfail guilt repair --full +yes n 2/dev/null | shouldfail guilt repair --full cmd list_files -yes y | cmd guilt repair --full +yes y 2/dev/null | cmd guilt repair --full cmd list_files @@ -33,6 +33,6 @@ cmd guilt push -a cmd list_files -yes Y | cmd guilt repair --full +yes Y 2/dev/null | cmd guilt repair --full cmd list_files -- 1.8.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT] [PATCH 4/7] Handle empty patches and patches with only a header.
From: Per Cederqvist ced...@opera.com Date: Mon, 30 Apr 2012 12:29:55 +0200 git apply --numstat in Git 1.7.10 gives an error message unless the patch contains a diff, so don't attempt to apply it unless we find a '^diff'. Signed-off-by: Per Cederqvist ced...@opera.com Acked-by: Jeff Sipek jef...@josefsipek.net Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- guilt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guilt b/guilt index 7f6806e..5bcc498 100755 --- a/guilt +++ b/guilt @@ -611,7 +611,7 @@ push_patch() cd_to_toplevel # apply the patch if and only if there is something to apply - if [ `git apply --numstat $p | wc -l` -gt 0 ]; then + if grep -q '^diff ' $p [ `git apply --numstat $p | wc -l` -gt 0 ]; then if [ $bail_action = abort ]; then reject= fi -- 1.8.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT] [PATCH 5/7] Fix fatal guilt graph error in sha1sum invocation.
From: Per Cederqvist ced...@opera.com
Date: Wed, 14 Mar 2012 14:34:38 +0100
Fix the compatibility function sha1 so that it reads from
stdin (and not a file with a zero-length file name) when
no argument is supplied.
[jn: adapted to also handle newer versions of OpenSSL,
based on reports from Andreas Schwab and John Szakmeister.
$ openssl dgst -sha1/dev/null
da39a3ee5e6b4b0d3255bfef95601890afd80709
$ openssl version
OpenSSL 0.9.8o 01 Jun 2010
$ openssl dgst -sha1 /dev/null
(stdin)= da39a3ee5e6b4b0d3255bfef95601890afd80709
$ openssl version
OpenSSL 1.0.0d 8 Feb 2011
]
Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
os.Darwin | 7 ++-
os.Linux | 7 ++-
os.SunOS | 7 ++-
3 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/os.Darwin b/os.Darwin
index 470f5fb..3f23121 100644
--- a/os.Darwin
+++ b/os.Darwin
@@ -27,7 +27,12 @@ head_n()
# usage: sha1 [file]
sha1()
{
- openssl dgst -sha1 $1 | sed s,SHA1.\(.*\).= \(.*\),\2 \1,
+ if [ $# = 1 ]
+ then
+ openssl dgst -sha1 $1 | sed s,SHA1.\(.*\).= \(.*\),\2 \1,
+ else
+ openssl dgst -sha1 | sed 's,\(.*= \)*\(.*\),\2 -,'
+ fi
}
# usage: cp_a src dst
diff --git a/os.Linux b/os.Linux
index 30b9cb0..aaebf88 100644
--- a/os.Linux
+++ b/os.Linux
@@ -30,7 +30,12 @@ head_n()
# usage: sha1 [file]
sha1()
{
- sha1sum $1
+ if [ $# = 1 ]
+ then
+ sha1sum $1
+ else
+ sha1sum
+ fi
}
# usage: cp_a src dst
diff --git a/os.SunOS b/os.SunOS
index 30b9cb0..aaebf88 100644
--- a/os.SunOS
+++ b/os.SunOS
@@ -30,7 +30,12 @@ head_n()
# usage: sha1 [file]
sha1()
{
- sha1sum $1
+ if [ $# = 1 ]
+ then
+ sha1sum $1
+ else
+ sha1sum
+ fi
}
# usage: cp_a src dst
--
1.8.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT] [PATCH 6/7] Change git branch when patches are applied.
From: Per Cederqvist ced...@opera.com Date: Mon, 30 Apr 2012 12:24:06 +0200 Apply patches on a separate branch. The separate branch is automatically created when Guilt pushes something, and removed when no patches are applied. The name is formed by prepending guilt/ to the original branch. This breaks the upstream relationship, so a mistaken git push while patches are applied will no longer mess up your upstream repository. Update the testsuite and documentation. Thanks to Junio C Hamano for suggesting this solution to my problem. Signed-off-by: Per Cederqvist ced...@opera.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- This one was a bit experimental if I remember correctly, so you may want to skip it or make the feature optional. Documentation/guilt.7 | 4 + guilt | 52 +- guilt-branch | 12 +- guilt-commit | 7 + guilt-import-commit | 4 +- guilt-repair | 7 +- regression/scaffold | 3 +- regression/t-052.out | 24 +-- regression/t-052.sh | 7 +- regression/t-061.out | 468 ++ regression/t-061.sh | 148 11 files changed, 713 insertions(+), 23 deletions(-) create mode 100644 regression/t-061.out create mode 100755 regression/t-061.sh diff --git a/Documentation/guilt.7 b/Documentation/guilt.7 index 860e6d6..c3fdb3d 100644 --- a/Documentation/guilt.7 +++ b/Documentation/guilt.7 @@ -43,6 +43,10 @@ guards: This file contains any guards that should be applied to the series when series: This file contains a list of all the patch filenames relative to the per\-branch patch directory\. Empty and commented out lines are ignored\. status: This file contains the state of the stack\. What patches are applied\. +.SH BRANCH USAGE +When you have pushed a patch, Guilt automatically changes to a freshly created Git branch\. The name of the new branch is formed by prepending \fBguilt/\fR to the name of the original branch\. This is done so that you do not accidentally push a set of Guilt patches to a remote Git repository\. Once you pop all patches Guilt automatically changes back to the original branch\. + +This is mostly transparent\. The only thing you need to remember is that if you use \fBgit checkout\fR to switch to a branch while you have Guilt patches applied, you should use \fBgit checkout guilt/BRANCH\fR instead of \fBgit checkout BRANCH\fR when you want to change back later. .SH HOOKS Any guilt operation may execute zero or more hook scripts which can be used to run any housekeeping commands or even abort the execution of the command\. .SH HOOKS DIRECTORY diff --git a/guilt b/guilt index 5bcc498..66a671a 100755 --- a/guilt +++ b/guilt @@ -408,9 +408,9 @@ head_check() return 0 ;; esac - if [ `git rev-parse refs/heads/$branch` != `git rev-parse $1` ]; then + if [ `git rev-parse refs/heads/\`git_branch\`` != `git rev-parse $1` ]; then disp Expected HEAD commit $1 2 - disp got `git rev-parse refs/heads/$branch` 2 + disp got `git rev-parse refs/heads/\`git_branch\`` 2 return 1 fi return 0 @@ -500,6 +500,11 @@ pop_many_patches() n=`expr $n - $2` head_n $n $applied $applied.tmp mv $applied.tmp $applied + if [ -z `get_top 2/dev/null` ] [ `git symbolic-ref HEAD` = refs/heads/$GUILT_PREFIX$branch ] ! $old_style_prefix + then + git symbolic-ref HEAD refs/heads/$branch + git update-ref -d refs/heads/$GUILT_PREFIX$branch + fi ) } @@ -585,7 +590,13 @@ commit() # commit treeish=`git write-tree` commitish=`git commit-tree $treeish -p $2 $TMP_MSG` - git update-ref HEAD $commitish + if $old_style_prefix || git rev-parse --verify --quiet refs/heads/$GUILT_PREFIX$branch /dev/null + then + git update-ref HEAD $commitish + else + git branch $GUILT_PREFIX$branch $commitish + git symbolic-ref HEAD refs/heads/$GUILT_PREFIX$branch + fi # mark patch as applied git update-ref refs/patches/$branch/$pname HEAD @@ -825,6 +836,9 @@ guilt_push_diff_context=1 # default diffstat value: true or false DIFFSTAT_DEFAULT=false +# Prefix for guilt branches. +GUILT_PREFIX=guilt/ + # # Parse any part of .git/config that belongs to us # @@ -839,7 +853,28 @@ diffstat=`git config --bool guilt.diffstat` GUILT_DIR=$GIT_DIR/patches -branch=`get_branch` +# To make it harder to accidentally do git push with a guilt patch +# applied, guilt push changes branch from e.g. master to +# guilt/master. Set $git_branch to the full branch name, and +# $branch to the
[GUILT] [PATCH 7/7] Drop unneeded git version check.
Git's compatibility record is pretty good, so there's no need to worry that newer versions of git will break the git config command. Without this change, guilt errors out for git 1.8. With it, all tests pass. Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Thanks for reading. guilt | 11 --- 1 file changed, 11 deletions(-) diff --git a/guilt b/guilt index 66a671a..6cb43e3 100755 --- a/guilt +++ b/guilt @@ -26,17 +26,6 @@ SUBDIRECTORY_OK=1 . $(git --exec-path)/git-sh-setup # -# Git version check -# -gitver=`git --version | cut -d' ' -f3 | sed -e 's/^debian\.//'` -case $gitver in - 1.5.*) ;; # git config - 1.6.*) ;; # git config - 1.7.*) ;; # git config - *) die Unsupported version of git ($gitver) ;; -esac - -# # Shell library # usage() -- 1.8.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] attr: fix off-by-one directory component length calculation
Duy Nguyen pclo...@gmail.com writes:
On Wed, Jan 16, 2013 at 08:08:03AM +0700, Duy Nguyen wrote:
Actually I'd like to remove that function.
This is what I had in mind:
I think the replacement logic to find the basename is moderately
inferiour to the original. For one thing (this may be somewhat
subjective), it is less readable now. Also the original only
scanned the string from the beginning once (instead of letting
strlen() to scan once and go back).
The new code structure to inline the basename finding part and to
pass the dirlen down the callchain may make sense, though.
-- 8 --
Subject: [PATCH] attr: avoid calling find_basename() twice per path
find_basename() is only used inside collect_all_attrs(), called once
in prepare_attr_stack, then again after prepare_attr_stack()
returns. Both calls return exact same value. Reorder the code to do it
once.
While at it, make use of pathlen to stop searching early if
possible.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
attr.c | 46 +++---
1 file changed, 19 insertions(+), 27 deletions(-)
diff --git a/attr.c b/attr.c
index cfc6748..04cb9a0 100644
--- a/attr.c
+++ b/attr.c
@@ -564,32 +564,12 @@ static void bootstrap_attr_stack(void)
attr_stack = elem;
}
-static const char *find_basename(const char *path)
-{
- const char *cp, *last_slash = NULL;
-
- for (cp = path; *cp; cp++) {
- if (*cp == '/' cp[1])
- last_slash = cp;
- }
- return last_slash ? last_slash + 1 : path;
-}
-
-static void prepare_attr_stack(const char *path)
+static void prepare_attr_stack(const char *path, int dirlen)
{
struct attr_stack *elem, *info;
- int dirlen, len;
+ int len;
const char *cp;
- dirlen = find_basename(path) - path;
-
- /*
- * find_basename() includes the trailing slash, but we do
- * _not_ want it.
- */
- if (dirlen)
- dirlen--;
-
/*
* At the bottom of the attribute stack is the built-in
* set of attribute definitions, followed by the contents
@@ -769,15 +749,27 @@ static int macroexpand_one(int attr_nr, int rem)
static void collect_all_attrs(const char *path)
{
struct attr_stack *stk;
- int i, pathlen, rem;
- const char *basename;
+ int i, pathlen, rem, dirlen = 0;
+ const char *basename = path, *cp;
- prepare_attr_stack(path);
+ pathlen = strlen(path);
+
+ /*
+ * This loop is similar to strrchr(path, '/') except that the
+ * trailing slash is skipped.
+ */
+ for (cp = path + pathlen - 2; cp = path; cp--) {
+ if (*cp == '/') {
+ basename = cp + 1;
+ dirlen = cp - path;
+ break;
+ }
+ }
+
+ prepare_attr_stack(path, dirlen);
for (i = 0; i attr_nr; i++)
check_all_attr[i].value = ATTR__UNKNOWN;
- basename = find_basename(path);
- pathlen = strlen(path);
rem = attr_nr;
for (stk = attr_stack; 0 rem stk; stk = stk-prev)
rem = fill(path, pathlen, basename, stk, rem);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jan 2013, #06; Mon, 14)
Adam Spiers g...@adamspiers.org writes: On Mon, Jan 14, 2013 at 10:23 PM, Junio C Hamano gits...@pobox.com wrote: * as/check-ignore (2013-01-10) 12 commits (merged to 'next' on 2013-01-14 at 9df2afc) + t0008: avoid brace expansion + add git-check-ignore sub-command + setup.c: document get_pathspec() + add.c: extract new die_if_path_beyond_symlink() for reuse + add.c: extract check_path_for_gitlink() from treat_gitlinks() for reuse + pathspec.c: rename newly public functions for clarity + add.c: move pathspec matchers into new pathspec.c for reuse + add.c: remove unused argument from validate_pathspec() + dir.c: improve docs for match_pathspec() and match_pathspec_depth() + dir.c: provide clear_directory() for reclaiming dir_struct memory + dir.c: keep track of where patterns came from + dir.c: use a single struct exclude_list per source of excludes Add a new command git check-ignore for debugging .gitignore files. The above is v4 plus the t0008: avoid brace expansion fix. v4 is slightly outdated and not quite the right version to merge to 'next'. Sigh. The What's cooking is a report of what _has_ already happened. I would have appreciated if you said the above _before_ this happened. I'll post a v5 re-roll as per: Now the series is in 'next', it is too late to _replace_ it X-. Could you instead make an incremental updates on top? That way, we do not have to re-review the whole thing; we only need to review the changes relative to the old one, making sure that the fixes in the updates are better than the v4 version. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
