From ab40382869e2d88998d95c424485b1e2739861c6 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Tue, 27 Sep 2022 15:47:15 +0100 Subject: [PATCH 1/5] fix regression in editing scripts in devenv.exe --- src/Compiler/Driver/FxResolver.fs | 50 +++++++++++++++++-- .../FSharpProjectOptionsManager.fs | 2 +- .../src/FSharp.VS.FSI/fsiLanguageService.fs | 10 ++-- 3 files changed, 52 insertions(+), 10 deletions(-) diff --git a/src/Compiler/Driver/FxResolver.fs b/src/Compiler/Driver/FxResolver.fs index 2fa595ee15b..cecb58623b5 100644 --- a/src/Compiler/Driver/FxResolver.fs +++ b/src/Compiler/Driver/FxResolver.fs @@ -386,7 +386,7 @@ type internal FxResolver // On coreclr it uses the deps.json file // // On-demand because (a) some FxResolver are ephemeral (b) we want to avoid recomputation - let tryGetRunningTfm = + let tryGetRunningTfm() = let runningTfmOpt = let getTfmNumber (v: string) = let arr = v.Split([| '.' |], 3) @@ -806,12 +806,54 @@ type internal FxResolver RequireFxResolverLock(fxtok, "assuming all member require lock") tryGetSdkDir () |> replayWarnings) - /// Gets the selected target framework moniker, e.g netcore3.0, net472, and the running rid of the current machine + /// Gets + /// 1. The Target Framework Moniker (TFM) used for scripting (e.g netcore3.0, net472) + /// 2. The running RID of the current machine (e.g. win-x64) + /// + /// When analyzing scripts for editing, this is **not** necessarily the running TFM. Rather, it is the TFM to use for analysing + /// a script. + /// + /// Summary: + /// - When running scripts (isInteractive = true) this is identical to the running TFM. + /// + /// - When analyzing .NET Core scripts (isInteractive = false, tryGetSdkDir is Some), + /// the scripting TFM is determined from dotnet.runtimeconfig.json in the SDK directory + /// + /// - Otherwise, the running TFM is used. That is, if editing with .NET Framework/Core-based tooling a script is assumed + /// to be .NET Framework/Core respectively. + /// + /// The RID returned is always the RID of the running machine. member _.GetTfmAndRid() = fxlock.AcquireLock(fun fxtok -> RequireFxResolverLock(fxtok, "assuming all member require lock") - let runningTfm = tryGetRunningTfm + // Interactive processes read their own configuration to find the running tfm + + let targetTfm = + if isInteractive then + tryGetRunningTfm() + else + let sdkDir = tryGetSdkDir () |> replayWarnings + + match sdkDir with + | Some dir -> + let dotnetConfigFile = Path.Combine(dir, "dotnet.runtimeconfig.json") + try + use stream = FileSystem.OpenFileForReadShim(dotnetConfigFile) + let dotnetConfig = stream.ReadAllText() + let pattern = "\"tfm\": \"" + + let startPos = + dotnetConfig.IndexOf(pattern, StringComparison.OrdinalIgnoreCase) + + pattern.Length + + let endPos = dotnetConfig.IndexOf("\"", startPos) + let tfm = dotnetConfig[startPos .. endPos - 1] + tfm + with _ -> + tryGetRunningTfm() + | None -> + tryGetRunningTfm() // Coreclr has mechanism for getting rid // System.Runtime.InteropServices.RuntimeInformation.RuntimeIdentifier @@ -862,7 +904,7 @@ type internal FxResolver | Architecture.Arm64 -> baseRid + "-arm64" | _ -> baseRid + "-arm" - runningTfm, runningRid) + targetTfm, runningRid) static member ClearStaticCaches() = desiredDotNetSdkVersionForDirectoryCache.Clear() diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs index 815fab5e904..92d2cefaae5 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs @@ -182,7 +182,7 @@ type private FSharpProjectOptionsReactor (checker: FSharpChecker) = let! scriptProjectOptions, _ = checker.GetProjectOptionsFromScript(document.FilePath, sourceText.ToFSharpSourceText(), - SessionsProperties.fsiPreview, + previewEnabled=SessionsProperties.fsiPreview, assumeDotNetFramework=not SessionsProperties.fsiUseNetCore, userOpName=userOpName) diff --git a/vsintegration/src/FSharp.VS.FSI/fsiLanguageService.fs b/vsintegration/src/FSharp.VS.FSI/fsiLanguageService.fs index 8de6290a979..b943d3cca13 100644 --- a/vsintegration/src/FSharp.VS.FSI/fsiLanguageService.fs +++ b/vsintegration/src/FSharp.VS.FSI/fsiLanguageService.fs @@ -46,6 +46,11 @@ type FsiPropertyPage() = [] member this.FsiShadowCopy with get() = SessionsProperties.fsiShadowCopy and set (x:bool) = SessionsProperties.fsiShadowCopy <- x + [] + [] + [] + member this.FsiUseNetCore with get() = SessionsProperties.fsiUseNetCore and set (x:bool) = SessionsProperties.fsiUseNetCore <- x + [] [] [] @@ -56,11 +61,6 @@ type FsiPropertyPage() = [] member this.FsiPreview with get() = SessionsProperties.fsiPreview and set (x:bool) = SessionsProperties.fsiPreview <- x - [] - [] - [] - member this.FsiUseNetCore with get() = SessionsProperties.fsiUseNetCore and set (x:bool) = SessionsProperties.fsiUseNetCore <- x - // CompletionSet type internal FsiCompletionSet(imageList,source:Source) = inherit CompletionSet(imageList, source) From 8d76f23f83aefe001c2bb2d16b97b440e30e54e1 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Tue, 27 Sep 2022 15:49:58 +0100 Subject: [PATCH 2/5] fix regression in editing scripts in devenv.exe --- src/Compiler/Driver/CompilerImports.fs | 1 + src/Compiler/Driver/FxResolver.fs | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/Compiler/Driver/CompilerImports.fs b/src/Compiler/Driver/CompilerImports.fs index 56513446bd3..a329635b609 100644 --- a/src/Compiler/Driver/CompilerImports.fs +++ b/src/Compiler/Driver/CompilerImports.fs @@ -2142,6 +2142,7 @@ and [] TcImports CheckDisposed() let tcConfig = tcConfigP.Get ctok + let runMethod = match tcConfig.parallelReferenceResolution with | ParallelReferenceResolution.On -> NodeCode.Parallel diff --git a/src/Compiler/Driver/FxResolver.fs b/src/Compiler/Driver/FxResolver.fs index cecb58623b5..1ef04c4c690 100644 --- a/src/Compiler/Driver/FxResolver.fs +++ b/src/Compiler/Driver/FxResolver.fs @@ -386,7 +386,7 @@ type internal FxResolver // On coreclr it uses the deps.json file // // On-demand because (a) some FxResolver are ephemeral (b) we want to avoid recomputation - let tryGetRunningTfm() = + let tryGetRunningTfm () = let runningTfmOpt = let getTfmNumber (v: string) = let arr = v.Split([| '.' |], 3) @@ -831,13 +831,14 @@ type internal FxResolver let targetTfm = if isInteractive then - tryGetRunningTfm() + tryGetRunningTfm () else let sdkDir = tryGetSdkDir () |> replayWarnings match sdkDir with | Some dir -> let dotnetConfigFile = Path.Combine(dir, "dotnet.runtimeconfig.json") + try use stream = FileSystem.OpenFileForReadShim(dotnetConfigFile) let dotnetConfig = stream.ReadAllText() @@ -851,9 +852,8 @@ type internal FxResolver let tfm = dotnetConfig[startPos .. endPos - 1] tfm with _ -> - tryGetRunningTfm() - | None -> - tryGetRunningTfm() + tryGetRunningTfm () + | None -> tryGetRunningTfm () // Coreclr has mechanism for getting rid // System.Runtime.InteropServices.RuntimeInformation.RuntimeIdentifier From 1dfb909a1fc16c824fdd0fd16bd42d81efe8e472 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Tue, 27 Sep 2022 15:53:10 +0100 Subject: [PATCH 3/5] fix regression in editing scripts in devenv.exe --- src/Compiler/Driver/FxResolver.fs | 37 ++++++++++++++++--------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/Compiler/Driver/FxResolver.fs b/src/Compiler/Driver/FxResolver.fs index 1ef04c4c690..d4233c103f2 100644 --- a/src/Compiler/Driver/FxResolver.fs +++ b/src/Compiler/Driver/FxResolver.fs @@ -548,6 +548,24 @@ type internal FxResolver assemblyReferences |> List.iter traverseDependencies assemblies + let tryGetTfmFromSdkDir (sdkDir: string) = + let dotnetConfigFile = Path.Combine(sdkDir, "dotnet.runtimeconfig.json") + + try + use stream = FileSystem.OpenFileForReadShim(dotnetConfigFile) + let dotnetConfig = stream.ReadAllText() + let pattern = "\"tfm\": \"" + + let startPos = + dotnetConfig.IndexOf(pattern, StringComparison.OrdinalIgnoreCase) + + pattern.Length + + let endPos = dotnetConfig.IndexOf("\"", startPos) + let tfm = dotnetConfig[startPos .. endPos - 1] + tfm + with _ -> + tryGetRunningTfm () + // This list is the default set of references for "non-project" files. // // These DLLs are @@ -828,7 +846,6 @@ type internal FxResolver RequireFxResolverLock(fxtok, "assuming all member require lock") // Interactive processes read their own configuration to find the running tfm - let targetTfm = if isInteractive then tryGetRunningTfm () @@ -836,23 +853,7 @@ type internal FxResolver let sdkDir = tryGetSdkDir () |> replayWarnings match sdkDir with - | Some dir -> - let dotnetConfigFile = Path.Combine(dir, "dotnet.runtimeconfig.json") - - try - use stream = FileSystem.OpenFileForReadShim(dotnetConfigFile) - let dotnetConfig = stream.ReadAllText() - let pattern = "\"tfm\": \"" - - let startPos = - dotnetConfig.IndexOf(pattern, StringComparison.OrdinalIgnoreCase) - + pattern.Length - - let endPos = dotnetConfig.IndexOf("\"", startPos) - let tfm = dotnetConfig[startPos .. endPos - 1] - tfm - with _ -> - tryGetRunningTfm () + | Some dir -> tryGetTfmFromSdkDir () | None -> tryGetRunningTfm () // Coreclr has mechanism for getting rid From 3854c91ee57ae7af12c8fcf6538cc829a43bd00c Mon Sep 17 00:00:00 2001 From: Don Syme Date: Tue, 27 Sep 2022 15:59:15 +0100 Subject: [PATCH 4/5] fix regression in editing scripts in devenv.exe --- src/Compiler/Driver/FxResolver.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compiler/Driver/FxResolver.fs b/src/Compiler/Driver/FxResolver.fs index d4233c103f2..3f3e59c2be4 100644 --- a/src/Compiler/Driver/FxResolver.fs +++ b/src/Compiler/Driver/FxResolver.fs @@ -853,7 +853,7 @@ type internal FxResolver let sdkDir = tryGetSdkDir () |> replayWarnings match sdkDir with - | Some dir -> tryGetTfmFromSdkDir () + | Some dir -> tryGetTfmFromSdkDir dir | None -> tryGetRunningTfm () // Coreclr has mechanism for getting rid From 47564b35763a385fa4d7d1287ccb5874af1fe04c Mon Sep 17 00:00:00 2001 From: Don Syme Date: Tue, 27 Sep 2022 16:50:55 +0100 Subject: [PATCH 5/5] add tests --- src/Compiler/Driver/FxResolver.fs | 10 +++++----- tests/service/ScriptOptionsTests.fs | 29 ++++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/src/Compiler/Driver/FxResolver.fs b/src/Compiler/Driver/FxResolver.fs index 3f3e59c2be4..ae69346d689 100644 --- a/src/Compiler/Driver/FxResolver.fs +++ b/src/Compiler/Driver/FxResolver.fs @@ -382,16 +382,16 @@ type internal FxResolver let tryGetNetCoreRefsPackDirectoryRoot () = tryNetCoreRefsPackDirectoryRoot.Force() + let getTfmNumber (v: string) = + let arr = v.Split([| '.' |], 3) + arr[0] + "." + arr[1] + // Tries to figure out the tfm for the compiler instance. // On coreclr it uses the deps.json file // // On-demand because (a) some FxResolver are ephemeral (b) we want to avoid recomputation let tryGetRunningTfm () = let runningTfmOpt = - let getTfmNumber (v: string) = - let arr = v.Split([| '.' |], 3) - arr[0] + "." + arr[1] - // Compute TFM from System.Runtime.InteropServices.RuntimeInformation.FrameworkDescription // System.Runtime.InteropServices.RuntimeInformation.FrameworkDescription;; // val it: string = ".NET 6.0.7" @@ -928,7 +928,7 @@ type internal FxResolver let defaultReferences = if assumeDotNetFramework then getDotNetFrameworkDefaultReferences useFsiAuxLib, assumeDotNetFramework - else if useSdkRefs then + elif useSdkRefs then // Go fetch references let sdkDir = tryGetSdkRefsPackDirectory () |> replayWarnings diff --git a/tests/service/ScriptOptionsTests.fs b/tests/service/ScriptOptionsTests.fs index a533d7ac4c8..998120a81d4 100644 --- a/tests/service/ScriptOptionsTests.fs +++ b/tests/service/ScriptOptionsTests.fs @@ -24,11 +24,12 @@ let pi = Math.PI """ [] +[] [] [] let ``can generate options for different frameworks regardless of execution environment``(assumeNetFx, useSdk, flags) = let path = Path.GetTempPath() - let file = tryCreateTemporaryFileName () + let file = tryCreateTemporaryFileName () + ".fsx" let tempFile = Path.Combine(path, file) let _, errors = checker.GetProjectOptionsFromScript(tempFile, SourceText.ofString scriptSource, assumeDotNetFramework = assumeNetFx, useSdkRefs = useSdk, otherFlags = flags) @@ -37,6 +38,32 @@ let ``can generate options for different frameworks regardless of execution envi | [] -> () | errors -> failwithf "Error while parsing script with assumeDotNetFramework:%b, useSdkRefs:%b, and otherFlags:%A:\n%A" assumeNetFx useSdk flags errors +#if NETFRAMEWORK +// See https://github.com/dotnet/fsharp/pull/13994#issuecomment-1259663865 +// +// .NET Core-based tooling can't resolve nuget packages to .NET Framework references +[] +#endif +[] +[] +let ``can resolve nuget packages to right target framework for different frameworks regardless of execution environment``(assumeNetFx, useSdk, flags) = + let path = Path.GetTempPath() + let file = tryCreateTemporaryFileName () + ".fsx" + let tempFile = Path.Combine(path, file) + let scriptSource = """ +#r "nuget: FSharp.Data, 3.3.3" +open System +let pi = Math.PI +""" + let options, errors = + checker.GetProjectOptionsFromScript(tempFile, SourceText.ofString scriptSource, assumeDotNetFramework = assumeNetFx, useSdkRefs = useSdk, otherFlags = flags) + |> Async.RunImmediate + match errors with + | [] -> () + | errors -> failwithf "Error while parsing script with assumeDotNetFramework:%b, useSdkRefs:%b, and otherFlags:%A:\n%A" assumeNetFx useSdk flags errors + let expectedReferenceText = (if assumeNetFx then "net45" else "netstandard2.0") + let found = options.OtherOptions |> Array.exists (fun s -> s.Contains(expectedReferenceText) && s.Contains("FSharp.Data.dll")) + Assert.IsTrue(found) // This test atempts to use a bad SDK number 666.666.666 //