search

haskell / cabal

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

priority of `build-tools`/`build-tool-depends` vs. pre-built `dist/build/*` in sdist #6029

Open hvr opened 5 years ago

hvr commented 5 years ago

Problem manifestation

Here's a case I stumbled over where I'm not sure we have the proper semantics:

$ cabal v2-repl -zb bytestring-lexing==0.4.0 -w ghc-8.0.2  -j1
Resolving dependencies...
Build profile: -w ghc-8.0.2 -O1
In order, the following will be built (use -v for more details):
 - bytestring-lexing-0.4.0 (lib:bytestring-lexing) (requires build)
 - fake-package-0 (lib) (first run)
Configuring bytestring-lexing-0.4.0...
Preprocessing library for bytestring-lexing-0.4.0..
Building library for bytestring-lexing-0.4.0..
[1 of 4] Compiling Data.ByteString.Lex.Internal ( src/Data/ByteString/Lex/Internal.hs, dist/build/Data/ByteString/Lex/Internal.o )

src/Data/ByteString/Lex/Internal.hs:28:5: warning: [-Wdeprecations]
    In the use of ‘inlinePerformIO’
    (imported from Data.ByteString.Internal):
    Deprecated: "If you think you know what you are doing, use 'unsafePerformIO'. If you are sure you know what you are doing, use 'unsafeDupablePerformIO'. If you enjoy sharing an address space with a malevolent agent of chaos, try 'accursedUnutterablePerformIO'."
[2 of 4] Compiling Data.ByteString.Lex.Lazy.Double ( dist/build/Data/ByteString/Lex/Lazy/Double.hs, dist/build/Data/ByteString/Lex/Lazy/Double.o )

dist/build/Data/ByteString/Lex/Lazy/Double.hs:328:31: error:
    • Couldn't match expected type ‘[AlexAcc
...

however, compare to

$ cabal get bytestring-lexing-0.4.0 && cd bytestring-lexing-0.4.0 && cabal v2-build -w ghc-8.0.2
Resolving dependencies...
Build profile: -w ghc-8.0.2 -O1
In order, the following will be built (use -v for more details):
 - bytestring-lexing-0.4.0 (lib:bytestring-lexing) (first run)
Configuring bytestring-lexing-0.4.0...
Preprocessing library for bytestring-lexing-0.4.0..
Building library for bytestring-lexing-0.4.0..
[1 of 4] Compiling Data.ByteString.Lex.Internal ( src/Data/ByteString/Lex/Internal.hs, /tmp/bytestring-lexing-0.4.0/dist-newstyle/build/x86_64-linux/ghc-8.0.2/bytestring-lexing-0.4.0/build/Data/ByteString/Lex/Internal.o )

src/Data/ByteString/Lex/Internal.hs:28:5: warning: [-Wdeprecations]
    In the use of ‘inlinePerformIO’
    (imported from Data.ByteString.Internal):
    Deprecated: "If you think you know what you are doing, use 'unsafePerformIO'. If you are sure you know what you are doing, use 'unsafeDupablePerformIO'. If you enjoy sharing an address space with a malevolent agent of chaos, try 'accursedUnutterablePerformIO'."
[2 of 4] Compiling Data.ByteString.Lex.Lazy.Double ( /tmp/bytestring-lexing-0.4.0/dist-newstyle/build/x86_64-linux/ghc-8.0.2/bytestring-lexing-0.4.0/build/Data/ByteString/Lex/Lazy/Double.hs, /tmp/bytestring-lexing-0.4.0/dist-newstyle/build/x86_64-linux/ghc-8.0.2/bytestring-lexing-0.4.0/build/Data/ByteString/Lex/Lazy/Double.o )

/tmp/bytestring-lexing-0.4.0/dist-newstyle/build/x86_64-linux/ghc-8.0.2/bytestring-lexing-0.4.0/build/Data/ByteString/Lex/Lazy/Double.hs:459:1: warning: [-Wtabs]
    Tab character found here, and in 44 further locations.
    Please use spaces instead.
[3 of 4] Compiling Data.ByteString.Lex.Integral ( src/Data/ByteString/Lex/Integral.hs, /tmp/bytestring-lexing-0.4.0/dist-newstyle/build/x86_64-linux/ghc-8.0.2/bytestring-lexing-0.4.0/build/Data/ByteString/Lex/Integral.o )
[4 of 4] Compiling Data.ByteString.Lex.Double ( /tmp/bytestring-lexing-0.4.0/dist-newstyle/build/x86_64-linux/ghc-8.0.2/bytestring-lexing-0.4.0/build/Data/ByteString/Lex/Double.hs, /tmp/bytestring-lexing-0.4.0/dist-newstyle/build/x86_64-linux/ghc-8.0.2/bytestring-lexing-0.4.0/build/Data/ByteString/Lex/Double.o )

/tmp/bytestring-lexing-0.4.0/dist-newstyle/build/x86_64-linux/ghc-8.0.2/bytestring-lexing-0.4.0/build/Data/ByteString/Lex/Double.hs:497:1: warning: [-Wtabs]
    Tab character found here, and in 44 further locations.
    Please use spaces instead.

src/Data/ByteString/Lex/Double.x:106:19: warning: [-Wdeprecations]
    In the use of ‘inlinePerformIO’
    (imported from Data.ByteString.Internal):
    Deprecated: "If you think you know what you are doing, use 'unsafePerformIO'. If you are sure you know what you are doing, use 'unsafeDupablePerformIO'. If you enjoy sharing an address space with a malevolent agent of chaos, try 'accursedUnutterablePerformIO'."

Diagnosis

The difference between the two builds is that in the first case, the pre-generated

scanner is compiled (NB: that alex was still provisioned by v2-build to the build-process (in vain though) because it was declared via build-tools; but it wasn't invoked) , which is not compatible with GHC 8.0.2; in the 2nd case however the build-tools: alex declared in the package description is honored, and the

is generated (and the pre-generated one in the dist/build/ folder is ignored).

So the issue here is whether re-running the pre-processors (when those are specified in build-tools) shall take priority over using pre-generated ones in the sdist inside the magic dist/build folder.

And this is currently handled inconsistently in the local (unpacked) vs non-local case by v2-build.

Suggested resolution

If build-tools for the respective pre-processor is specified in the .cabal file, v2-build shall consistently always re-generate the respective source and use it, regardless of its existence in a dist/build folder.

23Skidoo commented 5 years ago

Agreed that we should be consistent, and regenerating when build-tools are present makes sense.

hvr commented 5 years ago

Another example is haskell-src-exts-1.13.5 which

  1. declares a build-tools: happy
  2. has a pre-generated http://hackage.haskell.org/package/haskell-src-exts-1.13.5/src/dist/build/Language/Haskell/Exts/InternalParser.hs
  3. as well as the happy source: http://hackage.haskell.org/package/haskell-src-exts-1.13.5/src/src/Language/Haskell/Exts/InternalParser.ly

And fails to build as a dependency built non-locally (i.e. via cabal repl -zb haskell-src-exts==1.13.5 -w ghc-7.8.4) but would succeed when unpacked and build locally.