Remove special casing of Windows in generic files
ClosedPublic

Authored by Phyx on May 21 2016, 3:34 AM.

Details

Summary

Remove some Windows specific code from the .m4 files
and have configure figure it out.

Unfortunately touchy can't be removed since there
is no mingw build of coreutils. Only msys builds
which would give us a dependency on the msys runtime.

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.
Phyx updated this revision to Diff 7676.May 21 2016, 3:34 AM
Phyx retitled this revision from to Remove special casing of Windows in generic files.
Phyx updated this object.
Phyx edited the test plan for this revision. (Show Details)
Phyx added a reviewer: hvr.
Phyx added a subscriber: Restricted Project.
hvr awarded a token.May 21 2016, 3:37 AM
thomie added a comment.EditedMay 21 2016, 4:44 AM

Unfortunately touchy can't be removed

Could you please document this somewhere, maybe in touchy.c. I've thought about deleting touchy before, and I'm surprised it isn't possible.

bgamari requested changes to this revision.May 24 2016, 4:04 AM
bgamari edited edge metadata.

I agree that it would be nice to mention the fact that we need to provide touchy since msys doesn't ship coreutils. There is already a rather long comment in touchy.c; perhaps just add a short paragraph at the beginning of that discussion (since the comment doesn't otherwise motivate *why* the program is necessary).

This revision now requires changes to proceed.May 24 2016, 4:04 AM
Phyx added a comment.May 24 2016, 4:40 AM
In D2248#64977, @thomie wrote:

Unfortunately touchy can't be removed

Could you please document this somewhere, maybe in touchy.c. I've thought about deleting touchy before, and I'm surprised it isn't possible.

The problem is that it seems it's not just used during the building of ghc, but it's also in SysTools. Which means we have to bundle a touch like program with the bindist.

Msys2 has an msys-coreutils but not a native mingw-w64-coreutils so if we use that one we need to also include other msys runtime dlls. Not sure that's any better than just building touchy then.

An alternative is the gnucoreutils for Windows project that does have a native version of touch. But that means we'll be pulling a binary in from another source. Though we already do something similar for perl. So that could be an option too.

hvr edited edge metadata.May 24 2016, 5:08 AM
In D2248#65260, @Phyx wrote:

Though we already do something similar for perl. So that could be an option too.

Fwiw, I still hope we can at some point drop the dependency on perl... I still don't like that we have to bundle 3rd party binaries with GHC bindists

Phyx updated this revision to Diff 7769.May 29 2016, 7:39 AM
Phyx edited edge metadata.
  • Touchy: Add comment to why touchy is needed
Phyx added a comment.May 29 2016, 7:59 AM

I added the comment, but have been looking at this some more.

So SysTools.hs uses the SettingsTouchCommand to make a command touch and exposes it.

On Windows this is pointing to touchy and ghc --info advertises it as ,("touch command","$topdir/touchy.exe")

But the bindist does not contain touchy at all, nor as far as I can see, is this command used.. (couldn't find it with a grep).

My question then is, does anyone know if touch is used at all by GHC? If it's not we can clean up touchy.

Phyx added a comment.May 29 2016, 8:55 AM

It is used in DriverPipeline in touchObjectFile. and the binary is in lib instead of bin.

bgamari requested changes to this revision.May 30 2016, 3:27 AM
bgamari edited edge metadata.

Are these libtool changes intentional?

mk/get-win32-tarballs.sh
67 ↗(On Diff #7769)

Hmm, what's going on here? I guess we'll need to mirror the libtool tarball as well?

This revision now requires changes to proceed.May 30 2016, 3:27 AM
Phyx added inline comments.May 30 2016, 12:56 PM
mk/get-win32-tarballs.sh
67 ↗(On Diff #7769)

Yeah we do, libtool is only 470kb though so bindist shouldn't grow. But currently Windows is the only platform without libtool support.
According to ghc --info ,("libtool command","") So it means that runLibTool is only not supported there.

It seems that this is used by linkBinary' in case staticlink is specified. Looking at it further, I come out at:

  = do 
2002     when (platformOS (targetPlatform dflags) `notElem` [OSiOS, OSDarwin]) $ 
2003       throwGhcExceptionIO (ProgramError "Static archive creation only supported on Darwin/OS X/iOS")

So It seems I can revert this for now.

Phyx updated this revision to Diff 7793.May 31 2016, 1:15 PM
Phyx edited edge metadata.
  • Windows: removed libtool
thomie accepted this revision.Jun 3 2016, 5:02 PM
thomie added a reviewer: thomie.

Thanks for the comment.

bgamari accepted this revision.Jun 8 2016, 3:53 AM
bgamari edited edge metadata.

Alright, thanks @Phyx!

This revision is now accepted and ready to land.Jun 8 2016, 3:53 AM
This revision was automatically updated to reflect the committed changes.
Phyx added a comment.Jun 9 2016, 4:33 AM

Cheers,

Thanks for the review @thomie and @bgamari