More compat with buggy Cabal buildable deps #3631

This is a direct continuation of my (now realized to be incorrect) patch
in 2a52ac2. 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.

Author: snoyberg

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.

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

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 }

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)

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"]