From 2e77ba51b509d458c1d997b2948afdb42594e7a0 Mon Sep 17 00:00:00 2001 From: "Edward Z. Yang" Date: Thu, 31 Dec 2015 23:54:11 -0800 Subject: [PATCH] Properly assign component ID/build dir for LibV09 test libraries Cabal's LibV09 support has always been a bit skeevy. The general idea was that a detailed-0.9 test-suite is built as a library and an Cabal-provided stub executable. In particular, the test suite library must be installed to the installed package database so that the executable can be compiled. Old versions of Cabal did something very skeevy here: they installed the test library as a "package", with the same package name as the "test-suite" stanza; furthermore, they built the products into the same directory as the library proper. Consequently, a lot of bad things could happen (both of which I've added tests for): 1. If the name of the test suite and the name of some other package coincide (and have the same version), they will clobber each other. In GHC 7.8 and earlier, this just flat out kills the build, because it will shadow. There's an explicit test to make sure test suites don't conflict with the package name, but you can get unlucky. 2. The test suite library is built into the same directory as the main library, which means that if the test library implements the same module name as something in the main library it will clobber the interface file and badness will ensue. This patchset fixes both of these issues, by (1) giving internal test libraries proper names which are guaranteed to be unique up to Cabal's dependency resolution, and (2) building the test suite library into a separate directory. In doing so, it also lays the groundwork for other types of internal libraries, e.g. #269, as well as extra (invisible) libraries which we may install. For GHC 7.8 and earlier, we follow the reserved namespace convention as per #3017. Signed-off-by: Edward Z. Yang --- Cabal/Cabal.cabal | 10 +- .../Distribution/PackageDescription/Check.hs | 28 +-- Cabal/Distribution/Simple/Build.hs | 32 +-- Cabal/Distribution/Simple/Configure.hs | 197 +++++++++++++----- Cabal/Distribution/Simple/GHC.hs | 6 +- Cabal/Distribution/Simple/LocalBuildInfo.hs | 4 +- Cabal/Distribution/Simple/Register.hs | 7 +- .../DuplicateModuleName.cabal | 25 +++ .../DuplicateModuleName/src/Foo.hs | 12 ++ .../DuplicateModuleName/tests/Foo.hs | 18 ++ .../DuplicateModuleName/tests2/Foo.hs | 18 ++ .../TestNameCollision/child/Child.hs | 2 + .../TestNameCollision/child/child.cabal | 19 ++ .../TestNameCollision/child/tests/Test.hs | 13 ++ .../TestNameCollision/parent/Parent.hs | 1 + .../TestNameCollision/parent/parent.cabal | 13 ++ Cabal/tests/PackageTests/Tests.hs | 16 ++ cabal-install/Distribution/Client/Install.hs | 3 +- 18 files changed, 323 insertions(+), 101 deletions(-) create mode 100644 Cabal/tests/PackageTests/DuplicateModuleName/DuplicateModuleName.cabal create mode 100644 Cabal/tests/PackageTests/DuplicateModuleName/src/Foo.hs create mode 100644 Cabal/tests/PackageTests/DuplicateModuleName/tests/Foo.hs create mode 100644 Cabal/tests/PackageTests/DuplicateModuleName/tests2/Foo.hs create mode 100644 Cabal/tests/PackageTests/TestNameCollision/child/Child.hs create mode 100644 Cabal/tests/PackageTests/TestNameCollision/child/child.cabal create mode 100644 Cabal/tests/PackageTests/TestNameCollision/child/tests/Test.hs create mode 100644 Cabal/tests/PackageTests/TestNameCollision/parent/Parent.hs create mode 100644 Cabal/tests/PackageTests/TestNameCollision/parent/parent.cabal diff --git a/Cabal/Cabal.cabal b/Cabal/Cabal.cabal index d108265061d..170a65862b1 100644 --- a/Cabal/Cabal.cabal +++ b/Cabal/Cabal.cabal @@ -82,6 +82,10 @@ extra-source-files: tests/PackageTests/CMain/my.cabal tests/PackageTests/DeterministicAr/Lib.hs tests/PackageTests/DeterministicAr/my.cabal + tests/PackageTests/DuplicateModuleName/DuplicateModuleName.cabal + tests/PackageTests/DuplicateModuleName/src/Foo.hs + tests/PackageTests/DuplicateModuleName/tests/Foo.hs + tests/PackageTests/DuplicateModuleName/tests2/Foo.hs tests/PackageTests/EmptyLib/empty/empty.cabal tests/PackageTests/Haddock/CPP.hs tests/PackageTests/Haddock/Literate.lhs @@ -112,7 +116,11 @@ extra-source-files: tests/PackageTests/TemplateHaskell/vanilla/Lib.hs tests/PackageTests/TemplateHaskell/vanilla/TH.hs tests/PackageTests/TemplateHaskell/vanilla/my.cabal - tests/PackageTests/Tests.hs + tests/PackageTests/TestNameCollision/child/Child.hs + tests/PackageTests/TestNameCollision/child/child.cabal + tests/PackageTests/TestNameCollision/child/tests/Test.hs + tests/PackageTests/TestNameCollision/parent/Parent.hs + tests/PackageTests/TestNameCollision/parent/parent.cabal tests/PackageTests/TestOptions/TestOptions.cabal tests/PackageTests/TestOptions/test-TestOptions.hs tests/PackageTests/TestStanza/my.cabal diff --git a/Cabal/Distribution/PackageDescription/Check.hs b/Cabal/Distribution/PackageDescription/Check.hs index 96e457fa1a3..95597aa7cb5 100644 --- a/Cabal/Distribution/PackageDescription/Check.hs +++ b/Cabal/Distribution/PackageDescription/Check.hs @@ -66,7 +66,7 @@ import Distribution.Version , asVersionIntervals, UpperBound(..), isNoVersion ) import Distribution.Package ( PackageName(PackageName), packageName, packageVersion - , Dependency(..), pkgName ) + , Dependency(..) ) import Distribution.Text ( display, disp ) @@ -317,14 +317,6 @@ checkTestSuite pkg test = PackageDistInexcusable $ "The package uses a C/C++/obj-C source file for the 'main-is' field. " ++ "To use this feature you must specify 'cabal-version: >= 1.18'." - - -- Test suites might be built as (internal) libraries named after - -- the test suite and thus their names must not clash with the - -- name of the package. - , check libNameClash $ - PackageBuildImpossible $ - "The test suite " ++ testName test - ++ " has the same name as the package." ] where moduleDuplicates = dups $ testModules test @@ -337,13 +329,8 @@ checkTestSuite pkg test = TestSuiteExeV10 _ f -> takeExtension f `notElem` [".hs", ".lhs"] _ -> False - libNameClash = testName test `elem` [ libName - | _lib <- maybeToList (library pkg) - , let PackageName libName = - pkgName (package pkg) ] - checkBenchmark :: PackageDescription -> Benchmark -> [PackageCheck] -checkBenchmark pkg bm = +checkBenchmark _pkg bm = catMaybes [ case benchmarkInterface bm of @@ -369,12 +356,6 @@ checkBenchmark pkg bm = PackageBuildImpossible $ "The 'main-is' field must specify a '.hs' or '.lhs' file " ++ "(even if it is generated by a preprocessor)." - - -- See comment for similar check on test suites. - , check libNameClash $ - PackageBuildImpossible $ - "The benchmark " ++ benchmarkName bm - ++ " has the same name as the package." ] where moduleDuplicates = dups $ benchmarkModules bm @@ -383,11 +364,6 @@ checkBenchmark pkg bm = BenchmarkExeV10 _ f -> takeExtension f `notElem` [".hs", ".lhs"] _ -> False - libNameClash = benchmarkName bm `elem` [ libName - | _lib <- maybeToList (library pkg) - , let PackageName libName = - pkgName (package pkg) ] - -- ------------------------------------------------------------ -- * Additional pure checks -- ------------------------------------------------------------ diff --git a/Cabal/Distribution/Simple/Build.hs b/Cabal/Distribution/Simple/Build.hs index 1ab823710d3..253cdfb739b 100644 --- a/Cabal/Distribution/Simple/Build.hs +++ b/Cabal/Distribution/Simple/Build.hs @@ -35,15 +35,14 @@ import qualified Distribution.Simple.Build.PathsModule as Build.PathsModule import Distribution.Package ( Package(..), PackageName(..), PackageIdentifier(..) - , Dependency(..), thisPackageVersion, packageName - , ComponentId(..), ComponentId(..) ) + , Dependency(..), thisPackageVersion ) import Distribution.Simple.Compiler ( Compiler, CompilerFlavor(..), compilerFlavor , PackageDB(..), PackageDBStack ) import Distribution.PackageDescription ( PackageDescription(..), BuildInfo(..), Library(..), Executable(..) , TestSuite(..), TestSuiteInterface(..), Benchmark(..) - , BenchmarkInterface(..), allBuildInfo, defaultRenaming ) + , BenchmarkInterface(..), allBuildInfo ) import qualified Distribution.InstalledPackageInfo as IPI import qualified Distribution.ModuleName as ModuleName import Distribution.ModuleName (ModuleName) @@ -55,7 +54,7 @@ import Distribution.Simple.BuildTarget import Distribution.Simple.PreProcess ( preprocessComponent, preprocessExtras, PPSuffixHandler ) import Distribution.Simple.LocalBuildInfo - ( LocalBuildInfo(compiler, buildDir, withPackageDB, withPrograms) + ( LocalBuildInfo(compiler, buildDir, withPackageDB, withPrograms, flagAssignment) , Component(..), componentName, getComponent, componentBuildInfo , ComponentLocalBuildInfo(..), pkgEnabledComponents , withComponentsInBuildOrder, componentsInBuildOrder @@ -65,6 +64,7 @@ import Distribution.Simple.Program.Types import Distribution.Simple.Program.Db import Distribution.Simple.BuildPaths ( autogenModulesDir, autogenModuleName, cppHeaderName, exeExtension ) +import Distribution.Simple.Configure (computeComponentId, computeCompatPackageKey) import Distribution.Simple.Register ( registerPackage, inplaceInstalledPackageInfo , doesPackageDBExist, deletePackageDB, createPackageDB ) @@ -249,7 +249,10 @@ buildComponent verbosity numJobs pkg_descr lbi0 suffixes extras <- preprocessExtras comp lbi info verbosity $ "Building test suite " ++ testName test ++ "..." buildLib verbosity numJobs pkg lbi lib libClbi - registerPackage verbosity (compiler lbi) (withPrograms lbi) False + -- NB: need to enable multiple instances here, because on 7.10+ + -- the package name is the same as the library, and we still + -- want the registration to go through. + registerPackage verbosity (compiler lbi) (withPrograms lbi) True (withPackageDB lbi) ipi let ebi = buildInfo exe exe' = exe { buildInfo = addExtraCSources ebi extras } @@ -401,17 +404,22 @@ testSuiteLibV09AsLibAndExe pkg_descr libExposed = True, libBuildInfo = bi } + cid = computeComponentId (package pkg_descr) + (CTestName (testName test)) + (map fst (componentPackageDeps clbi)) + (flagAssignment lbi) + (pkg_name, compat_key) = computeCompatPackageKey + (compiler lbi) (package pkg_descr) + (CTestName (testName test)) cid libClbi = LibComponentLocalBuildInfo { componentPackageDeps = componentPackageDeps clbi , componentPackageRenaming = componentPackageRenaming clbi - , componentId = ComponentId $ display (packageId pkg) - , componentCompatPackageKey = ComponentId $ display (packageId pkg) + , componentId = cid + , componentCompatPackageKey = compat_key , componentExposedModules = [IPI.ExposedModule m Nothing Nothing] } pkg = pkg_descr { - package = (package pkg_descr) { - pkgName = PackageName (testName test) - } + package = (package pkg_descr) { pkgName = pkg_name } , buildDepends = targetBuildDepends $ testBuildInfo test , executables = [] , testSuites = [] @@ -439,9 +447,7 @@ testSuiteLibV09AsLibAndExe pkg_descr : (filter (\(_, x) -> let PackageName name = pkgName x in name == "Cabal" || name == "base") (componentPackageDeps clbi)), - componentPackageRenaming = - Map.insert (packageName ipi) defaultRenaming - (componentPackageRenaming clbi) + componentPackageRenaming = Map.empty } testSuiteLibV09AsLibAndExe _ TestSuite{} _ _ _ _ = error "testSuiteLibV09AsLibAndExe: wrong kind" diff --git a/Cabal/Distribution/Simple/Configure.hs b/Cabal/Distribution/Simple/Configure.hs index 86ac7576958..721f6a64e56 100644 --- a/Cabal/Distribution/Simple/Configure.hs +++ b/Cabal/Distribution/Simple/Configure.hs @@ -35,6 +35,7 @@ module Distribution.Simple.Configure (configure, maybeGetPersistBuildConfig, findDistPref, findDistPrefOrDefault, computeComponentId, + computeCompatPackageKey, localBuildInfoFile, getInstalledPackages, getInstalledPackagesMonitorFiles, @@ -131,7 +132,7 @@ import Control.Exception ( Exception, evaluate, throw, throwIO, try ) import Control.Exception ( ErrorCall ) import Control.Monad - ( liftM, when, unless, foldM, filterM ) + ( liftM, when, unless, foldM, filterM, mplus ) import Distribution.Compat.Binary ( decodeOrFailIO, encode ) import GHC.Fingerprint ( Fingerprint(..), fingerprintString ) import Data.ByteString.Lazy (ByteString) @@ -152,7 +153,6 @@ import Data.Traversable import Data.Typeable import Data.Char ( chr, isAlphaNum ) import Numeric ( showIntAtBase ) -import Data.Bits ( shift ) import System.Directory ( doesFileExist, createDirectoryIfMissing, getTemporaryDirectory ) import System.FilePath @@ -405,11 +405,13 @@ configure (pkg_descr0, pbi) cfg = do (configDependencies cfg) installedPackageSet - -- The resolved package description, that does not contain any - -- conditionals, because we have have an assignment for every - -- flag, either picking them ourselves using a simple naive - -- algorithm, or having them be passed to us by - -- 'configConfigurationsFlags') + -- pkg_descr: The resolved package description, that does not contain any + -- conditionals, because we have have an assignment for + -- every flag, either picking them ourselves using a + -- simple naive algorithm, or having them be passed to + -- us by 'configConfigurationsFlags') + -- flags: The 'FlagAssignment' that the conditionals were + -- resolved with. -- -- NB: Why doesn't finalizing a package also tell us what the -- dependencies are (e.g. when we run the naive algorithm, @@ -418,7 +420,7 @@ configure (pkg_descr0, pbi) cfg = do -- if the flags are all chosen for us, this step is a simple -- matter of flattening according to that assignment. It's -- cleaner to then configure the dependencies afterwards. - pkg_descr + (pkg_descr, flags) <- configureFinalizedPackage verbosity cfg allConstraints (dependencySatisfiable @@ -642,6 +644,7 @@ configure (pkg_descr0, pbi) cfg = do let lbi = LocalBuildInfo { configFlags = cfg', + flagAssignment = flags, extraConfigArgs = [], -- Currently configure does not -- take extra args, but if it -- did they would go here. @@ -845,7 +848,7 @@ configureFinalizedPackage -> Compiler -> Platform -> GenericPackageDescription - -> IO PackageDescription + -> IO (PackageDescription, FlagAssignment) configureFinalizedPackage verbosity cfg allConstraints satisfies comp compPlatform pkg_descr0 = do let enableTest t = t { testEnabled = fromFlag (configTests cfg) } @@ -883,7 +886,7 @@ configureFinalizedPackage verbosity cfg ++ intercalate ", " [ name ++ "=" ++ display value | (FlagName name, value) <- flags ] - return pkg_descr + return (pkg_descr, flags) where addExtraIncludeLibDirs pkg_descr = let extraBi = mempty { extraLibDirs = configExtraLibDirs cfg @@ -1453,13 +1456,13 @@ reportComponentCycle cnames = -- | This method computes a default, "good enough" 'ComponentId' -- for a package. The intent is that cabal-install (or the user) will -- specify a more detailed IPID via the @--ipid@ flag if necessary. -computeComponentId :: PackageDescription +computeComponentId :: PackageIdentifier -> ComponentName -- TODO: careful here! -> [ComponentId] -- IPIDs of the component dependencies -> FlagAssignment - -> IO ComponentId -computeComponentId pkg_descr cname dep_ipids flagAssignment = do + -> ComponentId +computeComponentId pid cname dep_ipids flagAssignment = do -- show is found to be faster than intercalate and then replacement of -- special character used in intercalating. We cannot simply hash by -- doubly concating list, as it just flatten out the nested list, so @@ -1468,11 +1471,11 @@ computeComponentId pkg_descr cname dep_ipids flagAssignment = do -- For safety, include the package + version here -- for GHC 7.10, where just the hash is used as -- the package key - (display (package pkg_descr)) + (display pid) ++ (show $ dep_ipids) ++ show flagAssignment - return . ComponentId $ - display (package pkg_descr) + ComponentId $ + display pid ++ "-" ++ hash ++ (case cname of CLibName -> "" @@ -1481,16 +1484,118 @@ computeComponentId pkg_descr cname dep_ipids flagAssignment = do CExeName s -> "-" ++ s ++ ".exe" CTestName s -> "-" ++ s ++ ".test" CBenchName s -> "-" ++ s ++ ".bench") + +hashToBase62 :: String -> String +hashToBase62 s = showFingerprint $ fingerprintString s where + showIntAtBase62 x = showIntAtBase 62 representBase62 x "" representBase62 x | x < 10 = chr (48 + x) | x < 36 = chr (65 + x - 10) | x < 62 = chr (97 + x - 36) | otherwise = '@' - fpToInteger (Fingerprint a b) = - toInteger a * (shift (1 :: Integer) 64) + toInteger b - hashToBase62 s = showIntAtBase 62 representBase62 - (fpToInteger $ fingerprintString s) "" + showFingerprint (Fingerprint a b) = showIntAtBase62 a ++ showIntAtBase62 b + +-- | In GHC 8.0, the string we pass to GHC to use for symbol +-- names for a package can be an arbitrary, IPID-compatible string. +-- However, prior to GHC 8.0 there are some restrictions on what +-- format this string can be (due to how ghc-pkg parsed the key): +-- +-- 1. In GHC 7.10, the string had either be of the form +-- foo_ABCD, where foo is a non-semantic alphanumeric/hyphenated +-- prefix and ABCD is two base-64 encoded 64-bit integers, +-- or a GHC 7.8 style identifier. +-- +-- 2. In GHC 7.8, the string had to be a valid package identifier +-- like foo-0.1. +-- +-- So, the problem is that Cabal, in general, has a general IPID, +-- but needs to figure out a package key / package ID that the +-- old ghc-pkg will actually accept. But there's an EVERY WORSE +-- problem: if ghc-pkg decides to parse an identifier foo-0.1-xxx +-- as if it were a package identifier, which means it will SILENTLY +-- DROP the "xxx" (because it's a tag, and Cabal does not allow tags.) +-- So we must CONNIVE to ensure that we don't pick something that +-- looks like this. +-- +-- So this function attempts to define a mapping into the old formats. +-- +-- The mapping for GHC 7.8 and before: +-- +-- * For CLibName, we unconditionally use the 'PackageIdentifier'. +-- +-- * For sub-components, we create a new 'PackageIdentifier' which +-- is encoded in the following way. The test suite "qux" in package +-- "foobar-0.2" gets this package identifier "z-foobar-z-test-qux-0.2". +-- These package IDs have the form: +-- +-- cpid ::= "z-" package-id "-z-" component-type "-" component-name +-- component-type ::= "test" | "bench" | "exe" | "lib" +-- package-id and component-name have "-" ( "z" + ) "-" +-- segments encoded by adding an extra "z". +-- +-- The mapping for GHC 7.10: +-- +-- * For CLibName: +-- If the IPID is of the form foo-0.1-ABCDEF where foo_ABCDEF would +-- validly parse as a package key, we pass "ABCDEF". (NB: not +-- all hashes parse this way, because GHC 7.10 mandated that +-- these hashes be two base-62 encoded 64 bit integers), +-- but hashes that Cabal generated using 'computeComponentId' +-- are guaranteed to have this form. +-- +-- If it is not of this form, we rehash the IPID into the +-- correct form and pass that. +-- +-- * For sub-components, we rehash the IPID into the correct format +-- and pass that. +-- +computeCompatPackageKey + :: Compiler + -> PackageIdentifier + -> ComponentName + -> ComponentId + -> (PackageName, ComponentId) +computeCompatPackageKey comp pid cname cid@(ComponentId str) + | not (packageKeySupported comp) = + -- NB: the package ID in the database entry has to follow this + let zdashcode s = go s (Nothing :: Maybe Int) [] + where go [] _ r = reverse r + go ('-':z) (Just n) r | n > 0 = go z (Just 0) ('-':'z':r) + go ('-':z) _ r = go z (Just 0) ('-':r) + go ('z':z) (Just n) r = go z (Just (n+1)) ('z':r) + go (c:z) _ r = go z Nothing (c:r) + cname_str = case cname of + CLibName -> error "computeCompatPackageKey" + CTestName n -> "-z-test-" ++ zdashcode n + CBenchName n -> "-z-bench-" ++ zdashcode n + CExeName n -> "-z-exe-" ++ zdashcode n + package_name + | cname == CLibName = pkgName pid + | otherwise = PackageName $ "z-" ++ zdashcode (display pid) + ++ zdashcode cname_str + old_style_key + | cname == CLibName = display pid + | otherwise = display package_name ++ "-" + ++ display (pkgVersion pid) + in (package_name, ComponentId old_style_key) + | not (unifiedIPIDRequired comp) = + let mb_verbatim_key + = case simpleParse str :: Maybe PackageId of + -- Something like 'foo-0.1', use it verbatim. + -- (NB: hash tags look like tags, so they are parsed, + -- so the extra equality check tests if a tag was dropped.) + Just pid0 | display pid0 == str -> Just str + _ -> Nothing + mb_truncated_key + = let cand = reverse (takeWhile isAlphaNum (reverse str)) + in if length cand == 22 && all isAlphaNum cand + then Just cand + else Nothing + rehashed_key = hashToBase62 str + in (pkgName pid, ComponentId $ fromMaybe rehashed_key + (mb_verbatim_key `mplus` mb_truncated_key)) + | otherwise = (pkgName pid, cid) mkComponentsLocalBuildInfo :: ConfigFlags -> Compiler @@ -1508,37 +1613,21 @@ mkComponentsLocalBuildInfo cfg comp installedPackages pkg_descr internalPkgDeps externalPkgDeps holePkgDeps hole_insts graph flagAssignment = do -- Pre-compute library hash so we can setup internal deps - lib_hash@(ComponentId str) <- - -- TODO configIPID should have name changed - case configIPID cfg of - Flag lib_hash0 -> - -- Hack to reuse install dirs machinery - -- NB: no real IPID available at this point - let env = packageTemplateEnv (package pkg_descr) (ComponentId "") - str = fromPathTemplate (substPathTemplate env (toPathTemplate lib_hash0)) - in return (ComponentId str) - _ -> - computeComponentId pkg_descr CLibName (getDeps CLibName) flagAssignment - let extractCandidateCompatKey s - = case simpleParse s :: Maybe PackageId of - -- Something like 'foo-0.1', use it verbatim. - -- (NB: hash tags look like tags, so they are parsed, - -- so the extra equality check tests if a tag was dropped.) - Just pid | display pid == s -> s - -- Something like 'foo-0.1-XXX', take the stuff at the end. - -- TODO this won't work with component stuff - _ -> reverse (takeWhile isAlphaNum (reverse s)) - cand_compat_key = ComponentId (extractCandidateCompatKey str) - old_style_key = ComponentId (display (package pkg_descr)) - best_key = ComponentId str - compat_key = - if packageKeySupported comp - then if unifiedIPIDRequired comp - then best_key - else cand_compat_key - else old_style_key + -- TODO configIPID should have name changed + let cid = case configIPID cfg of + Flag cid0 -> + -- Hack to reuse install dirs machinery + -- NB: no real IPID available at this point + let env = packageTemplateEnv (package pkg_descr) + (ComponentId "") + str = fromPathTemplate + (substPathTemplate env (toPathTemplate cid0)) + in ComponentId str + _ -> + computeComponentId (package pkg_descr) CLibName (getDeps CLibName) flagAssignment + (_, compat_key) = computeCompatPackageKey comp (package pkg_descr) CLibName cid sequence - [ do clbi <- componentLocalBuildInfo lib_hash compat_key c + [ do clbi <- componentLocalBuildInfo cid compat_key c return (componentName c, clbi, cdeps) | (c, cdeps) <- graph ] where @@ -1553,7 +1642,7 @@ mkComponentsLocalBuildInfo cfg comp installedPackages pkg_descr -- we just take the subset for the package names this component -- needs. Note, this only works because we cannot yet depend on two -- versions of the same package. - componentLocalBuildInfo lib_hash compat_key component = + componentLocalBuildInfo cid compat_key component = case component of CLib lib -> do let exports = map (\n -> Installed.ExposedModule n Nothing Nothing) @@ -1565,7 +1654,7 @@ mkComponentsLocalBuildInfo cfg comp installedPackages pkg_descr (PD.exposedSignatures lib) let mb_reexports = resolveModuleReexports installedPackages (packageId pkg_descr) - lib_hash + cid externalPkgDeps lib reexports <- case mb_reexports of Left problems -> reportModuleReexportProblems problems @@ -1573,7 +1662,7 @@ mkComponentsLocalBuildInfo cfg comp installedPackages pkg_descr return LibComponentLocalBuildInfo { componentPackageDeps = cpds, - componentId = lib_hash, + componentId = cid, componentCompatPackageKey = compat_key, componentPackageRenaming = cprns, componentExposedModules = exports ++ reexports ++ esigs @@ -1600,7 +1689,7 @@ mkComponentsLocalBuildInfo cfg comp installedPackages pkg_descr then dedup $ [ (Installed.installedComponentId pkg, packageId pkg) | pkg <- selectSubset bi externalPkgDeps ] - ++ [ (lib_hash, pkgid) + ++ [ (cid, pkgid) | pkgid <- selectSubset bi internalPkgDeps ] else [ (Installed.installedComponentId pkg, packageId pkg) | pkg <- externalPkgDeps ] diff --git a/Cabal/Distribution/Simple/GHC.hs b/Cabal/Distribution/Simple/GHC.hs index f3a32fd85c5..5f8c26189c1 100644 --- a/Cabal/Distribution/Simple/GHC.hs +++ b/Cabal/Distribution/Simple/GHC.hs @@ -69,7 +69,7 @@ import Distribution.Simple.PackageIndex (InstalledPackageIndex) import qualified Distribution.Simple.PackageIndex as PackageIndex import Distribution.Simple.LocalBuildInfo ( LocalBuildInfo(..), ComponentLocalBuildInfo(..) - , absoluteInstallDirs, depLibraryPaths ) + , absoluteInstallDirs, depLibraryPaths, localComponentId ) import qualified Distribution.Simple.Hpc as Hpc import Distribution.Simple.InstallDirs hiding ( absoluteInstallDirs ) import Distribution.Simple.BuildPaths @@ -498,7 +498,9 @@ buildOrReplLib :: Bool -> Verbosity -> Cabal.Flag (Maybe Int) -> Library -> ComponentLocalBuildInfo -> IO () buildOrReplLib forRepl verbosity numJobs pkg_descr lbi lib clbi = do let libName = componentId clbi - libTargetDir = buildDir lbi + libTargetDir + | componentId clbi == localComponentId lbi = buildDir lbi + | otherwise = buildDir lbi display libName whenVanillaLib forceVanilla = when (forceVanilla || withVanillaLib lbi) whenProfLib = when (withProfLib lbi) diff --git a/Cabal/Distribution/Simple/LocalBuildInfo.hs b/Cabal/Distribution/Simple/LocalBuildInfo.hs index 1e3f0cb2f5b..f34cd8c4b00 100644 --- a/Cabal/Distribution/Simple/LocalBuildInfo.hs +++ b/Cabal/Distribution/Simple/LocalBuildInfo.hs @@ -67,7 +67,7 @@ import Distribution.InstalledPackageInfo (InstalledPackageInfo) import Distribution.PackageDescription ( PackageDescription(..), withLib, Library(libBuildInfo), withExe , Executable(exeName, buildInfo), withTest, TestSuite(..) - , BuildInfo(buildable), Benchmark(..), ModuleRenaming(..) ) + , BuildInfo(buildable), Benchmark(..), ModuleRenaming(..), FlagAssignment ) import qualified Distribution.InstalledPackageInfo as Installed import Distribution.Package ( PackageId, Package(..), ComponentId(..) @@ -104,6 +104,8 @@ data LocalBuildInfo = LocalBuildInfo { configFlags :: ConfigFlags, -- ^ Options passed to the configuration step. -- Needed to re-run configuration when .cabal is out of date + flagAssignment :: FlagAssignment, + -- ^ The final set of flags which were picked for this package extraConfigArgs :: [String], -- ^ Extra args on the command line for the configuration step. -- Needed to re-run configuration when .cabal is out of date diff --git a/Cabal/Distribution/Simple/Register.hs b/Cabal/Distribution/Simple/Register.hs index 44aee2db631..6e82a0e9bbe 100644 --- a/Cabal/Distribution/Simple/Register.hs +++ b/Cabal/Distribution/Simple/Register.hs @@ -43,7 +43,7 @@ module Distribution.Simple.Register ( import Distribution.Simple.LocalBuildInfo ( LocalBuildInfo(..), ComponentLocalBuildInfo(..) , ComponentName(..), getComponentLocalBuildInfo - , InstallDirs(..), absoluteInstallDirs ) + , InstallDirs(..), absoluteInstallDirs, localComponentId ) import Distribution.Simple.BuildPaths (haddockName) import qualified Distribution.Simple.GHC as GHC @@ -395,9 +395,12 @@ inplaceInstalledPackageInfo inplaceDir distPref pkg abi_hash lib lbi clbi = pkg abi_hash lib lbi clbi installDirs where adjustRelativeIncludeDirs = map (inplaceDir ) + libTargetDir + | componentId clbi == localComponentId lbi = buildDir lbi + | otherwise = buildDir lbi display (componentId clbi) installDirs = (absoluteInstallDirs pkg lbi NoCopyDest) { - libdir = inplaceDir buildDir lbi, + libdir = inplaceDir libTargetDir, datadir = inplaceDir dataDir pkg, docdir = inplaceDocdir, htmldir = inplaceHtmldir, diff --git a/Cabal/tests/PackageTests/DuplicateModuleName/DuplicateModuleName.cabal b/Cabal/tests/PackageTests/DuplicateModuleName/DuplicateModuleName.cabal new file mode 100644 index 00000000000..d3e326a3be7 --- /dev/null +++ b/Cabal/tests/PackageTests/DuplicateModuleName/DuplicateModuleName.cabal @@ -0,0 +1,25 @@ +name: DuplicateModuleName +version: 0.1.0.0 +license: BSD3 +author: Edward Z. Yang +maintainer: ezyang@cs.stanford.edu +build-type: Simple +cabal-version: >=1.10 + +library + exposed-modules: Foo + hs-source-dirs: src + build-depends: base, Cabal + default-language: Haskell2010 + +test-suite foo + type: detailed-0.9 + test-module: Foo + hs-source-dirs: tests + build-depends: base, Cabal, DuplicateModuleName + +test-suite foo2 + type: detailed-0.9 + test-module: Foo + hs-source-dirs: tests2 + build-depends: base, Cabal, DuplicateModuleName diff --git a/Cabal/tests/PackageTests/DuplicateModuleName/src/Foo.hs b/Cabal/tests/PackageTests/DuplicateModuleName/src/Foo.hs new file mode 100644 index 00000000000..a964bac5203 --- /dev/null +++ b/Cabal/tests/PackageTests/DuplicateModuleName/src/Foo.hs @@ -0,0 +1,12 @@ +module Foo where + +import Distribution.TestSuite + +tests :: IO [Test] +tests = return [Test $ TestInstance + { run = return (Finished (Fail "A")) + , name = "test A" + , tags = [] + , options = [] + , setOption = \_ _-> Left "No Options" + }] diff --git a/Cabal/tests/PackageTests/DuplicateModuleName/tests/Foo.hs b/Cabal/tests/PackageTests/DuplicateModuleName/tests/Foo.hs new file mode 100644 index 00000000000..c6c894ffc58 --- /dev/null +++ b/Cabal/tests/PackageTests/DuplicateModuleName/tests/Foo.hs @@ -0,0 +1,18 @@ +{-# LANGUAGE PackageImports #-} +module Foo where + +import Distribution.TestSuite +import qualified "DuplicateModuleName" Foo as T + +tests :: IO [Test] +tests = do + r <- T.tests + return $ [Test $ TestInstance + { run = return (Finished (Fail "B")) + , name = "test B" + , tags = [] + , options = [] + , setOption = \_ _-> Left "No Options" + }] ++ r + +this_is_test = True diff --git a/Cabal/tests/PackageTests/DuplicateModuleName/tests2/Foo.hs b/Cabal/tests/PackageTests/DuplicateModuleName/tests2/Foo.hs new file mode 100644 index 00000000000..68836baa8e3 --- /dev/null +++ b/Cabal/tests/PackageTests/DuplicateModuleName/tests2/Foo.hs @@ -0,0 +1,18 @@ +{-# LANGUAGE PackageImports #-} +module Foo where + +import Distribution.TestSuite +import qualified "DuplicateModuleName" Foo as T + +tests :: IO [Test] +tests = do + r <- T.tests + return $ [Test $ TestInstance + { run = return (Finished (Fail "C")) + , name = "test C" + , tags = [] + , options = [] + , setOption = \_ _-> Left "No Options" + }] ++ r + +this_is_test2 = True diff --git a/Cabal/tests/PackageTests/TestNameCollision/child/Child.hs b/Cabal/tests/PackageTests/TestNameCollision/child/Child.hs new file mode 100644 index 00000000000..c068dcf8b74 --- /dev/null +++ b/Cabal/tests/PackageTests/TestNameCollision/child/Child.hs @@ -0,0 +1,2 @@ +module Child where +import Parent diff --git a/Cabal/tests/PackageTests/TestNameCollision/child/child.cabal b/Cabal/tests/PackageTests/TestNameCollision/child/child.cabal new file mode 100644 index 00000000000..af3a7a5223f --- /dev/null +++ b/Cabal/tests/PackageTests/TestNameCollision/child/child.cabal @@ -0,0 +1,19 @@ +name: child +version: 0.1 +description: This defines the colliding detailed-0.9 test suite +license: BSD3 +author: Edward Z. Yang +maintainer: ezyang@cs.stanford.edu +build-type: Simple +cabal-version: >=1.10 + +library + exposed-modules: Child + build-depends: base, parent + default-language: Haskell2010 + +test-suite parent + type: detailed-0.9 + test-module: Test + hs-source-dirs: tests + build-depends: base, Cabal, child diff --git a/Cabal/tests/PackageTests/TestNameCollision/child/tests/Test.hs b/Cabal/tests/PackageTests/TestNameCollision/child/tests/Test.hs new file mode 100644 index 00000000000..f2325e79982 --- /dev/null +++ b/Cabal/tests/PackageTests/TestNameCollision/child/tests/Test.hs @@ -0,0 +1,13 @@ +module Test where + +import Distribution.TestSuite +import Child + +tests :: IO [Test] +tests = return $ [Test $ TestInstance + { run = return (Finished Pass) + , name = "test" + , tags = [] + , options = [] + , setOption = \_ _-> Left "No Options" + }] diff --git a/Cabal/tests/PackageTests/TestNameCollision/parent/Parent.hs b/Cabal/tests/PackageTests/TestNameCollision/parent/Parent.hs new file mode 100644 index 00000000000..2a93f378329 --- /dev/null +++ b/Cabal/tests/PackageTests/TestNameCollision/parent/Parent.hs @@ -0,0 +1 @@ +module Parent where diff --git a/Cabal/tests/PackageTests/TestNameCollision/parent/parent.cabal b/Cabal/tests/PackageTests/TestNameCollision/parent/parent.cabal new file mode 100644 index 00000000000..ea1d4132179 --- /dev/null +++ b/Cabal/tests/PackageTests/TestNameCollision/parent/parent.cabal @@ -0,0 +1,13 @@ +name: parent +version: 0.1 +description: This package is what the test suite is going to collide with +license: BSD3 +author: Edward Z. Yang +maintainer: ezyang@cs.stanford.edu +build-type: Simple +cabal-version: >=1.10 + +library + exposed-modules: Parent + build-depends: base + default-language: Haskell2010 diff --git a/Cabal/tests/PackageTests/Tests.hs b/Cabal/tests/PackageTests/Tests.hs index 88307b0a8b0..c2bd8d3ae6e 100644 --- a/Cabal/tests/PackageTests/Tests.hs +++ b/Cabal/tests/PackageTests/Tests.hs @@ -202,6 +202,22 @@ tests config = assertFailure $ "cabal has not calculated different Installed " ++ "package ID when source is changed." + , tc "DuplicateModuleName" $ do + cabal_build ["--enable-tests"] + r1 <- shouldFail $ cabal' "test" ["foo"] + assertOutputContains "test B" r1 + assertOutputContains "test A" r1 + r2 <- shouldFail $ cabal' "test" ["foo2"] + assertOutputContains "test C" r2 + assertOutputContains "test A" r2 + + , tc "TestNameCollision" $ do + withPackageDb $ do + withPackage "parent" $ cabal_install [] + withPackage "child" $ do + cabal_build ["--enable-tests"] + cabal "test" [] + ] where -- Shared test function for BuildDeps/InternalLibrary* tests. diff --git a/cabal-install/Distribution/Client/Install.hs b/cabal-install/Distribution/Client/Install.hs index cefb978c053..bb23bad677e 100644 --- a/cabal-install/Distribution/Client/Install.hs +++ b/cabal-install/Distribution/Client/Install.hs @@ -1397,8 +1397,7 @@ installUnpackedPackage verbosity buildLimit installLock numJobs -- Compute the IPID let flags (ReadyPackage (ConfiguredPackage _ x _ _) _) = x - ipid <- inDir workingDir - $ Configure.computeComponentId pkg CLibName + ipid = Configure.computeComponentId (PackageDescription.package pkg) CLibName (CD.libraryDeps (depends rpkg)) (flags rpkg) -- Make sure that we pass --libsubdir etc to 'setup configure' (necessary if