Merge pull request #4033 from ezyang/pr/T4025

Don't use installedPkgs for internal library library dirs.

Cabal/Cabal.cabal

@@ -271,6 +271,9 @@ extra-source-files:
   tests/PackageTests/Regression/T3294/T3294.cabal
   tests/PackageTests/Regression/T3847/Main.hs
   tests/PackageTests/Regression/T3847/T3847.cabal
+  tests/PackageTests/Regression/T4025/A.hs
+  tests/PackageTests/Regression/T4025/T4025.cabal
+  tests/PackageTests/Regression/T4025/exe/Main.hs
   tests/PackageTests/TemplateHaskell/dynamic/Exe.hs
   tests/PackageTests/TemplateHaskell/dynamic/Lib.hs
   tests/PackageTests/TemplateHaskell/dynamic/TH.hs

Cabal/Distribution/Simple/LocalBuildInfo.hs

@@ -268,13 +268,23 @@ depLibraryPaths inplace relative lbi clbi = do
           | inplace    = componentBuildDir lbi sub_clbi
           | otherwise  = dynlibdir (absoluteComponentInstallDirs pkgDescr lbi (componentUnitId sub_clbi) NoCopyDest)
 
-    let ipkgs          = allPackages (installedPkgs lbi)
+    -- Why do we go through all the trouble of a hand-crafting
+    -- internalLibs, when 'installedPkgs' actually contains the
+    -- internal libraries?  The trouble is that 'installedPkgs'
+    -- may contain *inplace* entries, which we must NOT use for
+    -- not inplace 'depLibraryPaths' (e.g., for RPATH calculation).
+    -- See #4025 for more details. This is all horrible but it
+    -- is a moot point if you are using a per-component build,
+    -- because you never have any internal libraries in this case;
+    -- they're all external.
+    let external_ipkgs = filter is_external (allPackages (installedPkgs lbi))
+        is_external ipkg = not (installedUnitId ipkg `elem` internalDeps)
         -- First look for dynamic libraries in `dynamic-library-dirs`, and use
         -- `library-dirs` as a fall back.
         getDynDir pkg  = case Installed.libraryDynDirs pkg of
                            [] -> Installed.libraryDirs pkg
                            d  -> d
-        allDepLibDirs  = concatMap getDynDir ipkgs
+        allDepLibDirs  = concatMap getDynDir external_ipkgs
 
         allDepLibDirs' = internalLibs ++ allDepLibDirs
     allDepLibDirsC <- traverse canonicalizePathNoFail allDepLibDirs'

Cabal/tests/PackageTests/Regression/T4025/A.hs

@@ -0,0 +1,4 @@
+module A where
+{-# NOINLINE a #-}
+a :: Int
+a = 23

Cabal/tests/PackageTests/Regression/T4025/T4025.cabal

@@ -0,0 +1,13 @@
+name: T4025
+version: 1.0
+build-type: Simple
+cabal-version: >= 1.10
+
+library
+  build-depends: base
+  exposed-modules: A
+
+executable exe
+  build-depends: T4025, base
+  hs-source-dirs: exe
+  main-is: Main.hs

Cabal/tests/PackageTests/Regression/T4025/exe/Main.hs

@@ -0,0 +1,2 @@
+import A
+main = print a

Cabal/tests/PackageTests/Tests.hs

@@ -547,6 +547,19 @@ tests config = do
   -- Test that we pick up include dirs from internal library
   tc "Regression/T2971a" $ cabal_build []
 
+  -- Test that we don't accidentally add the inplace directory to
+  -- an executable RPATH.  Don't test on Windows, which doesn't
+  -- support RPATH.
+  unlessWindows $ do
+    tc "Regression/T4025" $ do
+      cabal "configure" ["--enable-executable-dynamic"]
+      cabal "build" []
+      -- This should fail as it we should NOT be able to find the
+      -- dynamic library for the internal library (since we didn't
+      -- install it).  If we incorrectly encoded our local dist
+      -- dir in the RPATH, this will succeed.
+      shouldFail $ runExe' "exe" []
+
   -- Test error message we report when a non-buildable target is
   -- requested to be built
   -- TODO: We can give a better error message here, see #3858.