Skip to content

Commit

Permalink
Make missing framework error message list other architectures that we…
Browse files Browse the repository at this point in the history
…re found (dotnet#107156)

When erroring on a missing framework, check if there are versions of the framework for other architectures and list them for the user.
  • Loading branch information
elinor-fung committed Sep 9, 2024
1 parent 2ed43b6 commit 51c350c
Show file tree
Hide file tree
Showing 23 changed files with 124 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using Microsoft.DotNet.Cli.Build.Framework;
using Xunit;

using static Microsoft.DotNet.CoreSetup.Test.Constants;

namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.FrameworkResolution
{
public class ApplyPatchesSettings :
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
using System;
using Xunit;

using static Microsoft.DotNet.CoreSetup.Test.Constants;

namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.FrameworkResolution
{
public class ComplexHierarchies :
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public enum SettingLocation
public static Func<TestSettings, TestSettings> RollForwardSetting(
SettingLocation location,
string value,
string frameworkReferenceName = MicrosoftNETCoreApp)
string frameworkReferenceName = Constants.MicrosoftNETCoreApp)
{
if (value == null || location == SettingLocation.None)
{
Expand Down Expand Up @@ -114,7 +114,7 @@ public static Func<TestSettings, TestSettings> RollForwardSetting(
public static Func<TestSettings, TestSettings> RollForwardOnNoCandidateFxSetting(
SettingLocation location,
int? value,
string frameworkReferenceName = MicrosoftNETCoreApp)
string frameworkReferenceName = Constants.MicrosoftNETCoreApp)
{
if (!value.HasValue || location == SettingLocation.None)
{
Expand Down Expand Up @@ -143,7 +143,7 @@ public static Func<TestSettings, TestSettings> RollForwardOnNoCandidateFxSetting
public static Func<TestSettings, TestSettings> ApplyPatchesSetting(
SettingLocation location,
bool? value,
string frameworkReferenceName = MicrosoftNETCoreApp)
string frameworkReferenceName = Constants.MicrosoftNETCoreApp)
{
if (!value.HasValue || location == SettingLocation.None)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.FrameworkResolution
{
public abstract partial class FrameworkResolutionBase
{
protected const string MicrosoftNETCoreApp = "Microsoft.NETCore.App";

public static class ResolvedFramework
{
public const string NotFound = "[not found]";
Expand All @@ -23,7 +21,6 @@ protected CommandResult RunTest(
DotNetCli dotnet,
TestApp app,
TestSettings settings,
Action<CommandResult> resultAction = null,
bool? multiLevelLookup = false)
{
using (DotNetCliExtensions.DotNetCliCustomizer dotnetCustomizer = settings.DotnetCustomizer == null ? null : dotnet.Customize())
Expand Down Expand Up @@ -53,8 +50,6 @@ protected CommandResult RunTest(
.Environment(settings.Environment)
.Execute();

resultAction?.Invoke(result);

return result;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using Microsoft.DotNet.Cli.Build.Framework;
using Xunit;

using static Microsoft.DotNet.CoreSetup.Test.Constants;

namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.FrameworkResolution
{
public class FXVersionCLI :
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using Microsoft.DotNet.Cli.Build.Framework;
using Xunit;

using static Microsoft.DotNet.CoreSetup.Test.Constants;

namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.FrameworkResolution
{
public class IncludedFrameworksSettings :
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.DotNet.Cli.Build;
using Microsoft.DotNet.Cli.Build.Framework;
using Xunit;
using static Microsoft.DotNet.CoreSetup.Test.Constants;

namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.FrameworkResolution
{
Expand Down Expand Up @@ -217,6 +218,50 @@ public void FrameworkResolutionError(string tfm, bool? multiLevelLookup, bool ef
.And.HaveStdErrContaining("Ignoring FX version [9999.9.9] without .deps.json");
}

[Fact]
public void FrameworkResolutionError_ListOtherArchitectures()
{
using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(SharedState.DotNetMainHive.GreatestVersionHostFxrFilePath))
using (var otherArchArtifact = TestArtifact.Create("otherArch"))
{
string requestedVersion = "9999.9.9";
string[] otherArchs = ["arm64", "x64", "x86"];
var installLocations = new (string, string)[otherArchs.Length];
for (int i = 0; i < otherArchs.Length; i++)
{
string arch = otherArchs[i];

// Create a .NET install with Microsoft.NETCoreApp at the registered location
var dotnet = new DotNetBuilder(otherArchArtifact.Location, TestContext.BuiltDotNet.BinPath, arch)
.AddMicrosoftNETCoreAppFrameworkMockHostPolicy(requestedVersion)
.Build();
installLocations[i] = (arch, dotnet.BinPath);
}

registeredInstallLocationOverride.SetInstallLocation(installLocations);

CommandResult result = RunTest(
new TestSettings()
.WithRuntimeConfigCustomizer(c => c.WithFramework(MicrosoftNETCoreApp, requestedVersion))
.WithEnvironment(TestOnlyEnvironmentVariables.RegisteredConfigLocation, registeredInstallLocationOverride.PathValueOverride),
multiLevelLookup: null);

result.ShouldFailToFindCompatibleFrameworkVersion(MicrosoftNETCoreApp, requestedVersion)
.And.HaveStdErrContaining("The following frameworks for other architectures were found:");

// Error message should list framework found for other architectures
foreach ((string arch, string path) in installLocations)
{
if (arch == TestContext.BuildArchitecture)
continue;

string expectedPath = System.Text.RegularExpressions.Regex.Escape(Path.Combine(path, "shared", MicrosoftNETCoreApp));
result.Should()
.HaveStdErrMatching($@"{arch}\s*{requestedVersion} at \[{expectedPath}\]", System.Text.RegularExpressions.RegexOptions.Multiline);
}
}
}

private CommandResult RunTest(Func<RuntimeConfig, RuntimeConfig> runtimeConfig, bool? multiLevelLookup = true)
=> RunTest(new TestSettings().WithRuntimeConfigCustomizer(runtimeConfig), multiLevelLookup);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using Microsoft.DotNet.Cli.Build.Framework;
using Xunit;

using static Microsoft.DotNet.CoreSetup.Test.Constants;

namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.FrameworkResolution
{
public class RollForwardAndRollForwardOnNoCandidateFxSettings :
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
using System;
using Xunit;

using static Microsoft.DotNet.CoreSetup.Test.Constants;

namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.FrameworkResolution
{
public class RollForwardMultipleFrameworks :
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using Microsoft.DotNet.Cli.Build;
using Microsoft.DotNet.Cli.Build.Framework;
using System;
using Xunit;

using static Microsoft.DotNet.CoreSetup.Test.Constants;

namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.FrameworkResolution
{
public class RollForwardOnNoCandidateFx :
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
using System;
using Xunit;

using static Microsoft.DotNet.CoreSetup.Test.Constants;

namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.FrameworkResolution
{
public class RollForwardOnNoCandidateFxMultipleFrameworks :
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using Microsoft.DotNet.Cli.Build.Framework;
using Xunit;

using static Microsoft.DotNet.CoreSetup.Test.Constants;

namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.FrameworkResolution
{
public class RollForwardOnNoCandidateFxSettings :
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using Microsoft.DotNet.Cli.Build.Framework;
using Xunit;

using static Microsoft.DotNet.CoreSetup.Test.Constants;

namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.FrameworkResolution
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using Microsoft.DotNet.Cli.Build.Framework;
using Xunit;

using static Microsoft.DotNet.CoreSetup.Test.Constants;

namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.FrameworkResolution
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using Microsoft.DotNet.Cli.Build.Framework;
using Xunit;

using static Microsoft.DotNet.CoreSetup.Test.Constants;

namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.FrameworkResolution
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using Microsoft.DotNet.Cli.Build.Framework;
using Xunit;

using static Microsoft.DotNet.CoreSetup.Test.Constants;

namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.FrameworkResolution
{
public class RollForwardSettings :
Expand Down
6 changes: 5 additions & 1 deletion src/installer/tests/TestUtils/Constants.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;

namespace Microsoft.DotNet.CoreSetup.Test
{
public static class Constants
Expand Down Expand Up @@ -82,8 +84,10 @@ public static class DisableGuiErrors
public static class TestOnlyEnvironmentVariables
{
public const string DefaultInstallPath = "_DOTNET_TEST_DEFAULT_INSTALL_PATH";
public const string RegistryPath = "_DOTNET_TEST_REGISTRY_PATH";
public const string GloballyRegisteredPath = "_DOTNET_TEST_GLOBALLY_REGISTERED_PATH";

public static string RegisteredConfigLocation = OperatingSystem.IsWindows() ? RegistryPath : InstallLocationPath;
public const string RegistryPath = "_DOTNET_TEST_REGISTRY_PATH";
public const string InstallLocationPath = "_DOTNET_TEST_INSTALL_LOCATION_PATH";
}

Expand Down
2 changes: 1 addition & 1 deletion src/native/corehost/fxr/fx_resolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ StatusCode fx_resolver_t::resolve_frameworks_for_app(
_X("Architecture: %s"),
app_display_name,
get_current_arch_name());
display_missing_framework_error(resolution_failure.missing.get_fx_name(), resolution_failure.missing.get_fx_version(), pal::string_t(), dotnet_root, app_config.get_is_multilevel_lookup_disabled());
display_missing_framework_error(resolution_failure.missing.get_fx_name(), resolution_failure.missing.get_fx_version(), dotnet_root, app_config.get_is_multilevel_lookup_disabled());
break;
case StatusCode::FrameworkCompatFailure:
display_incompatible_framework_error(resolution_failure.incompatible_higher.get_fx_version(), resolution_failure.incompatible_lower);
Expand Down
1 change: 0 additions & 1 deletion src/native/corehost/fxr/fx_resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ class fx_resolver_t
static void display_missing_framework_error(
const pal::string_t& fx_name,
const pal::string_t& fx_version,
const pal::string_t& fx_dir,
const pal::string_t& dotnet_root,
bool disable_multilevel_lookup);
static void display_incompatible_framework_error(
Expand Down
41 changes: 27 additions & 14 deletions src/native/corehost/fxr/fx_resolver.messages.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include "fx_resolver.h"
#include "framework_info.h"
#include "install_info.h"

/**
* When the framework is referenced more than once in a non-compatible way, display detailed error message
Expand Down Expand Up @@ -92,23 +93,9 @@ void fx_resolver_t::display_summary_of_frameworks(
void fx_resolver_t::display_missing_framework_error(
const pal::string_t& fx_name,
const pal::string_t& fx_version,
const pal::string_t& fx_dir,
const pal::string_t& dotnet_root,
bool disable_multilevel_lookup)
{
std::vector<framework_info> framework_infos;
pal::string_t fx_ver_dirs;
if (fx_dir.length())
{
fx_ver_dirs = fx_dir;
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.c_str(), disable_multilevel_lookup, &framework_infos);

// Display the error message about missing FX.
if (fx_version.length())
Expand All @@ -122,6 +109,8 @@ void fx_resolver_t::display_missing_framework_error(

trace::error(_X(".NET location: %s\n"), dotnet_root.c_str());

std::vector<framework_info> framework_infos;
framework_info::get_all_framework_infos(dotnet_root, fx_name.c_str(), disable_multilevel_lookup, &framework_infos);
if (framework_infos.size())
{
trace::error(_X("The following frameworks were found:"));
Expand All @@ -135,6 +124,30 @@ void fx_resolver_t::display_missing_framework_error(
trace::error(_X("No frameworks were found."));
}

std::vector<std::pair<pal::architecture, std::vector<framework_info>>> other_arch_framework_infos;
install_info::enumerate_other_architectures(
[&](pal::architecture arch, const pal::string_t& install_location, bool is_registered)
{
std::vector<framework_info> other_arch_infos;
framework_info::get_all_framework_infos(install_location, fx_name.c_str(), disable_multilevel_lookup, &other_arch_infos);
if (!other_arch_infos.empty())
{
other_arch_framework_infos.push_back(std::make_pair(arch, std::move(other_arch_infos)));
}
});
if (!other_arch_framework_infos.empty())
{
trace::error(_X("\nThe following frameworks for other architectures were found:"));
for (const auto& arch_info_pair : other_arch_framework_infos)
{
trace::error(_X(" %s"), get_arch_name(arch_info_pair.first));
for (const framework_info& info : arch_info_pair.second)
{
trace::error(_X(" %s at [%s]"), info.version.as_str().c_str(), info.path.c_str());
}
}
}

pal::string_t url = get_download_url(fx_name.c_str(), fx_version.c_str());
trace::error(
_X("\n")
Expand Down
19 changes: 14 additions & 5 deletions src/native/corehost/fxr/install_info.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ bool install_info::print_environment(const pal::char_t* leading_whitespace)
return found_any;
}

bool install_info::print_other_architectures(const pal::char_t* leading_whitespace)
bool install_info::enumerate_other_architectures(std::function<void(pal::architecture, const pal::string_t&, bool)> callback)
{
bool found_any = false;
for (uint32_t i = 0; i < static_cast<uint32_t>(pal::architecture::__last); ++i)
Expand All @@ -47,13 +47,22 @@ bool install_info::print_other_architectures(const pal::char_t* leading_whitespa
{
found_any = true;
remove_trailing_dir_separator(&install_location);
callback(arch, install_location, is_registered);
}
}

return found_any;
}

bool install_info::print_other_architectures(const pal::char_t* leading_whitespace)
{
return enumerate_other_architectures(
[&](pal::architecture arch, const pal::string_t& install_location, bool is_registered)
{
trace::println(_X("%s%-5s [%s]"), leading_whitespace, get_arch_name(arch), install_location.c_str());
if (is_registered)
{
trace::println(_X("%s registered at [%s]"), leading_whitespace, pal::get_dotnet_self_registered_config_location(arch).c_str());
}
}
}

return found_any;
});
}
2 changes: 2 additions & 0 deletions src/native/corehost/fxr/install_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
#define __INSTALL_INFO_H__

#include "pal.h"
#include <functional>

namespace install_info
{
bool enumerate_other_architectures(std::function<void(pal::architecture, const pal::string_t&, bool)> callback);
bool print_environment(const pal::char_t* leading_whitespace);
bool print_other_architectures(const pal::char_t* leading_whitespace);
};
Expand Down
Loading

0 comments on commit 51c350c

Please sign in to comment.