SdkAssemblyResolver improvements (#2797)

* SdkAssemblyResolver: Try-find was taking the first version, but we want to collect all versions and evaluate the selection later. * Readme update ------------------------------------------------------ It still seems we have unreliable integration tests. I still blame the SdkAssemblyResolver not being able to pick proper .NET versions. There were some "try finds" which just took the first ever found version, and I think it should rather collect a list of versions to make the decision of the proper version later. Also added verbosity to intergration-test-log so we could better see what goes wrong, if something goes wrong.
fsprojects · Aug 30, 2024 · eeae957 · eeae957
1 parent 73dbe5b
commit eeae957
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 41 deletions.
diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml
@@ -25,7 +25,7 @@ jobs:
             - name: add fake runner as a tool
               run: dotnet tool install fake-cli --add-source "./src/app/fake-cli/bin/Debug" --version 1.0.0-1
             - name: Build
-              run: dotnet fake build --parallel 3
+              run: dotnet fake -v build --parallel 3 -e FAKE_DETAILED_ERRORS=true
             - name: publish build artifacts
               uses: actions/upload-artifact@v4
               with:

diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
@@ -1,5 +1,8 @@
 # Release Notes
 
+## 6.1.1 - 2024-08-19
+* BUGFIX: Assembly resolver to pick a found SDK instead of first SDK - https://github.com/fsprojects/FAKE/pull/2797/files
+
 ## 6.1.0 - 2024-07-27
 * BUGFIX: MSBuild.build adds a bad string at the end of properties, thanks @0x53A - https://github.com/fsprojects/FAKE/issues/2738
 * BUGFIX: Allow setting Msbuild max cpu on Linux, thanks @TheAngryByrd - https://github.com/fsprojects/FAKE/pull/2772

diff --git a/src/app/Fake.Runtime/SdkAssemblyResolver.fs b/src/app/Fake.Runtime/SdkAssemblyResolver.fs
@@ -45,12 +45,22 @@ type SdkAssemblyResolver(logLevel: Trace.VerboseLevel) =
     member this.SdkVersionRaws = RuntimeAssemblyVersions
 
     member this.SdkVersions =
-        RuntimeAssemblyVersions |> List.map (fun v -> ReleaseVersion(v + ".0"))
+        RuntimeAssemblyVersions
+        |> List.map (fun v ->
+            if String.IsNullOrEmpty v || v = "\"\"" then
+                ReleaseVersion "6.0.0"
+            elif v.Contains "." then
+                ReleaseVersion(v + ".0")
+            else
+                ReleaseVersion(v + ".0.0"))
 
     member this.PaketFrameworkIdentifiers =
         this.SdkVersions
         |> List.map (fun thisSdk ->
-            FrameworkIdentifier.DotNetFramework(FrameworkVersion.TryParse(thisSdk.Major.ToString()).Value))
+            match FrameworkVersion.TryParse(thisSdk.Major.ToString()) with
+            | Some v -> FrameworkIdentifier.DotNetFramework v
+            | None -> failwithf $"Paket: .NET not found: {thisSdk.Major.ToString()}")
+
 
     member this.SdkVersionRaw = RuntimeAssemblyVersions |> Seq.head
     member this.SdkVersion = this.SdkVersions |> Seq.head
@@ -143,21 +153,28 @@ type SdkAssemblyResolver(logLevel: Trace.VerboseLevel) =
         if this.LogLevel.PrintVerbose then
             Trace.tracefn "Trying to resolve runtime version from network.."
 
+        let sdkSet = this.SdkVersionRaws |> Set.ofList
+
         try
-            let sdkVersionReleases =
-                ProductCollection.GetAsync()
-                |> Async.AwaitTask
-                |> Async.RunSynchronously
-                |> List.ofSeq
-                |> List.find (fun product ->
-                    this.SdkVersionRaws
-                    |> List.exists (fun raws -> product.ProductVersion.Equals raws))
-
-            sdkVersionReleases.GetReleasesAsync()
+            task {
+                let! productCollection = ProductCollection.GetAsync()
+
+                let sdkVersionReleases =
+                    productCollection
+                    |> Seq.filter (fun product -> sdkSet |> Set.exists (fun raws -> product.ProductVersion.Equals raws))
+
+                let! releases =
+                    sdkVersionReleases
+                    |> Seq.map (fun rel -> rel.GetReleasesAsync())
+                    |> System.Threading.Tasks.Task.WhenAll
+
+                return releases |> Seq.collect (fun sdks -> sdks |> Seq.toList)
+            }
             |> Async.AwaitTask
             |> Async.RunSynchronously
-            |> List.ofSeq
+            |> Seq.toList
             |> Some
+
         with ex ->
             Trace.traceError $"Could not get SDK runtime version from network due to: {ex.Message}"
             None
@@ -179,7 +196,7 @@ type SdkAssemblyResolver(logLevel: Trace.VerboseLevel) =
             Trace.traceError $"Could not get SDK runtime version from cache due to: {ex.Message}"
             None
 
-    member this.GetProductReleaseForSdk(version: ReleaseVersion) =
+    member this.GetProductReleasesForSdk(version: ReleaseVersion) =
         let net60releases =
             if RuntimeResolverResolveMethod = "cache" then
                 // for testing only!
@@ -190,13 +207,16 @@ type SdkAssemblyResolver(logLevel: Trace.VerboseLevel) =
                 |> Option.orElseWith (this.TryResolveSdkRuntimeVersionFromCache)
 
         let sdkRelease (release: ProductRelease) =
-            release.Sdks
-            |> List.ofSeq
-            |> List.exists (fun sdk -> sdk.Version.Equals(version))
+            release.Sdks |> Seq.exists (fun sdk -> sdk.Version.Equals(version))
 
-        net60releases |> Option.bind (List.tryFind sdkRelease)
+        match net60releases with
+        | None -> []
+        | Some versions -> versions |> List.filter sdkRelease
 
-    member this.ResolveSdkRuntimeVersion() =
+    member this.GetProductReleaseForSdk(version: ReleaseVersion) =
+        this.GetProductReleasesForSdk version |> List.tryHead
+
+    member this.ResolveSdkRuntimeVersions() =
         let versionOptions (options: DotNet.VersionOptions) =
             // If a custom CLI path is provided, configure the version command
             // to use that path.  This really only accomodates a test scenarios
@@ -208,43 +228,58 @@ type SdkAssemblyResolver(logLevel: Trace.VerboseLevel) =
 
         let sdkVersion = DotNet.getVersion versionOptions |> ReleaseVersion
 
-        match this.GetProductReleaseForSdk sdkVersion with
-        | Some release ->
-            let version = release.Runtime.Version.ToString()
+        match this.GetProductReleasesForSdk sdkVersion with
+        | [] ->
+            failwithf
+                $"Could not find a suitable .NET 6 runtime version matching SDK version: {sdkVersion.ToString()} (You can also try setting environment variable FAKE_SDK_RESOLVER_CUSTOM_DOTNET_VERSION to e.g. {sdkVersion.Major.ToString()}.0 )"
+        | releases ->
+            let versions =
+                releases |> List.map (fun release -> release.Runtime.Version.ToString())
 
             if this.LogLevel.PrintVerbose then
-                Trace.trace $"Resolved runtime version: {version}"
+                versions
+                |> List.iter (fun version -> Trace.trace $"Resolved runtime version: {version}")
 
-            version
+            versions
 
-        | None ->
-            failwithf
-                $"Could not find a suitable .NET 6 runtime version matching SDK version: {sdkVersion.ToString()} (You can also try setting environment variable FAKE_SDK_RESOLVER_CUSTOM_DOTNET_VERSION to e.g. 8.0 )"
+    member this.ResolveSdkRuntimeVersion() =
+        this.ResolveSdkRuntimeVersions() |> Seq.head
 
     member this.SdkReferenceAssemblies() =
 
+        let runtimeVersions = this.ResolveSdkRuntimeVersions()
+
         let referenceAssembliesPaths =
-            this.SdkVersionRaws
-            |> List.choose (fun rawVersion ->
-                this.ResolveDotNetRoot()
-                |> Option.map (fun dotnetRoot ->
-                    dotnetRoot
-                    </> "packs"
-                    </> "Microsoft.NETCore.App.Ref"
-                    </> this.ResolveSdkRuntimeVersion()
-                    </> "ref"
-                    </> "net" + rawVersion))
+            runtimeVersions
+            |> List.map (fun runtimeVersion ->
+                this.SdkVersionRaws
+                |> List.choose (fun rawVersion ->
+                    this.ResolveDotNetRoot()
+                    |> Option.map (fun dotnetRoot ->
+                        dotnetRoot
+                        </> "packs"
+                        </> "Microsoft.NETCore.App.Ref"
+                        </> runtimeVersion
+                        </> "ref"
+                        </> "net" + rawVersion)))
 
         if Seq.isEmpty referenceAssembliesPaths then
             failwithf "Could not find referenced assemblies, please check installed SDK and runtime versions"
         else
+
             if this.LogLevel.PrintVerbose then
                 let paths = String.Join(",", referenceAssembliesPaths)
                 Trace.tracefn $"Resolved referenced SDK paths: {paths}"
 
-            let referenceAssembliesPath =
+            let existingReferenceAssembliesPaths =
                 referenceAssembliesPaths
-                |> List.tryFind (fun referenceAssembliesPath -> Directory.Exists referenceAssembliesPath)
+                |> List.collect (fun paths -> paths |> List.filter (fun path -> Directory.Exists path))
+
+            if this.LogLevel.PrintVerbose then
+                let paths = String.Join(",", existingReferenceAssembliesPaths)
+                Trace.tracefn $"Found referenced SDK paths: {paths}"
+
+            let referenceAssembliesPath = existingReferenceAssembliesPaths |> List.tryHead
 
             match referenceAssembliesPath with
             | Some pathFound -> Directory.GetFiles(pathFound, "*.dll") |> Seq.toList

diff --git a/src/test/Fake.DotNet.Cli.IntegrationTests/Main.fs b/src/test/Fake.DotNet.Cli.IntegrationTests/Main.fs
@@ -8,7 +8,7 @@ open System
 
 [<EntryPoint>]
 let main argv =
-    let config = defaultConfig |> ExpectoHelpers.addTimeout (TimeSpan.FromMinutes(20.))
+    let config = defaultConfig |> ExpectoHelpers.addTimeout (TimeSpan.FromMinutes(30.))
 
     Tests.runTestsInAssembly
         { config with