Fix #2342 and add tests to make sure we have good error messages.

matthid · matthid · commit 481ebeedddb6 · 2019-06-30T15:31:01.000+02:00
diff --git a/integrationtests/i002007-native-libs/before/build.fsx b/integrationtests/i002007-native-libs/before/build.fsx
@@ -12,12 +12,31 @@ let openConn (path:string) =
     let conn = new SQLiteConnection(builder.ToString())
     conn.OpenAndReturn()
 
+module Imports =
+    open System.Runtime.InteropServices
+    [<DllImport("kernel32.dll")>]
+    extern uint32 GetCurrentProcessId()
+    [<DllImport("unknown_dependency.dll")>]
+    extern uint32 UnknownFunctionInDll()
+    [<DllImport("SQLite.Interop.dll")>]
+    extern uint32 Fake_ShouldNotExistExtryPoint()
+
 // Default target
 Target.create "Default" (fun _ ->
   Trace.trace "Hello World from FAKE"
+  if Environment.isWindows then
+    // #2342: make sure defaults PATH dependencies still work, see https://github.com/fsharp/FAKE/issues/2342
+    printfn "Current process: %d" (Imports.GetCurrentProcessId())
+  
   use conn = openConn "temp.db"
   ()
 )
+Target.create "FailWithUnknown" (fun _ ->
+  printfn "UnknownFunctionInDll: %d" (Imports.UnknownFunctionInDll())
+)
+Target.create "FailWithMissingEntry" (fun _ ->
+  printfn "Fake_ShouldNotExistExtryPoint: %d" (Imports.Fake_ShouldNotExistExtryPoint())
+)
 
 // start build
 Target.runOrDefault "Default"
diff --git a/src/app/Fake.Runtime/CoreCache.fs b/src/app/Fake.Runtime/CoreCache.fs
@@ -331,7 +331,7 @@ This can happen for various reasons:
                     tracefn "Redirect assembly load to previously loaded assembly: %A" loadedName         
         result
 
-let findUnmanagedInRuntimeDeps (loadFromPath:string -> nativeint) (unmanagedDllName:string) (logLevel:Trace.VerboseLevel) (nativeLibraries:NativeLibrary list) =
+let findUnmanagedInRuntimeDeps (unmanagedDllName:string) (logLevel:Trace.VerboseLevel) (nativeLibraries:NativeLibrary list) =
     if logLevel.PrintVerbose then tracefn "Trying to resolve native library: %s" unmanagedDllName
     match nativeLibraries |> Seq.tryFind (fun l ->
             let fnExt = Path.GetFileName l.File
@@ -343,7 +343,7 @@ let findUnmanagedInRuntimeDeps (loadFromPath:string -> nativeint) (unmanagedDllN
         path
     | None ->
         // will failwithf later anyway.
-        if logLevel.PrintVerbose then tracefn "Could not resolve native library '%s'!" unmanagedDllName
+        if logLevel.PrintVerbose then tracefn "Could not resolve native library '%s', fallback to CLR-host!" unmanagedDllName
         null
 
 let resolveUnmanagedDependencyCached =
@@ -352,17 +352,19 @@ let resolveUnmanagedDependencyCached =
         let mutable wasCalled = false
         let path = libCache.GetOrAdd(unmanagedDllName, (fun _ ->
             wasCalled <- true
-            findUnmanagedInRuntimeDeps loadFromPath unmanagedDllName logLevel nativeLibraries))
-        
-        if wasCalled && isNull path then
-            let available =
-                if nativeLibraries.Length > 0 then
-                    "Available native libraries:\n - " + String.Join("\n - ", nativeLibraries |> Seq.map (fun n -> n.File))
-                else "No native libraries found!"
-            failwithf """Could not resolve native library '%s'.
+            findUnmanagedInRuntimeDeps unmanagedDllName logLevel nativeLibraries))
+
+        //if wasCalled && not wasFound then
+            //let available =
+            //    if nativeLibraries.Length > 0 then
+            //        "Available native libraries:\n - " + String.Join("\n - ", nativeLibraries |> Seq.map (fun n -> n.File))
+            //    else "No native libraries found!"
+            // TODO: In the future use the unmanaged assembly resolve event to throw this.            
+            (*failwithf """Could not resolve native library '%s'.
 %s
 
 This can happen for various reasons:
+- The file '%s' could not be found in the PATH environment variable.
 - The nuget cache (or packages folder) might be broken.
   -> Please save your state, open an issue and then
   - delete the source package of '%s' from the '~/.nuget' cache (and the 'packages' folder)
@@ -371,11 +373,12 @@ This can happen for various reasons:
   - try running fake again
   - the package should be downloaded again
 
--> If the above doesn't apply or you need help please open an issue!""" unmanagedDllName available unmanagedDllName
+-> If the above doesn't apply or you need help please open an issue!""" unmanagedDllName available unmanagedDllName unmanagedDllName*)
         if not wasCalled && not (isNull path) then
             if logLevel.PrintVerbose then
                 tracefn "Redirect native library '%s' to previous loaded library '%s'" unmanagedDllName path
-        loadFromPath path
+        // Zero means use CLR-Host strategy, see https://github.com/fsharp/FAKE/issues/2342
+        if isNull path then IntPtr.Zero else loadFromPath path
 
 #if NETSTANDARD1_6
 // See https://github.com/dotnet/coreclr/issues/6411
diff --git a/src/test/Fake.Core.IntegrationTests/SimpleHelloWorldTests.fs b/src/test/Fake.Core.IntegrationTests/SimpleHelloWorldTests.fs
@@ -48,29 +48,27 @@ let tests =
         stdErr.Trim() |> Expect.equal "empty exected" ""
 
     testCase "simple failed to compile" <| fun _ ->
-        try
-            fakeRunAndCheck "fail-to-compile.fsx" "fail-to-compile.fsx" "core-simple-failed-to-compile" |> ignProc
-            fail "Expected an compilation error and a nonzero exit code!"
-        with 
-        | FakeExecutionFailed(result) ->
-            let stdOut = String.Join("\n", result.Messages)
-            let stdErr = String.Join("\n", result.Errors)
-            stdErr.Contains("klajsdhgfasjkhd")
-                |> Expect.isTrue (sprintf "Standard Error Output should contain 'klajsdhgfasjkhd', but was: '%s', Out: '%s'" stdErr stdOut)
+        let result =
+            expectFailure "Expected an compilation error and a nonzero exit code!" (fun _ ->
+                fakeRunAndCheck "fail-to-compile.fsx" "fail-to-compile.fsx" "core-simple-failed-to-compile" |> ignProc)
+
+        let stdOut = String.Join("\n", result.Messages)
+        let stdErr = String.Join("\n", result.Errors)
+        stdErr.Contains("klajsdhgfasjkhd")
+            |> Expect.isTrue (sprintf "Standard Error Output should contain 'klajsdhgfasjkhd', but was: '%s', Out: '%s'" stdErr stdOut)
 
-            checkIntellisense "fail-to-compile.fsx" "core-simple-failed-to-compile"
+        checkIntellisense "fail-to-compile.fsx" "core-simple-failed-to-compile"
 
     testCase "simple runtime error" <| fun _ ->
-        try
-            fakeRunAndCheck "runtime-error.fsx" "runtime-error.fsx" "core-simple-runtime-error" |> ignProc
-            fail "Expected an runtime error and a nonzero exit code!"
-        with
-        | FakeExecutionFailed(result) ->
-            let stdOut = String.Join("\n", result.Messages)
-            let stdErr = String.Join("\n", result.Errors)
-            stdErr.Contains("runtime error")
-                |> Expect.isTrue (sprintf "Standard Error Output should contain 'runtime error', but was: '%s', Out: '%s'" stdErr stdOut)
-            checkIntellisense "runtime-error.fsx" "core-simple-runtime-error"
+        let result =
+            expectFailure "Expected an runtime error and a nonzero exit code!" (fun _ ->
+                fakeRunAndCheck "runtime-error.fsx" "runtime-error.fsx" "core-simple-runtime-error" |> ignProc)
+
+        let stdOut = String.Join("\n", result.Messages)
+        let stdErr = String.Join("\n", result.Errors)
+        stdErr.Contains("runtime error")
+            |> Expect.isTrue (sprintf "Standard Error Output should contain 'runtime error', but was: '%s', Out: '%s'" stdErr stdOut)
+        checkIntellisense "runtime-error.fsx" "core-simple-runtime-error"
 
     testCase "reference fake runtime" <| fun _ ->
         handleAndFormat <| fun () ->
@@ -155,6 +153,30 @@ let tests =
             fakeRunAndCheckInPath "build.fsx" "build.fsx" "i002025" "script" |> ignProc
 
     testCase "issue #2007 - native libs work" <| fun _ ->
+        // should "just" work
         handleAndFormat <| fun () ->
-            fakeRunAndCheck "build.fsx" "build.fsx" "i002007-native-libs" |> ignore
+            fakeRunAndCheck "build.fsx" "build.fsx" "i002007-native-libs" |> ignProc
+
+        // Should tell FAKE error story
+        let result =     
+            expectFailure "Expected missing entrypoint error" <| fun () ->
+                directFake (sprintf "run build.fsx -t FailWithMissingEntry") "i002007-native-libs" |> ignProc
+        let stdOut = String.Join("\n", result.Messages).Trim()
+        let stdErr = String.Join("\n", result.Errors)
+        (stdErr.Contains "Fake_ShouldNotExistExtryPoint" && stdErr.Contains "EntryPointNotFoundException:")
+            |> Expect.isTrue (sprintf "Standard Error Output should contain 'Fake_ShouldNotExistExtryPoint' and 'EntryPointNotFoundException:', but was: '%s', Out: '%s'" stdErr stdOut)
+        
+        let result =     
+            expectFailure "Expected missing entrypoint error" <| fun () ->
+                directFake (sprintf "run build.fsx -t FailWithUnknown") "i002007-native-libs" |> ignProc
+        let stdOut = String.Join("\n", result.Messages).Trim()
+        let stdErr = String.Join("\n", result.Errors)
+        (stdErr.Contains "unknown_dependency.dll" && stdErr.Contains "DllNotFoundException:")
+            |> Expect.isTrue (sprintf "Standard Error Output should contain 'unknown_dependency.dll' and 'DllNotFoundException:', but was: '%s', Out: '%s'" stdErr stdOut)
+        // TODO: enable instead of the above
+        //stdErr.Contains("Could not resolve native library 'unknown_dependency.dll'")
+        //    |> Expect.isTrue (sprintf "Standard Error Output should contain \"Could not resolve native library 'unknown_dependency.dll'\", but was: '%s', Out: '%s'" stdErr stdOut)
+        //stdErr.Contains("This can happen for various reasons")
+        //    |> Expect.isTrue (sprintf "Standard Error Output should contain \"This can happen for various reasons\", but was: '%s', Out: '%s'" stdErr stdOut)
+
   ]
diff --git a/src/test/Fake.Core.IntegrationTests/TestHelpers.fs b/src/test/Fake.Core.IntegrationTests/TestHelpers.fs
@@ -82,6 +82,16 @@ let handleAndFormat f =
         let stdErr = String.Join("\n", result.Errors)
         Expect.isTrue (sprintf "fake.exe failed with code %d\nOut: %s\nError: %s" result.ExitCode stdOut stdErr) false
         reraise() // for return value
+
+let expectFailure msg f =
+    try
+        f()
+        Expect.isTrue msg false
+        failwithf "%s" msg
+    with FakeExecutionFailed(result) ->
+        result
+
+
 let directFake command scenario =
     directFakeInPath command (scenarioTempPath scenario) null
 
