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

force roll forward when discovered .NET version is greater than FSAC TFM #1889

Merged
merged 2 commits into from
Jul 1, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 54 additions & 64 deletions src/Core/LanguageService.fs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ Consider:
}

/// runs `dotnet --version` in the current rootPath to determine the resolved sdk version from the global.json file.
let runtimeVersion () =
let sdkVersion () =
promise {
let! dotnet = tryFindDotnet ()

Expand Down Expand Up @@ -683,11 +683,11 @@ Consider:

if availableTFMs |> Seq.contains tfm then
printfn "TFM match found"
node.path.join (basePath, tfm, "fsautocomplete.dll")
tfm, node.path.join (basePath, tfm, "fsautocomplete.dll")
else
// find best-matching
let tfm = findBestTFM availableTFMs tfm
node.path.join (basePath, tfm, "fsautocomplete.dll")
tfm, node.path.join (basePath, tfm, "fsautocomplete.dll")

let isNetFolder (folder: string) =
printfn $"checking folder %s{folder}"
Expand All @@ -697,7 +697,8 @@ Consider:
&& let stat = node.fs.statSync (!!folder) in
stat.isDirectory ()

let fsacPathForTfm (tfm: string) =
/// locates the FSAC dll and TFM for that dll given a host TFM
let fsacPathForTfm (tfm: string): string * string =
match fsacNetcorePath with
| null
| "" ->
Expand All @@ -706,7 +707,8 @@ Consider:
probePathForTFMs binPath tfm
| userSpecified ->
if userSpecified.EndsWith ".dll" then
userSpecified
let tfm = node.path.basename(node.path.dirname userSpecified)
tfm, userSpecified
else
// if dir has tfm folders, probe
let filesAndFolders =
Expand All @@ -720,7 +722,8 @@ Consider:
probePathForTFMs userSpecified tfm
else
// no tfm paths, try to use `fsautocomplete.dll` from this directory
node.path.join (userSpecified, "fsautocomplete.dll")
let tfm = node.path.basename(node.path.dirname userSpecified)
tfm, node.path.join (userSpecified, "fsautocomplete.dll")

let tfmForSdkVersion (v: SemVer) =
match int v.major, int v.minor with
Expand All @@ -732,64 +735,51 @@ Consider:

let discoverDotnetArgs () =
promise {
let! (rollForwardArgs, necessaryEnvVariables, fsacPath) =
promise {
let! sdkVersionAtRootPath = runtimeVersion ()

match sdkVersionAtRootPath with
| Error e ->
printfn $"FSAC (NETCORE): {e}"
return [], [], ""
| Ok v ->
printfn "Parsed SDK version at root path: %s" v.raw
let tfm = tfmForSdkVersion v
printfn "Parsed SDK version to tfm: %s" tfm
let fsacPath = fsacPathForTfm tfm
printfn "Parsed TFM to fsac path: %s" fsacPath
return [], [], fsacPath
// if v.major >= 6.0 then
// // when we run on a sdk higher than 6.x (aka what FSAC is currently built/targeted for),
// // we have to tell the runtime to allow it to actually run on that runtime (instead of presenting 6.x as 5.x)
// // in order for msbuild resolution to work
// let args = [ "--roll-forward"; "LatestMajor" ]

// let envs =
// if v.prerelease <> null || v.prerelease.Count > 0 then
// [ "DOTNET_ROLL_FORWARD_TO_PRERELEASE", box 1 ]
// else
// []

// return args, envs, fsacPath
// else
// return [], [], fsacPath
}

let userDotnetArgs = "FSharp.fsac.dotnetArgs" |> Configuration.get [||]

let hasUserRollForward =
userDotnetArgs
|> Array.tryFindIndex (fun a -> a = "--roll-forward")
|> Option.map (fun _ -> true)
|> Option.defaultValue false

let hasUserFxVersion =
userDotnetArgs
|> Array.tryFindIndex (fun a -> a = "--fx-version")
|> Option.map (fun _ -> true)
|> Option.defaultValue false

let shouldApplyImplicitRollForward = not (hasUserFxVersion || hasUserRollForward)

let args =
[ if shouldApplyImplicitRollForward then
yield! rollForwardArgs
yield! userDotnetArgs ]

let envVariables =
[ if shouldApplyImplicitRollForward then
yield! necessaryEnvVariables ]

return args, envVariables, fsacPath
let! sdkVersionAtRootPath = sdkVersion ()

match sdkVersionAtRootPath with
| Error e ->
printfn $"Error finding dotnet version: {e}"
return failwith "Error finding dotnet version, do you have dotnet installed and on the PATH?"
| Ok sdkVersion ->
printfn "Parsed SDK version at root path: %s" sdkVersion.raw
let sdkTfm = tfmForSdkVersion sdkVersion
printfn "Parsed SDK version to tfm: %s" sdkTfm
let fsacTfm, fsacPath = fsacPathForTfm sdkTfm
printfn "Parsed TFM to fsac path: %s" fsacPath

let userDotnetArgs = "FSharp.fsac.dotnetArgs" |> Configuration.get [||]

let hasUserRollForward =
userDotnetArgs
|> Array.tryFindIndex (fun a -> a = "--roll-forward")
|> Option.map (fun _ -> true)
|> Option.defaultValue false

let hasUserFxVersion =
userDotnetArgs
|> Array.tryFindIndex (fun a -> a = "--fx-version")
|> Option.map (fun _ -> true)
|> Option.defaultValue false

let shouldApplyImplicitRollForward =
not (hasUserFxVersion || hasUserRollForward)
&& sdkTfm <> fsacTfm // if the SDK doesn't match one of our FSAC TFMs, then we're in compat mode
Copy link
Contributor Author

Choose a reason for hiding this comment

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

core of the change is here - if the derived 'default' TFM of the SDK in use is not the TFM of the version of FSAC that we just chose (e.g. net8.0 on a 8.0.100-preview SDK with a net7.0 TFM FSAC binary), then we must apply rollforward flags to force the runtime to choose 8.0 bits. otherwise on a system with 7.0 bits installed we'll use those.


let args =
userDotnetArgs

let envVariables =
[ if shouldApplyImplicitRollForward then
"DOTNET_ROLL_FORWARD", box "LatestMajor"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

pertinent change 1

match sdkVersion.prerelease with
| null -> ()
| pres when pres.Count > 0 ->
"DOTNET_ROLL_FORWARD_TO_PRERELEASE", box "true"
| _ -> () ]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if the sdk in use is a preview we must apply another flag to let the runtime float forward to those previews


return args, envVariables, fsacPath
}

let spawnNetCore dotnet : JS.Promise<Executable> =
Expand All @@ -803,7 +793,7 @@ Consider:
if parallelReferenceResolution then
yield "FCS_ParallelReferenceResolution", box "true" ]

printfn $"FSAC (NETCORE): '%s{fsacPath}'"
printfn $"""FSAC (NETCORE): '%s{fsacPath}'"""

let exeOpts = createEmpty<ExecutableOptions>

Expand Down