Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi-IDE Features for 2.9 #19040

Merged
merged 17 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion sdk/compiler/daml-extension/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ export function createLanguageClient(
// The user has not made an explicit choice.
args.push("--telemetry-ignored");
}
if (multiIDEVerbose === true) {
if (multiIDEVerbose === true && multiIDESupport === true) {
args.push("--verbose=yes");
}
const extraArgsString = config.get("extraArguments", "").trim();
Expand Down
353 changes: 248 additions & 105 deletions sdk/compiler/damlc/lib/DA/Cli/Damlc/Command/MultiIde.hs

Large diffs are not rendered by default.

14 changes: 10 additions & 4 deletions sdk/compiler/damlc/lib/DA/Cli/Damlc/Command/MultiIde/Parsing.hs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ module DA.Cli.Damlc.Command.MultiIde.Parsing (
putReqMethodAll,
putReqMethodSingleFromClient,
putReqMethodSingleFromServer,
putReqMethodSingleFromServerCoordinator,
putServerReq,
) where

Expand Down Expand Up @@ -68,7 +69,12 @@ putChunk handle payload = do
putReqMethodSingleFromServer
:: forall (m :: LSP.Method 'LSP.FromServer 'LSP.Request)
. MethodTrackerVar 'LSP.FromServer -> FilePath -> LSP.LspId m -> LSP.SMethod m -> IO ()
putReqMethodSingleFromServer tracker home id method = putReqMethod tracker id $ TrackedSingleMethodFromServer method home
putReqMethodSingleFromServer tracker home id method = putReqMethod tracker id $ TrackedSingleMethodFromServer method $ Just home

putReqMethodSingleFromServerCoordinator
:: forall (m :: LSP.Method 'LSP.FromServer 'LSP.Request)
. MethodTrackerVar 'LSP.FromServer -> LSP.LspId m -> LSP.SMethod m -> IO ()
putReqMethodSingleFromServerCoordinator tracker id method = putReqMethod tracker id $ TrackedSingleMethodFromServer method Nothing

putReqMethodSingleFromClient
:: forall (m :: LSP.Method 'LSP.FromClient 'LSP.Request)
Expand Down Expand Up @@ -149,12 +155,12 @@ parseServerMessageWithTracker tracker selfIde val = pickReqMethodTo tracker $ \e
parseClientMessageWithTracker
:: MethodTrackerVar 'LSP.FromServer
-> Aeson.Value
-> IO (Either String (LSP.FromClientMessage' (Product LSP.SMethod (Const FilePath))))
-> IO (Either String (LSP.FromClientMessage' (Product LSP.SMethod (Const (Maybe FilePath)))))
samuel-williams-da marked this conversation as resolved.
Show resolved Hide resolved
parseClientMessageWithTracker tracker val = pickReqMethodTo tracker $ \extract ->
case Aeson.parseEither (LSP.parseClientMessage (wrapParseMessageLookup . extract)) val of
Right (LSP.FromClientMess meth mess) -> (Right (LSP.FromClientMess meth mess), Nothing)
Right (LSP.FromClientRsp (Pair (TrackedSingleMethodFromServer method home) (Const newIxMap)) rsp) ->
(Right (LSP.FromClientRsp (Pair method (Const home)) rsp), Just newIxMap)
Right (LSP.FromClientRsp (Pair (TrackedSingleMethodFromServer method mHome) (Const newIxMap)) rsp) ->
(Right (LSP.FromClientRsp (Pair method (Const mHome)) rsp), Just newIxMap)
Left msg -> (Left msg, Nothing)

-- Takes a message from server and stores it if its a request, so that later messages from the client can deduce response context
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ stripLspPrefix (LSP.IdString (T.uncons -> Just ('t', rest))) = LSP.IdString $ T.
stripLspPrefix t = t

-- Prefixes applied to builtin and custom requests. Notifications do not have ids, responses do not need this logic.
addLspPrefixToServerMessage :: SubIDE -> LSP.FromServerMessage -> LSP.FromServerMessage
addLspPrefixToServerMessage :: SubIDEInstance -> LSP.FromServerMessage -> LSP.FromServerMessage
addLspPrefixToServerMessage _ res@(LSP.FromServerRsp _ _) = res
addLspPrefixToServerMessage ide res@(LSP.FromServerMess method params) =
case LSP.splitServerMethod method of
Expand Down
58 changes: 47 additions & 11 deletions sdk/compiler/damlc/lib/DA/Cli/Damlc/Command/MultiIde/Types.hs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@ import Control.Concurrent.STM.TMVar
import Control.Concurrent.MVar
import Control.Monad.STM
import qualified Data.ByteString.Lazy as BSL
import Data.Function (on)
import qualified Data.IxMap as IM
import qualified Data.Map as Map
import Data.Maybe (fromMaybe)
import qualified Data.Set as Set
import qualified Data.Text as T
import qualified Language.LSP.Types as LSP
import System.IO.Extra
Expand All @@ -35,7 +38,7 @@ data TrackedMethod (m :: LSP.Method from 'LSP.Request) where
TrackedSingleMethodFromServer
:: forall (m :: LSP.Method 'LSP.FromServer 'LSP.Request)
. LSP.SMethod m
-> FilePath -- Also store the IDE that sent the request
-> Maybe FilePath -- Also store the IDE that sent the request (or don't, for requests sent by the coordinator)
-> TrackedMethod m
TrackedAllMethod :: forall (m :: LSP.Method 'LSP.FromClient 'LSP.Request).
{ tamMethod :: LSP.SMethod m
Expand All @@ -59,7 +62,7 @@ tmMethod (TrackedAllMethod {tamMethod}) = tamMethod
type MethodTracker (from :: LSP.From) = IM.IxMap @(LSP.Method from 'LSP.Request) LSP.LspId TrackedMethod
type MethodTrackerVar (from :: LSP.From) = TVar (MethodTracker from)

data SubIDE = SubIDE
data SubIDEInstance = SubIDEInstance
{ ideInhandleAsync :: Async ()
, ideInHandle :: Handle
, ideInHandleChannel :: TChan BSL.ByteString
Expand All @@ -70,32 +73,60 @@ data SubIDE = SubIDE
, ideMessageIdPrefix :: T.Text
-- ^ Some unique string used to prefix message ids created by the SubIDE, to avoid collisions with other SubIDEs
-- We use the stringified process ID
, ideActive :: Bool
, ideUnitId :: String
-- ^ Unit ID of the package this SubIDE handles
-- Of the form "daml-script-0.0.1"
}

instance Eq SubIDEInstance where
-- ideMessageIdPrefix is derived from process id, so this equality is of the process.
(==) = (==) `on` ideMessageIdPrefix

instance Ord SubIDEInstance where
-- ideMessageIdPrefix is derived from process id, so this ordering is of the process.
compare = compare `on` ideMessageIdPrefix

-- We store an optional main ide, the currently closing ides (kept only so they can reply to their shutdowns), and open files
-- open files must outlive the main subide so we can re-send the TextDocumentDidOpen messages on new ide startup
data SubIDEData = SubIDEData
{ ideDataMain :: Maybe SubIDEInstance
, ideDataClosing :: Set.Set SubIDEInstance
, ideDataOpenFiles :: Set.Set FilePath
}

defaultSubIDEData :: SubIDEData
defaultSubIDEData = SubIDEData Nothing Set.empty Set.empty

lookupSubIde :: FilePath -> SubIDEs -> SubIDEData
lookupSubIde home ides = fromMaybe defaultSubIDEData $ Map.lookup home ides

-- SubIDEs placed in a TMVar. The emptyness representents a modification lock.
-- The lock unsures the following properties:
-- If multiple messages are sent to a new IDE at the same time, the first will create and hold a lock, while the rest wait on that lock (avoid multiple create)
-- We never attempt to send messages on a stale IDE. If we ever read SubIDEsVar with the intent to send a message on a SubIDE, we must hold the so a shutdown
-- cannot be sent on that IDE until we are done. This ensures that when a shutdown does occur, it is impossible for non-shutdown messages to be added to the
-- queue after the shutdown.
type SubIDEs = Map.Map FilePath SubIDE
type SubIDEs = Map.Map FilePath SubIDEData
samuel-williams-da marked this conversation as resolved.
Show resolved Hide resolved
type SubIDEsVar = TMVar SubIDEs

onlyActiveSubIdes :: SubIDEs -> SubIDEs
onlyActiveSubIdes = Map.filter ideActive

-- Stores the initialize messages sent by the client to be forwarded to SubIDEs when they are created.
type InitParams = LSP.InitializeParams
type InitParamsVar = MVar InitParams

-- Maps a packages unit id to its source location, using PackageOnDisk for all packages in multi-package.yaml
-- and PackageInDar for all known dars (currently extracted from data-dependencies)
data PackageSourceLocation = PackageOnDisk FilePath | PackageInDar FilePath
data PackageSourceLocation = PackageOnDisk FilePath | PackageInDar FilePath deriving Show
type MultiPackageYamlMapping = Map.Map String PackageSourceLocation
type MultiPackageYamlMappingVar = TMVar MultiPackageYamlMapping
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need these var synonyms? A quick search doesn't turn anything up for them being used outside the MultiIdeState.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like having them as it gives me an opportunity to name and comment them separately. Perhaps they could be newtypes to provide more value, but I think dropping them and moving all this into the MultiIdeState would make it quite painful to read

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe InitParams could go, but its adhering the content + tvar pattern I have going


-- Maps a dar path to the list of packages that directly depend on it
type DarDependentPackages = Map.Map FilePath [FilePath]
type DarDependentPackagesVar = TMVar DarDependentPackages

-- "Cache" for the home path of files
-- Cleared on daml.yaml modification and file deletion
type SourceFileHomes = Map.Map FilePath FilePath
type SourceFileHomesVar = TMVar SourceFileHomes

data MultiIdeState = MultiIdeState
{ fromClientMethodTrackerVar :: MethodTrackerVar 'LSP.FromClient
Expand All @@ -105,19 +136,24 @@ data MultiIdeState = MultiIdeState
, subIDEsVar :: SubIDEsVar
, initParamsVar :: InitParamsVar
, toClientChan :: TChan BSL.ByteString
, multiPackageMapping :: MultiPackageYamlMapping
, multiPackageMappingVar :: MultiPackageYamlMappingVar
, darDependentPackagesVar :: DarDependentPackagesVar
, debugPrint :: String -> IO ()
, multiPackageHome :: FilePath
, defaultPackagePath :: FilePath
, sourceFileHomesVar :: SourceFileHomesVar
}

newMultiIdeState :: MultiPackageYamlMapping -> FilePath -> FilePath -> (String -> IO ()) -> IO MultiIdeState
newMultiIdeState multiPackageMapping multiPackageHome defaultPackagePath debugPrint = do
newMultiIdeState :: FilePath -> FilePath -> (String -> IO ()) -> IO MultiIdeState
newMultiIdeState multiPackageHome defaultPackagePath debugPrint = do
(fromClientMethodTrackerVar :: MethodTrackerVar 'LSP.FromClient) <- newTVarIO IM.emptyIxMap
(fromServerMethodTrackerVar :: MethodTrackerVar 'LSP.FromServer) <- newTVarIO IM.emptyIxMap
subIDEsVar <- newTMVarIO @SubIDEs mempty
initParamsVar <- newEmptyMVar @InitParams
toClientChan <- atomically newTChan
multiPackageMappingVar <- newTMVarIO @MultiPackageYamlMapping mempty
darDependentPackagesVar <- newTMVarIO @DarDependentPackages mempty
sourceFileHomesVar <- newTMVarIO @SourceFileHomes mempty
pure MultiIdeState {..}

-- Forwarding
Expand Down
19 changes: 16 additions & 3 deletions sdk/compiler/damlc/lib/DA/Cli/Damlc/Command/MultiIde/Util.hs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import Data.Maybe (fromMaybe)
import qualified Language.LSP.Types as LSP
import qualified Language.LSP.Types.Capabilities as LSP
import System.Directory (doesDirectoryExist, listDirectory, withCurrentDirectory, canonicalizePath)
import System.FilePath (takeDirectory)
import System.FilePath (takeDirectory, takeExtension)
import System.IO.Extra
import System.IO.Unsafe (unsafePerformIO)

Expand Down Expand Up @@ -153,6 +153,17 @@ initializeResult = LSP.InitializeResult
true = Just (LSP.InL True)
false = Just (LSP.InL False)

registerFileWatchersMessage :: LSP.RequestMessage 'LSP.ClientRegisterCapability
registerFileWatchersMessage =
LSP.RequestMessage "2.0" (LSP.IdString "MultiIdeWatchedFiles") LSP.SClientRegisterCapability $ LSP.RegistrationParams $ LSP.List
[ LSP.SomeRegistration $ LSP.Registration "MultiIdeWatchedFiles" LSP.SWorkspaceDidChangeWatchedFiles $ LSP.DidChangeWatchedFilesRegistrationOptions $ LSP.List
[ LSP.FileSystemWatcher "**/multi-package.yaml" Nothing
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be worried about how this will interact with, say, a deeply nested complex folder that needs to be searched? E.g. a node_modules folder.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its a fair point, though this logic is handled by VSCode, which one would hope is implemented efficiently.
I'm not sure what could even be done here through, bar writing patterns with explicit exclusions of some directories - if glob support thats - and even then I don't think that should be the job of the language server to handle

, LSP.FileSystemWatcher "**/daml.yaml" Nothing
, LSP.FileSystemWatcher "**/*.dar" Nothing
, LSP.FileSystemWatcher "**/*.daml" Nothing
]
]

castLspId :: LSP.LspId m -> LSP.LspId m'
castLspId (LSP.IdString s) = LSP.IdString s
castLspId (LSP.IdInt i) = LSP.IdInt i
Expand All @@ -178,8 +189,10 @@ unitIdAndDepsFromDamlYaml :: FilePath -> IO (Either ConfigError (String, [FilePa
unitIdAndDepsFromDamlYaml path = do
handle (\(e :: ConfigError) -> return $ Left e) $ runExceptT $ do
project <- lift $ readProjectConfig $ ProjectPath path
deps <- except $ fromMaybe [] <$> queryProjectConfig ["data-dependencies"] project
canonDeps <- lift $ withCurrentDirectory path $ traverse canonicalizePath deps
dataDeps <- except $ fromMaybe [] <$> queryProjectConfig ["data-dependencies"] project
directDeps <- except $ fromMaybe [] <$> queryProjectConfig ["dependencies"] project
let directDarDeps = filter (\dep -> takeExtension dep == ".dar") directDeps
canonDeps <- lift $ withCurrentDirectory path $ traverse canonicalizePath $ dataDeps <> directDarDeps
name <- except $ queryProjectConfigRequired ["name"] project
version <- except $ queryProjectConfigRequired ["version"] project
pure (name <> "-" <> version, canonDeps)