Autogenerated modules may shadow real sources

[sergv]

Synopsis

Since 1007707 the modules listed in autogen-modules are processed by passing them to the relevant preprocessor (e.g. alex) and then the resulting .hs files are put into build directory (e.g. dist-newstyle/build/x86_64-linux/ghc-9.4.4/Cabal-3.9.0.0/build/). Definitely an improvement compared to putting them under src, but still not foolproof.

Consider cabal file that has either pregenerated module or autogen-modules depending on wether a flag is passed:

...
extra-source-files:
  src-inputs/**/*.hgen

build-type: Custom

custom-setup
  setup-depends: Cabal >= 3.8, base, directory

flag generate
  description:
    Generate files instead of using pregenerated ones
  default:
    False
  manual:
    True

library
  exposed-modules:
    Lib
    Generated

  -- This directory contains just Lib.hs
  hs-source-dirs:
    src

  if flag(generate)
    -- This directory contains Generated.hgen that custem Setup.hs knows
    -- how to transform to .hs file (simplest version just copies it).
    hs-source-dirs:
      src-inputs
    autogen-modules:
      Generated
  else
    -- Thtis directory contains Generated.hs module constructed by hand.
    -- In real world would be used for bootstrapping or something where we don't want to run preprocessor
    hs-source-dirs:
      gen
...

I would expect that the Generated module will pick up its source on each build depending on flag value, e.g.

$ cabal build -fgenerate
$ cabal build -f-generate

However currently cabal build -fgenerate will create Generated.hs under the build/ directory that won't get invalidated when I do cabal build -f-generate so the generated version will be picked up even though the pregenerated one should've been used.

Also see the discussion at the end.

Reproducer

Following patch adds full test case to the cabal test suite that demonstrates the problem (use git apply-patch to apply). It runs cabal run -fgenerate <exe> followed by cabal run -f-generate <exe> and captures their outputs. The Generated.hs module, which could be either really generated or picked up as is from a pregenerated source, contains a string identifying whether it was generated or pregenerated. Package's <exe> just prints that string. The generator program is a dummy identity transform for simplicity.

From 2b7108a0c1583cf7f432a577d3286497c2d8974a Mon Sep 17 00:00:00 2001
From: Sergey Vinokurov <serg.foo@gmail.com>
Date: Wed, 18 Jan 2023 00:17:01 +0000
Subject: [PATCH] Add test

---
 .../AutogenModulesToggling/Main.hs            | 12 ++++
 .../AutogenModulesToggling/Setup.hs           | 24 ++++++++
 .../AutogenModulesToggling/cabal.project      |  1 +
 .../AutogenModulesToggling/cabal.test.hs      |  6 ++
 .../AutogenModulesToggling/gen/Generated.hs   |  4 ++
 .../src-inputs/Generated.hgen                 |  4 ++
 .../AutogenModulesToggling/src/Lib.hs         |  6 ++
 .../AutogenModulesToggling/test.cabal         | 57 +++++++++++++++++++
 8 files changed, 114 insertions(+)
 create mode 100644 cabal-testsuite/PackageTests/AutogenModulesToggling/Main.hs
 create mode 100644 cabal-testsuite/PackageTests/AutogenModulesToggling/Setup.hs
 create mode 100644 cabal-testsuite/PackageTests/AutogenModulesToggling/cabal.project
 create mode 100644 cabal-testsuite/PackageTests/AutogenModulesToggling/cabal.test.hs
 create mode 100644 cabal-testsuite/PackageTests/AutogenModulesToggling/gen/Generated.hs
 create mode 100644 cabal-testsuite/PackageTests/AutogenModulesToggling/src-inputs/Generated.hgen
 create mode 100644 cabal-testsuite/PackageTests/AutogenModulesToggling/src/Lib.hs
 create mode 100644 cabal-testsuite/PackageTests/AutogenModulesToggling/test.cabal

diff --git a/cabal-testsuite/PackageTests/AutogenModulesToggling/Main.hs b/cabal-testsuite/PackageTests/AutogenModulesToggling/Main.hs
new file mode 100644
index 000000000..b14c74931
--- /dev/null
+++ b/cabal-testsuite/PackageTests/AutogenModulesToggling/Main.hs
@@ -0,0 +1,12 @@
+module Main (main) where
+
+import Lib (bar)
+
+main :: IO ()
+main = do
+  -- Make sure cabal sees this because this test is about which
+  -- 'Generated' module the 'Lib' was compiled against.
+  putStrLn "-----BEGIN CABAL OUTPUT-----"
+  putStrLn bar
+  putStrLn "-----END CABAL OUTPUT-----"
+
diff --git a/cabal-testsuite/PackageTests/AutogenModulesToggling/Setup.hs b/cabal-testsuite/PackageTests/AutogenModulesToggling/Setup.hs
new file mode 100644
index 000000000..2eab853cd
--- /dev/null
+++ b/cabal-testsuite/PackageTests/AutogenModulesToggling/Setup.hs
@@ -0,0 +1,24 @@
+
+module Main (main) where
+
+import Distribution.Simple
+import Distribution.Simple.LocalBuildInfo
+import Distribution.Simple.PreProcess
+import Distribution.Simple.Program
+import Distribution.Types.BuildInfo
+import Distribution.Verbosity
+
+import System.Directory
+
+ppHGen :: BuildInfo -> LocalBuildInfo -> ComponentLocalBuildInfo -> PreProcessor
+ppHGen _bi lbi _clbi = PreProcessor
+  { platformIndependent = True
+  , ppOrdering          = unsorted
+  , runPreProcessor     = mkSimplePreProcessor $ \inFile outFile verbosity ->
+      copyFile inFile outFile
+  }
+
+main :: IO ()
+main = defaultMainWithHooks simpleUserHooks
+  { hookedPreProcessors = ("hgen", ppHGen) : hookedPreProcessors simpleUserHooks
+  }
diff --git a/cabal-testsuite/PackageTests/AutogenModulesToggling/cabal.project b/cabal-testsuite/PackageTests/AutogenModulesToggling/cabal.project
new file mode 100644
index 000000000..52db9d1bc
--- /dev/null
+++ b/cabal-testsuite/PackageTests/AutogenModulesToggling/cabal.project
@@ -0,0 +1 @@
+packages: test.cabal
diff --git a/cabal-testsuite/PackageTests/AutogenModulesToggling/cabal.test.hs b/cabal-testsuite/PackageTests/AutogenModulesToggling/cabal.test.hs
new file mode 100644
index 000000000..c62975596
--- /dev/null
+++ b/cabal-testsuite/PackageTests/AutogenModulesToggling/cabal.test.hs
@@ -0,0 +1,6 @@
+import Test.Cabal.Prelude
+
+main :: IO ()
+main = cabalTest . recordMode RecordMarked $ do
+  cabal "v2-run" ["-fgenerate", "autogen-toggle-test"]
+  cabal "v2-run" ["-f-generate", "autogen-toggle-test"]
diff --git a/cabal-testsuite/PackageTests/AutogenModulesToggling/gen/Generated.hs b/cabal-testsuite/PackageTests/AutogenModulesToggling/gen/Generated.hs
new file mode 100644
index 000000000..08d4e8078
--- /dev/null
+++ b/cabal-testsuite/PackageTests/AutogenModulesToggling/gen/Generated.hs
@@ -0,0 +1,4 @@
+module Generated (foo) where
+
+foo :: String
+foo = "Prebuilt module, don't use in production"
diff --git a/cabal-testsuite/PackageTests/AutogenModulesToggling/src-inputs/Generated.hgen b/cabal-testsuite/PackageTests/AutogenModulesToggling/src-inputs/Generated.hgen
new file mode 100644
index 000000000..fa31359d3
--- /dev/null
+++ b/cabal-testsuite/PackageTests/AutogenModulesToggling/src-inputs/Generated.hgen
@@ -0,0 +1,4 @@
+module Generated (foo) where
+
+foo :: String
+foo = "Real module, ship to production"
diff --git a/cabal-testsuite/PackageTests/AutogenModulesToggling/src/Lib.hs b/cabal-testsuite/PackageTests/AutogenModulesToggling/src/Lib.hs
new file mode 100644
index 000000000..0e5ae8ead
--- /dev/null
+++ b/cabal-testsuite/PackageTests/AutogenModulesToggling/src/Lib.hs
@@ -0,0 +1,6 @@
+module Lib (bar) where
+
+import Generated (foo)
+
+bar :: String
+bar = "The module says: " ++ foo
diff --git a/cabal-testsuite/PackageTests/AutogenModulesToggling/test.cabal b/cabal-testsuite/PackageTests/AutogenModulesToggling/test.cabal
new file mode 100644
index 000000000..4ce680148
--- /dev/null
+++ b/cabal-testsuite/PackageTests/AutogenModulesToggling/test.cabal
@@ -0,0 +1,57 @@
+cabal-version: 3.0
+
+name: test
+version: 0.1
+category: Test
+maintainer: S
+synopsis: Test input
+description: Test input
+license: BSD-3-Clause
+
+extra-source-files:
+  src-inputs/**/*.hgen
+
+build-type: Custom
+
+custom-setup
+  setup-depends: Cabal >= 3.8, base, directory
+
+flag generate
+  description:
+    Generate files instead of using pregenerated ones
+  default:
+    False
+  manual:
+    True
+
+library
+  exposed-modules:
+    Lib
+    Generated
+
+  hs-source-dirs:
+    src
+
+  if flag(generate)
+    hs-source-dirs:
+      src-inputs
+    autogen-modules:
+      Generated
+    -- We don’t use any tools in this case but they’ll have to
+    -- go here
+    -- build-tool-depends:
+    --   alex:alex
+  else
+    hs-source-dirs:
+      gen
+
+  build-depends: base > 4
+  default-language: Haskell2010
+
+executable autogen-toggle-test
+  main-is: Main.hs
+  hs-source-dirs: .
+  default-language: Haskell2010
+  build-depends:
+    , base > 4
+    , test
-- 
2.38.1

When built with properly autogenerated module (-fgenerate) the example prints The module says: Real module, ship to production . When built with the prebuilt module (-f-generate) then it should print Prebuilt module, don't use in production.

Discussion

Currently the generated .hs gets picked up by GHC because its specified earlier than directories in src/ here https://github.com/haskell/cabal/blob/master/Cabal/src/Distribution/Simple/GHC/Internal.hs#L418. It seems src/ should've been specified first since if there's an explicit .hs file under src/ that then it should be picked up regardless of what's in build/.

[sergv]

Fixed with #8690.

[andreasabel]

In the light of #10060, the debated behavior here is maybe not a bug but simply a behavior that is beneficial in some situations but adversary in other situations.

So, I'd call #8690 not a bug-fix but just a behavioral change, and one should be aware that it fixes some workflows but breaks others.