Miscellaneous MetadataUpdater cleanups (#54751)

diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs
index e09eaa0..c2f06b8 100644 (file)
--- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs
+++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs
@@ -14,10 +14,9 @@ namespace System.Reflection.Metadata
     /// testsuite runs each test in sequence, loading the corresponding
     /// assembly, applying an update to it and observing the results.
     [Collection(nameof(ApplyUpdateUtil.NoParallelTests))]
-    [ConditionalClass(typeof(ApplyUpdateUtil), nameof (ApplyUpdateUtil.IsSupported))]
     public class ApplyUpdateTest
     {
-        [Fact]
+        [ConditionalFact(typeof(ApplyUpdateUtil), nameof (ApplyUpdateUtil.IsSupported))]
         [ActiveIssue("https://github.com/dotnet/runtime/issues/54617", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))]
         void StaticMethodBodyUpdate()
         {
@@ -40,7 +39,7 @@ namespace System.Reflection.Metadata
             });
         }
 
-        [Fact]
+        [ConditionalFact(typeof(ApplyUpdateUtil), nameof (ApplyUpdateUtil.IsSupported))]
         [ActiveIssue("https://github.com/dotnet/runtime/issues/52993", TestRuntimes.Mono)]
         void ClassWithCustomAttributes()
         {
@@ -122,12 +121,18 @@ namespace System.Reflection.Metadata
             Assert.Equal(typeof(string), result.GetType());
         }
 
-        [Fact]
-        [ActiveIssue("Returns true on mono", TestRuntimes.Mono)]
+        [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.TestUsingRemoteExecutor))]
         public static void IsSupported()
         {
             bool result = MetadataUpdater.IsSupported;
             Assert.False(result);
         }
+
+        [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.TestUsingLaunchEnvironment))]
+        public static void IsSupported2()
+        {
+            bool result = MetadataUpdater.IsSupported;
+            Assert.True(result);
+        }
     }
 }

diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs
index 235e94b..36d04c4 100644 (file)
--- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs
+++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs
@@ -19,20 +19,27 @@ namespace System.Reflection.Metadata
         ///
         /// We need:
         /// 1. Either DOTNET_MODIFIABLE_ASSEMBLIES=debug is set, or we can use the RemoteExecutor to run a child process with that environment; and,
-        /// 2. Either Mono in a supported configuration (interpreter as the execution engine, and the hot reload feature enabled), or CoreCLR; and,
+        /// 2. Either Mono in a supported configuration (interpreter as the execution engine, and the hot reload component enabled), or CoreCLR; and,
         /// 3. The test assemblies are compiled with Debug information (this is configured by setting EmitDebugInformation in ApplyUpdate\Directory.Build.props)
         public static bool IsSupported => (IsModifiableAssembliesSet || IsRemoteExecutorSupported) &&
             (!IsMonoRuntime || IsSupportedMonoConfiguration);
 
+        /// true if the current runtime was not launched with the apropriate settings for applying
+        /// updates (DOTNET_MODIFIABLE_ASSEMBLIES unset), but we can use the remote executor to
+        /// launch a child process that has the right setting.
+        public static bool TestUsingRemoteExecutor => IsRemoteExecutorSupported && !IsModifiableAssembliesSet;
+
+        /// true if the current runtime was launched with the appropriate settings for applying
+        /// updates (DOTNET_MODIFIABLE_ASSEMBLIES set, and if Mono, the interpreter is enabled).
+        public static bool TestUsingLaunchEnvironment => (!IsMonoRuntime || IsSupportedMonoConfiguration) && IsModifiableAssembliesSet;
+
         public static bool IsModifiableAssembliesSet =>
             String.Equals(DotNetModifiableAssembliesValue, Environment.GetEnvironmentVariable(DotNetModifiableAssembliesSwitch), StringComparison.InvariantCultureIgnoreCase);
 
         // static cctor for RemoteExecutor throws on wasm.
         public static bool IsRemoteExecutorSupported => !RuntimeInformation.IsOSPlatform(OSPlatform.Create("BROWSER")) && RemoteExecutor.IsSupported;
 
-        // copied from https://github.com/dotnet/arcade/blob/6cc4c1e9e23d5e65e88a8a57216b3d91e9b3d8db/src/Microsoft.DotNet.XUnitExtensions/src/DiscovererHelpers.cs#L16-L17
-        private static readonly Lazy<bool> s_isMonoRuntime = new Lazy<bool>(() => Type.GetType("Mono.RuntimeStructs") != null);
-        public static bool IsMonoRuntime => s_isMonoRuntime.Value;
+        public static bool IsMonoRuntime => PlatformDetection.IsMonoRuntime;
 
         private static readonly Lazy<bool> s_isSupportedMonoConfiguration = new Lazy<bool>(CheckSupportedMonoConfiguration);
 
@@ -41,7 +48,7 @@ namespace System.Reflection.Metadata
         // Not every build of Mono supports ApplyUpdate
         internal static bool CheckSupportedMonoConfiguration()
         {
-        // check that interpreter is enabled, and the build has hot reload capabilities enabled.
+            // check that interpreter is enabled, and the build has hot reload capabilities enabled.
             var isInterp = RuntimeFeature.IsDynamicCodeSupported && !RuntimeFeature.IsDynamicCodeCompiled;
             return isInterp && HasApplyUpdateCapabilities();
         }
@@ -75,7 +82,7 @@ namespace System.Reflection.Metadata
             string basename = assm.Location;
             if (basename == "")
                 basename = assm.GetName().Name + ".dll";
-            Console.Error.WriteLine($"Apply Delta Update for {basename}, revision {count}");
+            Console.Error.WriteLine($"Applying metadata update for {basename}, revision {count}");
 
             string dmeta_name = $"{basename}.{count}.dmeta";
             string dil_name = $"{basename}.{count}.dil";
@@ -86,8 +93,6 @@ namespace System.Reflection.Metadata
             MetadataUpdater.ApplyUpdate(assm, dmeta_data, dil_data, dpdb_data);
         }
 
-        internal static bool UseRemoteExecutor => !IsModifiableAssembliesSet;
-
         internal static void AddRemoteInvokeOptions (ref RemoteInvokeOptions options)
         {
             options = options ?? new RemoteInvokeOptions();
@@ -101,7 +106,7 @@ namespace System.Reflection.Metadata
         public static void TestCase(Action testBody,
                                     RemoteInvokeOptions options = null)
         {
-            if (UseRemoteExecutor)
+            if (TestUsingRemoteExecutor)
             {
                 Console.Error.WriteLine ($"Running test using RemoteExecutor");
                 AddRemoteInvokeOptions(ref options);
@@ -123,7 +128,7 @@ namespace System.Reflection.Metadata
                                     string arg1,
                                     RemoteInvokeOptions options = null)
         {
-            if (UseRemoteExecutor)
+            if (TestUsingRemoteExecutor)
             {
                 AddRemoteInvokeOptions(ref options);
                 RemoteExecutor.Invoke(testBody, arg1, options).Dispose();

diff --git a/src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs b/src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs
index ea7035e..973eb43 100644 (file)
--- a/src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs
+++ b/src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs
@@ -49,17 +49,17 @@ namespace System.Reflection.Metadata
 
         internal static string GetCapabilities() => s_ApplyUpdateCapabilities.Value;
 
-        public static bool IsSupported { get; } = ApplyUpdateEnabled() != 0;
+        public static bool IsSupported { get; } = ApplyUpdateEnabled(justComponentCheck: 0) != 0;
 
         private static Lazy<string> s_ApplyUpdateCapabilities = new Lazy<string>(() => InitializeApplyUpdateCapabilities());
 
         private static string InitializeApplyUpdateCapabilities()
         {
-            return ApplyUpdateEnabled() != 0 ? "Baseline" : string.Empty ;
+            return ApplyUpdateEnabled(justComponentCheck: 1) != 0 ? "Baseline" : string.Empty ;
         }
 
         [MethodImpl (MethodImplOptions.InternalCall)]
-        private static extern int ApplyUpdateEnabled ();
+        private static extern int ApplyUpdateEnabled (int justComponentCheck);
 
         [MethodImpl (MethodImplOptions.InternalCall)]
         private static unsafe extern void ApplyUpdate_internal (IntPtr base_assm, byte* dmeta_bytes, int dmeta_length, byte *dil_bytes, int dil_length, byte *dpdb_bytes, int dpdb_length);

diff --git a/src/mono/mono/metadata/icall-decl.h b/src/mono/mono/metadata/icall-decl.h
index b78d7c6..d6d052b 100644 (file)
--- a/src/mono/mono/metadata/icall-decl.h
+++ b/src/mono/mono/metadata/icall-decl.h
@@ -243,6 +243,6 @@ ICALL_EXPORT void ves_icall_System_Runtime_Intrinsics_X86_X86Base___cpuidex (int
 #endif
 
 ICALL_EXPORT void ves_icall_AssemblyExtensions_ApplyUpdate (MonoAssembly *assm, gconstpointer dmeta_bytes, int32_t dmeta_len, gconstpointer dil_bytes, int32_t dil_len, gconstpointer dpdb_bytes, int32_t dpdb_len);
-ICALL_EXPORT gint32 ves_icall_AssemblyExtensions_ApplyUpdateEnabled (void);
+ICALL_EXPORT gint32 ves_icall_AssemblyExtensions_ApplyUpdateEnabled (gint32 just_component_check);
 
 #endif // __MONO_METADATA_ICALL_DECL_H__

diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c
index c6e3905..51a8fc0 100644 (file)
--- a/src/mono/mono/metadata/icall.c
+++ b/src/mono/mono/metadata/icall.c
@@ -5787,9 +5787,11 @@ ves_icall_AssemblyExtensions_ApplyUpdate (MonoAssembly *assm,
        mono_error_set_pending_exception (error);
 }
 
-gint32 ves_icall_AssemblyExtensions_ApplyUpdateEnabled (void)
+gint32 ves_icall_AssemblyExtensions_ApplyUpdateEnabled (gint32 just_component_check)
 {
-       return mono_metadata_update_available () && mono_metadata_update_enabled (NULL);
+       // if just_component_check is true, we only care whether the hot_reload component is enabled,
+       // not whether the environment is appropriately setup to apply updates.
+       return mono_metadata_update_available () && (just_component_check || mono_metadata_update_enabled (NULL));
 }
 
 MonoBoolean