Skip to content

Commit 689e85c

Browse files
amesgenerikdclaude
committed
active-repositories: Don't override fully-deprecated packages
When active-repositories includes a repo with :override, and that repo's preferred-versions marks all its versions of a package as deprecated, the index-combining step previously still applied full override semantics, hiding all versions of that package from earlier repos. Fix by consulting preferred-versions when combining indexes: if lookupDependency finds no preferred version for a package in the override repo, fall back to merge semantics so earlier-repo versions remain visible. Two changes: - PackageIndex: add overrideOrMerge, a per-package Override/Merge strategy - IndexUtils: add deprecationAwareStrategy, wired into getSourcePackagesAtIndexState Fixes #8502 This code was originally authored by Alexander Esgen and sumbitted in a PR over 2 years ago. Erik manually rebased that code onto master and used Claude to add tests and do some very minor refactoring to Alexander's code to make it more testable. Co-Authored-By: Erik de Castro Lopo <erikd@mega-nerd.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 9c13a3e commit 689e85c

5 files changed

Lines changed: 275 additions & 1 deletion

File tree

cabal-install-solver/src/Distribution/Solver/Types/PackageIndex.hs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ module Distribution.Solver.Types.PackageIndex (
2222
-- * Updates
2323
merge,
2424
override,
25+
OverrideOrMerge(..),
26+
overrideOrMerge,
2527
insert,
2628
deletePackageName,
2729
deletePackageId,
@@ -181,6 +183,28 @@ override i1@(PackageIndex m1) i2@(PackageIndex m2) =
181183
expensiveAssert (invariant i1 && invariant i2) $
182184
mkPackageIndex (Map.unionWith (\_l r -> r) m1 m2)
183185

186+
data OverrideOrMerge = Override | Merge
187+
deriving (Eq, Show)
188+
189+
-- | Combined override-or-merge of two indexes.
190+
--
191+
-- For any package, either 'override' or 'merge' the packages from the second
192+
-- index into the first based on the supplied predicate.
193+
--
194+
overrideOrMerge ::
195+
Package pkg
196+
=> (PackageName -> OverrideOrMerge)
197+
-> PackageIndex pkg
198+
-> PackageIndex pkg
199+
-> PackageIndex pkg
200+
overrideOrMerge strategy i1@(PackageIndex m1) i2@(PackageIndex m2) =
201+
expensiveAssert (invariant i1 && invariant i2) $
202+
mkPackageIndex (Map.unionWithKey overridePkg m1 m2)
203+
where
204+
overridePkg name l r = case strategy name of
205+
Override -> r
206+
Merge -> mergeBuckets l r
207+
184208
-- | Inserts a single package into the index.
185209
--
186210
-- This is equivalent to (but slightly quicker than) using 'mappend' or

cabal-install/src/Distribution/Client/IndexUtils.hs

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ module Distribution.Client.IndexUtils
3030
, ActiveRepos
3131
, filterSkippedActiveRepos
3232
, applyStrategy
33+
, addIndex
34+
, deprecationAwareStrategy
3335
, Index (..)
3436
, RepoIndexState (..)
3537
, PackageEntry (..)
@@ -374,7 +376,7 @@ getSourcePackagesAtIndexState verbosity repoCtxt mb_idxState mb_activeRepos = do
374376
]
375377

376378
let pkgs :: PackageIndex UnresolvedSourcePackage
377-
pkgs = foldl' (\acc (rd, s) -> applyStrategy acc (rdIndex rd, s)) mempty pkgss'
379+
pkgs = foldl' (\acc (rd, s) -> addIndex acc (rdIndex rd, rdPreferences rd, s)) mempty pkgss'
378380

379381
-- Note: preferences combined without using CombineStrategy
380382
let prefs :: Map PackageName VersionRange
@@ -419,6 +421,45 @@ applyStrategy acc (_, CombineStrategySkip) = acc
419421
applyStrategy acc (idx, CombineStrategyMerge) = PackageIndex.merge acc idx
420422
applyStrategy acc (idx, CombineStrategyOverride) = PackageIndex.override acc idx
421423

424+
-- | Fold one package index and its preferred-versions into an accumulator
425+
-- according to a 'CombineStrategy'.
426+
--
427+
-- Like 'applyStrategy', but for 'CombineStrategyOverride' consults the
428+
-- repo's @preferred-versions@ via 'deprecationAwareStrategy': if all
429+
-- versions of a package are deprecated in the override repo, merge
430+
-- semantics are used for that package instead of override semantics.
431+
addIndex
432+
:: Package pkg
433+
=> PackageIndex pkg
434+
-> (PackageIndex pkg, [Dependency], CombineStrategy)
435+
-> PackageIndex pkg
436+
addIndex acc (idx, prefs, CombineStrategyOverride) =
437+
PackageIndex.overrideOrMerge (deprecationAwareStrategy idx prefsByPkg) acc idx
438+
where
439+
prefsByPkg =
440+
Map.fromListWith
441+
intersectVersionRanges
442+
[(name, range) | Dependency name range _ <- prefs]
443+
addIndex acc (idx, _, s) = applyStrategy acc (idx, s)
444+
445+
-- | Per-package override-or-merge decision for a 'CombineStrategyOverride' repo.
446+
--
447+
-- Returns 'PackageIndex.Merge' when every version of the package in the
448+
-- override index is deprecated (i.e. excluded by the repo's
449+
-- @preferred-versions@), so that versions from earlier repos remain visible.
450+
-- Returns 'PackageIndex.Override' otherwise.
451+
deprecationAwareStrategy
452+
:: Package pkg
453+
=> PackageIndex pkg
454+
-> Map PackageName VersionRange
455+
-> PackageName
456+
-> PackageIndex.OverrideOrMerge
457+
deprecationAwareStrategy idx prefsByPkg pkgname
458+
| Just pkgPrefs <- Map.lookup pkgname prefsByPkg
459+
, null $ PackageIndex.lookupDependency idx pkgname pkgPrefs =
460+
PackageIndex.Merge
461+
| otherwise = PackageIndex.Override
462+
422463
-- | Read a repository index from disk, from the local file specified by
423464
-- the 'Repo'.
424465
--

cabal-install/tests/UnitTests/Distribution/Client/IndexUtils.hs

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@ import Distribution.Client.IndexUtils.ActiveRepos
55
import qualified Distribution.Compat.NonEmptySet as NES
66
import Distribution.Package
77
import Distribution.Simple.Utils (toUTF8LBS)
8+
import Distribution.Solver.Types.PackageIndex (OverrideOrMerge (..))
89
import qualified Distribution.Solver.Types.PackageIndex as PackageIndex
910
import Distribution.Types.LibraryName
1011
import Distribution.Version
1112

1213
import qualified Data.List as List
14+
import qualified Data.Map.Strict as Map
1315

1416
import Test.Tasty
1517
import Test.Tasty.HUnit
@@ -18,6 +20,9 @@ tests :: [TestTree]
1820
tests =
1921
[ simpleVersionsParserTests
2022
, indexCombiningTests
23+
, overrideOrMergeTests
24+
, deprecationAwareStrategyTests
25+
, addIndexTests
2126
]
2227

2328
-- ---------------------------------------------------------------------------
@@ -193,3 +198,183 @@ repoBar1 = PackageIndex.fromList [bar1]
193198
repoFoo12, repoFoo1bar1 :: PackageIndex.PackageIndex PackageIdentifier
194199
repoFoo12 = PackageIndex.fromList [foo1, foo2]
195200
repoFoo1bar1 = PackageIndex.fromList [foo1, bar1]
201+
202+
-- ---------------------------------------------------------------------------
203+
-- overrideOrMerge tests
204+
--
205+
-- These test PackageIndex.overrideOrMerge directly, which is the building
206+
-- block for conditionally falling back to merge when all versions of a
207+
-- package in the override repo are deprecated (issue #8502).
208+
-- ---------------------------------------------------------------------------
209+
210+
overrideOrMergeTests :: TestTree
211+
overrideOrMergeTests =
212+
testGroup
213+
"overrideOrMerge"
214+
[ testCase "all-Override strategy matches plain override" $
215+
-- When strategy always returns Override, result equals PackageIndex.override
216+
let result = PackageIndex.overrideOrMerge (const Override) repoFoo12 repoFoo2
217+
expected = PackageIndex.override repoFoo12 repoFoo2
218+
in allPkgs result @?= allPkgs expected
219+
, testCase "all-Merge strategy matches plain merge" $
220+
-- When strategy always returns Merge, result equals PackageIndex.merge
221+
let result = PackageIndex.overrideOrMerge (const Merge) repoFoo12 repoFoo2
222+
expected = PackageIndex.merge repoFoo12 repoFoo2
223+
in allPkgs result @?= allPkgs expected
224+
, testCase "Override: second index wins entire package bucket" $
225+
-- repoFoo12 has foo-1.0 and foo-2.0; repoFoo2 has only foo-2.0.
226+
-- Override means repoFoo2 wins the 'foo' bucket, so foo-1.0 is hidden.
227+
allPkgs (PackageIndex.overrideOrMerge (const Override) repoFoo12 repoFoo2)
228+
@?= [foo2]
229+
, testCase "Merge: both buckets combined, duplicates removed" $
230+
-- repoFoo12 has foo-1.0 and foo-2.0; repoFoo2 has only foo-2.0.
231+
-- Merge keeps foo-1.0 and foo-2.0 (foo-2.0 deduplicated).
232+
allPkgs (PackageIndex.overrideOrMerge (const Merge) repoFoo12 repoFoo2)
233+
@?= List.sort [foo1, foo2]
234+
, testCase "mixed strategy: Override for foo, Merge for bar" $
235+
-- repoFoo1bar1 has foo-1.0 and bar-1.0; second index has foo-2.0 and bar-1.0.
236+
-- foo is overridden (foo-1.0 hidden), bar is merged (bar-1.0 deduplicated).
237+
let i2 = PackageIndex.fromList [foo2, bar1]
238+
strategy name
239+
| name == mkPackageName "foo" = Override
240+
| otherwise = Merge
241+
in allPkgs (PackageIndex.overrideOrMerge strategy repoFoo1bar1 i2)
242+
@?= List.sort [foo2, bar1]
243+
, testCase "deprecated fallback: Merge when override repo has only deprecated versions" $
244+
-- Simulates the issue-8502 scenario: repo-a has foo-1.0; repo-b (override)
245+
-- has foo-2.0 but all its versions are deprecated. The caller detects this
246+
-- and passes Merge for 'foo', so foo-1.0 remains visible alongside foo-2.0.
247+
let repoA = PackageIndex.fromList [foo1]
248+
repoB = PackageIndex.fromList [foo2] -- pretend all deprecated
249+
strategy name
250+
| name == mkPackageName "foo" = Merge -- fall back because all deprecated
251+
| otherwise = Override
252+
in allPkgs (PackageIndex.overrideOrMerge strategy repoA repoB)
253+
@?= List.sort [foo1, foo2]
254+
, testCase "package absent from second index: first index versions kept" $
255+
-- bar is only in repoFoo1bar1, not in repoFoo2; it survives regardless of strategy.
256+
allPkgs (PackageIndex.overrideOrMerge (const Override) repoFoo1bar1 repoFoo2)
257+
@?= List.sort [foo2, bar1]
258+
, testCase "package absent from first index: second index versions appear" $
259+
allPkgs (PackageIndex.overrideOrMerge (const Override) repoFoo1 repoBar1)
260+
@?= List.sort [foo1, bar1]
261+
, testCase "empty first index: second index fully visible" $
262+
allPkgs (PackageIndex.overrideOrMerge (const Override) mempty repoFoo12)
263+
@?= List.sort [foo1, foo2]
264+
, testCase "empty second index: first index unchanged" $
265+
allPkgs (PackageIndex.overrideOrMerge (const Override) repoFoo12 mempty)
266+
@?= List.sort [foo1, foo2]
267+
]
268+
269+
allPkgs :: PackageIndex.PackageIndex PackageIdentifier -> [PackageIdentifier]
270+
allPkgs = List.sort . PackageIndex.allPackages
271+
272+
-- ---------------------------------------------------------------------------
273+
-- deprecationAwareStrategy tests
274+
--
275+
-- Tests for the per-package Override/Merge decision used when applying a
276+
-- CombineStrategyOverride repo. The three cases are:
277+
-- 1. Package absent from preferred-versions -> Override
278+
-- 2. Package present, some versions preferred -> Override
279+
-- 3. Package present, no versions preferred -> Merge (all deprecated)
280+
-- ---------------------------------------------------------------------------
281+
282+
deprecationAwareStrategyTests :: TestTree
283+
deprecationAwareStrategyTests =
284+
testGroup
285+
"deprecationAwareStrategy"
286+
[ testCase "package absent from preferred-versions gives Override" $
287+
-- No entry for 'foo' in prefs, so the repo is not restricting it.
288+
strat repoFoo1 Map.empty fooName @?= Override
289+
, testCase "package present with matching versions gives Override" $
290+
-- foo-1.0 is in the index and satisfies ">= 1.0", so not all deprecated.
291+
strat repoFoo1 (prefs fooName (orLaterVersion v1)) fooName @?= Override
292+
, testCase "package present but no versions match gives Merge" $
293+
-- foo-1.0 is in the index but the pref ">= 2.0" excludes it: all deprecated.
294+
strat repoFoo1 (prefs fooName (orLaterVersion v2)) fooName @?= Merge
295+
, testCase "unrelated package in prefs does not affect result" $
296+
-- Prefs only mention 'bar'; 'foo' has no pref entry, so Override.
297+
strat repoFoo1 (prefs barName (orLaterVersion v1)) fooName @?= Override
298+
, testCase "package absent from index but in prefs gives Merge" $
299+
-- The pref entry exists but the index is empty, so lookupDependency
300+
-- returns [], meaning no preferred version exists.
301+
strat mempty (prefs fooName (orLaterVersion v2)) fooName @?= Merge
302+
, testCase "multiple packages decided independently" $
303+
-- foo is deprecated (pref excludes foo-1.0), bar is not (bar-1.0 satisfies >= 1.0).
304+
let p =
305+
Map.unionWith
306+
intersectVersionRanges
307+
(prefs fooName (orLaterVersion v2))
308+
(prefs barName (orLaterVersion v1))
309+
in do
310+
strat repoFoo1bar1 p fooName @?= Merge
311+
strat repoFoo1bar1 p barName @?= Override
312+
]
313+
where
314+
strat = deprecationAwareStrategy
315+
fooName = mkPackageName "foo"
316+
barName = mkPackageName "bar"
317+
v1 = mkVersion [1, 0]
318+
v2 = mkVersion [2, 0]
319+
prefs name vr = Map.singleton name vr
320+
321+
-- ---------------------------------------------------------------------------
322+
-- addIndex tests
323+
--
324+
-- Tests for the top-level addIndex, which is the function used by
325+
-- getSourcePackagesAtIndexState to fold each repository's index into the
326+
-- accumulator. Unlike applyStrategy, addIndex consults preferred-versions
327+
-- for CombineStrategyOverride, falling back to merge when all versions of a
328+
-- package are deprecated. A regression to plain override would cause the
329+
-- "all deprecated" test to fail.
330+
-- ---------------------------------------------------------------------------
331+
332+
addIndexTests :: TestTree
333+
addIndexTests =
334+
testGroup
335+
"addIndex"
336+
[ testCase "Skip: index not added" $
337+
run [(repoFoo1, [], CombineStrategySkip)]
338+
@?= []
339+
, testCase "Merge: index added" $
340+
run [(repoFoo1, [], CombineStrategyMerge)]
341+
@?= [foo1]
342+
, testCase "Override with no prefs: behaves like plain override" $
343+
run
344+
[ (repoFoo12, [], CombineStrategyMerge)
345+
, (repoFoo2, [], CombineStrategyOverride)
346+
]
347+
@?= [foo2]
348+
, testCase "Override with prefs matching some versions: still overrides" $
349+
-- foo-2.0 satisfies ">= 2.0", so not all deprecated; override applies.
350+
run
351+
[ (repoFoo12, [], CombineStrategyMerge)
352+
, (repoFoo2, [dep fooName (orLaterVersion v2)], CombineStrategyOverride)
353+
]
354+
@?= [foo2]
355+
, testCase "Override with all versions deprecated: falls back to merge" $
356+
-- foo-2.0 does not satisfy ">= 3.0", so all versions deprecated;
357+
-- override falls back to merge, keeping foo-1.0 from the first repo.
358+
run
359+
[ (repoFoo1, [], CombineStrategyMerge)
360+
, (repoFoo2, [dep fooName (orLaterVersion v3)], CombineStrategyOverride)
361+
]
362+
@?= List.sort [foo1, foo2]
363+
, testCase "Override: only the deprecated package falls back, others still override" $
364+
-- foo is all-deprecated in override repo → merge; bar has no prefs → override.
365+
run
366+
[ (repoFoo1bar1, [], CombineStrategyMerge)
367+
,
368+
( PackageIndex.fromList [foo2, bar1]
369+
, [dep fooName (orLaterVersion v3)]
370+
, CombineStrategyOverride
371+
)
372+
]
373+
@?= List.sort [foo1, foo2, bar1]
374+
]
375+
where
376+
run = allPkgs . List.foldl' (\acc (idx, ps, s) -> addIndex acc (idx, ps, s)) mempty
377+
fooName = mkPackageName "foo"
378+
v2 = mkVersion [2, 0]
379+
v3 = mkVersion [3, 0]
380+
dep name vr = Dependency name vr (NES.singleton LMainLibName)

changelog.d/pr-11760

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
synopsis: "active-repositories: don't override fully-deprecated packages"
3+
packages: [cabal-install, cabal-install-solver]
4+
prs: 11760
5+
---
6+
7+
When `active-repositories` includes a repo with `:override`, and that
8+
repo's `preferred-versions` marks all its versions of a package as
9+
deprecated, the index-combining step previously still applied full
10+
override semantics, hiding all versions of that package from earlier
11+
repos.
12+
13+
Fix by consulting `preferred-versions` when combining indexes: if no
14+
version of the package in the override repo is preferred, fall back to
15+
merge semantics so earlier-repo versions remain visible.
16+
17+
Fixes https://github.com/haskell/cabal/issues/8502

doc/cabal-project-description-file.rst

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -782,6 +782,13 @@ The following settings control the behavior of the dependency solver:
782782
present in my-repository only in version 2.0, and the :override forbids
783783
searching for other versions of X further up the list.
784784

785+
There is one exception: if all versions of a package in the overriding
786+
repository are deprecated (i.e. excluded by that repository's
787+
``preferred-versions``), :override falls back to merge semantics for that
788+
package, so versions from earlier repositories remain visible. This avoids
789+
a situation where a fully-deprecated override inadvertently hides all
790+
usable versions of a package.
791+
785792
:override has no effect for package names that aren't present in the
786793
overriding repository.
787794

0 commit comments

Comments
 (0)