Hi Junio,
On Tue, 10 May 2022, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@xxxxxx>
> >
> > By allowing the path to be enclosed in double-quotes, we can avoid
> > the limitation that paths cannot contain colons.
> > ...
> > + struct strbuf buf = STRBUF_INIT;
> > + const char *p = arg;
> > +
> > + if (*p != '"')
> > + p = strchr(p, ':');
> > + else if (unquote_c_style(&buf, p, &p) < 0)
> > + die(_("unclosed quote: '%s'"), arg);
>
> Even though I do not think people necessarily would want to use
> colons in their pathnames (it has problems interoperating with other
> systems), lifting the limitation is a good thing to do. I totally
> forgot that we designed unquote_c_style() to self terminate and
> return the end pointer to the caller so the caller does not have to
> worry, which is very nice.
>
> Even if this step weren't here in the series, I would have thought
> the mode bits issue was more serious than "no colons in path"
> limitation, but given that we address this unusual corner case
> limitation, I would think we should address the hardcoded mode bits
> at the same time.
>
> > diff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh
> > index 8ff1257f1a0..5b8bbfc2692 100755
> > --- a/t/t5003-archive-zip.sh
> > +++ b/t/t5003-archive-zip.sh
> > @@ -207,13 +207,21 @@ check_zip with_untracked
> > check_added with_untracked untracked untracked
> >
> > test_expect_success UNZIP 'git archive --format=zip --add-file-with-content' '
> > + if test_have_prereq FUNNYNAMES
> > + then
> > + QUOTED=quoted:colon
> > + else
> > + QUOTED=quoted
> > + fi &&
>
> ;-)
>
> > git archive --format=zip >with_file_with_content.zip \
> > + --add-file-with-content=\"$QUOTED\": \
> > --add-file-with-content=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 $QUOTED &&
>
> Looks OK, even though it probably is a good idea to have dq around
> $QUOTED, so that future developers can easily insert SP into its
> value to use a bit more common but still a bit more problematic
> pathnames in the test.
I actually decided against this because reading
"$QUOTED"
would mislead future me to think that the double quotes that enclose
$QUOTED are the quotes that the variable's name talks about. But the
quotes are actually the escaped ones that are passed to `git archive`
above.
So, to help future Dscho should they read this code six months from now or
even later, I wanted to specifically only add quotes to the `git archive`
call to make the intention abundantly clear.
Ciao,
Dscho
