[PATCH 1/2] Move the file read logic to read_patch_file() in builtin-apply.c

[PATCH 1/2] Move the file read logic to read_patch_file() in builtin-apply.c

[Mike Hommey]

This will allow to extend the read logic further. We also return a better
error message than usage() when the given filename can't be opened, and
avoid whitespace options not being set when reading from stdin with the
"-" argument as a side effect.

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 builtin-apply.c |   32 +++++++++++++++++---------------
 1 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/builtin-apply.c b/builtin-apply.c
index f2e9a33..8c8162a 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -182,11 +182,23 @@ static void say_patch_name(FILE *output, const char *pre,
 #define CHUNKSIZE (8192)
 #define SLOP (16)
 
-static void read_patch_file(struct strbuf *sb, int fd)
+static void read_patch_file(struct strbuf *sb, const char *filename)
 {
+	int fd;
+
+	if (!strcmp(filename, "-")) {
+		fd = 0;
+	} else {
+		fd = open(filename, O_RDONLY);
+		if (fd < 0)
+			die("git-apply: could not open %s: %s", filename,
+			    strerror(errno));
+	}
+
 	if (strbuf_read(sb, fd, 0) < 0)
 		die("git-apply: read returned %s", strerror(errno));
 
+	close(fd);
 	/*
 	 * Make sure that we have some slop in the buffer
 	 * so that we can do speculative "memcmp" etc, and
@@ -2705,7 +2717,7 @@ static void prefix_patches(struct patch *p)
 	}
 }
 
-static int apply_patch(int fd, const char *filename, int inaccurate_eof)
+static int apply_patch(const char *filename, int inaccurate_eof)
 {
 	size_t offset;
 	struct strbuf buf;
@@ -2714,7 +2726,7 @@ static int apply_patch(int fd, const char *filename, int inaccurate_eof)
 
 	strbuf_init(&buf, 0);
 	patch_input_file = filename;
-	read_patch_file(&buf, fd);
+	read_patch_file(&buf, filename);
 	offset = 0;
 	while (offset < buf.len) {
 		struct patch *patch;
@@ -2807,13 +2819,7 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
 		char *end;
-		int fd;
 
-		if (!strcmp(arg, "-")) {
-			errs |= apply_patch(0, "<stdin>", inaccurate_eof);
-			read_stdin = 0;
-			continue;
-		}
 		if (!prefixcmp(arg, "--exclude=")) {
 			struct excludes *x = xmalloc(sizeof(*x));
 			x->path = arg + 10;
@@ -2916,17 +2922,13 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
 		if (0 < prefix_length)
 			arg = prefix_filename(prefix, prefix_length, arg);
 
-		fd = open(arg, O_RDONLY);
-		if (fd < 0)
-			usage(apply_usage);
 		read_stdin = 0;
 		set_default_whitespace_mode(whitespace_option);
-		errs |= apply_patch(fd, arg, inaccurate_eof);
-		close(fd);
+		errs |= apply_patch(arg, inaccurate_eof);
 	}
 	set_default_whitespace_mode(whitespace_option);
 	if (read_stdin)
-		errs |= apply_patch(0, "<stdin>", inaccurate_eof);
+		errs |= apply_patch("-", inaccurate_eof);
 	if (whitespace_error) {
 		if (squelch_whitespace_errors &&
 		    squelch_whitespace_errors < whitespace_error) {
-- 
1.5.3.7

[PATCH 2/2] Add support for URLs to git-apply

[Mike Hommey]

Instead of doing several "wget -O - url | git-apply -" in a raw, you now
can just git-apply url1 url2 ...

Signed-off-by: Mike Hommey <mh@glandium.org>
---

After spending an afternoon wgetting patches to applying them, I got fed up
and just added basic url support to git-apply. Note that the fwrite_strbuf
function should just live in http.c, but until the http api is fully switched
to use strbufs instead of its struct buffer (which I'm planning to do), it
can stay like this.

 Documentation/git-apply.txt |    3 +-
 builtin-apply.c             |   66 +++++++++++++++++++++++++++++++++++++------
 2 files changed, 59 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index bae3e7b..2d5d725 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -25,7 +25,8 @@ OPTIONS
 -------
 <patch>...::
 	The files to read patch from.  '-' can be used to read
-	from the standard input.
+	from the standard input. They can also be http, https or
+	ftp URLs.
 
 --stat::
 	Instead of applying the patch, output diffstat for the
diff --git a/builtin-apply.c b/builtin-apply.c
index 8c8162a..e213bd2 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -12,6 +12,9 @@
 #include "blob.h"
 #include "delta.h"
 #include "builtin.h"
+#ifndef NO_CURL
+#include "http.h"
+#endif
 
 /*
  *  --check turns on checking that the working tree matches the
@@ -182,12 +185,55 @@ static void say_patch_name(FILE *output, const char *pre,
 #define CHUNKSIZE (8192)
 #define SLOP (16)
 
-static void read_patch_file(struct strbuf *sb, const char *filename)
+#ifndef NO_CURL
+static size_t fwrite_strbuf(const void *ptr, size_t eltsize,
+			size_t nmemb, struct strbuf *buffer)
+{
+	size_t size = eltsize * nmemb;
+	strbuf_add(buffer, ptr, size);
+	return size;
+}
+
+static int used_http;
+
+static void read_patch_url(struct strbuf *sb, const char *url)
+{
+	struct active_request_slot *slot;
+	struct slot_results results;
+
+	if (! used_http) {
+		http_init();
+		used_http = 1;
+	}
+
+	slot = get_active_slot();
+	slot->results = &results;
+	curl_easy_setopt(slot->curl, CURLOPT_FILE, sb);
+	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_strbuf);
+	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);
+
+	if (start_active_slot(slot)) {
+		run_active_slot(slot);
+		if (results.curl_result != CURLE_OK)
+			die("git-apply: could not open %s", url);
+	}
+}
+#endif
+
+static void read_patch(struct strbuf *sb, const char *filename)
 {
 	int fd;
 
 	if (!strcmp(filename, "-")) {
 		fd = 0;
+#ifndef NO_CURL
+	} else if (!strncmp(filename, "http://", 7) ||
+		   !strncmp(filename, "https://", 8) ||
+		   !strncmp(filename, "ftp://", 6)) {
+		read_patch_url(sb, filename);
+		return;
+#endif
 	} else {
 		fd = open(filename, O_RDONLY);
 		if (fd < 0)
@@ -199,13 +245,6 @@ static void read_patch_file(struct strbuf *sb, const char *filename)
 		die("git-apply: read returned %s", strerror(errno));
 
 	close(fd);
-	/*
-	 * Make sure that we have some slop in the buffer
-	 * so that we can do speculative "memcmp" etc, and
-	 * see to it that it is NUL-filled.
-	 */
-	strbuf_grow(sb, SLOP);
-	memset(sb->buf + sb->len, 0, SLOP);
 }
 
 static unsigned long linelen(const char *buffer, unsigned long size)
@@ -2726,7 +2765,14 @@ static int apply_patch(const char *filename, int inaccurate_eof)
 
 	strbuf_init(&buf, 0);
 	patch_input_file = filename;
-	read_patch_file(&buf, filename);
+	read_patch(&buf, filename);
+	/*
+	 * Make sure that we have some slop in the buffer
+	 * so that we can do speculative "memcmp" etc, and
+	 * see to it that it is NUL-filled.
+	 */
+	strbuf_grow(&buf, SLOP);
+	memset(buf.buf + buf.len, 0, SLOP);
 	offset = 0;
 	while (offset < buf.len) {
 		struct patch *patch;
@@ -2926,6 +2972,8 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
 		set_default_whitespace_mode(whitespace_option);
 		errs |= apply_patch(arg, inaccurate_eof);
 	}
+	if (used_http)
+		http_cleanup();
 	set_default_whitespace_mode(whitespace_option);
 	if (read_stdin)
 		errs |= apply_patch("-", inaccurate_eof);
-- 
1.5.3.7

Re: [PATCH 2/2] Add support for URLs to git-apply

[Mike Hommey]

On Sun, Dec 09, 2007 at 11:04:32AM +0100, Mike Hommey wrote:
> Instead of doing several "wget -O - url | git-apply -" in a raw, you now
> can just git-apply url1 url2 ...
> 
> Signed-off-by: Mike Hommey <mh@glandium.org>
> ---
> 
> After spending an afternoon wgetting patches to applying them, I got fed up
> and just added basic url support to git-apply. Note that the fwrite_strbuf
> function should just live in http.c, but until the http api is fully switched
> to use strbufs instead of its struct buffer (which I'm planning to do), it
> can stay like this.

I'm going to resend these 2 patches, with some others that do some changes
to the http code.

Mike

Re: [PATCH 2/2] Add support for URLs to git-apply

[Andreas Ericsson]

Mike Hommey wrote:
> Instead of doing several "wget -O - url | git-apply -" in a raw, you now
> can just git-apply url1 url2 ...
> 

I seriously like this idea. Combined with gitweb (or cgit), it could be
used as a cherry-pick from someone else's repo :)

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH 2/2] Add support for URLs to git-apply

[Junio C Hamano]

Andreas Ericsson <ae@op5.se> writes:

> Mike Hommey wrote:
>> Instead of doing several "wget -O - url | git-apply -" in a raw, you now
>> can just git-apply url1 url2 ...
>>
>
> I seriously like this idea. Combined with gitweb (or cgit), it could be
> used as a cherry-pick from someone else's repo :)

FWIW, my initial impression is that I seriously dislike this.  It may be
good if the patch were to git-am, but when git-apply rejects an
inapplicable patch, there won't be nothing left for you to recover with
and you need to re-download the patch anyway.

Note that I said my "initial" impression.  I reserve the right to change
my mind, as always ;-)

Re: [PATCH 2/2] Add support for URLs to git-apply

[Mike Hommey]

On Sun, Dec 09, 2007 at 02:54:58PM -0800, Junio C Hamano wrote:
> Andreas Ericsson <ae@op5.se> writes:
> 
> > Mike Hommey wrote:
> >> Instead of doing several "wget -O - url | git-apply -" in a raw, you now
> >> can just git-apply url1 url2 ...
> >>
> >
> > I seriously like this idea. Combined with gitweb (or cgit), it could be
> > used as a cherry-pick from someone else's repo :)
> 
> FWIW, my initial impression is that I seriously dislike this.  It may be
> good if the patch were to git-am, but when git-apply rejects an
> inapplicable patch, there won't be nothing left for you to recover with
> and you need to re-download the patch anyway.

There are some usecase differences between git-apply and git-am.
Probably, this change would be good to have on both.

Mike

Re: [PATCH 2/2] Add support for URLs to git-apply

[Johannes Schindelin]

Hi,

On Mon, 10 Dec 2007, Mike Hommey wrote:

> On Sun, Dec 09, 2007 at 02:54:58PM -0800, Junio C Hamano wrote:
> > Andreas Ericsson <ae@op5.se> writes:
> > 
> > > Mike Hommey wrote:
> > >> Instead of doing several "wget -O - url | git-apply -" in a raw, 
> > >> you now can just git-apply url1 url2 ...
> > >>
> > >
> > > I seriously like this idea. Combined with gitweb (or cgit), it could 
> > > be used as a cherry-pick from someone else's repo :)
> > 
> > FWIW, my initial impression is that I seriously dislike this.  It may 
> > be good if the patch were to git-am, but when git-apply rejects an 
> > inapplicable patch, there won't be nothing left for you to recover 
> > with and you need to re-download the patch anyway.
> 
> There are some usecase differences between git-apply and git-am. 
> Probably, this change would be good to have on both.

But what about Junio's comments about a failed patch?  You really want to 
hammer that poor webserver?

My first thought when seeing your patch was: this would give us a chance 
to "clone" via gitweb.  And while doing so, all but kill those webservers.  
So I thought it was wrong.

When Junio mentioned git-am it was obvious to me that this is the "right" 
solution.

I mean, we go out of our way to be nice to the servers, putting more load 
onto the clients, because there are many clients, but only one server 
(which is unfair).

Please address these issues before further arguing that both apply and am 
should learn about URLs.

Ciao,
Dscho