All of lore.kernel.org
 help / color / mirror / Atom feed
* What's in git.git (Mar 2009, #02; Thu, 05)
@ 2009-03-05 10:07 Junio C Hamano
  2009-03-07 19:14 ` René Scharfe
  0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2009-03-05 10:07 UTC (permalink / raw)
  To: git

The release is out, and immediately after that somebody spots a minor
bug.

If things go as planned, tomorrow we will see a mass graduation to the
master branch of topics that have been cooking in next during the pre
release freeze period.

* The 'maint' branch has these fixes since v1.6.2

John Tapsell (1):
  Make the 'lock file' exists error more informative

Junio C Hamano (1):
  Beginning of 1.6.2 maintenance track


* The 'master' branch has these since v1.6.2 in addition to the above.

Carlos Manuel Duclos Vergara (1):
  git-archive: add --output=<file> to send output to a file

Christian Couder (1):
  rev-list: estimate number of bisection step left

Jeff King (1):
  improve missing repository error message

John Tapsell (3):
  Modify description file to say what this file is
  Google has renamed the imap folder
  Improve error message for git-filter-branch

Keith Cascio (2):
  Use DIFF_XDL_SET/DIFF_OPT_SET instead of raw bit-masking
  Fix neglect of diff_setup()/diff_setup_done() symmetry.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: What's in git.git (Mar 2009, #02; Thu, 05)
  2009-03-05 10:07 What's in git.git (Mar 2009, #02; Thu, 05) Junio C Hamano
@ 2009-03-07 19:14 ` René Scharfe
  2009-03-08 18:12   ` [PATCH 1/4] parseopt: add PARSE_OPT_KEEP_UNKNOWN René Scharfe
                     ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: René Scharfe @ 2009-03-07 19:14 UTC (permalink / raw)
  To: Junio C Hamano, carlos.duclos; +Cc: git

Junio C Hamano schrieb:
> * The 'master' branch has these since v1.6.2 in addition to the above.
> 
> Carlos Manuel Duclos Vergara (1):
>   git-archive: add --output=<file> to send output to a file

It just hit me that this is option can be used for a DoS attack (or
perhaps worse) when used in connection with --remote.  We need to apply
it on the client side instead of sending it to the remote end.  And
git-upload-archive needs to filter it out.  Ugh.

Here's a quick and dirty patch to do the latter.

---
 archive.c |   14 +++++++++-----
 archive.h |    2 +-
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/archive.c b/archive.c
index c6aea83..c7534d7 100644
--- a/archive.c
+++ b/archive.c
@@ -260,7 +260,8 @@ static void create_output_file(const char *output_file)
 	  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 struct archiver **ar,
+			      struct archiver_args *args, int local)
 {
 	const char *format = "tar";
 	const char *base = NULL;
@@ -310,8 +311,11 @@ static int parse_archive_args(int argc, const char **argv,
 	if (!base)
 		base = "";
 
-	if (output)
+	if (output) {
+		if (!local)
+			die("Unexpected option --output");
 		create_output_file(output);
+	}
 
 	if (list) {
 		for (i = 0; i < ARRAY_SIZE(archivers); i++)
@@ -343,13 +347,13 @@ static int parse_archive_args(int argc, const char **argv,
 }
 
 int write_archive(int argc, const char **argv, const char *prefix,
-		int setup_prefix)
+		int local)
 {
 	const struct archiver *ar = NULL;
 	struct archiver_args args;
 
-	argc = parse_archive_args(argc, argv, &ar, &args);
-	if (setup_prefix && prefix == NULL)
+	argc = parse_archive_args(argc, argv, &ar, &args, local);
+	if (local && prefix == NULL)
 		prefix = setup_git_directory();
 
 	parse_treeish_arg(argv, &args, prefix);
diff --git a/archive.h b/archive.h
index 0b15b35..f6c3c89 100644
--- a/archive.h
+++ b/archive.h
@@ -24,6 +24,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);
+extern int write_archive(int argc, const char **argv, const char *prefix, int local);
 
 #endif	/* ARCHIVE_H */
-- 
1.6.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 1/4] parseopt: add PARSE_OPT_KEEP_UNKNOWN
  2009-03-07 19:14 ` René Scharfe
@ 2009-03-08 18:12   ` René Scharfe
  2009-03-08 20:24     ` Junio C Hamano
  2009-03-08 18:15   ` [PATCH 2/4] parseopt: add PARSE_OPT_NO_INTERNAL_HELP René Scharfe
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: René Scharfe @ 2009-03-08 18:12 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, carlos.duclos, Pierre Habouzit

Add a parseopt flag, PARSE_OPT_KEEP_UNKNOWN, that can be used to keep
unknown options in argv, similar to the existing KEEP flags.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 parse-options.c |   12 +++++++++---
 parse-options.h |    1 +
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 4c5d09d..39808ae 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -274,7 +274,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 			case -1:
 				return parse_options_usage(usagestr, options);
 			case -2:
-				return PARSE_OPT_UNKNOWN;
+				goto unknown;
 			}
 			if (ctx->opt)
 				check_typos(arg + 1, options);
@@ -292,7 +292,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 					 */
 					ctx->argv[0] = xstrdup(ctx->opt - 1);
 					*(char *)ctx->argv[0] = '-';
-					return PARSE_OPT_UNKNOWN;
+					goto unknown;
 				}
 			}
 			continue;
@@ -314,8 +314,14 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 		case -1:
 			return parse_options_usage(usagestr, options);
 		case -2:
-			return PARSE_OPT_UNKNOWN;
+			goto unknown;
 		}
+		continue;
+unknown:
+		if (!(ctx->flags & PARSE_OPT_KEEP_UNKNOWN))
+			return PARSE_OPT_UNKNOWN;
+		ctx->out[ctx->cpidx++] = ctx->argv[0];
+		ctx->opt = NULL;
 	}
 	return PARSE_OPT_DONE;
 }
diff --git a/parse-options.h b/parse-options.h
index 9122905..b7d08b1 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -21,6 +21,7 @@ enum parse_opt_flags {
 	PARSE_OPT_KEEP_DASHDASH = 1,
 	PARSE_OPT_STOP_AT_NON_OPTION = 2,
 	PARSE_OPT_KEEP_ARGV0 = 4,
+	PARSE_OPT_KEEP_UNKNOWN = 8,
 };
 
 enum parse_opt_option_flags {
-- 
1.6.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/4] parseopt: add PARSE_OPT_NO_INTERNAL_HELP
  2009-03-07 19:14 ` René Scharfe
  2009-03-08 18:12   ` [PATCH 1/4] parseopt: add PARSE_OPT_KEEP_UNKNOWN René Scharfe
@ 2009-03-08 18:15   ` René Scharfe
  2009-03-08 18:16   ` [PATCH 3/4] parseopt: make usage optional René Scharfe
  2009-03-08 18:21   ` [PATCH 4/4] archive: use parseopt for local-only options René Scharfe
  3 siblings, 0 replies; 13+ messages in thread
From: René Scharfe @ 2009-03-08 18:15 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, carlos.duclos, Pierre Habouzit

Add a parseopt flag, PARSE_OPT_NO_INTERNAL_HELP, that turns off internal
handling of -h, --help and --help-all.  This allows the implementation
of custom help option handlers or incremental parsers.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 parse-options.c |   10 ++++++----
 parse-options.h |    1 +
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 39808ae..8b21dea 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -253,6 +253,8 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 		       const struct option *options,
 		       const char * const usagestr[])
 {
+	int internal_help = !(ctx->flags & PARSE_OPT_NO_INTERNAL_HELP);
+
 	/* we must reset ->opt, unknown short option leave it dangling */
 	ctx->opt = NULL;
 
@@ -268,7 +270,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 
 		if (arg[1] != '-') {
 			ctx->opt = arg + 1;
-			if (*ctx->opt == 'h')
+			if (internal_help && *ctx->opt == 'h')
 				return parse_options_usage(usagestr, options);
 			switch (parse_short_opt(ctx, options)) {
 			case -1:
@@ -279,7 +281,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 			if (ctx->opt)
 				check_typos(arg + 1, options);
 			while (ctx->opt) {
-				if (*ctx->opt == 'h')
+				if (internal_help && *ctx->opt == 'h')
 					return parse_options_usage(usagestr, options);
 				switch (parse_short_opt(ctx, options)) {
 				case -1:
@@ -306,9 +308,9 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 			break;
 		}
 
-		if (!strcmp(arg + 2, "help-all"))
+		if (internal_help && !strcmp(arg + 2, "help-all"))
 			return usage_with_options_internal(usagestr, options, 1);
-		if (!strcmp(arg + 2, "help"))
+		if (internal_help && !strcmp(arg + 2, "help"))
 			return parse_options_usage(usagestr, options);
 		switch (parse_long_opt(ctx, arg + 2, options)) {
 		case -1:
diff --git a/parse-options.h b/parse-options.h
index b7d08b1..f8ef1db 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -22,6 +22,7 @@ enum parse_opt_flags {
 	PARSE_OPT_STOP_AT_NON_OPTION = 2,
 	PARSE_OPT_KEEP_ARGV0 = 4,
 	PARSE_OPT_KEEP_UNKNOWN = 8,
+	PARSE_OPT_NO_INTERNAL_HELP = 16,
 };
 
 enum parse_opt_option_flags {
-- 
1.6.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/4] parseopt: make usage optional
  2009-03-07 19:14 ` René Scharfe
  2009-03-08 18:12   ` [PATCH 1/4] parseopt: add PARSE_OPT_KEEP_UNKNOWN René Scharfe
  2009-03-08 18:15   ` [PATCH 2/4] parseopt: add PARSE_OPT_NO_INTERNAL_HELP René Scharfe
@ 2009-03-08 18:16   ` René Scharfe
  2009-03-08 20:25     ` Junio C Hamano
  2009-03-08 18:21   ` [PATCH 4/4] archive: use parseopt for local-only options René Scharfe
  3 siblings, 1 reply; 13+ messages in thread
From: René Scharfe @ 2009-03-08 18:16 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, carlos.duclos, Pierre Habouzit

Allow usagestr to be NULL and don't display anything a help screen in
this case.  This is useful to implement incremental parsers.

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

diff --git a/parse-options.c b/parse-options.c
index 8b21dea..51e804b 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -364,6 +364,9 @@ int parse_options(int argc, const char **argv, const struct option *options,
 int usage_with_options_internal(const char * const *usagestr,
 				const struct option *opts, int full)
 {
+	if (!usagestr)
+		return PARSE_OPT_HELP;
+
 	fprintf(stderr, "usage: %s\n", *usagestr++);
 	while (*usagestr && **usagestr)
 		fprintf(stderr, "   or: %s\n", *usagestr++);
-- 
1.6.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/4] archive: use parseopt for local-only options
  2009-03-07 19:14 ` René Scharfe
                     ` (2 preceding siblings ...)
  2009-03-08 18:16   ` [PATCH 3/4] parseopt: make usage optional René Scharfe
@ 2009-03-08 18:21   ` René Scharfe
  2009-03-08 20:20     ` Junio C Hamano
  3 siblings, 1 reply; 13+ messages in thread
From: René Scharfe @ 2009-03-08 18:21 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, carlos.duclos, Pierre Habouzit

Replace the hand-rolled parsers that find and remove --remote and --exec
by a parseopt parser that also handles --output.

All three options only have a meaning if no remote server is used or on
the local side.  They must be rejected by upload-archive and should not
be sent to the server by archive.

We can't use a single parser for both remote and local side because the
remote end possibly understands a different set of options than the
local side.  A local parser would then wrongly accuse options valid on
the other side as being incorrect.

This patch implements a very forgiving parser that understands only the
three options mentioned above.  All others are passed to the normal,
complete parser in archive.c (running either locally in archive, or
remotely in upload-archive).  This normal parser definition contains
dummy entries for the three options, in order for them to appear in the
help screen.

The parseopt parser allows multiple occurrences of --remote and --exec
unlike the previous one; the one specified last wins.  This looseness
is acceptable, I think.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 archive.c         |   18 +--------
 builtin-archive.c |  103 +++++++++++++++++++---------------------------------
 2 files changed, 40 insertions(+), 81 deletions(-)

diff --git a/archive.c b/archive.c
index c6aea83..96b62d4 100644
--- a/archive.c
+++ b/archive.c
@@ -239,19 +239,6 @@ static void parse_treeish_arg(const char **argv,
 	ar_args->time = archive_time;
 }
 
-static void create_output_file(const char *output_file)
-{
-	int output_fd = open(output_file, O_CREAT | O_WRONLY | O_TRUNC, 0666);
-	if (output_fd < 0)
-		die("could not create archive file: %s ", output_file);
-	if (output_fd != 1) {
-		if (dup2(output_fd, 1) < 0)
-			die("could not redirect output");
-		else
-			close(output_fd);
-	}
-}
-
 #define OPT__COMPR(s, v, h, p) \
 	{ OPTION_SET_INT, (s), NULL, (v), NULL, (h), \
 	  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, (p) }
@@ -306,13 +293,12 @@ static int parse_archive_args(int argc, const char **argv,
 		die("Unexpected option --remote");
 	if (exec)
 		die("Option --exec can only be used together with --remote");
+	if (output)
+		die("Unexpected option --output");
 
 	if (!base)
 		base = "";
 
-	if (output)
-		create_output_file(output);
-
 	if (list) {
 		for (i = 0; i < ARRAY_SIZE(archivers); i++)
 			printf("%s\n", archivers[i].name);
diff --git a/builtin-archive.c b/builtin-archive.c
index 5ceec43..60adef9 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -5,44 +5,35 @@
 #include "cache.h"
 #include "builtin.h"
 #include "archive.h"
+#include "parse-options.h"
 #include "pkt-line.h"
 #include "sideband.h"
 
-static int run_remote_archiver(const char *remote, int argc,
-			       const char **argv)
+static void create_output_file(const char *output_file)
+{
+	int output_fd = open(output_file, O_CREAT | O_WRONLY | O_TRUNC, 0666);
+	if (output_fd < 0)
+		die("could not create archive file: %s ", output_file);
+	if (output_fd != 1) {
+		if (dup2(output_fd, 1) < 0)
+			die("could not redirect output");
+		else
+			close(output_fd);
+	}
+}
+
+static int run_remote_archiver(int argc, const char **argv,
+			       const char *remote, const char *exec)
 {
 	char *url, buf[LARGE_PACKET_MAX];
 	int fd[2], i, len, rv;
 	struct child_process *conn;
-	const char *exec = "git-upload-archive";
-	int exec_at = 0, exec_value_at = 0;
-
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-		if (!prefixcmp(arg, "--exec=")) {
-			if (exec_at)
-				die("multiple --exec specified");
-			exec = arg + 7;
-			exec_at = i;
-		} 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;
-		}
-	}
 
 	url = xstrdup(remote);
 	conn = git_connect(fd, url, exec, 0);
 
-	for (i = 1; i < argc; i++) {
-		if (i == exec_at || i == exec_value_at)
-			continue;
+	for (i = 1; i < argc; i++)
 		packet_write(fd[1], "argument %s\n", argv[i]);
-	}
 	packet_flush(fd[1]);
 
 	len = packet_read_line(fd[0], buf, sizeof(buf));
@@ -69,51 +60,33 @@ static int run_remote_archiver(const char *remote, int argc,
 	return !!rv;
 }
 
-static const char *extract_remote_arg(int *ac, const char **av)
-{
-	int ix, iy, cnt = *ac;
-	int no_more_options = 0;
-	const char *remote = NULL;
-
-	for (ix = iy = 1; ix < cnt; ix++) {
-		const char *arg = av[ix];
-		if (!strcmp(arg, "--"))
-			no_more_options = 1;
-		if (!no_more_options) {
-			if (!prefixcmp(arg, "--remote=")) {
-				if (remote)
-					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;
-		}
-		if (ix != iy)
-			av[iy] = arg;
-		iy++;
-	}
-	if (remote) {
-		av[--cnt] = NULL;
-		*ac = cnt;
-	}
-	return remote;
-}
+#define PARSE_OPT_KEEP_ALL ( PARSE_OPT_KEEP_DASHDASH | 	\
+			     PARSE_OPT_KEEP_ARGV0 | 	\
+			     PARSE_OPT_KEEP_UNKNOWN |	\
+			     PARSE_OPT_NO_INTERNAL_HELP	)
 
 int cmd_archive(int argc, const char **argv, const char *prefix)
 {
+	const char *exec = "git-upload-archive";
+	const char *output = NULL;
 	const char *remote = NULL;
+	struct option local_opts[] = {
+		OPT_STRING(0, "output", &output, "file",
+			"write the archive to this file"),
+		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, local_opts, NULL, PARSE_OPT_KEEP_ALL);
+
+	if (output)
+		create_output_file(output);
 
-	remote = extract_remote_arg(&argc, argv);
 	if (remote)
-		return run_remote_archiver(remote, argc, argv);
+		return run_remote_archiver(argc, argv, remote, exec);
 
 	setvbuf(stderr, NULL, _IOLBF, BUFSIZ);
 
-- 
1.6.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/4] archive: use parseopt for local-only options
  2009-03-08 18:21   ` [PATCH 4/4] archive: use parseopt for local-only options René Scharfe
@ 2009-03-08 20:20     ` Junio C Hamano
  0 siblings, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2009-03-08 20:20 UTC (permalink / raw)
  To: René Scharfe; +Cc: git, carlos.duclos, Pierre Habouzit

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

> The parseopt parser allows multiple occurrences of --remote and --exec
> unlike the previous one; the one specified last wins.  This looseness
> is acceptable, I think.

I agree.

If we care about this very deeply, we can add "this option is supposed to
be singleton" option to the parse_options infrastructure.  Besides, the
"last one wins" rule is often more convenient than "there has to be at
most one" rule in real life.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] parseopt: add PARSE_OPT_KEEP_UNKNOWN
  2009-03-08 18:12   ` [PATCH 1/4] parseopt: add PARSE_OPT_KEEP_UNKNOWN René Scharfe
@ 2009-03-08 20:24     ` Junio C Hamano
  2009-03-08 20:30       ` Junio C Hamano
                         ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Junio C Hamano @ 2009-03-08 20:24 UTC (permalink / raw)
  To: René Scharfe; +Cc: git, carlos.duclos, Pierre Habouzit

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

> Add a parseopt flag, PARSE_OPT_KEEP_UNKNOWN, that can be used to keep
> unknown options in argv, similar to the existing KEEP flags.

Very nice.

The only caveat I can think of is with PARSE_OPT_STOP_AT_NON_OPTION set
(which is not default), you can correctly handle:

	git cmd --known --unknown=value arg0 arg1

but cannot correctly handle:

	git cmd --known --unknown value arg0 arg1

An update to Documentation/technical/api-parse-options.txt that

 (1) describes this new option; and

 (2) warns about this issue.

It might even make sense to diagnose PARSE_OPT_STOP_AT_NON_OPTION used
together with PARSE_OPT_KEEP_UNKNOWN as a programming error.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] parseopt: make usage optional
  2009-03-08 18:16   ` [PATCH 3/4] parseopt: make usage optional René Scharfe
@ 2009-03-08 20:25     ` Junio C Hamano
  2009-03-09 20:19       ` René Scharfe
  0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2009-03-08 20:25 UTC (permalink / raw)
  To: René Scharfe; +Cc: git, carlos.duclos, Pierre Habouzit

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

> Allow usagestr to be NULL and don't display anything a help screen in
> this case.  This is useful to implement incremental parsers.

Can I amend "s/anything a/any/"?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] parseopt: add PARSE_OPT_KEEP_UNKNOWN
  2009-03-08 20:24     ` Junio C Hamano
@ 2009-03-08 20:30       ` Junio C Hamano
  2009-03-09 20:26       ` [PATCH 5/4] parseopt: document KEEP_ARGV0, KEEP_UNKNOWN, NO_INTERNAL_HELP René Scharfe
  2009-03-09 20:57       ` [PATCH 6/4] parseopt: prevent KEEP_UNKNOWN and STOP_AT_NON_OPTION from being used together René Scharfe
  2 siblings, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2009-03-08 20:30 UTC (permalink / raw)
  To: René Scharfe; +Cc: git, carlos.duclos, Pierre Habouzit

Junio C Hamano <gitster@pobox.com> writes:

> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>
>> Add a parseopt flag, PARSE_OPT_KEEP_UNKNOWN, that can be used to keep
>> unknown options in argv, similar to the existing KEEP flags.
>
> Very nice.
>
> The only caveat I can think of is with PARSE_OPT_STOP_AT_NON_OPTION set
> (which is not default), you can correctly handle:
>
> 	git cmd --known --unknown=value arg0 arg1
>
> but cannot correctly handle:
>
> 	git cmd --known --unknown value arg0 arg1
>
> An update to Documentation/technical/api-parse-options.txt that
>
>  (1) describes this new option; and
>
>  (2) warns about this issue.

"is necessary" is necessary here to complete my sentence.  Sorry.

> It might even make sense to diagnose PARSE_OPT_STOP_AT_NON_OPTION used
> together with PARSE_OPT_KEEP_UNKNOWN as a programming error.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] parseopt: make usage optional
  2009-03-08 20:25     ` Junio C Hamano
@ 2009-03-09 20:19       ` René Scharfe
  0 siblings, 0 replies; 13+ messages in thread
From: René Scharfe @ 2009-03-09 20:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, carlos.duclos, Pierre Habouzit

Junio C Hamano schrieb:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> 
>> Allow usagestr to be NULL and don't display anything a help screen in
>> this case.  This is useful to implement incremental parsers.
> 
> Can I amend "s/anything a/any/"?

Oh, definitely.

Thanks,
René

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 5/4] parseopt: document KEEP_ARGV0, KEEP_UNKNOWN, NO_INTERNAL_HELP
  2009-03-08 20:24     ` Junio C Hamano
  2009-03-08 20:30       ` Junio C Hamano
@ 2009-03-09 20:26       ` René Scharfe
  2009-03-09 20:57       ` [PATCH 6/4] parseopt: prevent KEEP_UNKNOWN and STOP_AT_NON_OPTION from being used together René Scharfe
  2 siblings, 0 replies; 13+ messages in thread
From: René Scharfe @ 2009-03-09 20:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, carlos.duclos, Pierre Habouzit

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 Documentation/technical/api-parse-options.txt |   27 +++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
index 539863b..dc72987 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -66,6 +66,12 @@ Steps to parse options
 non-option arguments in `argv[]`.
 `argc` is updated appropriately because of the assignment.
 +
+You can also pass NULL instead of a usage array as fourth parameter of
+parse_options(), to avoid displaying a help screen with usage info and
+option list.  This should only be done if necessary, e.g. to implement
+a limited parser for only a subset of the options that needs to be run
+before the full parser, which in turn shows the full help message.
++
 Flags are the bitwise-or of:
 
 `PARSE_OPT_KEEP_DASHDASH`::
@@ -77,6 +83,27 @@ Flags are the bitwise-or of:
 	Using this flag, processing is stopped at the first non-option
 	argument.
 
+`PARSE_OPT_KEEP_ARGV0`::
+	Keep the first argument, which contains the program name.  It's
+	removed from argv[] by default.
+
+`PARSE_OPT_KEEP_UNKNOWN`::
+	Keep unknown arguments instead of erroring out.  This doesn't
+	work for all combinations of arguments as users might expect
+	it to do.  E.g. if the first argument in `--unknown --known`
+	takes a value (which we can't know), the second one is
+	mistakenly interpreted as a known option.  Similarly, if
+	`PARSE_OPT_STOP_AT_NON_OPTION` is set, the second argument in
+	`--unknown value` will be mistakenly interpreted as a
+	non-option, not as a value belonging to the unknown option,
+	stopping the parser early.
+
+`PARSE_OPT_NO_INTERNAL_HELP`::
+	By default, parse_options() handles `-h`, `--help` and
+	`--help-all` internally, by showing a help screen.  This option
+	turns it off and allows one to add custom handlers for these
+	options, or to just leave them unknown.
+
 Data Structure
 --------------
 
-- 
1.6.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 6/4] parseopt: prevent KEEP_UNKNOWN and STOP_AT_NON_OPTION from being used together
  2009-03-08 20:24     ` Junio C Hamano
  2009-03-08 20:30       ` Junio C Hamano
  2009-03-09 20:26       ` [PATCH 5/4] parseopt: document KEEP_ARGV0, KEEP_UNKNOWN, NO_INTERNAL_HELP René Scharfe
@ 2009-03-09 20:57       ` René Scharfe
  2 siblings, 0 replies; 13+ messages in thread
From: René Scharfe @ 2009-03-09 20:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, carlos.duclos, Pierre Habouzit

As suggested by Junio, disallow the flags PARSE_OPT_KEEP_UNKNOWN and
PARSE_OPT_STOP_AT_NON_OPTION to be turned on at the same time, as a
value of an unknown option could be mistakenly classified as a
non-option, stopping the parser early.  E.g.:

	git cmd --known --unknown value arg0 arg1

The parser should have stopped at "arg0", but it already stops at
"value".

This patch makes parse_options() die if the two flags are used in
combination.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 Documentation/technical/api-parse-options.txt |    3 ++-
 parse-options.c                               |    3 +++
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
index dc72987..f5d4ac1 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -96,7 +96,8 @@ Flags are the bitwise-or of:
 	`PARSE_OPT_STOP_AT_NON_OPTION` is set, the second argument in
 	`--unknown value` will be mistakenly interpreted as a
 	non-option, not as a value belonging to the option, stopping
-	the parser early.
+	the parser early.  That's why parse_options() errors out if
+	both options are set.
 
 `PARSE_OPT_NO_INTERNAL_HELP`::
 	By default, parse_options() handles `-h`, `--help` and
diff --git a/parse-options.c b/parse-options.c
index 51e804b..cf71bcf 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -244,6 +244,9 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
 	ctx->out  = argv;
 	ctx->cpidx = ((flags & PARSE_OPT_KEEP_ARGV0) != 0);
 	ctx->flags = flags;
+	if ((flags & PARSE_OPT_KEEP_UNKNOWN) &&
+	    (flags & PARSE_OPT_STOP_AT_NON_OPTION))
+		die("STOP_AT_NON_OPTION and KEEP_UNKNOWN don't go together");
 }
 
 static int usage_with_options_internal(const char * const *,
-- 
1.6.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2009-03-09 20:59 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-03-05 10:07 What's in git.git (Mar 2009, #02; Thu, 05) Junio C Hamano
2009-03-07 19:14 ` René Scharfe
2009-03-08 18:12   ` [PATCH 1/4] parseopt: add PARSE_OPT_KEEP_UNKNOWN René Scharfe
2009-03-08 20:24     ` Junio C Hamano
2009-03-08 20:30       ` Junio C Hamano
2009-03-09 20:26       ` [PATCH 5/4] parseopt: document KEEP_ARGV0, KEEP_UNKNOWN, NO_INTERNAL_HELP René Scharfe
2009-03-09 20:57       ` [PATCH 6/4] parseopt: prevent KEEP_UNKNOWN and STOP_AT_NON_OPTION from being used together René Scharfe
2009-03-08 18:15   ` [PATCH 2/4] parseopt: add PARSE_OPT_NO_INTERNAL_HELP René Scharfe
2009-03-08 18:16   ` [PATCH 3/4] parseopt: make usage optional René Scharfe
2009-03-08 20:25     ` Junio C Hamano
2009-03-09 20:19       ` René Scharfe
2009-03-08 18:21   ` [PATCH 4/4] archive: use parseopt for local-only options René Scharfe
2009-03-08 20:20     ` Junio C Hamano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.