Re: [PATCH 0/6] Create Git/Packet.pm
- From
Jacob Keller <jacob.keller@gmail.com>
- Date
- Oct 26, 2017, 09:07 UTC
- Message-ID
- <CA+P7+xpcC7Daw5k3=XFyZRsqiBib6yY2YzLpjYfcS7yqfUzTpg@mail.gmail.com>
- In-Reply-To
- <xmqq4lqmfoy7.fsf@gitster.mtv.corp.google.com>
On Wed, Oct 25, 2017 at 10:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 30 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> Note that the correct blib path starts with `C:\BuildAgent\_work` and
>> the line
>>
>> use lib (split(/:/, $ENV{GITPERLLIB}));
>>
>> splits off the drive letter from the rest of the path. Obviously, this
>> fails to Do The Right Thing, and simply points to Yet Another Portability
>> Problem with Git's reliance on Unix scripting.
>
> In our C code, we have "#define PATH_SEP ';'", and encourage our
> code to be careful and use it. Is there something similar for Perl
> scripts, I wonder.
>
> I notice that t/{t0202,t9000,t9700}/test.pl share the same
> split(/:/, $ENV{GITPERLLIB}); forcing this specific variable to use
> the non-platform convention to accomodate the use of split(/:/)
> certainly is a workaround, but it does feel dirty.
>
> It is hard to imagine that we were the first people who wants to
> split the value of a variable into a list, where the value is a list
> of paths, concatenated into a single string with a delimiter that
> may be platform specific. I wonder if we are going against a best
> practice established in the Perl world, simply because we don't know
> about it (i.e. basically, it would say "don't split at a colon
> because not all world is Unix; use $this_module instead", similar to
> "don't split at a slash, use File::Spec instead to extract path
> components").
>I thought there was a way to do this in File::Spec, but that's only for splitting regular paths, and not for splitting a list of paths separated by ":" or ";"
We probably should find a better solution to allow this to work with windows style paths...? I know that python provides os.pathsep, but I haven't seen an equivalent for perl yet.
The Env[1] core modules suggests using $Config::Config{path_sep}[2].. maybe we should be using this?
Thanks, Jake
[1] https://perldoc.perl.org/Env.html [2] https://perldoc.perl.org/Config.html