threads / patch / 5441

patch, 3 partsgit-daemon: plug new upload-tar command

Subject: [PATCH 0/3] git-daemon: plug new upload-tar command

## tl;dr

11 messages between Aug 31, 2006 and Sep 2, 2006. Diffs are folded; open one to read it.

replies: 10people: 4as markdown or json

Franck Bui-Huu· Aug 31, 2006, 12:35 UTC · lore

Here's a simple patchset that basically teach git-daemon about the upload-tar command added by Junio's commit:

217542640ed219c980fff2b3c307c4520120f20f
With a patched daemon it's now possible to do:
$ git tar-tree --remote=git://host/path/to/repo HEAD > repo.tar

It also creates a 'service' structure to ease future command integration (like git internal commands handling does). I'm not sure how usefull it will be, but it improves at least code readability.

Something that may be usefull now is to make git-daemon able to compress data it sends back...

		Franck
---
 builtin-tar-tree.c |    2 +
 daemon.c           |   85 ++++++++++++++++++++++++++++++++++++++++++----------
 git.c              |    2 +
 3 files changed, 71 insertions(+), 18 deletions(-)
Franck Bui-Huu· Aug 31, 2006, 12:35 UTC · re: Franck Bui-Huu · lore

[PATCH 1/3] daemon.c: introduce daemon's service

Signed-off-by: Franck Bui-Huu <vagabon.xyz@gmail.com>
---
 daemon.c |   78 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 61 insertions(+), 17 deletions(-)
Show changes to daemon.c +61 −17
diff --git a/daemon.c b/daemon.c
index 66ec830..ed3a13d 100644
--- a/daemon.c
+++ b/daemon.c
@@ -232,16 +232,57 @@ static char *path_ok(char *dir)
 	return NULL;		/* Fallthrough. Deny by default */
 }
 
-static int upload(char *dir)
+/*
+ * Services we're able to deal with.
+ */
+static int service_upload_pack(const char *dir, const char *args)
 {
 	/* Timeout as string */
 	char timeout_buf[64];
+
+	snprintf(timeout_buf, sizeof timeout_buf, "--timeout=%u", timeout);
+
+	/* git-upload-pack only ever reads stuff, so this is safe */
+	execl_git_cmd("upload-pack", "--strict", timeout_buf, ".", NULL);
+	return -1;
+}
+
+/* service options */
+#define NEED_REPO	(1<<0)
+
+struct service_info {
+	const char *name;
+	int (*fn)(const char *dir, const char *args);
+	int options;
+};
+
+static struct service_info services[] = {
+	{ "git-upload-pack", service_upload_pack, NEED_REPO },
+};
+
+static int run_service(char *cmdline)
+{
+	struct service_info *serv;
 	const char *path;
+	size_t len;
+	int i;
 
-	loginfo("Request for '%s'", dir);
+	for (i = 0; i < ARRAY_SIZE(services); i++) {
+		serv = &services[i];
+		
+		len = strlen(serv->name);
+		if (strncmp(cmdline, serv->name, len))
+			continue;
+		if (cmdline[len] != ' ')
+			continue;
+		goto found;
+	}
+	return -1;
+found:
+	cmdline += len + 1;
+	path = NULL;
 
-	if (!(path = path_ok(dir)))
-		return -1;
+	loginfo("Request '%s' for '%s'", serv->name, cmdline);
 
 	/*
 	 * Security on the cheap.
@@ -253,30 +294,33 @@ static int upload(char *dir)
 	 * path_ok() uses enter_repo() and does whitelist checking.
 	 * We only need to make sure the repository is exported.
 	 */
+	if (serv->options & NEED_REPO) {
+		if (!(path = path_ok(cmdline)))
+			return -1;
 
-	if (!export_all_trees && access("git-daemon-export-ok", F_OK)) {
-		logerror("'%s': repository not exported.", path);
-		errno = EACCES;
-		return -1;
+		if (!export_all_trees && access("git-daemon-export-ok", F_OK)) {
+			logerror("'%s': repository not exported.", path);
+			errno = EACCES;
+			return -1;
+		}
+		cmdline += strlen(path) + 1;
 	}
-
+	
 	/*
 	 * We'll ignore SIGTERM from now on, we have a
 	 * good client.
 	 */
 	signal(SIGTERM, SIG_IGN);
 
-	snprintf(timeout_buf, sizeof timeout_buf, "--timeout=%u", timeout);
+	return serv->fn(path, cmdline);
 
-	/* git-upload-pack only ever reads stuff, so this is safe */
-	execl_git_cmd("upload-pack", "--strict", timeout_buf, ".", NULL);
-	return -1;
 }
 
 static int execute(struct sockaddr *addr)
 {
 	static char line[1000];
 	int pktlen, len;
+	int rv;
 
 	if (addr) {
 		char addrbuf[256] = "";
@@ -313,11 +357,11 @@ #endif
 	if (len && line[len-1] == '\n')
 		line[--len] = 0;
 
-	if (!strncmp("git-upload-pack ", line, 16))
-		return upload(line+16);
+	rv = run_service(line);
+	if (rv < 0)
+		logerror("Protocol error: '%s'", line);
 
-	logerror("Protocol error: '%s'", line);
-	return -1;
+	return rv;
 }
 
 

-- 
1.4.2
Franck Bui-Huu· Aug 31, 2006, 13:07 UTC · re: Franck Bui-Huu · lore

Re: [PATCH 1/3] daemon.c: introduce daemon's service

Franck Bui-Huu wrote:
Show 53 quoted lines
> Signed-off-by: Franck Bui-Huu <vagabon.xyz@gmail.com>
> ---
>  daemon.c |   78 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
>  1 files changed, 61 insertions(+), 17 deletions(-)
> 
> diff --git a/daemon.c b/daemon.c
> index 66ec830..ed3a13d 100644
> --- a/daemon.c
> +++ b/daemon.c
> @@ -232,16 +232,57 @@ static char *path_ok(char *dir)
>  	return NULL;		/* Fallthrough. Deny by default */
>  }
>  
> -static int upload(char *dir)
> +/*
> + * Services we're able to deal with.
> + */
> +static int service_upload_pack(const char *dir, const char *args)
>  {
>  	/* Timeout as string */
>  	char timeout_buf[64];
> +
> +	snprintf(timeout_buf, sizeof timeout_buf, "--timeout=%u", timeout);
> +
> +	/* git-upload-pack only ever reads stuff, so this is safe */
> +	execl_git_cmd("upload-pack", "--strict", timeout_buf, ".", NULL);
> +	return -1;
> +}
> +
> +/* service options */
> +#define NEED_REPO	(1<<0)
> +
> +struct service_info {
> +	const char *name;
> +	int (*fn)(const char *dir, const char *args);
> +	int options;
> +};
> +
> +static struct service_info services[] = {
> +	{ "git-upload-pack", service_upload_pack, NEED_REPO },
> +};
> +
> +static int run_service(char *cmdline)
> +{
> +	struct service_info *serv;
>  	const char *path;
> +	size_t len;
> +	int i;
>  
> -	loginfo("Request for '%s'", dir);
> +	for (i = 0; i < ARRAY_SIZE(services); i++) {
> +		serv = &services[i];
> +		
OMG, trailing white spaces !
Show 39 quoted lines
> +		len = strlen(serv->name);
> +		if (strncmp(cmdline, serv->name, len))
> +			continue;
> +		if (cmdline[len] != ' ')
> +			continue;
> +		goto found;
> +	}
> +	return -1;
> +found:
> +	cmdline += len + 1;
> +	path = NULL;
>  
> -	if (!(path = path_ok(dir)))
> -		return -1;
> +	loginfo("Request '%s' for '%s'", serv->name, cmdline);
>  
>  	/*
>  	 * Security on the cheap.
> @@ -253,30 +294,33 @@ static int upload(char *dir)
>  	 * path_ok() uses enter_repo() and does whitelist checking.
>  	 * We only need to make sure the repository is exported.
>  	 */
> +	if (serv->options & NEED_REPO) {
> +		if (!(path = path_ok(cmdline)))
> +			return -1;
>  
> -	if (!export_all_trees && access("git-daemon-export-ok", F_OK)) {
> -		logerror("'%s': repository not exported.", path);
> -		errno = EACCES;
> -		return -1;
> +		if (!export_all_trees && access("git-daemon-export-ok", F_OK)) {
> +			logerror("'%s': repository not exported.", path);
> +			errno = EACCES;
> +			return -1;
> +		}
> +		cmdline += strlen(path) + 1;
>  	}
> -
> +	
ditto
		Franck
Franck Bui-Huu· Aug 31, 2006, 15:15 UTC · re: Franck Bui-Huu · lore

[PATCH 1/3] daemon.c: introduce daemon's service [take #2]

Signed-off-by: Franck Bui-Huu <vagabon.xyz@gmail.com>
---
 daemon.c |   77 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 60 insertions(+), 17 deletions(-)
Show changes to daemon.c +60 −17
diff --git a/daemon.c b/daemon.c
index 66ec830..f5f6927 100644
--- a/daemon.c
+++ b/daemon.c
@@ -232,16 +232,57 @@ static char *path_ok(char *dir)
 	return NULL;		/* Fallthrough. Deny by default */
 }
 
-static int upload(char *dir)
+/*
+ * Services we're able to deal with.
+ */
+static int service_upload_pack(const char *dir, const char *args)
 {
 	/* Timeout as string */
 	char timeout_buf[64];
+
+	snprintf(timeout_buf, sizeof timeout_buf, "--timeout=%u", timeout);
+
+	/* git-upload-pack only ever reads stuff, so this is safe */
+	execl_git_cmd("upload-pack", "--strict", timeout_buf, ".", NULL);
+	return -1;
+}
+
+/* service options */
+#define NEED_REPO	(1<<0)
+
+struct service_info {
+	const char *name;
+	int (*fn)(const char *dir, const char *args);
+	int options;
+};
+
+static struct service_info services[] = {
+	{ "git-upload-pack", service_upload_pack, NEED_REPO },
+};
+
+static int run_service(char *cmdline)
+{
+	struct service_info *serv;
 	const char *path;
+	size_t len;
+	int i;
 
-	loginfo("Request for '%s'", dir);
+	for (i = 0; i < ARRAY_SIZE(services); i++) {
+		serv = &services[i];
 
-	if (!(path = path_ok(dir)))
-		return -1;
+		len = strlen(serv->name);
+		if (strncmp(cmdline, serv->name, len))
+			continue;
+		if (cmdline[len] != ' ')
+			continue;
+		goto found;
+	}
+	return -1;
+found:
+	cmdline += len + 1;
+	path = NULL;
+
+	loginfo("Request '%s' for '%s'", serv->name, cmdline);
 
 	/*
 	 * Security on the cheap.
@@ -253,11 +294,16 @@ static int upload(char *dir)
 	 * path_ok() uses enter_repo() and does whitelist checking.
 	 * We only need to make sure the repository is exported.
 	 */
+	if (serv->options & NEED_REPO) {
+		if (!(path = path_ok(cmdline)))
+			return -1;
 
-	if (!export_all_trees && access("git-daemon-export-ok", F_OK)) {
-		logerror("'%s': repository not exported.", path);
-		errno = EACCES;
-		return -1;
+		if (!export_all_trees && access("git-daemon-export-ok", F_OK)) {
+			logerror("'%s': repository not exported.", path);
+			errno = EACCES;
+			return -1;
+		}
+		cmdline += strlen(path) + 1;
 	}
 
 	/*
@@ -266,17 +312,14 @@ static int upload(char *dir)
 	 */
 	signal(SIGTERM, SIG_IGN);
 
-	snprintf(timeout_buf, sizeof timeout_buf, "--timeout=%u", timeout);
-
-	/* git-upload-pack only ever reads stuff, so this is safe */
-	execl_git_cmd("upload-pack", "--strict", timeout_buf, ".", NULL);
-	return -1;
+	return serv->fn(path, cmdline);
 }
 
 static int execute(struct sockaddr *addr)
 {
 	static char line[1000];
 	int pktlen, len;
+	int rv;
 
 	if (addr) {
 		char addrbuf[256] = "";
@@ -313,11 +356,11 @@ #endif
 	if (len && line[len-1] == '\n')
 		line[--len] = 0;
 
-	if (!strncmp("git-upload-pack ", line, 16))
-		return upload(line+16);
+	rv = run_service(line);
+	if (rv < 0)
+		logerror("Protocol error: '%s'", line);
 
-	logerror("Protocol error: '%s'", line);
-	return -1;
+	return rv;
 }
 
 

-- 
1.4.2
Franck Bui-Huu· Aug 31, 2006, 12:35 UTC · re: Franck Bui-Huu · lore

[PATCH 2/3] daemon.c: added upload-tar service

Signed-off-by: Franck Bui-Huu <vagabon.xyz@gmail.com>
---
 daemon.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
Show changes to daemon.c +7 −0
diff --git a/daemon.c b/daemon.c
index ed3a13d..381e6ae 100644
--- a/daemon.c
+++ b/daemon.c
@@ -247,6 +247,12 @@ static int service_upload_pack(const cha
 	return -1;
 }
 
+static int service_upload_tar(const char *dir, const char *args)
+{
+	execl_git_cmd("upload-tar", dir, NULL);
+	return -1;
+}
+
 /* service options */
 #define NEED_REPO	(1<<0)
 
@@ -258,6 +264,7 @@ struct service_info {
 
 static struct service_info services[] = {
 	{ "git-upload-pack", service_upload_pack, NEED_REPO },
+	{ "git-upload-tar", service_upload_tar, NEED_REPO },
 };
 
 static int run_service(char *cmdline)

-- 
1.4.2
Franck Bui-Huu· Aug 31, 2006, 12:35 UTC · re: Franck Bui-Huu · lore

[PATCH 3/3] git-tar-tree.c: no need to be in a git repo when using --remote

Signed-off-by: Franck Bui-Huu <vagabon.xyz@gmail.com>
---
 builtin-tar-tree.c |    2 ++
 git.c              |    2 +-
 2 files changed, 3 insertions(+), 1 deletions(-)
Show changes to 2 files +3 −1

builtin-tar-tree.c, git.c

diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c
index 61a4135..ad1a8a6 100644
--- a/builtin-tar-tree.c
+++ b/builtin-tar-tree.c
@@ -390,6 +390,8 @@ int cmd_tar_tree(int argc, const char **
 		usage(tar_tree_usage);
 	if (!strncmp("--remote=", argv[1], 9))
 		return remote_tar(argc, argv);
+	if (prefix == NULL)
+		prefix = setup_git_directory();
 	return generate_tar(argc, argv, prefix);
 }
 
diff --git a/git.c b/git.c
index bd07289..c323d30 100644
--- a/git.c
+++ b/git.c
@@ -262,7 +262,7 @@ static void handle_internal_command(int 
 		{ "show", cmd_show, RUN_SETUP | USE_PAGER },
 		{ "stripspace", cmd_stripspace },
 		{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
-		{ "tar-tree", cmd_tar_tree, RUN_SETUP },
+		{ "tar-tree", cmd_tar_tree },
 		{ "zip-tree", cmd_zip_tree, RUN_SETUP },
 		{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },
 		{ "update-index", cmd_update_index, RUN_SETUP },

-- 
1.4.2
Rene Scharfe· Aug 31, 2006, 17:45 UTC · re: Franck Bui-Huu · lore

Re: [PATCH 0/3] git-daemon: plug new upload-tar command

Hi,
Franck Bui-Huu schrieb:
> Here's a simple patchset that basically teach git-daemon about the 
> upload-tar command added by Junio's commit:

Junio has similar code in the 'next' branch. And he correctly pointed out that with the arrival of git-zip-tree the time has come to think about a format independent tree-to-archive converter command to avoid re-implementing essentially the same thing under the names of git-upload-zip, git-upload-rar etc.

I'm trying for a few days now to find time for implementing a git-archive command, but I'm failing. And I won't be able to do so before the weekend (at least).

I propose to make the command line syntax more similar to the one of git-ls-tree (e.g. --prefix instead of optional second non-option parameter for base dir, support for path specs). In a previous mail I also proposed to merge the upload command into git-archive, but now that I thought a bit about it it doesn't make sense to me anymore.

So if you beat me to it, that would be great. Or if you have a better idea, that would be also great. :-)

I have to go now..
René
Franck Bui-Huu· Sep 2, 2006, 08:14 UTC · re: Rene Scharfe · lore

Re: [PATCH 0/3] git-daemon: plug new upload-tar command

Hi,
2006/8/31, Rene Scharfe <rene.scharfe@lsrfire.ath.cx>:
> Junio has similar code in the 'next' branch.  And he correctly pointed
duh, wasn't aware of it. :(
Show 5 quoted lines
> out that with the arrival of git-zip-tree the time has come to think
> about a format independent tree-to-archive converter command to avoid
> re-implementing essentially the same thing under the names of
> git-upload-zip, git-upload-rar etc.
>

That would make sense if you're going to implement any possible archive formats ;)

Show 10 quoted lines
> I'm trying for a few days now to find time for implementing a
> git-archive command, but I'm failing.  And I won't be able to do so
> before the weekend (at least).
>
> I propose to make the command line syntax more similar to the one of
> git-ls-tree (e.g. --prefix instead of optional second non-option
> parameter for base dir, support for path specs).  In a previous mail I
> also proposed to merge the upload command into git-archive, but now that
> I thought a bit about it it doesn't make sense to me anymore.
>

Maybe it's time for a "upload-archive" that would implement a common git archiver protocol used by all git-{zip,tar,..}-tree. And by that time move git-tar-tree protocol stuff into git-archive.

> So if you beat me to it, that would be great.  Or if you have a better
> idea, that would be also great. :-)
>

Well I'll try to start something, not sure to have a lot of time though. Please contact me before starting anything, I would be sad to write something for /dev/null again ;)

One point would make git-tar-tree able to compress data and git-daemon would refuse any requests for a non compressed archive format.

-- 
               Franck

-- 
VGER BF report: U 0.499999
Junio C Hamano· Sep 2, 2006, 10:32 UTC · re: Franck Bui-Huu · lore

Re: [PATCH 0/3] git-daemon: plug new upload-tar command

"Franck Bui-Huu" <vagabon.xyz@gmail.com> writes:
Show 6 quoted lines
>> So if you beat me to it, that would be great.  Or if you have a better
>> idea, that would be also great. :-)
>
> Well I'll try to start something, not sure to have a lot of time
> though. Please contact me before starting anything, I would be sad to
> write something for /dev/null again ;)

I do not necessarily think your effort were for /dev/null; for example, I was hoping you defend [PATCH 3/3].

While it makes sense to make "tar-tree --remote" usable outside a git managed repository, I think people expect the connection to obey core.gitproxy if the command is run inside a repository that has a configuration file.

Jakub Narebski· Sep 2, 2006, 10:36 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/3] git-daemon: plug new upload-tar command

Junio C Hamano wrote:
Show 16 quoted lines
> "Franck Bui-Huu" <vagabon.xyz@gmail.com> writes:
> 
>>> So if you beat me to it, that would be great.  Or if you have a better
>>> idea, that would be also great. :-)
>>
>> Well I'll try to start something, not sure to have a lot of time
>> though. Please contact me before starting anything, I would be sad to
>> write something for /dev/null again ;)
> 
> I do not necessarily think your effort were for /dev/null; for
> example, I was hoping you defend [PATCH 3/3].
> 
> While it makes sense to make "tar-tree --remote" usable outside
> a git managed repository, I think people expect the connection
> to obey core.gitproxy if the command is run inside a repository
> that has a configuration file.

If I remember correclty git now support (totally undocumented) per-user configuration file, so one can have core.gitproxy even outside git repository. Although some warning that you are running from outside repository and configuration might be not what you want would be nice. 

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Franck Bui-Huu· Sep 2, 2006, 20:12 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/3] git-daemon: plug new upload-tar command

2006/9/2, Junio C Hamano <junkio@cox.net>:
Show 12 quoted lines
> "Franck Bui-Huu" <vagabon.xyz@gmail.com> writes:
>
> >> So if you beat me to it, that would be great.  Or if you have a better
> >> idea, that would be also great. :-)
> >
> > Well I'll try to start something, not sure to have a lot of time
> > though. Please contact me before starting anything, I would be sad to
> > write something for /dev/null again ;)
>
> I do not necessarily think your effort were for /dev/null; for
> example, I was hoping you defend [PATCH 3/3].
>

Well, with a new git-archive-tree command, I thought it would make sense to put the remote logic there and let the git-tar-tree be a local command. But Rene has a different approach, please see his new thread "Add git-archive-tree".

Show 5 quoted lines
> While it makes sense to make "tar-tree --remote" usable outside
> a git managed repository, I think people expect the connection
> to obey core.gitproxy if the command is run inside a repository
> that has a configuration file.
>
Make sense.
-- 
               Franck

-- 
VGER BF report: U 0.799618

← back to recent threads