10

Update Proj-Info and solidify .net 6 support by baronfel · Pull Request #825 · f...

 2 years ago
source link: https://github.com/fsharp/FsAutoComplete/pull/825
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client

Conversation

Reviewers

No reviews

Assignees

No one assigned

Labels
None yet
Projects

None yet

Milestone

No milestone

Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants

Copy link

Contributor

baronfel commented on Aug 12

No description provided.

@@ -125,11 +125,27 @@ type BackgroundServiceServer(state: State, client: FsacClient) =

let mutable latestRuntimeVersion = lazy None

//TODO: does the backgroundservice ever get config updates?

do

let allowedVersionRange =

let maxVersion = System.Environment.Version.Major + 1

Copy link

Contributor

Author

@baronfel baronfel on Aug 12

edited

this version range will be 5.something on .net 5 and 6.something on .net 6, so we should ensure that the SDK/Runtime we load for FSX is within that bound.

This still floats to the highest in that bracket, but we can very easily allow some user customization here and just change the way we create the Range.

@@ -186,8 +186,11 @@ type FSharpCompilerServiceChecker(backgroundServiceEnabled, hasAnalyzers) =

logQueueLength optsLogger (Log.setMessage "Getting NetCore options for script file {file}" >> Log.addContextDestructured "file" file)

let allFlags = Array.append [| "--targetprofile:netstandard" |] fsiAdditionalArguments

let! (opts, errors) = checker.GetProjectOptionsFromScript(UMX.untag file, source, assumeDotNetFramework = false, useSdkRefs = true, useFsiAuxLib = true, otherFlags = allFlags, userOpName = "getNetCoreScriptOptions")

optsLogger.trace (Log.setMessage "Got NetCore options {opts} for file {file} with errors {errors}" >> Log.addContextDestructured "file" file >> Log.addContextDestructured "opts" opts >> Log.addContextDestructured "errors" errors)

Copy link

Contributor

Author

@baronfel baronfel on Aug 12

might revert this now that I've debugged enough to have confidence in this code path again

let allModifications = replaceFrameworkRefs >> addLoadedFiles >> resolveRelativeFilePaths

return allModifications opts, errors

let modified = allModifications opts

optsLogger.trace (Log.setMessage "Replaced options to {opts}" >> Log.addContextDestructured "opts" opts)

Copy link

Contributor

Author

@baronfel baronfel on Aug 12

same here

@@ -304,8 +307,24 @@ type FSharpCompilerServiceChecker(backgroundServiceEnabled, hasAnalyzers) =

then ()

else

sdkRoot <- Some directory

sdkVersion <- Environment.latest3xSdkVersion directory

runtimeVersion <- Environment.latest3xRuntimeVersion directory

//TODO(CH): should this range be user-assignable somehow?

Copy link

Contributor

Author

@baronfel baronfel on Aug 12

similar point as in the background services: how should this be user-assignable?

@@ -24,14 +24,12 @@ let testTimeout =

let loaders = [

"Ionide WorkspaceLoader", WorkspaceLoader.Create

// These are commented out because of https://github.com/ionide/proj-info/issues/109

// "MSBuild Project Graph WorkspaceLoader", WorkspaceLoaderViaProjectGraph.Create

"MSBuild Project Graph WorkspaceLoader", WorkspaceLoaderViaProjectGraph.Create

Copy link

Contributor

Author

@baronfel baronfel on Aug 12

graph loader works in the new proj-info, so we can reenable it

Copy link

Contributor

Author

baronfel commented on Aug 18

Test hangs are due to a typeloadexception in the msbuild dlls. We're hitting a corner case in the test suite for some reason that not caught in a try-catch inside the loader loop in projinfo (which we should probably address in that library).

"Could not load type 'Microsoft.Build.Framework.IMetadataContainer' from assembly 'Microsoft.Build.Framework, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'."

problem is:

  • we've got msbuild libs versions 16.x in this repo due to FCS at minimum (so we're bound to those?)
  • new projinfo version loads msbuild dlls from the sdk directory by setting an assemblyresolve handler (hell, old one did too so wtf is happening)
  • sdk msbuild dlls are version 15.1.x for sdk version 5.0.3xx
  • insert version mismatch here

Weirdly, the 6.0.1xx previews have the same assemblyversion.

Copy link

Contributor

Author

baronfel commented 3 days ago

edited

Current status on this is that it works for .net 6 FSI scripts, but project loading has some kind of a hang - even on .net 5 now we start loading the project but it never completes. We need some additional logging/tracing here to have insight to what's happening in proj-info.

Once that's resolved things should start lighting back up, I expect tests to be as green as they were before this change. The next hurdle will be .net 6 testing. We may initially do manual testing on .net 6 SDKs just to get it out of the door, but we'll need .net capable FAKE runners or a pivot to .net 6 build projects in order to run the tests in a .net 6 target to keep that verification active in the future.

Copy link

Contributor

Author

baronfel commented 2 days ago

@Krzysztof-Cieslak did some additional debugging and for some reason (for local usage) project cracking is loading an 15.x msbuild dll, which of course isn't resolvable in the SDK directory we're pinned to because those are 16.x. Where does this come from? no one knows.

FSharpLint and FCS itself have 16.x msbuild dependencies, but this 15.x dependency can't be found easily by either of us. Might need to dip into binlogs to trace where it's coming from.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment


About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK