From c0c7db689a44fbe53069be639c826074fb108791 Mon Sep 17 00:00:00 2001 From: Matthew Pickering Date: Tue, 27 Aug 2024 16:41:45 +0100 Subject: [PATCH] Only munge internal dependencies when printing when there is no explicit syntax * In `postProcessInternalDeps` the shadowing logic which existed prior to cabal format 3.4 is implemented in a post processing step. The algorithm there replaces any references to internal sublibraries with an explicit qualifier. For example if you write.. ``` library build-depends: foo library foo ... ``` this is reinterpreted as ``` library build-depends: mylib:foo library foo ... ``` * In `preProcessInternalDeps` the inverse transformation takes place, the goal is to replace `mylib:foo` with just `foo`. * Things go wrong if you are using version 3.0 for your cabal file because - In 3.0 the qualifier syntax is introduced so you can be expliciit about sublibrary dependencies - The shadowing semantics of non-qualified dependencies still exists. So the situation is that the user is explicit about the sublibrary ``` library library qux build-depends: mylib:{mylib, foo} library foo ``` 1. Post-process leaves this alone, the user is already explicit about depending on a sublibrary. 2. Pre-processing then rewrites `mylib:{mylib, foo}` into two dependencies, `mylib` and `foo` (no qualifier). 3. When parsed these are two separate dependencies rather than treated as one dependency, roundtrip test fails. Solution: Only perform the reverse transformation when the cabal library version is <= 3.0 and doesn't support the explicit syntax. Now what happens in these two situations: 1. ``` library build-depends: foo library foo ... ``` this is reinterpreted as ``` library build-depends: mylib:foo library foo ... ``` then printed and parsed exactly the same way. 2. Explicit syntax is parsed and printed without being munged (when supported) Note: Mixins only supported sublibrary qualifiers from 3.4 whilst dependencies supported this from 3.0, hence the lack of guard on the mixins case. Fixes #10283 --- .../src/Distribution/PackageDescription/PrettyPrint.hs | 7 +++++-- .../tests/ParserTests/regressions/issue-6083-b.format | 6 +++--- validate.sh | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/Cabal-syntax/src/Distribution/PackageDescription/PrettyPrint.hs b/Cabal-syntax/src/Distribution/PackageDescription/PrettyPrint.hs index b03b1b99ada..8303a65e7d5 100644 --- a/Cabal-syntax/src/Distribution/PackageDescription/PrettyPrint.hs +++ b/Cabal-syntax/src/Distribution/PackageDescription/PrettyPrint.hs @@ -271,7 +271,7 @@ preProcessInternalDeps specVer gpd transformD :: Dependency -> [Dependency] transformD (Dependency pn vr ln) - | pn == thisPn = + | pn == thisPn && specVer < CabalSpecV3_0 = if LMainLibName `NES.member` ln then Dependency thisPn vr mainLibSet : sublibs else sublibs @@ -282,9 +282,12 @@ preProcessInternalDeps specVer gpd ] transformD d = [d] + -- Always perform transformation for mixins as syntax was only introduced in 3.4 + -- This guard is uncessary as no transformations take place when cabalSpec < CabalSpecV3_4 but + -- it more clearly signifies the intent. transformM :: Mixin -> Mixin transformM (Mixin pn (LSubLibName uqn) inc) - | pn == thisPn = + | pn == thisPn && specVer < CabalSpecV3_4 = mkMixin (unqualComponentNameToPackageName uqn) LMainLibName inc transformM m = m diff --git a/Cabal-tests/tests/ParserTests/regressions/issue-6083-b.format b/Cabal-tests/tests/ParserTests/regressions/issue-6083-b.format index d209d572be0..255c71de998 100644 --- a/Cabal-tests/tests/ParserTests/regressions/issue-6083-b.format +++ b/Cabal-tests/tests/ParserTests/regressions/issue-6083-b.format @@ -6,7 +6,7 @@ library default-language: Haskell2010 build-depends: base, - sublib + issue:sublib library sublib default-language: Haskell2010 @@ -15,10 +15,10 @@ executable demo-a main-is: Main.hs build-depends: issue, - sublib + issue:sublib executable demo-b main-is: Main.hs build-depends: issue, - sublib + issue:sublib diff --git a/validate.sh b/validate.sh index ff1c9e139a9..4fd67ffff55 100755 --- a/validate.sh +++ b/validate.sh @@ -419,7 +419,7 @@ CMD="$($CABALLISTBIN Cabal-tests:test:no-thunks-test) $TESTSUITEJOBS --hide-succ # See #10284 for why this value is pinned. -HACKAGE_TESTS_INDEX_STATE="--index-state=2024-08-25" +HACKAGE_TESTS_INDEX_STATE="--index-state=2024-08-28" CMD=$($CABALLISTBIN Cabal-tests:test:hackage-tests) (cd Cabal-tests && timed $CMD read-fields $HACKAGE_TESTS_INDEX_STATE) || exit 1