Fix #171 and bump to proto-lens-protoc-0.3.1.1. (#193)

With `Cabal-2.0` and later, put the autogenerated modules of each component
(library, executables, tests, benchmarks) in their own directory.  This
eliminates some confusing warnings and/or build errors.

(Note: this required a change to the release script for generating sdists;
long-term we should replace it with something like #185.)

With `Cabal-1.*`, continue to put all autogenerated modules together
in the same directory.

An example of the previous issue, when an executable depended on a library
in the same `.cabal` file and the library provided proto modules:
```
<no location info>: warning: [-Wmissing-home-modules]
    These modules are needed for compilation but not listed in your .cabal
    file's other-modules: Proto.Lib
```

proto-lens-protoc/Changelog.md

@@ -1,5 +1,8 @@
 # Changelog for `proto-lens-protoc`
 
+## v0.3.1.1
+- Fix management of generated files between Cabal components (#171).
+
 ## v0.3.1.0
 - Bump the dependency on `base` for `ghc-8.4.2`.
 - Bump the dependency to `Cabal-2.2.*`.

proto-lens-protoc/package.yaml

@@ -1,5 +1,5 @@
 name: proto-lens-protoc
-version: '0.3.1.0'
+version: '0.3.1.1'
 synopsis: Protocol buffer compiler for the proto-lens library.
 description: >
   Turn protocol buffer files (.proto) into Haskell files (.hs) which
@@ -37,6 +37,7 @@ library:
     - lens-labels == 0.2.*
     - pretty == 1.1.*
     - process >= 1.2 && < 1.7
+    - temporary == 1.2.*
   exposed-modules:
     - Data.ProtoLens.Compiler.Combinators
     - Data.ProtoLens.Compiler.Definitions

proto-lens-protoc/src/Data/ProtoLens/Setup.hs

@@ -26,12 +26,11 @@ module Data.ProtoLens.Setup
 
 import Control.DeepSeq (force)
 import Control.Monad (filterM, forM_, guard, when)
-#if MIN_VERSION_Cabal(2,0,0)
-import qualified Data.Map as Map
-#endif
 import qualified Data.ByteString as BS
+import qualified Data.Map as Map
 import Data.Maybe (maybeToList)
 import qualified Data.Set as Set
+import qualified Data.Text as T
 import Distribution.ModuleName (ModuleName)
 import qualified Distribution.ModuleName as ModuleName
 import qualified Distribution.InstalledPackageInfo as InstalledPackageInfo
@@ -56,6 +55,7 @@ import Distribution.Simple.LocalBuildInfo
     ( LocalBuildInfo(..)
     , absoluteInstallDirs
     , ComponentName(..)
+    , ComponentLocalBuildInfo
     , componentPackageDeps
 #if MIN_VERSION_Cabal(2,0,0)
     , allComponentsInBuildOrder
@@ -78,21 +78,23 @@ import Distribution.Simple
 import Distribution.Verbosity (Verbosity)
 import System.FilePath
     ( (</>)
+    , (<.>)
     , equalFilePath
     , isRelative
     , makeRelative
     , takeDirectory
     , takeExtension
     )
 import System.Directory
-    ( createDirectoryIfMissing
+    ( copyFile
+    , createDirectoryIfMissing
     , doesDirectoryExist
     , doesFileExist
     , findExecutable
     , removeDirectoryRecursive
-    , renameFile
     )
 import System.IO (hPutStrLn, stderr)
+import System.IO.Temp (withSystemTempDirectory)
 import System.Process (callProcess)
 
 import qualified Data.ProtoLens.Compiler.Plugin as Plugin
@@ -158,15 +160,11 @@ generatingProtos root = generatingSpecificProtos root getProtos
     getProtos l = do
       -- Replicate Cabal's own logic for parsing file globs.
       files <- concat <$> mapM matchFileGlob (extraSrcFiles $ localPkgDescr l)
-      let activeModules = Set.fromList $ collectActiveModules l
-      pure . filter (\f -> relativeFileToProtoModule f
-                            `Set.member` activeModules)
+      pure
            . filter (\f -> takeExtension f == ".proto")
            . map (makeRelative root)
            . filter (isSubdirectoryOf root)
            $ files
-    relativeFileToProtoModule
-        = ModuleName.fromString . Plugin.moduleNameStr "Proto"
 
 
 -- | Augment the given 'UserHooks' to auto-generate Haskell files from the
@@ -204,51 +202,69 @@ generatingSpecificProtos root getProtos hooks = hooks
     generate l = getProtos l >>= generateSources root l
 
 -- | Generate Haskell source files for the given input .proto files.
+--
+-- Process all the proto files that are referenced in the exposed-modules
+-- or other-modules of some "active" component, and write them all to a
+-- single temporary directory.  (For example, passing --no-enable-tests
+-- makes all test-suite components inactive.)
+--
+-- Then, for each active component, copy the corresponding module files
+-- over to its specific autogen directory (if Cabal-2.*) or to the global
+-- autogen directory (if Cabal-1.*).  However, don't actually do the copy
+-- if it's the same as what's already there.  This way, we don't needlessly
+-- touch the generated .hs files when nothing changes, and thus don't
+-- needlessly make GHC recompile them (as it considers their modification
+-- times for that).
 generateSources :: FilePath -- ^ The root directory
                 -> LocalBuildInfo
                 -> [FilePath] -- ^ Proto files relative to the root directory.
                 -> IO ()
-generateSources _ _ [] = return ()
-generateSources root l files = do
+generateSources root l files = withSystemTempDirectory "protoc-out" $ \tmpDir -> do
     -- Collect import paths from build-depends of this package.
     importDirs <- filterM doesDirectoryExist
                      [ InstalledPackageInfo.dataDir info </> protoLensImportsPrefix
                      | info <- collectDeps l
                      ]
-    -- Generate .hs files into a temporary directory, then move them over
-    -- to the target (autogen) directory only if they are different from
-    -- what's already there. This way, we don't needlessly touch the generated
-    -- .hs files when nothing changes, and thus don't needlessly make GHC
-    -- recompile them (as it considers their modification times for that).
-    let tmpAutogenModulesDir = autogenModulesDir l ++ "-protoc-tmpOutDir"
-    -- Generate .hs files into temp dir.
-    generateProtosWithImports (root : importDirs) tmpAutogenModulesDir
+    -- Generate .hs files for all active components into a single temporary
+    -- directory.
+    let activeModules = collectActiveModules l
+    let allModules = Set.fromList . concat . map snd $ activeModules
+    let usedInComponent f = ModuleName.fromString (Plugin.moduleNameStr "Proto" f)
+                          `Set.member` allModules
+    generateProtosWithImports (root : importDirs) tmpDir
                               -- Applying 'root </>' does nothing if the path is already
                               -- absolute.
-                              (map (root </>) files)
-    -- Discover generated files.
-    -- `getDirectoryContentsRecursive` is lazy IO, so we `force` through
-    -- the list to make it strict hereinafter.
-    !generatedFiles <- force <$> getDirectoryContentsRecursive tmpAutogenModulesDir
-    -- Move files to autogen dir only if file contents are different.
-    forM_ generatedFiles $ \pathRelativeToTmpDir -> do
-        let sourcePath = tmpAutogenModulesDir </> pathRelativeToTmpDir
-        let targetPath = autogenModulesDir l </> pathRelativeToTmpDir
-        identical <- do
-            targetExists <- doesFileExist targetPath
-            if not targetExists
-                then return False
-                else do
-                    -- This could be done in a streaming fashion,
-                    -- but since the .hs files usually easily fit
-                    -- into RAM, this is OK.
-                    sourceContents <- BS.readFile sourcePath
-                    targetContents <- BS.readFile targetPath
-                    return (sourceContents == targetContents)
-        -- Do the move if necessary.
-        when (not identical) $ do
-            createDirectoryIfMissing True (takeDirectory targetPath)
-            renameFile sourcePath targetPath
+                              $ map (root </>) $ filter usedInComponent files
+    -- Copy each active component's files over to its autogen directory, but
+    -- only if they've changed since last time.
+    forM_ activeModules $ \(compBI, mods) -> forM_ mods $ \m -> do
+          let f = T.unpack (Plugin.outputFilePath $ ModuleName.toFilePath m)
+          let sourcePath = tmpDir </> f
+          sourceExists <- doesFileExist sourcePath
+          when sourceExists $ do
+            let dest = autogenComponentModulesDir l compBI </> f
+            copyIfDifferent sourcePath dest
+
+-- Note: we do a copy rather than a move since a given module may be used in
+-- more than one component.
+copyIfDifferent :: FilePath -> FilePath -> IO ()
+copyIfDifferent sourcePath targetPath = do
+    targetExists <- doesFileExist targetPath
+    identical <- do
+        if not targetExists
+            then return False
+            else do
+                -- This could be done in a streaming fashion,
+                -- but since the .hs files usually easily fit
+                -- into RAM, this is OK.
+                sourceContents <- BS.readFile sourcePath
+                targetContents <- BS.readFile targetPath
+                return (sourceContents == targetContents)
+    -- Do the move if necessary.
+    when (not identical) $ do
+        createDirectoryIfMissing True (takeDirectory targetPath)
+        copyFile sourcePath targetPath
+
 
 -- | Copy each .proto file into the installed "data-dir" path,
 -- so that it can be included by other packages that depend on this one.
@@ -281,22 +297,30 @@ protoLensImportsPrefix = "proto-lens-imports"
 fudgePackageDesc :: LocalBuildInfo -> PackageDescription -> PackageDescription
 fudgePackageDesc lbi p = p
     { library =
-        (\lib -> lib { libBuildInfo = fudgeBuildInfo (libBuildInfo lib) })
+        (\lib -> lib { libBuildInfo = fudgeBuildInfo CLibName $ libBuildInfo lib })
             <$> library p
     , executables =
-        (\exe -> exe { buildInfo = fudgeBuildInfo (buildInfo exe) })
+        (\exe -> exe { buildInfo = fudgeBuildInfo (CExeName $ exeName exe)
+                                        $ buildInfo exe })
             <$> executables p
     , testSuites =
-        (\test -> test { testBuildInfo = fudgeBuildInfo (testBuildInfo test) })
+        (\test -> test { testBuildInfo = fudgeBuildInfo (CTestName $ testName test)
+                                            $ testBuildInfo test })
             <$> testSuites p
     , benchmarks =
         (\bench -> bench { benchmarkBuildInfo =
-                              fudgeBuildInfo (benchmarkBuildInfo bench) })
+                              fudgeBuildInfo (CBenchName $ benchmarkName bench)
+                                  $ benchmarkBuildInfo bench })
             <$> benchmarks p
     }
   where
-    fudgeBuildInfo bi =
-        bi { hsSourceDirs = autogenModulesDir lbi : hsSourceDirs bi }
+    comps = allComponents lbi
+    fudgeBuildInfo n bi
+        | Just compLBI <- Map.lookup n comps
+            = bi { hsSourceDirs = autogenComponentModulesDir lbi compLBI
+                                    : hsSourceDirs bi }
+        | otherwise = bi -- Could happen if a component isn't active; try
+                         -- anyway and see whether Cabal complains later on.
 
 -- | Returns whether the @root@ is a parent folder of @f@.
 isSubdirectoryOf :: FilePath -> FilePath -> Bool
@@ -331,6 +355,7 @@ generateProtosWithImports
     -> FilePath -- ^ The output directory for the generated Haskell files.
     -> [FilePath] -- ^ The .proto files to process.
     -> IO ()
+generateProtosWithImports _ _ [] = return ()
 generateProtosWithImports imports output files = do
     protoLensProtoc
         <- findExecutableOrDie "proto-lens-protoc"
@@ -366,25 +391,22 @@ findExecutableOrDie name debugMsg = do
 -- | Collect all the module names that we need to build.
 -- For example: only include test-suites if we're building with tests enabled
 -- (e.g., `stack test` vs `stack build`).
-collectActiveModules :: LocalBuildInfo -> [ModuleName]
-collectActiveModules l = let
-    in (activeLib >>= exposedModules)
-        ++ concatMap otherModules
-            (concat
-                [ libBuildInfo <$> activeLib
-                , buildInfo <$> activeExes
-                , testBuildInfo <$> activeTests
-                , benchmarkBuildInfo <$> activeBenchmarks
-                ])
+collectActiveModules
+    :: LocalBuildInfo -> [(ComponentLocalBuildInfo, [ModuleName])]
+collectActiveModules l = map (\(n, l) -> (l, f n)) $ Map.toList $ allComponents l
   where
     p = localPkgDescr l
-    activeLib = guard (active CLibName) >> maybeToList (library p)
-    activeExes = filter (active . CExeName . exeName) $ executables p
-    activeTests = filter (active . CTestName . testName) $ testSuites p
-    activeBenchmarks = filter (active . CBenchName . benchmarkName)
-                          $ benchmarks p
-    comps = Set.fromList $ allComponentNames l
-    active = (`Set.member` comps)
+    f CLibName = maybeToList (library p) >>= \l -> exposedModules l
+                                                    ++ otherModules (libBuildInfo l)
+    f (CExeName n) = otherModules . buildInfo $ exes Map.! n
+    f (CTestName n) = otherModules . testBuildInfo $ tests Map.! n
+    f (CBenchName n) = otherModules . benchmarkBuildInfo $ benchs Map.! n
+#if MIN_VERSION_Cabal(2,0,0)
+    f _ = []  -- TODO: other lib kinds; for now just suppress the warning
+#endif
+    exes = Map.fromList [(exeName e, e) | e <- executables p]
+    tests = Map.fromList [(testName e, e) | e <- testSuites p]
+    benchs = Map.fromList [(benchmarkName e, e) | e <- benchmarks p]
 
 -------------------------------------------------------
 -- Compatibility layer between Cabal-1.* and Cabal-2.*
@@ -405,21 +427,25 @@ collectDeps l = do
 #endif
 
 -- | All the components that will be built by this Cabal command.
-allComponentNames :: LocalBuildInfo -> [ComponentName]
+allComponents :: LocalBuildInfo -> Map.Map ComponentName ComponentLocalBuildInfo
 #if MIN_VERSION_Cabal(2,0,0)
-allComponentNames l = Map.keys $ componentNameMap l
+allComponents l = fmap requireOne $ componentNameMap l
+  where
+    -- TODO: this doesn't support Backpack, which can have more than one
+    -- ComponentLocalBuildInfo associated with a name.
+    requireOne [x] = x
+    requireOne xs = error $ "Data.ProtoLens.Setup.allComponents: expected one "
+                          ++ "component per name, got " ++ show xs
+
 #else
-allComponentNames l = [c | (c, _, _) <- componentsConfigs l]
+allComponents l = Map.fromList [(c, b) | (c, b, _) <- componentsConfigs l]
 #endif
 
--- | Get the package-level "autogen" directory where we're putting the
--- generated .hs files.  (The 'BuildPaths.autogenModulesDir' file was
--- deprecated in Cabal-2.0 in favor of module-specific directories
--- (@autogenComponentModulesDir@), but our setup currently needs the more
--- global location.)
-autogenModulesDir :: LocalBuildInfo -> FilePath
+-- | Get the component-level "autogen" directory where we're putting the
+-- generated .hs files.  (For Cabal-1.0, use the shared 'BuildPaths.autogenModulesDir'.)
+autogenComponentModulesDir :: LocalBuildInfo -> ComponentLocalBuildInfo -> FilePath
 #if MIN_VERSION_Cabal(2,0,0)
-autogenModulesDir = BuildPaths.autogenPackageModulesDir
+autogenComponentModulesDir = BuildPaths.autogenComponentModulesDir
 #else
-autogenModulesDir = BuildPaths.autogenModulesDir
+autogenComponentModulesDir lbi _ = BuildPaths.autogenModulesDir lbi
 #endif

proto-lens-tests/package.yaml

@@ -38,6 +38,7 @@ library:
   source-dirs: src
   exposed-modules:
     - Data.ProtoLens.TestUtil
+    - Proto.Lib
 
 tests:
 
@@ -263,3 +264,11 @@ tests:
       - proto-lens-tests
     other-modules:
       - Proto.Pathological
+
+  dependent_test:
+    main: dependent_test.hs
+    source-dirs: tests
+    dependencies:
+      - proto-lens-tests
+    other-modules:
+      - Proto.Dependent

proto-lens-tests/tests/dependent.proto

@@ -0,0 +1,9 @@
+syntax = "proto3";
+
+import "lib.proto";
+
+package dependent;
+
+message Dependent {
+  lib.LibMessage x = 1;
+}

proto-lens-tests/tests/dependent_test.hs

@@ -0,0 +1,15 @@
+module Main (main) where
+
+import Data.ProtoLens (def)
+import Test.Framework (defaultMain)
+import Test.Framework.Providers.HUnit (testCase)
+import Test.HUnit ((@=?))
+
+import Proto.Dependent (Dependent)
+import Proto.Lib (LibMessage)
+
+main :: IO ()
+main = defaultMain
+    [ testCase "LibMessage" $ (def :: LibMessage) @=? def
+    , testCase "Dependent" $ (def :: Dependent) @=? def
+    ]

proto-lens-tests/tests/lib.proto

@@ -0,0 +1,5 @@
+syntax = "proto3";
+
+package lib;
+
+message LibMessage {}

release.sh

@@ -7,11 +7,15 @@ export CABAL_SANDBOX_CONFIG=$PWD/cabal.sandbox.config
 
 for p in lens-labels proto-lens proto-lens-protoc
 do
-  (cd $p && cabal install)
+  (cd $p && \
+    cabal install --force-reinstall)
 done
 
 for p in proto-lens-combinators proto-lens-protobuf-types
 do
-  (cd $p && cabal configure && cabal sdist)
+  (cd $p && \
+    cabal install --enable-tests --enable-benchmarks --only-dependencies && \
+    cabal configure --enable-tests --enable-benchmarks && \
+    cabal sdist)
 done