feedback

Author: KevinRansom

src/fsharp/Microsoft.DotNet.DependencyManager/DependencyProvider.fs

@@ -420,7 +420,7 @@ type DependencyProvider (assemblyProbingPaths: AssemblyResolutionProbe, nativePr
             ))
         match result with
         | Ok res ->
-            dllResolveHandler.RefreshPathsInEnvironment()
+            dllResolveHandler.RefreshPathsInEnvironment(res.Roots)
             res
         | Error (errorNumber, errorData) ->
             reportError.Invoke(ErrorReportType.Error, errorNumber, errorData)

src/fsharp/Microsoft.DotNet.DependencyManager/Microsoft.DotNet.DependencyManager.fsproj

@@ -4,7 +4,7 @@
 
   <PropertyGroup>
     <OutputType>Library</OutputType>
-    <TargetFrameworks>netstandard2.0</TargetFrameworks>
+    <TargetFramework>netstandard2.0</TargetFramework>
     <IsPackable>true</IsPackable>
     <AssemblyName>Microsoft.DotNet.DependencyManager</AssemblyName>
     <NoWarn>$(NoWarn);45;55;62;75;1204</NoWarn>

src/fsharp/Microsoft.DotNet.DependencyManager/NativeDllResolveHandler.fs

@@ -107,36 +107,43 @@ type NativeDllResolveHandlerCoreClr (nativeProbingRoots: NativeResolutionProbe)
             ()
 
 type NativeDllResolveHandler (nativeProbingRoots: NativeResolutionProbe) =
-    let addedPaths = ConcurrentBag<string>()
     let handler:IDisposable option =
         if isRunningOnCoreClr then
             Some (new NativeDllResolveHandlerCoreClr(nativeProbingRoots) :> IDisposable)
         else
             None
 
-    member internal _.RefreshPathsInEnvironment() =
-        for probePath in (nativeProbingRoots.Invoke()) do
-            let path =
-                let p = Environment.GetEnvironmentVariable("PATH")
-                if not(p.EndsWith(";", StringComparison.OrdinalIgnoreCase)) then p + ";"
-                else p
+    let appendSemiColon (p:string) =
+        if not(p.EndsWith(";", StringComparison.OrdinalIgnoreCase)) then
+            p + ";"
+        else
+            p
+
+    let addedPaths = ConcurrentBag<string>()
 
-            if not (probePath.Contains(path + ";")) then
-                Environment.SetEnvironmentVariable("PATH", path + probePath + ";")
-                addedPaths.Add path
+    let addProbeToProcessPath probePath =
+        let probe = appendSemiColon probePath
+        let path = appendSemiColon (Environment.GetEnvironmentVariable("PATH"))
+        if not (path.Contains(probe)) then
+            Environment.SetEnvironmentVariable("PATH", path + probe)
+            addedPaths.Add probe
+
+    let removeProbeFromProcessPath probePath =
+        if not(String.IsNullOrWhiteSpace(probePath)) then
+            let probe = appendSemiColon probePath
+            let path = appendSemiColon (Environment.GetEnvironmentVariable("PATH"))
+            if path.Contains(probe) then Environment.SetEnvironmentVariable("PATH", path.Replace(probe, ""))
+
+    member internal _.RefreshPathsInEnvironment(roots: string seq) =
+        for probePath in roots do
+            addProbeToProcessPath probePath
 
     interface IDisposable with
         member _.Dispose() =
             match handler with
             | None -> ()
             | Some handler -> handler.Dispose()
 
-            for probePath in addedPaths do
-                let path =
-                    let p = Environment.GetEnvironmentVariable("PATH")
-                    if not(p.EndsWith(";", StringComparison.OrdinalIgnoreCase)) then p + ";"
-                    else p
-
-                Environment.SetEnvironmentVariable("PATH", path.Replace(probePath + ";", ""))
-                let mutable _p = null
-                addedPaths.TryTake(ref _p) |> ignore
+            let mutable probe:string = null
+            while (addedPaths.TryTake(&probe)) do
+                removeProbeFromProcessPath probe

src/fsharp/Microsoft.DotNet.DependencyManager/NativeDllResolveHandler.fsi

@@ -14,6 +14,6 @@ type NativeDllResolveHandler =
     /// Construct a new NativeDllResolveHandler
     new: nativeProbingRoots: NativeResolutionProbe -> NativeDllResolveHandler
 
-    member internal RefreshPathsInEnvironment: unit -> unit
+    member internal RefreshPathsInEnvironment: string seq -> unit
 
     interface IDisposable

tests/FSharp.Compiler.Private.Scripting.UnitTests/DependencyManagerInteractiveTests.fs

@@ -657,6 +657,55 @@ x |> Seq.iter(fun r ->
         try Assembly.Load("NoneSuchAssembly") |> ignore with _ -> ()
         Assert.False (assemblyFound, "Invoke the assemblyProbingRoots callback -- Error the AssemblyResolve still fired ")
 
+    [<Fact>]
+    member __.``Verify that Dispose cleans up the native paths added``() =
+        let nativeProbingRoots () = Seq.empty<string>
+
+        let appendSemiColon (p:string) =
+            if not(p.EndsWith(";", StringComparison.OrdinalIgnoreCase)) then
+                p + ";"
+            else
+                p
+
+        let reportError =
+            let report errorType code message =
+                match errorType with
+                | ErrorReportType.Error -> printfn "PackageManagementError %d : %s" code message
+                | ErrorReportType.Warning -> printfn "PackageManagementWarning %d : %s" code message
+            ResolvingErrorReport (report)
+
+        let mutable initialPath:string = null
+        let mutable currentPath:string = null
+        let mutable finalPath:string =  null
+        do
+            initialPath <- appendSemiColon (Environment.GetEnvironmentVariable("PATH"))
+            use dp = new DependencyProvider(NativeResolutionProbe(nativeProbingRoots))
+            let idm = dp.TryFindDependencyManagerByKey(Seq.empty, "", reportError, "nuget")
+            let mutable currentPath:string = null
+            if RuntimeInformation.IsOSPlatform(OSPlatform.Windows) then
+                let result = dp.Resolve(idm, ".fsx", [|"r", "Microsoft.Data.Sqlite,3.1.7"|], reportError, "netstandard2.0")
+                Assert.Equal(true, result.Success)
+                currentPath <-  appendSemiColon (Environment.GetEnvironmentVariable("PATH"))
+        finalPath <- appendSemiColon (Environment.GetEnvironmentVariable("PATH"))
+        Assert.True(currentPath <> initialPath)     // The path was modified by #r "nuget: ..."
+        Assert.Equal(finalPath, initialPath)        // IDispose correctly cleaned up the path
+
+        initialPath <- null
+        currentPath <- null
+        finalPath <-  null
+        do
+            initialPath <- appendSemiColon (Environment.GetEnvironmentVariable("PATH"))
+            let mutable currentPath:string = null
+            use dp = new DependencyProvider(NativeResolutionProbe(nativeProbingRoots))
+            let idm = dp.TryFindDependencyManagerByKey(Seq.empty, "", reportError, "nuget")
+            let result = dp.Resolve(idm, ".fsx", [|"r", "Microsoft.Data.Sqlite,3.1.7"|], reportError, "netcoreapp3.1")
+            Assert.Equal(true, result.Success)
+            currentPath <-  appendSemiColon (Environment.GetEnvironmentVariable("PATH"))
+        finalPath <- appendSemiColon (Environment.GetEnvironmentVariable("PATH"))
+        Assert.True(currentPath <> initialPath)      // The path was modified by #r "nuget: ..."
+        Assert.Equal(finalPath, initialPath)        // IDispose correctly cleaned up the path
+
+        ()
 
     [<Fact>]
     member __.``Verify that #help produces help text for fsi + dependency manager``() =

vsintegration/Vsix/VisualFSharpFull/VisualFSharpFull.csproj

@@ -95,7 +95,6 @@
       <NgenArchitecture>All</NgenArchitecture>
       <NgenPriority>2</NgenPriority>
       <Private>True</Private>
-      <AdditionalProperties>TargetFramework=netstandard2.0</AdditionalProperties>
     </ProjectReference>
     <ProjectReference Include="$(FSharpSourcesRoot)\fsharp\FSharp.Core\FSharp.Core.fsproj">
       <Project>{DED3BBD7-53F4-428A-8C9F-27968E768605}</Project>