From b9552ddcdce8f4027077a6d77be6138bc5472c09 Mon Sep 17 00:00:00 2001 From: Albert Szilvasy <66087606+szilvaa-adsk@users.noreply.github.com> Date: Wed, 13 Sep 2023 18:13:13 -0700 Subject: [PATCH 01/11] Permissive handling for runtimeconfig.json --- src/native/corehost/fxr/fx_muxer.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/native/corehost/fxr/fx_muxer.cpp b/src/native/corehost/fxr/fx_muxer.cpp index cbaf90aa69cba2..899eb88ba0988f 100644 --- a/src/native/corehost/fxr/fx_muxer.cpp +++ b/src/native/corehost/fxr/fx_muxer.cpp @@ -800,10 +800,15 @@ int fx_muxer_t::initialize_for_runtime_config( if (already_initialized) { std::unordered_map config_properties; - rc = get_init_info_for_secondary_component(host_info, mode, runtime_config, existing_context, config_properties); - if (rc != StatusCode::Success) - return rc; + // If runtime is already initialized then we can be more permissive with the config. + // It is missing then assume it is the same as the existing one. + if (pal::file_exists(runtime_config)) + { + rc = get_init_info_for_secondary_component(host_info, mode, runtime_config, existing_context, config_properties); + if (rc != StatusCode::Success) + return rc; + } rc = host_context_t::create_secondary(existing_context->hostpolicy_contract, config_properties, initialization_options, context); } else From bd5bd6fde16fa79febb531afdcde07cd768a9b26 Mon Sep 17 00:00:00 2001 From: Albert Szilvasy <66087606+szilvaa-adsk@users.noreply.github.com> Date: Mon, 18 Sep 2023 19:07:48 -0700 Subject: [PATCH 02/11] take 2: avoid changing hostfxr.dll --- src/native/corehost/fxr/fx_muxer.cpp | 11 ++---- src/native/corehost/fxr_resolver.h | 50 ++++++++++++++++------------ 2 files changed, 32 insertions(+), 29 deletions(-) diff --git a/src/native/corehost/fxr/fx_muxer.cpp b/src/native/corehost/fxr/fx_muxer.cpp index 899eb88ba0988f..cbaf90aa69cba2 100644 --- a/src/native/corehost/fxr/fx_muxer.cpp +++ b/src/native/corehost/fxr/fx_muxer.cpp @@ -800,15 +800,10 @@ int fx_muxer_t::initialize_for_runtime_config( if (already_initialized) { std::unordered_map config_properties; + rc = get_init_info_for_secondary_component(host_info, mode, runtime_config, existing_context, config_properties); + if (rc != StatusCode::Success) + return rc; - // If runtime is already initialized then we can be more permissive with the config. - // It is missing then assume it is the same as the existing one. - if (pal::file_exists(runtime_config)) - { - rc = get_init_info_for_secondary_component(host_info, mode, runtime_config, existing_context, config_properties); - if (rc != StatusCode::Success) - return rc; - } rc = host_context_t::create_secondary(existing_context->hostpolicy_contract, config_properties, initialization_options, context); } else diff --git a/src/native/corehost/fxr_resolver.h b/src/native/corehost/fxr_resolver.h index 3df6c2de3053d3..b9101528cf22c9 100644 --- a/src/native/corehost/fxr_resolver.h +++ b/src/native/corehost/fxr_resolver.h @@ -67,34 +67,42 @@ int load_fxr_and_get_delegate(hostfxr_delegate_type type, THostPathToConfigCallb if (status != StatusCode::Success) return status; - hostfxr_initialize_parameters parameters { - sizeof(hostfxr_initialize_parameters), - host_path.c_str(), - dotnet_root.c_str() - }; + if (pal::file_exists(config_path)) + { + hostfxr_initialize_parameters parameters { + sizeof(hostfxr_initialize_parameters), + host_path.c_str(), + dotnet_root.c_str() + }; - hostfxr_set_error_writer_fn set_error_writer_fn = reinterpret_cast(pal::get_symbol(fxr, "hostfxr_set_error_writer")); + hostfxr_set_error_writer_fn set_error_writer_fn = reinterpret_cast(pal::get_symbol(fxr, "hostfxr_set_error_writer")); - { - propagate_error_writer_t propagate_error_writer_to_hostfxr(set_error_writer_fn); + { + propagate_error_writer_t propagate_error_writer_to_hostfxr(set_error_writer_fn); - hostfxr_handle context; - int rc = hostfxr_initialize_for_runtime_config(config_path.c_str(), ¶meters, &context); - if (!STATUS_CODE_SUCCEEDED(rc)) - return rc; + hostfxr_handle context; + int rc = hostfxr_initialize_for_runtime_config(config_path.c_str(), ¶meters, &context); + if (!STATUS_CODE_SUCCEEDED(rc)) + return rc; - on_before_run(fxr, context); + on_before_run(fxr, context); - rc = hostfxr_get_runtime_delegate(context, type, delegate); + rc = hostfxr_get_runtime_delegate(context, type, delegate); - int rcClose = hostfxr_close(context); - if (rcClose != StatusCode::Success) - { - assert(false && "Failed to close host context"); - trace::verbose(_X("Failed to close host context: 0x%x"), rcClose); - } + int rcClose = hostfxr_close(context); + if (rcClose != StatusCode::Success) + { + assert(false && "Failed to close host context"); + trace::verbose(_X("Failed to close host context: 0x%x"), rcClose); + } - return rc; + return rc; + } + } + else + { + // null context means use the current one, if none exists it will fail + return hostfxr_get_runtime_delegate(nullptr, type, delegate); } } From 3e7eb8640133855b35adeca0b9016e7e9e2ad801 Mon Sep 17 00:00:00 2001 From: Albert Szilvasy <66087606+szilvaa-adsk@users.noreply.github.com> Date: Tue, 19 Sep 2023 14:12:55 -0700 Subject: [PATCH 03/11] Fix tests --- .../NativeHosting/Comhost.cs | 2 +- src/native/corehost/fxr_resolver.h | 33 ++++++++++--------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/NativeHosting/Comhost.cs b/src/installer/tests/HostActivation.Tests/NativeHosting/Comhost.cs index 4d023023d9791f..dbaab41ca6671d 100644 --- a/src/installer/tests/HostActivation.Tests/NativeHosting/Comhost.cs +++ b/src/installer/tests/HostActivation.Tests/NativeHosting/Comhost.cs @@ -141,7 +141,7 @@ public void ActivateClass_ValidateIErrorInfoResult() .Execute(); result.Should().Pass() - .And.HaveStdOutContaining($"The specified runtimeconfig.json [{missingRuntimeConfig}] does not exist"); + .And.HaveStdOutContaining($"runtimeconfig.json [{missingRuntimeConfig}] does not exist"); } } diff --git a/src/native/corehost/fxr_resolver.h b/src/native/corehost/fxr_resolver.h index b9101528cf22c9..bff2397968145d 100644 --- a/src/native/corehost/fxr_resolver.h +++ b/src/native/corehost/fxr_resolver.h @@ -67,18 +67,16 @@ int load_fxr_and_get_delegate(hostfxr_delegate_type type, THostPathToConfigCallb if (status != StatusCode::Success) return status; - if (pal::file_exists(config_path)) + hostfxr_set_error_writer_fn set_error_writer_fn = reinterpret_cast(pal::get_symbol(fxr, "hostfxr_set_error_writer")); { - hostfxr_initialize_parameters parameters { - sizeof(hostfxr_initialize_parameters), - host_path.c_str(), - dotnet_root.c_str() - }; - - hostfxr_set_error_writer_fn set_error_writer_fn = reinterpret_cast(pal::get_symbol(fxr, "hostfxr_set_error_writer")); - + propagate_error_writer_t propagate_error_writer_to_hostfxr(set_error_writer_fn); + if (pal::file_exists(config_path)) { - propagate_error_writer_t propagate_error_writer_to_hostfxr(set_error_writer_fn); + hostfxr_initialize_parameters parameters { + sizeof(hostfxr_initialize_parameters), + host_path.c_str(), + dotnet_root.c_str() + }; hostfxr_handle context; int rc = hostfxr_initialize_for_runtime_config(config_path.c_str(), ¶meters, &context); @@ -98,11 +96,16 @@ int load_fxr_and_get_delegate(hostfxr_delegate_type type, THostPathToConfigCallb return rc; } - } - else - { - // null context means use the current one, if none exists it will fail - return hostfxr_get_runtime_delegate(nullptr, type, delegate); + else + { + // null context means use the current one, if none exists it will fail + int rc = hostfxr_get_runtime_delegate(nullptr, type, delegate); + if (rc == StatusCode::HostInvalidState) + { + trace::error(_X("Expected active runtime context because runtimeconfig.json [%s] does not exist."), config_path.c_str()); + } + return rc; + } } } From f2f1c2e2e5b7af6d6e1ab43204e2d93f0bb6ef5d Mon Sep 17 00:00:00 2001 From: Albert Szilvasy <66087606+szilvaa-adsk@users.noreply.github.com> Date: Wed, 20 Sep 2023 14:23:19 -0700 Subject: [PATCH 04/11] Make permissive handling optional. Opt ComHost out and IjwHost in. --- src/native/corehost/comhost/comhost.cpp | 5 +++-- src/native/corehost/fxr_resolver.h | 2 +- src/native/corehost/ijwhost/ijwhost.cpp | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/native/corehost/comhost/comhost.cpp b/src/native/corehost/comhost/comhost.cpp index c5317bf31503fa..79050d4d7ff4db 100644 --- a/src/native/corehost/comhost/comhost.cpp +++ b/src/native/corehost/comhost/comhost.cpp @@ -94,7 +94,7 @@ namespace delegates.delegate_no_load_cxt = nullptr; get_function_pointer_fn get_function_pointer; - int status = load_fxr_and_get_delegate( + int status = load_fxr_and_get_delegate( hostfxr_delegate_type::hdt_get_function_pointer, [app_path](const pal::string_t& host_path, pal::string_t* config_path_out) { @@ -123,7 +123,8 @@ namespace *load_context = nullptr; // Default context } }, - reinterpret_cast(&get_function_pointer) + reinterpret_cast(&get_function_pointer), + false // do not ignore missing config file if there's an active context ); if (status != StatusCode::Success) return status; diff --git a/src/native/corehost/fxr_resolver.h b/src/native/corehost/fxr_resolver.h index bff2397968145d..ffac61567c2282 100644 --- a/src/native/corehost/fxr_resolver.h +++ b/src/native/corehost/fxr_resolver.h @@ -18,7 +18,7 @@ namespace fxr_resolver } template -int load_fxr_and_get_delegate(hostfxr_delegate_type type, THostPathToConfigCallback host_path_to_config_path, TBeforeRunCallback on_before_run, void** delegate) +int load_fxr_and_get_delegate(hostfxr_delegate_type type, THostPathToConfigCallback host_path_to_config_path, TBeforeRunCallback on_before_run, void** delegate, bool try_ignore_missing_config) { pal::dll_t fxr; diff --git a/src/native/corehost/ijwhost/ijwhost.cpp b/src/native/corehost/ijwhost/ijwhost.cpp index 7f1684cca62630..4d9925366e6b97 100644 --- a/src/native/corehost/ijwhost/ijwhost.cpp +++ b/src/native/corehost/ijwhost/ijwhost.cpp @@ -41,7 +41,8 @@ pal::hresult_t get_load_in_memory_assembly_delegate(pal::dll_t handle, load_in_m return StatusCode::Success; }, [](pal::dll_t fxr, hostfxr_handle context){ }, - reinterpret_cast(&get_function_pointer) + reinterpret_cast(&get_function_pointer), + true // ignore missing config file if there's an active context ); if (status != StatusCode::Success) return status; From 991bb3a45bbe531d2123d4c87cb427ad08919b5a Mon Sep 17 00:00:00 2001 From: Albert Szilvasy <66087606+szilvaa-adsk@users.noreply.github.com> Date: Wed, 20 Sep 2023 14:45:32 -0700 Subject: [PATCH 05/11] Update code to use the newly introduced param --- src/native/corehost/fxr_resolver.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native/corehost/fxr_resolver.h b/src/native/corehost/fxr_resolver.h index ffac61567c2282..8d6301f0befe2b 100644 --- a/src/native/corehost/fxr_resolver.h +++ b/src/native/corehost/fxr_resolver.h @@ -70,7 +70,7 @@ int load_fxr_and_get_delegate(hostfxr_delegate_type type, THostPathToConfigCallb hostfxr_set_error_writer_fn set_error_writer_fn = reinterpret_cast(pal::get_symbol(fxr, "hostfxr_set_error_writer")); { propagate_error_writer_t propagate_error_writer_to_hostfxr(set_error_writer_fn); - if (pal::file_exists(config_path)) + if (!try_ignore_missing_config || pal::file_exists(config_path)) { hostfxr_initialize_parameters parameters { sizeof(hostfxr_initialize_parameters), From 1ff63355384fe210a67ea09f5a6c98830f6dfd65 Mon Sep 17 00:00:00 2001 From: Albert Szilvasy <66087606+szilvaa-adsk@users.noreply.github.com> Date: Thu, 21 Sep 2023 16:07:24 -0700 Subject: [PATCH 06/11] Updated tests --- .../NativeHosting/Ijwhost.cs | 59 ++++++++++++++++--- .../corehost/test/nativehost/nativehost.cpp | 13 ++++ 2 files changed, 65 insertions(+), 7 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/NativeHosting/Ijwhost.cs b/src/installer/tests/HostActivation.Tests/NativeHosting/Ijwhost.cs index e7efe5eaafff84..19a5a1f0db7bc2 100644 --- a/src/installer/tests/HostActivation.Tests/NativeHosting/Ijwhost.cs +++ b/src/installer/tests/HostActivation.Tests/NativeHosting/Ijwhost.cs @@ -17,19 +17,65 @@ public class Ijwhost : IClassFixture { private readonly SharedTestState sharedState; + private string IjwLibraryRuntimeConfigPath { get; } + public Ijwhost(SharedTestState sharedTestState) { sharedState = sharedTestState; + // Create a runtimeconfig.json for the C++/CLI test library. + // This cannot be shared because some tests delete it + IjwLibraryRuntimeConfigPath = Path.ChangeExtension(sharedState.IjwLibraryPath, ".runtimeconfig.json"); + new RuntimeConfig(IjwLibraryRuntimeConfigPath) + .WithFramework(new RuntimeConfig.Framework(Constants.MicrosoftNETCoreApp, sharedState.RepoDirectories.MicrosoftNETCoreAppVersion)) + .Save(); } - [Fact] - public void LoadLibrary() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void LoadLibrary(bool no_runtimeconfig) { string [] args = { "ijwhost", sharedState.IjwLibraryPath, "NativeEntryPoint" }; + if (no_runtimeconfig) + { + File.Delete(IjwLibraryRuntimeConfigPath); + } + + CommandResult result = sharedState.CreateNativeHostCommand(args, sharedState.RepoDirectories.BuiltDotnet) + .Execute(); + + if (no_runtimeconfig) + { + result.Should().Fail() + .And.HaveStdErrContaining($"Expected active runtime context because runtimeconfig.json [{IjwLibraryRuntimeConfigPath}] does not exist."); + } + else + { + result.Should().Pass() + .And.HaveStdOutContaining("[C++/CLI] NativeEntryPoint: calling managed class") + .And.HaveStdOutContaining("[C++/CLI] ManagedClass: AssemblyLoadContext = \"Default\" System.Runtime.Loader.DefaultAssemblyLoadContext"); + } + } + + [Fact] + public void LoadLibraryWithoutRuntimeConfigButActiveRuntime() + { + // construct runtimeconfig.json + var startupConfigPath = Path.Combine(Path.GetDirectoryName(IjwLibraryRuntimeConfigPath),"host.runtimeconfig.json"); + string [] args = { + "ijwhost", + sharedState.IjwLibraryPath, + "NativeEntryPoint", + sharedState.HostFxrPath, // optional 4th and 5th arguments that tell nativehost to start the runtime before loading the C++/CLI library + startupConfigPath + }; + + File.Move(IjwLibraryRuntimeConfigPath, startupConfigPath); + CommandResult result = sharedState.CreateNativeHostCommand(args, sharedState.RepoDirectories.BuiltDotnet) .Execute(); @@ -63,6 +109,7 @@ public void ManagedHost(bool selfContained) public class SharedTestState : SharedTestStateBase { + public string HostFxrPath { get; } public string IjwLibraryPath { get; } public TestProjectFixture ManagedHostFixture_FrameworkDependent { get; } @@ -70,6 +117,9 @@ public class SharedTestState : SharedTestStateBase public SharedTestState() { + var dotNet = new Microsoft.DotNet.Cli.Build.DotNetCli(RepoDirectories.BuiltDotnet); + HostFxrPath = dotNet.GreatestVersionHostFxrFilePath; + string folder = Path.Combine(BaseDirectory, "ijw"); Directory.CreateDirectory(folder); @@ -82,11 +132,6 @@ public SharedTestState() IjwLibraryPath = Path.Combine(folder, ijwLibraryName); File.Copy(Path.Combine(RepoDirectories.HostTestArtifacts, ijwLibraryName), IjwLibraryPath); - // Create a runtimeconfig.json for the C++/CLI test library - new RuntimeConfig(Path.Combine(folder, "ijw.runtimeconfig.json")) - .WithFramework(new RuntimeConfig.Framework(Constants.MicrosoftNETCoreApp, RepoDirectories.MicrosoftNETCoreAppVersion)) - .Save(); - ManagedHostFixture_FrameworkDependent = new TestProjectFixture("ManagedHost", RepoDirectories) .EnsureRestored() .PublishProject(selfContained: false); diff --git a/src/native/corehost/test/nativehost/nativehost.cpp b/src/native/corehost/test/nativehost/nativehost.cpp index 214e2b9af9122c..97840f1369b609 100644 --- a/src/native/corehost/test/nativehost/nativehost.cpp +++ b/src/native/corehost/test/nativehost/nativehost.cpp @@ -525,6 +525,19 @@ int main(const int argc, const pal::char_t *argv[]) return -1; } + // 2 optional arguments indicating whether we should start the runtime + if (argc > 5) + { + const pal::string_t hostfxr_path = argv[4]; + const pal::char_t* config_path = argv[5]; + pal::stringstream_t test_output; + if (!host_context_test::config(host_context_test::check_properties::none, hostfxr_path, config_path, 0, nullptr, test_output)) + { + std::cout << "Failed to start runtime from path: " << tostr(hostfxr_path).data() << std::endl; + return EXIT_FAILURE; + } + } + const pal::string_t ijw_library_path = argv[2]; std::vector entry_point_name = tostr(argv[3]); From f48afe45ac7751a318dcaa949a4771048b2343a7 Mon Sep 17 00:00:00 2001 From: Albert Szilvasy <66087606+szilvaa-adsk@users.noreply.github.com> Date: Fri, 22 Sep 2023 08:18:43 -0700 Subject: [PATCH 07/11] Fix test --- src/native/corehost/test/nativehost/nativehost.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/native/corehost/test/nativehost/nativehost.cpp b/src/native/corehost/test/nativehost/nativehost.cpp index 97840f1369b609..09ec47b259b7bf 100644 --- a/src/native/corehost/test/nativehost/nativehost.cpp +++ b/src/native/corehost/test/nativehost/nativehost.cpp @@ -556,8 +556,16 @@ int main(const int argc, const pal::char_t *argv[]) std::cout << "Failed to find entry point: " << entry_point_name.data() << std::endl; return EXIT_FAILURE; } - - entry_point(); + try + { + entry_point(); + } + catch (...) + { + // entry_point will throw in some tests, this is expected. + // We must catch this exception to ensure that the CRT does not pop a modal dialog + return EXIT_FAILURE; + } return EXIT_SUCCESS; } #endif From d2ac784c23b2f72c77a36bc7fbc7a01661a86a8d Mon Sep 17 00:00:00 2001 From: Albert Szilvasy <66087606+szilvaa-adsk@users.noreply.github.com> Date: Mon, 25 Sep 2023 11:52:33 -0700 Subject: [PATCH 08/11] Roll back change that is not required --- .../tests/HostActivation.Tests/NativeHosting/Comhost.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/installer/tests/HostActivation.Tests/NativeHosting/Comhost.cs b/src/installer/tests/HostActivation.Tests/NativeHosting/Comhost.cs index dbaab41ca6671d..4d023023d9791f 100644 --- a/src/installer/tests/HostActivation.Tests/NativeHosting/Comhost.cs +++ b/src/installer/tests/HostActivation.Tests/NativeHosting/Comhost.cs @@ -141,7 +141,7 @@ public void ActivateClass_ValidateIErrorInfoResult() .Execute(); result.Should().Pass() - .And.HaveStdOutContaining($"runtimeconfig.json [{missingRuntimeConfig}] does not exist"); + .And.HaveStdOutContaining($"The specified runtimeconfig.json [{missingRuntimeConfig}] does not exist"); } } From ad99629bbaebc02980fa2295a65563dfda6dbde8 Mon Sep 17 00:00:00 2001 From: Albert Szilvasy <66087606+szilvaa-adsk@users.noreply.github.com> Date: Mon, 25 Sep 2023 12:53:28 -0700 Subject: [PATCH 09/11] Delete runtimeconfig.json This ensures consistent start point for each test --- .../tests/HostActivation.Tests/NativeHosting/Ijwhost.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/installer/tests/HostActivation.Tests/NativeHosting/Ijwhost.cs b/src/installer/tests/HostActivation.Tests/NativeHosting/Ijwhost.cs index 19a5a1f0db7bc2..169cd1e2f175f3 100644 --- a/src/installer/tests/HostActivation.Tests/NativeHosting/Ijwhost.cs +++ b/src/installer/tests/HostActivation.Tests/NativeHosting/Ijwhost.cs @@ -13,7 +13,7 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.NativeHosting { [PlatformSpecific(TestPlatforms.Windows)] // IJW is only supported on Windows - public class Ijwhost : IClassFixture + public class Ijwhost : IClassFixture, IDisposable { private readonly SharedTestState sharedState; @@ -30,6 +30,11 @@ public Ijwhost(SharedTestState sharedTestState) .Save(); } + public void Dispose() + { + File.Delete(IjwLibraryRuntimeConfigPath); + } + [Theory] [InlineData(true)] [InlineData(false)] From 63f308b3a1f4238f5b3b3eb8d9a5154a133aad76 Mon Sep 17 00:00:00 2001 From: Albert Szilvasy <66087606+szilvaa-adsk@users.noreply.github.com> Date: Thu, 28 Sep 2023 15:42:18 -0700 Subject: [PATCH 10/11] Refactor test according to review comments --- .../NativeHosting/Ijwhost.cs | 123 ++++++++++-------- 1 file changed, 70 insertions(+), 53 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/NativeHosting/Ijwhost.cs b/src/installer/tests/HostActivation.Tests/NativeHosting/Ijwhost.cs index 169cd1e2f175f3..5092eb6d9b2361 100644 --- a/src/installer/tests/HostActivation.Tests/NativeHosting/Ijwhost.cs +++ b/src/installer/tests/HostActivation.Tests/NativeHosting/Ijwhost.cs @@ -13,26 +13,13 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.NativeHosting { [PlatformSpecific(TestPlatforms.Windows)] // IJW is only supported on Windows - public class Ijwhost : IClassFixture, IDisposable + public class Ijwhost : IClassFixture { private readonly SharedTestState sharedState; - private string IjwLibraryRuntimeConfigPath { get; } - public Ijwhost(SharedTestState sharedTestState) { sharedState = sharedTestState; - // Create a runtimeconfig.json for the C++/CLI test library. - // This cannot be shared because some tests delete it - IjwLibraryRuntimeConfigPath = Path.ChangeExtension(sharedState.IjwLibraryPath, ".runtimeconfig.json"); - new RuntimeConfig(IjwLibraryRuntimeConfigPath) - .WithFramework(new RuntimeConfig.Framework(Constants.MicrosoftNETCoreApp, sharedState.RepoDirectories.MicrosoftNETCoreAppVersion)) - .Save(); - } - - public void Dispose() - { - File.Delete(IjwLibraryRuntimeConfigPath); } [Theory] @@ -40,53 +27,61 @@ public void Dispose() [InlineData(false)] public void LoadLibrary(bool no_runtimeconfig) { - string [] args = { - "ijwhost", - sharedState.IjwLibraryPath, - "NativeEntryPoint" - }; - if (no_runtimeconfig) - { - File.Delete(IjwLibraryRuntimeConfigPath); - } - - CommandResult result = sharedState.CreateNativeHostCommand(args, sharedState.RepoDirectories.BuiltDotnet) - .Execute(); - - if (no_runtimeconfig) - { - result.Should().Fail() - .And.HaveStdErrContaining($"Expected active runtime context because runtimeconfig.json [{IjwLibraryRuntimeConfigPath}] does not exist."); - } - else + // make a copy of the shared state because we will modify it + using (var testState = sharedState.Copy()) { - result.Should().Pass() - .And.HaveStdOutContaining("[C++/CLI] NativeEntryPoint: calling managed class") - .And.HaveStdOutContaining("[C++/CLI] ManagedClass: AssemblyLoadContext = \"Default\" System.Runtime.Loader.DefaultAssemblyLoadContext"); + string [] args = { + "ijwhost", + testState.IjwLibraryPath, + "NativeEntryPoint" + }; + if (no_runtimeconfig) + { + File.Delete(testState.IjwLibraryRuntimeConfigPath); + } + + CommandResult result = testState.CreateNativeHostCommand(args, testState.RepoDirectories.BuiltDotnet) + .Execute(); + + if (no_runtimeconfig) + { + result.Should().Fail() + .And.HaveStdErrContaining($"Expected active runtime context because runtimeconfig.json [{testState.IjwLibraryRuntimeConfigPath}] does not exist."); + } + else + { + result.Should().Pass() + .And.HaveStdOutContaining("[C++/CLI] NativeEntryPoint: calling managed class") + .And.HaveStdOutContaining("[C++/CLI] ManagedClass: AssemblyLoadContext = \"Default\" System.Runtime.Loader.DefaultAssemblyLoadContext"); + } } } [Fact] public void LoadLibraryWithoutRuntimeConfigButActiveRuntime() { - // construct runtimeconfig.json - var startupConfigPath = Path.Combine(Path.GetDirectoryName(IjwLibraryRuntimeConfigPath),"host.runtimeconfig.json"); - string [] args = { - "ijwhost", - sharedState.IjwLibraryPath, - "NativeEntryPoint", - sharedState.HostFxrPath, // optional 4th and 5th arguments that tell nativehost to start the runtime before loading the C++/CLI library - startupConfigPath - }; + // make a copy of the shared state because we will modify it + using (var testState = sharedState.Copy()) + { + // construct runtimeconfig.json + var startupConfigPath = Path.Combine(Path.GetDirectoryName(testState.IjwLibraryRuntimeConfigPath),"host.runtimeconfig.json"); + string [] args = { + "ijwhost", + testState.IjwLibraryPath, + "NativeEntryPoint", + testState.HostFxrPath, // optional 4th and 5th arguments that tell nativehost to start the runtime before loading the C++/CLI library + startupConfigPath + }; - File.Move(IjwLibraryRuntimeConfigPath, startupConfigPath); + File.Move(testState.IjwLibraryRuntimeConfigPath, startupConfigPath); - CommandResult result = sharedState.CreateNativeHostCommand(args, sharedState.RepoDirectories.BuiltDotnet) - .Execute(); + CommandResult result = testState.CreateNativeHostCommand(args, testState.RepoDirectories.BuiltDotnet) + .Execute(); - result.Should().Pass() - .And.HaveStdOutContaining("[C++/CLI] NativeEntryPoint: calling managed class") - .And.HaveStdOutContaining("[C++/CLI] ManagedClass: AssemblyLoadContext = \"Default\" System.Runtime.Loader.DefaultAssemblyLoadContext"); + result.Should().Pass() + .And.HaveStdOutContaining("[C++/CLI] NativeEntryPoint: calling managed class") + .And.HaveStdOutContaining("[C++/CLI] ManagedClass: AssemblyLoadContext = \"Default\" System.Runtime.Loader.DefaultAssemblyLoadContext"); + } } [Theory] @@ -116,16 +111,26 @@ public class SharedTestState : SharedTestStateBase { public string HostFxrPath { get; } public string IjwLibraryPath { get; } - + public string IjwLibraryRuntimeConfigPath { get; } public TestProjectFixture ManagedHostFixture_FrameworkDependent { get; } public TestProjectFixture ManagedHostFixture_SelfContained { get; } public SharedTestState() + :this(null) + {} + + private SharedTestState(SharedTestState other = null) { var dotNet = new Microsoft.DotNet.Cli.Build.DotNetCli(RepoDirectories.BuiltDotnet); HostFxrPath = dotNet.GreatestVersionHostFxrFilePath; - string folder = Path.Combine(BaseDirectory, "ijw"); + string folder = Path.Combine(BaseDirectory, other==null?"ijw":"ijw_copy"); + + // make sure we start with a clean slate + if (Directory.Exists(folder)) + { + Directory.Delete(folder); + } Directory.CreateDirectory(folder); // Copy over ijwhost @@ -137,6 +142,12 @@ public SharedTestState() IjwLibraryPath = Path.Combine(folder, ijwLibraryName); File.Copy(Path.Combine(RepoDirectories.HostTestArtifacts, ijwLibraryName), IjwLibraryPath); + // Create a runtimeconfig.json for the C++/CLI test library + IjwLibraryRuntimeConfigPath = Path.Combine(folder, "ijw.runtimeconfig.json"); + new RuntimeConfig(IjwLibraryRuntimeConfigPath) + .WithFramework(new RuntimeConfig.Framework(Constants.MicrosoftNETCoreApp, RepoDirectories.MicrosoftNETCoreAppVersion)) + .Save(); + ManagedHostFixture_FrameworkDependent = new TestProjectFixture("ManagedHost", RepoDirectories) .EnsureRestored() .PublishProject(selfContained: false); @@ -153,6 +164,12 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); } + + public SharedTestState Copy() + { + return new SharedTestState(this); + } + } } } From 9c3f85450430c5c08dc5793a9edf411a63165bac Mon Sep 17 00:00:00 2001 From: Albert Szilvasy <66087606+szilvaa-adsk@users.noreply.github.com> Date: Fri, 6 Oct 2023 10:58:46 -0700 Subject: [PATCH 11/11] Change tests according to review comments --- .../NativeHosting/Ijwhost.cs | 59 ++++++------------- 1 file changed, 19 insertions(+), 40 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/NativeHosting/Ijwhost.cs b/src/installer/tests/HostActivation.Tests/NativeHosting/Ijwhost.cs index 5092eb6d9b2361..ac14c328ecfaf8 100644 --- a/src/installer/tests/HostActivation.Tests/NativeHosting/Ijwhost.cs +++ b/src/installer/tests/HostActivation.Tests/NativeHosting/Ijwhost.cs @@ -27,26 +27,26 @@ public Ijwhost(SharedTestState sharedTestState) [InlineData(false)] public void LoadLibrary(bool no_runtimeconfig) { - // make a copy of the shared state because we will modify it - using (var testState = sharedState.Copy()) + // make a copy of a portion of the shared state because we will modify it + using (var app = sharedState.IjwApp.Copy()) { string [] args = { "ijwhost", - testState.IjwLibraryPath, + app.AppDll, "NativeEntryPoint" }; if (no_runtimeconfig) { - File.Delete(testState.IjwLibraryRuntimeConfigPath); + File.Delete(app.RuntimeConfigJson); } - CommandResult result = testState.CreateNativeHostCommand(args, testState.RepoDirectories.BuiltDotnet) + CommandResult result = sharedState.CreateNativeHostCommand(args, sharedState.RepoDirectories.BuiltDotnet) .Execute(); if (no_runtimeconfig) { result.Should().Fail() - .And.HaveStdErrContaining($"Expected active runtime context because runtimeconfig.json [{testState.IjwLibraryRuntimeConfigPath}] does not exist."); + .And.HaveStdErrContaining($"Expected active runtime context because runtimeconfig.json [{app.RuntimeConfigJson}] does not exist."); } else { @@ -60,22 +60,22 @@ public void LoadLibrary(bool no_runtimeconfig) [Fact] public void LoadLibraryWithoutRuntimeConfigButActiveRuntime() { - // make a copy of the shared state because we will modify it - using (var testState = sharedState.Copy()) + // make a copy of a portion of the shared state because we will modify it + using (var app = sharedState.IjwApp.Copy()) { // construct runtimeconfig.json - var startupConfigPath = Path.Combine(Path.GetDirectoryName(testState.IjwLibraryRuntimeConfigPath),"host.runtimeconfig.json"); + var startupConfigPath = Path.Combine(Path.GetDirectoryName(app.RuntimeConfigJson),"host.runtimeconfig.json"); string [] args = { "ijwhost", - testState.IjwLibraryPath, + app.AppDll, "NativeEntryPoint", - testState.HostFxrPath, // optional 4th and 5th arguments that tell nativehost to start the runtime before loading the C++/CLI library + sharedState.HostFxrPath, // optional 4th and 5th arguments that tell nativehost to start the runtime before loading the C++/CLI library startupConfigPath }; - File.Move(testState.IjwLibraryRuntimeConfigPath, startupConfigPath); + File.Move(app.RuntimeConfigJson, startupConfigPath); - CommandResult result = testState.CreateNativeHostCommand(args, testState.RepoDirectories.BuiltDotnet) + CommandResult result = sharedState.CreateNativeHostCommand(args, sharedState.RepoDirectories.BuiltDotnet) .Execute(); result.Should().Pass() @@ -91,7 +91,7 @@ public void ManagedHost(bool selfContained) { string [] args = { "ijwhost", - sharedState.IjwLibraryPath, + sharedState.IjwApp.AppDll, "NativeEntryPoint" }; TestProjectFixture fixture = selfContained ? sharedState.ManagedHostFixture_SelfContained : sharedState.ManagedHostFixture_FrameworkDependent; @@ -110,41 +110,26 @@ public void ManagedHost(bool selfContained) public class SharedTestState : SharedTestStateBase { public string HostFxrPath { get; } - public string IjwLibraryPath { get; } - public string IjwLibraryRuntimeConfigPath { get; } public TestProjectFixture ManagedHostFixture_FrameworkDependent { get; } public TestProjectFixture ManagedHostFixture_SelfContained { get; } + public TestApp IjwApp {get;} public SharedTestState() - :this(null) - {} - - private SharedTestState(SharedTestState other = null) { var dotNet = new Microsoft.DotNet.Cli.Build.DotNetCli(RepoDirectories.BuiltDotnet); HostFxrPath = dotNet.GreatestVersionHostFxrFilePath; - - string folder = Path.Combine(BaseDirectory, other==null?"ijw":"ijw_copy"); - - // make sure we start with a clean slate - if (Directory.Exists(folder)) - { - Directory.Delete(folder); - } - Directory.CreateDirectory(folder); - + string folder = Path.Combine(BaseDirectory, "ijw"); + IjwApp = new TestApp(folder, "ijw"); // Copy over ijwhost string ijwhostName = "ijwhost.dll"; File.Copy(Path.Combine(RepoDirectories.HostArtifacts, ijwhostName), Path.Combine(folder, ijwhostName)); // Copy over the C++/CLI test library string ijwLibraryName = "ijw.dll"; - IjwLibraryPath = Path.Combine(folder, ijwLibraryName); - File.Copy(Path.Combine(RepoDirectories.HostTestArtifacts, ijwLibraryName), IjwLibraryPath); + File.Copy(Path.Combine(RepoDirectories.HostTestArtifacts, ijwLibraryName), Path.Combine(folder, ijwLibraryName)); // Create a runtimeconfig.json for the C++/CLI test library - IjwLibraryRuntimeConfigPath = Path.Combine(folder, "ijw.runtimeconfig.json"); - new RuntimeConfig(IjwLibraryRuntimeConfigPath) + new RuntimeConfig(Path.Combine(folder, "ijw.runtimeconfig.json")) .WithFramework(new RuntimeConfig.Framework(Constants.MicrosoftNETCoreApp, RepoDirectories.MicrosoftNETCoreAppVersion)) .Save(); @@ -164,12 +149,6 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); } - - public SharedTestState Copy() - { - return new SharedTestState(this); - } - } } }