RE: [PATCH v4 2/7] archive --add-file-with-contents: allow paths containing colons
- From
rsbecker@nexbridge.com <rsbecker@nexbridge.com>
- Date
- May 10, 2022, 22:23 UTC
- Message-ID
- <03d801d864bc$85fe62a0$91fb27e0$@nexbridge.com>
- In-Reply-To
- <xmqqmtfp6ohc.fsf@gitster.g>
On May 10, 2022 5:57 PM, Junio C Hamano wrote:
Show 57 quoted lines
>"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
>writes:
>
>> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>>
>> 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.A test case for .gitignore in this would be good too. People on our exotic platform do this stuff as a matter of course. As an example, a name of $Z3P4:12399334 being used as a named pipe (associated with the unique name of a process) actually has been seen in the wild recently. My solution was to wild card this and/or contain it in an ignored directory. Regards, Randall