Thread (1 message) 1 message, 1 author, 2016-06-15
DORMANTno replies

[PATCH] Fix archive format with -- on the command line

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:47:53
Subsystem: the rest · Maintainer: Linus Torvalds

Possibly related (same subject, not in this thread)

Giving --format from the command line, or using output file extention to
DWIM the output format, with a pathspec that is disambiguated with an
explicit double-dash on the command line, e.g.

    git archive -o file --format=zip HEAD -- path
    git archive -o file.zip HEAD -- path

didn't work correctly.

This was because the code reordered (when one was given) or added (when
the former was inferred) a --format argument at the end, effectively
making it to "archive HEAD -- path --format=zip", i.e. an extra pathspec
that is unlikely to match anything.

The command line argument list should always be "options, revs and then
paths", and we should set a good example by inserting the --format at the
beginning instead.

Reported-by: Ilari Liusvaara <redacted>
Signed-off-by: Junio C Hamano <redacted>
---

 * So here is one with a proper commit log message. 

 builtin-archive.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/builtin-archive.c b/builtin-archive.c
index 12351e9..8ef5ab3 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -106,13 +106,17 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
 	if (format) {
 		sprintf(fmt_opt, "--format=%s", format);
 		/*
-		 * This is safe because either --format and/or --output must
-		 * have been given on the original command line if we get to
-		 * this point, and parse_options() must have eaten at least
-		 * one argument, i.e. we have enough room to append to argv[].
+		 * We have enough room in argv[] to muck it in place,
+		 * because either --format and/or --output must have
+		 * been given on the original command line if we get
+		 * to this point, and parse_options() must have eaten
+		 * it, i.e. we can add back one element to the array.
+		 * But argv[] may contain "--"; we should make it the
+		 * first option.
 		 */
-		argv[argc++] = fmt_opt;
-		argv[argc] = NULL;
+		memmove(argv + 2, argv + 1, sizeof(*argv) * argc);
+		argv[1] = fmt_opt;
+		argv[++argc] = NULL;
 	}
 
 	if (remote)
-- 
1.6.6.rc2
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help