Spurious duplicate modules error

[Bodigrim]

name:          duplicate-modules
version:       0.0
cabal-version: 2.0
build-type:    Simple
category:      NA
maintainer:    NA
synopsis:      NA
description:   NA NA
license:       BSD3
license-file:  LICENSE

flag expose

library
  default-language: Haskell2010
  if flag(expose)
    exposed-modules: Foo
  else
    other-modules: Foo

cabal check is unhappy with this:

Warning: The package will not build sanely due to these errors:
Warning: Duplicate modules in library: Foo
Warning: Hackage would reject this package.

I understand that it's a tricky situation for cabal and that potentially there could be exponentially many flag combinations, but practically, when number of flags is < 5, I'd expect cabal to evaluate all possible combinations and validate respective configurations independently.

[Mikolaj]

I think it's worth designing a check that warns of such use of flags (even when the two module differ and so there's no error). I've just heard today experienced devs stating that cabal flags should not change a package API, as is the case in your example. Otherwise, flipping flags potentially causes breakage and cabal can't guess if it should rebuild half of package database or not to prevent it. Not to mention the horror of ensuring my package can handle various API-changing flags combinations of packages it depends on.

Also, each if-then-else can double the time solving takes, because it can change what a package depends on. That gets nasty long before 5 flags.

[Mikolaj]

Having said that, if your goal is to conditionally expose internal modules, perhaps it can be done by putting them in an internal library, using in a library with exposed modules, and placing visibility:True of the internal library inside a conditional? I haven't tried that, so I'd be interested if it works or in any other workaround you may find.

[emilypi]

It sounds like we're just parsing through the relevant flags other-modules and exposed-modules without considering conditionals, which tells me check needs to be smarter about the context in which these entries occur. Let's see if we can hunt this one down, but that's my working suspicion 😄

[Mikolaj]

@emilypi: however, which flag values should check consider?

[emilypi]

@Mikolaj let's think about it - i dont know check that well, but i did run into this comment which is probably the root of our problem:

   -- flatten the generic package description into a regular package
    -- description
    -- TODO: this may give more warnings than it should give;
    --       consider two branches of a condition, one saying
    --          ghc-options: -Wall
    --       and the other
    --          ghc-options: -Werror
    --      joined into
    --          ghc-options: -Wall -Werror
    --      checkPackages will yield a warning on the last line, but it
    --      would not on each individual branch.
    --      Hovever, this is the same way hackage does it, so we will yield
    --      the exact same errors as it will.

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

Line 62 in 92ef94e

-- TODO: this may give more warnings than it should give;

"Flattening" branches seems to be happening here, and its wrong in this case.

[emilypi]

AHA!

-- | Flatten a generic package description by ignoring all conditions and just
-- join the field descriptors into on package description.  Note, however,
-- that this may lead to inconsistent field values, since all values are
-- joined into one field, which may not be possible in the original package
-- description, due to the use of exclusive choices (if ... else ...).
--
-- TODO: One particularly tricky case is defaulting.  In the original package
-- description, e.g., the source directory might either be the default or a
-- certain, explicitly set path.  Since defaults are filled in only after the
-- package has been resolved and when no explicit value has been set, the
-- default path will be missing from the package description returned by this
-- function.

From

cabal/Cabal/src/Distribution/PackageDescription/Configuration.hs

Line 496 in 3a37f45

-- | Flatten a generic package description by ignoring all conditions and just

is the culprit.

[emilypi]

Related: #4629

[ptkato]

Curiouser and curiouser! Apart from performance, is there any other reason as to why this flattening behaviour, without considering the conditionals, would be desired?

[emilypi]

Laziness (in the pejorative sense) :)

[jneira]

I would say this is duplicate of #4629 so I would close it

[Mikolaj]

Unless @Bodigrim objects, I think it makes sense to close.

[gbaz]

definitely a dup, closing.