Re: [ANNOUNCE] Git v2.16.0-rc1

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 9, 2018, 23:55 UTC
Message-ID: <xmqq608a37ve.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqqy3l63dzy.fsf@gitster.mtv.corp.google.com>

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

Show 33 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> diff --git a/t/t0021/rot13-filter.pl b/t/t0021/rot13-filter.pl
>> index f1678851de9..470107248eb 100644
>> --- a/t/t0021/rot13-filter.pl
>> +++ b/t/t0021/rot13-filter.pl
>> @@ -31,7 +31,22 @@
>>  #
>>  
>>  use 5.008;
>> -use lib (split(/:/, $ENV{GITPERLLIB}));
>> +sub gitperllib {
>> +...
>> +	if ($ENV{GITPERLLIB} =~ /;/) {
>> +		return split(/;/, $ENV{GITPERLLIB});
>> +	}
>> +	return split(/:/, $ENV{GITPERLLIB});
>> +}
>
> This cannot be the whole story for a few reasons.
>
>  - In t/test-lib.sh we see this:
>
>    GITPERLLIB="$GIT_BUILD_DIR"/perl/blib/lib:"$GIT_BUILD_DIR"/perl/blib/arch/auto/Git
>    export GITPERLLIB
>
>    If this part wants to split with ';', then the joining needs to
>    be done with ';' to match, no?
>
>  - In addition to t0021, there are similar split with colon in 0202,
>    9000 and 9700, yet I am getting the feeling that you observed the
>    issue only in0021, to which I do not think of a good explanation
>    why.

This somehow vaguely rang a bell, and I dug this thing up from the archive, [*1*] which ended like so:

    >> 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.
    >>
    > 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?

    I was testing this recently on the Perl included with Git for
    Windows and it returns : for the path separator even though it's
    on Windows, so I don't think that would work. The Perl in Git
    for Windows seems to want UNIX-style inputs (something Dscho
    seemed to allude to in his response earlier.). I'm not sure why
    it's that way, but he probably knows.

Your initial response in this thread made it sound as if -rc1 is the only thing that changed, but looking at the differences between -rc0 and -rc1, which does not touch t0021 or any other instances of "split(/:/, $ENV{GITPERLLIB})", I am wondering if it is possible that perhaps the way Perl is built for GfW has been changed recently and we can safely and sanely use $Config::Config{path_sep} (contrary to what was found in late Oct in the message quoted above) now?

In any case, I'd prefer this issue to be resolved properly before -rc2; a patch to t0021/rot13-filter.pl alone does not smell like a "proper solution" that is based on the understanding of the root cause (and that is why I spent time digging the list archive).

Thanks.

[Reference]

*1* https://public-inbox.org/git/CAGyf7-EjKaHgwkN9trO4mFvba9odbWCzA9Jh0Pk6ZE6FOskOYg@mail.gmail.com/