Git development
 help / color / mirror / Atom feed
* [RFC PATCH v2 16/16] Smart HTTP fetch: gzip requests
From: Shawn O. Pearce @ 2009-10-13  2:25 UTC (permalink / raw)
  To: git; +Cc: Daniel Barkalow
In-Reply-To: <1255400715-10508-1-git-send-email-spearce@spearce.org>

The upload-pack requests are mostly plain text and they compress
rather well.  Deflating them with Content-Encoding: gzip can easily
drop the size of the request by 50%, reducing the amount of data
to transfer as we negotiate the common commits.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
CC: Daniel Barkalow <barkalow@iabervon.org>
---
 remote-curl.c |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 31d1d34..d53215d 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -261,11 +261,12 @@ static size_t rpc_in(const void *ptr, size_t eltsize,
 	return size;
 }
 
-static int post_rpc(struct rpc_state *state)
+static int post_rpc(struct rpc_state *state, int use_gzip)
 {
 	struct active_request_slot *slot;
 	struct slot_results results;
 	struct curl_slist *headers = NULL;
+	unsigned char *gzip_body = NULL;
 	int err = 0, large_request = 0;
 
 	/* Try to load the entire request, if we can fit it into the
@@ -279,6 +280,7 @@ static int post_rpc(struct rpc_state *state)
 
 		if (left < LARGE_PACKET_MAX) {
 			large_request = 1;
+			use_gzip = 0;
 			break;
 		}
 
@@ -311,6 +313,48 @@ static int post_rpc(struct rpc_state *state)
 			fflush(stderr);
 		}
 
+	} else if (use_gzip && 1024 < state->len) {
+		/* The client backend isn't giving us compressed data so
+		 * we can try to deflate it ourselves, this may save on.
+		 * the transfer time.
+		 */
+		size_t size;
+		z_stream stream;
+		int ret;
+
+		memset(&stream, 0, sizeof(stream));
+		ret = deflateInit2(&stream, Z_DEFAULT_COMPRESSION,
+				Z_DEFLATED, (15 + 16),
+				8, Z_DEFAULT_STRATEGY);
+		if (ret != Z_OK)
+			die("cannot deflate request; zlib init error %d", ret);
+		size = deflateBound(&stream, state->len);
+		gzip_body = xmalloc(size);
+
+		stream.next_in = (unsigned char *)state->buf;
+		stream.avail_in = state->len;
+		stream.next_out = gzip_body;
+		stream.avail_out = size;
+
+		ret = deflate(&stream, Z_FINISH);
+		if (ret != Z_STREAM_END)
+			die("cannot deflate request; zlib deflate error %d", ret);
+
+		ret = deflateEnd(&stream);
+		if (ret != Z_OK)
+			die("cannot deflate request; zlib end error %d", ret);
+
+		size = stream.total_out;
+
+		headers = curl_slist_append(headers, "Content-Encoding: gzip");
+		curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, gzip_body);
+		curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, size);
+
+		if (state->verbose) {
+			fprintf(stderr, "POST %s (gzip %lu to %lu bytes)\n",
+				state->service_name, state->len, size);
+			fflush(stderr);
+		}
 	} else {
 		/* We know the complete request size in advance, use the
 		 * more normal Content-Length approach.
@@ -336,11 +380,13 @@ static int post_rpc(struct rpc_state *state)
 		}
 	}
 	curl_slist_free_all(headers);
+	free(gzip_body);
 	return err;
 }
 
 static int one_shot_rpc_service(const char *service,
 	int verbose,
+	int use_gzip,
 	const char **client_argv,
 	struct discovery *heads,
 	struct strbuf *result)
@@ -384,7 +430,7 @@ static int one_shot_rpc_service(const char *service,
 			break;
 		state.pos = 0;
 		state.len = n;
-		err |= post_rpc(&state);
+		err |= post_rpc(&state, use_gzip);
 	}
 	if (result)
 		strbuf_read(result, client.out, 0);
@@ -474,6 +520,7 @@ static int fetch_git(struct fetch_args *args,
 
 	err = one_shot_rpc_service("git-upload-pack",
 		args->verbose,
+		1 /* gzip request */,
 		argv, heads, &res);
 	if (res.len)
 		safe_write(1, res.buf, res.len);
@@ -611,6 +658,7 @@ static int push_git(struct discovery *heads,
 
 	err = one_shot_rpc_service("git-receive-pack",
 		args->verbose,
+		0 /* no gzip */,
 		argv, heads, &res);
 	if (res.len)
 		safe_write(1, res.buf, res.len);
-- 
1.6.5.52.g0ff2e

^ permalink raw reply related

* [RFC PATCH v2 09/16] Move WebDAV HTTP push under remote-curl
From: Shawn O. Pearce @ 2009-10-13  2:25 UTC (permalink / raw)
  To: git; +Cc: Daniel Barkalow, Tay Ray Chuan, Mike Hommey
In-Reply-To: <1255400715-10508-1-git-send-email-spearce@spearce.org>

The remote helper interface now supports the push capability,
which can be used to ask the implementation to push one or more
specs to the remote repository.  For remote-curl we implement this
by calling the existing WebDAV based git-http-push executable.

Internally the helper interface uses the push_refs transport hook
so that the complexity of the refspec parsing and matching can be
reused between remote implementations.  When possible however the
helper protocol uses source ref name rather than the source SHA-1,
thereby allowing the helper to access this name if it is useful.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
CC: Daniel Barkalow <barkalow@iabervon.org>
CC: Tay Ray Chuan <rctay89@gmail.com>
CC: Mike Hommey <mh@glandium.org>
---
 Documentation/git-remote-helpers.txt |   43 ++++++++++-
 http-push.c                          |   43 ++++++++---
 remote-curl.c                        |  101 ++++++++++++++++++++++---
 transport-helper.c                   |  140 +++++++++++++++++++++++++++++++++-
 transport.c                          |   31 --------
 5 files changed, 302 insertions(+), 56 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index 334ab30..f8234d0 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -34,6 +34,10 @@ Commands are given by the caller on the helper's standard input, one per line.
 	value of the ref. A space-separated list of attributes follows
 	the name; unrecognized attributes are ignored. After the
 	complete list, outputs a blank line.
++
+If 'push' is supported this may be called as 'list for-push'
+to obtain the current refs prior to sending one or more 'push'
+commands to the helper.
 
 'fetch' <sha1> <name>::
 	Fetches the given object, writing the necessary objects to the
@@ -53,6 +57,22 @@ Supported if the helper has the "fetch" capability.
 	file under GIT_DIR/objects/pack which is keeping a pack
 	until refs can be suitably updated.
 
+'push' +<src>:<dst>::
+	Pushes the given <src> commit or branch locally to the
+	remote branch described by <dst>.  A batch sequence of
+	one or more push commands is terminated with a blank line.
++
+Zero or more protocol options may be entered after the last 'push'
+command, before the batch's terminating blank line.
++
+When the push is complete, outputs one or more 'ok <dst>' or
+'error <dst> <why>?' lines to indicate success or failure of
+each pushed ref.  The status report output is terminated by
+a blank line.  The option field <why> may be quoted in a C
+style string if it contains an LF.
++
+Supported if the helper has the "push" capability.
+
 If a fatal error occurs, the program writes the error message to
 stderr and exits. The caller should expect that a suitable error
 message has been printed if the child closes the connection without
@@ -70,8 +90,12 @@ CAPABILITIES
 'fetch-multiple'::
 	This helper supports the 'fetch-multiple' command.
 
+'push'::
+	This helper supports the 'push' command.
+
 'option' <name>::
-	This helper supports the option <name> under fetch-multiple.
+	This helper supports the option <name> under fetch-multiple
+	and push.
 
 REF LIST ATTRIBUTES
 -------------------
@@ -100,6 +124,23 @@ To enable an option the helper must list it in 'capabilities'.
 'option thin'::
 	Transfer the data as a thin pack if possible.
 
+PUSH OPTIONS
+------------
+
+'option dry-run':
+	Pretend like the push update will take place, but don't
+	actually perform actions which would modify the state of
+	the remote side.
+
+'option verbose':
+	Be more verbose in progress output to stderr.
+
+'option thin'::
+	Transfer the data as a thin pack if possible.
+
+'option receivepack' <command>::
+	The program to use on the remote side to receive a pack.
+
 Documentation
 -------------
 Documentation by Daniel Barkalow.
diff --git a/http-push.c b/http-push.c
index 00e83dc..9010ccc 100644
--- a/http-push.c
+++ b/http-push.c
@@ -78,6 +78,7 @@ static int push_verbosely;
 static int push_all = MATCH_REFS_NONE;
 static int force_all;
 static int dry_run;
+static int helper_status;
 
 static struct object_list *objects;
 
@@ -1813,6 +1814,10 @@ int main(int argc, char **argv)
 				dry_run = 1;
 				continue;
 			}
+			if (!strcmp(arg, "--helper-status")) {
+				helper_status = 1;
+				continue;
+			}
 			if (!strcmp(arg, "--verbose")) {
 				push_verbosely = 1;
 				http_is_verbose = 1;
@@ -1941,9 +1946,14 @@ int main(int argc, char **argv)
 
 		if (is_null_sha1(ref->peer_ref->new_sha1)) {
 			if (delete_remote_branch(ref->name, 1) == -1) {
-				error("Could not remove %s", ref->name);
+				if (helper_status)
+					printf("error %s cannot remove\n", ref->name);
+				else
+					error("Could not remove %s", ref->name);
 				rc = -4;
 			}
+			else if (helper_status)
+				printf("ok %s\n", ref->name);
 			new_refs++;
 			continue;
 		}
@@ -1951,6 +1961,8 @@ int main(int argc, char **argv)
 		if (!hashcmp(ref->old_sha1, ref->peer_ref->new_sha1)) {
 			if (push_verbosely || 1)
 				fprintf(stderr, "'%s': up-to-date\n", ref->name);
+			if (helper_status)
+				printf("ok %s up to date\n", ref->name);
 			continue;
 		}
 
@@ -1968,12 +1980,15 @@ int main(int argc, char **argv)
 				 * commits at the remote end and likely
 				 * we were not up to date to begin with.
 				 */
-				error("remote '%s' is not an ancestor of\n"
-				      "local '%s'.\n"
-				      "Maybe you are not up-to-date and "
-				      "need to pull first?",
-				      ref->name,
-				      ref->peer_ref->name);
+				if (helper_status)
+					printf("error %s non-fast forward\n", ref->name);
+				else
+					error("remote '%s' is not an ancestor of\n"
+						  "local '%s'.\n"
+						  "Maybe you are not up-to-date and "
+						  "need to pull first?",
+						  ref->name,
+						  ref->peer_ref->name);
 				rc = -2;
 				continue;
 			}
@@ -1987,14 +2002,20 @@ int main(int argc, char **argv)
 		if (strcmp(ref->name, ref->peer_ref->name))
 			fprintf(stderr, " using '%s'", ref->peer_ref->name);
 		fprintf(stderr, "\n  from %s\n  to   %s\n", old_hex, new_hex);
-		if (dry_run)
+		if (dry_run) {
+			if (helper_status)
+				printf("ok %s\n", ref->name);
 			continue;
+		}
 
 		/* Lock remote branch ref */
 		ref_lock = lock_remote(ref->name, LOCK_TIME);
 		if (ref_lock == NULL) {
-			fprintf(stderr, "Unable to lock remote branch %s\n",
-				ref->name);
+			if (helper_status)
+				printf("error %s lock error\n", ref->name);
+			else
+				fprintf(stderr, "Unable to lock remote branch %s\n",
+					ref->name);
 			rc = 1;
 			continue;
 		}
@@ -2045,6 +2066,8 @@ int main(int argc, char **argv)
 
 		if (!rc)
 			fprintf(stderr, "    done\n");
+		if (helper_status)
+			printf("%s %s\n", !rc ? "ok" : "error", ref->name);
 		unlock_remote(ref_lock);
 		check_locks();
 	}
diff --git a/remote-curl.c b/remote-curl.c
index e5d9768..000bb52 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -1,8 +1,11 @@
 #include "cache.h"
+#include "exec_cmd.h"
 #include "remote.h"
 #include "strbuf.h"
 #include "walker.h"
 #include "http.h"
+#include "run-command.h"
+
 
 static struct remote *remote;
 static const char *url;
@@ -86,6 +89,20 @@ static struct ref *get_refs(void)
 	return refs;
 }
 
+static void output_refs(struct ref *refs)
+{
+	struct ref *posn;
+	for (posn = refs; posn; posn = posn->next) {
+		if (posn->symref)
+			printf("@%s %s\n", posn->symref, posn->name);
+		else
+			printf("%s %s\n", sha1_to_hex(posn->old_sha1), posn->name);
+	}
+	printf("\n");
+	fflush(stdout);
+	free_refs(refs);
+}
+
 struct fetch_args {
 	unsigned verbose : 1;
 };
@@ -173,10 +190,74 @@ static void parse_fetch(struct strbuf *buf, int multiple)
 	strbuf_reset(buf);
 }
 
+struct push_args {
+	unsigned dry_run : 1,
+		verbose : 1;
+};
+
+static int push_dav(struct push_args *args, int nr_spec, char **specs)
+{
+	const char **argv = xmalloc((10 + nr_spec) * sizeof(char*));
+	int argc = 0, i;
+
+	argv[argc++] = "http-push";
+	argv[argc++] = "--helper-status";
+	if (args->dry_run)
+		argv[argc++] = "--dry-run";
+	if (args->verbose)
+		argv[argc++] = "--verbose";
+	argv[argc++] = url;
+	for (i = 0; i < nr_spec; i++)
+		argv[argc++] = specs[i];
+	argv[argc++] = NULL;
+
+	if (run_command_v_opt(argv, RUN_GIT_CMD))
+		die("git-%s failed", argv[0]);
+	free(argv);
+	return 0;
+}
+
+static void parse_push(struct strbuf *buf)
+{
+	char **specs = NULL;
+	int alloc_spec = 0, nr_spec = 0, i;
+	struct push_args args;
+
+	memset(&args, 0, sizeof(args));
+	do {
+		if (!prefixcmp(buf->buf, "push ")) {
+			ALLOC_GROW(specs, nr_spec + 1, alloc_spec);
+			specs[nr_spec++] = xstrdup(buf->buf + 5);
+
+		} else if (!strcmp(buf->buf, "option dry-run"))
+			args.dry_run = 1;
+		else if (!strcmp(buf->buf, "option verbose"))
+			args.verbose = 1;
+		else
+			die("http transport does not support %s", buf->buf);
+
+		strbuf_reset(buf);
+		if (strbuf_getline(buf, stdin, '\n') == EOF)
+			return;
+		if (!*buf->buf)
+			break;
+	} while (1);
+
+	if (push_dav(&args, nr_spec, specs))
+		exit(128); /* error already reported */
+	for (i = 0; i < nr_spec; i++)
+		free(specs[i]);
+	free(specs);
+
+	printf("\n");
+	fflush(stdout);
+}
+
 int main(int argc, const char **argv)
 {
 	struct strbuf buf = STRBUF_INIT;
 
+	git_extract_argv0_path(argv[0]);
 	setup_git_directory();
 	if (argc < 2) {
 		fprintf(stderr, "Remote needed\n");
@@ -204,19 +285,19 @@ int main(int argc, const char **argv)
 			parse_fetch(&buf, 1);
 
 		} else if (!strcmp(buf.buf, "list")) {
-			struct ref *refs = get_refs();
-			struct ref *posn;
-			for (posn = refs; posn; posn = posn->next) {
-				if (posn->symref)
-					printf("@%s %s\n", posn->symref, posn->name);
-				else
-					printf("%s %s\n", sha1_to_hex(posn->old_sha1), posn->name);
-			}
-			printf("\n");
-			fflush(stdout);
+			output_refs(get_refs());
+
+		} else if (!strcmp(buf.buf, "list for-push")) {
+			output_refs(get_refs());
+
+		} else if (!prefixcmp(buf.buf, "push ")) {
+			parse_push(&buf);
+
 		} else if (!strcmp(buf.buf, "capabilities")) {
 			printf("fetch\n");
 			printf("fetch-multiple\n");
+			printf("push\n");
+			printf("option dry-run\n");
 			printf("option verbose\n");
 			printf("\n");
 			fflush(stdout);
diff --git a/transport-helper.c b/transport-helper.c
index bb6cd1b..60fdb16 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -1,6 +1,6 @@
 #include "cache.h"
 #include "transport.h"
-
+#include "quote.h"
 #include "run-command.h"
 #include "commit.h"
 #include "diff.h"
@@ -15,7 +15,8 @@ struct helper_data
 	FILE *out;
 	struct string_list options;
 	unsigned fetch : 1,
-		fetch_multiple : 1;
+		fetch_multiple : 1,
+		push : 1;
 };
 
 static struct child_process *get_helper(struct transport *transport)
@@ -54,6 +55,8 @@ static struct child_process *get_helper(struct transport *transport)
 			data->fetch = 1;
 		if (!strcmp(buf.buf, "fetch-multiple"))
 			data->fetch_multiple = 1;
+		if (!strcmp(buf.buf, "push"))
+			data->push = 1;
 		if (!prefixcmp(buf.buf, "option ")) {
 			const char *name = buf.buf + strlen("option ");
 			if (!string_list_lookup(name, &data->options))
@@ -103,7 +106,7 @@ static int set_helper_option(struct transport *transport,
 
 	get_helper(transport);
 
-	if (!data->fetch_multiple)
+	if (!data->fetch_multiple && !data->push)
 		return 1;
 
 	if (!strcmp(name, TRANS_OPT_THIN))
@@ -211,6 +214,131 @@ static int fetch(struct transport *transport,
 	return -1;
 }
 
+static int push_refs(struct transport *transport,
+		struct ref *remote_refs, int flags)
+{
+	int force_all = flags & TRANSPORT_PUSH_FORCE;
+	int mirror = flags & TRANSPORT_PUSH_MIRROR;
+	struct helper_data *data = transport->data;
+	struct strbuf buf = STRBUF_INIT;
+	struct child_process *helper;
+	struct ref *ref;
+
+	if (!remote_refs)
+		return 0;
+
+	helper = get_helper(transport);
+	if (!data->push)
+		return 1;
+
+	for (ref = remote_refs; ref; ref = ref->next) {
+		if (ref->peer_ref)
+			hashcpy(ref->new_sha1, ref->peer_ref->new_sha1);
+		else if (!mirror)
+			continue;
+
+		ref->deletion = is_null_sha1(ref->new_sha1);
+		if (!ref->deletion &&
+			!hashcmp(ref->old_sha1, ref->new_sha1)) {
+			ref->status = REF_STATUS_UPTODATE;
+			continue;
+		}
+
+		if (force_all)
+			ref->force = 1;
+
+		strbuf_addstr(&buf, "push ");
+		if (!ref->deletion) {
+			if (ref->force)
+				strbuf_addch(&buf, '+');
+			if (ref->peer_ref)
+				strbuf_addstr(&buf, ref->peer_ref->name);
+			else
+				strbuf_addstr(&buf, sha1_to_hex(ref->new_sha1));
+		}
+		strbuf_addch(&buf, ':');
+		strbuf_addstr(&buf, ref->name);
+		strbuf_addch(&buf, '\n');
+	}
+
+	standard_options(transport);
+	if (flags & TRANSPORT_PUSH_DRY_RUN) {
+		if (save_option(transport, "dry-run", ""))
+			die("helper %s does not support dry-run", data->name);
+	}
+	if (flags & TRANSPORT_PUSH_VERBOSE)
+		save_option(transport, "verbose", "");
+	for_each_string_list(print_options, &data->options, &buf);
+	strbuf_addch(&buf, '\n');
+
+	if (write_in_full(helper->in, buf.buf, buf.len) != buf.len)
+		exit(128);
+
+	ref = remote_refs;
+	while (1) {
+		char *refname, *msg;
+		int status;
+
+		strbuf_reset(&buf);
+		if (strbuf_getline(&buf, data->out, '\n') == EOF)
+			exit(128); /* child died, message supplied already */
+		if (!buf.len)
+			break;
+
+		if (!prefixcmp(buf.buf, "ok ")) {
+			status = REF_STATUS_OK;
+			refname = buf.buf + 3;
+		} else if (!prefixcmp(buf.buf, "error ")) {
+			status = REF_STATUS_REMOTE_REJECT;
+			refname = buf.buf + 6;
+		} else
+			die("expected ok/error, helper said '%s'\n", buf.buf);
+
+		msg = strchr(refname, ' ');
+		if (msg) {
+			struct strbuf msg_buf = STRBUF_INIT;
+			const char *end;
+
+			*msg++ = '\0';
+			if (!unquote_c_style(&msg_buf, msg, &end))
+				msg = strbuf_detach(&msg_buf, NULL);
+			else
+				msg = xstrdup(msg);
+			strbuf_release(&msg_buf);
+
+			if (!strcmp(msg, "no match")) {
+				status = REF_STATUS_NONE;
+				free(msg);
+				msg = NULL;
+			}
+			else if (!strcmp(msg, "up to date")) {
+				status = REF_STATUS_UPTODATE;
+				free(msg);
+				msg = NULL;
+			}
+			else if (!strcmp(msg, "non-fast forward")) {
+				status = REF_STATUS_REJECT_NONFASTFORWARD;
+				free(msg);
+				msg = NULL;
+			}
+		}
+
+		if (ref)
+			ref = find_ref_by_name(ref, refname);
+		if (!ref)
+			ref = find_ref_by_name(remote_refs, refname);
+		if (!ref) {
+			warning("helper reported unexpected status of %s", refname);
+			continue;
+		}
+
+		ref->status = status;
+		ref->remote_status = msg;
+	}
+	strbuf_release(&buf);
+	return 0;
+}
+
 static struct ref *get_refs_list(struct transport *transport, int for_push)
 {
 	struct helper_data *data = transport->data;
@@ -222,7 +350,10 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)
 
 	helper = get_helper(transport);
 
-	write_str_in_full(helper->in, "list\n");
+	if (data->push && for_push)
+		write_str_in_full(helper->in, "list for-push\n");
+	else
+		write_str_in_full(helper->in, "list\n");
 
 	while (1) {
 		char *eov, *eon;
@@ -263,6 +394,7 @@ int transport_helper_init(struct transport *transport, const char *name)
 	transport->set_option = set_helper_option;
 	transport->get_refs_list = get_refs_list;
 	transport->fetch = fetch;
+	transport->push_refs = push_refs;
 	transport->disconnect = disconnect_helper;
 	return 0;
 }
diff --git a/transport.c b/transport.c
index 644a30a..6d9652d 100644
--- a/transport.c
+++ b/transport.c
@@ -349,35 +349,6 @@ static int rsync_transport_push(struct transport *transport,
 	return result;
 }
 
-#ifndef NO_CURL
-static int curl_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags)
-{
-	const char **argv;
-	int argc;
-
-	if (flags & TRANSPORT_PUSH_MIRROR)
-		return error("http transport does not support mirror mode");
-
-	argv = xmalloc((refspec_nr + 12) * sizeof(char *));
-	argv[0] = "http-push";
-	argc = 1;
-	if (flags & TRANSPORT_PUSH_ALL)
-		argv[argc++] = "--all";
-	if (flags & TRANSPORT_PUSH_FORCE)
-		argv[argc++] = "--force";
-	if (flags & TRANSPORT_PUSH_DRY_RUN)
-		argv[argc++] = "--dry-run";
-	if (flags & TRANSPORT_PUSH_VERBOSE)
-		argv[argc++] = "--verbose";
-	argv[argc++] = transport->url;
-	while (refspec_nr--)
-		argv[argc++] = *refspec++;
-	argv[argc] = NULL;
-	return !!run_command_v_opt(argv, RUN_GIT_CMD);
-}
-
-#endif
-
 struct bundle_transport_data {
 	int fd;
 	struct bundle_header header;
@@ -826,8 +797,6 @@ struct transport *transport_get(struct remote *remote, const char *url)
 		transport_helper_init(ret, "curl");
 #ifdef NO_CURL
 		error("git was compiled without libcurl support.");
-#else
-		ret->push = curl_transport_push;
 #endif
 
 	} else if (is_local(url) && is_file(url)) {
-- 
1.6.5.52.g0ff2e

^ permalink raw reply related

* [RFC PATCH v2 10/16] Git-aware CGI to provide dumb HTTP transport
From: Shawn O. Pearce @ 2009-10-13  2:25 UTC (permalink / raw)
  To: git
In-Reply-To: <1255400715-10508-1-git-send-email-spearce@spearce.org>

The git-http-backend CGI can be configured into any Apache server
using ScriptAlias, such as with the following configuration:

  LoadModule cgi_module /usr/libexec/apache2/mod_cgi.so
  LoadModule alias_module /usr/libexec/apache2/mod_alias.so
  ScriptAlias /git/ /usr/libexec/git-core/git-http-backend/

Repositories are accessed via the translated PATH_INFO.

The CGI is backwards compatible with the dumb client, allowing all
older HTTP clients to continue to download repositories which are
managed by the CGI.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .gitignore     |    1 +
 Makefile       |    1 +
 http-backend.c |  261 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 263 insertions(+), 0 deletions(-)
 create mode 100644 http-backend.c

diff --git a/.gitignore b/.gitignore
index 51a37b1..353d22f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -55,6 +55,7 @@ git-get-tar-commit-id
 git-grep
 git-hash-object
 git-help
+git-http-backend
 git-http-fetch
 git-http-push
 git-imap-send
diff --git a/Makefile b/Makefile
index fea237b..271c290 100644
--- a/Makefile
+++ b/Makefile
@@ -365,6 +365,7 @@ PROGRAMS += git-show-index$X
 PROGRAMS += git-unpack-file$X
 PROGRAMS += git-upload-pack$X
 PROGRAMS += git-var$X
+PROGRAMS += git-http-backend$X
 
 # List built-in command $C whose implementation cmd_$C() is not in
 # builtin-$C.o but is linked in as part of some other command.
diff --git a/http-backend.c b/http-backend.c
new file mode 100644
index 0000000..39cfd25
--- /dev/null
+++ b/http-backend.c
@@ -0,0 +1,261 @@
+#include "cache.h"
+#include "refs.h"
+#include "pkt-line.h"
+#include "object.h"
+#include "tag.h"
+#include "exec_cmd.h"
+#include "run-command.h"
+
+static const char content_type[] = "Content-Type";
+static const char content_length[] = "Content-Length";
+
+static char buffer[1024];
+
+static const char *http_date(unsigned long time)
+{
+	return show_date(time, 0, DATE_RFC2822);
+}
+
+static void format_write(const char *fmt, ...)
+{
+	va_list args;
+	unsigned n;
+
+	va_start(args, fmt);
+	n = vsnprintf(buffer, sizeof(buffer), fmt, args);
+	va_end(args);
+	if (n >= sizeof(buffer))
+		die("protocol error: impossibly long line");
+
+	safe_write(1, buffer, n);
+}
+
+static void write_status(unsigned code, const char *msg)
+{
+	format_write("Status: %u %s\r\n", code, msg);
+}
+
+static void write_header(const char *name, const char *value)
+{
+	format_write("%s: %s\r\n", name, value);
+}
+
+static void end_headers(void)
+{
+	safe_write(1, "\r\n", 2);
+}
+
+static void write_nocache(void)
+{
+	write_header("Expires", "Fri, 01 Jan 1980 00:00:00 GMT");
+	write_header("Pragma", "no-cache");
+	write_header("Cache-Control", "no-cache, max-age=0, must-revalidate");
+}
+
+static void write_cache_forever(void)
+{
+	unsigned long now = time(NULL);
+	write_header("Date", http_date(now));
+	write_header("Expires", http_date(now + 31536000));
+	write_header("Cache-Control", "public, max-age=31536000");
+}
+
+static NORETURN void not_found(const char *err, ...)
+{
+	va_list params;
+
+	write_status(404, "Not Found");
+	write_nocache();
+	end_headers();
+
+	va_start(params, err);
+	if (err && *err) {
+		vsnprintf(buffer, sizeof(buffer), err, params);
+		fprintf(stderr, "%s\n", buffer);
+	}
+	va_end(params);
+	exit(0);
+}
+
+static void write_file(const char *the_type, const char *name)
+{
+	const char *p = git_path("%s", name);
+	int fd;
+	struct stat sb;
+	uintmax_t remaining;
+
+	fd = open(p, O_RDONLY);
+	if (fd < 0)
+		not_found("Cannot open '%s': %s", p, strerror(errno));
+	if (fstat(fd, &sb) < 0)
+		die_errno("Cannot stat '%s'", p);
+	remaining = (uintmax_t)sb.st_size;
+
+	write_header(content_type, the_type);
+	write_header("Last-Modified", http_date(sb.st_mtime));
+	format_write("Content-Length: %" PRIuMAX "\r\n", remaining);
+	end_headers();
+
+	while (remaining) {
+		ssize_t n = xread(fd, buffer, sizeof(buffer));
+		if (n < 0)
+			die_errno("Cannot read '%s'", p);
+		n = safe_write(1, buffer, n);
+		if (n <= 0)
+			break;
+	}
+	close(fd);
+}
+
+static void get_text_file(char *name)
+{
+	write_nocache();
+	write_file("text/plain; charset=utf-8", name);
+}
+
+static void get_loose_object(char *name)
+{
+	write_cache_forever();
+	write_file("application/x-git-loose-object", name);
+}
+
+static void get_pack_file(char *name)
+{
+	write_cache_forever();
+	write_file("application/x-git-packed-objects", name);
+}
+
+static void get_idx_file(char *name)
+{
+	write_cache_forever();
+	write_file("application/x-git-packed-objects-toc", name);
+}
+
+static int show_text_ref(const char *name, const unsigned char *sha1,
+	int flag, void *cb_data)
+{
+	struct object *o = parse_object(sha1);
+	if (!o)
+		return 0;
+
+	format_write("%s\t%s\n", sha1_to_hex(sha1), name);
+	if (o->type == OBJ_TAG) {
+		o = deref_tag(o, name, 0);
+		if (!o)
+			return 0;
+		format_write("%s\t%s^{}\n", sha1_to_hex(o->sha1), name);
+	}
+
+	return 0;
+}
+
+static void get_info_refs(char *arg)
+{
+	write_nocache();
+	write_header(content_type, "text/plain; charset=utf-8");
+	end_headers();
+
+	for_each_ref(show_text_ref, NULL);
+}
+
+static void get_info_packs(char *arg)
+{
+	size_t objdirlen = strlen(get_object_directory());
+	struct packed_git *p;
+
+	write_nocache();
+	write_header(content_type, "text/plain; charset=utf-8");
+	end_headers();
+
+	prepare_packed_git();
+	for (p = packed_git; p; p = p->next) {
+		if (!p->pack_local)
+			continue;
+		format_write("P %s\n", p->pack_name + objdirlen + 6);
+	}
+	safe_write(1, "\n", 1);
+}
+
+static NORETURN void die_webcgi(const char *err, va_list params)
+{
+	write_status(500, "Internal Server Error");
+	write_nocache();
+	end_headers();
+
+	vsnprintf(buffer, sizeof(buffer), err, params);
+	fprintf(stderr, "fatal: %s\n", buffer);
+	exit(0);
+}
+
+static struct service_cmd {
+	const char *method;
+	const char *pattern;
+	void (*imp)(char *);
+} services[] = {
+	{"GET", "/HEAD$", get_text_file},
+	{"GET", "/info/refs$", get_info_refs},
+	{"GET", "/objects/info/packs$", get_info_packs},
+	{"GET", "/objects/info/[^/]*$", get_text_file},
+	{"GET", "/objects/[0-9a-f]{2}/[0-9a-f]{38}$", get_loose_object},
+	{"GET", "/objects/pack/pack-[0-9a-f]{40}\\.pack$", get_pack_file},
+	{"GET", "/objects/pack/pack-[0-9a-f]{40}\\.idx$", get_idx_file}
+};
+
+int main(int argc, char **argv)
+{
+	char *dir = getenv("PATH_TRANSLATED");
+	char *input_method = getenv("REQUEST_METHOD");
+	struct service_cmd *cmd = NULL;
+	char *cmd_arg = NULL;
+	int i;
+
+	set_die_routine(die_webcgi);
+
+	if (!dir)
+		die("No PATH_TRANSLATED from server");
+	if (!input_method)
+		die("No REQUEST_METHOD from server");
+	if (!strcmp(input_method, "HEAD"))
+		input_method = "GET";
+
+	for (i = 0; i < ARRAY_SIZE(services); i++) {
+		struct service_cmd *c = &services[i];
+		regex_t re;
+		regmatch_t out[1];
+
+		if (regcomp(&re, c->pattern, REG_EXTENDED))
+			die("Bogus regex in service table: %s", c->pattern);
+		if (!regexec(&re, dir, 1, out, 0)) {
+			size_t n = out[0].rm_eo - out[0].rm_so;
+
+			if (strcmp(input_method, c->method)) {
+				const char *proto = getenv("SERVER_PROTOCOL");
+				if (proto && !strcmp(proto, "HTTP/1.1"))
+					write_status(405, "Method Not Allowed");
+				else
+					write_status(400, "Bad Request");
+				write_nocache();
+				end_headers();
+				return 0;
+			}
+
+			cmd = c;
+			cmd_arg = xmalloc(n);
+			strncpy(cmd_arg, dir + out[0].rm_so + 1, n);
+			cmd_arg[n] = '\0';
+			dir[out[0].rm_so] = 0;
+			break;
+		}
+		regfree(&re);
+	}
+
+	if (!cmd)
+		not_found("Request not supported: '%s'", dir);
+
+	setup_path();
+	if (!enter_repo(dir, 0))
+		not_found("Not a git repository: '%s'", dir);
+
+	cmd->imp(cmd_arg);
+	return 0;
+}
-- 
1.6.5.52.g0ff2e

^ permalink raw reply related

* Re: Git 1.6.5 git clone unhandled exception using http protocol
From: eduard stefan @ 2009-10-13  3:36 UTC (permalink / raw)
  Cc: Git Mailing List, msysgit
In-Reply-To: <4AD24A46.5020705@gmail.com>

> Git was compiled with msysGit, the errors are generated inside msysGit shell,
> and VS2008 debugger gives this message:
> 
> "Unhandled exception at 0x00420354 in git-remote-curl.exe:
>  0xC0000005: Access violation reading location 0x00000000."

As a sidenote, 1.6.5 release crashes the same way.

After applying Shawn O. Pearce "Return of smart HTTP" patch series,
it no longer crashes, and "git clone http://github.com/loudej/spark.git" returns
"error: Unable to get pack file
http://github.com/loudej/spark.git/objects/pack/
pack-1bc121e71e2847622f814603ddb34444bfc6a16c.pack
The requested URL returned error: 502"
which seems to be more like a GitHub problem.

OTOH, cloning a local repositor serverd with mongoose works as expected:
"git clone http://192.168.194.24/test.git/"

Have a nice day,
  Eduard

^ permalink raw reply

* Re: [RFC PATCH v2 00/16] Return of smart HTTP
From: eduard stefan @ 2009-10-13  3:45 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1255400715-10508-1-git-send-email-spearce@spearce.org>

Shawn O. Pearce wrote:
> The series has gotten a lot larger since my last posting, but I have
> what appears to be a fully working client *AND* server implementation
> for both fetch and push, and the client should be supporting deeping
> shallow repositories over the smart variant of HTTP.
> 
> I've dropped the documentation patch from the series for now as
> I have quite a few edits queued up from folks in the last round
> (thanks for those!) that I have not yet applied.  So there is no
> point in sending that particular patch again.

I have apllied you patches on top of Git 1.6.5 release,
and they solved my http cloning crashes on windows
(msysGit environment).

Now git http cloning works just fine for local http servers,
but on GitHub I get an 502 error:
"error: Unable to get pack file
http://github.com/loudej/spark.git/objects/pack/
pack-1bc121e71e2847622f814603ddb34444bfc6a16c.pack
The requested URL returned error: 502"
which seems to be more like a GitHub problem,
since that URL returns an error when accessing it with a browser.

Have a nice day,
  Eduard

^ permalink raw reply

* Re: [RFC PATCH v2 07/16] remote-helpers: Fetch more than one ref in a batch
From: Daniel Barkalow @ 2009-10-13  3:56 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1255400715-10508-8-git-send-email-spearce@spearce.org>

On Mon, 12 Oct 2009, Shawn O. Pearce wrote:

> Some network protocols (e.g. native git://) are able to fetch more
> than one ref at a time and reduce the overall transfer cost by
> combining the requests into a single exchange.  Instead of feeding
> each fetch request one at a time to the helper, feed all of them
> at once so the helper can decide whether or not it should batch them.
> 
> Because 'fetch' was already released in 1.6.5 we introduce the new
> fetch-multiple capability/command to signal that the helper wants
> to use batch oriented approach to fetching refs.

In 1.6.5, there's no way to call a helper other than git-remote-curl, and 
no way to call git-remote-curl unless 1.6.5 was built with it. So I think 
the protocol is not set in stone quite yet. It's documentated for being an 
API, and it's supposed to be that, but it's not quite there in this 
version.

I think it should be generally a good idea to have a start signal and an 
end signal for a block of fetches (and, with the foreign stuff, it would 
be useful to have transport-helper tell the helper process when it was 
done making requests, so the helper process could tell the gfi process to 
exit and stop consuming the helper process's output). At worst, if the 
helper doesn't care, it can just ignore this information.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: Git: "No you can't handle my root!" (?)
From: Daniele Segato @ 2009-10-13  4:17 UTC (permalink / raw)
  To: Jeff King; +Cc: sylvain, git
In-Reply-To: <20091013014332.GB13737@coredump.intra.peff.net>

Il giorno lun, 12/10/2009 alle 21.43 -0400, Jeff King ha scritto:
> Take a look at:
> 
>   http://joey.kitenet.net/code/etckeeper/


thanks really interesting


> > can I have a git report of $HOME/.* (without . and ..)? (all user
> > setting)
> 
> This seems to work:
> 
>   $ cd ~
>   $ git init
>   $ echo '*' >.gitignore
>   $ echo '!.*' >.gitignore
> 
> > Or better: provide a list of directory under $HOME I want to track 
> 
> Same thing, but make your ! pattern more specific.

thanks again!

regards,
Daniele

^ permalink raw reply

* Re: [RFC PATCH v2 08/16] remote-helpers: Support custom transport options
From: Daniel Barkalow @ 2009-10-13  4:23 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1255400715-10508-9-git-send-email-spearce@spearce.org>

On Mon, 12 Oct 2009, Shawn O. Pearce wrote:

> Some transports, like the native pack transport implemented by
> fetch-pack, support useful features like depth or include tags.
> These should be exposed if the underlying helper knows how to
> use them and is based upon the same infrastructure.
>
> Helpers must advertise the options they support, any attempt
> to set an unsupported option will cause a failure.
> 
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> CC: Daniel Barkalow <barkalow@iabervon.org>
> ---
>  Documentation/git-remote-helpers.txt |   20 ++++++++++
>  remote-curl.c                        |   16 ++++++-
>  transport-helper.c                   |   70 ++++++++++++++++++++++++++++++++++
>  3 files changed, 103 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
> index e10ce99..334ab30 100644
> --- a/Documentation/git-remote-helpers.txt
> +++ b/Documentation/git-remote-helpers.txt
> @@ -46,6 +46,7 @@ Supported if the helper has the "fetch" capability.
>  'fetch-multiple'::
>  	Fetches multiple objects at once.  The fetch-multiple
>  	command is followed by one or more 'fetch' lines as above,
> +	zero or more 'option' lines for the supported options,
>  	and then a blank line to terminate the batch.  Outputs a
>  	single blank line when the entire batch is complete.
>  	Optionally may output a 'lock <file>' line indicating a
> @@ -69,6 +70,9 @@ CAPABILITIES
>  'fetch-multiple'::
>  	This helper supports the 'fetch-multiple' command.
>  
> +'option' <name>::
> +	This helper supports the option <name> under fetch-multiple.
> +

I'm a bit surprised that the options only apply in a fetch-multiple 
section, rather than getting set at the beginning and applying to 
everything for that run. At least, I think an "option" command should be 
useable outside of a fetch-multiple (or possible future grouping 
construct) and have global scope.

>  REF LIST ATTRIBUTES
>  -------------------
>  
> @@ -76,10 +80,26 @@ None are defined yet, but the caller must accept any which are supplied.
>  
>  FETCH OPTIONS
>  -------------
> +To enable an option the helper must list it in 'capabilities'.
>  
>  'option verbose'::
>  	Print more verbose activity messages to stderr.

I think you mis-split the above part; your previoud patch declared this 
option without declaring any way to use it. Might be worth allowing 
multiple "verboses" and "quiet" or "option verbosity quiet"/"option 
verbosity verbose verbose".

> +'option uploadpack' <command>::
> +	The program to use on the remote side to generate a pack.

I sort of feel like the helper ought to read this one out of the config 
file itself if it wants it. In general, it would be good to have 
transport.c and remote.c out of the business of knowing this sort of 
protocol-specific (albiet specific now to two protocols) information. (Of 
course, the native protocol's transport methods are in transport.c, so 
that's there, but I'd like to move that to a transport-native.c someday.)

> +'option depth' <depth>::
> +	Deepen the history of a shallow repository.
> +
> +'option keep'::
> +	Keep the transferred pack(s) with .keep files.
> +
> +'option followtags'::
> +	Aggressively fetch annotated tags if possible.

I assume this means to fetch tags which annotate objects we have or are 
fetching? (As opposed to fetching any annotated tag we could possibly 
fetch, even if we don't otherwise care about the tag or the thing it 
tags.) It's obvious in the context of git's config options, but I'd like 
this document to avoid assuming that context, and the option could apply 
more generally.

> +
> +'option thin'::
> +	Transfer the data as a thin pack if possible.

Does anyone still use non-default thinness? 

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: [RFC PATCH v2 09/16] Move WebDAV HTTP push under remote-curl
From: Mike Hommey @ 2009-10-13  4:41 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Daniel Barkalow, Tay Ray Chuan
In-Reply-To: <1255400715-10508-10-git-send-email-spearce@spearce.org>

On Mon, Oct 12, 2009 at 07:25:08PM -0700, Shawn O. Pearce wrote:
> The remote helper interface now supports the push capability,
> which can be used to ask the implementation to push one or more
> specs to the remote repository.  For remote-curl we implement this
> by calling the existing WebDAV based git-http-push executable.
> 
> Internally the helper interface uses the push_refs transport hook
> so that the complexity of the refspec parsing and matching can be
> reused between remote implementations.  When possible however the
> helper protocol uses source ref name rather than the source SHA-1,
> thereby allowing the helper to access this name if it is useful.

It's been a while I haven't followed changes in the remote code but this
looks nice, though I haven't checked thoroughly. I guess the next step
would be to kill http-push as an external program.

Mike

^ permalink raw reply

* Re: [PATCH/RFC 3/4] git check-ref-format --print
From: Jonathan Nieder @ 2009-10-13  4:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jens Lehmann, git, Shawn O. Pearce
In-Reply-To: <7vococ6v73.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

> I do not disagree with a desire to help fixing the unicode insanity on
> that platform, but I suspect that check-ref-format is a wrong place to
> tackle the issue.  You would need a similar filter for outputs from the
> likes of ls-files and "diff --name-only", iow, anything that deal with
> pathnames, no?
> 
> It would have be something like "check-ref-format --print | iconv ..."
> pipeline (conceptually, if not forcing the pipeline to the end users, that
> is).

GNU iconv does not write the various Unicode normalization forms, so it
would have to be something like "check-ref-format --print | charlint ..."
instead.  Regardless of the filesystem, it seems reasonable to consider é
(U+00e9) and é (U+0065 + U+0301) the same character when it appears in a
ref name, and one way to achieve this would be to pick one normalization
form and stick to it.  This does not seem so different from stripping out
empty path components.

As a side effect, that would deal with OS X’s strange handling of unicode
filenames for .git/refs/*.  Now that I think about it, if fighting OS X
were the only problem that needed to be solved, I don’t think I’d like
this solution so much.  The analogous solution to the also unsolved issue
of case insensitive filesystems is to force all ref names to lowercase.
Do we want to do that?  (The case insensitivity issue might not be as bad,
since the relevant filesystems will at least _preserve_ the case of
filenames in .git/refs.  Should we copy them and smudge new ref names to
match known ones that differ only in case?  Just thinking about the
problem makes me cringe.)

Coping with the unicode filename craziness in the working tree is a
separate issue, though probably a more important one.  I think Linus set
up a framework for solving it in
<http://thread.gmane.org/gmane.comp.version-control.git/77827>.

Regards,
Jonathan

^ permalink raw reply

* Re: Git: "No you can't handle my root!" (?)
From: Jeff King @ 2009-10-13  5:01 UTC (permalink / raw)
  To: Daniele Segato; +Cc: sylvain, git
In-Reply-To: <1255407433.15646.12.camel@localhost>

On Tue, Oct 13, 2009 at 06:17:13AM +0200, Daniele Segato wrote:

> > This seems to work:
> > 
> >   $ cd ~
> >   $ git init
> >   $ echo '*' >.gitignore
> >   $ echo '!.*' >.gitignore
> > 
> > > Or better: provide a list of directory under $HOME I want to track 
> > 
> > Same thing, but make your ! pattern more specific.
> 
> thanks again!

You're welcome, though while reading the quoted text I noticed a typo in
my instructions. The second echo should obviously be _appending_ to
.gitignore:

  $ echo '!.*' >>.gitignore

Hopefully that was obvious, but I thought I would point it out for the
record.

-Peff

^ permalink raw reply

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Thomas Rast @ 2009-10-13  6:36 UTC (permalink / raw)
  To: Junio C Hamano, Johannes Schindelin
  Cc: Euguess, Mikael Magnusson, Matthieu Moy, Jeff King, Jay Soffian,
	git
In-Reply-To: <7vr5t89qiw.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
> 
> > Your idea is also a backwards incompatible change, so we can just as
> > well implement the original suggestion and force scripts (or us) to
> > use some other means when they want to detach.  Say, why not just
> > invent an option along the lines of
> >
> >   git checkout {-d|--detach} $ref
> >
> > to make it explicit.
> 
> Or can't you go the other way, say
> 
> 	git checkout -t $remote_tracking
> 
> to create a local branch forking from the named remote tracking branch?

Sure, but we already have that and we still failed to fix the users,
so FWIW, I think Dscho's right and we should try fixing the UI next.

[I've also seen several users shoot themselves with detached HEADs to
the point where I explain the concept before even mentioning
checkout.]

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a   branch
From: Johannes Sixt @ 2009-10-13  6:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Anders Kaseorg, git
In-Reply-To: <7vr5t8coex.fsf@alter.siamese.dyndns.org>

Junio C Hamano schrieb:
> I even think that the support for an explicit branch name in the reset
> subcommand should eventually be deprecated, perhaps unless it matches what
> is stored in BISECT_START.

Goodness, NO!

> The documentation, does not even talk about what the optional <branch>
> argument is good for, even though it lists the optional <branch> in the
> synopsis section.

If I had know about this feature (yes, FEATURE), I would have used it like
this in the past:

  $ git branch tmp
  $ git bisect reset tmp
  $ git branch -d tmp

With the patch proposed by Anders this shortens to:

  $ git bisect reset HEAD

> Having said all that, four years and two months are looooooong time in git
> timescale, and I am discounting, without any evidence to judge either way,
> the possibility that people may have learned during that time to (ab)use
> this as a (very useful?) shortcut to finish the current bisection and
> switch to some entirely different branch.

In all the bisect runs that I have done in my live, the ONLY way I wanted
'bisect reset' to act was to NOT change the commit it currently was on.
The fact that it switched back to the commit or branch that the bisect was
started on, was always a major inconvenience.

So, I have no problem if you want to deprecate the branch parameter, if at
the same time bisect reset no longer switches to some other commit. ;)

> I offhand do not see a good rationale for such a shortcut to finish bisect
> and switch to another branch (IOW, I understand "it is shorter to type",
> but I do not see "it is often done and very useful"), but I am open to be
> enlightened by a workflow where such a shortcut is useful.

In my workflow, after I've found the bad commit, I always want bisect to
stay at the commit that it found. I don't want it to warp me somewhere
else; I want to make the decision where to go next myself.

-- Hannes

^ permalink raw reply

* Re: [RFC PATCH v2 00/16] Return of smart HTTP
From: Junio C Hamano @ 2009-10-13  6:42 UTC (permalink / raw)
  To: eduard stefan; +Cc: Shawn O. Pearce, git
In-Reply-To: <4AD3F7C5.2060203@gmail.com>

eduard stefan <eduard.stefan@gmail.com> writes:

> I have apllied you patches on top of Git 1.6.5 release,
> and they solved my http cloning crashes on windows
> (msysGit environment).

Unfortunately that is not a very good news.

As there is no chance Shawn's new series will be applied to 'maint' for
updating 1.6.5.X series, somebody needs to address the breakage without
introducing the whole new HTTP code.  Is the breakage you are seeing
limited only to msysgit environment, or does the same breakage appear on
other platforms?

^ permalink raw reply

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a   branch
From: Johannes Sixt @ 2009-10-13  6:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Anders Kaseorg, git
In-Reply-To: <7vaazw6uyi.fsf@alter.siamese.dyndns.org>

Junio C Hamano schrieb:
> I would understand it, if not agreeing that I also am often in that
> situation myself", if somebody does not even care which commit he was on
> before starting the bisection, but knows (or is willing to decide at that
> point) which branch (or even a specific commit, while still being
> detached) he wants to switch to.  And it would make sense to avoid an
> extra checkout that snaps back to the pre-bisection commit before
> switching to the new state he has chosen.

The situation that I'm faced quite frequently is that after I find a
regression, I cannot tell which released version did not have the
breakage. Hence, the first thing I have to do is to find a good commit.
Therefore, I jump around in ancient history until I find a good commit.
Then I start bisect. I certainly do NOT want to be warped back to this
ancient commit by 'bisect reset'.

-- Hannes

^ permalink raw reply

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a  branch
From: Junio C Hamano @ 2009-10-13  7:01 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Anders Kaseorg, git
In-Reply-To: <4AD420BC.5060506@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> In my workflow, after I've found the bad commit, I always want bisect to
> stay at the commit that it found. I don't want it to warp me somewhere
> else; I want to make the decision where to go next myself.

Are you sure about what you are saying?

Half of the time, the commit you test in your "git bisect" section would
be a "good" one, and immediately after you tell it "bisect good", it tells
you that some _other_ commit you marked "bad" is the first bad commit.  In
such a case, you won't be on the commit that the bisect has found.

So I _do_ agree that you would always want to stare at the commit that is
the first bad one, leaving the bisection session at the detached HEAD
state bisection session ends at is _totally_ different from what you want.

^ permalink raw reply

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a branch
From: Anders Kaseorg @ 2009-10-13  7:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vaazw6uyi.fsf@alter.siamese.dyndns.org>

On Mon, 12 Oct 2009, Junio C Hamano wrote:
> But I do not know how it hurts to still have bisect states around, in
> order to find where you want to go next.  Could you elaborate?

Mostly little irritations.  Extra bisect/* refs show up in gitk.  If you 
use __git_ps1 in your prompt (from git-completion.bash), it adds 
|BISECTING to your prompt.

Also, I just noticed that if you start a new bisection without ever 
cleaning up the old one, the next ‘git bisect reset’ will bring you back 
to HEAD before the old bisection instead of HEAD before the new one, which 
is not what you would expect if you forgot that the old bisection ever 
happened.

> I am inclined to ask you to come up with a paragraph in the 
> documentation to discuss how the optional <branch> (now it will be 
> <commit>) parameter to the reset subcommand is meant to be used and 
> re-submit the original patch, perhaps with an updated commit log 
> message.

I note that the ‘git checkout’ documentation mentions <branch> and not 
<commit>, perhaps to emphasize that HEAD will become attached to the 
branch if you specify a branch name.  Do you think it makes sense for 
these to be documented differently?

Anders

^ permalink raw reply

* Re: [PATCH 2/3] git config: clarify bool types
From: Michael J Gruber @ 2009-10-13  7:09 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Michael J Gruber, Junio C Hamano, git
In-Reply-To: <94a0d4530910121014k666207b9ub38fcecd47641ace@mail.gmail.com>

Felipe Contreras venit, vidit, dixit 12.10.2009 19:14:
> On Mon, Oct 12, 2009 at 3:30 PM, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
>> Felipe Contreras venit, vidit, dixit 12.10.2009 12:03:
>>> On Mon, Oct 12, 2009 at 8:01 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>>
>>>>> The value is what it is, the --bool and --bool-or-int options don't
>>>>> specify the value type, just how it is interpreted. For example: a value
>>>>> of '1' can be interpreted as 'true'.
>>>>
>>>> It is not really about "interpreting", but about showing, isn't it?
>>>
>>> Unless you are setting it, instead of reading it.
>>>
>>
>> I'd still suggest fixing the typo ("interpreted") and spelling out
>> "boolean".
> 
> Oops! You mean s/intepreted/interpreted/?

Yep :)

> 
> If we spell 'boolean' we might as well spell 'integer'; I think bool
> and int are fine.
> 

"int" is at least a standard type name in C, whereas "bool" is not; but,
yes, feel free to spell out "integer", or use "--int or --bool" as it
is, which is a back reference to the corresponding entries for "--int"
and "--bool", where things should be spelled out.

Michael

^ permalink raw reply

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Junio C Hamano @ 2009-10-13  7:16 UTC (permalink / raw)
  To: Thomas Rast
  Cc: Johannes Schindelin, Euguess, Mikael Magnusson, Matthieu Moy,
	Jeff King, Jay Soffian, git
In-Reply-To: <200910130836.57011.trast@student.ethz.ch>

Thomas Rast <trast@student.ethz.ch> writes:

>> Or can't you go the other way, say
>> 
>> 	git checkout -t $remote_tracking
>> 
>> to create a local branch forking from the named remote tracking branch?
>
> Sure, but we already have that and we still failed to fix the users,
> so FWIW, I think Dscho's right and we should try fixing the UI next.

What it means is that -t was a broken attempt to help the users at the UI
level, and I can surely see that.

So we need the set of new rules, say, for 1.7.0 release.  A strawman?

Assume that these are the only refs that exist:

    refs/remotes/origin/{master,next,nitfol}
    refs/remotes/xyzzy/{frotz,nitfol}
    refs/heads/master
    refs/tags/v1.0.0

#0. These will stay as is:

 $ git checkout mine               ;# switches to the branch
 $ git checkout $any_committish^0  ;# detaches

#1. These used to detach, but will create a local branch

 $ git checkout origin/next        ;# as if with -t
 $ git checkout xyzzy/frotz        ;# as if with -t (origin is not special)

#2. These are allowed only when unambiguous and there is no local branch yet.

 $ git checkout next               ;# ok
 $ git checkout frotz              ;# ok (origin is not special)
 $ git checkout nitfol             ;# not ok (ambiguous and origin is not special)

#3. These used to detach, but what should we do?

 $ git checkout v1.0.0             ;# detach, or refuse???
 $ git checkout origin/master      ;# detach, or refuse???

I can buy 0, 1, and 2, and I think it is a minor inconvenience if we
started refusing to detach in case #3, as people who want to detach can
always suffix ^0 or ~0 to make it a general committish.

Did I cover all cases?

^ permalink raw reply

* Re: [PATCH 1/2] user-manual: add global config section
From: Michael J Gruber @ 2009-10-13  7:19 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Michael J Gruber, git, Junio C Hamano, J. Bruce Fields,
	Jonathan Nieder
In-Reply-To: <94a0d4530910121009r52d45522jf1c27dd102db4ad9@mail.gmail.com>

Felipe Contreras venit, vidit, dixit 12.10.2009 19:09:
> On Mon, Oct 12, 2009 at 3:25 PM, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
>> Felipe Contreras venit, vidit, dixit 11.10.2009 22:43:
>>> So that users get to know how to configure git from the get-to with good
>>> practical example (color.ui = auto) that most people would probably like
>>> anyway.
>>>
>>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>>> ---
>>>  Documentation/user-manual.txt |   27 +++++++++++++++++++++++++++
>>>  1 files changed, 27 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
>>> index 67ebffa..ff2563a 100644
>>> --- a/Documentation/user-manual.txt
>>> +++ b/Documentation/user-manual.txt
>>> @@ -40,6 +40,33 @@ without any explanation.
>>>  Finally, see <<todo>> for ways that you can help make this manual more
>>>  complete.
>>>
>>> +[[getting-started]]
>>> +Getting started
>>> +=============
>>> +
>>> +Git's configuration is distributed among different locations--this manual will
>>> +only to deal with 'global' (for the user) and 'repository' variables, where
>>> +'repository' variables take precedence over 'global' ones.
>>
>> Well, you do talk about "system" below, and that's about it. Also, the
>> configuration is not really distributed among different locations. Most
>> newbies interested in a *D*VCS will misunderstand this (as git having
>> distributed configuration).
>>
>> Alternative:
>>
>> Git's default configuration can be changed on a system wide, global (per
>> user) and local (per repository) level, in the order of increasing
>> precedence.
> 
> When I read that it's not clear if the local level discards the global
> level completely or it's aggregated. If we specify that it's only the
> variables that take precedence it might be clearer:
> 
> Git's configuration is composed of variables that are stored in
> multiple locations: 'system' (all users), 'global' (for the user), and
> 'repository' -- in decreasing order of precedence.

Yep, although established lingo is "options" (not "variables"), and it's
really increasing order, not decreasing.

> 
>>> +
>>> +You would probably want to start setting up something useful:
>>> +------------------------------------------------
>>> +$ git config --global color.ui auto
>>> +------------------------------------------------
>>> +
>>> +This will make prettier the output of certain commands such as `git diff`, but
>>> +that's not important; what is important here is that `color.ui` has been
>>> +stored in the 'global' configuration.
>>
>> This will make certain commands such as `git diff` use colors in the
>> output. What is important here is that the value `auto` for the option
>> `color.ui` has been stored in the 'global' configuration. Use `--system`
>> for the system wide configuration; specifying neither `--system` nor
>> `--global` makes `git config` access the local configuration.
> 
> I think we should only mention (once) the system wide configuration,
> but not cover it. That's for system administrators, not users.
> 
>>> +
>>> +View and manually modify the configuration by opening `~/.gitconfig`:
>>
>> View and manually modify the global configuration by opening
>> `~/.gitconfig` in your editor or using `git config --global --edit`:
> 
> I have separate patches for 'git config --edit', but Junio suggested
> to hold them back because --edit is a relatively new option.
> 
>>> +------------------------------------------------
>>> +[color]
>>> +        ui = auto
>>> +------------------------------------------------
>>> +
>>> +Other locations are `/etc/gitconfig` (system), and `.git/config` (repository).
>>
>> I don't even think we should talk about locations here, "git config -e"
>> should be the first user's way to do it.
> 
> I disagree. Most useful configurations (color.ui, user.email) should
> be global. The complete newbie might think: cool, now I have my git
> properly configured (with 'git config -e'), and then when cloning a
> new repo (s)he would think: ok, git just forgot what I told him. When
> that happens (s)he would have to re-learn and re-configure git.
> 
> When users think about configuration, it's usually a 'global'
> configuration, so that's what we should teach from the beginning and
> make sure they understand the difference between 'global' and
> 'repository' configurations.

Sure. What I meant are the file locations, the actual paths. First
timers should use "git config -e" and "git config --global -e" if they
really want to edit their local and global config files. Better yet,
they should use "git config" and "git config --global" in their set and
get modes, because they make sure that there's no total garbage in the
config. The locations of the files are an implementation detail.

> 
>>> +
>>> +More git configurations will be covered in the rest of the manual, if you want
>>> +to learn more look at linkgit:git-config[1] for details.
>>
>> "Configurations" is ambiguous, it can be easily (mis)understood as
>> "types of configuration" (global, local etc.). Also, the above doesn't
>> really cover even one option. How about:
>>
>> This manual covers many configuration options (such as `color.ui.`). For
>> more details on the `git config` command as well as all configuration
>> options see linkgit:git-config[1].
> 
> Looks better, except s/configuration options/configuration variables/
> 

Uhm, no, for the reason mentioned above. While the man page of git
config is not completely consistent either, we're really talking about
configuration options. An "option" can be set to a "value", and the
thing you pass in order to do that can be called a "variable". For the
most part this is how git-config[1] uses this terminology.

Michael

^ permalink raw reply

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a  branch
From: Junio C Hamano @ 2009-10-13  7:21 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, Anders Kaseorg, git
In-Reply-To: <4AD42203.6030802@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> The situation that I'm faced quite frequently is that after I find a
> regression, I cannot tell which released version did not have the
> breakage. Hence, the first thing I have to do is to find a good commit.
> Therefore, I jump around in ancient history until I find a good commit.
> Then I start bisect. I certainly do NOT want to be warped back to this
> ancient commit by 'bisect reset'.

Unlike your other message, now, I can see _this_ one making sense very
much.

It is a very good explanation as to why BISECT_START (whose sole purpose
is to go back there) is not a very useful concept.  What you wrote deserve
to go to the "bisect reset" documentation to explain what the optional
<branch> argument (perhaps we would make it a <commit> with Anders's
patch) is good for and how it is intended to be used.

^ permalink raw reply

* Re: [RFC PATCH v2 12/16] Smart fetch and push over HTTP: server side
From: Johannes Sixt @ 2009-10-13  7:30 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1255400715-10508-13-git-send-email-spearce@spearce.org>

Shawn O. Pearce schrieb:
> diff --git a/http-backend.c b/http-backend.c
> index 39cfd25..adb3256 100644
> --- a/http-backend.c
> +++ b/http-backend.c

#include "run-command.h" is missing here because you added it already in
patch 10/16 unnecessarily.

> +	if (start_command(&cld))
> +		die_errno("Cannot start %s", argv[0]);
> ...
> +	if (finish_command(&cld))
> +		die("%s terminated with error", argv[0]);

start_command and finish_command already write an error message for you
that includes argv[0] and errno. You can just exit(1) here.

-- Hannes

^ permalink raw reply

* Re: [RFC PATCH v2 01/16] pkt-line: Add strbuf based functions
From: Johannes Sixt @ 2009-10-13  7:29 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1255400715-10508-2-git-send-email-spearce@spearce.org>

Shawn O. Pearce schrieb:
> -int packet_read_line(int fd, char *buffer, unsigned size)
> +static int packet_length(unsigned *ret_len, const char *linelen)
>  {
>  	int n;
> -	unsigned len;
> -	char linelen[4];
> -
> -	safe_read(fd, linelen, 4);
> +	unsigned len = 0;
>  
> -	len = 0;
>  	for (n = 0; n < 4; n++) {
>  		unsigned char c = linelen[n];
>  		len <<= 4;
> @@ -96,8 +116,20 @@ int packet_read_line(int fd, char *buffer, unsigned size)
>  			len += c - 'A' + 10;
>  			continue;
>  		}
> -		die("protocol error: bad line length character");
> +		return -1;
>  	}
> +	*ret_len = len;
> +	return 0;
> +}

len can be signed: Valid lengths fit into a signed int. Then you can
'return len;' on success and 'return -1;' on failure and don't need return
the result by reference. packet_read_line() ultimately converts it to int
anyway:

> +int packet_read_line(int fd, char *buffer, unsigned size)
> +{
> +	unsigned len;
> +	char linelen[4];
> +
> +	safe_read(fd, linelen, 4);
> +	if (packet_length(&len, linelen))
> +		die("protocol error: bad line length character");
>  	if (!len)
>  		return 0;
>  	len -= 4;
> @@ -107,3 +139,28 @@ int packet_read_line(int fd, char *buffer, unsigned size)
>  	buffer[len] = 0;
>  	return len;
>  }

-- Hannes

^ permalink raw reply

* [PATCH] gitk: Prevent garbage text at the end of the tag description
From: Björn Gustavsson @ 2009-10-13  7:34 UTC (permalink / raw)
  To: Paul Mackerras, git

When first clicking on a commit with a huge diff (in 1000 files
or more), and then clicking on a tag, garbage text could appear
after the tag description.

The problem is that commits are shown incrementally using a
run queue (implemented by the runq() and filerun() procedures).
When the user requests a tag to be shown, there may still be
jobs in the run queue updating the previous commit.

Solve this problem by implementing a new procedure
empty_run_queue() that will empty the run queue and cancel all
outstanding file events. Call it from showtag().

Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>
---

This is more an annoyance than a real bug, but I was quite
surprised the first time I saw it happen and at first searched
for a bug in my own scripts that had created the tags.

This patch is a suggested correction.

Here is an example of a repository where the problem can be
reproduced quite easily:

git://github.com/mfoemmel/erlang-otp.git

 gitk-git/gitk |   34 ++++++++++++++++++++++++++++++++--
 1 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index a0214b7..ee0d0c3 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -34,12 +34,16 @@ proc run args {
 }
 
 proc filerun {fd script} {
+    global pending_file_events
+
+    set pending_file_events($fd) $fd
     fileevent $fd readable [list filereadable $fd $script]
 }
 
 proc filereadable {fd script} {
-    global runq currunq
+    global runq currunq pending_file_events
 
+    unset pending_file_events($fd)
     fileevent $fd readable {}
     if {$runq eq {} && ![info exists currunq]} {
 	after idle dorunq
@@ -60,7 +64,7 @@ proc nukefile {fd} {
 }
 
 proc dorunq {} {
-    global isonrunq runq currunq
+    global isonrunq runq currunq pending_file_events
 
     set tstart [clock clicks -milliseconds]
     set t0 $tstart
@@ -79,6 +83,7 @@ proc dorunq {} {
 		# file readers return 2 if they could do more straight away
 		lappend runq [list $fd $script]
 	    } else {
+		set pending_file_events($fd) $fd
 		fileevent $fd readable [list filereadable $fd $script]
 	    }
 	} elseif {$fd eq {}} {
@@ -92,6 +97,29 @@ proc dorunq {} {
     }
 }
 
+# Empty the run queue, cancel all outstanding file events,
+# and close all files associated with file events.
+proc empty_run_queue {} {
+    global isonrunq runq pending_file_events
+
+    foreach entry $runq {
+	set fd [lindex $entry 0]
+	set script [lindex $entry 1]
+	if {$fd eq {}} {
+	    unset isonrunq($script)
+	} else {
+	    catch {close $fd}
+	}
+    }
+    set runq {}
+
+    foreach fd [array names pending_file_events] {
+	fileevent $fd readable {}
+	unset pending_file_events($fd)
+	catch {close $fd}
+    }
+}
+
 proc reg_instance {fd} {
     global commfd leftover loginstance
 
@@ -10261,6 +10289,8 @@ proc listrefs {id} {
 proc showtag {tag isnew} {
     global ctext tagcontents tagids linknum tagobjid
 
+    empty_run_queue
+
     if {$isnew} {
 	addtohistory [list showtag $tag 0]
     }
-- 
1.6.5.2.gd6127

^ permalink raw reply related

* Bad URL passed to RA lay
From: m.skoric @ 2009-10-13  7:35 UTC (permalink / raw)
  To: git

Hi List,

i have a problem with git-svn clone / fetch. I get following error while doing one of previous command -> "Bad URL passed to RA lay"
This happens because a branch doesn't exists in svn anymore and git wants to retrieve data from it. Here is the complete error message

Initializing parent: Abo-Uebernahme (Bug #994)@341
Found possible branch point: "quoted"..trunk => "quoted"...Abo-Uebernahme (Bug #994), 203
Found branch parent: (Abo-Uebernahme (Bug #994)@341) bb831869748c98bf97d105c5894ae65331c95c08
Bad URL passed to RA layer: Malformed URL for repository at /usr/bin/git-svn line 4311

git version 1.6.3.3

Aynone else has this Problem?
Can anyone help me?

Thanks in advance

Majk
______________________________________________________
GRATIS für alle WEB.DE-Nutzer: Die maxdome Movie-FLAT!
Jetzt freischalten unter http://movieflat.web.de

^ permalink raw reply


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