From ed44872a770715a6cae3aa0614d90cd562d2bfed Mon Sep 17 00:00:00 2001 From: Chet Husk Date: Wed, 17 Apr 2024 12:19:49 -0500 Subject: [PATCH 01/12] Allow loading of projects that are missing imports. --- src/Ionide.ProjInfo/Library.fs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Ionide.ProjInfo/Library.fs b/src/Ionide.ProjInfo/Library.fs index 96806644..ccbac47f 100644 --- a/src/Ionide.ProjInfo/Library.fs +++ b/src/Ionide.ProjInfo/Library.fs @@ -373,7 +373,6 @@ module ProjectLoader = and set (v: LoggerVerbosity): unit = () } - let internal stringWriterLogger (writer: StringWriter) = { new ILogger with member this.Initialize(eventSource: IEventSource) : unit = @@ -530,14 +529,17 @@ module ProjectLoader = let globalProperties = getGlobalProps path tfm globalProperties use pc = new ProjectCollection(globalProperties) - - let pi = pc.LoadProject(path, globalProperties, toolsVersion = null) + let loadSettings = + ProjectLoadSettings.RecordEvaluatedItemElements + ||| ProjectLoadSettings.ProfileEvaluation + ||| ProjectLoadSettings.IgnoreMissingImports + let project = Project(projectFile = path, globalProperties = globalProperties, toolsVersion = null, projectCollection = pc, loadSettings = loadSettings) use sw = new StringWriter() let loggers = createLoggers [ path ] binaryLogs sw - let pi = pi.CreateProjectInstance() + let pi = project.CreateProjectInstance() let build = pi.Build(designTimeBuildTargets isLegacyFrameworkProjFile, loggers) @@ -1030,6 +1032,7 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri buildParameters.ProjectLoadSettings <- ProjectLoadSettings.RecordEvaluatedItemElements ||| ProjectLoadSettings.ProfileEvaluation + ||| ProjectLoadSettings.IgnoreMissingImports buildParameters.LogInitialPropertiesAndItems <- true bm.BeginBuild(buildParameters) From 7e07e2c4b0065da315e3aa6de48f7415c59ef1e6 Mon Sep 17 00:00:00 2001 From: Chet Husk Date: Sat, 20 Apr 2024 17:21:54 -0500 Subject: [PATCH 02/12] Add basic test for missing explicit import support --- test/Ionide.ProjInfo.Tests/TestAssets.fs | 14 +++++-- test/Ionide.ProjInfo.Tests/Tests.fs | 38 +++++++++++++++---- test/examples/missing-import/Program.fs | 3 ++ .../missing-import/missing-import.fsproj | 14 +++++++ 4 files changed, 59 insertions(+), 10 deletions(-) create mode 100644 test/examples/missing-import/Program.fs create mode 100644 test/examples/missing-import/missing-import.fsproj diff --git a/test/Ionide.ProjInfo.Tests/TestAssets.fs b/test/Ionide.ProjInfo.Tests/TestAssets.fs index c80e531e..3684ec63 100644 --- a/test/Ionide.ProjInfo.Tests/TestAssets.fs +++ b/test/Ionide.ProjInfo.Tests/TestAssets.fs @@ -300,7 +300,15 @@ let ``NetSDK library referencing ProduceReferenceAssembly library`` = { "l2" / "l2.fsproj" TargetFrameworks = Map.ofList [ "netstandard2.0", sourceFiles [ "Library.fs" ] ] - ProjectReferences = [ - ``NetSDK library with ProduceReferenceAssembly`` - ] + ProjectReferences = [ ``NetSDK library with ProduceReferenceAssembly`` ] +} + +let ``Console app with missing direct Import`` = { + ProjDir = "missing-import" + AssemblyName = "missing-import" + ProjectFile = + "missing-import" + / "missing-import.fsproj" + TargetFrameworks = Map.ofList [ "net6.0", sourceFiles [ "Program.fs" ] ] + ProjectReferences = [] } diff --git a/test/Ionide.ProjInfo.Tests/Tests.fs b/test/Ionide.ProjInfo.Tests/Tests.fs index 0a1f1237..53a4c6fc 100644 --- a/test/Ionide.ProjInfo.Tests/Tests.fs +++ b/test/Ionide.ProjInfo.Tests/Tests.fs @@ -38,8 +38,7 @@ let pathForProject (test: TestAssetProjInfo) = pathForTestAssets test / test.ProjectFile -let implAssemblyForProject (test: TestAssetProjInfo) = - $"{test.AssemblyName}.dll" +let implAssemblyForProject (test: TestAssetProjInfo) = $"{test.AssemblyName}.dll" let refAssemblyForProject (test: TestAssetProjInfo) = Path.Combine("ref", implAssemblyForProject test) @@ -2157,14 +2156,17 @@ let referenceAssemblySupportTest toolsPath prefix (workspaceFactory: ToolsPath - |> Seq.toList Expect.hasLength parsed 2 "Should have loaded the F# lib and the referenced F# lib" - let fsharpProject = parsed |> Seq.find (fun p -> Path.GetFileName(p.ProjectFileName) = Path.GetFileName(childProj.ProjectFile)) + + let fsharpProject = + parsed + |> Seq.find (fun p -> Path.GetFileName(p.ProjectFileName) = Path.GetFileName(childProj.ProjectFile)) + let mapped = FCS.mapToFSharpProjectOptions fsharpProject parsed let referencedProjects = mapped.ReferencedProjects Expect.hasLength referencedProjects 1 "Should have a reference to the F# ProjectReference lib" match referencedProjects[0] with - | FSharpReferencedProject.FSharpReference(targetPath, _) -> - Expect.stringContains targetPath (refAssemblyForProject parentProj) "Should have found the ref assembly for the F# lib" + | FSharpReferencedProject.FSharpReference(targetPath, _) -> Expect.stringContains targetPath (refAssemblyForProject parentProj) "Should have found the ref assembly for the F# lib" | _ -> failwith "Should have found a F# reference" ) @@ -2180,6 +2182,25 @@ let testProjectLoadBadData = Expect.isNone proj.Response "should have loaded, detected bad data, and defaulted to empty" ) +let canLoadMissingImports toolsPath loaderType (workspaceFactory: ToolsPath -> IWorkspaceLoader) = + testCase + $"Can load projects with missing Imports - {loaderType}" + (fun () -> + let proj = ``Console app with missing direct Import`` + let projPath = pathForProject proj + + let loader = workspaceFactory toolsPath + + let parsed = + loader.LoadProjects [ projPath ] + |> Seq.toList + + Expect.equal parsed.Length 1 "Should have loaded the project" + let parsed = parsed[0] + Expect.equal 1 parsed.SourceFiles.Length "Should have cracked a single file" + Expect.equal "Program.fs" parsed.SourceFiles.[0] "Filename should be Program.fs" + ) + let tests toolsPath = let testSample3WorkspaceLoaderExpected = [ ExpectNotification.loading "c1.fsproj" @@ -2297,7 +2318,10 @@ let tests toolsPath = expensiveTests toolsPath WorkspaceLoader.Create csharpLibTest toolsPath WorkspaceLoader.Create - referenceAssemblySupportTest toolsPath (nameof(WorkspaceLoader)) WorkspaceLoader.Create - referenceAssemblySupportTest toolsPath (nameof(WorkspaceLoaderViaProjectGraph)) WorkspaceLoaderViaProjectGraph.Create + referenceAssemblySupportTest toolsPath (nameof (WorkspaceLoader)) WorkspaceLoader.Create + referenceAssemblySupportTest toolsPath (nameof (WorkspaceLoaderViaProjectGraph)) WorkspaceLoaderViaProjectGraph.Create + // tests that cover our ability to handle missing imports + canLoadMissingImports toolsPath (nameof (WorkspaceLoader)) WorkspaceLoader.Create + canLoadMissingImports toolsPath (nameof (WorkspaceLoaderViaProjectGraph)) WorkspaceLoaderViaProjectGraph.Create ] diff --git a/test/examples/missing-import/Program.fs b/test/examples/missing-import/Program.fs new file mode 100644 index 00000000..561c2d4a --- /dev/null +++ b/test/examples/missing-import/Program.fs @@ -0,0 +1,3 @@ +[] +let main args = + 1 diff --git a/test/examples/missing-import/missing-import.fsproj b/test/examples/missing-import/missing-import.fsproj new file mode 100644 index 00000000..90a1e9b2 --- /dev/null +++ b/test/examples/missing-import/missing-import.fsproj @@ -0,0 +1,14 @@ + + + + net6.0 + missing_import + + + + + + + + + From 18fc3191ccf44abcd56e755544dd922dc9bba2e1 Mon Sep 17 00:00:00 2001 From: Chet Husk Date: Sat, 20 Apr 2024 19:07:53 -0500 Subject: [PATCH 03/12] Get loading missing Imports working for both loaders --- .vscode/launch.json | 1 - src/Ionide.ProjInfo/Library.fs | 241 ++++++++++++++++------- test/Ionide.ProjInfo.Tests/TestAssets.fs | 4 +- test/Ionide.ProjInfo.Tests/Tests.fs | 36 +++- 4 files changed, 203 insertions(+), 79 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index a8f09603..099a5617 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -39,7 +39,6 @@ "name": "Ionide.ProjInfo.Tests", "type": "coreclr", "request": "launch", - "preLaunchTask": "build", "program": "${workspaceFolder}/test/Ionide.ProjInfo.Tests/bin/Debug/${input:tfm}/Ionide.ProjInfo.Tests.dll", "args": [ "--filter", diff --git a/src/Ionide.ProjInfo/Library.fs b/src/Ionide.ProjInfo/Library.fs index ccbac47f..b64f4684 100644 --- a/src/Ionide.ProjInfo/Library.fs +++ b/src/Ionide.ProjInfo/Library.fs @@ -331,12 +331,6 @@ module ProjectLoader = type LoadedProject = internal LoadedProject of ProjectInstance - [] - type ProjectLoadingStatus = - private - | Success of LoadedProject - | Error of string - let internal msBuildLogger = lazy (LogProvider.getLoggerByName "MsBuild") //lazy because dotnet test wont pickup our logger otherwise let msBuildToLogProvider () = @@ -391,9 +385,7 @@ module ProjectLoader = with set (v: LoggerVerbosity): unit = () } - let getTfm (path: string) readingProps isLegacyFrameworkProj = - let pi = ProjectInstance(path, globalProperties = readingProps, toolsVersion = null) - + let getTfm (pi: ProjectInstance) isLegacyFrameworkProj = let tfm = pi.GetPropertyValue( if isLegacyFrameworkProj then @@ -414,6 +406,21 @@ module ProjectLoader = else Some tfm + let loadProjectAndGetTFM (path: string) projectCollection readingProps isLegacyFrameworkProj = + let project = + Project( + projectFile = path, + globalProperties = readingProps, + toolsVersion = null, + projectCollection = projectCollection, + loadSettings = + (ProjectLoadSettings.IgnoreMissingImports + ||| ProjectLoadSettings.IgnoreInvalidImports) + ) + + let pi = project.CreateProjectInstance() + getTfm pi isLegacyFrameworkProj + let createLoggers (paths: string seq) (binaryLogs: BinaryLogGeneration) (sw: StringWriter) = let swLogger = stringWriterLogger (sw) let msBuildLogger = msBuildToLogProvider () @@ -442,8 +449,8 @@ module ProjectLoader = yield! loggers ] - let getGlobalProps (path: string) (tfm: string option) (globalProperties: (string * string) list) = - dict [ + let getGlobalProps (tfm: string option) (globalProperties: (string * string) list) (propsSetFromParentCollection: Set) = + [ "ProvideCommandLineArgs", "true" "DesignTimeBuild", "true" "SkipCompilerExecution", "true" @@ -458,6 +465,9 @@ module ProjectLoader = "DotnetProjInfo", "true" yield! globalProperties ] + |> List.filter (fun (ourProp, _) -> not (propsSetFromParentCollection.Contains ourProp)) + |> dict + /// /// These are a list of build targets that are run during a design-time build (mostly). @@ -504,7 +514,7 @@ module ProjectLoader = Init.setupForLegacyFramework msbuildBinaryDir | _ -> () - let loadProject (path: string) (binaryLogs: BinaryLogGeneration) globalProperties = + let loadProject (path: string) (binaryLogs: BinaryLogGeneration) (projectCollection: ProjectCollection) = try let isLegacyFrameworkProjFile = if @@ -519,21 +529,26 @@ module ProjectLoader = else false - let readingProps = getGlobalProps path None globalProperties + let collectionProps = + projectCollection.GlobalProperties.Keys + |> Set.ofSeq + + let readingProps = getGlobalProps None [] collectionProps if isLegacyFrameworkProjFile then setLegacyMsbuildProperties isLegacyFrameworkProjFile - let tfm = getTfm path readingProps isLegacyFrameworkProjFile + let tfm = loadProjectAndGetTFM path projectCollection readingProps isLegacyFrameworkProjFile - let globalProperties = getGlobalProps path tfm globalProperties + let globalProperties = getGlobalProps tfm [] collectionProps - use pc = new ProjectCollection(globalProperties) let loadSettings = ProjectLoadSettings.RecordEvaluatedItemElements - ||| ProjectLoadSettings.ProfileEvaluation - ||| ProjectLoadSettings.IgnoreMissingImports - let project = Project(projectFile = path, globalProperties = globalProperties, toolsVersion = null, projectCollection = pc, loadSettings = loadSettings) + ||| ProjectLoadSettings.ProfileEvaluation + ||| ProjectLoadSettings.IgnoreMissingImports + + let project = + Project(projectFile = path, globalProperties = globalProperties, toolsVersion = null, projectCollection = projectCollection, loadSettings = loadSettings) use sw = new StringWriter() @@ -544,11 +559,11 @@ module ProjectLoader = let build = pi.Build(designTimeBuildTargets isLegacyFrameworkProjFile, loggers) if build then - ProjectLoadingStatus.Success(LoadedProject pi) + Ok(LoadedProject pi) else - ProjectLoadingStatus.Error(sw.ToString()) + Error(sw.ToString()) with exc -> - ProjectLoadingStatus.Error(exc.Message) + Error(exc.Message) let getFscArgs (LoadedProject project) = project.Items @@ -768,11 +783,21 @@ module ProjectLoader = LoadTime = DateTime.Now TargetPath = props - |> Seq.tryPick (fun n -> if n.Name = "TargetPath" then Some n.Value else None) + |> Seq.tryPick (fun n -> + if n.Name = "TargetPath" then + Some n.Value + else + None + ) |> Option.defaultValue "" TargetRefPath = props - |> Seq.tryPick (fun n -> if n.Name = "TargetRefPath" then Some n.Value else None) + |> Seq.tryPick (fun n -> + if n.Name = "TargetRefPath" then + Some n.Value + else + None + ) ProjectOutputType = outputType ProjectSdkInfo = sdkInfo Items = compileItems @@ -835,20 +860,20 @@ module ProjectLoader = Result.Ok proj - /// - /// Main entry point for project loading. - /// - /// Full path to the `.fsproj` file - /// describes if and how to generate MsBuild binary logs - /// The global properties to use (e.g. Configuration=Release). Some additional global properties are pre-set by the tool - /// List of additional MsBuild properties that you want to obtain. - /// Returns the record instance representing the loaded project or string containing error message - let getProjectInfo (path: string) (globalProperties: (string * string) list) (binaryLogs: BinaryLogGeneration) (customProperties: string list) : Result = - let loadedProject = loadProject path binaryLogs globalProperties - - match loadedProject with - | ProjectLoadingStatus.Success project -> getLoadedProjectInfo path customProperties project - | ProjectLoadingStatus.Error e -> Result.Error e +// /// +// /// Main entry point for project loading. +// /// +// /// Full path to the `.fsproj` file +// /// describes if and how to generate MsBuild binary logs +// /// The propertyCollection to load the project into. This will provide global properties by default. +// /// List of additional MsBuild properties that you want to obtain. +// /// Returns the record instance representing the loaded project or string containing error message +// let getProjectInfo (path: string) projectCollection (binaryLogs: BinaryLogGeneration) (customProperties: string list) : Result = +// let loadedProject = loadProject path binaryLogs projectCollection + +// match loadedProject with +// | ProjectLoadingStatus.Success project -> getLoadedProjectInfo path customProperties project +// | ProjectLoadingStatus.Error e -> Result.Error e /// A type that turns project files or solution files into deconstructed options. /// Use this in conjunction with the other ProjInfo libraries to turn these options into @@ -896,10 +921,20 @@ module WorkspaceLoaderViaProjectGraph = type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (string * string) list) = let (ToolsPath toolsPath) = toolsPath - let globalProperties = defaultArg globalProperties [] let logger = LogProvider.getLoggerFor () let loadingNotification = new Event() + let projectCollection = + new ProjectCollection( + globalProperties = dict (defaultArg globalProperties []), + loggers = null, + remoteLoggers = null, + toolsetDefinitionLocations = ToolsetDefinitionLocations.Local, + loadProjectsReadOnly = true, + maxNodeCount = Environment.ProcessorCount, + onlyLogCriticalEvents = false + ) + let handleProjectGraphFailures f = try f () @@ -917,11 +952,61 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri None - let projectInstanceFactory projectPath (_globalProperties: IDictionary) (projectCollection: ProjectCollection) = - let tfm = ProjectLoader.getTfm projectPath (dict globalProperties) false - //let globalProperties = globalProperties |> Seq.toList |> List.map (fun (KeyValue(k,v)) -> (k,v)) - let globalProperties = ProjectLoader.getGlobalProps projectPath tfm globalProperties - ProjectInstance(projectPath, globalProperties, toolsVersion = null, projectCollection = projectCollection) + let projectInstanceFactory projectPath (globalProperties: IDictionary) (projectCollection: ProjectCollection) = + + // it's _super_ important that the 'same' project (path + properties) is only created once in a project collection, so we have to check on this here + let findOrCreateMatchingProject path (collection: ProjectCollection) globalProps = + let createNewProject properties = + Project( + projectFile = projectPath, + projectCollection = projectCollection, + globalProperties = properties, + toolsVersion = null, + loadSettings = + (ProjectLoadSettings.IgnoreMissingImports + ||| ProjectLoadSettings.IgnoreInvalidImports) + ) + + let hasSameGlobalProperties (globalProps: IDictionary) (incomingProject: Project) = + if + incomingProject.GlobalProperties.Count + <> globalProps.Count + then + false + else + globalProps + |> Seq.forall (fun (KeyValue(k, v)) -> + incomingProject.GlobalProperties.ContainsKey k + && incomingProject.GlobalProperties.[k] = v + ) + + match projectCollection.GetLoadedProjects(path) with + | null -> createNewProject globalProps + | existingProjects when existingProjects.Count = 0 -> createNewProject globalProps + | existingProjects -> + existingProjects + |> Seq.tryFind (hasSameGlobalProperties globalProps) + |> Option.defaultWith (fun _ -> createNewProject globalProps) + + let loadedProject = findOrCreateMatchingProject projectPath projectCollection globalProperties + + let projInstance = loadedProject.CreateProjectInstance() + let tfm = ProjectLoader.getTfm projInstance false + + let ourGlobalProperties = + ProjectLoader.getGlobalProps + tfm + [] + (globalProperties.Keys + |> Set.ofSeq) + + let combined = Dictionary(globalProperties) + + for kvp in ourGlobalProperties do + combined.Add(kvp.Key, kvp.Value) + + let tfm_specific_project = findOrCreateMatchingProject projectPath projectCollection combined + tfm_specific_project.CreateProjectInstance() let projectGraphProjects (paths: string seq) = @@ -937,7 +1022,7 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri with | [ x ] -> let g: ProjectGraph = - ProjectGraph(x, projectCollection = ProjectCollection.GlobalProjectCollection, projectInstanceFactory = projectInstanceFactory) + ProjectGraph(x, projectCollection = projectCollection, projectInstanceFactory = projectInstanceFactory) // When giving ProjectGraph a singular project, g.EntryPointNodes only contains that project. // To get it to build the Graph with all the dependencies we need to look at all the ProjectNodes // and tell the graph to use all as potentially an entrypoint @@ -945,7 +1030,7 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri g.ProjectNodes |> Seq.map (fun pn -> ProjectGraphEntryPoint pn.ProjectInstance.FullPath) - ProjectGraph(nodes, projectCollection = ProjectCollection.GlobalProjectCollection, projectInstanceFactory = projectInstanceFactory) + ProjectGraph(nodes, projectCollection = projectCollection, projectInstanceFactory = projectInstanceFactory) | xs -> let entryPoints = @@ -953,7 +1038,7 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri |> Seq.map ProjectGraphEntryPoint |> List.ofSeq - ProjectGraph(entryPoints, projectCollection = ProjectCollection.GlobalProjectCollection, projectInstanceFactory = projectInstanceFactory) + ProjectGraph(entryPoints, projectCollection = projectCollection, projectInstanceFactory = projectInstanceFactory) graph @@ -1033,6 +1118,7 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri ProjectLoadSettings.RecordEvaluatedItemElements ||| ProjectLoadSettings.ProfileEvaluation ||| ProjectLoadSettings.IgnoreMissingImports + ||| ProjectLoadSettings.IgnoreInvalidImports buildParameters.LogInitialPropertiesAndItems <- true bm.BeginBuild(buildParameters) @@ -1163,7 +1249,18 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri WorkspaceLoaderViaProjectGraph(toolsPath, ?globalProperties = globalProperties) :> IWorkspaceLoader type WorkspaceLoader private (toolsPath: ToolsPath, ?globalProperties: (string * string) list) = - let globalProperties = defaultArg globalProperties [] + + let projectCollection = + new ProjectCollection( + globalProperties = dict (defaultArg globalProperties []), + loggers = null, + remoteLoggers = null, + toolsetDefinitionLocations = ToolsetDefinitionLocations.Local, + maxNodeCount = Environment.ProcessorCount, + onlyLogCriticalEvents = false, + loadProjectsReadOnly = true + ) + let loadingNotification = new Event() interface IWorkspaceLoader with @@ -1180,28 +1277,8 @@ type WorkspaceLoader private (toolsPath: ToolsPath, ?globalProperties: (string * |> Seq.toList let rec loadProject p = - let res = ProjectLoader.getProjectInfo p globalProperties binaryLogs customProperties - match res with - | Ok project -> - try - cache.Add(p, project) - - let lst = - project.ReferencedProjects - |> Seq.choose (fun n -> - if cache.ContainsKey n.ProjectFileName then - None - else - Some n.ProjectFileName - ) - |> Seq.toList - - let info = Some project - lst, info - with exc -> - loadingNotification.Trigger(WorkspaceProjectState.Failed(p, GenericError(p, exc.Message))) - [], None + match ProjectLoader.loadProject p binaryLogs projectCollection with | Error msg when msg.Contains "The project file could not be loaded." -> loadingNotification.Trigger(WorkspaceProjectState.Failed(p, ProjectNotFound(p))) [], None @@ -1215,6 +1292,32 @@ type WorkspaceLoader private (toolsPath: ToolsPath, ?globalProperties: (string * | Error msg -> loadingNotification.Trigger(WorkspaceProjectState.Failed(p, GenericError(p, msg))) [], None + | Ok project -> + let mappedProjectInfo = ProjectLoader.getLoadedProjectInfo p customProperties project + + match mappedProjectInfo with + | Ok project -> + try + cache.Add(p, project) + + let lst = + project.ReferencedProjects + |> Seq.choose (fun n -> + if cache.ContainsKey n.ProjectFileName then + None + else + Some n.ProjectFileName + ) + |> Seq.toList + + let info = Some project + lst, info + with exc -> + loadingNotification.Trigger(WorkspaceProjectState.Failed(p, GenericError(p, exc.Message))) + [], None + | Error msg -> + loadingNotification.Trigger(WorkspaceProjectState.Failed(p, GenericError(p, msg))) + [], None let rec loadProjectList (projectList: string list) = for p in projectList do diff --git a/test/Ionide.ProjInfo.Tests/TestAssets.fs b/test/Ionide.ProjInfo.Tests/TestAssets.fs index 3684ec63..cdcfbaae 100644 --- a/test/Ionide.ProjInfo.Tests/TestAssets.fs +++ b/test/Ionide.ProjInfo.Tests/TestAssets.fs @@ -306,9 +306,7 @@ let ``NetSDK library referencing ProduceReferenceAssembly library`` = { let ``Console app with missing direct Import`` = { ProjDir = "missing-import" AssemblyName = "missing-import" - ProjectFile = - "missing-import" - / "missing-import.fsproj" + ProjectFile = "missing-import.fsproj" TargetFrameworks = Map.ofList [ "net6.0", sourceFiles [ "Program.fs" ] ] ProjectReferences = [] } diff --git a/test/Ionide.ProjInfo.Tests/Tests.fs b/test/Ionide.ProjInfo.Tests/Tests.fs index 53a4c6fc..34e071c7 100644 --- a/test/Ionide.ProjInfo.Tests/Tests.fs +++ b/test/Ionide.ProjInfo.Tests/Tests.fs @@ -1764,12 +1764,14 @@ let testLoadProject toolsPath = ] |> checkExitCodeZero - let projResult = ProjectLoader.getProjectInfo projPath [] BinaryLogGeneration.Off [] + let collection = new Microsoft.Build.Evaluation.ProjectCollection() - match projResult with - | Result.Ok proj -> Expect.equal proj.ProjectFileName projPath "project file names" + match ProjectLoader.loadProject projPath BinaryLogGeneration.Off collection with | Result.Error err -> failwith $"{err}" - + | Result.Ok proj -> + match ProjectLoader.getLoadedProjectInfo projPath [] proj with + | Result.Ok proj -> Expect.equal proj.ProjectFileName projPath "project file names" + | Result.Error err -> failwith $"{err}" ) let testProjectSystem toolsPath workspaceLoader workspaceFactory = @@ -2190,6 +2192,28 @@ let canLoadMissingImports toolsPath loaderType (workspaceFactory: ToolsPath -> I let projPath = pathForProject proj let loader = workspaceFactory toolsPath + let logger = Log.create (sprintf "Test '%s'" $"Can load projects with missing Imports - {loaderType}") + + loader.Notifications.Add( + function + | WorkspaceProjectState.Failed(projPath, errors) -> + logger.error ( + Message.eventX "Failed to load project {project} with {errors}" + >> Message.setField "project" projPath + >> Message.setField "errors" errors + ) + | WorkspaceProjectState.Loading p -> + logger.info ( + Message.eventX "Loading project {project}" + >> Message.setField "project" p + ) + | WorkspaceProjectState.Loaded(p, knownProjects, fromCache) -> + logger.info ( + Message.eventX "Loaded project {project}(fromCache: {fromCache})" + >> Message.setField "project" p + >> Message.setField "fromCache" fromCache + ) + ) let parsed = loader.LoadProjects [ projPath ] @@ -2197,8 +2221,8 @@ let canLoadMissingImports toolsPath loaderType (workspaceFactory: ToolsPath -> I Expect.equal parsed.Length 1 "Should have loaded the project" let parsed = parsed[0] - Expect.equal 1 parsed.SourceFiles.Length "Should have cracked a single file" - Expect.equal "Program.fs" parsed.SourceFiles.[0] "Filename should be Program.fs" + Expect.equal 3 parsed.SourceFiles.Length "Should have Program.fs, AssemblyInfo, and AssemblyAttributes" + Expect.stringEnds parsed.SourceFiles[2] "Program.fs" "Filename should be Program.fs" ) let tests toolsPath = From 7c0050a6ee22af251cacfa068852d22b579b1b4e Mon Sep 17 00:00:00 2001 From: Chet Husk Date: Sat, 20 Apr 2024 19:17:17 -0500 Subject: [PATCH 04/12] Isolate loaded projects to a unique collection per-load-request to prevent stale data --- src/Ionide.ProjInfo/Library.fs | 15 +++++++++------ test/Ionide.ProjInfo.Tests/Tests.fs | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/Ionide.ProjInfo/Library.fs b/src/Ionide.ProjInfo/Library.fs index b64f4684..b435b64d 100644 --- a/src/Ionide.ProjInfo/Library.fs +++ b/src/Ionide.ProjInfo/Library.fs @@ -924,7 +924,7 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri let logger = LogProvider.getLoggerFor () let loadingNotification = new Event() - let projectCollection = + let projectCollection () = new ProjectCollection( globalProperties = dict (defaultArg globalProperties []), loggers = null, @@ -1012,6 +1012,8 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri handleProjectGraphFailures <| fun () -> + let per_request_collection = projectCollection () + paths |> Seq.iter (fun p -> loadingNotification.Trigger(WorkspaceProjectState.Loading p)) @@ -1022,7 +1024,7 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri with | [ x ] -> let g: ProjectGraph = - ProjectGraph(x, projectCollection = projectCollection, projectInstanceFactory = projectInstanceFactory) + ProjectGraph(x, projectCollection = per_request_collection, projectInstanceFactory = projectInstanceFactory) // When giving ProjectGraph a singular project, g.EntryPointNodes only contains that project. // To get it to build the Graph with all the dependencies we need to look at all the ProjectNodes // and tell the graph to use all as potentially an entrypoint @@ -1030,7 +1032,7 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri g.ProjectNodes |> Seq.map (fun pn -> ProjectGraphEntryPoint pn.ProjectInstance.FullPath) - ProjectGraph(nodes, projectCollection = projectCollection, projectInstanceFactory = projectInstanceFactory) + ProjectGraph(nodes, projectCollection = per_request_collection, projectInstanceFactory = projectInstanceFactory) | xs -> let entryPoints = @@ -1038,7 +1040,7 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri |> Seq.map ProjectGraphEntryPoint |> List.ofSeq - ProjectGraph(entryPoints, projectCollection = projectCollection, projectInstanceFactory = projectInstanceFactory) + ProjectGraph(entryPoints, projectCollection = per_request_collection, projectInstanceFactory = projectInstanceFactory) graph @@ -1250,7 +1252,7 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri type WorkspaceLoader private (toolsPath: ToolsPath, ?globalProperties: (string * string) list) = - let projectCollection = + let projectCollection () = new ProjectCollection( globalProperties = dict (defaultArg globalProperties []), loggers = null, @@ -1270,6 +1272,7 @@ type WorkspaceLoader private (toolsPath: ToolsPath, ?globalProperties: (string * override __.LoadProjects(projects: string list, customProperties, binaryLogs) = let cache = Dictionary() + let per_request_collection = projectCollection () let getAllKnown () = cache @@ -1278,7 +1281,7 @@ type WorkspaceLoader private (toolsPath: ToolsPath, ?globalProperties: (string * let rec loadProject p = - match ProjectLoader.loadProject p binaryLogs projectCollection with + match ProjectLoader.loadProject p binaryLogs per_request_collection with | Error msg when msg.Contains "The project file could not be loaded." -> loadingNotification.Trigger(WorkspaceProjectState.Failed(p, ProjectNotFound(p))) [], None diff --git a/test/Ionide.ProjInfo.Tests/Tests.fs b/test/Ionide.ProjInfo.Tests/Tests.fs index 34e071c7..5f93bcea 100644 --- a/test/Ionide.ProjInfo.Tests/Tests.fs +++ b/test/Ionide.ProjInfo.Tests/Tests.fs @@ -2068,7 +2068,7 @@ let addFileToProject (projPath: string) fileName = let loadProjfileFromDiskTests toolsPath workspaceLoader (workspaceFactory: ToolsPath -> IWorkspaceLoader) = testCase |> withLog - $"can load project from disk everytime {workspaceLoader}" + $"can load project from disk everytime - {workspaceLoader}" (fun logger fs -> let loader = workspaceFactory toolsPath From 3d3a331280947395846aec1cee0309bdb7e46f36 Mon Sep 17 00:00:00 2001 From: Chet Husk Date: Sat, 20 Apr 2024 19:18:57 -0500 Subject: [PATCH 05/12] Dispose the per-request collections --- src/Ionide.ProjInfo/Library.fs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Ionide.ProjInfo/Library.fs b/src/Ionide.ProjInfo/Library.fs index b435b64d..9a8f3478 100644 --- a/src/Ionide.ProjInfo/Library.fs +++ b/src/Ionide.ProjInfo/Library.fs @@ -1012,7 +1012,7 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri handleProjectGraphFailures <| fun () -> - let per_request_collection = projectCollection () + use per_request_collection = projectCollection () paths |> Seq.iter (fun p -> loadingNotification.Trigger(WorkspaceProjectState.Loading p)) @@ -1272,7 +1272,7 @@ type WorkspaceLoader private (toolsPath: ToolsPath, ?globalProperties: (string * override __.LoadProjects(projects: string list, customProperties, binaryLogs) = let cache = Dictionary() - let per_request_collection = projectCollection () + use per_request_collection = projectCollection () let getAllKnown () = cache From 40b54581ee52ff2ecbb6b34cd6053c7a84cceae7 Mon Sep 17 00:00:00 2001 From: Chet Husk Date: Sat, 20 Apr 2024 19:24:07 -0500 Subject: [PATCH 06/12] Unify the logic around safely getting or creating a project in a collection --- src/Ionide.ProjInfo/Library.fs | 85 +++++++++++++++------------------- 1 file changed, 37 insertions(+), 48 deletions(-) diff --git a/src/Ionide.ProjInfo/Library.fs b/src/Ionide.ProjInfo/Library.fs index 9a8f3478..86ce2c92 100644 --- a/src/Ionide.ProjInfo/Library.fs +++ b/src/Ionide.ProjInfo/Library.fs @@ -385,6 +385,40 @@ module ProjectLoader = with set (v: LoggerVerbosity): unit = () } + // it's _super_ important that the 'same' project (path + properties) is only created once in a project collection, so we have to check on this here + let findOrCreateMatchingProject path (collection: ProjectCollection) globalProps = + let createNewProject properties = + Project( + projectFile = path, + projectCollection = collection, + globalProperties = properties, + toolsVersion = null, + loadSettings = + (ProjectLoadSettings.IgnoreMissingImports + ||| ProjectLoadSettings.IgnoreInvalidImports) + ) + + let hasSameGlobalProperties (globalProps: IDictionary) (incomingProject: Project) = + if + incomingProject.GlobalProperties.Count + <> globalProps.Count + then + false + else + globalProps + |> Seq.forall (fun (KeyValue(k, v)) -> + incomingProject.GlobalProperties.ContainsKey k + && incomingProject.GlobalProperties.[k] = v + ) + + match collection.GetLoadedProjects(path) with + | null -> createNewProject globalProps + | existingProjects when existingProjects.Count = 0 -> createNewProject globalProps + | existingProjects -> + existingProjects + |> Seq.tryFind (hasSameGlobalProperties globalProps) + |> Option.defaultWith (fun _ -> createNewProject globalProps) + let getTfm (pi: ProjectInstance) isLegacyFrameworkProj = let tfm = pi.GetPropertyValue( @@ -407,17 +441,7 @@ module ProjectLoader = Some tfm let loadProjectAndGetTFM (path: string) projectCollection readingProps isLegacyFrameworkProj = - let project = - Project( - projectFile = path, - globalProperties = readingProps, - toolsVersion = null, - projectCollection = projectCollection, - loadSettings = - (ProjectLoadSettings.IgnoreMissingImports - ||| ProjectLoadSettings.IgnoreInvalidImports) - ) - + let project = findOrCreateMatchingProject path projectCollection readingProps let pi = project.CreateProjectInstance() getTfm pi isLegacyFrameworkProj @@ -953,42 +977,7 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri None let projectInstanceFactory projectPath (globalProperties: IDictionary) (projectCollection: ProjectCollection) = - - // it's _super_ important that the 'same' project (path + properties) is only created once in a project collection, so we have to check on this here - let findOrCreateMatchingProject path (collection: ProjectCollection) globalProps = - let createNewProject properties = - Project( - projectFile = projectPath, - projectCollection = projectCollection, - globalProperties = properties, - toolsVersion = null, - loadSettings = - (ProjectLoadSettings.IgnoreMissingImports - ||| ProjectLoadSettings.IgnoreInvalidImports) - ) - - let hasSameGlobalProperties (globalProps: IDictionary) (incomingProject: Project) = - if - incomingProject.GlobalProperties.Count - <> globalProps.Count - then - false - else - globalProps - |> Seq.forall (fun (KeyValue(k, v)) -> - incomingProject.GlobalProperties.ContainsKey k - && incomingProject.GlobalProperties.[k] = v - ) - - match projectCollection.GetLoadedProjects(path) with - | null -> createNewProject globalProps - | existingProjects when existingProjects.Count = 0 -> createNewProject globalProps - | existingProjects -> - existingProjects - |> Seq.tryFind (hasSameGlobalProperties globalProps) - |> Option.defaultWith (fun _ -> createNewProject globalProps) - - let loadedProject = findOrCreateMatchingProject projectPath projectCollection globalProperties + let loadedProject = ProjectLoader.findOrCreateMatchingProject projectPath projectCollection globalProperties let projInstance = loadedProject.CreateProjectInstance() let tfm = ProjectLoader.getTfm projInstance false @@ -1005,7 +994,7 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri for kvp in ourGlobalProperties do combined.Add(kvp.Key, kvp.Value) - let tfm_specific_project = findOrCreateMatchingProject projectPath projectCollection combined + let tfm_specific_project = ProjectLoader.findOrCreateMatchingProject projectPath projectCollection combined tfm_specific_project.CreateProjectInstance() let projectGraphProjects (paths: string seq) = From 103824e6a0506119206ebcc0df1a2e954831102e Mon Sep 17 00:00:00 2001 From: Chet Husk Date: Sat, 20 Apr 2024 19:26:20 -0500 Subject: [PATCH 07/12] No for real safely create a project --- src/Ionide.ProjInfo/Library.fs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Ionide.ProjInfo/Library.fs b/src/Ionide.ProjInfo/Library.fs index 86ce2c92..cfe372b2 100644 --- a/src/Ionide.ProjInfo/Library.fs +++ b/src/Ionide.ProjInfo/Library.fs @@ -571,9 +571,7 @@ module ProjectLoader = ||| ProjectLoadSettings.ProfileEvaluation ||| ProjectLoadSettings.IgnoreMissingImports - let project = - Project(projectFile = path, globalProperties = globalProperties, toolsVersion = null, projectCollection = projectCollection, loadSettings = loadSettings) - + let project = findOrCreateMatchingProject path projectCollection globalProperties use sw = new StringWriter() let loggers = createLoggers [ path ] binaryLogs sw From a7d980754a60f187bb30496a69e2170e1b10a807 Mon Sep 17 00:00:00 2001 From: Chet Husk Date: Sat, 20 Apr 2024 19:55:58 -0500 Subject: [PATCH 08/12] fix the project-uniqueness comparison --- src/Ionide.ProjInfo/Library.fs | 89 +++++++++++++++++++++-------- test/Ionide.ProjInfo.Tests/Tests.fs | 2 +- 2 files changed, 67 insertions(+), 24 deletions(-) diff --git a/src/Ionide.ProjInfo/Library.fs b/src/Ionide.ProjInfo/Library.fs index cfe372b2..085efb1d 100644 --- a/src/Ionide.ProjInfo/Library.fs +++ b/src/Ionide.ProjInfo/Library.fs @@ -385,18 +385,59 @@ module ProjectLoader = with set (v: LoggerVerbosity): unit = () } + let mergeGlobalProperties (collection: ProjectCollection) (otherProperties: IDictionary) = + let combined = Dictionary(collection.GlobalProperties) + + for kvp in otherProperties do + combined.Add(kvp.Key, kvp.Value) + + combined + // it's _super_ important that the 'same' project (path + properties) is only created once in a project collection, so we have to check on this here let findOrCreateMatchingProject path (collection: ProjectCollection) globalProps = let createNewProject properties = - Project( - projectFile = path, - projectCollection = collection, - globalProperties = properties, - toolsVersion = null, - loadSettings = - (ProjectLoadSettings.IgnoreMissingImports - ||| ProjectLoadSettings.IgnoreInvalidImports) - ) + try + Project( + projectFile = path, + projectCollection = collection, + globalProperties = properties, + toolsVersion = null, + loadSettings = + (ProjectLoadSettings.IgnoreMissingImports + ||| ProjectLoadSettings.IgnoreInvalidImports) + ) + with :? System.InvalidOperationException as ex -> + + // if the project is already loaded throw a nicer message + let message = System.Text.StringBuilder() + + message + .AppendLine("The project '{path}' already exists in the project collection with the same global properties.") + .AppendLine("The global properties requested were:") + |> ignore + + for (KeyValue(k, v)) in properties do + message.AppendLine($" {k} = {v}") + |> ignore + + message.AppendLine() + |> ignore + + message.AppendLine("There are projects of the following properties already in the collection:") + |> ignore + + for project in collection.GetLoadedProjects(path) do + message.AppendLine($"Evaluation #{project.LastEvaluationId}") + |> ignore + + for (KeyValue(k, v)) in project.GlobalProperties do + message.AppendLine($" {k} = {v}") + |> ignore + + message.AppendLine() + |> ignore + + failwith (message.ToString()) let hasSameGlobalProperties (globalProps: IDictionary) (incomingProject: Project) = if @@ -411,13 +452,19 @@ module ProjectLoader = && incomingProject.GlobalProperties.[k] = v ) - match collection.GetLoadedProjects(path) with - | null -> createNewProject globalProps - | existingProjects when existingProjects.Count = 0 -> createNewProject globalProps - | existingProjects -> - existingProjects - |> Seq.tryFind (hasSameGlobalProperties globalProps) - |> Option.defaultWith (fun _ -> createNewProject globalProps) + lock + (collection) + (fun _ -> + match collection.GetLoadedProjects(path) with + | null -> createNewProject globalProps + | existingProjects when existingProjects.Count = 0 -> createNewProject globalProps + | existingProjects -> + let totalGlobalProps = mergeGlobalProperties collection globalProps + + existingProjects + |> Seq.tryFind (hasSameGlobalProperties totalGlobalProps) + |> Option.defaultWith (fun _ -> createNewProject globalProps) + ) let getTfm (pi: ProjectInstance) isLegacyFrameworkProj = let tfm = @@ -985,14 +1032,10 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri tfm [] (globalProperties.Keys - |> Set.ofSeq) - - let combined = Dictionary(globalProperties) - - for kvp in ourGlobalProperties do - combined.Add(kvp.Key, kvp.Value) + |> Set.ofSeq + |> Set.union (Set.ofSeq projectCollection.GlobalProperties.Keys)) - let tfm_specific_project = ProjectLoader.findOrCreateMatchingProject projectPath projectCollection combined + let tfm_specific_project = ProjectLoader.findOrCreateMatchingProject projectPath projectCollection ourGlobalProperties tfm_specific_project.CreateProjectInstance() let projectGraphProjects (paths: string seq) = diff --git a/test/Ionide.ProjInfo.Tests/Tests.fs b/test/Ionide.ProjInfo.Tests/Tests.fs index 5f93bcea..24691586 100644 --- a/test/Ionide.ProjInfo.Tests/Tests.fs +++ b/test/Ionide.ProjInfo.Tests/Tests.fs @@ -365,7 +365,7 @@ let testLegacyFrameworkMultiProject toolsPath workspaceLoader isRelease (workspa let testSample2 toolsPath workspaceLoader isRelease (workspaceFactory: ToolsPath * (string * string) list -> IWorkspaceLoader) = testCase |> withLog - (sprintf "can load sample2 - %s - isRelease is %b" workspaceLoader isRelease) + (sprintf "can load sample2 - isRelease is %b - %s" isRelease workspaceLoader) (fun logger fs -> let testDir = inDir fs "load_sample2" copyDirFromAssets fs ``sample2 NetSdk library``.ProjDir testDir From f99e8e956fa2ce3a1e7e13d8096b4e9ba09e90aa Mon Sep 17 00:00:00 2001 From: Chet Husk Date: Sat, 20 Apr 2024 21:16:21 -0500 Subject: [PATCH 09/12] A spot of code cleanup --- src/Ionide.ProjInfo/Library.fs | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/src/Ionide.ProjInfo/Library.fs b/src/Ionide.ProjInfo/Library.fs index 085efb1d..6a3ff740 100644 --- a/src/Ionide.ProjInfo/Library.fs +++ b/src/Ionide.ProjInfo/Library.fs @@ -612,12 +612,6 @@ module ProjectLoader = let tfm = loadProjectAndGetTFM path projectCollection readingProps isLegacyFrameworkProjFile let globalProperties = getGlobalProps tfm [] collectionProps - - let loadSettings = - ProjectLoadSettings.RecordEvaluatedItemElements - ||| ProjectLoadSettings.ProfileEvaluation - ||| ProjectLoadSettings.IgnoreMissingImports - let project = findOrCreateMatchingProject path projectCollection globalProperties use sw = new StringWriter() @@ -929,21 +923,6 @@ module ProjectLoader = Result.Ok proj -// /// -// /// Main entry point for project loading. -// /// -// /// Full path to the `.fsproj` file -// /// describes if and how to generate MsBuild binary logs -// /// The propertyCollection to load the project into. This will provide global properties by default. -// /// List of additional MsBuild properties that you want to obtain. -// /// Returns the record instance representing the loaded project or string containing error message -// let getProjectInfo (path: string) projectCollection (binaryLogs: BinaryLogGeneration) (customProperties: string list) : Result = -// let loadedProject = loadProject path binaryLogs projectCollection - -// match loadedProject with -// | ProjectLoadingStatus.Success project -> getLoadedProjectInfo path customProperties project -// | ProjectLoadingStatus.Error e -> Result.Error e - /// A type that turns project files or solution files into deconstructed options. /// Use this in conjunction with the other ProjInfo libraries to turn these options into /// ones compatible for use with FCS directly. From 9ab3cd228ac00b0a09079b4e2434eac6e6e31cce Mon Sep 17 00:00:00 2001 From: Chet Husk Date: Sun, 21 Apr 2024 11:44:56 -0500 Subject: [PATCH 10/12] better logging and diagnostics --- src/Ionide.ProjInfo/Library.fs | 85 ++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 36 deletions(-) diff --git a/src/Ionide.ProjInfo/Library.fs b/src/Ionide.ProjInfo/Library.fs index 6a3ff740..da9b1a52 100644 --- a/src/Ionide.ProjInfo/Library.fs +++ b/src/Ionide.ProjInfo/Library.fs @@ -331,10 +331,10 @@ module ProjectLoader = type LoadedProject = internal LoadedProject of ProjectInstance - let internal msBuildLogger = lazy (LogProvider.getLoggerByName "MsBuild") //lazy because dotnet test wont pickup our logger otherwise + let internal projectLoaderLogger = LogProvider.getLoggerByName "ProjectLoader" let msBuildToLogProvider () = - let msBuildLogger = msBuildLogger.Value + let msBuildLogger = (LogProvider.getLoggerByName "MsBuild") //lazy because dotnet test wont pickup our logger otherwise { new ILogger with member this.Initialize(eventSource: IEventSource) : unit = @@ -393,26 +393,18 @@ module ProjectLoader = combined - // it's _super_ important that the 'same' project (path + properties) is only created once in a project collection, so we have to check on this here - let findOrCreateMatchingProject path (collection: ProjectCollection) globalProps = - let createNewProject properties = - try - Project( - projectFile = path, - projectCollection = collection, - globalProperties = properties, - toolsVersion = null, - loadSettings = - (ProjectLoadSettings.IgnoreMissingImports - ||| ProjectLoadSettings.IgnoreInvalidImports) - ) - with :? System.InvalidOperationException as ex -> + type ProjectAlreadyLoaded(projectPath: string, collection: ProjectCollection, properties: IDictionary, innerException) = + inherit System.Exception("", innerException) + let mutable _message = null + override this.Message = + match _message with + | null -> // if the project is already loaded throw a nicer message let message = System.Text.StringBuilder() message - .AppendLine("The project '{path}' already exists in the project collection with the same global properties.") + .AppendLine($"The project '{projectPath}' already exists in the project collection with the same global properties.") .AppendLine("The global properties requested were:") |> ignore @@ -426,7 +418,7 @@ module ProjectLoader = message.AppendLine("There are projects of the following properties already in the collection:") |> ignore - for project in collection.GetLoadedProjects(path) do + for project in collection.GetLoadedProjects(projectPath) do message.AppendLine($"Evaluation #{project.LastEvaluationId}") |> ignore @@ -437,7 +429,26 @@ module ProjectLoader = message.AppendLine() |> ignore - failwith (message.ToString()) + _message <- message.ToString() + | _ -> () + + _message + + // it's _super_ important that the 'same' project (path + properties) is only created once in a project collection, so we have to check on this here + let findOrCreateMatchingProject path (collection: ProjectCollection) globalProps = + let createNewProject properties = + try + Project( + projectFile = path, + projectCollection = collection, + globalProperties = properties, + toolsVersion = null, + loadSettings = + (ProjectLoadSettings.IgnoreMissingImports + ||| ProjectLoadSettings.IgnoreInvalidImports) + ) + with :? System.InvalidOperationException as ex -> + raise (ProjectAlreadyLoaded(path, collection, properties, ex)) let hasSameGlobalProperties (globalProps: IDictionary) (incomingProject: Project) = if @@ -626,6 +637,12 @@ module ProjectLoader = else Error(sw.ToString()) with exc -> + projectLoaderLogger.error ( + Log.setMessage "Generic error while loading project {path}" + >> Log.addExn exc + >> Log.addContextDestructured "path" path + ) + Error(exc.Message) let getFscArgs (LoadedProject project) = @@ -1309,24 +1326,20 @@ type WorkspaceLoader private (toolsPath: ToolsPath, ?globalProperties: (string * match mappedProjectInfo with | Ok project -> - try - cache.Add(p, project) - - let lst = - project.ReferencedProjects - |> Seq.choose (fun n -> - if cache.ContainsKey n.ProjectFileName then - None - else - Some n.ProjectFileName - ) - |> Seq.toList + cache.Add(p, project) + + let lst = + project.ReferencedProjects + |> Seq.choose (fun n -> + if cache.ContainsKey n.ProjectFileName then + None + else + Some n.ProjectFileName + ) + |> Seq.toList - let info = Some project - lst, info - with exc -> - loadingNotification.Trigger(WorkspaceProjectState.Failed(p, GenericError(p, exc.Message))) - [], None + let info = Some project + lst, info | Error msg -> loadingNotification.Trigger(WorkspaceProjectState.Failed(p, GenericError(p, msg))) [], None From 967f2c982e0c2b719268ddf3a7b84b793ec7c2ed Mon Sep 17 00:00:00 2001 From: Chet Husk Date: Sun, 21 Apr 2024 11:54:31 -0500 Subject: [PATCH 11/12] CI: fix CI breaks when uploading the same asset (nuget packages in this case) multiple times we don't need to keep the nugets from PR builds so we'll save that step just for the release. --- .github/workflows/build.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 27c48cb3..6b63052f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -74,12 +74,6 @@ jobs: env: BuildNet7: ${{ matrix.build_net7 }} - - name: Upload NuGet packages - uses: actions/upload-artifact@v2 - with: - name: packages - path: src/**/*.nupkg - - name: Archive test results uses: actions/upload-artifact@v3 From 74e8b10565eaaf159edf04ba5894dfb49454b302 Mon Sep 17 00:00:00 2001 From: Chet Husk Date: Sun, 21 Apr 2024 13:50:25 -0500 Subject: [PATCH 12/12] Fix logger initialization for the new message --- src/Ionide.ProjInfo/Library.fs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Ionide.ProjInfo/Library.fs b/src/Ionide.ProjInfo/Library.fs index da9b1a52..ad683f7b 100644 --- a/src/Ionide.ProjInfo/Library.fs +++ b/src/Ionide.ProjInfo/Library.fs @@ -331,10 +331,10 @@ module ProjectLoader = type LoadedProject = internal LoadedProject of ProjectInstance - let internal projectLoaderLogger = LogProvider.getLoggerByName "ProjectLoader" + let internal projectLoaderLogger = lazy (LogProvider.getLoggerByName "ProjectLoader") let msBuildToLogProvider () = - let msBuildLogger = (LogProvider.getLoggerByName "MsBuild") //lazy because dotnet test wont pickup our logger otherwise + let msBuildLogger = LogProvider.getLoggerByName "MsBuild" { new ILogger with member this.Initialize(eventSource: IEventSource) : unit = @@ -637,7 +637,7 @@ module ProjectLoader = else Error(sw.ToString()) with exc -> - projectLoaderLogger.error ( + projectLoaderLogger.Value.error ( Log.setMessage "Generic error while loading project {path}" >> Log.addExn exc >> Log.addContextDestructured "path" path