Check for dotnet.dll when enumerating SDK paths

src/installer/tests/HostActivation.Tests/NativeHostApis.cs

@@ -54,7 +54,6 @@ public void Breadcrumb_thread_does_not_finish_when_app_has_unhandled_exception()
 
         private class SdkResolutionFixture
         {
-            private readonly string _builtDotnet;
             private readonly TestProjectFixture _fixture;
 
             public DotNetCli Dotnet { get; }
@@ -87,8 +86,7 @@ private class SdkResolutionFixture
 
             public SdkResolutionFixture(SharedTestState state)
             {
-                _builtDotnet = Path.Combine(TestArtifact.TestArtifactsPath, "sharedFrameworkPublish");
-                Dotnet = new DotNetCli(_builtDotnet);
+                Dotnet = new DotNetCli(RepoDirectoriesProvider.Default.BuiltDotnet);
 
                 _fixture = state.HostApiInvokerAppFixture.Copy();
 
@@ -100,17 +98,20 @@ public SdkResolutionFixture(SharedTestState state)
 
                 foreach (string sdk in ProgramFilesGlobalSdks)
                 {
-                    Directory.CreateDirectory(Path.Combine(ProgramFilesGlobalSdkDir, sdk));
+                    AddSdkDirectory(ProgramFilesGlobalSdkDir, sdk);
                 }
                 foreach (string sdk in SelfRegisteredGlobalSdks)
                 {
-                    Directory.CreateDirectory(Path.Combine(SelfRegisteredGlobalSdkDir, sdk));
+                    AddSdkDirectory(SelfRegisteredGlobalSdkDir, sdk);
                 }
                 foreach (string sdk in LocalSdks)
                 {
-                    Directory.CreateDirectory(Path.Combine(LocalSdkDir, sdk));
+                    AddSdkDirectory(LocalSdkDir, sdk);
                 }
 
+                // Empty SDK directory - this should not be recognized as a valid SDK directory
+                Directory.CreateDirectory(Path.Combine(LocalSdkDir, "9.9.9"));
+
                 foreach ((string fwName, string[] fwVersions) in ProgramFilesGlobalFrameworks)
                 {
                     foreach (string fwVersion in fwVersions)
@@ -121,6 +122,13 @@ public SdkResolutionFixture(SharedTestState state)
                     foreach (string fwVersion in fwVersions)
                         Directory.CreateDirectory(Path.Combine(LocalFrameworksDir, fwName, fwVersion));
                 }
+
+                static void AddSdkDirectory(string sdkDir, string version)
+                {
+                    string versionDir = Path.Combine(sdkDir, version);
+                    Directory.CreateDirectory(versionDir);
+                    File.WriteAllText(Path.Combine(versionDir, "dotnet.dll"), string.Empty);
+                }
             }
         }
 

src/installer/tests/HostActivation.Tests/SDKLookup.cs

@@ -97,12 +97,16 @@ public void SdkLookup_Global_Json_Single_Digit_Patch_Rollup()
             // Add SDK versions
             AddAvailableSdkVersions("9999.3.600");
 
+            // Add empty SDK version that is an exact match - should not be used
+            Directory.CreateDirectory(Path.Combine(ExecutableDotNet.BinPath, "sdk", "9999.3.4-global-dummy"));
+
             // Specified SDK version: 9999.3.4-global-dummy
-            // Exe: 9999.4.1, 9999.3.4-dummy, 9999.3.3, 9999.3.4, 9999.3.5-dummy, 9999.3.600
+            // Exe: 9999.4.1, 9999.3.4-dummy, 9999.3.3, 9999.3.4, 9999.3.5-dummy, 9999.3.600, 9999.3.4-global.dummy (empty)
             // Expected: 9999.3.5-dummy from exe dir
             RunTest()
                 .Should().Pass()
-                .And.HaveStdErrContaining(ExpectedResolvedSdkOutput("9999.3.5-dummy"));
+                .And.HaveStdErrContaining(ExpectedResolvedSdkOutput("9999.3.5-dummy"))
+                .And.HaveStdErrContaining("Ignoring version [9999.3.4-global-dummy] without dotnet.dll");
 
             // Add SDK versions
             AddAvailableSdkVersions("9999.3.4-global-dummy");
@@ -313,13 +317,17 @@ public void SdkLookup_Must_Pick_The_Highest_Semantic_Version()
             // Add SDK versions
             AddAvailableSdkVersions("9999.0.52000000");
 
+            // Add empty SDK version that is higher than any available version - should not be used
+            Directory.CreateDirectory(Path.Combine(ExecutableDotNet.BinPath, "sdk", "9999.1.0"));
+
             // Specified SDK version: none
             // Cwd: 10000.0.0                 --> should not be picked
             // Exe: 9999.0.0, 9999.0.3-dummy.9, 9999.0.3-dummy.10, 9999.0.3, 9999.0.100, 9999.0.80, 9999.0.5500000, 9999.0.52000000
             // Expected: 9999.0.52000000 from exe dir
             RunTest()
                 .Should().Pass()
-                .And.HaveStdErrContaining(ExpectedResolvedSdkOutput("9999.0.52000000"));
+                .And.HaveStdErrContaining(ExpectedResolvedSdkOutput("9999.0.52000000"))
+                .And.HaveStdErrContaining("Ignoring version [9999.1.0] without dotnet.dll");
 
             // Verify we have the expected SDK versions
             RunTest("--list-sdks")
@@ -331,7 +339,8 @@ public void SdkLookup_Must_Pick_The_Highest_Semantic_Version()
                 .And.HaveStdOutContaining("9999.0.100")
                 .And.HaveStdOutContaining("9999.0.80")
                 .And.HaveStdOutContaining("9999.0.5500000")
-                .And.HaveStdOutContaining("9999.0.52000000");
+                .And.HaveStdOutContaining("9999.0.52000000")
+                .And.NotHaveStdOutContaining("9999.1.0");
         }
 
         [Theory]

src/native/corehost/fxr/fx_muxer.cpp

@@ -1070,13 +1070,8 @@ int fx_muxer_t::handle_cli(
         return StatusCode::LibHostSdkFindFailure;
     }
 
-    append_path(&sdk_dotnet, _X("dotnet.dll"));
-
-    if (!pal::file_exists(sdk_dotnet))
-    {
-        trace::error(_X("Found .NET SDK, but did not find dotnet.dll at [%s]"), sdk_dotnet.c_str());
-        return StatusCode::LibHostSdkFindFailure;
-    }
+    append_path(&sdk_dotnet, SDK_DOTNET_DLL);
+    assert(pal::file_exists(sdk_dotnet));
 
     // Transform dotnet [command] [args] -> dotnet dotnet.dll [command] [args]
 

src/native/corehost/fxr/sdk_info.cpp

@@ -40,6 +40,39 @@ bool compare_by_version_ascending_then_hive_depth_descending(const sdk_info &a,
     return false;
 }
 
+void sdk_info::enumerate_sdk_paths(
+    const pal::string_t& sdk_dir,
+    std::function<bool(const fx_ver_t&, const pal::string_t&)> should_skip_version,
+    std::function<void(const fx_ver_t&, const pal::string_t&, const pal::string_t&)> callback)
+{
+    std::vector<pal::string_t> versions;
+    pal::readdir_onlydirectories(sdk_dir, &versions);
+    for (const pal::string_t& version_str : versions)
+    {
+        // Make sure we filter out any non-version folders.
+        fx_ver_t version;
+        if (!fx_ver_t::parse(version_str, &version, false))
+        {
+            trace::verbose(_X("Ignoring invalid version [%s]"), version_str.c_str());
+            continue;
+        }
+
+        if (should_skip_version(version, version_str))
+            continue;
+
+        // Check for the existence of dotnet.dll
+        pal::string_t sdk_version_dir = sdk_dir;
+        append_path(&sdk_version_dir, version_str.c_str());
+        if (!library_exists_in_dir(sdk_version_dir, SDK_DOTNET_DLL, nullptr))
+        {
+            trace::verbose(_X("Ignoring version [%s] without ") SDK_DOTNET_DLL, version_str.c_str());
+            continue;
+        }
+
+        callback(version, version_str, sdk_version_dir);
+    }
+}
+
 void sdk_info::get_all_sdk_infos(
     const pal::string_t& own_dir,
     std::vector<sdk_info>* sdk_infos)
@@ -51,32 +84,18 @@ void sdk_info::get_all_sdk_infos(
 
     for (pal::string_t dir : hive_dir)
     {
-        auto base_dir = dir;
-        trace::verbose(_X("Gathering SDK locations in [%s]"), base_dir.c_str());
-
-        append_path(&base_dir, _X("sdk"));
-
-        if (pal::directory_exists(base_dir))
-        {
-            std::vector<pal::string_t> versions;
-            pal::readdir_onlydirectories(base_dir, &versions);
-            for (const auto& ver : versions)
+        trace::verbose(_X("Gathering SDK locations in [%s]"), dir.c_str());
+        append_path(&dir, _X("sdk"));
+        enumerate_sdk_paths(
+            dir,
+            [](const fx_ver_t&, const pal::string_t&) { return false; },
+            [&](const fx_ver_t& version, const pal::string_t& version_str, const pal::string_t& full_path)
             {
-                // Make sure we filter out any non-version folders.
-                fx_ver_t parsed;
-                if (fx_ver_t::parse(ver, &parsed, false))
-                {
-                    trace::verbose(_X("Found SDK version [%s]"), ver.c_str());
-
-                    auto full_dir = base_dir;
-                    append_path(&full_dir, ver.c_str());
-
-                    sdk_info info(base_dir, full_dir, parsed, hive_depth);
-
-                    sdk_infos->push_back(info);
-                }
+                trace::verbose(_X("Found SDK version [%s]"), version_str.c_str());
+                sdk_info info(dir, full_path, version, hive_depth);
+                sdk_infos->push_back(info);
             }
-        }
+        );
 
         hive_depth++;
     }

src/native/corehost/fxr/sdk_info.h

@@ -6,6 +6,7 @@
 
 #include "pal.h"
 #include "fx_ver.h"
+#include <functional>
 
 struct sdk_info
 {
@@ -15,6 +16,11 @@ struct sdk_info
         , version(version)
         , hive_depth(hive_depth) { }
 
+    static void enumerate_sdk_paths(
+        const pal::string_t& sdk_dir,
+        std::function<bool(const fx_ver_t&, const pal::string_t&)> should_skip_version,
+        std::function<void(const fx_ver_t&, const pal::string_t&, const pal::string_t&)> callback);
+
     static void get_all_sdk_infos(
         const pal::string_t& own_dir,
         std::vector<sdk_info>* sdk_infos);

src/native/corehost/fxr/sdk_resolver.cpp

@@ -464,15 +464,21 @@ bool sdk_resolver::resolve_sdk_path_and_version(const pal::string_t& dir, pal::s
         auto probe_path = dir;
         append_path(&probe_path, requested_version.as_str().c_str());
 
-        if (pal::directory_exists(probe_path))
+        pal::string_t sdk_dll_maybe = probe_path;
+        append_path(&sdk_dll_maybe, SDK_DOTNET_DLL);
+        if (pal::file_exists(sdk_dll_maybe))
         {
-            trace::verbose(_X("Found requested SDK directory [%s]"), probe_path.c_str());
+            trace::verbose(_X("Found requested SDK [%s]"), probe_path.c_str());
             sdk_path = std::move(probe_path);
             resolved_version = requested_version;
 
             // The SDK path has been resolved
             return true;
         }
+        else if (trace::is_enabled() && pal::directory_exists(probe_path))
+        {
+            trace::verbose(_X("Ignoring version [%s] without ") SDK_DOTNET_DLL, requested_version.as_str().c_str());
+        }
     }
 
     if (roll_forward == sdk_roll_forward_policy::disable)
@@ -481,46 +487,42 @@ bool sdk_resolver::resolve_sdk_path_and_version(const pal::string_t& dir, pal::s
         return false;
     }
 
-    vector<pal::string_t> versions;
-    pal::readdir_onlydirectories(dir, &versions);
-
     bool changed = false;
     pal::string_t resolved_version_str = resolved_version.is_empty() ? pal::string_t{} : resolved_version.as_str();
-    for (auto&& version : versions)
-    {
-        fx_ver_t ver;
-        if (!fx_ver_t::parse(version, &ver, false))
+    sdk_info::enumerate_sdk_paths(
+        dir,
+        [&](const fx_ver_t& version, const pal::string_t& version_str)
         {
-            trace::verbose(_X("Ignoring invalid version [%s]"), version.c_str());
-            continue;
-        }
+            if (!matches_policy(version))
+            {
+                trace::verbose(_X("Ignoring version [%s] because it does not match the roll-forward policy"), version_str.c_str());
+                return true;
+            }
 
-        if (!matches_policy(ver))
-        {
-            trace::verbose(_X("Ignoring version [%s] because it does not match the roll-forward policy"), version.c_str());
-            continue;
-        }
+            if (!is_better_match(version, resolved_version))
+            {
+                trace::verbose(
+                    _X("Ignoring version [%s] because it is not a better match than [%s]"),
+                    version_str.c_str(),
+                    resolved_version_str.empty() ? _X("none") : resolved_version_str.c_str()
+                );
+                return true;
+            }
 
-        if (!is_better_match(ver, resolved_version))
+            return false;
+        },
+        [&](const fx_ver_t& version, const pal::string_t& version_str, const pal::string_t& full_path)
         {
             trace::verbose(
-                _X("Ignoring version [%s] because it is not a better match than [%s]"),
-                version.c_str(),
+                _X("Version [%s] is a better match than [%s]"),
+                version_str.c_str(),
                 resolved_version_str.empty() ? _X("none") : resolved_version_str.c_str()
             );
-            continue;
-        }
-
-        trace::verbose(
-            _X("Version [%s] is a better match than [%s]"),
-            version.c_str(),
-            resolved_version_str.empty() ? _X("none") : resolved_version_str.c_str()
-        );
 
-        changed = true;
-        resolved_version = ver;
-        resolved_version_str = std::move(version);
-    }
+            changed = true;
+            resolved_version = version;
+            resolved_version_str = std::move(version_str);
+        });
 
     if (changed)
     {

src/native/corehost/hostmisc/utils.h

@@ -45,6 +45,8 @@
 
 #define DOTNET_ROOT_ENV_VAR _X("DOTNET_ROOT")
 
+#define SDK_DOTNET_DLL _X("dotnet.dll")
+
 bool ends_with(const pal::string_t& value, const pal::string_t& suffix, bool match_case);
 bool starts_with(const pal::string_t& value, const pal::string_t& prefix, bool match_case);