Skip to content

Commit

Permalink
hpc: Don't cover non-dependency libraries
Browse files Browse the repository at this point in the history
We were passing the flag --coverage-for for every library in the package
when building the testsuites.

However, if a particular testsuite doesn't depend on one of the packages
passed with --coverage-for we would crash.

Since we look for the unit ids of all packages given in --coverage-for in the
package database (to find hpc artifacts), if the testsuite builds before
the library this lookup would fail.

The fix is easy: don't pass --coverage-for=<lib> to <testsuite> if
<testsuite> doesn't depend on <lib>. If <lib> is an indirect dependency
of <testsuite> it'll no longer be covered by HPC, but this is a weird
behaviour that I doubt anyone relies on.

Fixes #10046
  • Loading branch information
alt-romes committed Aug 16, 2024
1 parent 23e65cf commit c3b8cee
Show file tree
Hide file tree
Showing 10 changed files with 67 additions and 8 deletions.
4 changes: 3 additions & 1 deletion Cabal/src/Distribution/Simple/Configure.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1280,7 +1280,9 @@ configureComponents
extraCoverageUnitIds = case enabled of
-- Whole package configure, add package libs
ComponentRequestedSpec{} -> mapMaybe mbCompUnitId buildComponents
-- Component configure, no need to do anything
-- Component configure, no need to do anything since
-- extra-coverage-for will be passed for all other components that
-- should be covered.
OneComponentRequestedSpec{} -> []
mbCompUnitId LibComponentLocalBuildInfo{componentUnitId} = Just componentUnitId
mbCompUnitId _ = Nothing
Expand Down
18 changes: 11 additions & 7 deletions cabal-install/src/Distribution/Client/ProjectPlanning.hs
Original file line number Diff line number Diff line change
Expand Up @@ -4058,7 +4058,7 @@ setupHsConfigureFlags
Just _ -> error "non-library dependency"
Nothing -> LMainLibName

configCoverageFor = determineCoverageFor elabPkgSourceId plan
configCoverageFor = determineCoverageFor elab plan

setupHsConfigureArgs
:: ElaboratedConfiguredPackage
Expand Down Expand Up @@ -4469,13 +4469,14 @@ inplaceBinRoot layout config package =

-- The list of non-pre-existing libraries without module holes, i.e. the
-- main library and sub-libraries components of all the local packages in
-- the project that do not require instantiations or are instantiations.
-- the project that are dependencies of the components being built and that do
-- not require instantiations or are instantiations.
determineCoverageFor
:: PackageId
-- ^ The 'PackageId' of the package or component being configured
:: ElaboratedConfiguredPackage
-- ^ The package or component being configured
-> ElaboratedInstallPlan
-> Flag [UnitId]
determineCoverageFor configuredPkgSourceId plan =
determineCoverageFor configuredPkg plan =
Flag
$ mapMaybe
( \case
Expand All @@ -4488,15 +4489,18 @@ determineCoverageFor configuredPkgSourceId plan =
$ Graph.toList
$ InstallPlan.toGraph plan
where
shouldCoverPkg elab@ElaboratedConfiguredPackage{elabModuleShape, elabPkgSourceId, elabLocalToProject} =
libDeps = elabLibDependencies configuredPkg
shouldCoverPkg elab@ElaboratedConfiguredPackage{elabModuleShape, elabPkgSourceId = pkgSID, elabLocalToProject} =
elabLocalToProject
&& not (isIndefiniteOrInstantiation elabModuleShape)
-- TODO(#9493): We can only cover libraries in the same package
-- as the testsuite
&& configuredPkgSourceId == elabPkgSourceId
&& elabPkgSourceId configuredPkg == pkgSID
-- Libraries only! We don't cover testsuite modules, so we never need
-- the paths to their mix dirs. Furthermore, we do not install testsuites...
&& maybe False (\case CLibName{} -> True; CNotLibName{} -> False) (elabComponentName elab)
-- We only want coverage for libraries which are dependencies of the given one
&& pkgSID `elem` map (confSrcId . fst) libDeps

isIndefiniteOrInstantiation :: ModuleShape -> Bool
isIndefiniteOrInstantiation = not . Set.null . modShapeRequires
28 changes: 28 additions & 0 deletions cabal-testsuite/PackageTests/Regression/T10046/aa.cabal
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
cabal-version: 3.0
name: aa
version: 0.1.0.0
license: NONE
build-type: Simple
extra-doc-files: CHANGELOG.md

library
exposed-modules: MyLib
build-depends: base, template-haskell
hs-source-dirs: src
default-language: Haskell2010

test-suite bb-test
default-language: Haskell2010
type: exitcode-stdio-1.0
hs-source-dirs: testbb
main-is: Main.hs
build-depends:
base, aa

test-suite aa-test
default-language: Haskell2010
type: exitcode-stdio-1.0
hs-source-dirs: test
main-is: Main.hs
build-depends:
base
1 change: 1 addition & 0 deletions cabal-testsuite/PackageTests/Regression/T10046/app/Main.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
main = pure ()
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
packages: .
6 changes: 6 additions & 0 deletions cabal-testsuite/PackageTests/Regression/T10046/cabal.test.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import Test.Cabal.Prelude

-- T10046
main = cabalTest $ recordMode DoNotRecord $ do
out <- cabal' "test" ["all"]
assertOutputDoesNotContain "Failed to find the installed unit 'aa-0.1.0.0-inplace'" out
11 changes: 11 additions & 0 deletions cabal-testsuite/PackageTests/Regression/T10046/src/MyLib.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{-# LANGUAGE TemplateHaskell #-}
module MyLib where

import Control.Concurrent
import Language.Haskell.TH

-- Must take longer to compile than the testsuite
$(do runIO $
threadDelay (5*1000*1000) -- 5s
[d| data X |]
)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
main = pure ()
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
main = pure ()
4 changes: 4 additions & 0 deletions changelog.d/issue-10046
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
synopsis: Bug fix - Don't pass --coverage-for for non-dependency libs of testsuite
packages: cabal-install
issues: #10046
prs: #10250

0 comments on commit c3b8cee

Please sign in to comment.