Re: [PATCH 0/6] Create Git/Packet.pm

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Junio,

On Thu, 26 Oct 2017, Junio C Hamano wrote:

> 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 not only dirty, it *still* does not work with that workaround: Note
that GITPERLLIB is *also* set in the bin-wrappers/*...

And even then, it does not work: somewhere along the way, the path is
converted to a Windows path with backslashes, and for some reason MSYS2
Perl fails to handle that.

The best workaround I found in Git's source code (yes, it took me 2h to
investigate this littly problem, but at least I got an in-depth
understanding of the issue) was to pass BLIB_DIR instead, and not perform
a split but generate the two paths to include in Perl instead. Of course,
that would break out-of-tree usage of GITPERLLIB...

That's why I settled on the out-of-tree workaround that Windows
contributors will have to somehow figure out (as if it was not hard enough
already to contribute to Git for Windows...).

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

No, the test suite has plenty of use cases for that. It usually works.

The problem is that t0021 contains very complex code that goes back and
forth between the C layer and the scripted layer. At one stage, the
pseudo-Unix paths are converted to Windows paths, with drive prefix and
backslashes, separated by semicolons. And somewhere along the lines, this
cannot be converted back.

I *think* that it happens when the bin-wrapper for git.exe is executed
from inside Git itself, or some such.

> 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").

We go against best practices by having crucial parts of Git implemented as
Perl scripts. But you knew that ;-)

Ciao,
Dscho