Merge pull request #11101 from fendor/fendor/interactive-is-the-first-argument-3.16

Always pass '--interactive' as the first ghc argument

Author: mergify[bot]

Cabal/src/Distribution/Simple/Program/GHC.hs

@@ -692,6 +692,22 @@ runGHCWithResponseFile fileNameTemplate encoding tempFileOptions verbosity ghcPr
 
           runProgramInvocation verbosity newInvocation
 
+-- Note [Make --interactive the first argument to GHC]
+-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+-- The ghc argument @--interactive@ needs to be the first argument to the
+-- ghc invocation, because Haskell Language Server used to rely on this.
+-- This was initially changed for Cabal 3.16, but it broke all existing Haskell
+-- Language Server prebuilt binaries.
+-- To avoid this, we uphold this assumption in Haskell Language Server until the next
+-- Cabal release (3.18).
+--
+-- The solution is to make sure that @--interactive@ is not passed as an argument in
+-- the response file that is usually passed to ghc.
+-- Instead, we filter out @--interactive@ and always pass it as the first argument,
+-- if it exists.
+--
+-- We plan to remove this Hack in Cabal 3.18.
+
 -- Start the repl.  Either use `ghc`, or the program specified by the --with-repl flag.
 runReplProgram
   :: Maybe FilePath
@@ -704,11 +720,23 @@ runReplProgram
   -> Maybe (SymbolicPath CWD (Dir Pkg))
   -> GhcOptions
   -> IO ()
-runReplProgram withReplProg tempFileOptions verbosity ghcProg comp platform mbWorkDir ghcOpts =
+runReplProgram withReplProg _tempFileOptions verbosity ghcProg comp platform mbWorkDir ghcOpts =
   let replProg = case withReplProg of
         Just path -> ghcProg{programLocation = FoundOnSystem path}
         Nothing -> ghcProg
-   in runGHCWithResponseFile "ghci.rsp" Nothing tempFileOptions verbosity replProg comp platform mbWorkDir ghcOpts
+   in do
+        -- in runGHCWithResponseFile "ghci.rsp" Nothing tempFileOptions verbosity replProg comp platform mbWorkDir ghcOpts
+        -- See Note [Make --interactive the first argument to GHC]
+        -- In Cabal 3.18, restore the line above.
+        invocation <- ghcInvocation verbosity replProg comp platform mbWorkDir ghcOpts
+        let invocation' =
+              let
+                argsWithoutInteractive = filter (/= "--interactive") (progInvokeArgs invocation)
+               in
+                invocation
+                  { progInvokeArgs = ["--interactive"] <> argsWithoutInteractive
+                  }
+        runProgramInvocation verbosity invocation'
 
 ghcInvocation
   :: Verbosity

cabal-install/src/Distribution/Client/CmdRepl.hs

@@ -481,7 +481,8 @@ replAction flags@NixStyleFlags{extraFlags = r@ReplFlags{..}, ..} targetStrings g
             unit_files_ordered :: [FilePath]
             unit_files_ordered =
               let (active_unit_files, other_units) = partition (\fp -> Just fp == active_unit_fp) unit_files
-               in -- GHC considers the last unit passed to be the active one
+               in -- older GHC versions consider the last unit passed to be the active one.
+                  -- GHC 9.14 no longer has the notion of "active" units.
                   other_units ++ active_unit_files
 
             convertParStrat :: ParStratX Int -> ParStratX String

cabal-testsuite/PackageTests/WithRepl/InteractiveIsFirstTest/Main.hs

@@ -0,0 +1,5 @@
+import System.Environment
+import Control.Monad
+
+main :: IO ()
+main = putStrLn . unwords =<< getArgs

cabal-testsuite/PackageTests/WithRepl/InteractiveIsFirstTest/cabal-with-repl-exe.cabal

@@ -0,0 +1,21 @@
+cabal-version:      2.4
+name:               cabal-with-repl-exe
+version:            0.1.0.0
+build-type:         Simple
+
+executable test-exe
+  main-is:          Main.hs
+  build-depends:    base
+  default-language: Haskell2010
+
+library internal1
+  hs-source-dirs:   source1
+  build-depends:    base
+  exposed-modules:  Lib1
+  default-language: Haskell2010
+
+library internal2
+  hs-source-dirs:   source2
+  build-depends:    base
+  exposed-modules:  Lib2
+  default-language: Haskell2010

cabal-testsuite/PackageTests/WithRepl/InteractiveIsFirstTest/cabal.project

@@ -0,0 +1 @@
+packages: .

cabal-testsuite/PackageTests/WithRepl/InteractiveIsFirstTest/cabal.test.hs

@@ -0,0 +1,39 @@
+{-# LANGUAGE OverloadedStrings #-}
+import Distribution.Simple.Program
+import Distribution.Simple.Program.GHC
+import Distribution.Simple.Utils
+import Test.Cabal.Prelude
+
+-- On windows this test passed but then fails in CI with
+--
+-- D:\a\_temp\cabal-testsuite-12064\cabal-multi.dist\work\.\dist\multi-out-63884\paths\cabal-with-repl-exe-0.1.0.0-inplace-test-exe: removeDirectoryRecursive:removeContentsRecursive:removePathRecursive:removeContentsRecursive:removePathRecursive:DeleteFile "\\\\?\\D:\\a\\_temp\\cabal-testsuite-12064\\cabal-multi.dist\\work\\dist\\multi-out-63884\\paths\\cabal-with-repl-exe-0.1.0.0-inplace-test-exe": permission denied (The process cannot access the file because it is being used by another process.)
+--
+
+main = do
+  -- Test that '--with-repl' with one target lists '--interactive' as the first argument
+  -- This test should be deleted, once we reach Cabal 3.18, and enough HLS
+  -- binaries have been released that support response file syntax.
+  -- See Note [Make --interactive the first argument to GHC]
+  mkTest "repl-single-target" $ \exePath -> do
+    res <- cabalWithStdin "v2-repl" ["--with-repl=" ++ exePath, "test-exe"] ""
+    assertOutputMatches "^--interactive" res
+  mkTest "repl-multi-target" $ \exePath -> do
+    requireGhcSupportsMultiRepl
+    res <- cabalWithStdin "v2-repl" ["--enable-multi-repl", "--with-repl=" ++ exePath, "internal1", "internal2"] ""
+    assertOutputMatches "^--interactive" res
+  mkTest "repl-multi-target-all" $ \exePath -> do
+    requireGhcSupportsMultiRepl
+    res <- cabalWithStdin "v2-repl" ["--enable-multi-repl", "--with-repl=" ++ exePath, "all"] ""
+    assertOutputMatches "^--interactive" res
+
+
+mkTest name act = do
+  skipIfCIAndWindows 11026
+  cabalTest' name $ recordMode DoNotRecord $ do
+    -- Build the executable
+    cabal' "v2-build" ["test-exe"]
+    -- Get the path to the built executable
+    withPlan $ do
+      exePath <- planExePath "cabal-with-repl-exe" "test-exe"
+      act exePath
+

cabal-testsuite/PackageTests/WithRepl/InteractiveIsFirstTest/source1/Lib1.hs

@@ -0,0 +1,4 @@
+module Lib1 where
+
+y :: Int
+y = 42

cabal-testsuite/PackageTests/WithRepl/InteractiveIsFirstTest/source2/Lib2.hs

@@ -0,0 +1,4 @@
+module Lib2 where
+
+x :: Int
+x = 42

changelog.d/pr-11101

@@ -0,0 +1,14 @@
+synopsis: Always pass '--interactive' as the first ghc argument
+packages: Cabal
+prs: #11101
+issues: #11099
+description: {
+We recently changed Cabal to use response files for all GHC arguments by default.
+Unfortunately, this broke a couple of downstream consumers of cabal, notably Haskell Language Server and doctest, which both assume that `--interactive` is the first argument of the `ghc` invocation by `cabal repl`.
+
+This regression was fixed by implementing the `--with-repl` (#9115) argument, and tools, such as hie-bios and doctest, have been updated to take advantage of this. However, this renders already published HLS binaries, so any HLS version <=2.12.0.0, incompatible with `cabal-3.16`, as these old binaries can't be easily updated.
+
+In other words, no released HLS binary (at the point of this commit), is compatible with the cabal-3.16.0.0. Users have to build HLS from source to have a working toolchain.
+
+To give us a slightly better migration window, we undo some of the improvements to cabal, and make sure that `--interactive` is always passed as the first argument to the underlying `ghc` invocation. This hack is supposed to be temporary, and removed for cabal 3.18.
+}