[PATCH 1/6] archive: add write_archive()

[PATCH 1/6] archive: add write_archive()

[Rene Scharfe]

Both archive and upload-archive have to parse command line arguments and
then call the archiver specific write function.  Move the duplicate code
to a new function, write_archive().

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 archive.c                |   18 ++++++++++++++++++
 archive.h                |    1 +
 builtin-archive.c        |   13 +------------
 builtin-upload-archive.c |   10 +---------
 4 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/archive.c b/archive.c
index b8b45ba..75eb257 100644
--- a/archive.c
+++ b/archive.c
@@ -155,3 +155,21 @@ int write_archive_entries(struct archiver_args *args,
 		err = 0;
 	return err;
 }
+
+int write_archive(int argc, const char **argv, const char *prefix,
+		int setup_prefix)
+{
+	const struct archiver *ar = NULL;
+	struct archiver_args args;
+	int tree_idx;
+
+	tree_idx = parse_archive_args(argc, argv, &ar, &args);
+	if (setup_prefix && prefix == NULL)
+		prefix = setup_git_directory();
+
+	argv += tree_idx;
+	parse_treeish_arg(argv, &args, prefix);
+	parse_pathspec_arg(argv + 1, &args);
+
+	return ar->write_archive(&args);
+}
diff --git a/archive.h b/archive.h
index 4a02371..6b5fe5a 100644
--- a/archive.h
+++ b/archive.h
@@ -41,5 +41,6 @@ extern int write_tar_archive(struct archiver_args *);
 extern int write_zip_archive(struct archiver_args *);
 
 extern int write_archive_entries(struct archiver_args *args, write_archive_entry_fn_t write_entry);
+extern int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix);
 
 #endif	/* ARCHIVE_H */
diff --git a/builtin-archive.c b/builtin-archive.c
index df97724..502b339 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -232,9 +232,6 @@ static const char *extract_remote_arg(int *ac, const char **av)
 
 int cmd_archive(int argc, const char **argv, const char *prefix)
 {
-	const struct archiver *ar = NULL;
-	struct archiver_args args;
-	int tree_idx;
 	const char *remote = NULL;
 
 	remote = extract_remote_arg(&argc, argv);
@@ -243,13 +240,5 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
 
 	setvbuf(stderr, NULL, _IOLBF, BUFSIZ);
 
-	tree_idx = parse_archive_args(argc, argv, &ar, &args);
-	if (prefix == NULL)
-		prefix = setup_git_directory();
-
-	argv += tree_idx;
-	parse_treeish_arg(argv, &args, prefix);
-	parse_pathspec_arg(argv + 1, &args);
-
-	return ar->write_archive(&args);
+	return write_archive(argc, argv, prefix, 1);
 }
diff --git a/builtin-upload-archive.c b/builtin-upload-archive.c
index 13a6c62..cc37b36 100644
--- a/builtin-upload-archive.c
+++ b/builtin-upload-archive.c
@@ -19,12 +19,9 @@ static const char lostchild[] =
 
 static int run_upload_archive(int argc, const char **argv, const char *prefix)
 {
-	const struct archiver *ar;
-	struct archiver_args args;
 	const char *sent_argv[MAX_ARGS];
 	const char *arg_cmd = "argument ";
 	char *p, buf[4096];
-	int treeish_idx;
 	int sent_argc;
 	int len;
 
@@ -66,12 +63,7 @@ static int run_upload_archive(int argc, const char **argv, const char *prefix)
 	sent_argv[sent_argc] = NULL;
 
 	/* parse all options sent by the client */
-	treeish_idx = parse_archive_args(sent_argc, sent_argv, &ar, &args);
-
-	parse_treeish_arg(sent_argv + treeish_idx, &args, prefix);
-	parse_pathspec_arg(sent_argv + treeish_idx + 1, &args);
-
-	return ar->write_archive(&args);
+	return write_archive(sent_argc, sent_argv, prefix, 0);
 }
 
 static void error_clnt(const char *fmt, ...)
-- 
1.6.0.rc0.42.g186458

[PATCH 2/6] archive: move parameter parsing code to archive.c

[Rene Scharfe]

write_archive() in archive.c is the only callsite for the command line
parsing functions located in builtin-archive.c.  Move them to the place
where they are used, un-export them and make them static, as hinted at
by Stephan.

Cc: Stephan Beyer <s-beyer@gmx.net>
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 archive.c         |  137 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 archive.h         |    8 ---
 builtin-archive.c |  137 -----------------------------------------------------
 3 files changed, 137 insertions(+), 145 deletions(-)

diff --git a/archive.c b/archive.c
index 75eb257..c4662a2 100644
--- a/archive.c
+++ b/archive.c
@@ -1,8 +1,19 @@
 #include "cache.h"
 #include "commit.h"
+#include "tree-walk.h"
 #include "attr.h"
 #include "archive.h"
 
+static const char archive_usage[] = \
+"git archive --format=<fmt> [--prefix=<prefix>/] [--verbose] [<extra>] <tree-ish> [path...]";
+
+#define USES_ZLIB_COMPRESSION 1
+
+const struct archiver archivers[] = {
+	{ "tar", write_tar_archive },
+	{ "zip", write_zip_archive, USES_ZLIB_COMPRESSION },
+};
+
 static void format_subst(const struct commit *commit,
                          const char *src, size_t len,
                          struct strbuf *buf)
@@ -156,6 +167,132 @@ int write_archive_entries(struct archiver_args *args,
 	return err;
 }
 
+static const struct archiver *lookup_archiver(const char *name)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(archivers); i++) {
+		if (!strcmp(name, archivers[i].name))
+			return &archivers[i];
+	}
+	return NULL;
+}
+
+static void parse_pathspec_arg(const char **pathspec,
+		struct archiver_args *ar_args)
+{
+	ar_args->pathspec = get_pathspec(ar_args->base, pathspec);
+}
+
+static void parse_treeish_arg(const char **argv,
+		struct archiver_args *ar_args, const char *prefix)
+{
+	const char *name = argv[0];
+	const unsigned char *commit_sha1;
+	time_t archive_time;
+	struct tree *tree;
+	const struct commit *commit;
+	unsigned char sha1[20];
+
+	if (get_sha1(name, sha1))
+		die("Not a valid object name");
+
+	commit = lookup_commit_reference_gently(sha1, 1);
+	if (commit) {
+		commit_sha1 = commit->object.sha1;
+		archive_time = commit->date;
+	} else {
+		commit_sha1 = NULL;
+		archive_time = time(NULL);
+	}
+
+	tree = parse_tree_indirect(sha1);
+	if (tree == NULL)
+		die("not a tree object");
+
+	if (prefix) {
+		unsigned char tree_sha1[20];
+		unsigned int mode;
+		int err;
+
+		err = get_tree_entry(tree->object.sha1, prefix,
+				     tree_sha1, &mode);
+		if (err || !S_ISDIR(mode))
+			die("current working directory is untracked");
+
+		tree = parse_tree_indirect(tree_sha1);
+	}
+	ar_args->tree = tree;
+	ar_args->commit_sha1 = commit_sha1;
+	ar_args->commit = commit;
+	ar_args->time = archive_time;
+}
+
+static int parse_archive_args(int argc, const char **argv,
+		const struct archiver **ar, struct archiver_args *args)
+{
+	const char *format = "tar";
+	const char *base = "";
+	int compression_level = -1;
+	int verbose = 0;
+	int i;
+
+	for (i = 1; i < argc; i++) {
+		const char *arg = argv[i];
+
+		if (!strcmp(arg, "--list") || !strcmp(arg, "-l")) {
+			for (i = 0; i < ARRAY_SIZE(archivers); i++)
+				printf("%s\n", archivers[i].name);
+			exit(0);
+		}
+		if (!strcmp(arg, "--verbose") || !strcmp(arg, "-v")) {
+			verbose = 1;
+			continue;
+		}
+		if (!prefixcmp(arg, "--format=")) {
+			format = arg + 9;
+			continue;
+		}
+		if (!prefixcmp(arg, "--prefix=")) {
+			base = arg + 9;
+			continue;
+		}
+		if (!strcmp(arg, "--")) {
+			i++;
+			break;
+		}
+		if (arg[0] == '-' && isdigit(arg[1]) && arg[2] == '\0') {
+			compression_level = arg[1] - '0';
+			continue;
+		}
+		if (arg[0] == '-')
+			die("Unknown argument: %s", arg);
+		break;
+	}
+
+	/* We need at least one parameter -- tree-ish */
+	if (argc - 1 < i)
+		usage(archive_usage);
+	*ar = lookup_archiver(format);
+	if (!*ar)
+		die("Unknown archive format '%s'", format);
+
+	args->compression_level = Z_DEFAULT_COMPRESSION;
+	if (compression_level != -1) {
+		if ((*ar)->flags & USES_ZLIB_COMPRESSION)
+			args->compression_level = compression_level;
+		else {
+			die("Argument not supported for format '%s': -%d",
+					format, compression_level);
+		}
+	}
+	args->verbose = verbose;
+	args->base = base;
+	args->baselen = strlen(base);
+
+	return i;
+}
+
 int write_archive(int argc, const char **argv, const char *prefix,
 		int setup_prefix)
 {
diff --git a/archive.h b/archive.h
index 6b5fe5a..f6ceaeb 100644
--- a/archive.h
+++ b/archive.h
@@ -26,14 +26,6 @@ struct archiver {
 	unsigned int flags;
 };
 
-extern int parse_archive_args(int argc, const char **argv, const struct archiver **ar, struct archiver_args *args);
-
-extern void parse_treeish_arg(const char **treeish,
-			      struct archiver_args *ar_args,
-			      const char *prefix);
-
-extern void parse_pathspec_arg(const char **pathspec,
-			       struct archiver_args *args);
 /*
  * Archive-format specific backends.
  */
diff --git a/builtin-archive.c b/builtin-archive.c
index 502b339..4dd2716 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -5,21 +5,9 @@
 #include "cache.h"
 #include "builtin.h"
 #include "archive.h"
-#include "commit.h"
-#include "tree-walk.h"
 #include "pkt-line.h"
 #include "sideband.h"
 
-static const char archive_usage[] = \
-"git archive --format=<fmt> [--prefix=<prefix>/] [--verbose] [<extra>] <tree-ish> [path...]";
-
-#define USES_ZLIB_COMPRESSION 1
-
-const struct archiver archivers[] = {
-	{ "tar", write_tar_archive },
-	{ "zip", write_zip_archive, USES_ZLIB_COMPRESSION },
-};
-
 static int run_remote_archiver(const char *remote, int argc,
 			       const char **argv)
 {
@@ -74,131 +62,6 @@ static int run_remote_archiver(const char *remote, int argc,
 	return !!rv;
 }
 
-static const struct archiver *lookup_archiver(const char *name)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(archivers); i++) {
-		if (!strcmp(name, archivers[i].name))
-			return &archivers[i];
-	}
-	return NULL;
-}
-
-void parse_pathspec_arg(const char **pathspec, struct archiver_args *ar_args)
-{
-	ar_args->pathspec = get_pathspec(ar_args->base, pathspec);
-}
-
-void parse_treeish_arg(const char **argv, struct archiver_args *ar_args,
-		       const char *prefix)
-{
-	const char *name = argv[0];
-	const unsigned char *commit_sha1;
-	time_t archive_time;
-	struct tree *tree;
-	const struct commit *commit;
-	unsigned char sha1[20];
-
-	if (get_sha1(name, sha1))
-		die("Not a valid object name");
-
-	commit = lookup_commit_reference_gently(sha1, 1);
-	if (commit) {
-		commit_sha1 = commit->object.sha1;
-		archive_time = commit->date;
-	} else {
-		commit_sha1 = NULL;
-		archive_time = time(NULL);
-	}
-
-	tree = parse_tree_indirect(sha1);
-	if (tree == NULL)
-		die("not a tree object");
-
-	if (prefix) {
-		unsigned char tree_sha1[20];
-		unsigned int mode;
-		int err;
-
-		err = get_tree_entry(tree->object.sha1, prefix,
-				     tree_sha1, &mode);
-		if (err || !S_ISDIR(mode))
-			die("current working directory is untracked");
-
-		tree = parse_tree_indirect(tree_sha1);
-	}
-	ar_args->tree = tree;
-	ar_args->commit_sha1 = commit_sha1;
-	ar_args->commit = commit;
-	ar_args->time = archive_time;
-}
-
-int parse_archive_args(int argc, const char **argv, const struct archiver **ar,
-		struct archiver_args *args)
-{
-	const char *format = "tar";
-	const char *base = "";
-	int compression_level = -1;
-	int verbose = 0;
-	int i;
-
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-
-		if (!strcmp(arg, "--list") || !strcmp(arg, "-l")) {
-			for (i = 0; i < ARRAY_SIZE(archivers); i++)
-				printf("%s\n", archivers[i].name);
-			exit(0);
-		}
-		if (!strcmp(arg, "--verbose") || !strcmp(arg, "-v")) {
-			verbose = 1;
-			continue;
-		}
-		if (!prefixcmp(arg, "--format=")) {
-			format = arg + 9;
-			continue;
-		}
-		if (!prefixcmp(arg, "--prefix=")) {
-			base = arg + 9;
-			continue;
-		}
-		if (!strcmp(arg, "--")) {
-			i++;
-			break;
-		}
-		if (arg[0] == '-' && isdigit(arg[1]) && arg[2] == '\0') {
-			compression_level = arg[1] - '0';
-			continue;
-		}
-		if (arg[0] == '-')
-			die("Unknown argument: %s", arg);
-		break;
-	}
-
-	/* We need at least one parameter -- tree-ish */
-	if (argc - 1 < i)
-		usage(archive_usage);
-	*ar = lookup_archiver(format);
-	if (!*ar)
-		die("Unknown archive format '%s'", format);
-
-	args->compression_level = Z_DEFAULT_COMPRESSION;
-	if (compression_level != -1) {
-		if ((*ar)->flags & USES_ZLIB_COMPRESSION)
-			args->compression_level = compression_level;
-		else {
-			die("Argument not supported for format '%s': -%d",
-					format, compression_level);
-		}
-	}
-	args->verbose = verbose;
-	args->base = base;
-	args->baselen = strlen(base);
-
-	return i;
-}
-
 static const char *extract_remote_arg(int *ac, const char **av)
 {
 	int ix, iy, cnt = *ac;
-- 
1.6.0.rc0.42.g186458

[PATCH 3/6] archive: define MAX_ARGS where it's needed

[Rene Scharfe]

MAX_EXTRA_ARGS is not used anymore, so remove it.  MAX_ARGS is used only
in builtin-upload-archive.c, so define it there.  Also report the actual
value we're comparing against when the number of args is too big.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 archive.h                |    3 ---
 builtin-upload-archive.c |    3 ++-
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/archive.h b/archive.h
index f6ceaeb..929368d 100644
--- a/archive.h
+++ b/archive.h
@@ -1,9 +1,6 @@
 #ifndef ARCHIVE_H
 #define ARCHIVE_H
 
-#define MAX_EXTRA_ARGS	32
-#define MAX_ARGS	(MAX_EXTRA_ARGS + 32)
-
 struct archiver_args {
 	const char *base;
 	size_t baselen;
diff --git a/builtin-upload-archive.c b/builtin-upload-archive.c
index cc37b36..a9b02fa 100644
--- a/builtin-upload-archive.c
+++ b/builtin-upload-archive.c
@@ -16,6 +16,7 @@ static const char deadchild[] =
 static const char lostchild[] =
 "git upload-archive: archiver process was lost";
 
+#define MAX_ARGS (64)
 
 static int run_upload_archive(int argc, const char **argv, const char *prefix)
 {
@@ -45,7 +46,7 @@ static int run_upload_archive(int argc, const char **argv, const char *prefix)
 		if (len == 0)
 			break;	/* got a flush */
 		if (sent_argc > MAX_ARGS - 2)
-			die("Too many options (>29)");
+			die("Too many options (>%d)", MAX_ARGS - 2);
 
 		if (p[len-1] == '\n') {
 			p[--len] = 0;
-- 
1.6.0.rc0.42.g186458

[PATCH 4/6] archive: declare struct archiver where it's needed

[Rene Scharfe]

Move the declaration of struct archiver to archive.c, as this is the only
file left where it is used.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 archive.c |    6 +++++-
 archive.h |    6 ------
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/archive.c b/archive.c
index c4662a2..f834b5f 100644
--- a/archive.c
+++ b/archive.c
@@ -9,7 +9,11 @@ static const char archive_usage[] = \
 
 #define USES_ZLIB_COMPRESSION 1
 
-const struct archiver archivers[] = {
+const struct archiver {
+	const char *name;
+	write_archive_fn_t write_archive;
+	unsigned int flags;
+} archivers[] = {
 	{ "tar", write_tar_archive },
 	{ "zip", write_zip_archive, USES_ZLIB_COMPRESSION },
 };
diff --git a/archive.h b/archive.h
index 929368d..0b15b35 100644
--- a/archive.h
+++ b/archive.h
@@ -17,12 +17,6 @@ typedef int (*write_archive_fn_t)(struct archiver_args *);
 
 typedef int (*write_archive_entry_fn_t)(struct archiver_args *args, const unsigned char *sha1, const char *path, size_t pathlen, unsigned int mode, void *buffer, unsigned long size);
 
-struct archiver {
-	const char *name;
-	write_archive_fn_t write_archive;
-	unsigned int flags;
-};
-
 /*
  * Archive-format specific backends.
  */
-- 
1.6.0.rc0.42.g186458

[PATCH 5/6] archive: allow --exec and --remote without equal sign

[Rene Scharfe]

Allow "--remote repo" and "--exec cmd" in addition to "--remote=repo" and
"--exec=cmd" to make their usage consistent with parameters handled by
parse_options().

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 builtin-archive.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/builtin-archive.c b/builtin-archive.c
index 4dd2716..22445ac 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -15,7 +15,7 @@ static int run_remote_archiver(const char *remote, int argc,
 	int fd[2], i, len, rv;
 	struct child_process *conn;
 	const char *exec = "git-upload-archive";
-	int exec_at = 0;
+	int exec_at = 0, exec_value_at = 0;
 
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
@@ -24,7 +24,14 @@ static int run_remote_archiver(const char *remote, int argc,
 				die("multiple --exec specified");
 			exec = arg + 7;
 			exec_at = i;
-			break;
+		} else if (!strcmp(arg, "--exec")) {
+			if (exec_at)
+				die("multiple --exec specified");
+			if (i + 1 >= argc)
+				die("option --exec requires a value");
+			exec = argv[i + 1];
+			exec_at = i;
+			exec_value_at = ++i;
 		}
 	}
 
@@ -32,7 +39,7 @@ static int run_remote_archiver(const char *remote, int argc,
 	conn = git_connect(fd, url, exec, 0);
 
 	for (i = 1; i < argc; i++) {
-		if (i == exec_at)
+		if (i == exec_at || i == exec_value_at)
 			continue;
 		packet_write(fd[1], "argument %s\n", argv[i]);
 	}
@@ -78,6 +85,13 @@ static const char *extract_remote_arg(int *ac, const char **av)
 					die("Multiple --remote specified");
 				remote = arg + 9;
 				continue;
+			} else if (!strcmp(arg, "--remote")) {
+				if (remote)
+					die("Multiple --remote specified");
+				if (++ix >= cnt)
+					die("option --remote requires a value");
+				remote = av[ix];
+				continue;
 			}
 			if (arg[0] != '-')
 				no_more_options = 1;
-- 
1.6.0.rc0.42.g186458

[PATCH 5/6] archive: allow --exec and --remote without equal sign

[Rene Scharfe]

Convert git archive to parse_options().  The parameters --remote and --exec
are still handled by their special parser.  Define them anyway in order for
them to show up in the usage notice.

Note: in a command like "git archive --prefix --remote=a/ HEAD", the string
"--remote=a/" will be interpreted as a remote option, not a prefix, because
that special parser sees it first.  If one needs such a strange prefix, it
needs to be specified like this: "git archive --prefix=--remote=a/ HEAD"
(with an equal sign).

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 archive.c |  110 +++++++++++++++++++++++++++++++++++++-----------------------
 1 files changed, 68 insertions(+), 42 deletions(-)

diff --git a/archive.c b/archive.c
index f834b5f..5b40e26 100644
--- a/archive.c
+++ b/archive.c
@@ -3,9 +3,15 @@
 #include "tree-walk.h"
 #include "attr.h"
 #include "archive.h"
-
-static const char archive_usage[] = \
-"git archive --format=<fmt> [--prefix=<prefix>/] [--verbose] [<extra>] <tree-ish> [path...]";
+#include "parse-options.h"
+
+static char const * const archive_usage[] = {
+	"git archive [options] <tree-ish> [path...]",
+	"git archive --list",
+	"git archive --remote <repo> [--exec <cmd>] [options] <tree-ish> [path...]",
+	"git archive --remote <repo> [--exec <cmd>] --list",
+	NULL
+};
 
 #define USES_ZLIB_COMPRESSION 1
 
@@ -175,6 +181,9 @@ static const struct archiver *lookup_archiver(const char *name)
 {
 	int i;
 
+	if (!name)
+		return NULL;
+
 	for (i = 0; i < ARRAY_SIZE(archivers); i++) {
 		if (!strcmp(name, archivers[i].name))
 			return &archivers[i];
@@ -232,51 +241,70 @@ static void parse_treeish_arg(const char **argv,
 	ar_args->time = archive_time;
 }
 
+#define OPT__COMPR(s, v, h, p) \
+	{ OPTION_SET_INT, (s), NULL, (v), NULL, (h), \
+	  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, (p) }
+#define OPT__COMPR_HIDDEN(s, v, p) \
+	{ OPTION_SET_INT, (s), NULL, (v), NULL, "", \
+	  PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_HIDDEN, NULL, (p) }
+
 static int parse_archive_args(int argc, const char **argv,
 		const struct archiver **ar, struct archiver_args *args)
 {
 	const char *format = "tar";
-	const char *base = "";
+	const char *base = NULL;
+	const char *remote = NULL;
+	const char *exec = NULL;
 	int compression_level = -1;
 	int verbose = 0;
 	int i;
-
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-
-		if (!strcmp(arg, "--list") || !strcmp(arg, "-l")) {
-			for (i = 0; i < ARRAY_SIZE(archivers); i++)
-				printf("%s\n", archivers[i].name);
-			exit(0);
-		}
-		if (!strcmp(arg, "--verbose") || !strcmp(arg, "-v")) {
-			verbose = 1;
-			continue;
-		}
-		if (!prefixcmp(arg, "--format=")) {
-			format = arg + 9;
-			continue;
-		}
-		if (!prefixcmp(arg, "--prefix=")) {
-			base = arg + 9;
-			continue;
-		}
-		if (!strcmp(arg, "--")) {
-			i++;
-			break;
-		}
-		if (arg[0] == '-' && isdigit(arg[1]) && arg[2] == '\0') {
-			compression_level = arg[1] - '0';
-			continue;
-		}
-		if (arg[0] == '-')
-			die("Unknown argument: %s", arg);
-		break;
+	int list = 0;
+	struct option opts[] = {
+		OPT_GROUP(""),
+		OPT_STRING(0, "format", &format, "fmt", "archive format"),
+		OPT_STRING(0, "prefix", &base, "prefix",
+			"prepend prefix to each pathname in the archive"),
+		OPT__VERBOSE(&verbose),
+		OPT__COMPR('0', &compression_level, "store only", 0),
+		OPT__COMPR('1', &compression_level, "compress faster", 1),
+		OPT__COMPR_HIDDEN('2', &compression_level, 2),
+		OPT__COMPR_HIDDEN('3', &compression_level, 3),
+		OPT__COMPR_HIDDEN('4', &compression_level, 4),
+		OPT__COMPR_HIDDEN('5', &compression_level, 5),
+		OPT__COMPR_HIDDEN('6', &compression_level, 6),
+		OPT__COMPR_HIDDEN('7', &compression_level, 7),
+		OPT__COMPR_HIDDEN('8', &compression_level, 8),
+		OPT__COMPR('9', &compression_level, "compress better", 9),
+		OPT_GROUP(""),
+		OPT_BOOLEAN('l', "list", &list,
+			"list supported archive formats"),
+		OPT_GROUP(""),
+		OPT_STRING(0, "remote", &remote, "repo",
+			"retrieve the archive from remote repository <repo>"),
+		OPT_STRING(0, "exec", &exec, "cmd",
+			"path to the remote git-upload-archive command"),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, opts, archive_usage, 0);
+
+	if (remote)
+		die("Unexpected option --remote");
+	if (exec)
+		die("Option --exec can only be used together with --remote");
+
+	if (!base)
+		base = "";
+
+	if (list) {
+		for (i = 0; i < ARRAY_SIZE(archivers); i++)
+			printf("%s\n", archivers[i].name);
+		exit(0);
 	}
 
 	/* We need at least one parameter -- tree-ish */
-	if (argc - 1 < i)
-		usage(archive_usage);
+	if (argc < 1)
+		usage_with_options(archive_usage, opts);
 	*ar = lookup_archiver(format);
 	if (!*ar)
 		die("Unknown archive format '%s'", format);
@@ -294,7 +322,7 @@ static int parse_archive_args(int argc, const char **argv,
 	args->base = base;
 	args->baselen = strlen(base);
 
-	return i;
+	return argc;
 }
 
 int write_archive(int argc, const char **argv, const char *prefix,
@@ -302,13 +330,11 @@ int write_archive(int argc, const char **argv, const char *prefix,
 {
 	const struct archiver *ar = NULL;
 	struct archiver_args args;
-	int tree_idx;
 
-	tree_idx = parse_archive_args(argc, argv, &ar, &args);
+	argc = parse_archive_args(argc, argv, &ar, &args);
 	if (setup_prefix && prefix == NULL)
 		prefix = setup_git_directory();
 
-	argv += tree_idx;
 	parse_treeish_arg(argv, &args, prefix);
 	parse_pathspec_arg(argv + 1, &args);
 
-- 
1.6.0.rc0.42.g186458

Re: [PATCH 5/6] archive: allow --exec and --remote without equal sign

[René Scharfe]

Rene Scharfe schrieb:
> Convert git archive to parse_options().  The parameters --remote and --exec
> are still handled by their special parser.  Define them anyway in order for
> them to show up in the usage notice.

Hmpf, the _real_ subject of this email is "[PATCH 6/6] archive: convert
to parse_options()"..

René

Re: [PATCH 5/6] archive: allow --exec and --remote without equal sign

[Junio C Hamano]

Rene Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Allow "--remote repo" and "--exec cmd" in addition to "--remote=repo" and
> "--exec=cmd" to make their usage consistent with parameters handled by
> parse_options().
>
> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
> ---
>  builtin-archive.c |   20 +++++++++++++++++---
>  1 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/builtin-archive.c b/builtin-archive.c
> index 4dd2716..22445ac 100644
> --- a/builtin-archive.c
> +++ b/builtin-archive.c
> @@ -15,7 +15,7 @@ static int run_remote_archiver(const char *remote, int argc,
>  	int fd[2], i, len, rv;
>  	struct child_process *conn;
>  	const char *exec = "git-upload-archive";
> -	int exec_at = 0;
> +	int exec_at = 0, exec_value_at = 0;
>  
>  	for (i = 1; i < argc; i++) {
>  		const char *arg = argv[i];
> @@ -24,7 +24,14 @@ static int run_remote_archiver(const char *remote, int argc,
>  				die("multiple --exec specified");
>  			exec = arg + 7;
>  			exec_at = i;
> -			break;

Interesting...  With this "break", we would have never triggered the
multiple --exec error, wouldn't we?

The patch itself looked fine.  Thanks.

Re: [PATCH 5/6] archive: allow --exec and --remote without equal sign

[Junio C Hamano]

Rene Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Convert git archive to parse_options().  The parameters --remote and --exec
> are still handled by their special parser.  Define them anyway in order for
> them to show up in the usage notice.
>
> Note: in a command like "git archive --prefix --remote=a/ HEAD", the string
> "--remote=a/" will be interpreted as a remote option, not a prefix, because
> that special parser sees it first.  If one needs such a strange prefix, it
> needs to be specified like this: "git archive --prefix=--remote=a/ HEAD"
> (with an equal sign).
>
> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
> ---
>  archive.c |  110 +++++++++++++++++++++++++++++++++++++-----------------------
>  1 files changed, 68 insertions(+), 42 deletions(-)

Hmph, somewhat dubious.

The real point of parse-options was to make the code smaller, easier to
maintain and command line handling more consistent.  At least this patch
seems to fail on the two out of three counts.

All of the other patches made obvious sense to me and are queued for -rc1
but I'd like to backburner this one.

Re: PATCH 6/6] archive: convert,to parse_options() [was: [PATCH 5/6] archive: allow --exec and --remote without equal sign]

[René Scharfe]

Junio C Hamano schrieb:
>>  archive.c |  110 +++++++++++++++++++++++++++++++++++++-----------------------
>>  1 files changed, 68 insertions(+), 42 deletions(-)
> 
> Hmph, somewhat dubious.
> 
> The real point of parse-options was to make the code smaller, easier to
> maintain and command line handling more consistent.  At least this patch
> seems to fail on the two out of three counts.

Well, if we hid away the compression level handling in a macro defined
in parse-options.h, we could save sixteen lines of code.  The patch
makes the four modes of running archive more explicit, adding three
usage lines.  Three empty lines are added -- they don't really increase
the code's size.

Handling --exec and --remote takes six lines; we didn't do that before
at this place, but have to now, since we want them to show up in the
usage.  We have to handle --no-format and --no-prefix, which adds four
lines.

So I don't think the bigger size make this patch dubious, but of course
I'm biased.  Disallowing --no-format (using a new OPT_STRING_NONEG?) and
adding an OPT__COMPRESSION helper might be a good idea (reducing line
count in archive.c by seventeen).

Having parse-options provide a way to make --exec and --remote appear in
the usage but to reject them (OPT_UNKNOWN?) is a bit too strange, though.

> All of the other patches made obvious sense to me and are queued for -rc1
> but I'd like to backburner this one.

Fair enough.

René