[PATCH 2/2] bundle: rewrite builtin to use parse-options

[Ramkumar Ramachandra <artagnon@gmail.com>]

The git-bundle builtin currently parses command-line options by hand;
this is fragile, and reports cryptic errors on failure.  Use the
parse-options library to do the parsing instead.

Encouraged-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 builtin/bundle.c  |   91 +++++++++++++++++++++++++++++++----------------------
 t/t5704-bundle.sh |    2 +-
 2 files changed, 54 insertions(+), 39 deletions(-)

diff --git a/builtin/bundle.c b/builtin/bundle.c
index 92a8a60..13ed770 100644
--- a/builtin/bundle.c
+++ b/builtin/bundle.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "cache.h"
+#include "parse-options.h"
 #include "bundle.h"
 
 /*
@@ -9,57 +10,71 @@
  * bundle supporting "fetch", "pull", and "ls-remote".
  */
 
-static const char builtin_bundle_usage[] =
-  "git bundle create <file> <git-rev-list args>\n"
-  "   or: git bundle verify <file>\n"
-  "   or: git bundle list-heads <file> [<refname>...]\n"
-  "   or: git bundle unbundle <file> [<refname>...]";
+static const char * builtin_bundle_usage[] = {
+	"git bundle create <file> <git-rev-list args>",
+	"git bundle verify <file>",
+	"git bundle list-heads <file> [<refname>...]",
+	"git bundle unbundle <file> [<refname>...]",
+	NULL
+};
 
 int cmd_bundle(int argc, const char **argv, const char *prefix)
 {
-	struct bundle_header header;
-	const char *cmd, *bundle_file;
+	int prefix_length;
 	int bundle_fd = -1;
-	char buffer[PATH_MAX];
+	const char *subcommand, *bundle_file;
+	struct bundle_header header;
+	struct option options[] = { OPT_END() };
 
-	if (argc < 3)
-		usage(builtin_bundle_usage);
+	argc = parse_options(argc, argv, prefix, options,
+			builtin_bundle_usage, PARSE_OPT_STOP_AT_NON_OPTION);
 
-	cmd = argv[1];
-	bundle_file = argv[2];
-	argc -= 2;
-	argv += 2;
+	if (argc < 2)
+		usage_with_options(builtin_bundle_usage, options);
+	subcommand = argv[0];
 
-	if (prefix && bundle_file[0] != '/') {
-		snprintf(buffer, sizeof(buffer), "%s/%s", prefix, bundle_file);
-		bundle_file = buffer;
-	}
+	argc = parse_options(argc, argv, prefix, options,
+			builtin_bundle_usage, PARSE_OPT_STOP_AT_NON_OPTION);
+
+	/* Disallow stray arguments */
+	if ((strcmp(subcommand, "create") && argc > 2) ||
+		(!strcmp(subcommand, "verify") && argc > 1))
+		usage_with_options(builtin_bundle_usage, options);
 
-	memset(&header, 0, sizeof(header));
-	if (strcmp(cmd, "create") && (bundle_fd =
-				read_bundle_header(bundle_file, &header)) < 0)
-		return 1;
+	prefix_length = prefix ? strlen(prefix) : 0;
+	bundle_file = prefix_filename(prefix, prefix_length, argv[0]);
 
-	if (!strcmp(cmd, "verify")) {
+	/* Read out bundle header, except in the "create" case */
+	if (strcmp(subcommand, "create")) {
+		memset(&header, 0, sizeof(header));
+		bundle_fd = read_bundle_header(bundle_file, &header);
+		if (bundle_fd < 0)
+			die_errno(_("Failed to open bundle file '%s'"), bundle_file);
+	}
+
+	if (!strcmp(subcommand, "create")) {
+		if (!startup_info->have_repository)
+			die(_("Need a repository to create a bundle."));
+		return create_bundle(&header, bundle_file, argc, argv);
+	} else if (!strcmp(subcommand, "verify")) {
 		close(bundle_fd);
 		if (verify_bundle(&header, 1))
-			return 1;
+			return -1; /* Error already reported */
 		fprintf(stderr, _("%s is okay\n"), bundle_file);
-		return 0;
-	}
-	if (!strcmp(cmd, "list-heads")) {
+	} else if (!strcmp(subcommand, "list-heads")) {
 		close(bundle_fd);
-		return !!list_bundle_refs(&header, argc, argv);
-	}
-	if (!strcmp(cmd, "create")) {
-		if (!startup_info->have_repository)
-			die(_("Need a repository to create a bundle."));
-		return !!create_bundle(&header, bundle_file, argc, argv);
-	} else if (!strcmp(cmd, "unbundle")) {
-		if (!startup_info->have_repository)
+		return list_bundle_refs(&header, argc, argv);
+	} else if (!strcmp(subcommand, "unbundle")) {
+		if (!startup_info->have_repository) {
+			close(bundle_fd);
 			die(_("Need a repository to unbundle."));
-		return !!unbundle(&header, bundle_fd, 0) ||
+		}
+		return unbundle(&header, bundle_fd, 0) ||
 			list_bundle_refs(&header, argc, argv);
-	} else
-		usage(builtin_bundle_usage);
+	} else {
+		close(bundle_fd);
+		usage_with_options(builtin_bundle_usage, options);
+	}
+
+	return 0;
 }
diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh
index 09ff4f1..8e3f677 100755
--- a/t/t5704-bundle.sh
+++ b/t/t5704-bundle.sh
@@ -53,7 +53,7 @@ test_expect_success 'disallow stray command-line options' '
 	test_must_fail git bundle create --junk bundle second third
 '
 
-test_expect_failure 'disallow stray command-line arguments' '
+test_expect_success 'disallow stray command-line arguments' '
 	git bundle create bundle second third &&
 	test_must_fail git bundle verify bundle junk
 '
-- 
1.7.4.1