language: check dflags for errors (#2748)

* language: check dflags for errors We add a check when we build the dflags for cases that will lead to a failed build and emmit a clearer error message. Currently this only includes a check, to see whether the current installed unit id is also imported as a package from the package database. * exclude ghc-prim from check * exclude code generation from dflag check * add an internal option to turn dflags check off
digital-asset · Sep 6, 2019 · aa1e951 · aa1e951
1 parent 2e29d3c
commit aa1e951
Showing 5 changed files with 42 additions and 2 deletions.
diff --git a/compiler/damlc/BUILD.bazel b/compiler/damlc/BUILD.bazel
@@ -303,5 +303,6 @@ daml_doc_test(
     name = "daml-stdlib-doctest",
     package_name = "daml-stdlib",
     srcs = ["//compiler/damlc/daml-stdlib-src"],
+    flags = ["--no-dflags-check"],
     ignored_srcs = ["LibraryModules.daml"],
 )
diff --git a/compiler/damlc/daml-opts/daml-opts-types/DA/Daml/Options/Types.hs b/compiler/damlc/daml-opts/daml-opts-types/DA/Daml/Options/Types.hs
@@ -63,6 +63,9 @@ data Options = Options
   -- ^ Information about dlint usage.
   , optIsGenerated :: Bool
     -- ^ Whether we're compiling generated code. Then we allow internal imports.
+  , optDflagCheck :: Bool
+    -- ^ Whether to check dflags. In some cases we want to turn this check of. For example when
+    -- migrating or running the daml doc test.
   , optCoreLinting :: Bool
     -- ^ Whether to enable linting of the generated GHC Core. (Used in testing.)
   , optHaddock :: Haddock
@@ -150,6 +153,7 @@ defaultOptions mbVersion =
         , optScenarioValidation = ScenarioValidationFull
         , optDlintUsage = DlintDisabled
         , optIsGenerated = False
+        , optDflagCheck = True
         , optCoreLinting = False
         , optHaddock = Haddock False
         }

diff --git a/compiler/damlc/daml-opts/daml-opts/DA/Daml/Options.hs b/compiler/damlc/daml-opts/daml-opts/DA/Daml/Options.hs
@@ -44,7 +44,8 @@ toCompileOpts options@Options{..} reportProgress =
                 setupDamlGHC options
                 GHC.getSession
             pkg <- liftIO $ generatePackageState optPackageDbs optHideAllPkgs $ map (second toRenaming) optPackageImports
-            return env{hsc_dflags = setPackageDynFlags pkg $ hsc_dflags env}
+            dflags <- liftIO $ checkDFlags options $ setPackageDynFlags pkg $ hsc_dflags env
+            return env{hsc_dflags = dflags}
       , optPkgLocationOpts = HieCore.IdePkgLocationOptions
           { optLocateHieFile = locateInPkgDb "hie"
           , optLocateSrcFile = locateInPkgDb "daml"
@@ -267,3 +268,21 @@ setupDamlGHC options@Options{..} = do
 
     modifySession $ \h ->
       h { hsc_dflags = dflags', hsc_IC = (hsc_IC h) {ic_dflags = dflags' } }
+
+-- | Check for bad @DynFlags@.
+-- Checks:
+--    * thisInstalledUnitId not contained in loaded packages.
+checkDFlags :: Options -> DynFlags -> IO DynFlags
+checkDFlags Options {..} dflags@DynFlags {..}
+    | not optDflagCheck || thisInstalledUnitId == toInstalledUnitId primUnitId =
+        pure dflags
+    | otherwise = do
+        case lookupPackage dflags $
+             DefiniteUnitId $ DefUnitId thisInstalledUnitId of
+            Nothing -> pure dflags
+            Just _conf ->
+                fail $
+                "Package " <> installedUnitIdString thisInstalledUnitId <>
+                " imports a package with the same name. \
+            \ Please check your dependencies and rename the package you are compiling \
+            \ or the dependency."
diff --git a/compiler/damlc/lib/DA/Cli/Damlc.hs b/compiler/damlc/lib/DA/Cli/Damlc.hs
@@ -706,6 +706,7 @@ execMigrate projectOpts opts0 inFile1_ inFile2_ mbDir = do
                         , optPackageDbs = [projectPkgDb]
                         , optIfaceDir = Just (dbPath </> installedUnitIdString iuid)
                         , optIsGenerated = True
+                        , optDflagCheck = False
                         , optMbPackageName = Just $ installedUnitIdString iuid
                         }
                 withDamlIdeState opts' loggerH diagnosticsLogger $ \ide ->
@@ -874,6 +875,7 @@ optionsParser numProcessors enableScenarioService parsePkgName = Options
     <*> pure (optScenarioValidation $ defaultOptions Nothing)
     <*> dlintUsageOpt
     <*> pure False
+    <*> optNoDflagCheck
     <*> pure False
     <*> pure (Haddock False)
   where
@@ -934,6 +936,15 @@ optionsParser numProcessors enableScenarioService parsePkgName = Options
             ]
 
 
+    optNoDflagCheck :: Parser Bool
+    optNoDflagCheck =
+      flag True False $
+      help "Dont check generated GHC DynFlags for errors." <>
+      long "no-dflags-check" <>
+      internal
+
+
+
 optGhcCustomOptions :: Parser [String]
 optGhcCustomOptions =
     fmap concat $ many $

diff --git a/rules_daml/daml.bzl b/rules_daml/daml.bzl
@@ -149,10 +149,11 @@ def _daml_doctest_impl(ctx):
       set -eou pipefail
       DAMLC=$(rlocation $TEST_WORKSPACE/{damlc})
       rlocations () {{ for i in $@; do echo $(rlocation $TEST_WORKSPACE/$i); done; }}
-      $DAMLC doctest --package-name {package_name}-`cat $(rlocation $TEST_WORKSPACE/{version_file})` $(rlocations "{files}")
+      $DAMLC doctest {flags} --package-name {package_name}-`cat $(rlocation $TEST_WORKSPACE/{version_file})` $(rlocations "{files}")
     """.format(
         damlc = ctx.executable.damlc.short_path,
         package_name = ctx.attr.package_name,
+        flags = " ".join(ctx.attr.flags),
         version_file = ctx.file.version.path,
         files = " ".join([
             f.short_path
@@ -189,6 +190,10 @@ daml_doc_test = rule(
             allow_files = True,
             default = Label("//compiler/damlc"),
         ),
+        "flags": attr.string_list(
+            default = [],
+            doc = "Flags for damlc invokation.",
+        ),
         "package_name": attr.string(),
         "version": attr.label(
             allow_single_file = True,