From b41415e56850732f17d41e217813fb5cbc996209 Mon Sep 17 00:00:00 2001
From: Michael Snoyman <michael@snoyman.com>
Date: Thu, 14 Dec 2017 10:09:15 +0200
Subject: [PATCH] More compat with buggy Cabal buildable deps #3631

This is a direct continuation of my (now realized to be incorrect) patch
in 2a52ac28d28fda0b637cbb09c3c74d1de5605a1f. This ensures that tests and
benchmarks are only switched to `buildable = True` if we're enabling
them, otherwise we can end up with cyclic dependency errors.
---
 ChangeLog.md                                   |  2 +-
 src/Stack/BuildPlan.hs                         |  2 +-
 src/Stack/Package.hs                           | 18 ++++++++++++------
 src/Stack/Snapshot.hs                          |  2 +-
 .../integration/tests/3631-build-http2/Main.hs |  6 ++++++
 5 files changed, 21 insertions(+), 9 deletions(-)
 create mode 100644 test/integration/tests/3631-build-http2/Main.hs

diff --git a/ChangeLog.md b/ChangeLog.md
index 9e13aebe43..d1eb9ec1e0 100644
--- a/ChangeLog.md
+++ b/ChangeLog.md
@@ -14,7 +14,7 @@ Bug fixes:
 
 * For versions of Cabal before 1.24, ensure that the dependencies of
   non-buildable components are part of the build plan to work around an old
-  Cabal bug.
+  Cabal bug. See [#3631](https://github.com/commercialhaskell/stack/issues/3631).
 * Run the Cabal file checking in the `sdist` command more reliably by
   allowing the Cabal library to flatten the
   `GenericPackageDescription` itself.
diff --git a/src/Stack/BuildPlan.hs b/src/Stack/BuildPlan.hs
index 8bd0a7346c..ccd4a44e60 100644
--- a/src/Stack/BuildPlan.hs
+++ b/src/Stack/BuildPlan.hs
@@ -198,7 +198,7 @@ gpdPackageDeps
     -> Map FlagName Bool
     -> Map PackageName VersionRange
 gpdPackageDeps gpd cv platform flags =
-    Map.filterWithKey (const . (/= name)) (packageDependencies cv pkgDesc)
+    Map.filterWithKey (const . (/= name)) (packageDependencies pkgConfig pkgDesc)
     where
         name = gpdPackageName gpd
         -- Since tests and benchmarks are both enabled, doesn't matter
diff --git a/src/Stack/Package.hs b/src/Stack/Package.hs
index cf07877779..cec33da759 100644
--- a/src/Stack/Package.hs
+++ b/src/Stack/Package.hs
@@ -356,7 +356,7 @@ packageFromPackageDescription packageConfig pkgFlags (PackageDescriptionPair pkg
     pkgId = package pkg
     name = fromCabalPackageName (pkgName pkgId)
     deps = M.filterWithKey (const . not . isMe) (M.union
-        (packageDependencies (packageConfigCompilerVersion packageConfig) pkg)
+        (packageDependencies packageConfig pkg)
         -- We include all custom-setup deps - if present - in the
         -- package deps themselves. Stack always works with the
         -- invariant that there will be a single installed package
@@ -610,25 +610,31 @@ getBuildComponentDir (Just name) = parseRelDir (name FilePath.</> (name ++ "-tmp
 -- being 7.10 or earlier. This obviously makes our function a lot more
 -- fun to write...
 packageDependencies
-  :: CompilerVersion 'CVActual
+  :: PackageConfig
   -> PackageDescription
   -> Map PackageName VersionRange
-packageDependencies ghcVersion pkg' =
+packageDependencies pkgConfig pkg' =
   M.fromListWith intersectVersionRanges $
   map (depName &&& depRange) $
   concatMap targetBuildDepends (allBuildInfo' pkg) ++
   maybe [] setupDepends (setupBuildInfo pkg)
   where
     pkg
-      | getGhcVersion ghcVersion >= $(mkVersion "8.0") = pkg'
+      | getGhcVersion (packageConfigCompilerVersion pkgConfig) >= $(mkVersion "8.0") = pkg'
       -- Set all components to buildable. Only need to worry about
       -- library, exe, test, and bench, since others didn't exist in
       -- older Cabal versions
       | otherwise = pkg'
         { library = (\c -> c { libBuildInfo = go (libBuildInfo c) }) <$> library pkg'
         , executables = (\c -> c { buildInfo = go (buildInfo c) }) <$> executables pkg'
-        , testSuites = (\c -> c { testBuildInfo = go (testBuildInfo c) }) <$> testSuites pkg'
-        , benchmarks = (\c -> c { benchmarkBuildInfo = go (benchmarkBuildInfo c) }) <$> benchmarks pkg'
+        , testSuites =
+            if packageConfigEnableTests pkgConfig
+              then (\c -> c { testBuildInfo = go (testBuildInfo c) }) <$> testSuites pkg'
+              else testSuites pkg'
+        , benchmarks =
+            if packageConfigEnableBenchmarks pkgConfig
+              then (\c -> c { benchmarkBuildInfo = go (benchmarkBuildInfo c) }) <$> benchmarks pkg'
+              else benchmarks pkg'
         }
 
     go bi = bi { buildable = True }
diff --git a/src/Stack/Snapshot.hs b/src/Stack/Snapshot.hs
index b6027d2d89..4889ff3fdc 100644
--- a/src/Stack/Snapshot.hs
+++ b/src/Stack/Snapshot.hs
@@ -769,7 +769,7 @@ calculate gpd platform compilerVersion loc flags hide options =
       , lpiGhcOptions = options
       , lpiPackageDeps = Map.map fromVersionRange
                        $ Map.filterWithKey (const . (/= name))
-                       $ packageDependencies compilerVersion pd
+                       $ packageDependencies pconfig pd
       , lpiProvidedExes =
             Set.fromList
           $ map (ExeName . T.pack . C.unUnqualComponentName . C.exeName)
diff --git a/test/integration/tests/3631-build-http2/Main.hs b/test/integration/tests/3631-build-http2/Main.hs
new file mode 100644
index 0000000000..c56b326c01
--- /dev/null
+++ b/test/integration/tests/3631-build-http2/Main.hs
@@ -0,0 +1,6 @@
+import StackTest
+
+main :: IO ()
+main = do
+  stack ["build", "--resolver=lts-6.35", "--dry-run", "http2"]
+  stack ["build", "--resolver=lts-6.35", "http2"]