Force reconfigure when deps rebuild commercialhaskell#2781

ChangeLog.md

@@ -150,6 +150,11 @@ Bug fixes:
   [#3476](https://github.com/commercialhaskell/stack/issues/3476).
 * Properly handle relative paths stored in the precompiled cache files. See
   [#3431](https://github.com/commercialhaskell/stack/issues/3431).
+* In some cases, Cabal does not realize that it needs to reconfigure, and must
+  be told to do so automatically. This would manifest as a "shadowed
+  dependency" error message. We now force a reconfigure whenever a dependency is
+  built, even if the package ID remained the same. See
+  [#2781](https://github.com/commercialhaskell/stack/issues/2781).
 
 
 ## 1.5.1

src/Stack/Build/ConstructPlan.hs

@@ -354,6 +354,7 @@ addFinal lp package isAllInOne = do
                 , taskType = TTFiles lp Local -- FIXME we can rely on this being Local, right?
                 , taskAllInOne = isAllInOne
                 , taskCachePkgSrc = CacheSrcLocal (toFilePath (lpDir lp))
+                , taskAnyMissing = not $ Set.null missing
                 }
     tell mempty { wFinals = Map.singleton (packageName package) res }
 
@@ -562,6 +563,7 @@ installPackageGivenDeps isAllInOne ps package minstalled (missing, present, minL
                     PSIndex loc _ _ pkgLoc -> TTIndex package (loc <> minLoc) pkgLoc
             , taskAllInOne = isAllInOne
             , taskCachePkgSrc = toCachePkgSrc ps
+            , taskAnyMissing = not $ Set.null missing
             }
 
 -- Update response in the lib map. If it is an error, and there's

src/Stack/Build/Execute.hs

@@ -787,11 +787,12 @@ ensureConfig :: HasEnvConfig env
              -> RIO env () -- ^ announce
              -> (ExcludeTHLoading -> [String] -> RIO env ()) -- ^ cabal
              -> Path Abs File -- ^ .cabal file
+             -> Task
              -> RIO env Bool
-ensureConfig newConfigCache pkgDir ExecuteEnv {..} announce cabal cabalfp = do
+ensureConfig newConfigCache pkgDir ExecuteEnv {..} announce cabal cabalfp task = do
     newCabalMod <- liftIO (fmap modTime (D.getModificationTime (toFilePath cabalfp)))
     needConfig <-
-        if boptsReconfigure eeBuildOpts
+        if boptsReconfigure eeBuildOpts || taskAnyMissing task
             then return True
             else do
                 -- We can ignore the components portion of the config
@@ -1305,7 +1306,7 @@ singleBuild runInBase ac@ActionContext {..} ee@ExecuteEnv {..} task@Task {..} in
                       ("Building all executables for `" <> packageNameText (packageName package) <>
                        "' once. After a successful build of all of them, only specified executables will be rebuilt."))
 
-            _neededConfig <- ensureConfig cache pkgDir ee (announce ("configure" <> annSuffix executableBuildStatuses)) cabal cabalfp
+            _neededConfig <- ensureConfig cache pkgDir ee (announce ("configure" <> annSuffix executableBuildStatuses)) cabal cabalfp task
 
             let installedMapHasThisPkg :: Bool
                 installedMapHasThisPkg =

src/Stack/SDist.hs

@@ -291,6 +291,7 @@ getSDistFileList lp =
         , taskPresent = Map.empty
         , taskAllInOne = True
         , taskCachePkgSrc = CacheSrcLocal (toFilePath (lpDir lp))
+        , taskAnyMissing = True
         }
 
 normalizeTarballPaths :: HasRunner env => [FilePath] -> RIO env [FilePath]

src/Stack/Types/Build.hs

@@ -404,6 +404,17 @@ data Task = Task
     , taskAllInOne        :: !Bool
     -- ^ indicates that the package can be built in one step
     , taskCachePkgSrc     :: !CachePkgSrc
+    , taskAnyMissing      :: !Bool
+    -- ^ Were any of the dependencies missing? The reason this is
+    -- necessary is... hairy. And as you may expect, a bug in
+    -- Cabal. See:
+    -- <https://github.com/haskell/cabal/issues/4728#issuecomment-337937673>. The
+    -- problem is that Cabal may end up generating the same package ID
+    -- for a dependency, even if the ABI has changed. As a result,
+    -- without this field, Stack would think that a reconfigure is
+    -- unnecessary, when in fact we _do_ need to reconfigure. The
+    -- details here suck. We really need proper hashes for package
+    -- identifiers.
     }
     deriving Show