snowleopard / hadrian

Hadrian: a new build system for the Glasgow Haskell Compiler. Now merged into the GHC tree!
https://gitlab.haskell.org/ghc/ghc/tree/master/hadrian
MIT License
208 stars 37 forks source link

Fix Windows build without --enable-distro-toolchain #564

Closed snowleopard closed 6 years ago

snowleopard commented 6 years ago

UPDATE: I fixed the issue below by passing --enable-distro-toolchain to configure: #565. We need to make it possible to build Hadrian without --enable-distro-toolchain, but this has lower priority.


Now hitting this error on Windows:

/---------------------------------------------------------------------------------\
| Successfully built program 'ghc-pkg' (Stage0).                                  |
| Executable: _build/stage0/bin/ghc-pkg.exe                                       |
| Program synopsis: A utility for querying and managing the GHC package database. |
\---------------------------------------------------------------------------------/
ghc.exe: could not execute: C:\msys\home\nam83\ghc\_build\stage0\lib/../mingw/bin/gcc.exe
ghc.exe: could not execute: C:\msys\home\nam83\ghc\_build\stage0\lib/../mingw/bin/gcc.exe
shakeArgsWith     0.001s    0%
Function shake   13.529s    0%
Database read     0.000s    0%
With database     0.000s    0%
Running rules  1780.870s   99%  =========================
Total          1794.401s  100%
Error when running Shake build system:
* _build/stage1/lib/package.conf.d/rts-1.0.conf
* OracleQ (PackageDataFile (Context {stage = Stage1, package = Package {pkgLanguage = C, pkgType = Library, pkgName = "rts", pkgPath = "rts"}, way = v}))
* _build/stage1/rts/setup-config
ExitFailure 1
snowleopard commented 6 years ago

The error is thrown from here:

https://github.com/snowleopard/hadrian/blob/master/src/Hadrian/Haskell/Cabal/Parse.hs#L158

@alpmestan @angerman Any ideas how to fix this? Somehow, the path to gcc is wrong on Windows and is equal to C:\msys\home\nam83\ghc\_build\stage0\lib/../mingw/bin/gcc.exe on my machine.

Here is the list of arguments passed to defaultMainWithHooksNoReadArgs:

argList = ["configure",
"--cabal-file", "rts/rts.cabal",
"--distdir", "C:/msys/home/nam83/ghc/_build/stage1/rts",
"--ipid", "$pkg-$version",
"--prefix", "${pkgroot}/..",
"--with-ghc=C:/msys/home/nam83/ghc/_build/stage0/bin/ghc.exe",
"--with-ghc-pkg=C:/msys/home/nam83/ghc/_build/stage0/bin/ghc-pkg.exe",
"--ghc-pkg-option=--global-package-db=C:/msys/home/nam83/ghc/_build/stage1/lib/package.conf.d",
"--enable-library-vanilla",
"--enable-library-for-ghci",
"--disable-library-profiling",
"--disable-shared",
"--configure-option=CFLAGS=-fno-stack-protector -IC:/msys/home/nam83/ghc/_build/generated -IC:/msys/home/nam83/ghc/rts -IC:/msys/home/nam83/ghc/includes",
"--configure-option=CPPFLAGS=-I_build/generated",
"--gcc-options=-fno-stack-protector -IC:/msys/home/nam83/ghc/_build/generated -IC:/msys/home/nam83/ghc/rts -IC:/msys/home/nam83/ghc/includes",
"--configure-option=--with-cc=C:/msys/home/nam83/ghc/inplace/mingw/bin/gcc.exe",
"--ghc-option=-ghcversion-file=C:/msys/home/nam83/ghc/_build/generated/ghcversion.h",
"--with-gcc=C:/msys/home/nam83/ghc/inplace/mingw/bin/gcc.exe",
"--with-ld=C:/msys/home/nam83/ghc/inplace/mingw/bin/ld.exe",
"--with-ar=C:/msys/home/nam83/ghc/inplace/mingw/bin/ar.exe",
"--with-alex=C:/Users/nam83/AppData/Roaming/stack/snapshots/180ba486/bin/alex.exe",
"--with-happy=C:/msys/usr/local/bin/happy.exe",
"-v0",
"--configure-option=--quiet",
"--configure-option=--disable-option-checking"]
alpmestan commented 6 years ago

@snowleopard Hmmm... I'm really confused. Given those 2 options from argList:

[...]
"--configure-option=--with-cc=C:/msys/home/nam83/ghc/inplace/mingw/bin/gcc.exe",
[...]
"--with-gcc=C:/msys/home/nam83/ghc/inplace/mingw/bin/gcc.exe",
[...]

which seem just fine... why/how do we end up with C:\msys\home\nam83\ghc\_build\stage0\lib/../mingw/bin/gcc.exe ?

Can you confirm that the path to gcc that we end up mentionning twice in argList is at least correct?

snowleopard commented 6 years ago

@alpmestan Yes, the path in argList is correct. I have no idea where the incorrect one comes from :(

Could this be responsible: "--prefix", "${pkgroot}/.."? It has got a .. in it...

alpmestan commented 6 years ago

Hmm, maybe? Sounds a bit odd though, why would the install path (--prefix) get mixed with the path to the C compiler, with the latter being truncated to its last 3 path components? I'm quite puzzled. Do we get any more useful information if you add "-v3" somewhere in the call to Cabal ?

snowleopard commented 6 years ago

@alpmestan Thanks for the suggestion to try -v3! I think this helps to understand the problem. Here are two final lines of the verbose output:

"D:/msys/home/nam83/ghc/_build/stage0/bin/ghc.exe" "-hide-all-packages" "-c" "C:\Users\nam83\AppData\Local\Temp\7172-0.c" "-o" "C:\Users\nam83\AppData\Local\Temp\7172-1.o" "-ghcversion-file=D:/msys/home/nam83/ghc/_build/generated/ghcversion.h"
ghc.exe: could not execute: D:\msys\home\nam83\ghc\_build\stage0\lib/../mingw/bin/gcc.exe

As you can see we are calling Stage1 GHC that we built, and I think it's got an incorrect built-it path to GCC. So, the real issue is that Stage1 GHC is broken -- it cannot compile C files.

Any ideas how to fix this?

snowleopard commented 6 years ago

Just confirmed:

D:\msys\home\nam83\ghc\_build\stage0\bin>ghc.exe -c 1.c
ghc.exe: could not execute: D:\msys\home\nam83\ghc\_build\stage0\lib/../mingw/bin/gcc.exe
alpmestan commented 6 years ago

@snowleopard Could you share the contents of your hadrian/cfg/system.cfg (to see what ./configure filled the template with) ?

alpmestan commented 6 years ago

And maybe the output of _build\stage0\bin\ghc --info while you're at it?

snowleopard commented 6 years ago

@alpmestan Sure!

Configuration:

# This file is processed by the configure script.
# See hadrian/src/UserSettings.hs for user-defined settings.
#===========================================================

# Paths to builders:
#===================

alex           = /c/Users/nam83/AppData/Roaming/stack/snapshots/180ba486/bin/alex
ar             = D:/msys/home/nam83/ghc/inplace/mingw/bin/ar.exe
autoreconf     = autoreconf
cc             = D:/msys/home/nam83/ghc/inplace/mingw/bin/gcc.exe
happy          = /c/Users/nam83/AppData/Roaming/local/bin/happy
hs-cpp         = D:/msys/home/nam83/ghc/inplace/mingw/bin/gcc.exe
ld             = D:/msys/home/nam83/ghc/inplace/mingw/bin/ld.exe
make           = make
nm             = D:/msys/home/nam83/ghc/inplace/mingw/bin/nm.exe
objdump        = D:/msys/home/nam83/ghc/inplace/mingw/bin/objdump.exe
ranlib         = D:/msys/home/nam83/ghc/inplace/mingw/bin/ranlib.exe
sphinx-build   = 
system-ar      = C:/Users/nam83/AppData/Local/Programs/stack/x86_64-windows/ghc-8.2.2/lib/../mingw/bin/ar.exe
system-cc      = C:/Users/nam83/AppData/Local/Programs/stack/x86_64-windows/ghc-8.2.2/lib/../mingw/bin/gcc.exe
system-ghc     = c:/Users/nam83/AppData/Local/Programs/stack/x86_64-windows/ghc-8.2.2/bin/ghc
system-ghc-pkg = c:/Users/nam83/AppData/Local/Programs/stack/x86_64-windows/ghc-8.2.2/bin/ghc-pkg
tar            = /usr/bin/tar
patch          = /usr/bin/patch
perl           = D:/msys/home/nam83/ghc/inplace/perl/perl
ln-s           = cp -pR
xelatex        = /d/TeX/MiKTeX/texmf/miktex/bin/xelatex

# Python 3 is required to run test driver.
# See: https://github.com/ghc/ghc/blob/master/testsuite/mk/boilerplate.mk#L220
python         = python3

# Information about builders:
#============================

ar-supports-at-file = YES
cc-clang-backend    = 0
cc-llvm-backend     = 0
gcc-is-clang        = 
hs-cpp-args         = -E -undef -traditional

# Build options:
#===============

solaris-broken-shld  = NO
split-objects-broken = NO
ghc-unregisterised   = NO
ghc-source-path      = D:/msys/home/nam83/ghc
leading-underscore   = NO

# Information about build, host and target systems:
#==================================================

build-platform        = x86_64-unknown-mingw32
build-arch            = x86_64
build-os              = mingw32
build-vendor          = unknown

host-platform         = x86_64-unknown-mingw32
host-arch             = x86_64
host-os               = mingw32
host-vendor           = unknown

target-platform       = x86_64-unknown-mingw32
target-platform-full  = x86_64-unknown-mingw32
target-arch           = x86_64
target-os             = mingw32
target-vendor         = unknown
llvm-target           = x86_64-unknown-windows

cross-compiling       = NO

dynamic-extension     = .dll

ghc-version           = 8.2.2
ghc-major-version     = 8
ghc-minor-version     = 2
ghc-patch-level       = 2

supports-this-unit-id = @SUPPORTS_THIS_UNIT_ID@

project-name          = The Glorious Glasgow Haskell Compilation System
project-version       = 8.5.20180410
project-version-int   = 805
project-patch-level   = 20180410
project-patch-level1  = 20180410
project-patch-level2  = 
project-git-commit-id = b2eb9addcc42f6b9149015436150cd01297346b5

# Compilation and linking flags:
#===============================

conf-cc-args-stage0         =  -fno-stack-protector
conf-cc-args-stage1         =  -fno-stack-protector
conf-cc-args-stage2         =  -fno-stack-protector

conf-cpp-args-stage0        = 
conf-cpp-args-stage1        = 
conf-cpp-args-stage2        = 

conf-gcc-linker-args-stage0 = 
conf-gcc-linker-args-stage1 =  
conf-gcc-linker-args-stage2 =  

conf-ld-linker-args-stage0  = 
conf-ld-linker-args-stage1  = 
conf-ld-linker-args-stage2  = 

# Include and library directories:
#=================================

curses-lib-dir    = 

iconv-include-dir = 
iconv-lib-dir     = 

gmp-include-dir   = 
gmp-lib-dir       = 

use-system-ffi    = NO
ffi-include-dir   = 
ffi-lib-dir       = 

# Optional Dependencies:
#=======================

with-libdw = NO
have-lib-mingw-ex = YES

# Installation:
#=======================

install-prefix          = /usr/local
install-bindir          = /usr/local/bin
install-libdir          = /usr/local/lib
install-datarootdir     = /usr/local/share

install         = /usr/bin/install -c
install-program = /usr/bin/install -c -m 755
install-script  = /usr/bin/install -c -m 755
install-data    = /usr/bin/install -c -m 644
install-dir     = /usr/bin/install -c -m 755 -d

GHC info:

 [("Project name","The Glorious Glasgow Haskell Compilation System")
 ,("GCC extra via C opts"," -fwrapv -fno-builtin")
 ,("C compiler command","D:\\msys\\home\\nam83\\ghc\\_build\\stage0\\lib/../mingw/bin/gcc.exe")
 ,("C compiler flags"," -fno-stack-protector")
 ,("C compiler link flags"," ")
 ,("C compiler supports -no-pie","YES")
 ,("Haskell CPP command","D:\\msys\\home\\nam83\\ghc\\_build\\stage0\\lib/../mingw/bin/gcc.exe")
 ,("Haskell CPP flags","-E -undef -traditional")
 ,("ld command","D:\\msys\\home\\nam83\\ghc\\_build\\stage0\\lib/../mingw/bin/ld.exe")
 ,("ld flags","")
 ,("ld supports compact unwind","YES")
 ,("ld supports build-id","YES")
 ,("ld supports filelist","NO")
 ,("ld is GNU ld","YES")
 ,("ar command","D:\\msys\\home\\nam83\\ghc\\_build\\stage0\\lib/../mingw/bin/ar.exe")
 ,("ar flags","q")
 ,("ar supports at file","YES")
 ,("ranlib command","D:\\msys\\home\\nam83\\ghc\\_build\\stage0\\lib/../mingw/bin/ranlib.exe")
 ,("touch command","D:\\msys\\home\\nam83\\ghc\\_build\\stage0\\lib/bin/touchy.exe")
 ,("dllwrap command","D:\\msys\\home\\nam83\\ghc\\_build\\stage0\\lib/../mingw/bin/dllwrap.exe")
 ,("windres command","D:\\msys\\home\\nam83\\ghc\\_build\\stage0\\lib/../mingw/bin/windres.exe")
 ,("libtool command","libtool")
 ,("perl command","D:\\msys\\home\\nam83\\ghc\\_build\\stage0\\lib/../perl/perl.exe")
 ,("cross compiling","NO")
 ,("target os","OSMinGW32")
 ,("target arch","ArchX86_64")
 ,("target word size","8")
 ,("target has GNU nonexec stack","False")
 ,("target has .ident directive","True")
 ,("target has subsections via symbols","False")
 ,("target has RTS linker","YES")
 ,("Unregisterised","NO")
 ,("LLVM llc command","llc")
 ,("LLVM opt command","opt")
 ,("LLVM clang command","clang")
 ,("Project version","8.5.20180410")
 ,("Project Git commit id","b2eb9addcc42f6b9149015436150cd01297346b5")
 ,("Booter version","8.2.2")
 ,("Stage","1")
 ,("Build platform","x86_64-unknown-mingw32")
 ,("Host platform","x86_64-unknown-mingw32")
 ,("Target platform","x86_64-unknown-mingw32")
 ,("Have interpreter","YES")
 ,("Object splitting supported","YES")
 ,("Have native code generator","YES")
 ,("Support SMP","YES")
 ,("Tables next to code","YES")
 ,("RTS ways","v l debug thr thr_debug thr_l")
 ,("RTS expects libdw","NO")
 ,("Support dynamic-too","NO")
 ,("Support parallel --make","YES")
 ,("Support reexported-modules","YES")
 ,("Support thinning and renaming package flags","YES")
 ,("Support Backpack","YES")
 ,("Requires unified installed package IDs","YES")
 ,("Uses package keys","YES")
 ,("Uses unit IDs","YES")
 ,("Dynamic by default","NO")
 ,("GHC Dynamic","NO")
 ,("GHC Profiled","NO")
 ,("Leading underscore","NO")
 ,("Debug on","False")
 ,("LibDir","D:\\msys\\home\\nam83\\ghc\\_build\\stage0\\lib")
 ,("Global Package DB","D:\\msys\\home\\nam83\\ghc\\_build\\stage0\\lib\\package.conf.d")
 ]

As you can see, we have lots of incorrect paths there, including the cause of this issue:

 ,("C compiler command","D:\\msys\\home\\nam83\\ghc\\_build\\stage0\\lib/../mingw/bin/gcc.exe")
snowleopard commented 6 years ago

The configuration path to GCC on the other hand is correct:

C:/Users/nam83/AppData/Local/Programs/stack/x86_64-windows/ghc-8.2.2/lib/../mingw/bin/gcc.exe
snowleopard commented 6 years ago

The top-level settings file contains this line: ("C compiler command", "$topdir/../mingw/bin/gcc.exe").

snowleopard commented 6 years ago

I think I figured out the problem: when GHC was built in the inplace directory, it used the bundled gcc from inplace/mingw/bin. Now that we moved GHC binaries to _build/stageN/bin it can no longer locate the bundled gcc using the "$topdir/../mingw/bin/gcc.exe" recipe.

alpmestan commented 6 years ago

Hmm... That sounds like a very plausible explanation. In settings.in I can see:

 ("C compiler command", "@SettingsCCompilerCommand@"),

so... should we augment the logic that produces that value, to cover the case where we don't build inplace? I'm not sure what the additional logic should do though. The current one is implemented in aclocal.m4:

# FP_SETTINGS
# ----------------------------------
# Set the variables used in the settings file
AC_DEFUN([FP_SETTINGS],
[
    if test "$windows" = YES -a "$EnableDistroToolchain" = "NO"
    then
        mingw_bin_prefix=mingw/bin/
        SettingsCCompilerCommand="\$topdir/../${mingw_bin_prefix}gcc.exe"
        SettingsHaskellCPPCommand="\$topdir/../${mingw_bin_prefix}gcc.exe"
        SettingsHaskellCPPFlags="$HaskellCPPArgs"
        SettingsLdCommand="\$topdir/../${mingw_bin_prefix}ld.exe"
        SettingsArCommand="\$topdir/../${mingw_bin_prefix}ar.exe"
        SettingsRanlibCommand="\$topdir/../${mingw_bin_prefix}ranlib.exe"
        SettingsPerlCommand='$topdir/../perl/perl.exe'
        SettingsDllWrapCommand="\$topdir/../${mingw_bin_prefix}dllwrap.exe"
        SettingsWindresCommand="\$topdir/../${mingw_bin_prefix}windres.exe"
        SettingsTouchCommand='$topdir/bin/touchy.exe'
    elif test "$EnableDistroToolchain" = "YES"
    [...]

I'm not entirely sure how we can say "use the C toolchain at <root of ghc source tree>/inplace/mingw/bin/" (the "root of ghc source tree" part in particular). This is anyway going to be problematic when using GHC produced this way outside of the source tree. So I suppose the nicer fix is to actually copy gcc (and perphaps the whole mingw dir that lives under inplace) wherever it is that GHC is expecting them, under <build root>/stage<N>/mingw?

alpmestan commented 6 years ago

Oh, I see you're trying to use the "enable the distro toolchain" knob, that should be even simpler =)

snowleopard commented 6 years ago

@alpmestan Yep, let's see if it works :) Currently building on my machine too.

snowleopard commented 6 years ago

Ha, it worked! We now hit the _build/stage1/lib/package.conf.d/rts-1.0.conf issue on AppVeyor :)

snowleopard commented 6 years ago

@Mistuke Thanks for implementing --enable-distro-toolchain in https://phabricator.haskell.org/D3637!

Mistuke commented 6 years ago

@snowleopard you're correct in that the issue is happening because of the layout change. SysTools has a hard assumption about the layout of GHC's binaries.

In particular it assumes:

-- Assuming we are running ghc, accessed by path  $(stuff)/<foo>/ghc.exe,
-- return the path $(stuff)/lib.

But now the layout is different, not just for the build but in the resulting tarball if you were to package up ghc. Have you thought about what the layout should be in the final binary distribution. Once you know that SysTools can be patched to support Hadrian's layout.

Also be careful with copying or changing mingw's layout. that gcc.exe is actually a stub that we build, and it has some assumptions about where the real gcc and it's libraries are. See https://github.com/ghc/ghc/blob/master/driver/gcc/gcc.c

snowleopard commented 6 years ago

Have you thought about what the layout should be in the final binary distribution

@Mistuke No, I haven't thought of this and I don't think I have enough knowledge to make the right call. Could you please advise?

Naively I'd suggest something along the lines @alpmestan mentioned: simply copy mingw from inplace into the build directory, but your comment suggests that this may be not the best solution. What are the alternatives?

Mistuke commented 6 years ago

Sure, I'll have to take a closer look at the out of tree layout Hadrian uses before I can comment. I'll take a look tomorrow. :)

On Thu, Apr 12, 2018, 00:12 Andrey Mokhov notifications@github.com wrote:

Have you thought about what the layout should be in the final binary distribution

@Mistuke https://github.com/Mistuke No, I haven't thought of this and I don't think I have enough knowledge to make the right call. Could you please advise?

Naively I'd suggest something along the lines @alpmestan https://github.com/alpmestan mentioned: simply copy mingw from inplace into the build directory, but your comment suggests that this may be not the best solution. What are the alternatives?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/snowleopard/hadrian/issues/564#issuecomment-380624170, or mute the thread https://github.com/notifications/unsubscribe-auth/ABH3KTiFW_owU_QfAX-xfy6ZGQWLpbYMks5tno3egaJpZM4TNgKS .

snowleopard commented 6 years ago

Great, thanks @Mistuke!

Mistuke commented 6 years ago

hmm trying to build the tree to take a look but I keep getting

hadrian: Encountered missing dependencies:
template-haskell ==2.13.* && ==2.14.0.0

shakeArgsWith    0.001s    0%
Function shake  17.432s   58%  =========================
Database read    0.009s    0%
With database    0.001s    0%
Running rules   12.299s   41%  =================
Total           29.742s  100%
Error when running Shake build system:
* _build/stage1/bin/unlit.exe
* OracleQ (PackageDataFile (Context {stage = Stage1, package = Package {pkgLanguage = Haskell, pkgType = Program, pkgName = "unlit", pkgPath = "utils/unlit"}, way = v}))
* _build/stage1/utils/unlit/setup-config
* _build/stage0/bin/ghc.exe
* OracleQ (PackageDataFile (Context {stage = Stage0, package = Package {pkgLanguage = Haskell, pkgType = Program, pkgName = "ghc-bin", pkgPath = "ghc"}, way = v}))
* _build/stage0/ghc/setup-config
* _build/stage0/lib/package.conf.d/ghc-8.5.conf
* OracleQ (PackageDataFile (Context {stage = Stage0, package = Package {pkgLanguage = Haskell, pkgType = Library, pkgName = "ghc", pkgPath = "compiler"}, way = v}))
* _build/stage0/compiler/setup-config
* _build/stage0/lib/package.conf.d/ghci-8.5.conf
* OracleQ (PackageDataFile (Context {stage = Stage0, package = Package {pkgLanguage = Haskell, pkgType = Library, pkgName = "ghci", pkgPath = "libraries/ghci"}, way = v}))
* _build/stage0/libraries/ghci/setup-config
ExitFailure 1

seen this before?

In any case,

Naively I'd suggest something along the lines @alpmestan mentioned: simply copy mingw from inplace into the build directory, but your comment suggests that this may be not the best solution. What are the alternatives?

This is fine for the final distribution layout. All you need is for the mingw folder to be next to bin. The current system packs them as:

.
├── bin
├── doc
│   └── html
├── lib
│   ├── array-0.5.1.1
│   ├── base-4.9.1.0
│   ├── bin
│   ├── binary-0.8.3.0
│   ├── bytestring-0.10.8.1
│   ├── Cabal-1.24.2.0
│   ├── containers-0.5.7.1
│   ├── deepseq-1.4.2.0
│   ├── directory-1.3.0.0
│   ├── filepath-1.4.1.1
│   ├── ghc-8.0.2
│   ├── ghc-boot-8.0.2
│   ├── ghc-boot-th-8.0.2
│   ├── ghci-8.0.2
│   ├── ghc-prim-0.5.0.0
│   ├── haskeline-0.7.3.0
│   ├── hoopl-3.10.2.1
│   ├── hpc-0.6.0.3
│   ├── html
│   ├── include
│   ├── integer-gmp-1.0.0.1
│   ├── latex
│   ├── package.conf.d
│   ├── pretty-1.1.3.3
│   ├── process-1.4.3.0
│   ├── rts
│   ├── template-haskell-2.11.1.0
│   ├── time-1.6.0.1
│   ├── transformers-0.5.2.0
│   ├── Win32-2.3.1.1
│   └── xhtml-3000.2.1
├── man
│   └── man1
├── mingw
│   ├── bin
│   ├── i686-w64-mingw32
│   ├── include
│   ├── lib
│   └── share
└── perl

44 directories

so with this layout everything should work.

Now back to building, the problem is that --enable-distro-toolchain creates non-relocatable builds. The paths will enter the settings file in a way that a bindist won't work.

Since your layout seems to be <build root>/stage<N>/<bin|lib> we can add a configure flag --hadrian that simply unpacks the ghc-tarballs into <build root>/mingw/. This would satisfy the constraints and also move the binaries out of the source folder per hadrian's philosophy. Would an extra configure flag be acceptable?

snowleopard commented 6 years ago

@Mistuke Thanks! I haven't seen the template-haskell failure yet.

we can add a configure flag --hadrian

I think I'd prefer to automate this in Hadrian -- copy the directory or directly unpack the tarball in the right place. There are two reasons: 1) build root is configurable in Hadrian, so there may be several of them, and 2) I don't want to scare away potential Hadrian users by adding another potential obstacle, however minor.

What do you think?

Mistuke commented 6 years ago

I think I'd prefer to automate this in Hadrian -- copy the directory or directly unpack the tarball in the right place.

I wouldn't unpack the tarballs yourself, as we do some modification to the files after unpacking. like copy the gcc stub in, rename the old gcc etc.

Copying the unpacked directory is fine, though you'll have to do it after configure. So probably at build start time? And you'll end up taking twice the memory. Moving may be a better option especially if it's on the same disk, but there of course if the downloads are automated, you'll end up downloading and unpacking them on each build. which is less the ideal.

snowleopard commented 6 years ago

Copying the unpacked directory is fine, though you'll have to do it after configure

I think the correct order will be enforced using dependencies.

Copying does seem inefficient and a bit annoying. Especially, since most GHC devs probably don't care about a relocatable build... Perhaps, this could be optional?

Anyway, let me actually measure how much time it takes. Perhaps, we are trying to save a second :)

snowleopard commented 6 years ago

@Mistuke Oh, I just realised that we actually need two copies: stage0/mingw and stage1/mingw... So, all previously discussed options don't look good any more! Duplicate mingw within the same build root looks ridiculous. It's also much heavier than I expected: 361M on my machine.

alpmestan commented 6 years ago

So what are our options? Given the platforms we need to support.

snowleopard commented 6 years ago

@alpmestan Maybe the copying of mingw should only be done by the install rule/script. Most GHC devs don't care about relocatable builds -- they just want to build GHC and validate it.

Mistuke commented 6 years ago

@snowleopard No you just need one. We can change SysTools to walk higher up the chain one more directory to try and find the mingw folder. Allowing you to be able to place it next to the stageN folders instead of inside them.

Mistuke commented 6 years ago

@snowleopard sure, but if you don't produce a relocatable build, it means you have to modify the settings file in the install rule to make it relocate-able again. But then it may get out of sync with the one configure outputs. Or you have to rerun configure to get a relocatable settings file.

snowleopard commented 6 years ago

We can change SysTools to walk higher up the chain one more directory to try and find the mingw folder. Allowing you to be able to place it next to the stageN folders instead of inside them.

@Mistuke Aha, this sounds like a good compromise. Could you do this change?

Then, I presume, the install rule will simply copy the mingw folder inside stageN. At this point we probably don't care too much about duplication.

alpmestan commented 6 years ago

@Mistuke Aha, this sounds like a good compromise. Could you do this change?

I think I can give it a shot, will ping @Mistuke if I run into trouble.

snowleopard commented 6 years ago

Thanks @alpmestan!

Mistuke commented 6 years ago

Then, I presume, the install rule will simply copy the mingw folder inside stageN. At this point we probably don't care too much about duplication.

We don't actually do install usually I think. But you can easily optimize this by having the binary-dist rule compress the mingw folder in the write place at compression them. So you don't have any duplication.

Unless I'm mistaken and @bgamari can correct me on this. We usually always compress and then decompress the tarball for testing. So we don't have a direct install rule from stage2.

alpmestan commented 6 years ago

@Mistuke @snowleopard How does this look: https://github.com/alpmestan/ghc/commit/f429ee8ed0ed72983372beda638f3de467bacd20 ? Andrey, could you perhaps try out this patch? I intend to give it a spin in a Windows VM as soon as I can but I have to run away from my laptop now.

alpmestan commented 6 years ago

To be clear, this would require hadrian to place mingw/perl under <build root>/{mingw, perl}, except in the bindists I suppose.

Mistuke commented 6 years ago

Thanks Alp! I've added some feedback.

On Fri, Apr 13, 2018, 17:45 Alp Mestanogullari notifications@github.com wrote:

To be clear, this would require hadrian to place mingw/perl under <build root>/{mingw, perl}, except in the bindists I suppose.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/snowleopard/hadrian/issues/564#issuecomment-381194324, or mute the thread https://github.com/notifications/unsubscribe-auth/ABH3KWhd6eGYvO31H7zK-g06Ndhc_T44ks5toNY1gaJpZM4TNgKS .

alpmestan commented 6 years ago

@Mistuke Wow... sorry for the silly mistakes. I also refactored findToolDir as suggested. I got around to spinning up a build with this patch in a Windows VM. I then tried compiling a C file with inplace/bin/ghc-stage1.exe, it works. The affected commands in the settings file do use the new $tooldir variable, etc. So at least it looks like I did not break the inplace workflow. The patch: https://github.com/alpmestan/ghc/commit/d1b3323fde74a4dc4cd6d1da417f6af1a91009a6

@snowleopard Could you perhaps try it out whenever you get a chance to?

snowleopard commented 6 years ago

@alpmestan Thank you, this looks good to me!

Shall we also do anything about the last occurrence of $topdir in $topdir/bin/touchy.exe? See #570.

alpmestan commented 6 years ago

Oh, I didn't change this one but please do let me know if you'd like me to change how the path to touchy is determined in the same patch. Judging from the comments in #570 though, it's not obvious to me whether the problem is in hadrian or in aclocal.m4/settings.in. Could you share your thoughts on how you think we should fix touchy and whether it should go in my patch mentionned above, in case we need a change on the GHC side?

alpmestan commented 6 years ago

Got a diff up on phabricator at https://phabricator.haskell.org/D4598 -- feedback/review welcome (if/once the build passes).

snowleopard commented 6 years ago

Thanks @alpmestan! Hope it will soon be accepted.

I think relocating touchy belongs to a separate patch, because it feels like an unrelated problem.

alpmestan commented 6 years ago

Sounds good to me. GHC HEAD is however broken at the moment so CI errors out on something completely unrelated. And the diff won't be reviewable/mergeable until CI passes IIRC (due to a recent phabricator update).

Mistuke commented 6 years ago

You can pick a working build hash from my CI and rebase to that https://github.com/Mistuke/GhcWindowsBuild/commits/master

You can also manually request a review at the bottom which will take the issue out of draft. However I'd like to see CI build before I accept it. Codewise it looks fine though.

On Mon, Apr 16, 2018, 11:44 Alp Mestanogullari notifications@github.com wrote:

Sounds good to me. GHC HEAD is however broken at the moment so CI errors out on something completely unrelated. And the diff won't be reviewable/mergeable until CI passes IIRC.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/snowleopard/hadrian/issues/564#issuecomment-381557912, or mute the thread https://github.com/notifications/unsubscribe-auth/ABH3Kbl6s6TcMw7mLE5YWeUW6oR0WVi0ks5tpHYZgaJpZM4TNgKS .

alpmestan commented 6 years ago

However I'd like to see CI build before I accept it.

Yeah, me too! =)

alpmestan commented 6 years ago

@snowleopard My patch passed GHC's CI and @Mistuke accepted it, so it'll land soon. As soon as it's in master, we should be able to get rid of the distro toolchain flag to configure.

@Mistuke Thanks for being so responsive!

snowleopard commented 6 years ago

@alpmestan @Mistuke Awesome, thank you both!

Mistuke commented 6 years ago

No problem :) I pushed the commit as well.

alpmestan commented 6 years ago

@snowleopard Do you want to try getting rid of --enable-distro-toolchain?

snowleopard commented 6 years ago

@alpmestan I'm working on this -- now testing on my machine.