genprimopcode: Support Unicode properly

2 files changed, 38 insertions, 3 deletions

diff --git a/hadrian/src/Builder.hs b/hadrian/src/Builder.hs
index e29bf4316c..160eb44f7a 100644
--- a/hadrian/src/Builder.hs
+++ b/hadrian/src/Builder.hs
@@ -307,7 +307,8 @@ instance H.Builder Builder where
                 -- Capture stdout and write it to the output file.
                 captureStdout = do
                     Stdout stdout <- cmd' [path] buildArgs
-                    writeFileChanged output stdout
+                    -- see Note [Capture stdout as a ByteString]
+                    writeFileChangedBS output stdout
             case builder of
                 Ar Pack _ -> do
                     useTempFile <- flag ArSupportsAtFile
@@ -332,7 +333,8 @@ instance H.Builder Builder where
                 GenPrimopCode -> do
                     stdin <- readFile' input
                     Stdout stdout <- cmd' (Stdin stdin) [path] buildArgs
-                    writeFileChanged output stdout
+                    -- see Note [Capture stdout as a ByteString]
+                    writeFileChangedBS output stdout
 
                 GhcPkg Copy _ -> do
                     Stdout pkgDesc <- cmd' [path]
@@ -509,6 +511,19 @@ applyPatch dir patch = do
 --  tell if an Exit or ExitCode value is returned in `r`. So we use our own
 --  HasExit type class to provide the `hasExit` predicate that tells us if we
 --  should throw an exception as `cmd` would do in case of failure or not.
+--
+-- Note [Capture stdout as a ByteString]
+-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+-- As of shake-0.19.6, capturing a process stdout as a `String` using `Stdout`
+-- mangles the encoding if it some other use of `Stdout` also captures it as a
+-- `ByteString`; see <https://github.com/ndmitchell/shake/issues/828>. This
+-- can cause us real problems, since `cmd'` (see Note [cmd wrapper]) *always*
+-- captures stdout as a `ByteString`.
+--
+-- Fortunately, a simple workaround is to avoid capturing stdout as a `String`
+-- in the first place. It’s usually unnecessary (and is in fact pointless work),
+-- as most of the time the captured output is immediately written to a file, so
+-- we can just treat it as an opaque binary stream.
 
 
 -- | Wrapper for Shake's 'cmd'
diff --git a/hadrian/src/Hadrian/Utilities.hs b/hadrian/src/Hadrian/Utilities.hs
index e2fb221940..40b7835e55 100644
--- a/hadrian/src/Hadrian/Utilities.hs
+++ b/hadrian/src/Hadrian/Utilities.hs
@@ -18,7 +18,7 @@ module Hadrian.Utilities (
     -- * File system operations
     copyFile, copyFileUntracked, createFileLink, fixFile,
     makeExecutable, moveFile, removeFile, createDirectory, copyDirectory,
-    moveDirectory, removeDirectory, removeFile_,
+    moveDirectory, removeDirectory, removeFile_, writeFileChangedBS,
 
     -- * Diagnostic info
     Colour (..), ANSIColour (..), putColoured, shouldUseColor,
@@ -49,6 +49,7 @@ import Development.Shake.Classes
 import Development.Shake.FilePath
 import System.Environment (lookupEnv)
 
+import qualified Data.ByteString        as BS
 import qualified Control.Exception.Base as IO
 import qualified Data.HashMap.Strict    as Map
 import qualified System.Directory.Extra as IO
@@ -394,6 +395,25 @@ removeDirectory dir = do
     putProgressInfo $ "| Remove directory " ++ dir
     liftIO . whenM (IO.doesDirectoryExist dir) $ IO.removeDirectoryRecursive dir
 
+-- | Like Shake's 'writeFileChanged', but accepts a 'ByteString'.
+writeFileChangedBS :: FilePath -> BS.ByteString -> Action ()
+writeFileChangedBS name new = do
+    liftIO $ IO.createDirectoryIfMissing True $ takeDirectory name
+    exists <- liftIO $ IO.doesFileExist name
+    if exists
+      then do
+        old <- liftIO $ BS.readFile name
+        when (old /= new) $ do
+          liftIO $ removeFile_ name
+          do_write
+      else
+        do_write
+  where
+    do_write = do
+      putProgressInfo $ "| Write file " ++ name
+      liftIO $ BS.writeFile name new
+
+
 -- | Terminal output colours
 data Colour
     = Dull ANSIColour   -- ^ 8-bit ANSI colours