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

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

[Franck Bui-Huu]

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(-)

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

[Franck Bui-Huu]

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];
+		
+		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

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

[Franck Bui-Huu]

Signed-off-by: Franck Bui-Huu <vagabon.xyz@gmail.com>
---
 daemon.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

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

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

[Franck Bui-Huu]

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(-)

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

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

[Franck Bui-Huu]

Franck Bui-Huu wrote:
> 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 !

> +		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

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

[Franck Bui-Huu]

Signed-off-by: Franck Bui-Huu <vagabon.xyz@gmail.com>
---
 daemon.c |   77 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 60 insertions(+), 17 deletions(-)

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

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

[Rene Scharfe]

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é

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

[Franck Bui-Huu]

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. :(

> 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 ;)

> 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

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

[Junio C Hamano]

"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.

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

[Jakub Narebski]

Junio C Hamano wrote:

> "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

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

[Franck Bui-Huu]

2006/9/2, Junio C Hamano <junkio@cox.net>:
> "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".

> 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