haskell / cabal

Official upstream development repository for Cabal and cabal-install
https://haskell.org/cabal
Other
1.62k stars 697 forks source link

Cabal sdist concatenates names of conditional Main module names #9724

Open amigalemming opened 8 months ago

amigalemming commented 8 months ago

A minimal Cabal package description:

Cabal-Version:      2.4
Name:               cabal-cond-main
Version:            0.1

Flag alternative
  Description: Alternative implementation
  Default: False

Executable cabal-cond-main
  If flag(alternative)
    Main-Is: MainA.hs
  Else
    Main-Is: MainB.hs

I run

$ cabal-3.10 sdist
Error: cabal-3.10: MainA.hsMainB.hs doesn't exist

I guess there is some Monoid instances screwed up.

amigalemming commented 8 months ago

Maybe the deeper problem is that PackageDescription can only store one Main-Is per executable.

amigalemming commented 8 months ago

I tried this work-around:

Cabal-Version:      2.4
Name:               cabal-cond-main
Version:            0.1

Flag alternative
  Description: Alternative implementation
  Default: False

Executable cabal-cond-main
  Main-Is: Main.hs
  If flag(alternative)
    Hs-Source-Dirs: src-a
  Else
    Hs-Source-Dirs: src-b

This lacks src-b/Main.hs in the tarball. I can fix this with Extra-Source-Files, though.

ffaf1 commented 8 months ago

See #9659. Thanks for having provided a workaround.

Again a bug caused by flattenPackageDescription.

amigalemming commented 8 months ago

Thanks for having provided a workaround.

This is how we ensured inclusion of all files before Cabal-1.18 or so, because at this time Cabal chose only one of each branches in an If/Else-tree.

amigalemming commented 8 months ago

I tried the first package description, again, with cabal-install/HEAD:

$ cabal-head sdist
Ambiguous values for modulePath field: '"MainA.hs"' and '"MainB.hs"'
CallStack (from HasCallStack):
  error, called at src/Distribution/Types/UnqualComponentName.hs:128:7 in Cabal-syntax-3.11.0.0-d54fadae152d752a73dcccd915a662b3b5c3d9fb8cb0cf56dac9da702763d64d:Distribution.Types.UnqualComponentName
gbaz commented 8 months ago

Maybe the deeper problem is that PackageDescription can only store one Main-Is per executable.

I think the right thing is to make main-is a list rather than a single value, and then to enforce at build time (and check time) that the list is single-valued.

amigalemming commented 8 months ago

I think the right thing is to make main-is a list rather than a single value, and then to enforce at build time (and check time) that the list is single-valued.

Maybe PackageDescription is simply not the right structure to collect the files that should go into the tarball?

gbaz commented 8 months ago

Maybe PackageDescription is simply not the right structure to collect the files that should go into the tarball?

Actually yeah, good point.

We currently have listPackageSourcesWithDie verbosity thisDie dir (flattenPackageDescription gpd) knownSuffixHandlers

We should arguably instead replace the call to flattenPackageDescription with something that directly recursively traverses the generic package description collecting files -- this will be slightly more efficient too!

In particular we can foldCondTree over every condTree within the description: https://hackage.haskell.org/package/Cabal-syntax-3.10.2.0/docs/Distribution-Types-CondTree.html#v:foldCondTree

(note that the "inclusive" and "exclusive" merges are passed in, so we can actually make them the same -- list append -- for both)