Re: [PATCH 1/1] exec_cmd: RUNTIME_PREFIX on some POSIX systems

On Sun, Nov 19 2017, Dan Jacques jotted:

Firstly the promise of this is very neat. I'm happy to offer any help I can give.

If you run run:

    make -j8 prefix=/tmp/git RUNTIME_PREFIX=YesPlease RUNTIME_PREFIX_PERL= CFLAGS="-O0 -g" all install

And then:

    make -j8 prefix=/tmp/git RUNTIME_PREFIX=YesPlease RUNTIME_PREFIX_PERL=YesPlease CFLAGS="-O0 -g" all install

You end up with this:

    $ tree /tmp/git/{lib,share/perl}
    /tmp/git/lib
    └── x86_64-linux-gnu
        └── perl
            └── 5.26.1
                ├── auto
                │   └── Git
                └── perllocal.pod
    /tmp/git/share/perl
    └── 5.26.1
        [...]
        └── Git.pm

You need to bust the perl/PM.stamp cache as a function of your RUNTIME_PREFIX_PERL variable (or NO_PERL_MAKEMAKER).

Other than that, is this whole NO_PERL_MAKEMAKER workaround just because you couldn't figure out what the target RELPERLPATH is in $prefix/$RELPERLPATH, which in this case is share/perl/5.26.1 ?

I don't remember offhand how to extract that, but htis is built into perl itself, see e.g.:

    $ PERL5LIB= /usr/bin/perl -E 'say for grep { m[share|lib] } @INC'
    /usr/local/lib/x86_64-linux-gnu/perl/5.26.1
    /usr/local/share/perl/5.26.1
    /usr/lib/x86_64-linux-gnu/perl5/5.26
    /usr/share/perl5
    /usr/lib/x86_64-linux-gnu/perl/5.26
    /usr/share/perl/5.26
    /usr/local/lib/site_perl
    /usr/lib/x86_64-linux-gnu/perl-base

So it's in Config.pm somewhere IIRC. But your patch doesn't discuss why you toggled NO_PERL_MAKEMAKER, is it purely to work around this issue, and if we had some aesy way to figure out the target $RELPERLPATH we wouldn't need to do that?

Seems a bit of baby & bathwater there :)

Aside from that:

1. The regex match you're doing to munge the dir could be done as a
   catdir($orig, '..', '..', 'lib'), that doesn't work as discussed
   above, but *might* be more portable. I say might because I don't know
   if the path string is always normalized to be unix-like, but if not
   this won't work e.g on Windows where it'll have \ not /.

2. You are 'use'-ing FindBin there unconditionally (use is not function
   local in perl), and implicitly assuming it loads File::Spec.

   Ignoring the NO_PERL_MAKEMAKER caveats above I'd suggest something
   like this:

       #!/usr/bin/perl

       # BEGIN RUNTIME_PREFIX_PERL=YesPlease generated code.
       #
       # This finds our Git::* libraries at relative locations.
       BEGIN {
           use lib split /:/,
           (
               $ENV{GITPERLLIB}
               ||
               do {
                   require FindBin;
                   require File::Spec;
                   File::Spec->catdir($FindBin::Bin, '..', '..', 'lib');
               }
           );
       }
       # END RUNTIME_PREFIX_PERL=YesPlease generated code.

       # Copyright (C) 2006, Eric Wong <normalperson@yhbt.net>
       # License: GPL v2 or later

   It's also nice to have some whitespace / comments to note that this
   is generated code.

 3. I may be squinting at this wrong but it seems to me that between
    your v1 and v2 reading GITPERLLIB here no longer makes any sense at
    all. You used to set it in git itself, now it takes priority but
    nothing sets it, presumably you'd have some external wrapper script
    that'll set it?

    Now if I compile with RUNTIME_PREFIX=YesPlease I get magic
    auto-discovery of C program paths, right? But it'll still fallback
    to the system perl libs (if any) unless
    RUNTIME_PREFIX_PERL=YesPlease is set. Shouldn't we just make
    RUNTIME_PREFIX=YesPlease Just Work for everything?