Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Windows] Specify full path to pl2bat.bat #459

Open
shawnlaffan opened this issue Aug 16, 2024 · 13 comments
Open

[Windows] Specify full path to pl2bat.bat #459

shawnlaffan opened this issue Aug 16, 2024 · 13 comments

Comments

@shawnlaffan
Copy link

See StrawberryPerl/Perl-Dist-Strawberry#174 for further discussion (mainly StrawberryPerl/Perl-Dist-Strawberry#174 (comment) and StrawberryPerl/Perl-Dist-Strawberry#174 (comment)).

The fixin code in ExtUtils::MM_Win32 does not specify the full path to the file. This generally works but things can be tripped up when running with nested MSYS2 and Windows shells, where MSYS2 style paths do not work when passed to Windows shells (e.g. /c/somepath/bin/pl2bat.bat is treated as C:\c\somepath\bin\pl2bat.bat). In such cases the make utility throws an error and the build stops.

One way to avoid this is to specify the full path to pl2bat in ExtUtils::MM_Win32 when creating the Makefile.

$self->{FIXIN} ||= $self->{PERL_CORE} ?
"\$(PERLRUN) -I$self->{PERL_SRC}\\cpan\\ExtUtils-PL2Bat\\lib $self->{PERL_SRC}\\win32\\bin\\pl2bat.pl" :
'pl2bat.bat';

This would then match all the other utilities which are specified using their full paths.

If this is deemed useful then I can prep a PR.

CC @mohawk2

@Leont
Copy link
Member

Leont commented Aug 16, 2024

One way to avoid this is to specify the full path to pl2bat in ExtUtils::MM_Win32 when creating the Makefile.

How would one know the full path to pl2bat? I suspect that's tricker to get right than what one may expect.

@shawnlaffan
Copy link
Author

How would one know the full path to pl2bat? I suspect that's tricker to get right than what one may expect.

The default location would be next to the perl executable, so it would be something like (my $f = $^X) =~ s/perl.exe/pl2bat.bat/.

If that does not exist then fall back to the bare name and let the system find it in the path.

@Leont
Copy link
Member

Leont commented Aug 18, 2024

@sisyphus do you think the above would work?

@sisyphus
Copy link
Contributor

@sisyphus do you think the above would work?

I don't even know where to start thinking about this.
I can say that, in my own builds of Windows perls, pl2bat.bat and perl.exe are not in the same directory.
My pl2bat.bat (of which there is only one) is in perl-5.40.0\bin, but because my builds of perl are "architectured", the various perl.exe files will be in perl-5.40.0\bin\MSWin32-x64-multi-thread, perl-5.40.0\bin\MSWin32-x64-multi-thread-ld, perl-5.40.0\bin\MSWin32-x64-multi-thread-quadmath, perl-5.40.0\bin\MSWin32-x86-multi-thread, perl-5.40.0\bin\MSWin32-x86-multi-thread-64int, etc.
This structuring I use is a long supported one ... not some unofficial one I've just made up for myself.
I could, of course, always adapt and do things differently if I had to.
However, there could be (unlikely ?) others building perls with the same "arhitectured" structure.
With Strawberry Perl, of course, pl2bat.bat and perl.exe are always installed alongside each other.

To "fall back to the bare name and let the system find it in the path" could also be rather interesting for me during a make test or make install stage.
My builds are conducted by running a perl script (in an old perl-5.16 build of ActiveState Perl), so it's quite likely that the ActiveState-5.16 "pl2bat.bat" would get found.
Yes ... I know that this approach is insane ... but it has worked fine for me for about the last 6 years ... and "perl" is the only scripting language with which I am comfortable.

Irrespective of the crazy way I might do things, I think it's a good defensive programming technique to be specifying a (correct) fully qualified path to pl2bat.bat, rather than just letting $ENV{PATH} locate it.

Anyway ... I'm happy to test any PRs that are presented..
But I really don't get it ... are we aiming to be able to run Strawberry Perl in the MSYS shell ?
If so, why ?
If not, why are we accepting the intrusion of an MSYS path as something we need to work around or accommodate ?

Sorry - I reckon I'm probably missing the point(s) here. I admit that I haven't really done much in the way of trying to get my head around it.

@shawnlaffan
Copy link
Author

So the system on Windows is more complex than I assumed. That said, "falling back to the path" is leaving things as they are currently, for which FIXIN = pl2bat.bat.

If there is an easy way to locate the pl2bat.bat for the current perl then I think it would be good to set the full path in the generated Makefile. Is there a File::Which analogue available to ExtUtils::MakeMaker? Or is there already an EU::MM or Config variable that points to the relevant location? Maybe something that checks for pl2bat.bat one directory up from the dirname of $^X if it ends with $Config{archname} would work.

Another option is to document/clarify how to set the pl2bat path in a Makefile.PL. The assignment in MM_Win32.pm uses ||=, which implies it can be set before that code is run. However, it was not clear to me how to do that and my limited experiments did not work.

WRT the use of MSYS2 to run Strawberry Perl, it's the CI setup for PDL. Details are best provided by @mohawk2.

@sisyphus
Copy link
Contributor

sisyphus commented Aug 20, 2024

That said, "falling back to the path" is leaving things as they are currently, for which FIXIN = pl2bat.bat.

Yes - shortly after I posted it occurred to me that if any part of my perl builds were to execute pl2bat.bat, then it's probably the AS perl 5.16 pl2bat that's being executed.
If that's happening, then it's apparently of no matter ... and, fingers-crossed, I hope it stays that way ;-)

WRT the use of MSYS2 to run Strawberry Perl, it's the CI setup for PDL.

Having (temporarily) set up MSYS2 shell to run StrawberryPerl-5.40..0.1-PDL, I've built and tested PDL-2.082.
That worked fine for me.
I noticed that where the failing command (as reported by @mohawk2) references "pptemplate", in my build it was "pdl2" that was being referenced - and the 2 scripts are different.
But I don't immediately see how that could explain the CI build's incapacity to locate Strawberry's pl2bat.bat.

For reference, that failing command in the CI build was:

"C:\hostedtoolcache\windows\strawberry-perl\5.38.2\x64\perl\bin\perl.exe" -MExtUtils::Command -e cp -- pptemplate ..\..\blib\script\pptemplate pl2bat.bat ..\..\blib\script\pptemplate
Can't execute /c/hostedtoolcache/windows/strawberry-perl/5.38.2/x64/perl/bin/pl2bat.bat.

UPDATE
The command from my build that I thought was comparable to the failing CI command comes from (I think) Perldl2/Makefile, and it appears during make test.
The failing CI command comes from (I think) Basic/Gen/Makefile, and I gather it happens during make install.
/UPDATE

Is there something wrong with the setup of the CI environment ?
Or is there something wrong with my attempt to reproduce the failure that it's experiencing ?

UPDATE 2
I've now built, tested and installed PDL-2.089 twice (once from a manual build, and then using cpanm) in this MSYS2 shell using SP-5.40.0-PDL. without any problem at all.
I wonder if the CI environment has Strawberry's perl/bin folder && Strawberry's c/bin folder at the very beginning of $PATH, before everything else.
That might be crucial.
/UPDATE 2

@shawnlaffan
Copy link
Author

The trigger for the issue is the CI setup with what appear to be several levels of different shells.

It still remains useful for EUMM to give the full path to pl2bat.bat in cases where it can be easily located. That way it is consistent with the other paths in the generated makefiles.

@sisyphus
Copy link
Contributor

It still remains useful for EUMM to give the full path to pl2bat.bat in cases where it can be easily located.

To that end, I believe pl2bat.bat will always be installed in the directory specified by $Config{installbin}.
At least, that currently provides the correct location on my "architectured" perl installations, and also on Strawberry Perl.

So I think your suggestion of assigning "$Config{installbin}/pl2bat.bat" (iff it exists) with the fallback to plain "pl2bat.bat" is worth testing .... if SP wants to take it upon itself to solve a problem that is not of SP's (or perl's) making.

I don't know whether it's going to fix the problem with these CI tests, but it might.
And I can't see that it's going to be useful to anyone else ... though it might ... and it shouldn't cause anyone any angst.

If you can provide the simple patch before 5.41.3 is released, I'll incorporate into my 5.41.3 builds (which will be fairly extensive).
Of course, all I can establish is that it doesn't break anything for me.

@sisyphus
Copy link
Contributor

I believe pl2bat.bat will always be installed in the directory specified by $Config{installbin}

No ... that's the one that houses the perl.exe.
pl2bat.pl will go into $Config{installscript}.

Duh )-:

@Leont
Copy link
Member

Leont commented Aug 20, 2024

No ... that's the one that houses the perl.exe.
pl2bat.pl will go into $Config{installscript}.

I knew perl's configuration made this distinction, but I didn't know any platform actually used it. Good to know.

@shawnlaffan
Copy link
Author

I've added a commit to a forked repo. This shows no issues when building and testing PDL. That does not say much about the original problem as I have not been able to replicate it, but it does show that it builds a complex system.

shawnlaffan@48a452f

I have not opened a PR as the ternary is too repetitive for my liking (full path repeated twice). A less minimal change would convert the ||= and ternary to an if block. (I could open a draft PR but this approach has less by way of git resets and force pushes).

@sisyphus
Copy link
Contributor

I've added a commit to a forked repo. This shows no issues when building and testing PDL.

Looks good.
Having made the same change to my SP-5.40.0.1-PDL MM_Win32.pm, I re-built (and re-installed) PDL-2.089 in the MSYS2 shell.
All occurrences of "pl2bat.bat" became "C:\sp_64\sp-5.40.0.1-PDL\perl\bin\pl2bat.bat" as expected, and that all worked fine.

Hopefully, the problematic CI environment will also find the change to be acceptable.

I'll leave that change to MM_Win32.pm as a permanent alteration to my 5P-5.40.0.1-PDL installation, and do likewise to my own 5.40.0 builds of perl.

I wonder if the CI environment will accept the backslashes. (My MSYS2 environment clearly has no issue with them while building PDL.)
I think there's a chance the CI environment will also accept them inside the PDL build - despite the following, which (I think ??) proves nothing:

Owner@DESKTOP-88J497T MINGW64 /d/s/PDL-2.089
$ C:\sp\_64\sp-5.40.0.1-PDL\perl\bin\pl2bat.bat
-bash: C:sp_64sp-5.40.0.1-PDLperlbinpl2bat.bat: command not found

Owner@DESKTOP-88J497T MINGW64 /d/s/PDL-2.089
$ C:/sp/_64/sp-5.40.0.1-PDL/perl/bin/pl2bat.bat

Owner@DESKTOP-88J497T MINGW64 /d/s/PDL-2.089

You could change them to forward slashes if you want. AFAIK perl will still understand them, irrespective of the shell in which we're running.
Or just leave it as is, and wait and see.

@shawnlaffan
Copy link
Author

MSYS2 can handle Windows paths so long as they are quoted or escaped.

 'C:\perls\5.40.0.1_PDL\perl\bin\perl' -v | head -2 | tail -1
This is perl 5, version 40, subversion 0 (v5.40.0) built for MSWin32-x64-multi-thread

WRT using a forward slash, I think older systems don't handle this? I'm not sure if such systems are supported by EUMM, though.

In any case I'll open a PR later today (AEST). I'll add a second commit to neaten up the code to avoid repetition in the ternary. It can always be elided if deemed unnecessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants