Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,9 @@ public void AppHost_GUI_NoCustomErrorWriter_FrameworkMissing_ErrorReportedInDial
command.WaitForExit(true)
.Should().Fail()
.And.HaveStdErrContaining($"Showing error dialog for application: '{Path.GetFileName(appExe)}' - error code: 0x{expectedErrorCode}")
.And.HaveStdErrContaining("To run this application, you need to install a newer version of .NET");
.And.HaveStdErrContaining("You must install or update .NET to run this application.")
.And.HaveStdErrContaining("App host version:")
.And.HaveStdErrContaining("apphost_version=");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.IO;
using BundleTests.Helpers;
using Microsoft.DotNet.Cli.Build;
using Microsoft.DotNet.Cli.Build.Framework;
using Microsoft.DotNet.CoreSetup.Test;
using Microsoft.NET.HostModel.Bundle;
Expand All @@ -22,18 +23,22 @@ public BundledAppWithSubDirs(SharedTestState fixture)

private void RunTheApp(string path, TestProjectFixture fixture)
{
Command.Create(path)
.EnableTracingAndCaptureOutputs()
.EnvironmentVariable("DOTNET_ROOT", fixture.BuiltDotnet.BinPath)
.EnvironmentVariable("DOTNET_ROOT(x86)", fixture.BuiltDotnet.BinPath)
.EnvironmentVariable("DOTNET_MULTILEVEL_LOOKUP", "0")
.Execute()
RunTheApp(path, fixture.BuiltDotnet)
.Should()
.Pass()
.And
.HaveStdOutContaining("Wow! We now say hello to the big world and you.");
}

private CommandResult RunTheApp(string path, DotNetCli dotnet)
{
return Command.Create(path)
.EnableTracingAndCaptureOutputs()
.DotNetRoot(dotnet.BinPath)
.MultilevelLookup(false)
.Execute();
}

[InlineData(BundleOptions.None)]
[InlineData(BundleOptions.BundleNativeBinaries)]
[InlineData(BundleOptions.BundleAllContent)]
Expand All @@ -51,6 +56,32 @@ public void Bundled_Framework_dependent_App_Run_Succeeds(BundleOptions options)
RunTheApp(singleFile, fixture);
}

[Fact]
public void Bundle_Framework_dependent_NoBundleEntryPoint()
{
var fixture = sharedTestState.TestFrameworkDependentFixture.Copy();
UseFrameworkDependentHost(fixture);
var singleFile = BundleHelper.BundleApp(fixture, BundleOptions.None);

string dotnetWithMockHostFxr = SharedFramework.CalculateUniqueTestDirectory(Path.Combine(TestArtifact.TestArtifactsPath, "guiErrors"));
using (new TestArtifact(dotnetWithMockHostFxr))
{
Directory.CreateDirectory(dotnetWithMockHostFxr);
var dotnetBuilder = new DotNetBuilder(dotnetWithMockHostFxr, sharedTestState.RepoDirectories.BuiltDotnet, "mockhostfxrFrameworkMissingFailure")
.RemoveHostFxr()
.AddMockHostFxr(new Version(2, 2, 0));
var dotnet = dotnetBuilder.Build();

// Run the bundled app (extract files)
RunTheApp(singleFile, dotnet)
.Should()
.Fail()
.And.HaveStdErrContaining("You must install or update .NET to run this application.")
.And.HaveStdErrContaining("App host version:")
.And.HaveStdErrContaining("apphost_version=");
}
}

[InlineData(BundleOptions.None)]
[InlineData(BundleOptions.BundleNativeBinaries)]
[InlineData(BundleOptions.BundleAllContent)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public StaticHost(SharedTestState fixture)
}

[Fact]
private void Can_Run_App_With_StatiHost()
private void Can_Run_App_With_StaticHost()
{
var fixture = sharedTestState.TestFixture.Copy();

Expand All @@ -35,7 +35,7 @@ private void Can_Run_App_With_StatiHost()
}

[Fact]
private void Can_Run_SingleFile_App_With_StatiHost()
private void Can_Run_SingleFile_App_With_StaticHost()
{
var fixture = sharedTestState.TestFixture.Copy();

Expand Down
18 changes: 15 additions & 3 deletions src/native/corehost/apphost/apphost.windows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,16 +70,22 @@ namespace
return false;
}

pal::string_t get_runtime_not_found_message()
pal::string_t get_apphost_details_message()
{
pal::string_t msg = INSTALL_NET_DESKTOP_ERROR_MESSAGE _X("\n\n")
_X("Architecture: ");
pal::string_t msg = _X("Architecture: ");
msg.append(get_arch());
msg.append(_X("\n")
_X("App host version: ") _STRINGIFY(COMMON_HOST_PKG_VER) _X("\n\n"));
return msg;
}

pal::string_t get_runtime_not_found_message()
{
pal::string_t msg = INSTALL_NET_DESKTOP_ERROR_MESSAGE _X("\n\n");
msg.append(get_apphost_details_message());
return msg;
}

void show_error_dialog(const pal::char_t *executable_name, int error_code)
{
pal::string_t gui_errors_disabled;
Expand Down Expand Up @@ -108,6 +114,7 @@ namespace
dialogMsg = pal::string_t(INSTALL_OR_UPDATE_NET_ERROR_MESSAGE _X("\n\n"));
pal::string_t line;
pal::stringstream_t ss(g_buffered_errors);
bool foundCustomMessage = false;
while (std::getline(ss, line, _X('\n')))
{
const pal::char_t prefix[] = _X("Framework: '");
Expand All @@ -119,18 +126,23 @@ namespace
{
dialogMsg.append(line);
dialogMsg.append(_X("\n\n"));
foundCustomMessage = true;
}
else if (utils::starts_with(line, custom_prefix, true))
{
dialogMsg.erase();
dialogMsg.append(line.substr(utils::strlen(custom_prefix)));
dialogMsg.append(_X("\n\n"));
foundCustomMessage = true;
}
else if (try_get_url_from_line(line, url))
{
break;
}
}

if (!foundCustomMessage)
dialogMsg.append(get_apphost_details_message());
}
else if (error_code == StatusCode::BundleExtractionFailure)
{
Expand Down
33 changes: 25 additions & 8 deletions src/native/corehost/corehost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,27 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll)
#define CURHOST_EXE
#endif

void need_newer_framework_error()
void need_newer_framework_error(const pal::string_t& dotnet_root, const pal::string_t& host_path)
{
pal::string_t url = get_download_url();
trace::error(_X(" _ To run this application, you need to install a newer version of .NET Core."));
Copy link
Member

Choose a reason for hiding this comment

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

Can we get rid of the check for the _ prefix now?

const pal::char_t custom_prefix[] = _X(" _ ");

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't it necessary for the pre 7.0 runtimes?
7.0 doesn't use it anymore, but the apphost still needs to work downlevel for some rather weird cases.

Copy link
Member

Choose a reason for hiding this comment

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

I thought apphost itself was the only thing actually writing with that prefix? Because with downlevel hostfxr, we couldn't actually redirect the errors from hostfxr.

Copy link
Member Author

Choose a reason for hiding this comment

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

What about 7.0 apphost running against 6.0 hostfxr - the hostfxr might still use the _ prefix somewhere (I didn't check). In that case the error info will be redirected from hostfxr to apphost and if it contains _ it should be parsed.

Copy link
Member

Choose a reason for hiding this comment

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

I was pretty sure we only ever used it for apphost, but I didn't actually go back and check.

Either way, the UX and changes you have right now look good to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

I just searched 3.1 and it only uses the prefix in apphost - but I'm still nervous about doing this. Honestly I would rather keep it.

trace::error(_X(""));
trace::error(_X(" - %s&apphost_version=%s"), url.c_str(), _STRINGIFY(COMMON_HOST_PKG_VER));
trace::error(
INSTALL_OR_UPDATE_NET_ERROR_MESSAGE
_X("\n\n")
_X("App: %s\n")
_X("Architecture: %s\n")
_X("App host version: %s\n")
_X(".NET location: %s\n")
_X("\n")
_X("Learn about runtime installation:\n")
DOTNET_APP_LAUNCH_FAILED_URL
_X("\n\n")
_X("Download the .NET runtime:\n")
_X("%s&apphost_version=%s"),
host_path.c_str(),
get_arch(),
_STRINGIFY(COMMON_HOST_PKG_VER),
dotnet_root.c_str(),
get_download_url().c_str(),
_STRINGIFY(COMMON_HOST_PKG_VER));
}

#if defined(CURHOST_EXE)
Expand Down Expand Up @@ -211,7 +226,7 @@ int exe_start(const int argc, const pal::char_t* argv[])
{
// An outdated hostfxr can only be found for framework-related apps.
trace::error(_X("The required library %s does not support single-file apps."), fxr.fxr_path().c_str());
need_newer_framework_error();
need_newer_framework_error(fxr.dotnet_root(), host_path);
rc = StatusCode::FrameworkMissingFailure;
}
}
Expand All @@ -235,10 +250,12 @@ int exe_start(const int argc, const pal::char_t* argv[])

rc = hostfxr_main_startupinfo(argc, argv, host_path_cstr, dotnet_root_cstr, app_path_cstr);

// This check exists to provide an error message for UI apps when running 3.0 apps on 2.0 only hostfxr, which doesn't support error writer redirection.
// This check exists to provide an error message for apps when running 3.0 apps on 2.0 only hostfxr, which doesn't support error writer redirection.
// Note that this is not only for UI apps - on Windows we always write errors to event log as well (regardless of UI) and it uses
// the same mechanism of redirecting error writers.
if (trace::get_error_writer() != nullptr && rc == static_cast<int>(StatusCode::FrameworkMissingFailure) && set_error_writer == nullptr)
{
need_newer_framework_error();
need_newer_framework_error(fxr.dotnet_root(), host_path);
}
}
#if !defined(FEATURE_STATIC_HOST)
Expand Down