Re: [PATCH v6 2/7] archive --add-virtual-file: allow paths containing colons

[Junio C Hamano <gitster@pobox.com>]

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

> But in _this_ particular case, the "defensive" measure necessary is
> merely just to quote the shell variables properly, which nobody
> sensible would say too onerous.  I couldn't come up with anything
> remotely plausible to fill "for such and such reasons" myself when I
> tried to justify leaving the variables unquoted.
>
> Regardless of the quoting issue, we probably want to comment on what
> value exactly is in PATHNAME before the assignment, by the way.
>
> E.g.
>
> 	# The PATHNAME variable holds a filename encoded like a
> 	# string constant in C language (e.g. "\060" is digit "0")
> 	if test_have_prereq FUNNYNAMES
> 	then
> 		PATHNAME=quoted:colon:\\060zero
> 	else
> 		PATHNAME=quoted\\060zero
> 	fi
>
> That would not just protect only one aspect (i.e. we can pass a
> colon into the resulting filename) this change but the path goes
> through the c-unquoting rules.

Actually, I _think_ that pushes us beyond the "reasonably defensive
for the current need".  We'd need to prepare how the pathname is
expected to be unquoted for the later test

	test_path_is_file "$PATHNAME"

to work.  So here is what I queued as a fixup for this step on top
of the series.

 t/t5003-archive-zip.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git c/t/t5003-archive-zip.sh w/t/t5003-archive-zip.sh
index 3a5a052e8c..6addb6c684 100755
--- c/t/t5003-archive-zip.sh
+++ w/t/t5003-archive-zip.sh
@@ -209,19 +209,19 @@ check_added with_untracked untracked untracked
 test_expect_success UNZIP 'git archive --format=zip --add-virtual-file' '
 	if test_have_prereq FUNNYNAMES
 	then
-		PATHNAME=quoted:colon
+		PATHNAME="pathname with : colon"
 	else
-		PATHNAME=quoted
+		PATHNAME="pathname without colon"
 	fi &&
 	git archive --format=zip >with_file_with_content.zip \
-		--add-virtual-file=\"$PATHNAME\": \
+		--add-virtual-file=\""$PATHNAME"\": \
 		--add-virtual-file=hello:world $EMPTY_TREE &&
 	test_when_finished "rm -rf tmp-unpack" &&
 	mkdir tmp-unpack && (
 		cd tmp-unpack &&
 		"$GIT_UNZIP" ../with_file_with_content.zip &&
 		test_path_is_file hello &&
-		test_path_is_file $PATHNAME &&
+		test_path_is_file "$PATHNAME" &&
 		test world = $(cat hello)
 	)
 '