Git development
 help / color / mirror / Atom feed
* [PATCH 0/8] Add function strbuf_addstr_xml_quoted() and more
From: Michael Haggerty @ 2012-11-25 11:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeremy White, Johannes Schindelin, Michael Haggerty

There were two functions doing almost the same XML quoting of
character entities, so implement a library function
strbuf_addstr_xml_quoted() and use that in both places.

Along the way, do a lot of simplification within imap-send.c, which
was doing a lot of its own string management instead of using strbuf.

Please note that "git imap-send" is utterly absent from the test
suite, probably due to the difficulty of testing without a real IMAP
server.  I ran some manual tests after my changes and didn't find any
problems.

The bug that I reported on 2012-11-12, namely that

    git format-patch --signoff --stdout --attach origin | git imap-send

is broken, is not addressed by these patches.

Michael Haggerty (8):
  Add new function strbuf_add_xml_quoted()
  xml_entities(): use function strbuf_addstr_xml_quoted()
  lf_to_crlf(): NUL-terminate msg_data::data
  imap-send: store all_msgs as a strbuf
  imap-send: correctly report errors reading from stdin
  imap-send: change msg_data from storing (char *, len) to storing
    strbuf
  wrap_in_html(): use strbuf_addstr_xml_quoted()
  wrap_in_html(): process message in bulk rather than line-by-line

 http-push.c |  23 +--------
 imap-send.c | 157 +++++++++++++++++++++++++++---------------------------------
 strbuf.c    |  26 ++++++++++
 strbuf.h    |   6 +++
 4 files changed, 104 insertions(+), 108 deletions(-)

-- 
1.8.0

^ permalink raw reply

* [PATCH 1/8] Add new function strbuf_add_xml_quoted()
From: Michael Haggerty @ 2012-11-25 11:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeremy White, Johannes Schindelin, Michael Haggerty
In-Reply-To: <1353841721-16269-1-git-send-email-mhagger@alum.mit.edu>

Substantially the same code is present in http-push.c and imap-send.c,
so make a library function out of it.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 strbuf.c | 26 ++++++++++++++++++++++++++
 strbuf.h |  6 ++++++
 2 files changed, 32 insertions(+)

diff --git a/strbuf.c b/strbuf.c
index 05d0693..9a373be 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -425,6 +425,32 @@ void strbuf_add_lines(struct strbuf *out, const char *prefix,
 	strbuf_complete_line(out);
 }
 
+void strbuf_addstr_xml_quoted(struct strbuf *buf, const char *s)
+{
+	while (*s) {
+		size_t len = strcspn(s, "\"<>&");
+		strbuf_add(buf, s, len);
+		s += len;
+		switch (*s) {
+		case '"':
+			strbuf_addstr(buf, "&quot;");
+			break;
+		case '<':
+			strbuf_addstr(buf, "&lt;");
+			break;
+		case '>':
+			strbuf_addstr(buf, "&gt;");
+			break;
+		case '&':
+			strbuf_addstr(buf, "&amp;");
+			break;
+		case 0:
+			return;
+		}
+		s++;
+	}
+}
+
 static int is_rfc3986_reserved(char ch)
 {
 	switch (ch) {
diff --git a/strbuf.h b/strbuf.h
index aa386c6..ecae4e2 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -136,6 +136,12 @@ extern void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);
 
 extern void strbuf_add_lines(struct strbuf *sb, const char *prefix, const char *buf, size_t size);
 
+/*
+ * Append s to sb, with the characters '<', '>', '&' and '"' converted
+ * into XML entities.
+ */
+extern void strbuf_addstr_xml_quoted(struct strbuf *sb, const char *s);
+
 static inline void strbuf_complete_line(struct strbuf *sb)
 {
 	if (sb->len && sb->buf[sb->len - 1] != '\n')
-- 
1.8.0

^ permalink raw reply related

* [PATCH 3/8] lf_to_crlf(): NUL-terminate msg_data::data
From: Michael Haggerty @ 2012-11-25 11:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeremy White, Johannes Schindelin, Michael Haggerty
In-Reply-To: <1353841721-16269-1-git-send-email-mhagger@alum.mit.edu>

Through the rest of the file, the data member of struct msg_data is
kept NUL-terminated, and that fact is relied upon in a couple of
places.  Change lf_to_crlf() to preserve this invariant.

In fact, there are no execution paths in which lf_to_crlf() is called
and then its data member is required to be NUL-terminated, but it is
better to be consistent to prevent future confusion.

Document the invariant in the struct msg_data definition.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 imap-send.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/imap-send.c b/imap-send.c
index d42e471..c818b0c 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -69,8 +69,12 @@ struct store {
 };
 
 struct msg_data {
+	/* NUL-terminated data: */
 	char *data;
+
+	/* length of data (not including NUL): */
 	int len;
+
 	unsigned char flags;
 };
 
@@ -1276,7 +1280,7 @@ static void lf_to_crlf(struct msg_data *msg)
 			lfnum++;
 	}
 
-	new = xmalloc(msg->len + lfnum);
+	new = xmalloc(msg->len + lfnum + 1);
 	if (msg->data[0] == '\n') {
 		new[0] = '\r';
 		new[1] = '\n';
@@ -1297,6 +1301,7 @@ static void lf_to_crlf(struct msg_data *msg)
 		/* otherwise it already had CR before */
 		new[j++] = '\n';
 	}
+	new[j] = '\0';
 	msg->len += lfnum;
 	free(msg->data);
 	msg->data = new;
-- 
1.8.0

^ permalink raw reply related

* [PATCH 2/8] xml_entities(): use function strbuf_addstr_xml_quoted()
From: Michael Haggerty @ 2012-11-25 11:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeremy White, Johannes Schindelin, Michael Haggerty
In-Reply-To: <1353841721-16269-1-git-send-email-mhagger@alum.mit.edu>


Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 http-push.c | 23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/http-push.c b/http-push.c
index 8701c12..9923441 100644
--- a/http-push.c
+++ b/http-push.c
@@ -172,28 +172,7 @@ enum dav_header_flag {
 static char *xml_entities(const char *s)
 {
 	struct strbuf buf = STRBUF_INIT;
-	while (*s) {
-		size_t len = strcspn(s, "\"<>&");
-		strbuf_add(&buf, s, len);
-		s += len;
-		switch (*s) {
-		case '"':
-			strbuf_addstr(&buf, "&quot;");
-			break;
-		case '<':
-			strbuf_addstr(&buf, "&lt;");
-			break;
-		case '>':
-			strbuf_addstr(&buf, "&gt;");
-			break;
-		case '&':
-			strbuf_addstr(&buf, "&amp;");
-			break;
-		case 0:
-			return strbuf_detach(&buf, NULL);
-		}
-		s++;
-	}
+	strbuf_addstr_xml_quoted(&buf, s);
 	return strbuf_detach(&buf, NULL);
 }
 
-- 
1.8.0

^ permalink raw reply related

* [PATCH 5/8] imap-send: correctly report errors reading from stdin
From: Michael Haggerty @ 2012-11-25 11:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeremy White, Johannes Schindelin, Michael Haggerty
In-Reply-To: <1353841721-16269-1-git-send-email-mhagger@alum.mit.edu>

Previously, read_message() didn't distinguish between an error and eof
when reading its input.  This could have resulted in incorrect
behavior if there was an error: (1) reporting "nothing to send" if no
bytes were read or (2) sending an incomplete message if some bytes
were read before the error.

Change read_message() to return -1 on ferror()s and 0 on success, so
that the caller can recognize that an error occurred.  (The return
value used to be the length of the input read, which was redundant
because that is already available as the strbuf length.

Change the caller to report errors correctly.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 imap-send.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 50e223a..86cf603 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1398,7 +1398,7 @@ static int read_message(FILE *f, struct strbuf *all_msgs)
 			break;
 	} while (!feof(f));
 
-	return all_msgs->len;
+	return ferror(f) ? -1 : 0;
 }
 
 static int count_messages(struct strbuf *all_msgs)
@@ -1537,7 +1537,12 @@ int main(int argc, char **argv)
 	}
 
 	/* read the messages */
-	if (!read_message(stdin, &all_msgs)) {
+	if (read_message(stdin, &all_msgs)) {
+		fprintf(stderr, "error reading input\n");
+		return 1;
+	}
+
+	if (all_msgs.len == 0) {
 		fprintf(stderr, "nothing to send\n");
 		return 1;
 	}
-- 
1.8.0

^ permalink raw reply related

* [PATCH 4/8] imap-send: store all_msgs as a strbuf
From: Michael Haggerty @ 2012-11-25 11:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeremy White, Johannes Schindelin, Michael Haggerty
In-Reply-To: <1353841721-16269-1-git-send-email-mhagger@alum.mit.edu>

all_msgs is only used as a glorified string, therefore there is no
reason to declare it as a struct msg_data.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 imap-send.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index c818b0c..50e223a 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1391,26 +1391,20 @@ static void wrap_in_html(struct msg_data *msg)
 
 #define CHUNKSIZE 0x1000
 
-static int read_message(FILE *f, struct msg_data *msg)
+static int read_message(FILE *f, struct strbuf *all_msgs)
 {
-	struct strbuf buf = STRBUF_INIT;
-
-	memset(msg, 0, sizeof(*msg));
-
 	do {
-		if (strbuf_fread(&buf, CHUNKSIZE, f) <= 0)
+		if (strbuf_fread(all_msgs, CHUNKSIZE, f) <= 0)
 			break;
 	} while (!feof(f));
 
-	msg->len  = buf.len;
-	msg->data = strbuf_detach(&buf, NULL);
-	return msg->len;
+	return all_msgs->len;
 }
 
-static int count_messages(struct msg_data *msg)
+static int count_messages(struct strbuf *all_msgs)
 {
 	int count = 0;
-	char *p = msg->data;
+	char *p = all_msgs->buf;
 
 	while (1) {
 		if (!prefixcmp(p, "From ")) {
@@ -1431,7 +1425,7 @@ static int count_messages(struct msg_data *msg)
 	return count;
 }
 
-static int split_msg(struct msg_data *all_msgs, struct msg_data *msg, int *ofs)
+static int split_msg(struct strbuf *all_msgs, struct msg_data *msg, int *ofs)
 {
 	char *p, *data;
 
@@ -1439,7 +1433,7 @@ static int split_msg(struct msg_data *all_msgs, struct msg_data *msg, int *ofs)
 	if (*ofs >= all_msgs->len)
 		return 0;
 
-	data = &all_msgs->data[*ofs];
+	data = &all_msgs->buf[*ofs];
 	msg->len = all_msgs->len - *ofs;
 
 	if (msg->len < 5 || prefixcmp(data, "From "))
@@ -1509,7 +1503,8 @@ static int git_imap_config(const char *key, const char *val, void *cb)
 
 int main(int argc, char **argv)
 {
-	struct msg_data all_msgs, msg;
+	struct strbuf all_msgs = STRBUF_INIT;
+	struct msg_data msg;
 	struct store *ctx = NULL;
 	int ofs = 0;
 	int r;
-- 
1.8.0

^ permalink raw reply related

* [PATCH 6/8] imap-send: change msg_data from storing (char *, len) to storing strbuf
From: Michael Haggerty @ 2012-11-25 11:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeremy White, Johannes Schindelin, Michael Haggerty
In-Reply-To: <1353841721-16269-1-git-send-email-mhagger@alum.mit.edu>

struct msg_data stored (char *, len) of the data to be included in a
message, kept the character data NUL-terminated, etc., much like a
strbuf would do.  So change it to use a struct strbuf.  This makes the
code clearer and reduces copying a little bit.

A side effect of this change is that the memory for each message is
freed after it is used rather than leaked, though that detail is
unimportant given that imap-send is a top-level command.

--

For some reason, there is a bunch of infrastructure in this file for
dealing with IMAP flags, although there is nothing in the code that
actually allows any flags to be set.  If there is no plan to add
support for flags in the future, a bunch of code could be ripped out
and "struct msg_data" could be completely replaced with strbuf.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 imap-send.c | 92 +++++++++++++++++++++++++++++++------------------------------
 1 file changed, 47 insertions(+), 45 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 86cf603..a5e0e33 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -69,12 +69,7 @@ struct store {
 };
 
 struct msg_data {
-	/* NUL-terminated data: */
-	char *data;
-
-	/* length of data (not including NUL): */
-	int len;
-
+	struct strbuf data;
 	unsigned char flags;
 };
 
@@ -1268,46 +1263,49 @@ static int imap_make_flags(int flags, char *buf)
 	return d;
 }
 
-static void lf_to_crlf(struct msg_data *msg)
+static void lf_to_crlf(struct strbuf *msg)
 {
+	size_t new_len;
 	char *new;
 	int i, j, lfnum = 0;
 
-	if (msg->data[0] == '\n')
+	if (msg->buf[0] == '\n')
 		lfnum++;
 	for (i = 1; i < msg->len; i++) {
-		if (msg->data[i - 1] != '\r' && msg->data[i] == '\n')
+		if (msg->buf[i - 1] != '\r' && msg->buf[i] == '\n')
 			lfnum++;
 	}
 
-	new = xmalloc(msg->len + lfnum + 1);
-	if (msg->data[0] == '\n') {
+	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->data[0];
+		new[0] = msg->buf[0];
 		i = 1;
 		j = 1;
 	}
 	for ( ; i < msg->len; i++) {
-		if (msg->data[i] != '\n') {
-			new[j++] = msg->data[i];
+		if (msg->buf[i] != '\n') {
+			new[j++] = msg->buf[i];
 			continue;
 		}
-		if (msg->data[i - 1] != '\r')
+		if (msg->buf[i - 1] != '\r')
 			new[j++] = '\r';
 		/* otherwise it already had CR before */
 		new[j++] = '\n';
 	}
-	new[j] = '\0';
-	msg->len += lfnum;
-	free(msg->data);
-	msg->data = new;
+	strbuf_attach(msg, new, new_len, new_len + 1);
 }
 
-static int imap_store_msg(struct store *gctx, struct msg_data *data)
+/*
+ * 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)
 {
 	struct imap_store *ctx = (struct imap_store *)gctx;
 	struct imap *imap = ctx->imap;
@@ -1316,16 +1314,15 @@ static int imap_store_msg(struct store *gctx, struct msg_data *data)
 	int ret, d;
 	char flagstr[128];
 
-	lf_to_crlf(data);
+	lf_to_crlf(&msg->data);
 	memset(&cb, 0, sizeof(cb));
 
-	cb.dlen = data->len;
-	cb.data = xmalloc(cb.dlen);
-	memcpy(cb.data, data->data, data->len);
+	cb.dlen = msg->data.len;
+	cb.data = strbuf_detach(&msg->data, NULL);
 
 	d = 0;
-	if (data->flags) {
-		d = imap_make_flags(data->flags, flagstr);
+	if (msg->flags) {
+		d = imap_make_flags(msg->flags, flagstr);
 		flagstr[d++] = ' ';
 	}
 	flagstr[d] = 0;
@@ -1356,7 +1353,8 @@ static void encode_html_chars(struct strbuf *p)
 			strbuf_splice(p, i, 1, "&quot;", 6);
 	}
 }
-static void wrap_in_html(struct msg_data *msg)
+
+static void wrap_in_html(struct strbuf *msg)
 {
 	struct strbuf buf = STRBUF_INIT;
 	struct strbuf **lines;
@@ -1366,9 +1364,7 @@ static void wrap_in_html(struct msg_data *msg)
 	static char *pre_close = "</pre>\n";
 	int added_header = 0;
 
-	strbuf_attach(&buf, msg->data, msg->len, msg->len);
-	lines = strbuf_split(&buf, '\n');
-	strbuf_release(&buf);
+	lines = strbuf_split(msg, '\n');
 	for (p = lines; *p; p++) {
 		if (! added_header) {
 			if ((*p)->len == 1 && *((*p)->buf) == '\n') {
@@ -1385,8 +1381,8 @@ static void wrap_in_html(struct msg_data *msg)
 	}
 	strbuf_addstr(&buf, pre_close);
 	strbuf_list_free(lines);
-	msg->len  = buf.len;
-	msg->data = strbuf_detach(&buf, NULL);
+	strbuf_release(msg);
+	*msg = buf;
 }
 
 #define CHUNKSIZE 0x1000
@@ -1425,34 +1421,39 @@ static int count_messages(struct strbuf *all_msgs)
 	return count;
 }
 
-static int split_msg(struct strbuf *all_msgs, struct msg_data *msg, int *ofs)
+/*
+ * Copy the next message from all_msgs, starting at offset *ofs, to
+ * msg.  Update *ofs to the start of the following message.  Return
+ * true iff a message was successfully copied.
+ */
+static int split_msg(struct strbuf *all_msgs, struct strbuf *msg, int *ofs)
 {
 	char *p, *data;
+	size_t len;
 
-	memset(msg, 0, sizeof *msg);
 	if (*ofs >= all_msgs->len)
 		return 0;
 
 	data = &all_msgs->buf[*ofs];
-	msg->len = all_msgs->len - *ofs;
+	len = all_msgs->len - *ofs;
 
-	if (msg->len < 5 || prefixcmp(data, "From "))
+	if (len < 5 || prefixcmp(data, "From "))
 		return 0;
 
 	p = strchr(data, '\n');
 	if (p) {
-		p = &p[1];
-		msg->len -= p-data;
-		*ofs += p-data;
+		p++;
+		len -= p - data;
+		*ofs += p - data;
 		data = p;
 	}
 
 	p = strstr(data, "\nFrom ");
 	if (p)
-		msg->len = &p[1] - data;
+		len = &p[1] - data;
 
-	msg->data = xmemdupz(data, msg->len);
-	*ofs += msg->len;
+	strbuf_add(msg, data, len);
+	*ofs += len;
 	return 1;
 }
 
@@ -1504,7 +1505,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;
+	struct msg_data msg = {STRBUF_INIT, 0};
 	struct store *ctx = NULL;
 	int ofs = 0;
 	int r;
@@ -1564,11 +1565,12 @@ int main(int argc, char **argv)
 	ctx->name = imap_folder;
 	while (1) {
 		unsigned percent = n * 100 / total;
+
 		fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
-		if (!split_msg(&all_msgs, &msg, &ofs))
+		if (!split_msg(&all_msgs, &msg.data, &ofs))
 			break;
 		if (server.use_html)
-			wrap_in_html(&msg);
+			wrap_in_html(&msg.data);
 		r = imap_store_msg(ctx, &msg);
 		if (r != DRV_OK)
 			break;
-- 
1.8.0

^ permalink raw reply related

* [PATCH 7/8] wrap_in_html(): use strbuf_addstr_xml_quoted()
From: Michael Haggerty @ 2012-11-25 11:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeremy White, Johannes Schindelin, Michael Haggerty
In-Reply-To: <1353841721-16269-1-git-send-email-mhagger@alum.mit.edu>

Use the new function to quote characters as they are being added to
buf, rather than quoting them in *p and then copying them into buf.
This increases code sharing, and changes the algorithm from O(N^2) to
O(N) in the number of characters in a line.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 imap-send.c | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index a5e0e33..b73c913 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1339,21 +1339,6 @@ static int imap_store_msg(struct store *gctx, struct msg_data *msg)
 	return DRV_OK;
 }
 
-static void encode_html_chars(struct strbuf *p)
-{
-	int i;
-	for (i = 0; i < p->len; i++) {
-		if (p->buf[i] == '&')
-			strbuf_splice(p, i, 1, "&amp;", 5);
-		if (p->buf[i] == '<')
-			strbuf_splice(p, i, 1, "&lt;", 4);
-		if (p->buf[i] == '>')
-			strbuf_splice(p, i, 1, "&gt;", 4);
-		if (p->buf[i] == '"')
-			strbuf_splice(p, i, 1, "&quot;", 6);
-	}
-}
-
 static void wrap_in_html(struct strbuf *msg)
 {
 	struct strbuf buf = STRBUF_INIT;
@@ -1372,12 +1357,12 @@ static void wrap_in_html(struct strbuf *msg)
 				strbuf_addbuf(&buf, *p);
 				strbuf_addstr(&buf, pre_open);
 				added_header = 1;
-				continue;
+			} else {
+				strbuf_addbuf(&buf, *p);
 			}
+		} else {
+			strbuf_addstr_xml_quoted(&buf, (*p)->buf);
 		}
-		else
-			encode_html_chars(*p);
-		strbuf_addbuf(&buf, *p);
 	}
 	strbuf_addstr(&buf, pre_close);
 	strbuf_list_free(lines);
-- 
1.8.0

^ permalink raw reply related

* Re: Python extension commands in git - request for policy change
From: Felipe Contreras @ 2012-11-25 11:19 UTC (permalink / raw)
  To: esr; +Cc: git
In-Reply-To: <20121125095356.GA22279@thyrsus.com>

On Sun, Nov 25, 2012 at 10:53 AM, Eric S. Raymond <esr@thyrsus.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com>:
>> If your friends jump off a bridge, would you? Yes, using python has
>> served them well, but as opposed to what? Other scripting languages? I
>> don't think so.
>
> The competition that Python won was *precisely* against other scripting
> languages, notably shell and Perl.  Both used to be much more heavily
> used in system scripting than they are now.

Against shell and perl yes, not against the rest.

>> What if my extension only supports python 2.7? Or what if my extension
>> wants to support 2.0?
>
> I propose that if 2.6 can't support it, then that should be considered
> grounds to reject it.

Seems sensible, but I don't know what "rejection" would actually mean.
My "extensions" are on the way to the contrib area. Is the contrib
area supposed to have different rules? I don't know.

Either way, making a script work on python 2.6 is probably easier than
trying to "reject" it.

>> Yes, they should _if_ they know what version they need. In my
>> extensions I really have no idea.
>
> Then you shouldn't submit those extensions to be folded into core git.

Too late.

>> > 3) We should be unconditionally be encouraging extensions to move
>> > from shell and Perl to Python.  This would be a clear net gain is
>> > portability and maintainability.
>>
>> NO! It's up to the developer to choose what language to use,
>
> I agree.  You seem to be raising a lot of straw men.  'Encouragement'
> does not equate to beating anyone who makes an unpopular choice over
> the head.

I don't see what this means in practical terms. People are going to
write code in whatever language they want to write code in. How
exactly are "we" going to "encourage" them not to do that is not
entirely clear to me.

I don't think there's such a thing as "git leadership" that would be
able to take these policy decisions, and if there was one, I don't
think the evidence presented would be enough to weigh in either way.

> I am also not suggesting that the whole git core ought to be hoicked
> over to Python.  I was thinking mainly about extension subcommands,
> not what's in libgit now.

Subcommands are also probably more efficient in c. And lets remember
that most people use git through the *official* subcommands.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Nguyen Thai Ngoc Duy @ 2012-11-25 11:25 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Felipe Contreras, Eric S. Raymond, git
In-Reply-To: <50B1F684.5020805@alum.mit.edu>

On Sun, Nov 25, 2012 at 5:44 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> On the contrary, there is *constant* traffic on the mailing list about
> incompatibilities between different shell implementations (sh, dash,
> bash, etc), not to mention those in other utilities (sed, grep, etc)
> that one is forced to work with in shell scripts.  Compatibility is a
> *huge* pain when developing shell code for git.  The fact that users
> typically don't encounter such problems is due to the hard work of POSIX
> lawyers on the mailing list correcting the compatibility errors of
> mortal programmers.

I think we still are in the process of moving away from shell-based
commands (not the shell interface), just not enough man power to do it
fast. The only shell-based command with active development is
git-submodule. So most shell PITA is in the test suite.

> The most important issues to consider when imagining a future with a
> hybrid of code in C and some scripting language "X" are:
>
> * Portability: is "X" available on all platforms targeted by git, in
>   usable and mutually-compatible versions?
>
> * Startup time: Is the time to start the "X" interpreter prohibitive?
>   (On my computer, "python -c pass", which starts the Python
>   interpreter and does nothing, takes about 24ms.)  This overhead would
>   be incurred by every command that is not pure C.
>
> * Should the scripting language access the C functionality only by
>   calling pure-C executables or by dynamically or statically linking to
>   a binary module interface?  If the former, then the granularity of
>   interactions between "X" and C is necessarily coarse, and "X" cannot
>   be used to implement anything but the outermost layer of
>   functionality.  If the latter, then the way would be clear to
>   implement much more of git in "X" (and lua would also be worth
>   considering).
>
> * Learning curve for developers: how difficult is it for a typical git
>   developer to become conversant with "X", considering both (1) how
>   likely is it that the typical git developer already knows "X" and
>   (2) how straightforward and predictable is the language "X"?
>   In this category I think that Python has a huge advantage over
>   Perl, though certainly opinions will differ and Ruby would also be
>   a contender.

* We might also need an embedded language variant, like Jeff's lua
experiment. I'd be nice if "X" can also take this role.
-- 
Duy

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Felipe Contreras @ 2012-11-25 11:40 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Eric S. Raymond, git
In-Reply-To: <50B1F684.5020805@alum.mit.edu>

On Sun, Nov 25, 2012 at 11:44 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> On 11/25/2012 09:53 AM, Felipe Contreras wrote:
>> On Sun, Nov 25, 2012 at 3:44 AM, Eric S. Raymond <esr@thyrsus.com> wrote:
>>> 1) In 2012, we can specify a "floor" Python version of 2.6 (shipped in
>>> 2008) and be pretty much guaranteed it will be anywhere we want to
>>> deploy except Windows.  Windows will remain a problem because Python
>>> isn't part of the stock install, but that's an equal or worse problem
>>> for shell and Perl - and at least the Python project ships a binary
>>> installer for Windows.
>>
>> What if my extension only supports python 2.7? Or what if my extension
>> wants to support 2.0?
>
> There would obviously have to be a policy like "all Python code in core
> git must run on any Python interpreter with 2.6 <= version < 3.0", just
> as there are policies about what C and shell features are allowed.  If
> you happen to want to support earlier versions of Python, I don't see
> why anybody would stop you as long as your code also runs in the
> mandated versions.

Of course, but there are experts in C and shell around, not so many
python experts. So if somebody sneaks in a python program that makes
use of features specific to python 2.7, I doubt anybody would notice.
And if they did, I doubt that would be reason enough for rejection,
supposing that porting to 2.6 would be difficult enough.

Anyway, I think this is all guesswork.

> Of course Perl will have the same problem if Perl6 ever materializes.

It *might*, it might not be as severe.

>>> 2) Python extension commands should test the Python version on startup
>>> and die loudly but gracefully in the rare case that they don't find
>>> what they need.
>>
>> Yes, they should _if_ they know what version they need. In my
>> extensions I really have no idea.
>
> Then simply (with the help of the mailing list) ensure that your
> extensions run under 2.6 (or whatever the chosen minimum version is) and
> everything will be OK.  It is not an error to specify 2.6 as the minimum
> version even though your script happens also to run on older versions :-)

Who would do that? I don't see a lot of people.

>>> 3) We should be unconditionally be encouraging extensions to move
>>> from shell and Perl to Python.  This would be a clear net gain is
>>> portability and maintainability.
>>
>> NO! It's up to the developer to choose what language to use, and I
>> find it very chauvinist of you to say "python is better, so let's all
>> use python". So far you have listed a few advantages of python, but
>> you haven't explained so far what is wrong with shell and perl.
>
> Given that some languages are accepted in git-core and others are not,
> it's already not "up to the developer to choose what language to use".
> At best there is a short list of "blessed" languages, and the developer
> can choose among only those.

They are not because they haven't been proposed. Things change.

>> In fact, while advancing python you have made clear a problem with
>> python; the version requirements. So far I have *never* encountered a
>> problem with git because of my bash version, or my perl version. And
>> we haven't touched to the python3 mess yet. To me, those are
>> advantages of shell and perl.
>
> On the contrary, there is *constant* traffic on the mailing list about
> incompatibilities between different shell implementations (sh, dash,
> bash, etc), not to mention those in other utilities (sed, grep, etc)
> that one is forced to work with in shell scripts.  Compatibility is a
> *huge* pain when developing shell code for git.  The fact that users
> typically don't encounter such problems is due to the hard work of POSIX
> lawyers on the mailing list correcting the compatibility errors of
> mortal programmers.

*Theoretical* incompatibilities on probably obscure systems. *I* have
never seen such compatibility issues *in practice*.

>> Actually, I don't care if 'git foo' is written in perl, or shell, or
>> c; as long as it *works*. And I would hate it if 'git rebase' ever
>> told me that I need a newer version of python, or worst; that I don't
>> have python in my system (Arch Linux ships 'python2', not 'python').
>
> The configure script would locate the correct interpreter and the build
> would adjust the scripts' shebang lines, just as things are tweaked
> within Perl scripts at build time.

Arch Linux doesn't use no configure script. And what if I'm building
git myself (I've hit the issue multiple times)? Perl might have
similar issues on other systems, but not on Arch Linux; /usr/bin/perl
is there.

>>> 4) We should be encouraging C code to move to Python, too.  There's
>>> little gain in portability on this path because modern C has cleaned
>>> up its act a lot, but the drop in expected bug loads would be well
>>> worth the porting effort.  Segfaults are not your friend, and the x2 to
>>> x5 drop in line count would do very good things for long-term
>>> maintainability.
>>
>> Definitely NO! I really really doubt git in python would be able to
>> achieve the same performance as git in c, but to show me wrong, it
>> wouldn't be very difficult to run a few measurements with python
>> dulwich *if* we are even to begin considering this point.
>>
>> And are segmentation faults really that different from python's
>> exceptions? Not to the user.
>
> There is one huge difference: it C it is all too easy to write code that
> leads to a security hole due to buffer overflows and other memory
> management errors.  Code written in a scripting language are largely
> immune to such problems (except of course for any such bugs in the
> interpreter itself, but the testing of the interpreter is shared across
> many projects and users).
>
> It would be insane to rewrite performance-critical C code in any
> scripting language, but there is a huge penumbra of code that is not
> performance critical and that mutates rapidly.  Such code is much easier
> to write and maintain in a sane scripting language if the portability
> issues can be mastered.

I think git developers are perfectly able to write such a code.

> The most important issues to consider when imagining a future with a
> hybrid of code in C and some scripting language "X" are:
>
> * Portability: is "X" available on all platforms targeted by git, in
>   usable and mutually-compatible versions?
>
> * Startup time: Is the time to start the "X" interpreter prohibitive?
>   (On my computer, "python -c pass", which starts the Python
>   interpreter and does nothing, takes about 24ms.)  This overhead would
>   be incurred by every command that is not pure C.

Agree.

> * Should the scripting language access the C functionality only by
>   calling pure-C executables or by dynamically or statically linking to
>   a binary module interface?  If the former, then the granularity of
>   interactions between "X" and C is necessarily coarse, and "X" cannot
>   be used to implement anything but the outermost layer of
>   functionality.  If the latter, then the way would be clear to
>   implement much more of git in "X" (and lua would also be worth
>   considering).

I think this is very far fetched at the moment. Proposals such as
libgit2 are moving things forward, but we are pretty far from a goal
like that.

> * Learning curve for developers: how difficult is it for a typical git
>   developer to become conversant with "X", considering both (1) how
>   likely is it that the typical git developer already knows "X" and
>   (2) how straightforward and predictable is the language "X"?
>   In this category I think that Python has a huge advantage over
>   Perl, though certainly opinions will differ and Ruby would also be
>   a contender.

Right, but I have the feeling that most git developers are perfectly
familiar with C already. In order to move to something else, and all
the necessary burden of learning, or becoming more familiar with X, a
compelling argument must be put forward, and I haven't seen such an
argument.

> Personally, I regret wasting my time programming pointer arithmetic in
> git modules that are not performance-critical (and correcting bugs by
> others in these areas).  And I'm tired of having an idea to improve a
> git feature only to find that it is implemented in shell, where not even
> arrays are available.  I would therefore welcome more friendliness
> towards a decent scripting language in the git project.

Me too, if ruby was one of them.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Felipe Contreras @ 2012-11-25 11:48 UTC (permalink / raw)
  To: esr; +Cc: Nguyen Thai Ngoc Duy, git, msysGit
In-Reply-To: <20121125095429.GB22279@thyrsus.com>

On Sun, Nov 25, 2012 at 10:54 AM, Eric S. Raymond <esr@thyrsus.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com>:
>> On Sun, Nov 25, 2012 at 6:18 AM, Eric S. Raymond <esr@thyrsus.com> wrote:
>> > Nguyen Thai Ngoc Duy <pclouds@gmail.com>:
>> >> These may apply to other languages as well. Where do we draw a line?
>> >
>> > I'm in favor of the general policy of avoiding scripting languages
>> > other than the top three most widely deployed.  At the moment that
>> > means shell, Python, Perl; on present trends, in a few years Perl
>> > (dropping in popularity) might be passed by Ruby on the way up.
>>
>> Top three according to whom?
>
> According to the LOC counts in git's codebase.

Not according to ohloh:

1) shell 33%
2) tcl 9%
3) perl 9.7%

4) python 1.8%

And this is a non-sequitur; you are proposing to change git policies
based on numbers that are a direct result of git's policies?

https://www.ohloh.net/p/git/analyses/latest/languages_summary

-- 
Felipe Contreras

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: David Lang @ 2012-11-25 11:51 UTC (permalink / raw)
  To: Eric S. Raymond; +Cc: Michael Haggerty, Felipe Contreras, git
In-Reply-To: <20121125105707.GA25212@thyrsus.com>

On Sun, 25 Nov 2012, Eric S. Raymond wrote:

> Michael Haggerty <mhagger@alum.mit.edu>:
>> There is, of course, the awkward issue of how/when to transition to
>> Python 3.x, which is *not* backwards compatible with Python 2.x.  I
>> expect that when the time comes there will be volunteers (myself
>> included) willing to help adapt Python scripts to the new version, but
>> the problem shouldn't be minimized.
>
> 2to3 actually does a pretty good job.  It doesn't reduce the
> transition cost to zero, but I find it does reduce that cost to an
> easily manageable level even on quite large codebases.
>
>> It would be insane to rewrite performance-critical C code in any
>> scripting language, but there is a huge penumbra of code that is not
>> performance critical and that mutates rapidly.
>
> Indeed.  In the git architecture there is a pretty clear dividing line -
> to a first approximation, plumbing should remain C but porcelain should
> probably not.  (Not that I am advocating forcing such a move - but it would
> be good to allow it to happen.)
>
> The 80-20 rule (80% of the execution time is spent in 20% of the code)
> helps us here.  The *other* 80% of the code can move to a scripting
> language with no significant performance loss.  To find out what needs
> to stay in C, run a profiler!

Remember that old code is tested code. The mere act of re-writing it from 
scratch is likely to introduce new bugs due to 'simplifications' by the person 
re-writing the code.

If a particular piece of code has a track record of being buggy, this may be 
overwelmed by the fresh start and new attention (plus whatever theoretical 
advantage any particular language provides), but unless it's suspect, re-writing 
it for the sole reason of changing the language is unlikely to be a win.

In addition, a good programmer working in a 'bad' language that they are very 
familiar with is going to write better code than that same programmer would 
write in a 'good' language that they are not familiar with.

I git, the programmers are very familiar with C and Bash, but far less familiar 
with either Perl or Python (although from what I see, far more familiar with 
Perl than Python)

If it's something going into contrib, where the core developers are not needing 
to maintain it, the language it's written in matters far less than if it's 
something that's going to be in the core. If it's in the core, it needs to be in 
a language that the core developers are comforatable with.

You may think that C and Bash are poor choices, but that is what the community 
is familar with.

You are far from the first person to say that git should be re-written (or at 
least large portions of it) in the language-of-the-day, and you won't be the 
last (even, or especially if it does get re-written in Python ;-)

David Lang

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Stefano Lattarini @ 2012-11-25 12:01 UTC (permalink / raw)
  To: David Lang; +Cc: Eric S. Raymond, Michael Haggerty, Felipe Contreras, git
In-Reply-To: <alpine.DEB.2.02.1211250344360.32333@nftneq.ynat.uz>

Hi David.  One minor but important correction ...

On 11/25/2012 12:51 PM, David Lang wrote:
>
> You may think that C and Bash are poor choices, but that is what the
> community is familar with.
>
Actually, it is C and POSIX shell -- not merely bash.  Indeed, the shell
code in Git is expected to work with the Solaris Korn shell, the BSD
/bin/sh, the dash shell (which is now the default /bin/sh on Debian and
Ubuntu), etc.

(Oh, and on the python vs. C vs. shell diatribe I'm mostly neutral,
mostly because I'm no Git developer, and I have no "cents to throw").

Regards,
  Stefano

^ permalink raw reply

* [PATCH] Fix typo in remote set-head usage
From: Antoine Pelisse @ 2012-11-25 13:43 UTC (permalink / raw)
  To: git; +Cc: tim.henigan, Antoine Pelisse

parenthesis are not matching in `builtin_remote_sethead_usage`
as a square bracket is closing something never opened.
---
This will also break all translation files, should I also send a patch
to update them ?

Cheers,
Antoine Pelisse

 builtin/remote.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index a5a4b23..937484d 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -39,7 +39,7 @@ static const char * const builtin_remote_rm_usage[] = {
 };

 static const char * const builtin_remote_sethead_usage[] = {
-	N_("git remote set-head <name> (-a | -d | <branch>])"),
+	N_("git remote set-head <name> (-a | -d | <branch>)"),
 	NULL
 };

--
1.7.9.5

^ permalink raw reply related

* Re: [PATCH master] convert: The native line-ending is \r\n on MinGW
From: Mr_and_Mrs_D @ 2012-11-25 15:39 UTC (permalink / raw)
  To: git
In-Reply-To: <7vbp8aqtuz.fsf@alter.siamese.dyndns.org>

I am on windows 7 Pro - mingwin

I decided to turn autocrlf to false and use .gitattributes instead and was
bitten by this bug :

http://stackoverflow.com/questions/13531988/git-line-endings-renormalize-does-not-seem-to-checkout-the-right-line-endings

It took me 2 days to figure this out

Please fix



--
View this message in context: http://git.661346.n2.nabble.com/PATCH-v6-Add-core-eol-config-variable-tp5140844p7571889.html
Sent from the git mailing list archive at Nabble.com.

^ permalink raw reply

* Re: Re: Python extension commands in git - request for policy change
From: Erik Faye-Lund @ 2012-11-25 15:51 UTC (permalink / raw)
  To: esr; +Cc: Pat Thoyts, Nguyen Thai Ngoc Duy, git, msysGit
In-Reply-To: <20121125103316.GA24514@thyrsus.com>

On Sun, Nov 25, 2012 at 11:33 AM, Eric S. Raymond <esr@thyrsus.com> wrote:
> Pat Thoyts <patthoyts@gmail.com>:
>> Git for Windows simply ships everything we need to run git - so if a
>> desirable module requires a version of python, we will add that
>> version plus any required modules into the installer. We already have
>> a patch to provide python in the msysgit tree - it would just require
>> polishing up a little. I'm certain this is no problem for the other
>> windows port (cygwin) either.
>
> Thank you - I think this completely disposes of the "Windows is a blocker
> for scripting language X" argument, with the case X = Python in point.

As the one who wrote that patch; not at all. That patch is a horrible
mess, and it is not yet proven that the resulting python executable
works any more than a basic hello world.

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply

* Re: [PATCH] gitk tag delete/rename support
From: Leon KUKOVEC @ 2012-11-25 16:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Paul Mackerras
In-Reply-To: <7vzk26xkou.fsf@alter.siamese.dyndns.org>

Hi,

On Sun, Nov 25, 2012 at 7:26 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Thanks, but I prefer not to take patches to gitk-git/ directly;
> could you prepare a patch against Paul's tree at
>
>
>     git://ozlabs.org/~paulus/gitk
>
> and send it in that direction (paulus@) instead?

No problem. Will do that.

--
Best Regards,
        Leon

^ permalink raw reply

* Multiple threads of compression
From: Thorsten Glaser @ 2012-11-25 16:27 UTC (permalink / raw)
  To: git

Hi,

I’m asking here informally first, because my information relates
to a quite old version (the one from lenny-backports). A tl;dr
is at the end.

On a multi-core machine, the garbage collection of git, as well
as pack compression on the server side when someone clones a
repository remotely, the compression is normally done automatically
using multiple threads of execution.

That may be fine for your typical setups, but in my cases, I have
two scenarios where it isn’t:

ⓐ The machine where I want it to use only, say, 2 of my 4 or 8 cores
  as I’m also running some VMs on the box which eat up a lot of CPU
  and which I don’t want to slow down.

ⓑ The server VM which has been given 2 or 3 VCPUs to cope with all
  the load done by clients, but which is RAM-constrained to only
  512 or, when lucky, 768 MiB. It previously served only http/https
  and *yuk* Subversion, but now, git comes into the play, and I’ve
  seen the one server box I think about go down *HARD* because git
  ate up all RAM *and* swap when someone wanted to update their clone
  of a repository after someone else committed… well, an ~100 MiB large
  binary file they shouldn’t. (It required manual intervention on the
  server to kill that revision and then the objects coupled with it,
  but even *that* didn’t work, read on for more.)

In both cases, I had to apply a quick hack. One I can reproduce
by now is, that, on the first box, I added a --threads=2 to the
line calling git pack-objects in /usr/lib/git-core/git-repack,
like this:

   83 args="$args $local ${GIT_QUIET:+-q} $no_reuse$extra"
   84 names=$(git pack-objects --threads=2 --keep-true-parents --honor-pack-
keep --non-empty --all --reflog $arg
   85         exit 1

(By the way, wrapping source code at 80c is still way to go IMHO.)

On the second box, IIRC I added --threads=1, but that box got
subsequently upgraded from lenny to wheezy so any local modification
is lost (luckily, the problem didn’t occur again recently, or at
least I didn’t notice it, save for the VM load going up to 6-8
several times a day).

tl;dr: I would like to have a *global* option for git to restrict
the number of threads of execution it uses. Several subcommands,
like pack-objects, are already equipped with an optioin for this,
but unfortunately, these are seldom invoked by hand¹, so this can’t
work in my situations.

① automatic garbage collection, “git gc --aggressive --prune=now”,
  and cloning are the use cases I have at hand right now.

À propos, while here: is gc --aggressive safe to run on a live,
online-shared repository, or does it break other users accessing
the repository concurrently? (If it’s safe I’d very much like to do
that in a, say weekly, cronjob on FusionForge, our hosting system.)

Thanks in advance!
//mirabilos

^ permalink raw reply

* Re: Re: Python extension commands in git - request for policy change
From: Johannes Schindelin @ 2012-11-25 17:21 UTC (permalink / raw)
  To: Eric S. Raymond; +Cc: Nguyen Thai Ngoc Duy, git, msysGit
In-Reply-To: <20121125051809.GA3670@thyrsus.com>

Hi,

thank you Duy for thinking of Cc:ing the msysGit mailing list. We indeed
do not have a working Python in Git for Windows yet (mainly because I did
not review kusma's patch yet thanks to a non-fun-at-all side track).

On Sun, 25 Nov 2012, Eric S. Raymond wrote:

> Nguyen Thai Ngoc Duy <pclouds@gmail.com>:
> > These may apply to other languages as well. Where do we draw a line?
> 
> I'm in favor of the general policy of avoiding scripting languages
> other than the top three most widely deployed.

It is one thing to allow users to use the scripting languages of their
choice to do their work.

It is a different thing completely to allow the core of an important piece
of software like Git to consist of a hodge podge of languages. There are
so many problems already, both technical and social ones [*1*], that I would
really like to caution against letting even more languages creep into the
core. It is bad enough already.

Ciao,
Dscho

Footnote [*1*]: Technical problems include serious performance issues on
Windows when using shell/Perl scripting (see the many, many complaints
about git-svn just as an example), portability problems (I am thankful
that Junio seems to insist at least on POSIX compatibility of shell
scripts still even if there are very vocal forces trying to get lazy on
that front).

And do not underestimate the social problems with *requiring* contributors
to know yet another language well just because you let a core part be
written in that language. There is even a rule of thumb: increase the
number of languages used in your program == halve the number of potential
contributors. And if you think that this is theoretical: look at the mails
we got about Git GUI being written in Tcl/Tk (hardly a difficult language
to learn) and losing contributors over it.

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Eric S. Raymond @ 2012-11-25 17:32 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git
In-Reply-To: <CAMP44s1oRpm4QkhcbfAuxK8UTZnuSVfNhAQnmUd1xiwhwLEqGw@mail.gmail.com>

Felipe Contreras <felipe.contreras@gmail.com>:
> Seems sensible, but I don't know what "rejection" would actually mean.

Why is this mysterious?  We reject a patch when we don't choose to merge it.

> My "extensions" are on the way to the contrib area. Is the contrib
> area supposed to have different rules? I don't know.

I don't have a strong opinion about this.  I lean towards looser rules
for contrib because, among other things, it's a place for experiments
and we disclaim responsibility for maintaining it. But requiring 2.6
compatibility for Python scripts is not really onerous.

> Too late.

I'd be happy to help you out by auditing them for version dependencies.

> I don't see what this means in practical terms. People are going to
> write code in whatever language they want to write code in. How
> exactly are "we" going to "encourage" them not to do that is not
> entirely clear to me.

One way is by having clear guidelines for good practice that *include*
Python, and tell people exactly what the requirements are.

> Subcommands are also probably more efficient in c. And lets remember
> that most people use git through the *official* subcommands.

See my remarks on the 80-20 rule elsewhere in the thread.  Execessive
worship of "efficiency" is a great way to waste effort and pile up
hidden costs in maintainance problems.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Eric S. Raymond @ 2012-11-25 17:36 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Michael Haggerty, git
In-Reply-To: <CAMP44s0WYiV3hTE7u28_Wd59FkGfu3o_psS0gocpnibzN4--Fg@mail.gmail.com>

Felipe Contreras <felipe.contreras@gmail.com>:
> Of course, but there are experts in C and shell around, not so many
> python experts. So if somebody sneaks in a python program that makes
> use of features specific to python 2.7, I doubt anybody would notice.

I would.

> And if they did, I doubt that would be reason enough for rejection,
> supposing that porting to 2.6 would be difficult enough.

In cases like that, backporting is usually pretty easy.  Been there, done that.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Eric S. Raymond @ 2012-11-25 17:44 UTC (permalink / raw)
  To: David Lang; +Cc: Michael Haggerty, Felipe Contreras, git
In-Reply-To: <alpine.DEB.2.02.1211250344360.32333@nftneq.ynat.uz>

David Lang <david@lang.hm>:
> You may think that C and Bash are poor choices, but that is what the
> community is familar with.

I don't think C is a "poor" choice.  bash, on the other hand...so
many dependencies on tool quirks!

> You are far from the first person to say that git should be
> re-written (or at least large portions of it) in the
> language-of-the-day, and you won't be the last (even, or especially
> if it does get re-written in Python ;-)

I think you're overinterpreting.  Trying for One Big Rewrite in language
X is almost never a good idea and I don't advocate it.  Encouraging people
to migrate pieces as they feel motivated and resdy is a different matter.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Eric S. Raymond @ 2012-11-25 17:50 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Nguyen Thai Ngoc Duy, git, msysGit
In-Reply-To: <CAMP44s1cG=5D9DppHmB9CpgkgdEzM72KhQ1Q-kWrrDo8ST+r_g@mail.gmail.com>

Felipe Contreras <felipe.contreras@gmail.com>:
> Not according to ohloh:
> 
> 1) shell 33%
> 2) tcl 9%
> 3) perl 9.7%
> 
> 4) python 1.8%

Look in the Makefile - all that tcl code is buried in gitk.  We're
very, very lucky the author did such a good job, because it's a
potentially serious headache; who can maintain it?
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

^ permalink raw reply

* [PATCH] gitk tag delete/rename support
From: Leon KUKOVEC @ 2012-11-25 19:05 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras, Leon KUKOVEC
In-Reply-To: <CAOrOd9woDs16as+t-EReQ8NtXfYm9mpd0XaFFs-XL=pg+JK1Lw@mail.gmail.com>

Right clicking on a tag pops up a menu, which allows
tag to be renamed or deleted.

Signed-off-by: Leon KUKOVEC <leon.kukovec@gmail.com>
---
 gitk-git/gitk |  154 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 154 insertions(+)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index d93bd99..38cc233 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2032,6 +2032,7 @@ proc makewindow {} {
     global have_tk85 use_ttk NS
     global git_version
     global worddiff
+    global tagctxmenu
 
     # The "mc" arguments here are purely so that xgettext
     # sees the following string as needing to be translated
@@ -2581,6 +2582,13 @@ proc makewindow {} {
 	{mc "Run git gui blame on this line" command {external_blame_diff}}
     }
     $diff_menu configure -tearoff 0
+
+    set tagctxmenu .tagctxmenu
+    makemenu $tagctxmenu {
+	{mc "Rename this tag" command mvtag}
+	{mc "Delete this tag" command rmtag}
+    }
+    $tagctxmenu configure -tearoff 0
 }
 
 # Windows sends all mouse wheel events to the current focused window, not
@@ -6400,6 +6408,7 @@ proc drawtags {id x xt y1} {
 		   -font $font -tags [list tag.$id text]]
 	if {$ntags >= 0} {
 	    $canv bind $t <1> [list showtag $tag_quoted 1]
+	    $canv bind $t $ctxbut [list showtagmenu %X %Y $id $tag_quoted]
 	} elseif {$nheads >= 0} {
 	    $canv bind $t $ctxbut [list headmenu %X %Y $id $tag_quoted]
 	}
@@ -8931,6 +8940,113 @@ proc domktag {} {
     return 1
 }
 
+proc mvtag {} {
+    global mvtagtop
+    global tagmenuid tagmenutag tagctxmenu maintag NS
+    global mvtagtag
+
+    set mvtagtag $tagmenutag
+    set top .movetag
+    set mvtagtop $top
+    catch {destroy $top}
+    ttk_toplevel $top
+    make_transient $top .
+
+    ${NS}::label $top.msg -text [mc "Enter a new tag name:"]
+    ${NS}::entry $top.tag -width 60 -textvariable mvtagtag
+
+    grid $top.msg -sticky w -row 0 -column 0
+    grid $top.tag -sticky w -row 0 -column 1
+
+    ${NS}::frame $top.buts
+    ${NS}::button $top.buts.gen -text [mc "Rename"] -command mvtaggo
+    ${NS}::button $top.buts.can -text [mc "Cancel"] -command mvtagcan
+    bind $top <Key-Return> mvtaggo
+    bind $top <Key-Escape> mvtagcan
+    grid $top.buts.gen $top.buts.can
+    grid columnconfigure $top.buts 0 -weight 1 -uniform a
+    grid columnconfigure $top.buts 1 -weight 1 -uniform a
+    grid $top.buts - -pady 10 -sticky ew
+}
+
+proc domvtag {} {
+    global mvtagtop env tagids idtags tagmenutag tagmenuid mvtagtag
+
+    set tag $mvtagtag
+    set id $tagmenuid
+
+    # add tag
+    # XXX: reuse domktag including keeping comment from the original tag.
+    if {[catch {
+        exec git tag $tag $id
+    } err]} {
+        error_popup "[mc "Error renaming tag:"] $err" $mvtagtop
+        return 0
+    }
+
+    # delete old tag, content stored in $tagmenutag and $tagmenuid
+    dormtag
+
+    set tagids($tag) $id
+    lappend idtags($id) $tag
+    redrawtags $id
+    addedtag $id
+    dispneartags 0
+    run refill_reflist
+    return 1
+}
+
+proc rmtag {} {
+    global rmtagtop
+    global tagmenuid tagmenutag tagctxmenu maintag NS
+
+    set top .maketag
+    set rmtagtop $top
+    catch {destroy $top}
+    ttk_toplevel $top
+    make_transient $top .
+    ${NS}::label $top.title -text [mc "Delete tag"]
+    grid $top.title - -pady 10
+
+    ${NS}::label $top.msg -text [mc "You are about to delete a tag"]
+    ${NS}::label $top.tagname -foreground Red -text [mc "$tagmenutag"]
+    grid $top.msg -sticky w -row 0 -column 0
+    grid $top.tagname -sticky w -row 0 -column 1
+
+    ${NS}::frame $top.buts
+    ${NS}::button $top.buts.gen -text [mc "Delete"] -command rmtaggo
+    ${NS}::button $top.buts.can -text [mc "Cancel"] -command rmtagcan
+    bind $top <Key-Return> rmtaggo
+    bind $top <Key-Escape> rmtagcan
+    grid $top.buts.gen $top.buts.can
+    grid columnconfigure $top.buts 0 -weight 1 -uniform a
+    grid columnconfigure $top.buts 1 -weight 1 -uniform a
+    grid $top.buts - -pady 10 -sticky ew
+}
+
+proc dormtag {} {
+    global rmtagtop env tagids idtags tagmenutag tagmenuid
+
+    set tag $tagmenutag
+    set id $tagmenuid
+
+    if {[catch {
+        exec git tag -d $tag
+    } err]} {
+        error_popup "[mc "Error deleting tag:"] $err" $rmtagtop
+        return 0
+    }
+
+    unset tagids($tag)
+    set idx [lsearch $idtags($id) $tag]
+    set idtags($id) [lreplace $idtags($id) $idx $idx]
+
+    redrawtags $id
+    dispneartags 0
+    run refill_reflist
+    return 1
+}
+
 proc redrawtags {id} {
     global canv linehtag idpos currentid curview cmitlisted markedid
     global canvxmax iddrawn circleitem mainheadid circlecolors
@@ -8974,6 +9090,30 @@ proc mktaggo {} {
     mktagcan
 }
 
+proc rmtagcan {} {
+    global rmtagtop
+
+    catch {destroy $rmtagtop}
+    unset rmtagtop
+}
+
+proc rmtaggo {} {
+    if {![dormtag]} return
+    rmtagcan
+}
+
+proc mvtagcan {} {
+    global mvtagtop
+
+    catch {destroy $mvtagtop}
+    unset mvtagtop
+}
+
+proc mvtaggo {} {
+    if {![domvtag]} return
+    mvtagcan
+}
+
 proc writecommit {} {
     global rowmenuid wrcomtop commitinfo wrcomcmd NS
 
@@ -9288,6 +9428,20 @@ proc headmenu {x y id head} {
     tk_popup $headctxmenu $x $y
 }
 
+# context menu for a tag
+proc showtagmenu {x y id tag} {
+    global tagmenuid tagmenutag tagctxmenu maintag
+
+    stopfinding
+    set tagmenuid $id
+    set tagmenutag $tag
+    set state normal
+
+    $tagctxmenu entryconfigure 0 -state normal
+    $tagctxmenu entryconfigure 1 -state normal
+    tk_popup $tagctxmenu $x $y
+}
+
 proc cobranch {} {
     global headmenuid headmenuhead headids
     global showlocalchanges
-- 
1.7.9.5

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox