threads / patch / 11209

patch, 2 partsAdd support for URLs to git-apply

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

## tl;dr

7 messages between Dec 9, 2007 and Dec 10, 2007. Diffs are folded; open one to read it.

replies: 6people: 4as markdown or json

Mike Hommey· Dec 9, 2007, 10:04 UTC · lore

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

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(-)
Show changes to builtin-apply.c +17 −15
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
Mike Hommey· Dec 9, 2007, 10:04 UTC · re: Mike Hommey · lore

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(-)
Show changes to 2 files +59 −10

Documentation/git-apply.txt, builtin-apply.c

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
Mike Hommey· Dec 9, 2007, 17:02 UTC · re: Mike Hommey · lore

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

On Sun, Dec 09, 2007 at 11:04:32AM +0100, Mike Hommey wrote:
Show 11 quoted lines
> 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
Andreas Ericsson· Dec 9, 2007, 21:04 UTC · re: Mike Hommey · lore

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

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
Junio C Hamano· Dec 9, 2007, 22:54 UTC · re: Andreas Ericsson · lore

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

Andreas Ericsson <ae@op5.se> writes:
Show 7 quoted lines
> 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 ;-)

Mike Hommey· Dec 10, 2007, 06:46 UTC · re: Junio C Hamano · lore

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

On Sun, Dec 09, 2007 at 02:54:58PM -0800, Junio C Hamano wrote:
Show 14 quoted lines
> 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
Johannes Schindelin· Dec 10, 2007, 11:16 UTC · re: Mike Hommey · lore

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

Hi,
On Mon, 10 Dec 2007, Mike Hommey wrote:
Show 18 quoted lines
> 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

← back to recent threads