-
Notifications
You must be signed in to change notification settings - Fork 712
Fix #2066: buildDepends is just an accessor function, not a field #4383
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
Changes from all commits
6154796
e3ebcb2
0d41fbd
73713ef
3af7f3e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,8 +44,10 @@ import Distribution.Compiler | |
import Distribution.System | ||
import Distribution.Simple.Utils | ||
import Distribution.Text | ||
import Distribution.Compat.Lens | ||
import Distribution.Compat.ReadP as ReadP hiding ( char ) | ||
import qualified Distribution.Compat.ReadP as ReadP ( char ) | ||
import qualified Distribution.Types.BuildInfo.Lens as L | ||
import Distribution.Types.ComponentRequestedSpec | ||
import Distribution.Types.ForeignLib | ||
import Distribution.Types.Component | ||
|
@@ -351,18 +353,18 @@ overallDependencies enabled (TargetSet targets) = mconcat depss | |
-- | Collect up the targets in a TargetSet of tagged targets, storing the | ||
-- dependencies as we go. | ||
flattenTaggedTargets :: TargetSet PDTagged -> (Maybe Library, [(UnqualComponentName, Component)]) | ||
flattenTaggedTargets (TargetSet targets) = foldr untag (Nothing, []) targets | ||
where | ||
untag (_, Lib _) (Just _, _) = userBug "Only one library expected" | ||
untag (_, Lib l) (Nothing, comps) = (Just l, comps) | ||
untag (_, SubComp n c) (mb_lib, comps) | ||
| any ((== n) . fst) comps = | ||
userBug $ "There exist several components with the same name: '" ++ unUnqualComponentName n ++ "'" | ||
|
||
| otherwise = (mb_lib, (n, c) : comps) | ||
|
||
untag (_, PDNull) x = x -- actually this should not happen, but let's be liberal | ||
|
||
flattenTaggedTargets (TargetSet targets) = foldr untag (Nothing, []) targets where | ||
untag (depMap, pdTagged) accum = case (pdTagged, accum) of | ||
(Lib _, (Just _, _)) -> userBug "Only one library expected" | ||
(Lib l, (Nothing, comps)) -> (Just $ redoBD l, comps) | ||
(SubComp n c, (mb_lib, comps)) | ||
| any ((== n) . fst) comps -> | ||
userBug $ "There exist several components with the same name: '" ++ display n ++ "'" | ||
| otherwise -> (mb_lib, (n, redoBD c) : comps) | ||
(PDNull, x) -> x -- actually this should not happen, but let's be liberal | ||
where | ||
redoBD :: L.HasBuildInfo a => a -> a | ||
redoBD = set L.targetBuildDepends $ fromDepMap depMap | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does this do? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's been a while, but pretty sure this is the problematic bit via your choice a year ago. The difference between the new version and the old version is now we updated the components' |
||
|
||
------------------------------------------------------------------------------ | ||
-- Convert GenericPackageDescription to PackageDescription | ||
|
@@ -447,7 +449,6 @@ finalizePD userflags enabled satisfyDep | |
, executables = exes' | ||
, testSuites = tests' | ||
, benchmarks = bms' | ||
, buildDepends = fromDepMap (overallDependencies enabled targetSet) | ||
} | ||
, flagVals ) | ||
where | ||
|
@@ -517,38 +518,25 @@ flattenPackageDescription | |
, executables = reverse exes | ||
, testSuites = reverse tests | ||
, benchmarks = reverse bms | ||
, buildDepends = ldeps | ||
++ reverse sub_ldeps | ||
++ reverse pldeps | ||
++ reverse edeps | ||
++ reverse tdeps | ||
++ reverse bdeps | ||
} | ||
where | ||
(mlib, ldeps) = case mlib0 of | ||
Just lib -> let (l,ds) = ignoreConditions lib in | ||
(Just ((libFillInDefaults l) { libName = Nothing }), ds) | ||
Nothing -> (Nothing, []) | ||
(sub_libs, sub_ldeps) = foldr flattenLib ([],[]) sub_libs0 | ||
(flibs, pldeps) = foldr flattenFLib ([],[]) flibs0 | ||
(exes, edeps) = foldr flattenExe ([],[]) exes0 | ||
(tests, tdeps) = foldr flattenTst ([],[]) tests0 | ||
(bms, bdeps) = foldr flattenBm ([],[]) bms0 | ||
flattenLib (n, t) (es, ds) = | ||
let (e, ds') = ignoreConditions t in | ||
( (libFillInDefaults $ e { libName = Just n, libExposed = False }) : es, ds' ++ ds ) | ||
flattenFLib (n, t) (es, ds) = | ||
let (e, ds') = ignoreConditions t in | ||
( (flibFillInDefaults $ e { foreignLibName = n }) : es, ds' ++ ds ) | ||
flattenExe (n, t) (es, ds) = | ||
let (e, ds') = ignoreConditions t in | ||
( (exeFillInDefaults $ e { exeName = n }) : es, ds' ++ ds ) | ||
flattenTst (n, t) (es, ds) = | ||
let (e, ds') = ignoreConditions t in | ||
( (testFillInDefaults $ e { testName = n }) : es, ds' ++ ds ) | ||
flattenBm (n, t) (es, ds) = | ||
let (e, ds') = ignoreConditions t in | ||
( (benchFillInDefaults $ e { benchmarkName = n }) : es, ds' ++ ds ) | ||
mlib = f <$> mlib0 | ||
where f lib = (libFillInDefaults . fst . ignoreConditions $ lib) { libName = Nothing } | ||
sub_libs = flattenLib <$> sub_libs0 | ||
flibs = flattenFLib <$> flibs0 | ||
exes = flattenExe <$> exes0 | ||
tests = flattenTst <$> tests0 | ||
bms = flattenBm <$> bms0 | ||
flattenLib (n, t) = libFillInDefaults $ (fst $ ignoreConditions t) | ||
{ libName = Just n, libExposed = False } | ||
flattenFLib (n, t) = flibFillInDefaults $ (fst $ ignoreConditions t) | ||
{ foreignLibName = n } | ||
flattenExe (n, t) = exeFillInDefaults $ (fst $ ignoreConditions t) | ||
{ exeName = n } | ||
flattenTst (n, t) = testFillInDefaults $ (fst $ ignoreConditions t) | ||
{ testName = n } | ||
flattenBm (n, t) = benchFillInDefaults $ (fst $ ignoreConditions t) | ||
{ benchmarkName = n } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happened in this diff? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed the accumulation of deps and just did the flattening of components. Because of the removal of https://github.com/haskell/cabal/pull/4383/files#diff-12c8f4f2a1b901f8cbc37a619afafcb7L520, we no longer need that accumulation. |
||
|
||
-- This is in fact rather a hack. The original version just overrode the | ||
-- default values, however, when adding conditions we had to switch to a | ||
|
@@ -620,12 +608,10 @@ transformAllBuildDepends f gpd = gpd' | |
where | ||
onBI bi = bi { targetBuildDepends = map f $ targetBuildDepends bi } | ||
onSBI stp = stp { setupDepends = map f $ setupDepends stp } | ||
onPD pd = pd { buildDepends = map f $ buildDepends pd } | ||
|
||
pd' = onPD $ packageDescription gpd | ||
gpd' = transformAllCondTrees id id id id (map f) | ||
. transformAllBuildInfos onBI onSBI | ||
$ gpd { packageDescription = pd' } | ||
$ gpd | ||
|
||
-- | Walk all 'CondTree's inside a 'GenericPackageDescription' and apply | ||
-- appropriate transformations to all nodes. Helper function used by | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
'ghc-options: -O2' is rarely needed. Check that it is giving a real benefit and not just imposing longer compile times on your users. | ||
The package uses major bounded version syntax in the 'build-depends' field: base ^>=4.10.0, Cabal ^>=2.0.0, ghc ^>=8.2, ghc-paths ^>=0.1.0.9, xhtml ^>=3000.2.2, QuickCheck ^>=2.10, hspec ^>=2.4.4, ghc ^>=8.2. To use this new syntax the package need to specify at least 'cabal-version: >= 2.0'. Alternatively, if broader compatibility is important then use: base >=4.10.0 && <4.11, Cabal >=2.0.0 && <2.1, ghc >=8.2 && <8.3, ghc-paths >=0.1.0.9 && <0.2, xhtml >=3000.2.2 && <3000.3, QuickCheck >=2.10 && <2.11, hspec >=2.4.4 && <2.5, ghc >=8.2 && <8.3 | ||
The package uses major bounded version syntax in the 'build-depends' field: base ^>=4.10.0, Cabal ^>=2.0.0, ghc ^>=8.2, ghc-paths ^>=0.1.0.9, xhtml ^>=3000.2.2, ghc ^>=8.2, hspec ^>=2.4.4, QuickCheck ^>=2.10. To use this new syntax the package need to specify at least 'cabal-version: >= 2.0'. Alternatively, if broader compatibility is important then use: base >=4.10.0 && <4.11, Cabal >=2.0.0 && <2.1, ghc >=8.2 && <8.3, ghc-paths >=0.1.0.9 && <0.2, xhtml >=3000.2.2 && <3000.3, ghc >=8.2 && <8.3, hspec >=2.4.4 && <2.5, QuickCheck >=2.10 && <2.11 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes my BC senses tingle. What's the BC story for custom setups that call
buildDepends
. Can we preserve the old semantics under the old name?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/23Skidoo/all-custom-setups says
buildDepends
it isn't really used anywhere,pandoc-1.6
is from 2010There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah per the big comment at https://github.com/haskell/cabal/pull/4383/files#diff-d02dbf8d6911c6afb8c8cb25ebc6ea9cL127, anything that uses it seems bound to be incorrect anyways.