From e3830106ad9ae582b3aa7f96a31080adc506b5ba Mon Sep 17 00:00:00 2001 From: Moritz Kiefer Date: Wed, 11 Dec 2019 11:49:50 +0100 Subject: [PATCH] Fix package names in depends field in pkg configs (#3810) * Fix package names in depends field in pkg configs Previously, we derived this based on the DAR name which breaks if you use -o with rather confusing error messages. Now, we read it from the `Name` field in the manifest that we added in https://github.com/digital-asset/daml/pull/3805. CHANGELOG_BEGIN - [DAML Compiler] Fix an issue where transitive package dependencies resulted in packages not being found, if the DAR name was changed with `-o`. CHANGELOG_END * Fix package dependencies --- compiler/damlc/daml-compiler/BUILD.bazel | 3 + .../daml-compiler/src/DA/Daml/Compiler/Dar.hs | 54 +++++++++++++---- compiler/damlc/lib/DA/Cli/Damlc/Packaging.hs | 21 +------ compiler/damlc/tests/src/DA/Test/Packaging.hs | 58 +++++++++++++++++++ 4 files changed, 104 insertions(+), 32 deletions(-) diff --git a/compiler/damlc/daml-compiler/BUILD.bazel b/compiler/damlc/daml-compiler/BUILD.bazel index 62adf7ecab0..dca6d8f3dce 100644 --- a/compiler/damlc/daml-compiler/BUILD.bazel +++ b/compiler/damlc/daml-compiler/BUILD.bazel @@ -32,6 +32,7 @@ da_haskell_library( "utf8-string", "yaml", "zip", + "zip-archive", ], src_strip_prefix = "src", visibility = ["//visibility:public"], @@ -39,11 +40,13 @@ da_haskell_library( "//:sdk-version-hs-lib", "//compiler/daml-lf-ast", "//compiler/daml-lf-proto", + "//compiler/daml-lf-reader", "//compiler/damlc/daml-doctest", "//compiler/damlc/daml-ide-core", "//compiler/damlc/daml-opts:daml-opts-types", "//compiler/damlc/daml-preprocessor", "//compiler/scenario-service/client", + "//daml-assistant:daml-project-config", "//libs-haskell/da-hs-base", ], ) diff --git a/compiler/damlc/daml-compiler/src/DA/Daml/Compiler/Dar.hs b/compiler/damlc/daml-compiler/src/DA/Daml/Compiler/Dar.hs index d3505a1cb23..5271a3bde2f 100644 --- a/compiler/damlc/daml-compiler/src/DA/Daml/Compiler/Dar.hs +++ b/compiler/damlc/daml-compiler/src/DA/Daml/Compiler/Dar.hs @@ -12,17 +12,22 @@ module DA.Daml.Compiler.Dar , getDamlRootFiles , writeIfacesAndHie , mkConfFile + , expandSdkPackages ) where import qualified "zip" Codec.Archive.Zip as Zip +import qualified "zip-archive" Codec.Archive.Zip as ZipArchive import Control.Exception (assert) +import Control.Exception.Safe (handleIO) import Control.Monad.Extra import Control.Monad.IO.Class import Control.Monad.Trans.Class import Control.Monad.Trans.Maybe import qualified DA.Daml.LF.Ast as LF import DA.Daml.LF.Proto3.Archive (encodeArchiveAndHash) +import DA.Daml.LF.Reader (readDalfManifest, packageName) import DA.Daml.Options.Types +import DA.Daml.Project.Consts import qualified Data.ByteString as BS import qualified Data.ByteString.Lazy as BSL import qualified Data.ByteString.Lazy.Char8 as BSC @@ -146,7 +151,8 @@ buildDar service pkgConf@PackageConfigFields {..} ifDir dalfInput = do [ (T.pack $ unitIdString unitId, LF.dalfPackageBytes pkg, LF.dalfPackageId pkg) | (unitId, pkg) <- Map.toList dalfDependencies0 ] - let dataFiles = [mkConfFile pkgConf pkgModuleNames (T.unpack pkgId)] + confFile <- liftIO $ mkConfFile pkgConf pkgModuleNames (T.unpack pkgId) + let dataFiles = [confFile] srcRoot <- getSrcRoot pSrc pure $ createArchive @@ -262,15 +268,43 @@ pkgNameVersion n mbV = Nothing -> n Just v -> n ++ "-" ++ v -mkConfFile :: - PackageConfigFields -> [String] -> String -> (String, BS.ByteString) -mkConfFile PackageConfigFields {..} pkgModuleNames pkgId = (confName, bs) +-- Expand SDK package dependencies using the SDK root path. +-- E.g. `daml-trigger` --> `$DAML_SDK/daml-libs/daml-trigger.dar` +-- When invoked outside of the SDK, we will only error out +-- if there is actually an SDK package so that +-- When there is no SDK +expandSdkPackages :: [FilePath] -> IO [FilePath] +expandSdkPackages dars = do + mbSdkPath <- handleIO (\_ -> pure Nothing) $ Just <$> getSdkPath + mapM (expand mbSdkPath) dars where + isSdkPackage fp = takeExtension fp `notElem` [".dar", ".dalf"] + expand mbSdkPath fp + | fp `elem` basePackages = pure fp + | isSdkPackage fp = case mbSdkPath of + Just sdkPath -> pure $ sdkPath "daml-libs" fp <.> "dar" + Nothing -> fail $ "Cannot resolve SDK dependency '" ++ fp ++ "'. Use daml assistant." + | otherwise = pure fp + +mkConfFile :: + PackageConfigFields -> [String] -> String -> IO (String, BS.ByteString) +mkConfFile PackageConfigFields {..} pkgModuleNames pkgId = do + deps <- mapM darUnitId =<< expandSdkPackages pDependencies + pure (confName, confContent deps) + where + darUnitId "daml-stdlib" = pure damlStdlib + darUnitId "daml-prim" = pure "daml-prim" + darUnitId f + -- This case is used by data-dependencies. DALF names are not affected by + -- -o so this should be fine. + | takeExtension f == ".dalf" = pure $ dropExtension $ takeFileName f + darUnitId darPath = do + archive <- ZipArchive.toArchive . BSL.fromStrict <$> BS.readFile darPath + manifest <- either (\err -> fail $ "Failed to read manifest of " <> darPath <> ": " <> err) pure $ readDalfManifest archive + maybe (fail $ "Missing 'Name' attribute in manifest of " <> darPath) pure (packageName manifest) confName = pkgNameVersion pName pVersion ++ ".conf" key = fullPkgName pName pVersion pkgId - sanitizeBaseDeps "daml-stdlib" = damlStdlib - sanitizeBaseDeps dep = dep - bs = + confContent deps = BSC.toStrict $ BSC.pack $ unlines $ @@ -286,11 +320,7 @@ mkConfFile PackageConfigFields {..} pkgModuleNames pkgId = (confName, bs) , "import-dirs: ${pkgroot}" ++ "/" ++ key -- we really want '/' here , "library-dirs: ${pkgroot}" ++ "/" ++ key , "data-dir: ${pkgroot}" ++ "/" ++ key - , "depends: " ++ - unwords - [ sanitizeBaseDeps $ dropExtension $ takeFileName dep - | dep <- pDependencies - ] + , "depends: " ++ unwords deps ] -- | Helper to bundle up all files into a DAR. diff --git a/compiler/damlc/lib/DA/Cli/Damlc/Packaging.hs b/compiler/damlc/lib/DA/Cli/Damlc/Packaging.hs index 67f7d1785ea..f1de5934461 100644 --- a/compiler/damlc/lib/DA/Cli/Damlc/Packaging.hs +++ b/compiler/damlc/lib/DA/Cli/Damlc/Packaging.hs @@ -14,7 +14,6 @@ module DA.Cli.Damlc.Packaging import qualified "zip" Codec.Archive.Zip as Zip import qualified "zip-archive" Codec.Archive.Zip as ZipArchive -import Control.Exception.Safe (handleIO) import Control.Lens (toListOf) import Control.Monad import Data.Bifunctor @@ -47,7 +46,6 @@ import DA.Daml.LF.Ast.Optics (packageRefs) import qualified DA.Daml.LF.Proto3.Archive as Archive import DA.Daml.LF.Reader import DA.Daml.Options.Types -import DA.Daml.Project.Consts import qualified DA.Pretty import SdkVersion @@ -165,23 +163,6 @@ createProjectPackageDb opts thisSdkVer deps dataDeps = do forM_ depsExtracted $ \ExtractedDar{..} -> installDar dbPath edConfFiles edDalfs edSrcs --- Expand SDK package dependencies using the SDK root path. --- E.g. `daml-trigger` --> `$DAML_SDK/daml-libs/daml-trigger.dar` --- When invoked outside of the SDK, we will only error out --- if there is actually an SDK package so that --- When there is no SDK -expandSdkPackages :: [FilePath] -> IO [FilePath] -expandSdkPackages dars = do - mbSdkPath <- handleIO (\_ -> pure Nothing) $ Just <$> getSdkPath - mapM (expand mbSdkPath) dars - where - isSdkPackage fp = takeExtension fp `notElem` [".dar", ".dalf"] - expand mbSdkPath fp - | isSdkPackage fp = case mbSdkPath of - Just sdkPath -> pure $ sdkPath "daml-libs" fp <.> "dar" - Nothing -> fail $ "Cannot resolve SDK dependency '" ++ fp ++ "'. Use daml assistant." - | otherwise = pure fp - -- generate interface files and install them in the package database generateAndInstallIfaceFiles :: LF.Package @@ -225,7 +206,7 @@ generateAndInstallIfaceFiles dalf src opts workDir dbPath projectPackageDatabase (toNormalizedFilePath "./") [fp | (fp, _content) <- src'] -- write the conf file and refresh the package cache - let (cfPath, cfBs) = + (cfPath, cfBs) <- mkConfFile PackageConfigFields { pName = pkgName diff --git a/compiler/damlc/tests/src/DA/Test/Packaging.hs b/compiler/damlc/tests/src/DA/Test/Packaging.hs index 4278029161f..6cf2a9a36a2 100644 --- a/compiler/damlc/tests/src/DA/Test/Packaging.hs +++ b/compiler/damlc/tests/src/DA/Test/Packaging.hs @@ -371,6 +371,64 @@ tests damlc = testGroup "Packaging" -- Verify that the name in the manifest is independent of the DAR name. packageName manifest @?= Just "foobar-0.0.1" + , testCase "Transitive package deps" $ withTempDir $ \projDir -> do + -- Check that the depends field in the package config files does not depend on the name of the DAR. + let projA = projDir "a" + let projB = projDir "b" + let projC = projDir "c" + + createDirectoryIfMissing True (projA "src") + writeFileUTF8 (projA "daml.yaml") $ unlines + [ "sdk-version: " <> sdkVersion + , "name: a" + , "version: 0.0.1" + , "source: src" + , "dependencies: [daml-prim, daml-stdlib]" + ] + writeFileUTF8 (projA "src" "A.daml") $ unlines + [ "daml 1.2" + , "module A where" + ] + withCurrentDirectory projA $ callProcessSilent damlc ["build", "-o", "foo.dar"] + + createDirectoryIfMissing True (projB "src") + writeFileUTF8 (projB "daml.yaml") $ unlines + [ "sdk-version: " <> sdkVersion + , "name: b" + , "version: 0.0.1" + , "source: src" + , "dependencies:" + , " - daml-prim" + , " - daml-stdlib" + , " - " <> projA "foo.dar" + ] + writeFileUTF8 (projB "src" "B.daml") $ unlines + [ "daml 1.2" + , "module B where" + , "import A" + ] + withCurrentDirectory projB $ callProcessSilent damlc ["build", "-o", "bar.dar"] + + createDirectoryIfMissing True (projC "src") + writeFileUTF8 (projC "daml.yaml") $ unlines + [ "sdk-version: " <> sdkVersion + , "name: c" + , "version: 0.0.1" + , "source: src" + , "dependencies:" + , " - daml-prim" + , " - daml-stdlib" + , " - " <> projA "foo.dar" + , " - " <> projB "bar.dar" + ] + writeFileUTF8 (projC "src" "C.daml") $ unlines + [ "daml 1.2" + , "module C where" + , "import A" + , "import B" + ] + withCurrentDirectory projC $ callProcessSilent damlc ["build", "-o", "baz.dar"] + , dataDependencyTests damlc ]