Re: [PATCH v1 22/45] archive: convert to use parse_pathspec

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Duy Nguyen <pclouds@xxxxxxxxx> writes:

> On Sat, Mar 16, 2013 at 12:56 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Nguyễn Thái Ngọc Duy  <pclouds@xxxxxxxxx> writes:
>>
>>> @@ -232,11 +228,18 @@ static int path_exists(struct tree *tree, const char *path)
>>>  static void parse_pathspec_arg(const char **pathspec,
>>>               struct archiver_args *ar_args)
>>>  {
>>> -     ar_args->pathspec = pathspec = get_pathspec("", pathspec);
>>> +     /*
>>> +      * must be consistent with parse_pathspec in path_exists()
>>> +      * Also if pathspec patterns are dependent, we're in big
>>> +      * trouble as we test each one separately
>>> +      */
>>> +     parse_pathspec(&ar_args->pathspec, 0,
>>> +                    PATHSPEC_PREFER_FULL,
>>> +                    "", pathspec);
>>>       if (pathspec) {
>>>               while (*pathspec) {
>>>                       if (!path_exists(ar_args->tree, *pathspec))
>>> -                             die("path not found: %s", *pathspec);
>>> +                             die(_("pathspec '%s' did not match any files"), *pathspec);
>>>                       pathspec++;
>>>               }
>>
>> You do not use ar_args->pathspec even though you used parse_pathspec()
>> to grok it?  What's the point of this change?
>
> parse_pathspec() here is needed because write_archive_entries needs it
> later.

That is not the issue I was pointing out.  Even though you parse the
pathspec into args->pathspec, the "if() { while () {} }" here still
uses strings contained in **pathspec, as if they are literal strings
and not ":(glob)Documentation" and such, and will not match the named
directory.

Technically, erroring out saying "':(glob)Documentation' does not exist
as a path in the tree" is correct, but it would be nicer to have the
code inspect parse_pathspec() result and independently barf, saying
"this command does not support magic pathspecs, give me leading paths
and nothing else", until we do support magic pathspecs, no?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]