threads / rfc / 5453

patch[RFC] Add git-archive-tree

Subject: [PATCH][RFC] Add git-archive-tree

## tl;dr

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

replies: 16people: 3as markdown or json

Rene Scharfe· Sep 2, 2006, 12:23 UTC · lore

git-archive-tree is a command to make tar and ZIP archives of a git tree. It helps prevent a proliferation of git-{format}-tree commands. This is useful e.g. for remote archive fetching because we only need to write a single upload and a single download program that simply pass on the format option to git-archive-tree.

Speaking of remote, please note the absence of the --remote option of git-tar-tree. This is intentional; remote operations are special enough to deserve a separate (yet to be written) command.

Currently git-archive-tree -f tar is slower than git-tar-tree. This is because it is welded to the side of the existing code to minimize patch size, and I also suspect read_tree_recursive() to be quite a bit slower than builtin-tar-tree.c::traverse_tree().

 Documentation/git-archive-tree.txt |   99 +++++++++++++++++++++++++++++++++++++
 Makefile                           |    3 -
 archive.h                          |    6 ++
 builtin-archive-tree.c             |   92 ++++++++++++++++++++++++++++++++++
 builtin-tar-tree.c                 |   66 ++++++++++++++++++++++++
 builtin-zip-tree.c                 |   28 ++++++++++
 builtin.h                          |    1 
 git.c                              |    1 
 8 files changed, 295 insertions(+), 1 deletion(-)
Show changes to 8 files +295 −1

Documentation/git-archive-tree.txt, Makefile, archive.h, builtin-archive-tree.c, builtin-tar-tree.c, builtin-zip-tree.c, builtin.h, git.c

diff --git a/Documentation/git-archive-tree.txt b/Documentation/git-archive-tree.txt
new file mode 100644
index 0000000..122c482
--- /dev/null
+++ b/Documentation/git-archive-tree.txt
@@ -0,0 +1,99 @@
+git-archive-tree(1)
+===============
+
+NAME
+----
+git-archive-tree - Creates a archive of the files in the named tree
+
+
+SYNOPSIS
+--------
+'git-archive-tree' -f {tar|zip} [--prefix=<prefix>/] [-0|...|-9]
+	    <tree-ish> [path...]
+
+DESCRIPTION
+-----------
+Creates an archive of the specified format containing the tree structure
+for the named tree.  If <prefix> is specified it is prepended to the
+filenames in the archive.
+
+git-archive-tree behaves differently when given a tree ID versus when
+given a commit ID or tag ID.  In the first case the current time is used
+as modification time of each file in the archive.  In the latter case the
+commit time as recorded in the referenced commit object is used instead.
+Additionally the commit ID is stored in a global extended pax header if
+the tar format is used; it can be extracted using git-get-tar-commit-id.
+In ZIP files it is stored as a file comment.
+
+OPTIONS
+-------
+
+-f::
+	Format of the resulting archive, can be either 'tar' or 'zip'.
+
+<tree-ish>::
+	The tree or commit to produce an archive for.
+
+path::
+	If one or more paths are specified, include only these in the
+	archive, otherwise include all files and subdirectories.
+
+--prefix=<prefix>/::
+	Prepend <prefix>/ to each filename in the archive.
+
+-0::
+	Store files in the archive instead of compressing them.  This
+	option has no effect when the tar format is used.
+
+-9::
+        Highest and slowest compression level.  You can specify any
+        number from 1 to 9 to adjust compression speed and ratio.  This
+	option has no effect when the tar format is used.
+
+CONFIGURATION
+-------------
+By default, file and directories modes are set to 0666 or 0777 in tar
+archives.  It is possible to change this by setting the "umask" variable
+in the repository configuration as follows :
+
+[tar]
+        umask = 002	;# group friendly
+
+The special umask value "user" indicates that the user's current umask
+will be used instead. The default value remains 0, which means world
+readable/writable files and directories.
+
+EXAMPLES
+--------
+git archive -f tar --prefix=junk/ HEAD | (cd /var/tmp/ && tar xf -)::
+
+	Create a tar archive that contains the contents of the
+	latest commit on the current branch, and extracts it in
+	`/var/tmp/junk` directory.
+
+git archive -f tar --prefix=git-1.4.0/ v1.4.0 | gzip >git-1.4.0.tar.gz::
+
+	Create a compressed tarball for v1.4.0 release.
+
+git archive -f tar --prefix=git-1.4.0/ v1.4.0{caret}\{tree\} | gzip >git-1.4.0.tar.gz::
+
+	Create a compressed tarball for v1.4.0 release, but without a
+	global extended pax header.
+
+git archive -f zip --prefix=git-docs/ HEAD:Documentation/ > git-1.4.0-docs.zip::
+
+	Put everything in the current head's Documentation/ directory
+	into 'git-1.4.0-docs.zip', with the prefix 'git-docs/'.
+
+Author
+------
+Written by Rene Scharfe.
+
+Documentation
+--------------
+Documentation by David Greaves, Junio C Hamano and the git-list <git@vger.kernel.org>.
+
+GIT
+---
+Part of the gitlink:git[7] suite
+
diff --git a/Makefile b/Makefile
index 05bd77f..d0a1055 100644
--- a/Makefile
+++ b/Makefile
@@ -231,7 +231,7 @@ LIB_FILE=libgit.a
 XDIFF_LIB=xdiff/lib.a
 
 LIB_H = \
-	blob.h cache.h commit.h csum-file.h delta.h \
+	archive.h blob.h cache.h commit.h csum-file.h delta.h \
 	diff.h object.h pack.h pkt-line.h quote.h refs.h \
 	run-command.h strbuf.h tag.h tree.h git-compat-util.h revision.h \
 	tree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h
@@ -255,6 +255,7 @@ LIB_OBJS = \
 BUILTIN_OBJS = \
 	builtin-add.o \
 	builtin-apply.o \
+	builtin-archive-tree.o \
 	builtin-cat-file.o \
 	builtin-checkout-index.o \
 	builtin-check-ref-format.o \
diff --git a/archive.h b/archive.h
new file mode 100644
index 0000000..7813962
--- /dev/null
+++ b/archive.h
@@ -0,0 +1,6 @@
+#include "tree.h"
+
+typedef int (*write_archive_fn_t)(struct tree *tree, const unsigned char *commit_sha1, const char *prefix, time_t time, const char **pathspec);
+
+int write_tar_archive(struct tree *tree, const unsigned char *commit_sha1, const char *prefix, time_t time, const char **pathspec);
+int write_zip_archive(struct tree *tree, const unsigned char *commit_sha1, const char *prefix, time_t time, const char **pathspec);
diff --git a/builtin-archive-tree.c b/builtin-archive-tree.c
new file mode 100644
index 0000000..2c6ee60
--- /dev/null
+++ b/builtin-archive-tree.c
@@ -0,0 +1,92 @@
+/*
+ * Copyright (c) 2006 Rene Scharfe
+ */
+#include <time.h>
+#include "cache.h"
+#include "builtin.h"
+#include "commit.h"
+#include "tree-walk.h"
+#include "archive.h"
+
+static const char archive_usage[] =
+"git-archive-tree -f {tar|zip} [--prefix=<prefix>/] [-0|...|-9] <tree-ish> [path...]";
+
+static write_archive_fn_t parse_archive_format(const char *format)
+{
+	if (!strcmp(format, "tar"))
+		return write_tar_archive;
+	if (!strcmp(format, "zip"))
+		return write_zip_archive;
+	return NULL;
+}
+
+int cmd_archive_tree(int argc, const char **argv, const char *prefix)
+{
+	int more_args = 1;
+	const char *archive_prefix = "";
+	unsigned char sha1[20];
+	struct commit *commit;
+	time_t archive_time;
+	const char **pathspec;
+	const unsigned char *commit_sha1 = NULL;
+	write_archive_fn_t write_archive = NULL;
+	struct tree *tree;
+	int result;
+
+	while (argc > 2 && more_args) {
+		const char *arg = argv[1];
+		if (!strcmp(arg, "-f")) {
+			write_archive = parse_archive_format(argv[2]);
+			argv++;
+			argc--;
+		} else if (!strncmp(arg, "-f", 2))
+			write_archive = parse_archive_format(arg + 2);
+		else if (arg[0] == '-' && isdigit(arg[1]) && arg[2] == '\0')
+			zlib_compression_level = arg[1] - '0';
+		else if (!strcmp(arg, "--"))
+			more_args = 0;
+		else if (!strncmp(arg, "--prefix=", 9))
+			archive_prefix = arg + 9;
+		else if (arg[0] == '-')
+			usage(archive_usage);
+		else
+			break;
+		argv++;
+		argc--;
+	}
+
+	if (!write_archive)
+		usage(archive_usage);
+	if (argc < 2)
+		usage(archive_usage);
+	if (get_sha1(argv[1], sha1))
+		die("Not a valid object name %s", argv[1]);
+
+	commit = lookup_commit_reference_gently(sha1, 1);
+	if (commit)
+		commit_sha1 = commit->object.sha1;
+
+	tree = parse_tree_indirect(sha1);
+	if (!tree)
+		die("not a tree object");
+
+	if (prefix) {
+		unsigned char tree_sha1[20];
+		unsigned int mode;
+		int err = get_tree_entry(tree->object.sha1, prefix,
+		                         tree_sha1, &mode);
+		if (err || !S_ISDIR(mode))
+			die("current working directory is untracked");
+		free(tree);
+		tree = parse_tree_indirect(tree_sha1);
+	}
+
+	archive_time = commit ? commit->date : time(NULL);
+	pathspec = get_pathspec(archive_prefix, argv + 2);
+
+	result = write_archive(tree, commit_sha1, archive_prefix,
+	                       archive_time, pathspec);
+	free(tree);
+
+	return result;
+}
diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c
index 61a4135..e0da01e 100644
--- a/builtin-tar-tree.c
+++ b/builtin-tar-tree.c
@@ -9,6 +9,7 @@ #include "strbuf.h"
 #include "tar.h"
 #include "builtin.h"
 #include "pkt-line.h"
+#include "archive.h"
 
 #define RECORDSIZE	(512)
 #define BLOCKSIZE	(RECORDSIZE * 20)
@@ -338,6 +339,71 @@ static int generate_tar(int argc, const 
 	return 0;
 }
 
+static int write_tar_entry(const unsigned char *sha1,
+                           const char *base, int baselen,
+                           const char *filename, unsigned mode, int stage)
+{
+	static struct strbuf path;
+	int filenamelen = strlen(filename);
+	void *buffer;
+	char type[20];
+	unsigned long size;
+
+	if (!path.alloc) {
+		path.buf = xmalloc(PATH_MAX);
+		path.alloc = PATH_MAX;
+		path.len = path.eof = 0;
+	}
+	if (path.alloc < baselen + filenamelen) {
+		free(path.buf);
+		path.buf = xmalloc(baselen + filenamelen);
+		path.alloc = baselen + filenamelen;
+	}
+	memcpy(path.buf, base, baselen);
+	memcpy(path.buf + baselen, filename, filenamelen);
+	path.len = baselen + filenamelen;
+	if (S_ISDIR(mode)) {
+		strbuf_append_string(&path, "/");
+		buffer = NULL;
+		size = 0;
+	} else {
+		buffer = read_sha1_file(sha1, type, &size);
+		if (!buffer)
+			die("cannot read %s", sha1_to_hex(sha1));
+	}
+
+	write_entry(sha1, &path, mode, buffer, size);
+
+	return READ_TREE_RECURSIVE;
+}
+
+int write_tar_archive(struct tree *tree, const unsigned char *commit_sha1,
+                      const char *prefix, time_t time, const char **pathspec)
+{
+	int plen = strlen(prefix);
+
+	git_config(git_tar_config);
+
+	archive_time = time;
+
+	if (commit_sha1)
+		write_global_extended_header(commit_sha1);
+
+	if (prefix && plen > 0 && prefix[plen - 1] == '/') {
+		char *base = strdup(prefix);
+		int baselen = strlen(base);
+
+		while (baselen > 0 && base[baselen - 1] == '/')
+			base[--baselen] = '\0';
+		write_tar_entry(tree->object.sha1, "", 0, base, 040777, 0);
+		free(base);
+	}
+	read_tree_recursive(tree, prefix, plen, 0, pathspec, write_tar_entry);
+	write_trailer();
+
+	return 0;
+}
+
 static const char *exec = "git-upload-tar";
 
 static int remote_tar(int argc, const char **argv)
diff --git a/builtin-zip-tree.c b/builtin-zip-tree.c
index a5b834d..b142771 100644
--- a/builtin-zip-tree.c
+++ b/builtin-zip-tree.c
@@ -8,6 +8,7 @@ #include "blob.h"
 #include "tree.h"
 #include "quote.h"
 #include "builtin.h"
+#include "archive.h"
 
 static const char zip_tree_usage[] =
 "git-zip-tree [-0|...|-9] <tree-ish> [ <base> ]";
@@ -351,3 +352,30 @@ int cmd_zip_tree(int argc, const char **
 
 	return 0;
 }
+
+int write_zip_archive(struct tree *tree, const unsigned char *commit_sha1,
+                      const char *prefix, time_t time, const char **pathspec)
+{
+	int plen = strlen(prefix);
+
+	dos_time(&time, &zip_date, &zip_time);
+
+	zip_dir = xmalloc(ZIP_DIRECTORY_MIN_SIZE);
+	zip_dir_size = ZIP_DIRECTORY_MIN_SIZE;
+
+	if (prefix && plen > 0 && prefix[plen - 1] == '/') {
+		char *base = strdup(prefix);
+		int baselen = strlen(base);
+
+		while (baselen > 0 && base[baselen - 1] == '/')
+			base[--baselen] = '\0';
+		write_zip_entry(tree->object.sha1, "", 0, base, 040777, 0);
+		free(base);
+	}
+	read_tree_recursive(tree, prefix, plen, 0, pathspec, write_zip_entry);
+	write_zip_trailer(commit_sha1);
+
+	free(zip_dir);
+
+	return 0;
+}
diff --git a/builtin.h b/builtin.h
index 25431d7..febb9d0 100644
--- a/builtin.h
+++ b/builtin.h
@@ -53,6 +53,7 @@ extern int cmd_stripspace(int argc, cons
 extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
 extern int cmd_tar_tree(int argc, const char **argv, const char *prefix);
 extern int cmd_zip_tree(int argc, const char **argv, const char *prefix);
+extern int cmd_archive_tree(int argc, const char **argv, const char *prefix);
 extern int cmd_unpack_objects(int argc, const char **argv, const char *prefix);
 extern int cmd_update_index(int argc, const char **argv, const char *prefix);
 extern int cmd_update_ref(int argc, const char **argv, const char *prefix);
diff --git a/git.c b/git.c
index 05871ad..937b7f7 100644
--- a/git.c
+++ b/git.c
@@ -264,6 +264,7 @@ static void handle_internal_command(int 
 		{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
 		{ "tar-tree", cmd_tar_tree, RUN_SETUP },
 		{ "zip-tree", cmd_zip_tree, RUN_SETUP },
+		{ "archive-tree", cmd_archive_tree, RUN_SETUP },
 		{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },
 		{ "update-index", cmd_update_index, RUN_SETUP },
 		{ "update-ref", cmd_update_ref, RUN_SETUP },
-- 
VGER BF report: U 0.5
Rene Scharfe· Sep 2, 2006, 12:37 UTC · re: Rene Scharfe · lore

[PATCH] Add support for tgz archive format

 Documentation/git-archive-tree.txt |    9 ++---
 archive.h                          |    2 +
 builtin-archive-tree.c             |    4 +-
 builtin-tar-tree.c                 |   63 ++++++++++++++++++++++++++++++++-----
 4 files changed, 65 insertions(+), 13 deletions(-)
Show changes to 4 files +65 −13

Documentation/git-archive-tree.txt, archive.h, builtin-archive-tree.c, builtin-tar-tree.c

diff --git a/Documentation/git-archive-tree.txt b/Documentation/git-archive-tree.txt
index 122c482..25136d9 100644
--- a/Documentation/git-archive-tree.txt
+++ b/Documentation/git-archive-tree.txt
@@ -8,7 +8,7 @@ git-archive-tree - Creates a archive of the f
 
 SYNOPSIS
 --------
-'git-archive-tree' -f {tar|zip} [--prefix=<prefix>/] [-0|...|-9]
+'git-archive-tree' -f {tar|tgz|zip} [--prefix=<prefix>/] [-0|...|-9]
 	    <tree-ish> [path...]
 
 DESCRIPTION
@@ -29,7 +29,8 @@ OPTIONS
 -------
 
 -f::
-	Format of the resulting archive, can be either 'tar' or 'zip'.
+	Format of the resulting archive, can be either 'tar', 'tgz'
+	or 'zip'.
 
 <tree-ish>::
 	The tree or commit to produce an archive for.
@@ -71,11 +72,11 @@ git archive -f tar --prefix=junk/ HEAD |
 	latest commit on the current branch, and extracts it in
 	`/var/tmp/junk` directory.
 
-git archive -f tar --prefix=git-1.4.0/ v1.4.0 | gzip >git-1.4.0.tar.gz::
+git archive -f tgz --prefix=git-1.4.0/ v1.4.0 >git-1.4.0.tar.gz::
 
 	Create a compressed tarball for v1.4.0 release.
 
-git archive -f tar --prefix=git-1.4.0/ v1.4.0{caret}\{tree\} | gzip >git-1.4.0.tar.gz::
+git archive -f tgz --prefix=git-1.4.0/ v1.4.0{caret}\{tree\} >git-1.4.0.tar.gz::
 
 	Create a compressed tarball for v1.4.0 release, but without a
 	global extended pax header.
diff --git a/archive.h b/archive.h
index 7813962..6a03864 100644
--- a/archive.h
+++ b/archive.h
@@ -3,4 +3,6 @@ #include "tree.h"
 typedef int (*write_archive_fn_t)(struct tree *tree, const unsigned char *commit_sha1, const char *prefix, time_t time, const char **pathspec);
 
 int write_tar_archive(struct tree *tree, const unsigned char *commit_sha1, const char *prefix, time_t time, const char **pathspec);
+int write_tgz_archive(struct tree *tree, const unsigned char *commit_sha1,
+                      const char *prefix, time_t time, const char **pathspec);
 int write_zip_archive(struct tree *tree, const unsigned char *commit_sha1, const char *prefix, time_t time, const char **pathspec);
diff --git a/builtin-archive-tree.c b/builtin-archive-tree.c
index 2c6ee60..f61e26d 100644
--- a/builtin-archive-tree.c
+++ b/builtin-archive-tree.c
@@ -9,12 +9,14 @@ #include "tree-walk.h"
 #include "archive.h"
 
 static const char archive_usage[] =
-"git-archive-tree -f {tar|zip} [--prefix=<prefix>/] [-0|...|-9] <tree-ish> [path...]";
+"git-archive-tree -f {tar|tgz|zip} [--prefix=<prefix>/] [-0|...|-9] <tree-ish> [path...]";
 
 static write_archive_fn_t parse_archive_format(const char *format)
 {
 	if (!strcmp(format, "tar"))
 		return write_tar_archive;
+	if (!strcmp(format, "tgz"))
+		return write_tgz_archive;
 	if (!strcmp(format, "zip"))
 		return write_zip_archive;
 	return NULL;
diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c
index e0da01e..743d53a 100644
--- a/builtin-tar-tree.c
+++ b/builtin-tar-tree.c
@@ -23,6 +23,10 @@ static unsigned long offset;
 static time_t archive_time;
 static int tar_umask;
 
+static gzFile gzstdout;
+
+typedef void (*blocked_write_fn)(const void *data, unsigned long size);
+
 /* writes out the whole block, but only if it is full */
 static void write_if_needed(void)
 {
@@ -36,7 +40,7 @@ static void write_if_needed(void)
  * queues up writes, so that all our write(2) calls write exactly one
  * full block; pads writes to RECORDSIZE
  */
-static void write_blocked(const void *data, unsigned long size)
+static void do_write_blocked(const void *data, unsigned long size)
 {
 	const char *buf = data;
 	unsigned long tail;
@@ -68,19 +72,20 @@ static void write_blocked(const void *da
 	write_if_needed();
 }
 
+static blocked_write_fn write_blocked = do_write_blocked;
+
 /*
  * The end of tar archives is marked by 2*512 nul bytes and after that
  * follows the rest of the block (if any).
  */
 static void write_trailer(void)
 {
-	int tail = BLOCKSIZE - offset;
-	memset(block + offset, 0, tail);
-	write_or_die(1, block, BLOCKSIZE);
-	if (tail < 2 * RECORDSIZE) {
-		memset(block, 0, offset);
-		write_or_die(1, block, BLOCKSIZE);
-	}
+	char zeroes[RECORDSIZE];
+	memset(zeroes, 0, RECORDSIZE);
+	write_blocked(zeroes, RECORDSIZE);
+	write_blocked(zeroes, RECORDSIZE);
+	while (offset)
+		write_blocked(zeroes, RECORDSIZE);
 }
 
 static void strbuf_append_string(struct strbuf *sb, const char *s)
@@ -404,6 +409,48 @@ int write_tar_archive(struct tree *tree,
 	return 0;
 }
 
+static void write_blocked_gzip(const void *data, unsigned long size)
+{
+	const char *p = data;
+	int written;
+	unsigned long tail = size % RECORDSIZE;
+
+	while (size > 0) {
+		written = gzwrite(gzstdout, p, size);
+		if (written == 0) {
+			if (errno == EPIPE)
+				exit(0);
+			die("gzwrite error (%s)", strerror(errno));
+		}
+		size -= written;
+		p += written;
+	}
+
+	if (tail) {
+		z_off_t result = gzseek(gzstdout, RECORDSIZE - tail, SEEK_CUR);
+		if (result == -1)
+			die("gzseek error (%s)", strerror(errno));
+	}
+}
+
+int write_tgz_archive(struct tree *tree, const unsigned char *commit_sha1,
+                      const char *prefix, time_t time, const char **pathspec)
+{
+	int result;
+
+	gzstdout = gzdopen(1, "wb");
+	if (!gzstdout)
+		die("zlib is unable to open stdout");
+
+	write_blocked = write_blocked_gzip;
+	result = write_tar_archive(tree, commit_sha1, prefix, time, pathspec); 
+
+	if (gzclose(gzstdout) != Z_OK)
+		result = 1;
+
+	return result;
+}
+
 static const char *exec = "git-upload-tar";
 
 static int remote_tar(int argc, const char **argv)
-- 
VGER BF report: U 0.497484
Rene Scharfe· Sep 2, 2006, 13:10 UTC · re: Rene Scharfe · lore

Re: [PATCH][RFC] Add git-archive-tree

The two patches I sent are what I have been able to come up with so far. The next step would be to add archive-neutral upload and download support.

Having thought a bit about it I propose to keep git-archive-tree for local operations, only. It can be called by the uploader just like git-tar-tree is now called by git-upload-tar. As Franck suggested, the uploader should allow the list of archive formats it supports to be restricted in a config file. The range of allowed compression levels should also be configurable.

Does it make sense to change the wire protocol to simply send the command line options one by one?

The interface could be something like this:
   git-download-archive <repo> <git-archive-tree options...>
   git-upload-archive <directory>
Or, if the big number of git command names is a concern:
   git-archive-remote --upload|--download ...
What do you all think?

I won't have time for any of this over the next few days, so Franck, go wild. ;-)

René
-- 
VGER BF report: U 0.499999
Franck Bui-Huu· Sep 2, 2006, 20:13 UTC · re: Rene Scharfe · lore

Re: [PATCH][RFC] Add git-archive-tree

2006/9/2, Rene Scharfe <rene.scharfe@lsrfire.ath.cx>:
Show 5 quoted lines
> The two patches I sent are what I have been able to come up with so far.
> The next step would be to add archive-neutral upload and download support.
>
> Having thought a bit about it I propose to keep git-archive-tree for
> local operations, only.  It can be called by the uploader just like

Well I don't see why putting the remote operations in an other file. I was more thinking on something like this:

git-archive --format=<fmt> [--remote=<repo>] <tree-ish> [path...]

This main porcelain function would call directly functions provided by archivers lib. We will need to define an API which git-archive will use for local operations.

Symetrically, on the sever side we would have:
git-upload-archive --format=<fmt> <repo> [path...]
used by git-daemon. It will deal with protocol, paths and use archiver's lib.
Eventually, we would have 2 commands:

git-archive git-upload-archive

and get ride of

git-tar-tree git-zip-tree git-upload-tar git-upload-zip

Show 7 quoted lines
> git-tar-tree is now called by git-upload-tar.  As Franck suggested, the
> uploader should allow the list of archive formats it supports to be
> restricted in a config file.  The range of allowed compression levels
> should also be configurable.
>
> Does it make sense to change the wire protocol to simply send the
> command line options one by one?

That would make sense if the number of options grow up. Currently the remote protocol had been written by Junio, I just pick up that part from git-tar-tree and put it into git-archive. But if we allow pathspec for remote operations, then we need to send them to the uploader.

Show 8 quoted lines
>
> The interface could be something like this:
>
>    git-download-archive <repo> <git-archive-tree options...>
>    git-upload-archive <directory>
>
> Or, if the big number of git command names is a concern:
>

I think it is, IMHO. And that's why I think we could have only one command for building localy/remotely archive whatever the format. git-archive should be a main procelain command, and we should get rid of git-{tar,zip}-tree commands.

-- 
               Franck

-- 
VGER BF report: U 0.742177
Rene Scharfe· Sep 4, 2006, 18:22 UTC · re: Franck Bui-Huu · lore

Re: [PATCH][RFC] Add git-archive-tree

Franck Bui-Huu schrieb:
Show 12 quoted lines
> 2006/9/2, Rene Scharfe <rene.scharfe@lsrfire.ath.cx>:
>> The two patches I sent are what I have been able to come up with so far.
>> The next step would be to add archive-neutral upload and download
>> support.
>>
>> Having thought a bit about it I propose to keep git-archive-tree for
>> local operations, only.  It can be called by the uploader just like
> 
> Well I don't see why putting the remote operations in an other file. I
> was more thinking on something like this:
> 
> git-archive --format=<fmt> [--remote=<repo>] <tree-ish> [path...]

My intention was to put both halves of the wire protocol implementation into the same source file. I still think this is a good idea, but of course it's independent from any command line interface considerations.

Internally, remote and local operations do completely different things, so it doesn't make sense to mix them. But users probably don't care about such details and may prefer a single command to express "gimme that archive, no matter if it's made here or there". OK, sort of.

> This main porcelain function would call directly functions provided by
> archivers lib. We will need to define an API which git-archive will
> use for local operations.
Yes, this would be write_archive_fn_t in archive.h.
> Symetrically, on the sever side we would have:
> 
> git-upload-archive --format=<fmt> <repo> [path...]

No, git-upload-archive would accept only a single parameter: the path to the repository. It'd receive all other options via the wire protocol, just like git-upload-tar.

> used by git-daemon. It will deal with protocol, paths and use archiver's
> lib.

Except if you meant by this that git-daemon would handle the protocol etc., which I'd disagree with ("It" is slightly ambiguous here).

Show 11 quoted lines
> Eventually, we would have 2 commands:
> 
> git-archive
> git-upload-archive
> 
> and get ride of
> 
> git-tar-tree
> git-zip-tree
> git-upload-tar
> git-upload-zip

Let's keep the git-<verb>-<object> nomenclature used for most commands for the sake of consistency. In both cases you'd only type git-archiv<TAB> anyway. ;-)

We could remove the existing commands git-upload-tar and git-zip-tree right now, as they were never part of a release, yet. git-upload-zip doesn't exist, yet, and git-tar-tree would probably survive as legacy interface, calling git-archive-tree internals. Junio, am I correct regarding the cool-blooded killing of unreleased commands?

Show 13 quoted lines
>> git-tar-tree is now called by git-upload-tar.  As Franck suggested, the
>> uploader should allow the list of archive formats it supports to be
>> restricted in a config file.  The range of allowed compression levels
>> should also be configurable.
>>
>> Does it make sense to change the wire protocol to simply send the
>> command line options one by one?
> 
> That would make sense if the number of options grow up. Currently the
> remote protocol had been written by Junio, I just pick up that part
> from git-tar-tree and put it into git-archive. But if we allow
> pathspec for remote operations, then  we need to send them to the
> uploader.

If we simply send each option to the downloader without understanding them then the uploader can be kept really simple. The protocol can be kept simple, and it would be future-proof: we will never have to update it. I'm a bit worried about the security implications of such a setup, strangely _because_ I can't see a way to exploit it right now (and because it allows arbitrary input, which we must be able to cope with anyway).

Show 13 quoted lines
>>
>> The interface could be something like this:
>>
>>    git-download-archive <repo> <git-archive-tree options...>
>>    git-upload-archive <directory>
>>
>> Or, if the big number of git command names is a concern:
>>
> 
> I think it is, IMHO. And that's why I think we could have only one
> command for building localy/remotely archive whatever the format.
> git-archive should be a main procelain command, and we should get rid
> of git-{tar,zip}-tree commands.

OK, makes sense; users will never need to call git-upload-archive directly, and having a single command for all archiving is a good thing.

My next steps will be to make traverse_tree() support path specs, in order to achieve feature parity with read_tree_recursive(). I hope that the former keeps being significantly faster than the latter even after that. If it works out I'm going to convert git-zip-tree to use traverse_tree().

Other tasks: morphing git-upload-tar into git-upload-archive, replacing/enhancing the protocol, and adding a matching uploader (--remote= option handler). I won't start with that stuff until I get the performance sorted out. I hope this keeps me from stepping on your toes (again), Franck. ;-)

Thanks, René

Junio C Hamano· Sep 4, 2006, 20:09 UTC · re: Rene Scharfe · lore

Re: [PATCH][RFC] Add git-archive-tree

Rene Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
Show 10 quoted lines
> Franck Bui-Huu schrieb:
>>
>> Well I don't see why putting the remote operations in an other file. I
>> was more thinking on something like this:
>> 
>> git-archive --format=<fmt> [--remote=<repo>] <tree-ish> [path...]
>
> My intention was to put both halves of the wire protocol implementation
> into the same source file.  I still think this is a good idea, but of
> course it's independent from any command line interface considerations.
I think so too, and agree with your reasoning. 
> No, git-upload-archive would accept only a single parameter: the path to
> the repository.  It'd receive all other options via the wire protocol,
> just like git-upload-tar.
Same here.
Show 6 quoted lines
> We could remove the existing commands git-upload-tar and git-zip-tree
> right now, as they were never part of a release, yet.  git-upload-zip
> doesn't exist, yet, and git-tar-tree would probably survive as legacy
> interface, calling git-archive-tree internals.  Junio, am I correct
> regarding the cool-blooded killing of unreleased commands?
>>> git-tar-tree is now called by git-upload-tar.

I do think leaving tar-tree vs upload-tar protocol alone would not hurt development of this new archive vs upload-archive protocol. Also 'git-tar-tree --remote' has been part of git for some time now (1.4.2 has it). Having said that, I suspect nobody really relies on that, and I think this restructuring is a good change. Time to poll the userbase?

	Do people get upset if we said "git-tar-tree
	--remote=xyzzy.example.com" is no more, and you will
	have to say "git-fetch-archive -f tar xyzzy.example.com"?
>>> Does it make sense to change the wire protocol to simply send the
>>> command line options one by one?
>> 
>> That would make sense if the number of options grow up.

... or different archive backends want different kind of things (tar-tree without compression would not want -z6 but zip-tree would).

>> But if we allow
>> pathspec for remote operations, then  we need to send them to the
>> uploader.
Yup.
Show 7 quoted lines
> If we simply send each option to the downloader without understanding
> them then the uploader can be kept really simple.  The protocol can be
> kept simple, and it would be future-proof: we will never have to update
> it.  I'm a bit worried about the security implications of such a setup,
> strangely _because_ I can't see a way to exploit it right now (and
> because it allows arbitrary input, which we must be able to cope with
> anyway).
Show 13 quoted lines
>>>
>>> The interface could be something like this:
>>>
>>>    git-download-archive <repo> <git-archive-tree options...>
>>>    git-upload-archive <directory>
>>>
>> I think it is, IMHO. And that's why I think we could have only one
>> command for building localy/remotely archive whatever the format.
>> git-archive should be a main procelain command, and we should get rid
>> of git-{tar,zip}-tree commands.
>
> OK, makes sense; users will never need to call git-upload-archive
> directly, and having a single command for all archiving is a good thing.
So to recap:
 - "git-archive-tree --format <foo> <foo specific options> <tree>"
   would know how to create <foo> format archive and send the result
   to stdout.
 - "git-download-archive <repo> <git-archive-tree-command-line>"
   would talk with "git-upload-archive" in the remote repository,
   give archive-tree command line to it and receives the result.
 - "git-upload-archive <repo>" is not used by the end user.
   Underlying git-archive-tree command line options are sent
   over the protocol from download-archive, just like upload-tar
   does.

If this is what you mean, I think three of us are in agreement here.

> My next steps will be to make traverse_tree() support path specs, in
> order to achieve feature parity with read_tree_recursive().  I hope that
> the former keeps being significantly faster than the latter even after
> that.

A thing that have been bothering me for some time in pathspec area is that we have two (eh perhaps three) quite different pathspec semantics.

 - diff-tree family (and anything based on revision.c including
   git-log) is a strict prefix directory match (e.g. no
   wildcards, and "Documentation/howto" matches the directory
   but not "Documentation/howto-index.sh")
 - ls-files family (ls-files used to be an one odd man out, but
   git-grep mimics it, and "git-commit <paths>" uses ls-files
   internally so they form a family) is prefix match with
   fnmatch match upon wildcard (e.g. "Documentation/howto" still
   matches the directory but not "Documentation/howto-index.sh",
   but you can say "Documentation/howto*" to match both, and you
   can even say "Document*").

The former semantics is very friendly with "struct tree_desc" based traversal. Allowing pathspecs with ls-files style matching would probably more useful but more work. And there are at least four implementations of pathspec matcher with slightly different interfaces (ugh, sorry) if I am not mistaken: builtin-ls-files.c has one (match), tree-diff.c has another (interesting), builtin-grep.c has one (pathspec_matches), dir.c has another (match_pathspec).

We might be able to share code with para-walk.c in "pu" (it is designed to walk zero or more trees optionally with index and working tree in parallel and we would be using it to walk only single tree without index nor working tree). That would give me an incentive to clean up the code ;-) Currently it is a ten "WIP" patch series.

One thing it attempts to do on the side is to consolidate the diff-tree style ones into one by introducing another in read-cache.c (pathname_included) but it does not convert existing users of other matchers to use it yet.

Rene Scharfe· Sep 4, 2006, 22:02 UTC · re: Junio C Hamano · lore

Re: [PATCH][RFC] Add git-archive-tree

Junio C Hamano schrieb:
Show 15 quoted lines
> So to recap:
> 
> - "git-archive-tree --format <foo> <foo specific options> <tree>" 
> would know how to create <foo> format archive and send the result to
> stdout.
> 
> - "git-download-archive <repo> <git-archive-tree-command-line>" would
> talk with "git-upload-archive" in the remote repository, give
> archive-tree command line to it and receives the result.
> 
> - "git-upload-archive <repo>" is not used by the end user. Underlying
> git-archive-tree command line options are sent over the protocol from
> download-archive, just like upload-tar does.
> 
> If this is what you mean, I think three of us are in agreement here.

Well, this is just _one_ of the positions I've taken on this topic, I have to admit. Franck then convinced me that merging downloader and archiver into one command is nice for users (less commands to remember, keeps existing --remote option) even if it doesn't make sense technically, because their implementations have nothing in common.

This is a bikeshed colour discussion, and I'm bad at it. I'll shut up on this and simply follow the directions above.

Show 40 quoted lines
>> My next steps will be to make traverse_tree() support path specs,
>> in order to achieve feature parity with read_tree_recursive().  I
>> hope that the former keeps being significantly faster than the
>> latter even after that.
> 
> A thing that have been bothering me for some time in pathspec area is
> that we have two (eh perhaps three) quite different pathspec
> semantics.
> 
> - diff-tree family (and anything based on revision.c including 
> git-log) is a strict prefix directory match (e.g. no wildcards, and
> "Documentation/howto" matches the directory but not
> "Documentation/howto-index.sh")
> 
> - ls-files family (ls-files used to be an one odd man out, but 
> git-grep mimics it, and "git-commit <paths>" uses ls-files internally
> so they form a family) is prefix match with fnmatch match upon
> wildcard (e.g. "Documentation/howto" still matches the directory but
> not "Documentation/howto-index.sh", but you can say
> "Documentation/howto*" to match both, and you can even say
> "Document*").
> 
> The former semantics is very friendly with "struct tree_desc" based
> traversal.  Allowing pathspecs with ls-files style matching would
> probably more useful but more work.  And there are at least four
> implementations of pathspec matcher with slightly different
> interfaces (ugh, sorry) if I am not mistaken: builtin-ls-files.c has
> one (match), tree-diff.c has another (interesting), builtin-grep.c
> has one (pathspec_matches), dir.c has another (match_pathspec).
> 
> We might be able to share code with para-walk.c in "pu" (it is 
> designed to walk zero or more trees optionally with index and working
> tree in parallel and we would be using it to walk only single tree
> without index nor working tree).  That would give me an incentive to
> clean up the code ;-) Currently it is a ten "WIP" patch series.
> 
> One thing it attempts to do on the side is to consolidate the 
> diff-tree style ones into one by introducing another in read-cache.c
> (pathname_included) but it does not convert existing users of other
> matchers to use it yet.

Interesting. OK, I'll check out the existing implementations with an eye on consolidation and also take a look around that scary place named 'pu'. ;-)

So far I have failed in creating a traverse_tree() function with path spec match support which also is faster than read_tree_recursive(). Maybe the speed difference between 'git-tar-tree' and 'git-archive-tree -ftar' is caused by something else. I keep on trying.

René
Junio C Hamano· Sep 4, 2006, 22:20 UTC · re: Rene Scharfe · lore

Re: [PATCH][RFC] Add git-archive-tree

Rene Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
Show 7 quoted lines
> Junio C Hamano schrieb:
>...
> Well, this is just _one_ of the positions I've taken on this topic, I
> have to admit.  Franck then convinced me that merging downloader and
> archiver into one command is nice for users (less commands to remember,
> keeps existing --remote option) even if it doesn't make sense
> technically, because their implementations have nothing in common.

Ah, I was not following the thread closely and I agree with you and Franck now in that smaller number of commands the better. (IOW, download-archive in my 3-item list is not needed in the end user UI and can be implemented as "git-archive-tree --remote=<repo>).

Thanks for clarification.
Franck Bui-Huu· Sep 5, 2006, 11:43 UTC · re: Junio C Hamano · lore

Re: [PATCH][RFC] Add git-archive-tree

2006/9/5, Junio C Hamano <junkio@cox.net>:
Show 16 quoted lines
> Rene Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>
> > Junio C Hamano schrieb:
> >...
> > Well, this is just _one_ of the positions I've taken on this topic, I
> > have to admit.  Franck then convinced me that merging downloader and
> > archiver into one command is nice for users (less commands to remember,
> > keeps existing --remote option) even if it doesn't make sense
> > technically, because their implementations have nothing in common.
>
> Ah, I was not following the thread closely and I agree with you
> and Franck now in that smaller number of commands the better.
> (IOW, download-archive in my 3-item list is not needed in the
> end user UI and can be implemented as "git-archive-tree
> --remote=<repo>).
>

OK, I'm sending what I've been able to do so far. It has been done before you sent some feedbacks one day ago, so please forgive me if, for the moment, I called "git-archive" instead of "git-archive-tree" or if I missed some of your points; I'll change that.

I'm sending 2 patches, which are sent to get some comments. They're not for inclusion in anyways.

The pathspec area has been pick up from Rene's initial patch. I have not enough git's internal knowledge to improve it. But I'll be glad to look at this later (if you have any clues for starting point BTW).

-- 
               Franck
Junio C Hamano· Sep 2, 2006, 21:19 UTC · re: Rene Scharfe · lore

Re: [PATCH][RFC] Add git-archive-tree

Rene Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> Does it make sense to change the wire protocol to simply send the
> command line options one by one?

Which wire protocol are you talking about? The one between upload-tar and local "tar-tree --remote"? I think that one was a tar-tree specific hack and we do not want to mimic it.

Your idea of making archiver neutral upload/download pair makes sense. The daemon can invoke upload-archive with a single parameter (the repository, "."), just like upload_tar() in "pu:daemon.c" does [*1*], and upload-archive talks with the other end to know which archiver to run with what paramter. We would probably want to use some sort of side-band mechanism so that we can do progress-bar as well.

*1* Unrelated side note.  I find myself typing "git less" quite
    often ;-)
	git less pu:daemon.c
    and get "no such command, dummy" response.  
    Yes, I know I could alias it to "-p cat-file -p".  I am just
    too lazy to do so.
-- 
VGER BF report: U 0.709954
Rene Scharfe· Sep 2, 2006, 14:17 UTC · re: Rene Scharfe · lore

Re: [PATCH][RFC] Add git-archive-tree

Oops, forgot to sign off. Please consider both patches (git-archive-tree and tgz support) as

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
-- 
VGER BF report: U 0.500007
Franck Bui-Huu· Sep 2, 2006, 15:24 UTC · re: Rene Scharfe · lore

Re: [PATCH][RFC] Add git-archive-tree

Hi,
2006/9/2, Rene Scharfe <rene.scharfe@lsrfire.ath.cx>:
Show 5 quoted lines
> git-archive-tree is a command to make tar and ZIP archives of a git tree.
> It helps prevent a proliferation of git-{format}-tree commands.  This is
> useful e.g. for remote archive fetching because we only need to write a
> single upload and a single download program that simply pass on the
> format option to git-archive-tree.

OK, Rene you beat me. I started to cook up something this morning but had no time to go further.

I sent a starting implementation just because it seems complementary to the one you sent a couple hours ago: it supports '--remote' option. But it does _not_ have path spec support you introduced. I think it's cool feature but I would have to dig into git's internal to implement it, which would have taken me a while.

>From now, do you think we should import my work into your version or
'vice verca' ?
-- >8 --
Subject: [PATCH] Add git-archive (other implementation)
Signed-off-by: Franck Bui-Huu <vagabon.xyz@gmail.com>
---
 .gitignore          |    1
 Makefile            |    1
 builtin-archive.c   |  171 +++++++++++++++++++++++++++++++++++++++++++++++++++
 builtin.h           |    1
 generate-cmdlist.sh |    1
 git.c               |    1
 6 files changed, 176 insertions(+), 0 deletions(-)
Show changes to 6 files +176 −0

.gitignore, Makefile, builtin-archive.c, builtin.h, generate-cmdlist.sh, git.c

diff --git a/.gitignore b/.gitignore
index 78cb671..a3e7ca1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -8,6 +8,7 @@ git-apply
 git-applymbox
 git-applypatch
 git-archimport
+git-archive
 git-bisect
 git-branch
 git-cat-file
diff --git a/Makefile b/Makefile
index 164dbcf..8d963e0 100644
--- a/Makefile
+++ b/Makefile
@@ -266,6 +266,7 @@ LIB_OBJS = \
 BUILTIN_OBJS = \
 	builtin-add.o \
 	builtin-apply.o \
+	builtin-archive.o \
 	builtin-cat-file.o \
 	builtin-checkout-index.o \
 	builtin-check-ref-format.o \
diff --git a/builtin-archive.c b/builtin-archive.c
new file mode 100644
index 0000000..47898ee
--- /dev/null
+++ b/builtin-archive.c
@@ -0,0 +1,171 @@
+/*
+ * Copyright (c) 2006 Franck Bui-Huu
+ */
+#include "cache.h"
+#include "builtin.h"
+#include "exec_cmd.h"
+#include "pkt-line.h"
+
+#define MAX_ARGS	32
+
+static const char *extra_argv[MAX_ARGS];
+static int extra_argc = 1;
+static const char *base;
+static const char *remote;
+
+static const char archive_usage[] = \
+"git-archive -f <format> [extra] [--remote=<repo>] [--prefix=<base>]
<tree-ish>";
+
+/* archiver's options */
+#define NEED_BASE	(1<<0)
+
+struct archiver_info {
+	const char *name;
+	int (*fn)(int argc, const char **argv, const char *prefix);
+	int options;
+};
+
+static struct archiver_info archivers[] = {
+	{ "tar", cmd_tar_tree, NEED_BASE },
+	{ "zip", cmd_zip_tree, NEED_BASE },
+};
+
+static int run_remote_archiver(struct archiver_info *ar, char *url)
+{
+	int fd[2], len, rv;
+	char buf[1024];
+	pid_t pid;
+
+	/*
+	 * For now, remote operations does not support extra options
+	 */
+	if (extra_argc > 2)
+		die("Remote operation does not support extra options yet.");
+
+	sprintf(buf, "git-upload-%s", ar->name);
+	pid = git_connect(fd, url, buf);
+	if (pid < 0)
+		return pid;
+
+	packet_write(fd[1], "want %s\n", extra_argv[1]);
+	if (base)
+		packet_write(fd[1], "base %s\n", base);
+	packet_flush(fd[1]);
+
+	len = packet_read_line(fd[0], buf, sizeof(buf));
+	if (!len)
+		die("git-archive: expected ACK/NAK, got EOF");
+	if (buf[len-1] == '\n')
+		buf[--len] = 0;
+	if (strcmp(buf, "ACK")) {
+		if (len > 5 && !strncmp(buf, "NACK ", 5))
+			die("git-archive: NACK %s", buf + 5);
+		die("git-archive: protocol error");
+	}
+	if (packet_read_line(fd[0], buf, sizeof(buf)))
+		die("git-archive: expected a flush");
+
+	/* Now, start reading from fd[0] and spit it out to stdout */
+	rv = copy_fd(fd[0], 1);
+	close(fd[0]);
+
+	return !!(rv | finish_connect(pid));
+}
+
+void insert_extra_option(const char *option, int idx)
+{
+	const char **argv = extra_argv;	/* some shortcuts... */
+	int argc = extra_argc;
+
+	if (argc > MAX_ARGS - 2)
+		die("Too many args");
+	if (idx > MAX_ARGS - 1)
+		die("idx overflow extra_argv buffer !");
+
+	/* insert at last position ? */
+	if (idx < 0) {
+		argv[argc++] = option;
+		argv[argc] = NULL;
+	} else {
+		argv += idx;
+		memmove(argv + 1, argv, argc * sizeof(*argv));
+		*argv = option;
+	}
+	extra_argc++;
+}
+
+int run_archiver(const char *format)
+{
+	struct archiver_info *ar;
+	char buf[32];
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(archivers); i++) {
+		ar = &archivers[i];
+		if (strcmp(format, ar->name))
+			continue;
+		goto found;
+	}
+	return error("Unknown archive format '%s'\n", format);
+found:
+	if (remote) {
+		char *url = strdup(remote+9);	/* --remote=<repo> */
+		return run_remote_archiver(ar, url);
+	}
+	if (base) {
+		i = 1;
+		if (ar->options & NEED_BASE) {
+			base += 9; /* --prefix=<base> */
+			i = -1;
+		}
+		insert_extra_option(base, i);
+	}
+	sprintf(buf, "%s-tree", ar->name);
+	extra_argv[0] = buf;
+
+	return ar->fn(extra_argc, extra_argv, setup_git_directory());
+}
+
+static int show_list_formats(void)
+{
+	int i;
+	
+	for (i = 0; i < ARRAY_SIZE(archivers); i++) {
+		printf("%s\n", archivers[i].name);
+	}
+	return 0;
+}
+
+int cmd_archive(int argc, const char **argv, const char *prefix)
+{
+	static const char *format;
+	int i;
+
+	for (i = 1; i < argc; i++) {
+		const char *arg = argv[i];
+
+		if (!strcmp(arg, "--list") || !strcmp(arg, "-l")) {
+			return show_list_formats();
+		}
+		if (!strncmp(arg, "--format=", 9)) {
+			format = arg+9;
+			continue;
+		}
+		if (!strncmp(arg, "--prefix=", 9)) {
+			base = arg;
+			continue;
+		}
+		if (!strncmp(arg, "--remote=", 9)) {
+			remote = arg;
+			continue;
+		}
+		if (extra_argc > MAX_ARGS - 2)
+			die("Too many options");
+		extra_argv[extra_argc++] = arg;
+	}
+
+	if (!format)
+		die("You must specify an archive format");
+
+	return run_archiver(format);
+}
diff --git a/builtin.h b/builtin.h
index 25431d7..50852cd 100644
--- a/builtin.h
+++ b/builtin.h
@@ -15,6 +15,7 @@ extern int write_tree(unsigned char *sha

 extern int cmd_add(int argc, const char **argv, const char *prefix);
 extern int cmd_apply(int argc, const char **argv, const char *prefix);
+extern int cmd_archive(int argc, const char **argv, const char *prefix);
 extern int cmd_cat_file(int argc, const char **argv, const char *prefix);
 extern int cmd_checkout_index(int argc, const char **argv, const char *prefix);
 extern int cmd_check_ref_format(int argc, const char **argv, const
char *prefix);
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
index ec1eda2..5450918 100755
--- a/generate-cmdlist.sh
+++ b/generate-cmdlist.sh
@@ -12,6 +12,7 @@ struct cmdname_help common_cmds[] = {"
 sort <<\EOF |
 add
 apply
+archive
 bisect
 branch
 checkout
diff --git a/git.c b/git.c
index 403fb3a..ddb1620 100644
--- a/git.c
+++ b/git.c
@@ -218,6 +218,7 @@ static void handle_internal_command(int
 	} commands[] = {
 		{ "add", cmd_add, RUN_SETUP },
 		{ "apply", cmd_apply },
+		{ "archive", cmd_archive },
 		{ "cat-file", cmd_cat_file, RUN_SETUP },
 		{ "checkout-index", cmd_checkout_index, RUN_SETUP },
 		{ "check-ref-format", cmd_check_ref_format },
-- 
1.4.2.g8ed9-dirty

-- 
VGER BF report: U 0.5
Rene Scharfe· Sep 2, 2006, 16:08 UTC · re: Franck Bui-Huu · lore

Re: [PATCH][RFC] Add git-archive-tree

Franck Bui-Huu schrieb:
Show 11 quoted lines
> Hi,
> 
> 2006/9/2, Rene Scharfe <rene.scharfe@lsrfire.ath.cx>:
>> git-archive-tree is a command to make tar and ZIP archives of a git tree.
>> It helps prevent a proliferation of git-{format}-tree commands.  This is
>> useful e.g. for remote archive fetching because we only need to write a
>> single upload and a single download program that simply pass on the
>> format option to git-archive-tree.
> 
> OK, Rene you beat me. I started to cook up something this morning but
> had no time to go further.

I am sorry. My excuse is I was so angry the other night (for unrelated reasons) that I started coding and I didn't stop until git-archive-tree was working at three in the morning.

Show 5 quoted lines
> I sent a starting implementation just because it seems complementary
> to the one you sent a couple hours ago: it supports '--remote' option.
> But it does _not_ have path spec support you introduced. I think it's
> cool feature but I would have to dig into git's internal to implement
> it, which would have taken me a while.
I cheated by using the already existing function read_tree_recursive().
> From now, do you think we should import my work into your version or
> 'vice verca' ?

Of course I like mine better. :-) But first we need to agree on a direction. I think it makes sense to keep the network stuff out of the archiver, and to keep the downloader as simple ("dumb") as possible: it should pass on any options unaltered, including the format option.

That means the wire format changes completely, but since this feature hasn't been officially released, yet, this shouldn't be a problem, right?

So, I propose to not add anything (except new formats) to git-archive-tree and to add a separate downloader and uploader (maybe combined in one program) instead. That way the remote archive operations are format agnostic. OK, almost, because the uploader needs to determine the format and compression level in order to allow or disallow uploads based on that.

What do you think?

Thanks, René

-- 
VGER BF report: U 0.539485
Junio C Hamano· Sep 2, 2006, 21:27 UTC · re: Rene Scharfe · lore

Re: [PATCH][RFC] Add git-archive-tree

Rene Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> Currently git-archive-tree -f tar is slower than git-tar-tree.  This is
> because it is welded to the side of the existing code to minimize patch
> size, and I also suspect read_tree_recursive() to be quite a bit slower
> than builtin-tar-tree.c::traverse_tree().

Yes, I suspect "struct object" and friends are very inefficient to use for things like this. "struct tree_desc" based traverser is much preferred.

-- 
VGER BF report: U 0.772588
Rene Scharfe· Sep 6, 2006, 18:05 UTC · re: Junio C Hamano · lore

Re: [PATCH][RFC] Add git-archive-tree

Junio C Hamano schrieb:
Show 10 quoted lines
> Rene Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> 
>> Currently git-archive-tree -f tar is slower than git-tar-tree.  This is
>> because it is welded to the side of the existing code to minimize patch
>> size, and I also suspect read_tree_recursive() to be quite a bit slower
>> than builtin-tar-tree.c::traverse_tree().
> 
> Yes, I suspect "struct object" and friends are very inefficient
> to use for things like this.  "struct tree_desc" based traverser
> is much preferred.

Turns out the reason for git-archive-tree -f tar being 10% slower than git-tar-tree was a stupid memory leak in write_tar_entry(). *blush* It caused lots of brk() calls (i.e. system calls).

In order to simplify measurement, I commented out the body of write_entry(), which both git-archive-tree and git-tar-tree are calling to write their output. The rest left is basically two pure tree traversers, git-archive-tree using read_tree_recursive() and git-tar-tree using its struct tree_desc based traverse_tree().

I then let the two chew away on the kernel repository. And as kcachegrind impressively shows, all we do with our trees and objects is dwarfed by inflate(). In both cases more than 96.6% of the cost lies within libz. That's not too surprising, because archivers need to decompress _all_ objects, not only trees.

So for git-archive we can pretty much chose which traverser to use based on convenience.

As a second experiment I wrote a struct tree_desc based traverser plus a matching read_tree_recursive() compatibility wrapper (included below for reference, not for inclusion) and compared the performance of 'git-ls-tree -r -t' on the kernel repository with and without it. The result is that the relative cost of all functions from tree.c combined decreased from 0.93% to 0.66%. Ugh.

So while a struct tree_desc based traverser can be significantly faster than read_tree_recursive(), as soon as you actually start to do something to the trees that difference pales to insignificance.

René
Show changes to 2 files +101 −0

tree.c, tree.h

diff --git a/tree.c b/tree.c
index ea386e5..977a4aa 100644
--- a/tree.c
+++ b/tree.c
@@ -4,6 +4,7 @@ #include "blob.h"
 #include "commit.h"
 #include "tag.h"
 #include "tree-walk.h"
+#include "strbuf.h"
 #include <stdlib.h>
 
 const char *tree_type = "tree";
@@ -227,3 +228,99 @@ struct tree *parse_tree_indirect(const u
 			parse_object(obj->sha1);
 	} while (1);
 }
+
+static int do_read_tree_recursive_light(struct tree_desc *desc,
+                                        struct strbuf *base,
+                                        const char **match, read_tree_fn_t fn)
+{
+	struct name_entry entry;
+	int err = 0;
+	int baselen = base->len;
+
+	while (tree_entry(desc, &entry)) {
+		if (!match_tree_entry(base->buf, base->len, entry.path, entry.mode, match))
+			continue;
+
+		err = fn(entry.sha1, base->buf, base->len, entry.path, entry.mode, 0);
+		switch (err) {
+		case 0:
+			continue;
+		case READ_TREE_RECURSIVE:
+			break;
+		default:
+			return -1;
+		}
+
+		if (S_ISDIR(entry.mode)) {
+			struct tree_desc subtree;
+			char type[20];
+			void *buf;
+			int newbaselen;
+
+			buf = read_sha1_file(entry.sha1, type, &subtree.size);
+			if (!buf)
+				return error("cannot read %s",
+				             sha1_to_hex(entry.sha1));
+			if (strcmp(type, tree_type)) {
+				free(buf);
+				return error("Object %s not a tree",
+				             sha1_to_hex(entry.sha1));
+			}
+			subtree.buf = buf;
+
+			newbaselen = baselen + entry.pathlen + 1;
+			if (newbaselen > base->alloc) {
+				base->buf = xrealloc(base->buf, newbaselen);
+				base->alloc = newbaselen;
+			}
+			memcpy(base->buf + baselen, entry.path, entry.pathlen);
+			base->buf[baselen + entry.pathlen] = '/';
+			base->len = newbaselen;
+
+			err = do_read_tree_recursive_light(&subtree,
+			                                   base,
+			                                   match, fn);
+			base->len = baselen;
+			free(buf);
+			if (err)
+				break;
+		}
+	}
+
+	return err;
+}
+
+int read_tree_recursive_light(struct tree *tree,
+                              const char *base, int baselen, int stage,
+                              const char **match, read_tree_fn_t fn)
+{
+	unsigned char *sha1 = tree->object.sha1;
+	struct tree_desc desc;
+	char type[20];
+	void *buf;
+	int err;
+	struct strbuf sb;
+
+	sb.buf = xmalloc(PATH_MAX);
+	sb.alloc = PATH_MAX;
+	sb.len = 0;
+	if (baselen > sb.alloc) {
+		sb.buf = xrealloc(sb.buf, baselen);
+		sb.alloc = baselen;
+	}
+	memcpy(sb.buf, base, baselen);
+	sb.len = baselen;
+
+	desc.buf = buf = read_sha1_file(sha1, type, &desc.size);
+	if (!buf)
+		return error("Could not read %s", sha1_to_hex(sha1));
+	if (strcmp(type, tree_type)) {
+		free(buf);
+		return error("Object %s not a tree", sha1_to_hex(sha1));
+	}
+
+	err = do_read_tree_recursive_light(&desc, &sb, match, fn);
+	free(buf);
+
+	return err;
+}
diff --git a/tree.h b/tree.h
index dd25c53..2294bc2 100644
--- a/tree.h
+++ b/tree.h
@@ -30,4 +30,8 @@ extern int read_tree_recursive(struct tr
 
 extern int read_tree(struct tree *tree, int stage, const char **paths);
 
+extern int read_tree_recursive_light(struct tree *tree,
+                                     const char *base, int baselen, int stage,
+                                     const char **match, read_tree_fn_t fn);
+
 #endif /* TREE_H */
Junio C Hamano· Sep 6, 2006, 21:47 UTC · re: Rene Scharfe · lore

Re: [PATCH][RFC] Add git-archive-tree

Rene Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> I then let the two chew away on the kernel repository.  And as
> kcachegrind impressively shows, all we do with our trees and objects is
> dwarfed by inflate().

The diff output codepath has a logic that says "if the blob we are dealing with has the same object name as the corresponding blob in the index, and if the index entry is clean (i.e. it is known that the file sitting in the working tree matches the blob), then do not inflate() but use data from that file instead". It was originally done that way because we used to prepare temporary files out of blob and fed them to GNU diff, but I think it is still kept that way. I wonder if doing the same may be cheaper for archivers.

Rene Scharfe· Sep 17, 2006, 11:54 UTC · re: Junio C Hamano · lore

Re: [PATCH][RFC] Add git-archive-tree

Junio C Hamano schrieb:
Show 11 quoted lines
> Rene Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> 
>> I then let the two chew away on the kernel repository.  And as 
>> kcachegrind impressively shows, all we do with our trees and 
>> objects is dwarfed by inflate().
> 
> The diff output codepath has a logic that says "if the blob we are 
> dealing with has the same object name as the corresponding blob in 
> the index, and if the index entry is clean (i.e. it is known that the
>  file sitting in the working tree matches the blob), then do not 
> inflate() but use data from that file instead".

Nice idea. The tree traverser would need to provide the filenames relative to the current working directory in addition to the filenames as they are written to the archive. I guess your para-walk tree walker could be useful here. I sadly haven't found the time to look at it, yet, and now it even vanished from the pu branch.

A read is an order of magnitude faster than a deflate of the same data, at least that's what I guess from comparing the runtimes of git-tar-tree and tar. _However_, this doesn't account for I/O costs (in my tests the repo and all checked-out files were cache hot) and for any compression that would certainly be applied to the resulting archive. So the full runtime of archive creation wouldn't be that much shorter.

René

← back to recent threads