Fix the duplicate modules check

[gbaz]

Resolves #7776

Now if a package may have duplicate modules (subject to conditionals), cabal warns on this.

But if a package necessarily has duplicate modules (because they are not guarded by conditionals), cabal will error.

[andreasabel]

I don't understand the code, but I tested the product on hashable-1.2.7 and hashable-1.3 and found now that it works as expected: build and install succeed, check gives this warning:

Warning: Potential duplicate modules (subject to conditionals) in benchmark:
Data.Hashable.RandomSource

[fgaz]

Shouldn't this be PackageDistInexcusable, since in this case there's a branch where modules are duplicated?

[gbaz]

Inexecusable would prevent hackage upload. Given that people can write multiple if branches that are not obviously nonexclusive, but whose conditions (e.g. ghc versions) logically can't all be satisfied at once, this would make the check more restrictive than prior. So e.g. ghc-src-gen could be built but not uploaded. One could argue this would be cleaner, but my goal was to clarify existing checks and allow more clearly legit stuff to pass, not to add any new restrictions.

[fgaz]

Could you post hackage-tests results ( #7776 (comment) ) with master, master with this patch, and master with #7616 reverted?

[gbaz]

Master without this patch has 1116 build impossible. With it, there are only 525 build impossible, with the remaining 591 are now marked as suspicious.

So that seems about right.

[fgaz]

What is the number with #7616 reverted?

[gbaz]

Damn it, i ran that test the other day and then forgot to post the results, which are now on a lost terminal. But the build impossible was no less than with this patch, and perhaps even the exact same -- i.e. the results tied out as one would hope.

[jneira]

@Mergifyio rebase master

[mergify]

rebase master

✅ Branch has been successfully rebased

[jneira]

not fully understand the code neither but trust it 🙂
but I think this really need a changelog entry and a regression test, based in the hashable case f.e., bonus if it tests the warning

[jneira]

I think this really need a changelog entry and a regression test, based in the hashable case f.e., bonus if it tests the warning

[jneira]

@Mergifyio rebase master

[mergify]

rebase master

✅ Branch has been successfully rebased

[jneira]

What is the number with #7616 reverted?

Full hackage test check data for each case at index 2022-02-28T05:16:39Z

• with master:

D:\ws\haskell\cabal>cabal run Cabal-tests:hackage-tests -- check
Up to date
Reading index from: D:\cabal\packages\hackage.haskell.org\01-index.tar
157946 files processed
42067 files have lexer/parser warnings
115687 files have check warnings
1118 build impossible
7980 build warning
51952 build dist suspicious
36949 build dist suspicious warning
17688 build dist inexcusable

• with this pr:

157946 files processed
42067 files have lexer/parser warnings
115687 files have check warnings
525 build impossible
7980 build warning
52545 build dist suspicious
36949 build dist suspicious warning
17688 build dist inexcusable

• with handle conditionals in duplicate module checks #7616 reverted (https://github.com/jneira/cabal/tree/check-7616-reverted)

157946 files processed
42067 files have lexer/parser warnings
114905 files have check warnings
336 build impossible
7980 build warning
51952 build dist suspicious
36949 build dist suspicious warning
17688 build dist inexcusable

//cc @fgaz

[gbaz]

@Mergifyio rebase master

[mergify]

rebase master

✅ Branch has been successfully rebased

[jneira]

lgtm, thanks for the fix

[andreasabel]

Great to see this fixed!

BEFORE:

Configuring library for snap-server-1.1.2.0..
Error: cabal: Duplicate modules in library: System.SendFile

Duplicate modules in executable: System.SendFile

Duplicate modules in executable: System.SendFile

Duplicate modules in test suite: System.SendFile, System.SendFile.Tests

AFTER: works!