Skip to content

Update installedPkgs with internal deps, fix a bug #4006

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Cabal/Cabal.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,14 @@ extra-source-files:
tests/PackageTests/ReexportedModules/p/p.cabal
tests/PackageTests/ReexportedModules/q/A.hs
tests/PackageTests/ReexportedModules/q/q.cabal
tests/PackageTests/Regression/T2971/p/include/T2971test.h
tests/PackageTests/Regression/T2971/p/p.cabal
tests/PackageTests/Regression/T2971/q/Bar.hsc
tests/PackageTests/Regression/T2971/q/Foo.hs
tests/PackageTests/Regression/T2971/q/q.cabal
tests/PackageTests/Regression/T2971a/Main.hsc
tests/PackageTests/Regression/T2971a/T2971a.cabal
tests/PackageTests/Regression/T2971a/include/T2971a.h
tests/PackageTests/Regression/T3294/T3294.cabal
tests/PackageTests/Regression/T3847/Main.hs
tests/PackageTests/Regression/T3847/T3847.cabal
Expand Down
23 changes: 18 additions & 5 deletions Cabal/Distribution/Simple/Build.hs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import qualified Distribution.Simple.JHC as JHC
import qualified Distribution.Simple.LHC as LHC
import qualified Distribution.Simple.UHC as UHC
import qualified Distribution.Simple.HaskellSuite as HaskellSuite
import qualified Distribution.Simple.PackageIndex as Index

import qualified Distribution.Simple.Build.Macros as Build.Macros
import qualified Distribution.Simple.Build.PathsModule as Build.PathsModule
Expand All @@ -49,6 +50,7 @@ import qualified Distribution.Simple.Program.HcPkg as HcPkg
import Distribution.Simple.Compiler hiding (Flag)
import Distribution.PackageDescription hiding (Flag)
import qualified Distribution.InstalledPackageInfo as IPI
import Distribution.InstalledPackageInfo (InstalledPackageInfo)
import qualified Distribution.ModuleName as ModuleName

import Distribution.Simple.Setup
Expand All @@ -69,6 +71,7 @@ import Distribution.Verbosity

import Distribution.Compat.Graph (IsNode(..))

import Control.Monad
import qualified Data.Set as Set
import Data.List ( intersect )
import System.FilePath ( (</>), (<.>), takeDirectory )
Expand Down Expand Up @@ -96,18 +99,21 @@ build pkg_descr lbi flags suffixes = do

internalPackageDB <- createInternalPackageDB verbosity lbi distPref

for_ componentsToBuild $ \target -> do
foldM_ `flip` installedPkgs lbi `flip` componentsToBuild $ \index target -> do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance we can write this so someone might understand it?

let comp = targetComponent target
clbi = targetCLBI target
initialBuildSteps distPref pkg_descr lbi clbi verbosity
let bi = componentBuildInfo comp
progs' = addInternalBuildTools pkg_descr lbi bi (withPrograms lbi)
lbi' = lbi {
withPrograms = progs',
withPackageDB = withPackageDB lbi ++ [internalPackageDB]
withPackageDB = withPackageDB lbi ++ [internalPackageDB],
installedPkgs = index
}
buildComponent verbosity (buildNumJobs flags) pkg_descr
mb_ipi <- buildComponent verbosity (buildNumJobs flags) pkg_descr
lbi' suffixes comp clbi distPref
return (maybe index (Index.insert `flip` index) mb_ipi)
return ()
where
distPref = fromFlag (buildDistPref flags)
verbosity = fromFlag (buildVerbosity flags)
Expand Down Expand Up @@ -178,7 +184,7 @@ buildComponent :: Verbosity
-> Component
-> ComponentLocalBuildInfo
-> FilePath
-> IO ()
-> IO (Maybe InstalledPackageInfo)
buildComponent verbosity numJobs pkg_descr lbi suffixes
comp@(CLib lib) clbi distPref = do
preprocessComponent pkg_descr comp lbi clbi False verbosity suffixes
Expand All @@ -195,7 +201,8 @@ buildComponent verbosity numJobs pkg_descr lbi suffixes
-- Don't register inplace if we're only building a single component;
-- it's not necessary because there won't be any subsequent builds
-- that need to tag us
when (not (oneComponentRequested (componentEnabledSpec lbi))) $ do
if (not (oneComponentRequested (componentEnabledSpec lbi)))
then do
-- Register the library in-place, so exes can depend
-- on internally defined libraries.
pwd <- getCurrentDirectory
Expand All @@ -206,6 +213,8 @@ buildComponent verbosity numJobs pkg_descr lbi suffixes
debug verbosity $ "Registering inplace:\n" ++ (IPI.showInstalledPackageInfo installedPkgInfo)
registerPackage verbosity (compiler lbi) (withPrograms lbi) HcPkg.MultiInstance
(withPackageDB lbi) installedPkgInfo
return (Just installedPkgInfo)
else return Nothing

buildComponent verbosity numJobs pkg_descr lbi suffixes
comp@(CExe exe) clbi _ = do
Expand All @@ -215,6 +224,7 @@ buildComponent verbosity numJobs pkg_descr lbi suffixes
let ebi = buildInfo exe
exe' = exe { buildInfo = addExtraCSources ebi extras }
buildExe verbosity numJobs pkg_descr lbi exe' clbi
return Nothing


buildComponent verbosity numJobs pkg_descr lbi suffixes
Expand All @@ -227,6 +237,7 @@ buildComponent verbosity numJobs pkg_descr lbi suffixes
let ebi = buildInfo exe
exe' = exe { buildInfo = addExtraCSources ebi extras }
buildExe verbosity numJobs pkg_descr lbi exe' clbi
return Nothing


buildComponent verbosity numJobs pkg_descr lbi0 suffixes
Expand All @@ -253,6 +264,7 @@ buildComponent verbosity numJobs pkg_descr lbi0 suffixes
let ebi = buildInfo exe
exe' = exe { buildInfo = addExtraCSources ebi extras }
buildExe verbosity numJobs pkg_descr lbi exe' exeClbi
return Nothing -- Can't depend on test suite


buildComponent _ _ _ _ _
Expand All @@ -271,6 +283,7 @@ buildComponent verbosity numJobs pkg_descr lbi suffixes
let ebi = buildInfo exe
exe' = exe { buildInfo = addExtraCSources ebi extras }
buildExe verbosity numJobs pkg_descr lbi exe' exeClbi
return Nothing


buildComponent _ _ _ _ _
Expand Down
17 changes: 10 additions & 7 deletions Cabal/Distribution/Simple/PreProcess.hs
Original file line number Diff line number Diff line change
Expand Up @@ -434,13 +434,16 @@ ppHsc2hs bi lbi clbi =
++ ["-o", outFile, inFile]
}
where
-- TODO: installedPkgs contains ALL dependencies associated with
-- the package, but we really only want to look at packages for the
-- *current* dependency. We should use PackageIndex.dependencyClosure
-- on the direct depends of the component. The signature of this
-- function was recently refactored, so this should be fixable
-- now. Tracked with #2971 (which has a test case.)
pkgs = PackageIndex.topologicalOrder (packageHacks (installedPkgs lbi))
hacked_index = packageHacks (installedPkgs lbi)
-- Look only at the dependencies of the current component
-- being built! This relies on 'installedPkgs' maintaining
-- 'InstalledPackageInfo' for internal deps too; see #2971.
pkgs = PackageIndex.topologicalOrder $
case PackageIndex.dependencyClosure hacked_index
(map fst (componentPackageDeps clbi)) of
Left index' -> index'
Right inf ->
error ("ppHsc2hs: broken closure: " ++ show inf)
isOSX = case buildOS of OSX -> True; _ -> False
isELF = case buildOS of OSX -> False; Windows -> False; AIX -> False; _ -> True;
packageHacks = case compilerFlavor (compiler lbi) of
Expand Down
8 changes: 7 additions & 1 deletion Cabal/Distribution/Types/LocalBuildInfo.hs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,13 @@ data LocalBuildInfo = LocalBuildInfo {
installedPkgs :: InstalledPackageIndex,
-- ^ All the info about the installed packages that the
-- current package depends on (directly or indirectly).
-- Does NOT include internal dependencies.
-- The copy saved on disk does NOT include internal
-- dependencies (because we just don't have enough
-- information at this point to have an
-- 'InstalledPackageInfo' for an internal dep), but we
-- will often update it with the internal dependencies;
-- see for example 'Distribution.Simple.Build.build'.
-- (This admonition doesn't apply for per-component builds.)
pkgDescrFile :: Maybe FilePath,
-- ^ the filename containing the .cabal file, if available
localPkgDescr :: PackageDescription,
Expand Down
2 changes: 1 addition & 1 deletion Cabal/misc/gen-extra-source-files.hs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ whitelistedFiles = [ "ghc", "ghc-pkg", "ghc-7.10", "ghc-pkg-7.10", "ghc-pkg-ghc-

whitelistedExtensionss :: [String]
whitelistedExtensionss = map ('.' : )
[ "hs", "lhs", "c", "sh", "cabal", "hsc", "err", "out", "in", "project" ]
[ "hs", "lhs", "c", "h", "sh", "cabal", "hsc", "err", "out", "in", "project" ]

getOtherModulesFiles :: GenericPackageDescription -> [FilePath]
getOtherModulesFiles gpd = mainModules ++ map fromModuleName otherModules'
Expand Down
Empty file.
12 changes: 12 additions & 0 deletions Cabal/tests/PackageTests/Regression/T2971/p/p.cabal
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
name: p
version: 0.1.0.0
author: Edward Z. Yang
maintainer: [email protected]
build-type: Simple
cabal-version: >=1.10

library
build-depends: base
include-dirs: include
install-includes: T2971test.h
default-language: Haskell2010
3 changes: 3 additions & 0 deletions Cabal/tests/PackageTests/Regression/T2971/q/Bar.hsc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#include <T2971test.h>

main = putStrLn "hello world"
1 change: 1 addition & 0 deletions Cabal/tests/PackageTests/Regression/T2971/q/Foo.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
main = return ()
14 changes: 14 additions & 0 deletions Cabal/tests/PackageTests/Regression/T2971/q/q.cabal
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
name: q
version: 0.1.0.0
author: Edward Z. Yang
maintainer: [email protected]
build-type: Custom
cabal-version: >=1.10

executable bar
main-is: Bar.hs
build-depends: base

executable foo
main-is: Foo.hs
build-depends: base, p
2 changes: 2 additions & 0 deletions Cabal/tests/PackageTests/Regression/T2971a/Main.hsc
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#include <T2971a.h>
main = return ()
16 changes: 16 additions & 0 deletions Cabal/tests/PackageTests/Regression/T2971a/T2971a.cabal
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
name: T2971a
version: 0.1.0.0
author: Edward Z. Yang
maintainer: [email protected]
build-type: Simple
cabal-version: >=1.10

library
build-depends: base
include-dirs: include
install-includes: T2971a.h
default-language: Haskell2010

executable exe
main-is: Main.hs
build-depends: base, T2971a
Empty file.
13 changes: 13 additions & 0 deletions Cabal/tests/PackageTests/Tests.hs
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,19 @@ tests config = do
tc "Regression/T3847" $ do
cabal "configure" ["--disable-tests"]

-- Test that we don't pick up include-dirs from libraries
-- we didn't actually depend on.
tc "Regression/T2971" $ do
withPackageDb $ do
withPackage "p" $ cabal_install []
withPackage "q" $ do
cabal "configure" []
assertOutputContains "T2971test.h"
=<< shouldFail (cabal' "build" [])

-- Test that we pick up include dirs from internal library
tc "Regression/T2971a" $ cabal_build []

-- 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.
Expand Down