Skip to content

Commit e8e47f6

Browse files
committed
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. haskell#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 haskell#3017. Signed-off-by: Edward Z. Yang <[email protected]>
1 parent 3ea7566 commit e8e47f6

File tree

18 files changed

+325
-103
lines changed

18 files changed

+325
-103
lines changed

Cabal/Cabal.cabal

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,10 @@ extra-source-files:
8282
tests/PackageTests/CMain/my.cabal
8383
tests/PackageTests/DeterministicAr/Lib.hs
8484
tests/PackageTests/DeterministicAr/my.cabal
85+
tests/PackageTests/DuplicateModuleName/DuplicateModuleName.cabal
86+
tests/PackageTests/DuplicateModuleName/src/Foo.hs
87+
tests/PackageTests/DuplicateModuleName/tests/Foo.hs
88+
tests/PackageTests/DuplicateModuleName/tests2/Foo.hs
8589
tests/PackageTests/EmptyLib/empty/empty.cabal
8690
tests/PackageTests/Haddock/CPP.hs
8791
tests/PackageTests/Haddock/Literate.lhs
@@ -112,7 +116,11 @@ extra-source-files:
112116
tests/PackageTests/TemplateHaskell/vanilla/Lib.hs
113117
tests/PackageTests/TemplateHaskell/vanilla/TH.hs
114118
tests/PackageTests/TemplateHaskell/vanilla/my.cabal
115-
tests/PackageTests/Tests.hs
119+
tests/PackageTests/TestNameCollision/child/Child.hs
120+
tests/PackageTests/TestNameCollision/child/child.cabal
121+
tests/PackageTests/TestNameCollision/child/tests/Test.hs
122+
tests/PackageTests/TestNameCollision/parent/Parent.hs
123+
tests/PackageTests/TestNameCollision/parent/parent.cabal
116124
tests/PackageTests/TestOptions/TestOptions.cabal
117125
tests/PackageTests/TestOptions/test-TestOptions.hs
118126
tests/PackageTests/TestStanza/my.cabal

Cabal/Distribution/PackageDescription/Check.hs

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ import Distribution.Version
6666
, asVersionIntervals, UpperBound(..), isNoVersion )
6767
import Distribution.Package
6868
( PackageName(PackageName), packageName, packageVersion
69-
, Dependency(..), pkgName )
69+
, Dependency(..) )
7070

7171
import Distribution.Text
7272
( display, disp )
@@ -317,14 +317,6 @@ checkTestSuite pkg test =
317317
PackageDistInexcusable $
318318
"The package uses a C/C++/obj-C source file for the 'main-is' field. "
319319
++ "To use this feature you must specify 'cabal-version: >= 1.18'."
320-
321-
-- Test suites might be built as (internal) libraries named after
322-
-- the test suite and thus their names must not clash with the
323-
-- name of the package.
324-
, check libNameClash $
325-
PackageBuildImpossible $
326-
"The test suite " ++ testName test
327-
++ " has the same name as the package."
328320
]
329321
where
330322
moduleDuplicates = dups $ testModules test
@@ -337,13 +329,8 @@ checkTestSuite pkg test =
337329
TestSuiteExeV10 _ f -> takeExtension f `notElem` [".hs", ".lhs"]
338330
_ -> False
339331

340-
libNameClash = testName test `elem` [ libName
341-
| _lib <- maybeToList (library pkg)
342-
, let PackageName libName =
343-
pkgName (package pkg) ]
344-
345332
checkBenchmark :: PackageDescription -> Benchmark -> [PackageCheck]
346-
checkBenchmark pkg bm =
333+
checkBenchmark _pkg bm =
347334
catMaybes [
348335

349336
case benchmarkInterface bm of
@@ -369,12 +356,6 @@ checkBenchmark pkg bm =
369356
PackageBuildImpossible $
370357
"The 'main-is' field must specify a '.hs' or '.lhs' file "
371358
++ "(even if it is generated by a preprocessor)."
372-
373-
-- See comment for similar check on test suites.
374-
, check libNameClash $
375-
PackageBuildImpossible $
376-
"The benchmark " ++ benchmarkName bm
377-
++ " has the same name as the package."
378359
]
379360
where
380361
moduleDuplicates = dups $ benchmarkModules bm
@@ -383,11 +364,6 @@ checkBenchmark pkg bm =
383364
BenchmarkExeV10 _ f -> takeExtension f `notElem` [".hs", ".lhs"]
384365
_ -> False
385366

386-
libNameClash = benchmarkName bm `elem` [ libName
387-
| _lib <- maybeToList (library pkg)
388-
, let PackageName libName =
389-
pkgName (package pkg) ]
390-
391367
-- ------------------------------------------------------------
392368
-- * Additional pure checks
393369
-- ------------------------------------------------------------

Cabal/Distribution/Simple/Build.hs

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,14 @@ import qualified Distribution.Simple.Build.PathsModule as Build.PathsModule
3535

3636
import Distribution.Package
3737
( Package(..), PackageName(..), PackageIdentifier(..)
38-
, Dependency(..), thisPackageVersion, packageName
39-
, ComponentId(..), ComponentId(..) )
38+
, Dependency(..), thisPackageVersion )
4039
import Distribution.Simple.Compiler
4140
( Compiler, CompilerFlavor(..), compilerFlavor
4241
, PackageDB(..), PackageDBStack )
4342
import Distribution.PackageDescription
4443
( PackageDescription(..), BuildInfo(..), Library(..), Executable(..)
4544
, TestSuite(..), TestSuiteInterface(..), Benchmark(..)
46-
, BenchmarkInterface(..), allBuildInfo, defaultRenaming )
45+
, BenchmarkInterface(..), allBuildInfo )
4746
import qualified Distribution.InstalledPackageInfo as IPI
4847
import qualified Distribution.ModuleName as ModuleName
4948
import Distribution.ModuleName (ModuleName)
@@ -55,7 +54,7 @@ import Distribution.Simple.BuildTarget
5554
import Distribution.Simple.PreProcess
5655
( preprocessComponent, preprocessExtras, PPSuffixHandler )
5756
import Distribution.Simple.LocalBuildInfo
58-
( LocalBuildInfo(compiler, buildDir, withPackageDB, withPrograms)
57+
( LocalBuildInfo(compiler, buildDir, withPackageDB, withPrograms, flagAssignment)
5958
, Component(..), componentName, getComponent, componentBuildInfo
6059
, ComponentLocalBuildInfo(..), pkgEnabledComponents
6160
, withComponentsInBuildOrder, componentsInBuildOrder
@@ -65,6 +64,7 @@ import Distribution.Simple.Program.Types
6564
import Distribution.Simple.Program.Db
6665
import Distribution.Simple.BuildPaths
6766
( autogenModulesDir, autogenModuleName, cppHeaderName, exeExtension )
67+
import Distribution.Simple.Configure (computeComponentId, computeCompatPackageKey)
6868
import Distribution.Simple.Register
6969
( registerPackage, inplaceInstalledPackageInfo
7070
, doesPackageDBExist, deletePackageDB, createPackageDB )
@@ -249,7 +249,10 @@ buildComponent verbosity numJobs pkg_descr lbi0 suffixes
249249
extras <- preprocessExtras comp lbi
250250
info verbosity $ "Building test suite " ++ testName test ++ "..."
251251
buildLib verbosity numJobs pkg lbi lib libClbi
252-
registerPackage verbosity (compiler lbi) (withPrograms lbi) False
252+
-- NB: need to enable multiple instances here, because on 7.10+
253+
-- the package name is the same as the library, and we still
254+
-- want the registration to go through.
255+
registerPackage verbosity (compiler lbi) (withPrograms lbi) True
253256
(withPackageDB lbi) ipi
254257
let ebi = buildInfo exe
255258
exe' = exe { buildInfo = addExtraCSources ebi extras }
@@ -401,17 +404,22 @@ testSuiteLibV09AsLibAndExe pkg_descr
401404
libExposed = True,
402405
libBuildInfo = bi
403406
}
407+
cid = computeComponentId (package pkg_descr)
408+
(CTestName (testName test))
409+
(map fst (componentPackageDeps clbi))
410+
(flagAssignment lbi)
411+
(pkg_name, compat_key) = computeCompatPackageKey
412+
(compiler lbi) (package pkg_descr)
413+
(CTestName (testName test)) cid
404414
libClbi = LibComponentLocalBuildInfo
405415
{ componentPackageDeps = componentPackageDeps clbi
406416
, componentPackageRenaming = componentPackageRenaming clbi
407-
, componentId = ComponentId $ display (packageId pkg)
408-
, componentCompatPackageKey = ComponentId $ display (packageId pkg)
417+
, componentId = cid
418+
, componentCompatPackageKey = compat_key
409419
, componentExposedModules = [IPI.ExposedModule m Nothing Nothing]
410420
}
411421
pkg = pkg_descr {
412-
package = (package pkg_descr) {
413-
pkgName = PackageName (testName test)
414-
}
422+
package = (package pkg_descr) { pkgName = pkg_name }
415423
, buildDepends = targetBuildDepends $ testBuildInfo test
416424
, executables = []
417425
, testSuites = []
@@ -439,9 +447,7 @@ testSuiteLibV09AsLibAndExe pkg_descr
439447
: (filter (\(_, x) -> let PackageName name = pkgName x
440448
in name == "Cabal" || name == "base")
441449
(componentPackageDeps clbi)),
442-
componentPackageRenaming =
443-
Map.insert (packageName ipi) defaultRenaming
444-
(componentPackageRenaming clbi)
450+
componentPackageRenaming = Map.empty
445451
}
446452
testSuiteLibV09AsLibAndExe _ TestSuite{} _ _ _ _ = error "testSuiteLibV09AsLibAndExe: wrong kind"
447453

@@ -471,7 +477,7 @@ createInternalPackageDB :: Verbosity -> LocalBuildInfo -> FilePath
471477
createInternalPackageDB verbosity lbi distPref = do
472478
existsAlready <- doesPackageDBExist dbPath
473479
when existsAlready $ deletePackageDB dbPath
474-
createPackageDB verbosity (compiler lbi) (withPrograms lbi) True dbPath
480+
createPackageDB verbosity (compiler lbi) (withPrograms lbi) False dbPath
475481
return (SpecificPackageDB dbPath)
476482
where
477483
dbPath = case compilerFlavor (compiler lbi) of

0 commit comments

Comments
 (0)