Skip to content

Commit

Permalink
Check for <framework_name>.deps.json when enumerating framework paths (…
Browse files Browse the repository at this point in the history 
…#92032)

This adds a check for the framework's .deps.json instead of just the existence of the directory. To avoid extra file checks in the normal/happy path (where all framework version folder are valid), when resolving, it only does the check after resolving the best version match. And if that version directory isn't valid, it tries resolving again without it.

Backport of #90035
  • Loading branch information
@elinor-fung
elinor-fung committed Sep 18, 2023
1 parent 9036031 commit c2f1e7e
Show file tree
Hide file tree
Showing 9 changed files with 144 additions and 82 deletions.
16 changes: 13 additions & 3 deletions src/installer/tests/HostActivation.Tests/FrameworkResolution/MultipleHives.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using Microsoft.DotNet.Cli.Build;
using Microsoft.DotNet.Cli.Build.Framework;
using System;
using System.IO;
using System.Runtime.InteropServices;
using Xunit;

Expand All @@ -27,7 +28,8 @@ public void FrameworkHiveSelection_GlobalHiveWithBetterMatch()
RunTest(
runtimeConfig => runtimeConfig
.WithFramework(MicrosoftNETCoreApp, "5.0.0"))
.ShouldHaveResolvedFramework(MicrosoftNETCoreApp, "5.1.2");
.ShouldHaveResolvedFramework(MicrosoftNETCoreApp, "5.1.2")
.And.HaveStdErrContaining($"Ignoring FX version [5.0.0] without .deps.json");
}

[Fact]
Expand All @@ -37,7 +39,8 @@ public void FrameworkHiveSelection_MainHiveWithBetterMatch()
RunTest(
runtimeConfig => runtimeConfig
.WithFramework(MicrosoftNETCoreApp, "6.0.0"))
.ShouldHaveResolvedFramework(MicrosoftNETCoreApp, "6.1.2");
.ShouldHaveResolvedFramework(MicrosoftNETCoreApp, "6.1.2")
.And.HaveStdErrContaining($"Ignoring FX version [6.0.0] without .deps.json");
}

[Fact]
Expand All @@ -51,7 +54,8 @@ public void FrameworkHiveSelection_CurrentDirectoryIsIgnored()
.WithRuntimeConfigCustomizer(runtimeConfig => runtimeConfig
.WithFramework(MicrosoftNETCoreApp, "5.0.0"))
.WithWorkingDirectory(SharedState.DotNetCurrentHive.BinPath))
.ShouldHaveResolvedFramework(MicrosoftNETCoreApp, "5.2.0");
.ShouldHaveResolvedFramework(MicrosoftNETCoreApp, "5.2.0")
.And.HaveStdErrContaining($"Ignoring FX version [5.0.0] without .deps.json");
}

private CommandResult RunTest(Func<RuntimeConfig, RuntimeConfig> runtimeConfig)
Expand Down Expand Up @@ -89,6 +93,12 @@ public SharedTestState()
.AddMicrosoftNETCoreAppFrameworkMockHostPolicy("6.1.2")
.Build();

// Empty Microsoft.NETCore.App directory - should not be recognized as a valid framework
// Version is the best match for some test cases, but they should be ignored
string netCoreAppDir = Path.Combine(DotNetMainHive.BinPath, "shared", Constants.MicrosoftNETCoreApp);
Directory.CreateDirectory(Path.Combine(netCoreAppDir, "5.0.0"));
Directory.CreateDirectory(Path.Combine(netCoreAppDir, "6.0.0"));

DotNetGlobalHive = DotNet("GlobalHive")
.AddMicrosoftNETCoreAppFrameworkMockHostPolicy("5.1.2")
.AddMicrosoftNETCoreAppFrameworkMockHostPolicy("6.2.0")
Expand Down
9 changes: 7 additions & 2 deletions src/installer/tests/HostActivation.Tests/MultilevelSharedFxLookup.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ public MultilevelSharedFxLookup()
_builtSharedUberFxDir = Path.Combine(_builtDotnet, "shared", "Microsoft.UberFramework", _sharedFxVersion);
SharedFramework.CreateUberFrameworkArtifacts(_builtSharedFxDir, _builtSharedUberFxDir, SystemCollectionsImmutableAssemblyVersion, SystemCollectionsImmutableFileVersion);

// Empty Microsoft.NETCore.App directory - should not be recognized as a valid framework
Directory.CreateDirectory(Path.Combine(_exeSharedFxBaseDir, "9999.9.9"));

// Trace messages used to identify from which folder the framework was picked
_hostPolicyDllName = Path.GetFileName(fixture.TestProject.HostPolicyDll);
_exeSelectedMessage = $"The expected {_hostPolicyDllName} directory is [{_exeSharedFxBaseDir}";
Expand Down Expand Up @@ -237,7 +240,8 @@ public void SharedMultilevelFxLookup_Must_Verify_Folders_in_the_Correct_Order()
.CaptureStdErr()
.Execute()
.Should().Pass()
.And.HaveStdOutContaining("Microsoft.NETCore.App 9999.0.0");
.And.HaveStdOutContaining("Microsoft.NETCore.App 9999.0.0")
.And.HaveStdErrContaining("Ignoring FX version [9999.9.9] without .deps.json");
}

[Fact]
Expand Down Expand Up @@ -341,7 +345,8 @@ public void SharedMultilevelFxLookup_Must_Not_Roll_Forward_If_Framework_Version_
.And.HaveStdOutContaining("Microsoft.NETCore.App 9999.0.0-dummy2")
.And.HaveStdOutContaining("Microsoft.NETCore.App 9999.0.2")
.And.HaveStdOutContaining("Microsoft.NETCore.App 9999.0.3")
.And.HaveStdOutContaining("Microsoft.NETCore.App 9999.0.0-dummy3");
.And.HaveStdOutContaining("Microsoft.NETCore.App 9999.0.0-dummy3")
.And.HaveStdErrContaining("Ignoring FX version [9999.9.9] without .deps.json");
}
}
}
20 changes: 15 additions & 5 deletions src/installer/tests/HostActivation.Tests/NativeHostApis.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public SdkResolutionFixture(SharedTestState state)

Directory.CreateDirectory(WorkingDir);

// start with an empty global.json, it will be ignored, but prevent one lying on disk
// start with an empty global.json, it will be ignored, but prevent one lying on disk
// on a given machine from impacting the test.
File.WriteAllText(GlobalJson, "{}");

Expand All @@ -117,12 +117,22 @@ public SdkResolutionFixture(SharedTestState state)
foreach ((string fwName, string[] fwVersions) in ProgramFilesGlobalFrameworks)
{
foreach (string fwVersion in fwVersions)
Directory.CreateDirectory(Path.Combine(ProgramFilesGlobalFrameworksDir, fwName, fwVersion));
AddFrameworkDirectory(ProgramFilesGlobalFrameworksDir, fwName, fwVersion);
}
foreach ((string fwName, string[] fwVersions) in LocalFrameworks)
{
foreach (string fwVersion in fwVersions)
Directory.CreateDirectory(Path.Combine(LocalFrameworksDir, fwName, fwVersion));
AddFrameworkDirectory(LocalFrameworksDir, fwName, fwVersion);

// Empty framework directory - this should not be recognized as a valid framework directory
Directory.CreateDirectory(Path.Combine(LocalFrameworksDir, fwName, "9.9.9"));
}

static void AddFrameworkDirectory(string frameworkDir, string name, string version)
{
string versionDir = Path.Combine(frameworkDir, name, version);
Directory.CreateDirectory(versionDir);
File.WriteAllText(Path.Combine(versionDir, $"{name}.deps.json"), string.Empty);
}
}
}
Expand Down Expand Up @@ -230,7 +240,7 @@ public void Hostfxr_resolve_sdk2_without_global_json_and_disallowing_previews()
[Fact]
public void Hostfxr_resolve_sdk2_with_global_json_and_disallowing_previews()
{
// With global.json specifying a preview, roll forward to preview
// With global.json specifying a preview, roll forward to preview
// since flag has no impact if global.json specifies a preview.
// Also check that global.json that impacted resolution is reported.

Expand Down Expand Up @@ -511,7 +521,7 @@ public void Hostfxr_get_dotnet_environment_info_with_multilevel_lookup_only_self
public void Hostfxr_get_dotnet_environment_info_global_install_path()
{
var f = new SdkResolutionFixture(sharedTestState);

f.Dotnet.Exec(f.AppDll, new[] { "hostfxr_get_dotnet_environment_info" })
.CaptureStdOut()
.CaptureStdErr()
Expand Down
40 changes: 29 additions & 11 deletions src/installer/tests/TestUtils/DotNetBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ public DotNetBuilder(string basePath, string builtDotnet, string name)
public DotNetBuilder AddMicrosoftNETCoreAppFrameworkMockHostPolicy(string version)
{
// ./shared/Microsoft.NETCore.App/<version> - create a mock of the root framework
string netCoreAppPath = Path.Combine(_path, "shared", "Microsoft.NETCore.App", version);
Directory.CreateDirectory(netCoreAppPath);
string netCoreAppPath = AddFramework(Constants.MicrosoftNETCoreApp, version);

// ./shared/Microsoft.NETCore.App/<version>/hostpolicy.dll - this is a mock, will not actually load CoreCLR
string mockHostPolicyFileName = RuntimeInformationExtensions.GetSharedLibraryFileNameForCurrentPlatform("mockhostpolicy");
Expand Down Expand Up @@ -122,18 +121,17 @@ public DotNetBuilder RemoveHostFxr(Version version = null)
public DotNetBuilder AddMicrosoftNETCoreAppFrameworkMockCoreClr(string version, Action<NetCoreAppBuilder> customizer = null)
{
// ./shared/Microsoft.NETCore.App/<version> - create a mock of the root framework
string netCoreAppPath = Path.Combine(_path, "shared", "Microsoft.NETCore.App", version);
Directory.CreateDirectory(netCoreAppPath);
string netCoreAppPath = AddFramework(Constants.MicrosoftNETCoreApp, version);

string hostPolicyFileName = RuntimeInformationExtensions.GetSharedLibraryFileNameForCurrentPlatform("hostpolicy");
string coreclrFileName = RuntimeInformationExtensions.GetSharedLibraryFileNameForCurrentPlatform("coreclr");
string mockCoreclrFileName = RuntimeInformationExtensions.GetSharedLibraryFileNameForCurrentPlatform("mockcoreclr");

string currentRid = _repoDirectories.TargetRID;

NetCoreAppBuilder.ForNETCoreApp("Microsoft.NETCore.App", currentRid)
NetCoreAppBuilder.ForNETCoreApp(Constants.MicrosoftNETCoreApp, currentRid)
.WithStandardRuntimeFallbacks()
.WithProject("Microsoft.NETCore.App", version, p => p
.WithProject(Constants.MicrosoftNETCoreApp, version, p => p
.WithNativeLibraryGroup(null, g => g
// ./shared/Microsoft.NETCore.App/<version>/coreclr.dll - this is a mock, will not actually run CoreClr
.WithAsset((new NetCoreAppBuilder.RuntimeFileBuilder($"runtimes/{currentRid}/native/{coreclrFileName}"))
Expand All @@ -158,19 +156,18 @@ public DotNetBuilder AddMicrosoftNETCoreAppFrameworkMockCoreClr(string version,
/// <param name="version">Framework version</param>
/// <param name="runtimeConfigCustomizer">Customization function for the runtime config</param>
/// <remarks>
/// The added mock framework will only contain a runtime.config.json file.
/// The added mock framework will only contain a deps.json and a runtime.config.json file.
/// </remarks>
public DotNetBuilder AddFramework(
string name,
string version,
Action<RuntimeConfig> runtimeConfigCustomizer)
{
// ./shared/<name>/<version> - create a mock of effectively empty non-root framework
string path = Path.Combine(_path, "shared", name, version);
Directory.CreateDirectory(path);
// ./shared/<name>/<version> - create a mock of the framework
string path = AddFramework(name, version);

// ./shared/<name>/<version>/<name>.runtimeconfig.json - runtime config which can be customized
RuntimeConfig runtimeConfig = new RuntimeConfig(Path.Combine(path, name + ".runtimeconfig.json"));
RuntimeConfig runtimeConfig = new RuntimeConfig(Path.Combine(path, $"{name}.runtimeconfig.json"));
runtimeConfigCustomizer(runtimeConfig);
runtimeConfig.Save();

Expand All @@ -193,6 +190,27 @@ public DotNetBuilder AddMicrosoftNETCoreAppFrameworkMockCoreClr(string version,
return this;
}

/// <summary>
/// Add a minimal mock framework with the specified framework name and version
/// </summary>
/// <param name="name">Framework name</param>
/// <param name="version">Framework version</param>
/// <returns>Framework directory</returns>
/// <remarks>
/// The added mock framework will only contain a deps.json.
/// </remarks>
private string AddFramework(string name, string version)
{
// ./shared/<name>/<version> - create a mock of effectively the framework
string path = Path.Combine(_path, "shared", name, version);
Directory.CreateDirectory(path);

// ./shared/<name>/<version>/<name>.deps.json - empty file
File.WriteAllText(Path.Combine(path, $"{name}.deps.json"), string.Empty);

return path;
}

public DotNetCli Build()
{
return new DotNetCli(_path);
Expand Down
81 changes: 45 additions & 36 deletions src/native/corehost/fxr/framework_info.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,57 +34,66 @@ bool compare_by_name_and_version(const framework_info &a, const framework_info &

/*static*/ void framework_info::get_all_framework_infos(
const pal::string_t& own_dir,
const pal::string_t& fx_name,
const pal::char_t* fx_name,
std::vector<framework_info>* framework_infos)
{
std::vector<pal::string_t> hive_dir;
get_framework_and_sdk_locations(own_dir, &hive_dir);

int32_t hive_depth = 0;

for (pal::string_t dir : hive_dir)
for (const pal::string_t& dir : hive_dir)
{
auto fx_shared_dir = dir;
append_path(&fx_shared_dir, _X("shared"));

if (pal::directory_exists(fx_shared_dir))
if (!pal::directory_exists(fx_shared_dir))
continue;

std::vector<pal::string_t> fx_names;
if (fx_name != nullptr)
{
std::vector<pal::string_t> fx_names;
if (fx_name.length())
{
// Use the provided framework name
fx_names.push_back(fx_name);
}
else
{
// Read all frameworks, including "Microsoft.NETCore.App"
pal::readdir_onlydirectories(fx_shared_dir, &fx_names);
}
// Use the provided framework name
fx_names.push_back(fx_name);
}
else
{
// Read all frameworks, including "Microsoft.NETCore.App"
pal::readdir_onlydirectories(fx_shared_dir, &fx_names);
}

for (pal::string_t fx_name : fx_names)
{
auto fx_dir = fx_shared_dir;
append_path(&fx_dir, fx_name.c_str());
for (const pal::string_t& fx_name_local : fx_names)
{
auto fx_dir = fx_shared_dir;
append_path(&fx_dir, fx_name_local.c_str());

if (!pal::directory_exists(fx_dir))
continue;

trace::verbose(_X("Gathering FX locations in [%s]"), fx_dir.c_str());

if (pal::directory_exists(fx_dir))
std::vector<pal::string_t> versions;
pal::readdir_onlydirectories(fx_dir, &versions);
for (const pal::string_t& ver : versions)
{
// Make sure we filter out any non-version folders.
fx_ver_t parsed;
if (!fx_ver_t::parse(ver, &parsed, false))
continue;

// Check that the framework's .deps.json exists.
pal::string_t fx_version_dir = fx_dir;
append_path(&fx_version_dir, ver.c_str());
if (!library_exists_in_dir(fx_version_dir, fx_name_local + _X(".deps.json"), nullptr))
{
trace::verbose(_X("Gathering FX locations in [%s]"), fx_dir.c_str());

std::vector<pal::string_t> versions;
pal::readdir_onlydirectories(fx_dir, &versions);
for (const auto& ver : versions)
{
// 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 FX version [%s]"), ver.c_str());

framework_info info(fx_name, fx_dir, parsed, hive_depth);
framework_infos->push_back(info);
}
}
trace::verbose(_X("Ignoring FX version [%s] without .deps.json"), ver.c_str());
continue;
}

trace::verbose(_X("Found FX version [%s]"), ver.c_str());

framework_info info(fx_name_local, fx_dir, parsed, hive_depth);
framework_infos->push_back(info);
}
}

Expand All @@ -97,7 +106,7 @@ bool compare_by_name_and_version(const framework_info &a, const framework_info &
/*static*/ bool framework_info::print_all_frameworks(const pal::string_t& own_dir, const pal::string_t& leading_whitespace)
{
std::vector<framework_info> framework_infos;
get_all_framework_infos(own_dir, _X(""), &framework_infos);
get_all_framework_infos(own_dir, nullptr, &framework_infos);
for (framework_info info : framework_infos)
{
trace::println(_X("%s%s %s [%s]"), leading_whitespace.c_str(), info.name.c_str(), info.version.as_str().c_str(), info.path.c_str());
Expand Down
2 changes: 1 addition & 1 deletion src/native/corehost/fxr/framework_info.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ struct framework_info

static void get_all_framework_infos(
const pal::string_t& own_dir,
const pal::string_t& fx_name,
const pal::char_t* fx_name,
std::vector<framework_info>* framework_infos);

static bool print_all_frameworks(const pal::string_t& own_dir, const pal::string_t& leading_whitespace);
Expand Down
Loading

0 comments on commit c2f1e7e

Please sign in to comment.