Skip to content

Check for <framework_name>.deps.json when enumerating framework paths #90035

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
elinor-fung merged 1 commit into from
Aug 7, 2023
Merged

Check for <framework_name>.deps.json when enumerating framework paths #90035

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions src/installer/tests/HostActivation.Tests/FrameworkResolution/MultipleHives.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ public void FrameworkHiveSelection(string requestedVersion, string tfm, bool? mu
.WithTfm(tfm)
.WithFramework(MicrosoftNETCoreApp, requestedVersion),
multiLevelLookup)
.ShouldHaveResolvedFramework(MicrosoftNETCoreApp, resolvedVersion);
.ShouldHaveResolvedFramework(MicrosoftNETCoreApp, resolvedVersion)
.And.HaveStdErrContaining($"Ignoring FX version [{requestedVersion}] without .deps.json");
}

[Fact]
Expand Down Expand Up @@ -157,7 +158,8 @@ public void ListRuntimes(bool? multiLevelLookup)
new TestSettings().WithCommandLine("--list-runtimes"),
multiLevelLookup,
testApp: null)
.Should().HaveStdOut(expectedOutput);
.Should().HaveStdOut(expectedOutput)
.And.HaveStdErrContaining("Ignoring FX version [9999.9.9] without .deps.json");
}

[Theory]
Expand All @@ -180,7 +182,8 @@ public void DotnetInfo(bool? multiLevelLookup)
new TestSettings().WithCommandLine("--info"),
multiLevelLookup,
testApp: null)
.Should().HaveStdOutContaining(expectedOutput);
.Should().HaveStdOutContaining(expectedOutput)
.And.HaveStdErrContaining("Ignoring FX version [9999.9.9] without .deps.json");
}

[Theory]
Expand Down Expand Up @@ -210,7 +213,8 @@ public void FrameworkResolutionError(string tfm, bool? multiLevelLookup, bool ef
multiLevelLookup)
.Should().Fail()
.And.HaveStdErrContaining(expectedOutput)
.And.HaveStdErrContaining("https://aka.ms/dotnet/app-launch-failed");
.And.HaveStdErrContaining("https://aka.ms/dotnet/app-launch-failed")
.And.HaveStdErrContaining("Ignoring FX version [9999.9.9] without .deps.json");
}

private CommandResult RunTest(Func<RuntimeConfig, RuntimeConfig> runtimeConfig, bool? multiLevelLookup = true)
Expand Down Expand Up @@ -254,6 +258,14 @@ public SharedTestState()
.AddMicrosoftNETCoreAppFrameworkMockHostPolicy("7.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"));
Directory.CreateDirectory(Path.Combine(netCoreAppDir, "7.0.0"));
Directory.CreateDirectory(Path.Combine(netCoreAppDir, "9999.9.9"));

DotNetGlobalHive = DotNet("GlobalHive")
.AddMicrosoftNETCoreAppFrameworkMockHostPolicy("5.1.2")
.AddMicrosoftNETCoreAppFrameworkMockHostPolicy("6.1.4")
Expand Down
14 changes: 12 additions & 2 deletions src/installer/tests/HostActivation.Tests/NativeHostApis.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,15 @@ 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 AddSdkDirectory(string sdkDir, string version)
Expand All @@ -129,6 +132,13 @@ static void AddSdkDirectory(string sdkDir, string version)
Directory.CreateDirectory(versionDir);
File.WriteAllText(Path.Combine(versionDir, "dotnet.dll"), string.Empty);
}

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
40 changes: 29 additions & 11 deletions src/installer/tests/TestUtils/DotNetBuilder.cs
View file
Open in desktop
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
File.Copy(
Expand Down Expand Up @@ -120,14 +119,13 @@ 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 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/{Binaries.CoreClr.FileName}"))
Expand All @@ -152,20 +150,19 @@ 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,
Action<string> frameworkCustomizer = null)
{
// ./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 @@ -191,6 +188,27 @@ public DotNetBuilder AddMockSDK(
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ 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,
bool disable_multilevel_lookup,
std::vector<framework_info>* framework_infos)
{
Expand All @@ -43,49 +43,58 @@ bool compare_by_name_and_version(const framework_info &a, const framework_info &

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_local : fx_names)
{
auto fx_dir = fx_shared_dir;
append_path(&fx_dir, fx_name_local.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))
Copy link
Member

@vitek-karas vitek-karas Aug 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Maybe as a future cleanup - rename the helper as it's not about libraries... just files.

Copy link
Member Author

@elinor-fung elinor-fung Aug 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will plan to do that in a future cleanup.

{
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_local, 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 @@ -98,7 +107,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(""), /*disable_multilevel_lookup*/ true, &framework_infos);
get_all_framework_infos(own_dir, nullptr, /*disable_multilevel_lookup*/ true, &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
Open in desktop
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,
bool disable_multilevel_lookup,
std::vector<framework_info>* framework_infos);

Expand Down
51 changes: 32 additions & 19 deletions src/native/corehost/fxr/fx_resolver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,6 @@ namespace

if (best_match == fx_ver_t())
{
// This is not strictly necessary, we just need to return version which doesn't exist.
// But it's cleaner to return the desider reference then invalid -1.-1.-1 version.
best_match = fx_ref.get_fx_version_number();
trace::verbose(_X("Framework reference didn't resolve to any available version."));
}
else if (trace::is_enabled())
Expand Down Expand Up @@ -212,7 +209,8 @@ namespace
pal::string_t selected_fx_version;
fx_ver_t selected_ver;

for (pal::string_t dir : hive_dir)
pal::string_t deps_file_name = fx_ref.get_fx_name() + _X(".deps.json");
for (pal::string_t& dir : hive_dir)
{
auto fx_dir = dir;
trace::verbose(_X("Searching FX directory in [%s]"), fx_dir.c_str());
Expand All @@ -236,7 +234,7 @@ namespace
fx_ref.get_fx_version().c_str());

append_path(&fx_dir, fx_ref.get_fx_version().c_str());
if (pal::directory_exists(fx_dir))
if (library_exists_in_dir(fx_dir, deps_file_name, nullptr))
{
selected_fx_dir = fx_dir;
selected_fx_version = fx_ref.get_fx_version();
Expand All @@ -259,24 +257,39 @@ namespace
}

fx_ver_t resolved_ver = resolve_framework_reference_from_version_list(version_list, fx_ref);

pal::string_t resolved_ver_str = resolved_ver.as_str();
append_path(&fx_dir, resolved_ver_str.c_str());

if (pal::directory_exists(fx_dir))
while (resolved_ver != fx_ver_t())
{
if (selected_ver != fx_ver_t())
pal::string_t resolved_ver_str = resolved_ver.as_str();
pal::string_t resolved_fx_dir = fx_dir;
append_path(&resolved_fx_dir, resolved_ver_str.c_str());

// Check that the framework's .deps.json exists. To minimize the file checks done in the most common
// scenario (.deps.json exists), only check after resolving the version and if the .deps.json doesn't
// exist, attempt resolving again without that version.
if (!library_exists_in_dir(resolved_fx_dir, deps_file_name, nullptr))
{
// Compare the previous hive_dir selection with the current hive_dir to see which one is the better match
resolved_ver = resolve_framework_reference_from_version_list({ resolved_ver, selected_ver }, fx_ref);
// Remove the version and try resolving again
trace::verbose(_X("Ignoring FX version [%s] without .deps.json"), resolved_ver_str.c_str());
version_list.erase(std::find(version_list.cbegin(), version_list.cend(), resolved_ver));
resolved_ver = resolve_framework_reference_from_version_list(version_list, fx_ref);
}

if (resolved_ver != selected_ver)
else
{
trace::verbose(_X("Changing Selected FX version from [%s] to [%s]"), selected_fx_dir.c_str(), fx_dir.c_str());
selected_ver = resolved_ver;
selected_fx_dir = fx_dir;
selected_fx_version = resolved_ver_str;
if (selected_ver != fx_ver_t())
{
// Compare the previous hive_dir selection with the current hive_dir to see which one is the better match
resolved_ver = resolve_framework_reference_from_version_list({ resolved_ver, selected_ver }, fx_ref);
}

if (resolved_ver != selected_ver)
{
trace::verbose(_X("Changing Selected FX version from [%s] to [%s]"), selected_fx_dir.c_str(), resolved_fx_dir.c_str());
selected_ver = resolved_ver;
selected_fx_dir = resolved_fx_dir;
selected_fx_version = resolved_ver_str;
}

break;
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/native/corehost/fxr/fx_resolver.messages.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,14 @@ void fx_resolver_t::display_missing_framework_error(
if (fx_dir.length())
{
fx_ver_dirs = fx_dir;
framework_info::get_all_framework_infos(get_directory(fx_dir), fx_name, disable_multilevel_lookup, &framework_infos);
framework_info::get_all_framework_infos(get_directory(fx_dir), fx_name.c_str(), disable_multilevel_lookup, &framework_infos);
}
else
{
fx_ver_dirs = dotnet_root;
}

framework_info::get_all_framework_infos(dotnet_root, fx_name, disable_multilevel_lookup, &framework_infos);
framework_info::get_all_framework_infos(dotnet_root, fx_name.c_str(), disable_multilevel_lookup, &framework_infos);

// Display the error message about missing FX.
if (fx_version.length())
Expand Down
2 changes: 1 addition & 1 deletion src/native/corehost/fxr/hostfxr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ SHARED_API int32_t HOSTFXR_CALLTYPE hostfxr_get_dotnet_environment_info(
}

std::vector<framework_info> framework_infos;
framework_info::get_all_framework_infos(dotnet_dir, _X(""), /*disable_multilevel_lookup*/ true, &framework_infos);
framework_info::get_all_framework_infos(dotnet_dir, nullptr, /*disable_multilevel_lookup*/ true, &framework_infos);

std::vector<hostfxr_dotnet_environment_framework_info> environment_framework_infos;
std::vector<pal::string_t> framework_versions;
Expand Down