git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: git@vger.kernel.org, git-packagers@googlegroups.com,
	git-for-windows@googlegroups.com
Subject: Re: [ANNOUNCE] Git v2.16.0-rc1
Date: Tue, 09 Jan 2018 15:55:17 -0800	[thread overview]
Message-ID: <xmqq608a37ve.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <xmqqy3l63dzy.fsf@gitster.mtv.corp.google.com> (Junio C. Hamano's message of "Tue, 09 Jan 2018 13:42:57 -0800")

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

> 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/




  reply	other threads:[~2018-01-09 23:55 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-05 23:30 [ANNOUNCE] Git v2.16.0-rc1 Junio C Hamano
2018-01-06 12:59 ` Johannes Schindelin
2018-01-06 22:01   ` Johannes Schindelin
2018-01-09 21:42     ` Junio C Hamano
2018-01-09 23:55       ` Junio C Hamano [this message]
2018-01-10 17:40         ` Johannes Schindelin
2018-01-10 20:09           ` Junio C Hamano
2018-01-10 17:37       ` Johannes Schindelin
2018-01-10 17:54         ` [git-for-windows] " Johannes Sixt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq608a37ve.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git-for-windows@googlegroups.com \
    --cc=git-packagers@googlegroups.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).