Rewrite boot in Python
ClosedPublic

Authored by bgamari on Sep 2 2017, 1:20 PM.

Details

Summary

One step closer to being able to drop the Windows Perl tarball. We previously
attempted to do this in D3567 but were forced to revert due to Windows
problems.

Test Plan

Validate

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bgamari created this revision.Sep 2 2017, 1:20 PM
Phyx edited edge metadata.Sep 4 2017, 2:37 PM

hmm the boot seems to have worked correctly. I'm not sure what failed here...

bgamari updated this revision to Diff 13915.Sep 15 2017, 3:25 PM

Try rebasing

This Windows issue is bizarre. Locally I'm seeing the following failure,

$ ./boot
aclocal-1.15: error: aclocal: file '/msys64/usr/share/aclocal/xsize.m4' does not exist
aclocal-1.15: error: aclocal: file '/msys64/usr/share/aclocal/xsize.m4' does not exist
autoreconf: aclocal failed with exit status: 1
autoreconf: aclocal failed with exit status: 1
autoreconf in libraries/directory/ failed with exit code 1
autoreconf in libraries/terminfo/ failed with exit code 1
Creating libraries/array/ghc.mk
Creating libraries/base/ghc.mk
Creating libraries/binary/ghc.mk
Creating libraries/bytestring/ghc.mk
...

It looks like this is yet another case where the msys python works fine yet mingw64 fails.

bgamari added a comment.EditedSep 19 2017, 10:18 PM

It looks like the problem is that for some reason the mingw64 python executable sets ACLOCAL_PATH=C:\\msys64\\mingw64\\share\\aclocal;C:\\msys64\\usr\\share\\aclocal (presumably due to some code in /etc/profile). While there are a few autoconf-ish files in this directory, xsize.m4 is not one of them. The only copy of xsize.m4 I can find is /usr/share/aclocal/xsize.m4 (which is owned by the gettext package).

It looks like the problem is that the mingw-w64-x86_64-gettext package installs xsize.m4 in /mingw64/share/aclocal/xsize.m4 and not /mingw64/share/aclocal/xsize.m4 like ACLOCAL_PATH would suggest.

@Phyx, does this sound like a packaging issue?

Ahh, actually, this may just be a path compatibility issue,

$ cd ~/ghc/libraries/terminfo
$ ls C:\\msys64\\mingw64\\share\\aclocal\\xsize.m4
'C:\msys64\mingw64\share\aclocal\xsize.m4'
$ ACLOCAL_PATH="C:\\msys64\\mingw64\\share\\aclocal" autoreconf
aclocal-1.15: error: aclocal: file '/msys64/mingw64/share/aclocal/xsize.m4' does not exist
autoreconf: aclocal failed with exit status: 1
$ ACLOCAL_PATH="/mingw64/share/aclocal" autoreconf
$ 

It seems like autoreconf just doesn't support Windows paths in ACLOCAL_PATH.

Hmm, even stranger,

$ echo $ACLOCAL_PATH

$ export MSYSTEM=MINGW64
$ source /etc/profile
$ echo $ACLOCAL_PATH
/mingw64/share/aclocal:/usr/share/aclocal
$ python -c 'import os; print(os.environ["ACLOCAL_PATH"])'
/mingw64/share/aclocal:/usr/share/aclocal
$ python3 -c 'import os; print(os.environ["ACLOCAL_PATH"])'
C:\msys64\mingw64\share\aclocal;C:\msys64\usr\share\aclocal

A strange fellow that python is...

bgamari added a comment.EditedSep 19 2017, 10:36 PM

A dear; it turns out this isn't just Python. This is apparently a feature of the mingw implementation of getenv,

$ cat hi.c
#include <stdlib.h>
#include <stdio.h>

void main() {
        printf("%s\n", getenv("ACLOCAL_PATH"));
}
$ gcc hi.c
$ ./a.exe
C:\msys64\mingw64\share\aclocal;C:\msys64\usr\share\aclocal

Lovely...

Alright, so essentially what is happening is that with this patch we are using the mingw python3, which sees a mangled environment containing Windows paths due to its mingw dependency. This mangled environment is then visible to the autoreconf subprocess, which is an msys executable (namely perl and m4, I believe) and therefore (apparently) can't deal with Windows paths. I'm honestly not sure what to make of this other than to conclude that this refactoring just isn't worth the enormous amount of pain that it has inflicted. Silly Windows.

Phyx added a comment.Sep 21 2017, 2:56 AM

Alright, so essentially what is happening is that with this patch we are using the mingw python3, which sees a mangled environment containing Windows paths due to its mingw dependency. This mangled environment is then visible to the autoreconf subprocess, which is an msys executable (namely perl and m4, I believe) and therefore (apparently) can't deal with Windows paths. I'm honestly not sure what to make of this other than to conclude that this refactoring just isn't worth the enormous amount of pain that it has inflicted. Silly Windows.

Hmm can't this be solved with a bash --no-profile or whatever the option is called to not load the user profile? That should override the path. I think there's also a way to tell it not to inherent the paths from windows but doubts that's needed.

I'll give it a go once I get home.

In D3918#111988, @Phyx wrote:

Alright, so essentially what is happening is that with this patch we are using the mingw python3, which sees a mangled environment containing Windows paths due to its mingw dependency. This mangled environment is then visible to the autoreconf subprocess, which is an msys executable (namely perl and m4, I believe) and therefore (apparently) can't deal with Windows paths. I'm honestly not sure what to make of this other than to conclude that this refactoring just isn't worth the enormous amount of pain that it has inflicted. Silly Windows.

Hmm can't this be solved with a bash --no-profile or whatever the option is called to not load the user profile? That should override the path. I think there's also a way to tell it not to inherent the paths from windows but doubts that's needed.

I'll give it a go once I get home.

Unfortunately --noprofile doesn't seem to help. Indeed even using subprocess.Popen(['env', 'autoreconf'], cwd=dir_) doesn't change the situation.

Phyx added a comment.Sep 21 2017, 2:18 PM

I'll be back behind a computer this weekend and will take a look.

Phyx added a comment.Sep 28 2017, 3:07 PM

@bgamari so this works

diff --git a/boot b/boot
index dac88963fb..03ae6da3ad 100755
--- a/boot
+++ b/boot
@@ -7,6 +7,7 @@ import sys
 import argparse
 from textwrap import dedent
 import subprocess
+import re

 cwd = os.getcwd()

@@ -135,10 +136,20 @@ def boot_pkgs():
 def autoreconf():
     # Run autoreconf on everything that needs it.
     processes = {}
+    if os.name == 'nt':
+        # Get the normalized aclocal_path for Windows
+        ac_local = os.environ['ACLOCAL_PATH']
+        ac_local_arg = re.sub(r';', r':', ac_local)
+        ac_local_arg = re.sub(r'\\', r'/', ac_local_arg)
+        ac_local_arg = re.sub(r'(\w):/', r'/\1/', ac_local_arg)
+        reconf_cmd = 'ACLOCAL_PATH=%s autoreconf' % ac_local_arg
+    else:
+        reconf_cmd = 'autoreconf'
+
     for dir_ in ['.'] + glob.glob('libraries/*/'):
         if os.path.isfile(os.path.join(dir_, 'configure.ac')):
             print("Booting %s" % dir_)
-            processes[dir_] = subprocess.Popen(['sh', '-c', 'autoreconf'], cwd=dir_)
+            processes[dir_] = subprocess.Popen(['sh', '-c', reconf_cmd], cwd=dir_)

     # Wait for all child processes to finish.
     fail = False

It works by quering the current ACLOCAL_PATH and converting the Windows path you received
into a unix path (or close enough that the msys runtime can understand it). the ACLOCAL_PATH is
then set in the invocation, which then overrides the Windows paths the new sh call would have.

Because it's being done by sh which is a msys utility which understand unix paths, it correctly gets
passed down to the child.

Wow, alright. That is terrible but if it works so be it.

Phyx added a comment.Sep 30 2017, 4:09 PM

Haha yeah I wasn't very happy with it either but it seems that the paths
will always be reinterpreted to their windows counterparts. Initially I
tried getting sh to echo the path instead of reading the environment. But
once it hits the output functions it's translated again. And telling it not
to do this is a bigger hack.

The msys2 runtime should also understand windows paths if quoted, but the :
in the drive conflicts with the : for the path separator..

This revision was automatically updated to reflect the committed changes.
sgraf added a subscriber: sgraf.EditedOct 19 2017, 10:15 AM

So... When will the workaround be merged? I think the fact that we need that hack is a shame, but currently it just doesn't work on Windows by default.

Edit: Oh strange, I see this was already merged. I'm still getting an error, though:

Traceback (most recent call last):
  File "./boot", line 184, in <module>
    autoreconf()
  File "./boot", line 143, in autoreconf
    ac_local = os.environ['ACLOCAL_PATH']
  File "C:/Users/Sebastian/AppData/Local/Programs/stack/x86_64-windows/msys2-20150512/mingw64/lib/python3.5\os.py", line 725, in __getitem__
    raise KeyError(key) from None
KeyError: 'ACLOCAL_PATH'

Because ACLOCAL_PATH isn't set at all, obviously.

It seems that doing a lookup with default instead of plain dict lookup is enough:

ac_local = os.environ.get('ACLOCAL_PATH', '')
Phyx added a comment.Oct 19 2017, 10:53 AM
In D3918#114934, @sgraf wrote:

It seems that doing a lookup with default instead of plain dict lookup is enough:

ac_local = os.environ.get('ACLOCAL_PATH', '')

Sure that should get it not to crash, it probably won't work either. If you're missing the environment variable than something is wrong with your aclocal install. Are you using a custom profile that may be clearing it?

In D3918#114935, @Phyx wrote:

Sure that should get it not to crash, it probably won't work either. If you're missing the environment variable than something is wrong with your aclocal install. Are you using a custom profile that may be clearing it?

Hmm... Not that I know of. I'm going through stack exec --no-ghc-package-path bash, as I always do. I upgraded to a 8.2.1 nightly some time ago, but it doesn't work for the 8.0.2 version I used to use, either. Doing bash --noprofile (whatever that entails) doesn't work either.

sgraf added a comment.Oct 20 2017, 2:53 AM

Indeed, this seems to be an issue with stack exec --no-ghc-package-path. I tested on two windows machines. Plain mingw64 shells had $ACLOCAL_PATH set (though as UNIX paths, not as Windows paths), as did Git Bash.

Phyx added a comment.Oct 20 2017, 3:07 AM
In D3918#115049, @sgraf wrote:
Plain mingw64 shells had $ACLOCAL_PATH set (though as UNIX paths, not as Windows paths), as did Git Bash.

it's just the bash shell which is an msys2 utility translating them when it shows you them. If a Win32 application reads them they'll be in Windows format.
the regular expression would convert it only if a windows path, else would just keep the unix path and pass it on as is.

I'm not familiar with how stack starts the shell, but my guess is it's ignoring the profile or being started as a different user or started in isolation mode,
but then no other unix tools path would be set either.

sgraf added a comment.Oct 20 2017, 3:24 AM

I opened an issue on stacks tracker: https://github.com/commercialhaskell/stack/issues/3501

Meanwhile, I can just ./boot from plain mingw64.

I just tried to make FAST=YES in libraries/base and found that the makefile we used to have to support this is no longer there. This diff seems to be the culprit. I can work around it for now, but FYI this was useful!

I just tried to make FAST=YES in libraries/base and found that the makefile we used to have to support this is no longer there. This diff seems to be the culprit. I can work around it for now, but FYI this was useful!

See D4432.