From 9b56e737ef9910fcb3b6e80f2bef795459c6982c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksey=20Kliger=20=28=CE=BBgeek=29?= Date: Thu, 26 Sep 2024 18:29:04 -0400 Subject: [PATCH 1/6] [cdac-build-tool] don't embed resources (#108293) reviously we used EmbeddedResource to collect the contract descriptor baselines into cdac-build-tool and we also embedded the C source template. But we only ever run cdac-build-tool during the runtime build - the files are in the same source tree. Just pass their paths when invoking the tool, don't embed them. Fixes #107958 * don't embed baselines into cdac-build-tool * don't embed the template source file in the tool --- src/coreclr/debug/runtimeinfo/CMakeLists.txt | 6 ++- .../runtimeinfo}/contract-descriptor.c.in | 0 .../tools/cdac-build-tool/ComposeCommand.cs | 32 +++++++++++- .../ContractDescriptorSourceFileEmitter.cs | 17 +++--- .../cdac-build-tool/DataDescriptorModel.cs | 7 ++- .../cdac-build-tool/DirectoryBaselines.cs | 33 ++++++++++++ .../cdac-build-tool/EmbeddedBaselines.cs | 52 ------------------- .../cdac-build-tool/cdac-build-tool.csproj | 10 ---- 8 files changed, 80 insertions(+), 77 deletions(-) rename src/coreclr/{tools/cdac-build-tool/Resources => debug/runtimeinfo}/contract-descriptor.c.in (100%) create mode 100644 src/coreclr/tools/cdac-build-tool/DirectoryBaselines.cs delete mode 100644 src/coreclr/tools/cdac-build-tool/EmbeddedBaselines.cs diff --git a/src/coreclr/debug/runtimeinfo/CMakeLists.txt b/src/coreclr/debug/runtimeinfo/CMakeLists.txt index 77ecf9a4dd9ba..018e49645cd0d 100644 --- a/src/coreclr/debug/runtimeinfo/CMakeLists.txt +++ b/src/coreclr/debug/runtimeinfo/CMakeLists.txt @@ -67,7 +67,9 @@ else() if(NOT EXISTS "${CDAC_BUILD_TOOL_BINARY_PATH}") message(FATAL_ERROR "${CDAC_BUILD_TOOL_BINARY_PATH} does not exist") endif() + set(CONTRACT_DESCRIPTOR_INPUT "${CMAKE_CURRENT_SOURCE_DIR}/contract-descriptor.c.in") + set(CONTRACT_BASELINE_DIR "${CLR_REPO_ROOT_DIR}/docs/design/datacontracts/data") set(CONTRACT_FILE "${CMAKE_CURRENT_SOURCE_DIR}/contracts.jsonc") # generate the contract descriptor by running cdac-build-tool @@ -75,8 +77,8 @@ else() add_custom_command( OUTPUT "${CONTRACT_DESCRIPTOR_OUTPUT}" VERBATIM - COMMAND ${CLR_DOTNET_HOST_PATH} ${CDAC_BUILD_TOOL_BINARY_PATH} compose -o "${CONTRACT_DESCRIPTOR_OUTPUT}" -c "${CONTRACT_FILE}" $ - DEPENDS cdac_data_descriptor cee_wks_core $ "${CONTRACT_FILE}" + COMMAND ${CLR_DOTNET_HOST_PATH} ${CDAC_BUILD_TOOL_BINARY_PATH} compose -i "${CONTRACT_DESCRIPTOR_INPUT}" -o "${CONTRACT_DESCRIPTOR_OUTPUT}" -b "${CONTRACT_BASELINE_DIR}" -c "${CONTRACT_FILE}" $ + DEPENDS cdac_data_descriptor cee_wks_core $ "${CONTRACT_FILE}" "${CONTRACT_DESCRIPTOR_INPUT}" USES_TERMINAL ) diff --git a/src/coreclr/tools/cdac-build-tool/Resources/contract-descriptor.c.in b/src/coreclr/debug/runtimeinfo/contract-descriptor.c.in similarity index 100% rename from src/coreclr/tools/cdac-build-tool/Resources/contract-descriptor.c.in rename to src/coreclr/debug/runtimeinfo/contract-descriptor.c.in diff --git a/src/coreclr/tools/cdac-build-tool/ComposeCommand.cs b/src/coreclr/tools/cdac-build-tool/ComposeCommand.cs index 226244a88872a..7305bcd08808c 100644 --- a/src/coreclr/tools/cdac-build-tool/ComposeCommand.cs +++ b/src/coreclr/tools/cdac-build-tool/ComposeCommand.cs @@ -13,6 +13,8 @@ internal sealed class ComposeCommand : CliCommand private readonly CliArgument inputFiles = new("INPUT [INPUTS...]") { Arity = ArgumentArity.OneOrMore, Description = "One or more input files" }; private readonly CliOption outputFile = new("-o") { Arity = ArgumentArity.ExactlyOne, HelpName = "OUTPUT", Required = true, Description = "Output file" }; private readonly CliOption contractFile = new("-c") { Arity = ArgumentArity.ZeroOrMore, HelpName = "CONTRACT", Description = "Contract file (may be specified multiple times)" }; + private readonly CliOption baselinePath = new("-b", "--baseline") { Arity = ArgumentArity.ExactlyOne, HelpName = "BASELINEPATH", Description = "Directory containing the baseline contracts"}; + private readonly CliOption templateFile = new ("-i", "--input-template") { Arity = ArgumentArity.ExactlyOne, HelpName = "TEMPLATE", Description = "Contract descriptor template to be filled in" }; private readonly CliOption _verboseOption; public ComposeCommand(CliOption verboseOption) : base("compose") { @@ -20,6 +22,8 @@ public ComposeCommand(CliOption verboseOption) : base("compose") Add(inputFiles); Add(outputFile); Add(contractFile); + Add(baselinePath); + Add(templateFile); SetAction(Run); } @@ -37,9 +41,33 @@ private async Task Run(ParseResult parse, CancellationToken token = default Console.Error.WriteLine("No output file specified"); return 1; } + var baselinesDir = parse.GetValue(baselinePath); + if (baselinesDir == null) + { + Console.Error.WriteLine("No baseline path specified"); + return 1; + } + baselinesDir = System.IO.Path.GetFullPath(baselinesDir); + if (!System.IO.Directory.Exists(baselinesDir)) + { + Console.Error.WriteLine($"Baseline path {baselinesDir} does not exist"); + return 1; + } + var templateFilePath = parse.GetValue(templateFile); + if (templateFilePath == null) + { + Console.Error.WriteLine("No template file specified"); + return 1; + } + templateFilePath = System.IO.Path.GetFullPath(templateFilePath); + if (!System.IO.File.Exists(templateFilePath)) + { + Console.Error.WriteLine($"Template file {templateFilePath} does not exist"); + return 1; + } var contracts = parse.GetValue(contractFile); var verbose = parse.GetValue(_verboseOption); - var builder = new DataDescriptorModel.Builder(); + var builder = new DataDescriptorModel.Builder(baselinesDir); var scraper = new ObjectFileScraper(verbose, builder); foreach (var input in inputs) { @@ -70,7 +98,7 @@ private async Task Run(ParseResult parse, CancellationToken token = default } EnsureDirectoryExists(output); using var writer = new System.IO.StreamWriter(output); - var emitter = new ContractDescriptorSourceFileEmitter(); + var emitter = new ContractDescriptorSourceFileEmitter(templateFilePath); emitter.SetPlatformFlags(model.PlatformFlags); emitter.SetPointerDataCount(model.PointerDataCount); emitter.SetJsonDescriptor(model.ToJson()); diff --git a/src/coreclr/tools/cdac-build-tool/ContractDescriptorSourceFileEmitter.cs b/src/coreclr/tools/cdac-build-tool/ContractDescriptorSourceFileEmitter.cs index dde27a6858c6d..02927cb1ff84e 100644 --- a/src/coreclr/tools/cdac-build-tool/ContractDescriptorSourceFileEmitter.cs +++ b/src/coreclr/tools/cdac-build-tool/ContractDescriptorSourceFileEmitter.cs @@ -10,25 +10,24 @@ namespace Microsoft.DotNet.Diagnostics.DataContract.BuildTool; public partial class ContractDescriptorSourceFileEmitter { - public const string TemplateResourceName = "Microsoft.DotNet.Diagnostics.DataContract.Resources.contract-descriptor.c.in"; private const string JsonDescriptorKey = "jsonDescriptor"; private const string JsonDescriptorSizeKey = "jsonDescriptorSize"; private const string PointerDataCount = "pointerDataCount"; private const string PlatformFlags = "platformFlags"; + private readonly string _templateFilePath; - [GeneratedRegex("%%([a-zA-Z0-9_]+)%%", RegexOptions.CultureInvariant)] - private static partial Regex FindTemplatePlaceholderRegex(); - - internal static Stream GetTemplateStream() + public ContractDescriptorSourceFileEmitter(string templateFilePath) { - return typeof(ContractDescriptorSourceFileEmitter).Assembly.GetManifestResourceStream(TemplateResourceName)!; + _templateFilePath = templateFilePath; } - internal static string GetTemplateString() + [GeneratedRegex("%%([a-zA-Z0-9_]+)%%", RegexOptions.CultureInvariant)] + private static partial Regex FindTemplatePlaceholderRegex(); + + private string GetTemplateString() { - using var reader = new StreamReader(GetTemplateStream(), System.Text.Encoding.UTF8); - return reader.ReadToEnd(); + return File.ReadAllText(_templateFilePath); } public void SetPointerDataCount(int count) diff --git a/src/coreclr/tools/cdac-build-tool/DataDescriptorModel.cs b/src/coreclr/tools/cdac-build-tool/DataDescriptorModel.cs index d9d802d359978..55aa16283c4fa 100644 --- a/src/coreclr/tools/cdac-build-tool/DataDescriptorModel.cs +++ b/src/coreclr/tools/cdac-build-tool/DataDescriptorModel.cs @@ -84,14 +84,16 @@ public string ToJson() public class Builder { private string _baseline; + private readonly string _baselinesDir; private bool _baselineParsed; private readonly Dictionary _types = new(); private readonly Dictionary _globals = new(); private readonly Dictionary _contracts = new(); - public Builder() + public Builder(string baselinesDir) { _baseline = string.Empty; _baselineParsed = false; + _baselinesDir = baselinesDir; } public uint PlatformFlags {get; set;} @@ -151,7 +153,8 @@ public void SetBaseline(string baseline) { throw new InvalidOperationException($"Baseline already set to {_baseline} cannot set to {baseline}"); } - if (EmbeddedBaselines.BaselineNames.Contains(baseline)) + var baselines = new DirectoryBaselines(_baselinesDir); + if (baselines.BaselineNames.Contains(baseline)) { _baseline = baseline; } diff --git a/src/coreclr/tools/cdac-build-tool/DirectoryBaselines.cs b/src/coreclr/tools/cdac-build-tool/DirectoryBaselines.cs new file mode 100644 index 0000000000000..8891f8f88c8e5 --- /dev/null +++ b/src/coreclr/tools/cdac-build-tool/DirectoryBaselines.cs @@ -0,0 +1,33 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.IO; + +namespace Microsoft.DotNet.Diagnostics.DataContract.BuildTool; + +public class DirectoryBaselines +{ + private const string BaselineGlob = "*.json*"; // .json and .jsonc + + private readonly string _baselinesDir; + + public DirectoryBaselines(string baselinesDir) + { + _baselinesDir = baselinesDir; + } + + public string[] BaselineNames => GetBaselineNames(_baselinesDir); + + private static string[] GetBaselineNames(string baselineDir) + { + + var baselineNames = new List(); + foreach (var file in Directory.EnumerateFiles(baselineDir, BaselineGlob)) + { + var name = Path.GetFileNameWithoutExtension(file); + baselineNames.Add(name); + } + return baselineNames.ToArray(); + } +} diff --git a/src/coreclr/tools/cdac-build-tool/EmbeddedBaselines.cs b/src/coreclr/tools/cdac-build-tool/EmbeddedBaselines.cs deleted file mode 100644 index ac9cea8b6615e..0000000000000 --- a/src/coreclr/tools/cdac-build-tool/EmbeddedBaselines.cs +++ /dev/null @@ -1,52 +0,0 @@ -// 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 System.Collections.Generic; -using System.IO; -using System.Text.RegularExpressions; - -namespace Microsoft.DotNet.Diagnostics.DataContract.BuildTool; - -public partial class EmbeddedBaselines -{ - public const string TemplateResourceNamePrefix = "Microsoft.DotNet.Diagnostics.DataContract.Baseline:"; - public const string TemplateResourceNameEscapePrefix = @"Microsoft\.DotNet\.Diagnostics\.DataContract\.Baseline:"; - public const string TemplateResourceNameExt = ".jsonc"; - public const string TemplateResourceNameEscapeExt = @"\.jsonc"; - - [GeneratedRegex("^" + TemplateResourceNameEscapePrefix + "(.+)" + TemplateResourceNameEscapeExt + "$", RegexOptions.CultureInvariant)] - private static partial Regex BaselineRegex(); - - private static string[] GetBaselineNames() - { - var assembly = typeof(EmbeddedBaselines).Assembly; - var resources = assembly.GetManifestResourceNames(); - var baselineNames = new List(); - foreach (var resource in resources) - { - var match = BaselineRegex().Match(resource); - if (match.Success) - { - baselineNames.Add(match.Groups[1].Value); - } - } - return baselineNames.ToArray(); - } - - private static readonly Lazy> _baselineNames = new(GetBaselineNames); - public static IReadOnlyList BaselineNames => _baselineNames.Value; - - public static string GetBaselineContent(string name) - { - var assembly = typeof(EmbeddedBaselines).Assembly; - var resourceName = TemplateResourceNamePrefix + name + TemplateResourceNameExt; - using var stream = assembly.GetManifestResourceStream(resourceName); - if (stream == null) - { - throw new InvalidOperationException($"Baseline '{name}' not found"); - } - using var reader = new StreamReader(stream); - return reader.ReadToEnd(); - } -} diff --git a/src/coreclr/tools/cdac-build-tool/cdac-build-tool.csproj b/src/coreclr/tools/cdac-build-tool/cdac-build-tool.csproj index d7bbad0cdea22..976fbb4141e1d 100644 --- a/src/coreclr/tools/cdac-build-tool/cdac-build-tool.csproj +++ b/src/coreclr/tools/cdac-build-tool/cdac-build-tool.csproj @@ -19,14 +19,4 @@ - - - Microsoft.DotNet.Diagnostics.DataContract.Baseline: - - - - - - - From c20bdf6de579d42faa4d019c8332edbeef618c44 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 26 Sep 2024 17:00:29 -0700 Subject: [PATCH 2/6] Remove HelperMethodFrame from `Delegate` construction (#108217) * Remove HelperMethodFrame from Delegate construction --- .../src/System/Delegate.CoreCLR.cs | 17 ++- src/coreclr/debug/daccess/dacdbiimpl.cpp | 2 +- src/coreclr/vm/comdelegate.cpp | 141 ++++++++---------- src/coreclr/vm/comdelegate.h | 17 +-- src/coreclr/vm/ecall.cpp | 14 +- src/coreclr/vm/ecalllist.h | 5 - src/coreclr/vm/prestub.cpp | 2 +- src/coreclr/vm/qcallentrypoints.cpp | 1 + 8 files changed, 85 insertions(+), 114 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs index ee48dc77e94fe..664f218be6633 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs @@ -478,8 +478,21 @@ internal static unsafe bool InternalEqualTypes(object a, object b) // Used by the ctor. Do not call directly. // The name of this function will appear in managed stacktraces as delegate constructor. - [MethodImpl(MethodImplOptions.InternalCall)] - private extern void DelegateConstruct(object target, IntPtr slot); + private void DelegateConstruct(object target, IntPtr method) + { + // Via reflection you can pass in just about any value for the method. + // We can do some basic verification up front to prevent EE exceptions. + if (method == IntPtr.Zero) + { + throw new ArgumentNullException(nameof(method)); + } + + Delegate _this = this; + Construct(ObjectHandleOnStack.Create(ref _this), ObjectHandleOnStack.Create(ref target), method); + } + + [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "Delegate_Construct")] + private static partial void Construct(ObjectHandleOnStack _this, ObjectHandleOnStack target, IntPtr method); [MethodImpl(MethodImplOptions.InternalCall)] private static extern unsafe void* GetMulticastInvoke(MethodTable* pMT); diff --git a/src/coreclr/debug/daccess/dacdbiimpl.cpp b/src/coreclr/debug/daccess/dacdbiimpl.cpp index 0c9750c0282cb..770e81a9480e1 100644 --- a/src/coreclr/debug/daccess/dacdbiimpl.cpp +++ b/src/coreclr/debug/daccess/dacdbiimpl.cpp @@ -3391,7 +3391,7 @@ HRESULT DacDbiInterfaceImpl::GetDelegateType(VMPTR_Object delegateObject, Delega // - System.Private.CoreLib!System.Delegate.GetMethodImpl and System.Private.CoreLib!System.MulticastDelegate.GetMethodImpl // - System.Private.CoreLib!System.Delegate.GetTarget and System.Private.CoreLib!System.MulticastDelegate.GetTarget // - coreclr!COMDelegate::GetMethodDesc and coreclr!COMDelegate::FindMethodHandle - // - coreclr!COMDelegate::DelegateConstruct and the delegate type table in + // - coreclr!Delegate_Construct and the delegate type table in // - DELEGATE KINDS TABLE in comdelegate.cpp *delegateType = DelegateType::kUnknownDelegateType; diff --git a/src/coreclr/vm/comdelegate.cpp b/src/coreclr/vm/comdelegate.cpp index 59a55e2b1cd3d..b3dcb78a7e032 100644 --- a/src/coreclr/vm/comdelegate.cpp +++ b/src/coreclr/vm/comdelegate.cpp @@ -726,12 +726,10 @@ VOID GenerateShuffleArray(MethodDesc* pInvoke, MethodDesc *pTargetMeth, SArrayFPtr pairs + // passed out to unmanaged code. // One time init. void COMDelegate::Init() @@ -748,10 +746,10 @@ void COMDelegate::Init() s_pDelegateToFPtrHash = ::new PtrHashMap(); - LockOwner lock = {&COMDelegate::s_DelegateToFPtrHashCrst, IsOwnerOfCrst}; + LockOwner lock = {&s_DelegateToFPtrHashCrst, IsOwnerOfCrst}; s_pDelegateToFPtrHash->Init(TRUE, &lock); - m_pShuffleThunkCache = new ShuffleThunkCache(SystemDomain::GetGlobalLoaderAllocator()->GetStubHeap()); + s_pShuffleThunkCache = new ShuffleThunkCache(SystemDomain::GetGlobalLoaderAllocator()->GetStubHeap()); } #ifdef FEATURE_COMINTEROP @@ -792,7 +790,7 @@ LoaderHeap *DelegateEEClass::GetStubHeap() } -Stub* COMDelegate::SetupShuffleThunk(MethodTable * pDelMT, MethodDesc *pTargetMeth) +static Stub* SetupShuffleThunk(MethodTable* pDelMT, MethodDesc* pTargetMeth) { CONTRACTL { @@ -812,7 +810,7 @@ Stub* COMDelegate::SetupShuffleThunk(MethodTable * pDelMT, MethodDesc *pTargetMe StackSArray rShuffleEntryArray; GenerateShuffleArray(pMD, pTargetMeth, &rShuffleEntryArray); - ShuffleThunkCache* pShuffleThunkCache = m_pShuffleThunkCache; + ShuffleThunkCache* pShuffleThunkCache = s_pShuffleThunkCache; LoaderAllocator* pLoaderAllocator = pDelMT->GetLoaderAllocator(); if (pLoaderAllocator->IsCollectible()) @@ -848,8 +846,6 @@ Stub* COMDelegate::SetupShuffleThunk(MethodTable * pDelMT, MethodDesc *pTargetMe return pShuffleThunk; } - - static PCODE GetVirtualCallStub(MethodDesc *method, TypeHandle scopeType) { CONTRACTL @@ -1299,11 +1295,11 @@ LPVOID COMDelegate::ConvertToCallback(OBJECTREF pDelegateObj) LPVOID key = (LPVOID)pUMEntryThunk; // Assert that the entry isn't already in the hash. - _ASSERTE((LPVOID)INVALIDENTRY == COMDelegate::s_pDelegateToFPtrHash->LookupValue((UPTR)key, 0)); + _ASSERTE((LPVOID)INVALIDENTRY == s_pDelegateToFPtrHash->LookupValue((UPTR)key, 0)); { - CrstHolder ch(&COMDelegate::s_DelegateToFPtrHashCrst); - COMDelegate::s_pDelegateToFPtrHash->InsertValue((UPTR)key, pUMEntryThunk->GetObjectHandle()); + CrstHolder ch(&s_DelegateToFPtrHashCrst); + s_pDelegateToFPtrHash->InsertValue((UPTR)key, pUMEntryThunk->GetObjectHandle()); } } @@ -1342,7 +1338,7 @@ OBJECTREF COMDelegate::ConvertToDelegate(LPVOID pCallback, MethodTable* pMT) // Otherwise, we'll treat this as an unmanaged callsite. // Make sure that the pointer doesn't have the value of 1 which is our hash table deleted item marker. LPVOID DelegateHnd = (pUMEntryThunk != NULL) && ((UPTR)pUMEntryThunk != (UPTR)1) - ? COMDelegate::s_pDelegateToFPtrHash->LookupValue((UPTR)pUMEntryThunk, 0) + ? s_pDelegateToFPtrHash->LookupValue((UPTR)pUMEntryThunk, 0) : (LPVOID)INVALIDENTRY; if (DelegateHnd != (LPVOID)INVALIDENTRY) @@ -1476,8 +1472,8 @@ void COMDelegate::RemoveEntryFromFPtrHash(UPTR key) WRAPPER_NO_CONTRACT; // Remove this entry from the lookup hash. - CrstHolder ch(&COMDelegate::s_DelegateToFPtrHashCrst); - COMDelegate::s_pDelegateToFPtrHash->DeleteValue(key, NULL); + CrstHolder ch(&s_DelegateToFPtrHashCrst); + s_pDelegateToFPtrHash->DeleteValue(key, NULL); } extern "C" void QCALLTYPE Delegate_InitializeVirtualCallStub(QCall::ObjectHandleOnStack d, PCODE method) @@ -1566,114 +1562,97 @@ uint32_t MethodDescToNumFixedArgs(MethodDesc *pMD) return data; } -// This is the single constructor for all Delegates. The compiler -// doesn't provide an implementation of the Delegate constructor. We -// provide that implementation through an ECall call to this method. -FCIMPL3(void, COMDelegate::DelegateConstruct, Object* refThisUNSAFE, Object* targetUNSAFE, PCODE method) +// This is the single constructor for all Delegates. The compiler +// doesn't provide an implementation of the Delegate constructor. We +// provide that implementation through a QCall call to this method. +extern "C" void QCALLTYPE Delegate_Construct(QCall::ObjectHandleOnStack _this, QCall::ObjectHandleOnStack target, PCODE method) { - FCALL_CONTRACT; + QCALL_CONTRACT; + // If you modify this logic, please update DacDbiInterfaceImpl::GetDelegateType, DacDbiInterfaceImpl::GetDelegateType, // DacDbiInterfaceImpl::GetDelegateFunctionData, and DacDbiInterfaceImpl::GetDelegateTargetObject. + _ASSERTE(method != (PCODE)NULL); + BEGIN_QCALL; - struct _gc - { - DELEGATEREF refThis; - OBJECTREF target; - } gc; - - gc.refThis = (DELEGATEREF) ObjectToOBJECTREF(refThisUNSAFE); - gc.target = (OBJECTREF) targetUNSAFE; - - HELPER_METHOD_FRAME_BEGIN_PROTECT(gc); + GCX_COOP(); - // via reflection you can pass in just about any value for the method. - // we can do some basic verification up front to prevent EE exceptions. - if (method == (PCODE)NULL) - COMPlusThrowArgumentNull(W("method")); + DELEGATEREF refThis = (DELEGATEREF) ObjectToOBJECTREF(_this.Get()); + _ASSERTE(refThis != NULL); - _ASSERTE(gc.refThis); - _ASSERTE(method); + GCPROTECT_BEGIN(refThis); - // programmers could feed garbage data to DelegateConstruct(). + // Programmers could feed garbage data to DelegateConstruct(). // It's difficult to validate a method code pointer, but at least we'll // try to catch the easy garbage. _ASSERTE(isMemoryReadable(method, 1)); - MethodTable *pMTTarg = NULL; + MethodTable* pMTTarg = NULL; + if (target.Get() != NULL) + pMTTarg = target.Get()->GetMethodTable(); - if (gc.target != NULL) - { - pMTTarg = gc.target->GetMethodTable(); - } - - MethodDesc *pMethOrig = Entry2MethodDesc(method, pMTTarg); - MethodDesc *pMeth = pMethOrig; - - MethodTable* pDelMT = gc.refThis->GetMethodTable(); + MethodTable* pDelMT = refThis->GetMethodTable(); + MethodDesc* pMethOrig = Entry2MethodDesc(method, pMTTarg); + MethodDesc* pMeth = pMethOrig; + _ASSERTE(pMeth != NULL); LOG((LF_STUBS, LL_INFO1000, "In DelegateConstruct: for delegate type %s binding to method %s::%s%s, static = %d\n", pDelMT->GetDebugClassName(), pMeth->m_pszDebugClassName, pMeth->m_pszDebugMethodName, pMeth->m_pszDebugMethodSignature, pMeth->IsStatic())); - _ASSERTE(pMeth); - #ifdef _DEBUG // Assert that everything is OK...This is not some bogus // address...Very unlikely that the code below would work // for a random address in memory.... MethodTable* p = pMeth->GetMethodTable(); - _ASSERTE(p); + _ASSERTE(p != NULL); _ASSERTE(p->ValidateWithPossibleAV()); #endif // _DEBUG if (Nullable::IsNullableType(pMeth->GetMethodTable())) COMPlusThrow(kNotSupportedException); - DelegateEEClass *pDelCls = (DelegateEEClass*)pDelMT->GetClass(); - MethodDesc *pDelegateInvoke = COMDelegate::FindDelegateInvokeMethod(pDelMT); + DelegateEEClass* pDelCls = (DelegateEEClass*)pDelMT->GetClass(); + MethodDesc* pDelegateInvoke = COMDelegate::FindDelegateInvokeMethod(pDelMT); UINT invokeArgCount = MethodDescToNumFixedArgs(pDelegateInvoke); UINT methodArgCount = MethodDescToNumFixedArgs(pMeth); BOOL isStatic = pMeth->IsStatic(); if (!isStatic) - { methodArgCount++; // count 'this' - } - if (NeedsWrapperDelegate(pMeth)) - gc.refThis = CreateWrapperDelegate(gc.refThis, pMeth); + if (COMDelegate::NeedsWrapperDelegate(pMeth)) + refThis = COMDelegate::CreateWrapperDelegate(refThis, pMeth); if (pMeth->GetLoaderAllocator()->IsCollectible()) - gc.refThis->SetMethodBase(pMeth->GetLoaderAllocator()->GetExposedObject()); + refThis->SetMethodBase(pMeth->GetLoaderAllocator()->GetExposedObject()); // Open delegates. if (invokeArgCount == methodArgCount) { // set the target - gc.refThis->SetTarget(gc.refThis); + refThis->SetTarget(refThis); // set the shuffle thunk - Stub *pShuffleThunk = NULL; - if (!pMeth->IsStatic() && pMeth->HasRetBuffArg() && IsRetBuffPassedAsFirstArg()) - pShuffleThunk = pDelCls->m_pInstRetBuffCallStub; - else - pShuffleThunk = pDelCls->m_pStaticCallStub; - if (!pShuffleThunk) + Stub *pShuffleThunk = (!pMeth->IsStatic() && pMeth->HasRetBuffArg() && IsRetBuffPassedAsFirstArg()) + ? pDelCls->m_pInstRetBuffCallStub + : pDelCls->m_pStaticCallStub; + + if (pShuffleThunk == NULL) pShuffleThunk = SetupShuffleThunk(pDelMT, pMeth); - gc.refThis->SetMethodPtr(pShuffleThunk->GetEntryPoint()); + refThis->SetMethodPtr(pShuffleThunk->GetEntryPoint()); // set the ptr aux according to what is needed, if virtual need to call make virtual stub dispatch if (!pMeth->IsStatic() && pMeth->IsVirtual() && !pMeth->GetMethodTable()->IsValueType()) { PCODE pTargetCall = GetVirtualCallStub(pMeth, TypeHandle(pMeth->GetMethodTable())); - gc.refThis->SetMethodPtrAux(pTargetCall); - gc.refThis->SetInvocationCount((INT_PTR)(void *)pMeth); + refThis->SetMethodPtrAux(pTargetCall); + refThis->SetInvocationCount((INT_PTR)(void *)pMeth); } else { - gc.refThis->SetMethodPtrAux(method); + refThis->SetMethodPtrAux(method); } } else @@ -1682,7 +1661,10 @@ FCIMPL3(void, COMDelegate::DelegateConstruct, Object* refThisUNSAFE, Object* tar if (!pMeth->IsStatic()) { - if (pMTTarg) + if (target.Get() == NULL) + COMPlusThrow(kArgumentException, W("Arg_DlgtNullInst")); + + if (pMTTarg != NULL) { // Use the Unboxing stub for value class methods, since the value // class is constructed using the boxed instance. @@ -1711,24 +1693,21 @@ FCIMPL3(void, COMDelegate::DelegateConstruct, Object* refThisUNSAFE, Object* tar method = pMeth->GetMultiCallableAddrOfCode(); } } - - if (gc.target == NULL) - { - COMPlusThrow(kArgumentException, W("Arg_DlgtNullInst")); - } } #ifdef HAS_THISPTR_RETBUF_PRECODE else if (pMeth->HasRetBuffArg() && IsRetBuffPassedAsFirstArg()) + { method = pMeth->GetLoaderAllocator()->GetFuncPtrStubs()->GetFuncPtrStub(pMeth, PRECODE_THISPTR_RETBUF); + } #endif // HAS_THISPTR_RETBUF_PRECODE - gc.refThis->SetTarget(gc.target); - gc.refThis->SetMethodPtr((PCODE)(void *)method); + refThis->SetTarget(target.Get()); + refThis->SetMethodPtr((PCODE)(void *)method); } - HELPER_METHOD_FRAME_END(); + GCPROTECT_END(); + END_QCALL; } -FCIMPLEND MethodDesc *COMDelegate::GetMethodDesc(OBJECTREF orDelegate) { diff --git a/src/coreclr/vm/comdelegate.h b/src/coreclr/vm/comdelegate.h index 8b1750fc6579d..be68b5ef40dd4 100644 --- a/src/coreclr/vm/comdelegate.h +++ b/src/coreclr/vm/comdelegate.h @@ -29,21 +29,10 @@ BOOL GenerateShuffleArrayPortable(MethodDesc* pMethodSrc, MethodDesc *pMethodDst // This class represents the native methods for the Delegate class class COMDelegate { -private: - // friend VOID CPUSTUBLINKER::EmitShuffleThunk(...); - friend class CPUSTUBLINKER; - - static CrstStatic s_DelegateToFPtrHashCrst; // Lock for the following hash. - static PtrHashMap* s_pDelegateToFPtrHash; // Hash table containing the Delegate->FPtr pairs - // passed out to unmanaged code. public: - static ShuffleThunkCache *m_pShuffleThunkCache; - // One time init. static void Init(); - static FCDECL3(void, DelegateConstruct, Object* refThis, Object* target, PCODE method); - // Get the invoke method for the delegate. Used to transition delegates to multicast delegates. static FCDECL1(PCODE, GetMulticastInvoke, MethodTable* pDelegateMT); static FCDECL1(MethodDesc*, GetInvokeMethod, MethodTable* pDelegateMT); @@ -88,10 +77,6 @@ class COMDelegate // for UnmanagedCallersOnlyAttribute. static void ThrowIfInvalidUnmanagedCallersOnlyUsage(MethodDesc* pMD); -private: - static Stub* SetupShuffleThunk(MethodTable * pDelMT, MethodDesc *pTargetMeth); - -public: static MethodDesc* FindDelegateInvokeMethod(MethodTable *pMT); static BOOL IsDelegateInvokeMethod(MethodDesc *pMD); @@ -111,6 +96,8 @@ class COMDelegate BOOL fIsOpenDelegate); }; +extern "C" void QCALLTYPE Delegate_Construct(QCall::ObjectHandleOnStack _this, QCall::ObjectHandleOnStack target, PCODE method); + extern "C" PCODE QCALLTYPE Delegate_GetMulticastInvokeSlow(MethodTable* pDelegateMT); extern "C" PCODE QCALLTYPE Delegate_AdjustTarget(QCall::ObjectHandleOnStack target, PCODE method); diff --git a/src/coreclr/vm/ecall.cpp b/src/coreclr/vm/ecall.cpp index 99bbdeb397284..4e67d02a543ef 100644 --- a/src/coreclr/vm/ecall.cpp +++ b/src/coreclr/vm/ecall.cpp @@ -320,22 +320,20 @@ PCODE ECall::GetFCallImpl(MethodDesc * pMD, BOOL * pfSharedOrDynamicFCallImpl /* MethodTable * pMT = pMD->GetMethodTable(); // - // Delegate constructors are FCalls for which the entrypoint points to the target of the delegate - // We have to intercept these and set the call target to the helper COMDelegate::DelegateConstruct + // Delegate constructors are QCalls for which the entrypoint points to the target of the delegate + // We have to intercept these and set the call target to the managed helper Delegate.DelegateConstruct // if (pMT->IsDelegate()) { if (pfSharedOrDynamicFCallImpl) *pfSharedOrDynamicFCallImpl = TRUE; - // COMDelegate::DelegateConstruct is the only fcall used by user delegates. - // All the other gDelegateFuncs are only used by System.Delegate _ASSERTE(pMD->IsCtor()); // We need to set up the ECFunc properly. We don't want to use the pMD passed in, // since it may disappear. Instead, use the stable one on Delegate. Remember - // that this is 1:M between the FCall and the pMDs. - return GetFCallImpl(CoreLibBinder::GetMethod(METHOD__DELEGATE__CONSTRUCT_DELEGATE)); + // that this is 1:M between the method and the constructors. + return CoreLibBinder::GetMethod(METHOD__DELEGATE__CONSTRUCT_DELEGATE)->GetMultiCallableAddrOfCode(); } // COM imported classes have special constructors @@ -457,9 +455,7 @@ BOOL ECall::IsSharedFCallImpl(PCODE pImpl) PCODE pNativeCode = pImpl; - return - (pNativeCode == GetEEFuncEntryPoint(FCComCtor)) || - (pNativeCode == GetEEFuncEntryPoint(COMDelegate::DelegateConstruct)); + return (pNativeCode == GetEEFuncEntryPoint(FCComCtor)); } BOOL ECall::CheckUnusedECalls(SetSHash& usedIDs) diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index 8996b3a16d811..7373c45b0aa29 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -234,11 +234,6 @@ FCFuncEnd() FCFuncStart(gDelegateFuncs) FCFuncElement("GetMulticastInvoke", COMDelegate::GetMulticastInvoke) FCFuncElement("GetInvokeMethod", COMDelegate::GetInvokeMethod) - - // The FCall mechanism knows how to wire multiple different constructor calls into a - // single entrypoint, without the following entry. But we need this entry to satisfy - // frame creation within the body: - FCFuncElement("DelegateConstruct", COMDelegate::DelegateConstruct) FCFuncEnd() FCFuncStart(gMathFuncs) diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index f7447a60132f2..6528c0103e291 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -4017,7 +4017,7 @@ PCODE DynamicHelperFixup(TransitionBlock * pTransitionBlock, TADDR * pCell, DWOR } else { - target = ECall::GetFCallImpl(CoreLibBinder::GetMethod(METHOD__DELEGATE__CONSTRUCT_DELEGATE)); + target = CoreLibBinder::GetMethod(METHOD__DELEGATE__CONSTRUCT_DELEGATE)->GetMultiCallableAddrOfCode(); ctorData.pArg3 = NULL; } diff --git a/src/coreclr/vm/qcallentrypoints.cpp b/src/coreclr/vm/qcallentrypoints.cpp index 18e080bbe12d2..f9a7753135a49 100644 --- a/src/coreclr/vm/qcallentrypoints.cpp +++ b/src/coreclr/vm/qcallentrypoints.cpp @@ -93,6 +93,7 @@ static const Entry s_QCall[] = DllImportEntry(Delegate_InitializeVirtualCallStub) DllImportEntry(Delegate_GetMulticastInvokeSlow) DllImportEntry(Delegate_AdjustTarget) + DllImportEntry(Delegate_Construct) DllImportEntry(Delegate_InternalAlloc) DllImportEntry(Delegate_InternalAllocLike) DllImportEntry(Delegate_FindMethodHandle) From 69de6dd222461809e782625f18bfd8073f5b2e7c Mon Sep 17 00:00:00 2001 From: Noah Falk Date: Thu, 26 Sep 2024 20:10:42 -0700 Subject: [PATCH 3/6] Root the System.Runtime EventSource (#108266) Root the System.Runtime EventSource The System.Runtime EventSource (RuntimeEventSource), was unintentionally being garbage collected because it wasn't rooted. This caused runtime EventCounters to no longer be available after GC occurred. This was a regression from a recent change (https://github.com/dotnet/runtime/pull/106014). That change accidentally converted the static field that was intended to the root the object into a property getter that returned a new instance each time it was called. This fix converts the property back to being initialized only once. This will fix #107919 once it is backported. Co-authored-by: Tarek Mahmoud Sayed <10833894+tarekgh@users.noreply.github.com> --- .../src/System/Diagnostics/Tracing/RuntimeEventSource.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/RuntimeEventSource.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/RuntimeEventSource.cs index 558507211b17f..da0a5e6e437b5 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/RuntimeEventSource.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/RuntimeEventSource.cs @@ -23,7 +23,8 @@ public static class Keywords public const EventKeywords ProcessorCount = (EventKeywords)0x2; } - internal static RuntimeEventSource? Log => new RuntimeEventSource(); + // this roots the singleton instance of the event source + internal static RuntimeEventSource Log { get; } = new RuntimeEventSource(); private PollingCounter? _gcHeapSizeCounter; private IncrementingPollingCounter? _gen0GCCounter; private IncrementingPollingCounter? _gen1GCCounter; From 84b7c2f15ceec32e66b46231614dfcb3e334c6fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Fri, 27 Sep 2024 14:46:13 +0900 Subject: [PATCH 4/6] Unblock System.Linq testing in nativeAOT outerloop (#108302) --- src/libraries/System.Linq/tests/default.rd.xml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libraries/System.Linq/tests/default.rd.xml b/src/libraries/System.Linq/tests/default.rd.xml index f7f1972141754..cef0575a16557 100644 --- a/src/libraries/System.Linq/tests/default.rd.xml +++ b/src/libraries/System.Linq/tests/default.rd.xml @@ -287,6 +287,10 @@ + + + + From 01aa3d96bb2160144f167b1065e081521d133b48 Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz <32700855+ilonatommy@users.noreply.github.com> Date: Fri, 27 Sep 2024 12:19:14 +0200 Subject: [PATCH 5/6] [wasm] Change ICU `WasmBuildTests` to use `wasmconsole` and `wasmbrowser` templates (#108271) * Clean up icu tests (automatic mode). * Block the failing test, mark problems with issue links. --- .../AssertWasmSdkBundleOptions.cs | 4 +- .../Blazor/BlazorBuildOptions.cs | 2 +- .../Blazor/BlazorWasmProjectProvider.cs | 2 +- .../Blazor/IcuShardingTests.cs | 12 +- .../wasm/Wasm.Build.Tests/BrowserRunner.cs | 10 +- .../Wasm.Build.Tests/BuildProjectOptions.cs | 2 +- .../wasm/Wasm.Build.Tests/BuildTestBase.cs | 5 +- .../Common/AssertBundleOptionsBase.cs | 2 +- .../AssertTestMainJsAppBundleOptions.cs | 4 +- .../Common/GlobalizationMode.cs | 2 +- .../wasm/Wasm.Build.Tests/IcuShardingTests.cs | 70 ++++---- .../Wasm.Build.Tests/IcuShardingTests2.cs | 36 ++-- src/mono/wasm/Wasm.Build.Tests/IcuTests.cs | 161 ++++++++++-------- .../wasm/Wasm.Build.Tests/IcuTestsBase.cs | 111 ++++++++++-- .../Wasm.Build.Tests/ProjectProviderBase.cs | 14 +- .../Templates/WasmTemplateTests.cs | 117 +++---------- .../Templates/WasmTemplateTestsBase.cs | 125 ++++++++++++++ .../TestMainJsProjectProvider.cs | 2 +- .../WasmSdkBasedProjectProvider.cs | 2 +- 19 files changed, 417 insertions(+), 266 deletions(-) create mode 100644 src/mono/wasm/Wasm.Build.Tests/Templates/WasmTemplateTestsBase.cs diff --git a/src/mono/wasm/Wasm.Build.Tests/AssertWasmSdkBundleOptions.cs b/src/mono/wasm/Wasm.Build.Tests/AssertWasmSdkBundleOptions.cs index f83eaf616958f..b3d0f1a18caa7 100644 --- a/src/mono/wasm/Wasm.Build.Tests/AssertWasmSdkBundleOptions.cs +++ b/src/mono/wasm/Wasm.Build.Tests/AssertWasmSdkBundleOptions.cs @@ -11,7 +11,7 @@ public record AssertWasmSdkBundleOptions( bool IsPublish, string TargetFramework, string BinFrameworkDir, - string? PredefinedIcudt, + string? CustomIcuFile, GlobalizationMode GlobalizationMode = GlobalizationMode.Sharded, string BootJsonFileName = "blazor.boot.json", NativeFilesType ExpectedFileType = NativeFilesType.FromRuntimePack, @@ -25,7 +25,7 @@ public record AssertWasmSdkBundleOptions( IsPublish: IsPublish, TargetFramework: TargetFramework, BinFrameworkDir: BinFrameworkDir, - PredefinedIcudt: PredefinedIcudt, + CustomIcuFile: CustomIcuFile, GlobalizationMode: GlobalizationMode, ExpectedFileType: ExpectedFileType, RuntimeType: RuntimeType, diff --git a/src/mono/wasm/Wasm.Build.Tests/Blazor/BlazorBuildOptions.cs b/src/mono/wasm/Wasm.Build.Tests/Blazor/BlazorBuildOptions.cs index 025ea7e38eef8..b3111d60e6a8b 100644 --- a/src/mono/wasm/Wasm.Build.Tests/Blazor/BlazorBuildOptions.cs +++ b/src/mono/wasm/Wasm.Build.Tests/Blazor/BlazorBuildOptions.cs @@ -18,7 +18,7 @@ public record BlazorBuildOptions bool ExpectFingerprintOnDotnetJs = false, RuntimeVariant RuntimeType = RuntimeVariant.SingleThreaded, GlobalizationMode GlobalizationMode = GlobalizationMode.Sharded, - string PredefinedIcudt = "", + string CustomIcuFile = "", bool AssertAppBundle = true, string? BinFrameworkDir = null ); diff --git a/src/mono/wasm/Wasm.Build.Tests/Blazor/BlazorWasmProjectProvider.cs b/src/mono/wasm/Wasm.Build.Tests/Blazor/BlazorWasmProjectProvider.cs index ea6cc51fc6c9f..6f0a28f311e22 100644 --- a/src/mono/wasm/Wasm.Build.Tests/Blazor/BlazorWasmProjectProvider.cs +++ b/src/mono/wasm/Wasm.Build.Tests/Blazor/BlazorWasmProjectProvider.cs @@ -20,7 +20,7 @@ public void AssertBundle(BlazorBuildOptions options) TargetFramework: options.TargetFramework, BinFrameworkDir: options.BinFrameworkDir ?? FindBinFrameworkDir(options.Config, options.IsPublish, options.TargetFramework), GlobalizationMode: options.GlobalizationMode, - PredefinedIcudt: options.PredefinedIcudt, + CustomIcuFile: options.CustomIcuFile, ExpectFingerprintOnDotnetJs: options.ExpectFingerprintOnDotnetJs, ExpectedFileType: options.ExpectedFileType, RuntimeType: options.RuntimeType, diff --git a/src/mono/wasm/Wasm.Build.Tests/Blazor/IcuShardingTests.cs b/src/mono/wasm/Wasm.Build.Tests/Blazor/IcuShardingTests.cs index aa19657015185..9a6ba4e09ae8d 100644 --- a/src/mono/wasm/Wasm.Build.Tests/Blazor/IcuShardingTests.cs +++ b/src/mono/wasm/Wasm.Build.Tests/Blazor/IcuShardingTests.cs @@ -32,8 +32,8 @@ public async Task CustomIcuFileFromRuntimePack(string config, string fileName) id, config, WarnAsError: true, - GlobalizationMode: GlobalizationMode.PredefinedIcu, - PredefinedIcudt: fileName + GlobalizationMode: GlobalizationMode.Custom, + CustomIcuFile: fileName ); AddItemsPropertiesToProject( projectFile, @@ -65,8 +65,8 @@ public void NonExistingCustomFileAssertError(string config, string fileName, boo id, config, WarnAsError: false, - GlobalizationMode: GlobalizationMode.PredefinedIcu, - PredefinedIcudt: fileName + GlobalizationMode: GlobalizationMode.Custom, + CustomIcuFile: fileName )); } catch (XunitException ex) @@ -104,8 +104,8 @@ public async Task CustomFileNotFromRuntimePackAbsolutePath(string config) id, config, WarnAsError: false, - GlobalizationMode: GlobalizationMode.PredefinedIcu, - PredefinedIcudt: IcuTestsBase.CustomIcuPath + GlobalizationMode: GlobalizationMode.Custom, + CustomIcuFile: IcuTestsBase.CustomIcuPath )); await BlazorRunForBuildWithDotnetRun(new BlazorRunOptions() { Config = config }); } diff --git a/src/mono/wasm/Wasm.Build.Tests/BrowserRunner.cs b/src/mono/wasm/Wasm.Build.Tests/BrowserRunner.cs index 7234bd908c070..3d2dda1515a71 100644 --- a/src/mono/wasm/Wasm.Build.Tests/BrowserRunner.cs +++ b/src/mono/wasm/Wasm.Build.Tests/BrowserRunner.cs @@ -104,12 +104,13 @@ public async Task SpawnBrowserAsync( string browserUrl, bool headless = true, int timeout = 10000, - int maxRetries = 3 + int maxRetries = 3, + string language = "en-US" ) { var url = new Uri(browserUrl); Playwright = await Microsoft.Playwright.Playwright.CreateAsync(); // codespaces: ignore certificate error -> Microsoft.Playwright.PlaywrightException : net::ERR_CERT_AUTHORITY_INVALID - string[] chromeArgs = new[] { $"--explicitly-allowed-ports={url.Port}", "--ignore-certificate-errors" }; + string[] chromeArgs = new[] { $"--explicitly-allowed-ports={url.Port}", "--ignore-certificate-errors", $"--lang={language}" }; _testOutput.WriteLine($"Launching chrome ('{s_chromePath.Value}') via playwright with args = {string.Join(',', chromeArgs)}"); int attempt = 0; @@ -146,14 +147,15 @@ public async Task RunAsync( ToolCommand cmd, string args, bool headless = true, + string language = "en-US", Action? onConsoleMessage = null, Action? onServerMessage = null, Action? onError = null, Func? modifyBrowserUrl = null) { var urlString = await StartServerAndGetUrlAsync(cmd, args, onServerMessage); - var browser = await SpawnBrowserAsync(urlString, headless); - var context = await browser.NewContextAsync(); + var browser = await SpawnBrowserAsync(urlString, headless, language: language); + var context = await browser.NewContextAsync(new BrowserNewContextOptions { Locale = language }); return await RunAsync(context, urlString, headless, onConsoleMessage, onError, modifyBrowserUrl); } diff --git a/src/mono/wasm/Wasm.Build.Tests/BuildProjectOptions.cs b/src/mono/wasm/Wasm.Build.Tests/BuildProjectOptions.cs index c86c4cda6e7f9..d0bd02a2289a5 100644 --- a/src/mono/wasm/Wasm.Build.Tests/BuildProjectOptions.cs +++ b/src/mono/wasm/Wasm.Build.Tests/BuildProjectOptions.cs @@ -13,7 +13,7 @@ public record BuildProjectOptions Action? InitProject = null, bool? DotnetWasmFromRuntimePack = null, GlobalizationMode GlobalizationMode = GlobalizationMode.Sharded, - string? PredefinedIcudt = null, + string? CustomIcuFile = null, bool UseCache = true, bool ExpectSuccess = true, bool AssertAppBundle = true, diff --git a/src/mono/wasm/Wasm.Build.Tests/BuildTestBase.cs b/src/mono/wasm/Wasm.Build.Tests/BuildTestBase.cs index 1f120016ef6aa..50535fee0a330 100644 --- a/src/mono/wasm/Wasm.Build.Tests/BuildTestBase.cs +++ b/src/mono/wasm/Wasm.Build.Tests/BuildTestBase.cs @@ -630,8 +630,9 @@ public static int Main() public record BuildArgs(string ProjectName, string Config, bool AOT, - string ProjectFileContents, - string? ExtraBuildArgs); + string Id, + string? ExtraBuildArgs, + string? ProjectFileContents=null); public record BuildProduct(string ProjectDir, string LogFile, bool Result, string BuildOutput); public enum NativeFilesType { FromRuntimePack, Relinked, AOT }; diff --git a/src/mono/wasm/Wasm.Build.Tests/Common/AssertBundleOptionsBase.cs b/src/mono/wasm/Wasm.Build.Tests/Common/AssertBundleOptionsBase.cs index e985ebf458938..bf5301a53c762 100644 --- a/src/mono/wasm/Wasm.Build.Tests/Common/AssertBundleOptionsBase.cs +++ b/src/mono/wasm/Wasm.Build.Tests/Common/AssertBundleOptionsBase.cs @@ -12,7 +12,7 @@ public abstract record AssertBundleOptionsBase( bool IsPublish, string TargetFramework, string BinFrameworkDir, - string? PredefinedIcudt, + string? CustomIcuFile, string BundleDirName = "wwwroot", GlobalizationMode GlobalizationMode = GlobalizationMode.Sharded, string BootJsonFileName = "blazor.boot.json", diff --git a/src/mono/wasm/Wasm.Build.Tests/Common/AssertTestMainJsAppBundleOptions.cs b/src/mono/wasm/Wasm.Build.Tests/Common/AssertTestMainJsAppBundleOptions.cs index a7b028164d3cc..3f0188b06e20f 100644 --- a/src/mono/wasm/Wasm.Build.Tests/Common/AssertTestMainJsAppBundleOptions.cs +++ b/src/mono/wasm/Wasm.Build.Tests/Common/AssertTestMainJsAppBundleOptions.cs @@ -10,7 +10,7 @@ public record AssertTestMainJsAppBundleOptions( bool IsPublish, string TargetFramework, string BinFrameworkDir, - string? PredefinedIcudt, + string? CustomIcuFile, string ProjectName, string MainJS, GlobalizationMode GlobalizationMode = GlobalizationMode.Sharded, @@ -28,7 +28,7 @@ public record AssertTestMainJsAppBundleOptions( IsPublish: IsPublish, TargetFramework: TargetFramework, BinFrameworkDir: BinFrameworkDir, - PredefinedIcudt: PredefinedIcudt, + CustomIcuFile: CustomIcuFile, GlobalizationMode: GlobalizationMode, ExpectedFileType: ExpectedFileType, RuntimeType: RuntimeType, diff --git a/src/mono/wasm/Wasm.Build.Tests/Common/GlobalizationMode.cs b/src/mono/wasm/Wasm.Build.Tests/Common/GlobalizationMode.cs index 0b2130a944d42..250dbe7dc5b6b 100644 --- a/src/mono/wasm/Wasm.Build.Tests/Common/GlobalizationMode.cs +++ b/src/mono/wasm/Wasm.Build.Tests/Common/GlobalizationMode.cs @@ -9,6 +9,6 @@ public enum GlobalizationMode Sharded, // chosen based on locale Invariant, // no icu FullIcu, // full icu data: icudt.dat is loaded - PredefinedIcu, // user set WasmIcuDataFileName/BlazorIcuDataFileName value and we are loading that file + Custom, // user set WasmIcuDataFileName/BlazorIcuDataFileName value and we are loading that file Hybrid // reduced icu, missing data is provided by platform-native functions (web api for wasm) }; diff --git a/src/mono/wasm/Wasm.Build.Tests/IcuShardingTests.cs b/src/mono/wasm/Wasm.Build.Tests/IcuShardingTests.cs index dbaeba4bb1165..c4fdf83966bb8 100644 --- a/src/mono/wasm/Wasm.Build.Tests/IcuShardingTests.cs +++ b/src/mono/wasm/Wasm.Build.Tests/IcuShardingTests.cs @@ -2,11 +2,13 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; using System.IO; +using System.Linq; +using System.Threading.Tasks; using Xunit; using Xunit.Abstractions; using Xunit.Sdk; -using System.Collections.Generic; #nullable enable @@ -17,47 +19,35 @@ public class IcuShardingTests : IcuTestsBase public IcuShardingTests(ITestOutputHelper output, SharedBuildPerTestClassFixture buildContext) : base(output, buildContext) { } - public static IEnumerable IcuExpectedAndMissingCustomShardTestData(bool aot, RunHost host) - => ConfigWithAOTData(aot) - .Multiply( - new object[] { CustomIcuPath, s_customIcuTestedLocales, false }, - new object[] { CustomIcuPath, s_customIcuTestedLocales, true }) - .WithRunHosts(host) - .UnwrapItemsAsArrays(); - - public static IEnumerable IcuExpectedAndMissingAutomaticShardTestData(bool aot) - => ConfigWithAOTData(aot) - .Multiply( - new object[] { "fr-FR", GetEfigsTestedLocales(SundayNames.French)}, - new object[] { "ja-JP", GetCjkTestedLocales(SundayNames.Japanese) }, - new object[] { "sk-SK", GetNocjkTestedLocales(SundayNames.Slovak) }) - .WithRunHosts(BuildTestBase.s_hostsForOSLocaleSensitiveTests) - .UnwrapItemsAsArrays(); + public static IEnumerable IcuExpectedAndMissingCustomShardTestData(string config) => + from templateType in templateTypes + from aot in boolOptions + from onlyPredefinedCultures in boolOptions + // isOnlyPredefinedCultures = true fails with wasmbrowser: https://github.com/dotnet/runtime/issues/108272 + where !(onlyPredefinedCultures && templateType == "wasmbrowser") + select new object[] { config, templateType, aot, CustomIcuPath, s_customIcuTestedLocales, onlyPredefinedCultures }; + + public static IEnumerable IcuExpectedAndMissingAutomaticShardTestData(string config) + { + var locales = new Dictionary + { + { "fr-FR", GetEfigsTestedLocales(SundayNames.French) }, + { "ja-JP", GetCjkTestedLocales(SundayNames.Japanese) }, + { "sk-SK", GetNocjkTestedLocales(SundayNames.Slovak) } + }; + // "wasmconsole": https://github.com/dotnet/runtime/issues/82593 + return from aot in boolOptions + from locale in locales + select new object[] { config, "wasmbrowser", aot, locale.Key, locale.Value }; + } [Theory] - [MemberData(nameof(IcuExpectedAndMissingCustomShardTestData), parameters: new object[] { false, RunHost.NodeJS | RunHost.Chrome })] - [MemberData(nameof(IcuExpectedAndMissingCustomShardTestData), parameters: new object[] { true, RunHost.NodeJS | RunHost.Chrome })] - public void CustomIcuShard(BuildArgs buildArgs, string shardName, string testedLocales, bool onlyPredefinedCultures, RunHost host, string id) => - TestIcuShards(buildArgs, shardName, testedLocales, host, id, onlyPredefinedCultures); + [MemberData(nameof(IcuExpectedAndMissingCustomShardTestData), parameters: new object[] { "Release" })] + public async Task CustomIcuShard(string config, string templateType, bool aot, string customIcuPath, string customLocales, bool onlyPredefinedCultures) => + await TestIcuShards(config, templateType, aot, customIcuPath, customLocales, GlobalizationMode.Custom, onlyPredefinedCultures); [Theory] - [MemberData(nameof(IcuExpectedAndMissingAutomaticShardTestData), parameters: new object[] { false })] - [MemberData(nameof(IcuExpectedAndMissingAutomaticShardTestData), parameters: new object[] { true })] - public void AutomaticShardSelectionDependingOnEnvLocale(BuildArgs buildArgs, string environmentLocale, string testedLocales, RunHost host, string id) - { - string projectName = $"automatic_shard_{environmentLocale}_{buildArgs.Config}_{buildArgs.AOT}"; - bool dotnetWasmFromRuntimePack = !(buildArgs.AOT || buildArgs.Config == "Release"); - - buildArgs = buildArgs with { ProjectName = projectName }; - buildArgs = ExpandBuildArgs(buildArgs); - - string programText = GetProgramText(testedLocales); - _testOutput.WriteLine($"----- Program: -----{Environment.NewLine}{programText}{Environment.NewLine}-------"); - (_, string output) = BuildProject(buildArgs, - id: id, - new BuildProjectOptions( - InitProject: () => File.WriteAllText(Path.Combine(_projectDir!, "Program.cs"), programText), - DotnetWasmFromRuntimePack: dotnetWasmFromRuntimePack)); - string runOutput = RunAndTestWasmApp(buildArgs, buildDir: _projectDir, expectedExitCode: 42, host: host, id: id, environmentLocale: environmentLocale); - } + [MemberData(nameof(IcuExpectedAndMissingAutomaticShardTestData), parameters: new object[] { "Release" })] + public async Task AutomaticShardSelectionDependingOnEnvLocale(string config, string templateType, bool aot, string environmentLocale, string testedLocales) => + await BuildAndRunIcuTest(config, templateType, aot, testedLocales, GlobalizationMode.Sharded, language: environmentLocale); } diff --git a/src/mono/wasm/Wasm.Build.Tests/IcuShardingTests2.cs b/src/mono/wasm/Wasm.Build.Tests/IcuShardingTests2.cs index c04d5c1114dc0..38acd3f848566 100644 --- a/src/mono/wasm/Wasm.Build.Tests/IcuShardingTests2.cs +++ b/src/mono/wasm/Wasm.Build.Tests/IcuShardingTests2.cs @@ -3,6 +3,8 @@ using System; using System.IO; +using System.Linq; +using System.Threading.Tasks; using Xunit; using Xunit.Abstractions; using Xunit.Sdk; @@ -17,23 +19,27 @@ public class IcuShardingTests2 : IcuTestsBase public IcuShardingTests2(ITestOutputHelper output, SharedBuildPerTestClassFixture buildContext) : base(output, buildContext) { } - public static IEnumerable IcuExpectedAndMissingShardFromRuntimePackTestData(bool aot, RunHost host) - => ConfigWithAOTData(aot) - .Multiply( - new object[] { "icudt.dat", - $@"new Locale[] {{ + public static IEnumerable IcuExpectedAndMissingShardFromRuntimePackTestData(string config) + { + var locales = new Dictionary + { + { "icudt.dat", $@"new Locale[] {{ new Locale(""en-GB"", ""{SundayNames.English}""), new Locale(""zh-CN"", ""{SundayNames.Chinese}""), new Locale(""sk-SK"", ""{SundayNames.Slovak}""), new Locale(""xx-yy"", null) }}" }, - new object[] { "icudt_EFIGS.dat", GetEfigsTestedLocales() }, - new object[] { "icudt_CJK.dat", GetCjkTestedLocales() }, - new object[] { "icudt_no_CJK.dat", GetNocjkTestedLocales() }) - .WithRunHosts(host) - .UnwrapItemsAsArrays(); - + { "icudt_EFIGS.dat", GetEfigsTestedLocales() }, + { "icudt_CJK.dat", GetCjkTestedLocales() }, + { "icudt_no_CJK.dat", GetNocjkTestedLocales() } + }; + return + // "wasmconsole": https://github.com/dotnet/runtime/issues/82593 + // from templateType in templateTypes + from aot in boolOptions + from locale in locales + select new object[] { config, "wasmbrowser", aot, locale.Key, locale.Value }; + } [Theory] - [MemberData(nameof(IcuExpectedAndMissingShardFromRuntimePackTestData), parameters: new object[] { false, RunHost.NodeJS | RunHost.Chrome })] - [MemberData(nameof(IcuExpectedAndMissingShardFromRuntimePackTestData), parameters: new object[] { true, RunHost.NodeJS | RunHost.Chrome })] - public void DefaultAvailableIcuShardsFromRuntimePack(BuildArgs buildArgs, string shardName, string testedLocales, RunHost host, string id) => - TestIcuShards(buildArgs, shardName, testedLocales, host, id); + [MemberData(nameof(IcuExpectedAndMissingShardFromRuntimePackTestData), parameters: new object[] { "Release" })] + public async Task DefaultAvailableIcuShardsFromRuntimePack(string config, string templateType, bool aot, string shardName, string testedLocales) => + await TestIcuShards(config, templateType, aot, shardName, testedLocales, GlobalizationMode.Custom); } \ No newline at end of file diff --git a/src/mono/wasm/Wasm.Build.Tests/IcuTests.cs b/src/mono/wasm/Wasm.Build.Tests/IcuTests.cs index 0f58d24ac9f3d..6c0e2c5ae9c55 100644 --- a/src/mono/wasm/Wasm.Build.Tests/IcuTests.cs +++ b/src/mono/wasm/Wasm.Build.Tests/IcuTests.cs @@ -3,6 +3,8 @@ using System; using System.IO; +using System.Linq; +using System.Threading.Tasks; using Xunit; using Xunit.Abstractions; using Xunit.Sdk; @@ -17,97 +19,106 @@ public class IcuTests : IcuTestsBase public IcuTests(ITestOutputHelper output, SharedBuildPerTestClassFixture buildContext) : base(output, buildContext) { } - public static IEnumerable FullIcuWithInvariantTestData(bool aot, RunHost host) - => ConfigWithAOTData(aot) - .Multiply( - // in invariant mode, all locales should be missing - new object[] { true, true, "Array.Empty()" }, - new object[] { true, false, "Array.Empty()" }, - new object[] { false, false, GetEfigsTestedLocales() }, - new object[] { false, true, s_fullIcuTestedLocales}) - .WithRunHosts(host) - .UnwrapItemsAsArrays(); + public static IEnumerable FullIcuWithICustomIcuTestData(string config) => + from templateType in templateTypes + from aot in boolOptions + from fullIcu in boolOptions + select new object[] { config, templateType, aot, fullIcu }; - public static IEnumerable FullIcuWithICustomIcuTestData(bool aot, RunHost host) - => ConfigWithAOTData(aot) - .Multiply( - new object[] { true }, - new object[] { false }) - .WithRunHosts(host) - .UnwrapItemsAsArrays(); - - [Theory] - [MemberData(nameof(FullIcuWithInvariantTestData), parameters: new object[] { false, RunHost.NodeJS | RunHost.Chrome })] - [MemberData(nameof(FullIcuWithInvariantTestData), parameters: new object[] { true, RunHost.NodeJS | RunHost.Chrome })] - public void FullIcuFromRuntimePackWithInvariant(BuildArgs buildArgs, bool invariant, bool fullIcu, string testedLocales, RunHost host, string id) + public static IEnumerable FullIcuWithInvariantTestData(string config) { - string projectName = $"fullIcuInvariant_{fullIcu}_{invariant}_{buildArgs.Config}_{buildArgs.AOT}"; - bool dotnetWasmFromRuntimePack = !(buildArgs.AOT || buildArgs.Config == "Release"); - - buildArgs = buildArgs with { ProjectName = projectName }; - buildArgs = ExpandBuildArgs(buildArgs, extraProperties: $"{invariant}{fullIcu}"); - - string programText = GetProgramText(testedLocales); - _testOutput.WriteLine($"----- Program: -----{Environment.NewLine}{programText}{Environment.NewLine}-------"); - (_, string output) = BuildProject(buildArgs, - id: id, - new BuildProjectOptions( - InitProject: () => File.WriteAllText(Path.Combine(_projectDir!, "Program.cs"), programText), - DotnetWasmFromRuntimePack: dotnetWasmFromRuntimePack, - GlobalizationMode: invariant ? GlobalizationMode.Invariant : fullIcu ? GlobalizationMode.FullIcu : GlobalizationMode.Sharded)); - - string runOutput = RunAndTestWasmApp(buildArgs, buildDir: _projectDir, expectedExitCode: 42, host: host, id: id); + var locales = new object[][] + { + // in invariant mode, all locales should be missing + new object[] { true, true, "Array.Empty()" }, + new object[] { true, false, "Array.Empty()" }, + new object[] { false, false, GetEfigsTestedLocales() }, + new object[] { false, true, s_fullIcuTestedLocales } + }; + return from templateType in templateTypes + from aot in boolOptions + from locale in locales + select new object[] { config, templateType, aot, locale[0], locale[1], locale[2] }; } - [Theory] - [MemberData(nameof(FullIcuWithICustomIcuTestData), parameters: new object[] { false, RunHost.NodeJS | RunHost.Chrome })] - [MemberData(nameof(FullIcuWithICustomIcuTestData), parameters: new object[] { true, RunHost.NodeJS | RunHost.Chrome })] - public void FullIcuFromRuntimePackWithCustomIcu(BuildArgs buildArgs, bool fullIcu, RunHost host, string id) + public static IEnumerable IncorrectIcuTestData(string config) { - string projectName = $"fullIcuCustom_{fullIcu}_{buildArgs.Config}_{buildArgs.AOT}"; - bool dotnetWasmFromRuntimePack = !(buildArgs.AOT || buildArgs.Config == "Release"); + var customFiles = new Dictionary + { + { "icudtNonExisting.dat", true }, + { "incorrectName.dat", false } + }; + return from templateType in templateTypes + from customFile in customFiles + select new object[] { config, templateType, customFile.Key, customFile.Value }; + } + - buildArgs = buildArgs with { ProjectName = projectName }; - buildArgs = ExpandBuildArgs(buildArgs, extraProperties: $"{CustomIcuPath}{fullIcu}"); + [Theory] + [MemberData(nameof(FullIcuWithInvariantTestData), parameters: new object[] { "Release" })] + public async Task FullIcuFromRuntimePackWithInvariant(string config, string templateType, bool aot, bool invariant, bool fullIcu, string testedLocales) => + await BuildAndRunIcuTest( + config, + templateType, + aot, + testedLocales, + globalizationMode: invariant ? GlobalizationMode.Invariant : fullIcu ? GlobalizationMode.FullIcu : GlobalizationMode.Sharded, + extraProperties: + // https://github.com/dotnet/runtime/issues/94133: "wasmbrowser" should use WasmIncludeFullIcuData, not BlazorWebAssemblyLoadAllGlobalizationData + templateType == "wasmconsole" ? + $"{invariant}{fullIcu}{aot}" : + $"{invariant}{fullIcu}{aot}"); + [Theory] + [MemberData(nameof(FullIcuWithICustomIcuTestData), parameters: new object[] { "Release" })] + public async Task FullIcuFromRuntimePackWithCustomIcu(string config, string templateType, bool aot, bool fullIcu) + { + bool isBrowser = templateType == "wasmbrowser"; + string customIcuProperty = isBrowser ? "BlazorIcuDataFileName" : "WasmIcuDataFileName"; + string fullIcuProperty = isBrowser ? "BlazorWebAssemblyLoadAllGlobalizationData" : "WasmIncludeFullIcuData"; + string extraProperties = $"<{customIcuProperty}>{CustomIcuPath}<{fullIcuProperty}>{fullIcu}{aot}"; + string testedLocales = fullIcu ? s_fullIcuTestedLocales : s_customIcuTestedLocales; - string programText = GetProgramText(testedLocales); - _testOutput.WriteLine($"----- Program: -----{Environment.NewLine}{programText}{Environment.NewLine}-------"); - (_, string output) = BuildProject(buildArgs, - id: id, - new BuildProjectOptions( - InitProject: () => File.WriteAllText(Path.Combine(_projectDir!, "Program.cs"), programText), - DotnetWasmFromRuntimePack: dotnetWasmFromRuntimePack, - GlobalizationMode: fullIcu ? GlobalizationMode.FullIcu : GlobalizationMode.PredefinedIcu, - PredefinedIcudt: fullIcu ? "" : CustomIcuPath)); + GlobalizationMode globalizationMode = fullIcu ? GlobalizationMode.FullIcu : GlobalizationMode.Custom; + string customIcuFile = fullIcu ? "" : CustomIcuPath; + string output = await BuildAndRunIcuTest(config, templateType, aot, testedLocales, globalizationMode, extraProperties, icuFileName: customIcuFile); if (fullIcu) - Assert.Contains("$(WasmIcuDataFileName) has no effect when $(WasmIncludeFullIcuData) is set to true.", output); - - string runOutput = RunAndTestWasmApp(buildArgs, buildDir: _projectDir, expectedExitCode: 42, host: host, id: id); + Assert.Contains($"$({customIcuProperty}) has no effect when $({fullIcuProperty}) is set to true.", output); } [Theory] - [BuildAndRun(host: RunHost.None, parameters: new object[] { "icudtNonExisting.dat", true })] - [BuildAndRun(host: RunHost.None, parameters: new object[] { "incorrectName.dat", false })] - public void NonExistingCustomFileAssertError(BuildArgs buildArgs, string customFileName, bool isFilenameCorrect, string id) - { - string projectName = $"invalidCustomIcu_{buildArgs.Config}_{buildArgs.AOT}"; - buildArgs = buildArgs with { ProjectName = projectName }; - string customIcu = Path.Combine(BuildEnvironment.TestAssetsPath, customFileName); - buildArgs = ExpandBuildArgs(buildArgs, extraProperties: $"{customIcu}"); - - (_, string output) = BuildProject(buildArgs, - id: id, - new BuildProjectOptions( - InitProject: () => File.WriteAllText(Path.Combine(_projectDir!, "Program.cs"), s_mainReturns42), - ExpectSuccess: false)); - if (isFilenameCorrect) + [MemberData(nameof(IncorrectIcuTestData), parameters: new object[] { "Release" })] + public void NonExistingCustomFileAssertError(string config, string templateType, string customIcu, bool isFilenameFormCorrect) + { + bool isBrowser = templateType == "wasmbrowser"; + string customIcuProperty = isBrowser ? "BlazorIcuDataFileName" : "WasmIcuDataFileName"; + string extraProperties = $"<{customIcuProperty}>{customIcu}"; + + (BuildArgs buildArgs, string projectFile) = CreateIcuProject( + config, templateType, aot: false, "Array.Empty()", extraProperties); + string output = BuildIcuTest( + buildArgs, + isBrowser, + GlobalizationMode.Custom, + customIcu, + expectSuccess: false, + assertAppBundle: false); + + if (isBrowser) { - Assert.Contains($"File in location $(WasmIcuDataFileName)={customIcu} cannot be found neither when used as absolute path nor a relative runtime pack path.", output); + if (isFilenameFormCorrect) + { + Assert.Contains($"Could not find $({customIcuProperty})={customIcu}, or when used as a path relative to the runtime pack", output); + } + else + { + Assert.Contains($"File name in $({customIcuProperty}) has to start with 'icudt'.", output); + } } else { - Assert.Contains($"Custom ICU file name in path $(WasmIcuDataFileName)={customIcu} must start with 'icudt'.", output); + // https://github.com/dotnet/runtime/issues/102743: console apps should also require "icudt" at the beginning, unify it + Assert.Contains($"File in location $({customIcuProperty})={customIcu} cannot be found neither when used as absolute path nor a relative runtime pack path.", output); } } } diff --git a/src/mono/wasm/Wasm.Build.Tests/IcuTestsBase.cs b/src/mono/wasm/Wasm.Build.Tests/IcuTestsBase.cs index 0c1f5b1356227..3aefcdd0762f6 100644 --- a/src/mono/wasm/Wasm.Build.Tests/IcuTestsBase.cs +++ b/src/mono/wasm/Wasm.Build.Tests/IcuTestsBase.cs @@ -3,6 +3,8 @@ using System; using System.IO; +using System.Collections.Generic; +using System.Threading.Tasks; using Xunit.Abstractions; using Xunit.Sdk; @@ -10,12 +12,14 @@ namespace Wasm.Build.Tests; -public abstract class IcuTestsBase : TestMainJsTestBase +public abstract class IcuTestsBase : WasmTemplateTestsBase { public IcuTestsBase(ITestOutputHelper output, SharedBuildPerTestClassFixture buildContext) : base(output, buildContext) { } private const string _fallbackSundayNameEnUS = "Sunday"; + protected static string[] templateTypes = { "wasmconsole", "wasmbrowser" }; + protected static bool[] boolOptions = { false, true }; protected record SundayNames { @@ -87,7 +91,7 @@ protected string GetProgramText(string testedLocales, bool onlyPredefinedCulture string expectedSundayName = (expectMissing && !onlyPredefinedCultures) ? fallbackSundayName - : testLocale.SundayName; + : testLocale.SundayName ?? fallbackSundayName; var actualLocalizedSundayName = culture.DateTimeFormat.GetDayName(new DateTime(2000,01,02).DayOfWeek); if (expectedSundayName != actualLocalizedSundayName) {{ @@ -102,28 +106,105 @@ protected string GetProgramText(string testedLocales, bool onlyPredefinedCulture public record Locale(string Code, string? SundayName); "; - protected void TestIcuShards(BuildArgs buildArgs, string shardName, string testedLocales, RunHost host, string id, bool onlyPredefinedCultures=false) + protected async Task TestIcuShards(string config, string templateType, bool aot, string shardName, string testedLocales, GlobalizationMode globalizationMode, bool onlyPredefinedCultures=false) { - string projectName = $"shard_{Path.GetFileName(shardName)}_{buildArgs.Config}_{buildArgs.AOT}"; - bool dotnetWasmFromRuntimePack = !(buildArgs.AOT || buildArgs.Config == "Release"); - - buildArgs = buildArgs with { ProjectName = projectName }; + bool isBrowser = templateType == "wasmbrowser"; + string icuProperty = isBrowser ? "BlazorIcuDataFileName" : "WasmIcuDataFileName"; // https://github.com/dotnet/runtime/issues/94133 // by default, we remove resource strings from an app. ICU tests are checking exception messages contents -> resource string keys are not enough - string extraProperties = $"{shardName}false"; + string extraProperties = $"<{icuProperty}>{shardName}false{aot}"; if (onlyPredefinedCultures) extraProperties = $"{extraProperties}true"; - buildArgs = ExpandBuildArgs(buildArgs, extraProperties: extraProperties); + await BuildAndRunIcuTest(config, templateType, aot, testedLocales, globalizationMode, extraProperties, onlyPredefinedCultures, icuFileName: shardName); + } + protected (BuildArgs buildArgs, string projectFile) CreateIcuProject( + string config, + string templateType, + bool aot, + string testedLocales, + string extraProperties = "", + bool onlyPredefinedCultures=false) + { + string id = $"icu_{config}_{aot}_{GetRandomId()}"; + string projectFile = CreateWasmTemplateProject(id, templateType); + string projectDirectory = Path.GetDirectoryName(projectFile)!; + string projectName = Path.GetFileNameWithoutExtension(projectFile); + var buildArgs = new BuildArgs(projectName, config, aot, id, null); + buildArgs = ExpandBuildArgs(buildArgs); + AddItemsPropertiesToProject(projectFile, extraProperties: extraProperties); + + string programPath = Path.Combine(projectDirectory, "Program.cs"); string programText = GetProgramText(testedLocales, onlyPredefinedCultures); + File.WriteAllText(programPath, programText); _testOutput.WriteLine($"----- Program: -----{Environment.NewLine}{programText}{Environment.NewLine}-------"); - (_, string output) = BuildProject(buildArgs, - id: id, + + bool isBrowser = templateType == "wasmbrowser"; + string mainPath = isBrowser ? Path.Combine("wwwroot", "main.js") : "main.mjs"; + var replacements = isBrowser ? new Dictionary { + { "runMain", "runMainAndExit" }, + { ".create()", ".withConsoleForwarding().withElementOnExit().withExitCodeLogging().create()" } + } : new Dictionary { + { ".create()", ".withConsoleForwarding().withElementOnExit().withExitCodeLogging().create()" } + }; + UpdateProjectFile(mainPath, replacements); + RemoveContentsFromProjectFile(mainPath, ".create();", "await runMainAndExit();"); + return (buildArgs, projectFile); + } + + protected string BuildIcuTest( + BuildArgs buildArgs, + bool isBrowser, + GlobalizationMode globalizationMode, + string icuFileName = "", + bool expectSuccess = true, + bool assertAppBundle = true) + { + bool dotnetWasmFromRuntimePack = !(buildArgs.AOT || buildArgs.Config == "Release"); + (string _, string buildOutput) = BuildTemplateProject(buildArgs, + id: buildArgs.Id, new BuildProjectOptions( - InitProject: () => File.WriteAllText(Path.Combine(_projectDir!, "Program.cs"), programText), DotnetWasmFromRuntimePack: dotnetWasmFromRuntimePack, - GlobalizationMode: GlobalizationMode.PredefinedIcu, - PredefinedIcudt: shardName)); + CreateProject: false, + HasV8Script: false, + MainJS: isBrowser ? "main.js" : "main.mjs", + Publish: true, + TargetFramework: BuildTestBase.DefaultTargetFramework, + UseCache: false, + IsBrowserProject: isBrowser, + GlobalizationMode: globalizationMode, + CustomIcuFile: icuFileName, + ExpectSuccess: expectSuccess, + AssertAppBundle: assertAppBundle + )); + return buildOutput; + } - string runOutput = RunAndTestWasmApp(buildArgs, buildDir: _projectDir, expectedExitCode: 42, host: host, id: id); + protected async Task BuildAndRunIcuTest( + string config, + string templateType, + bool aot, + string testedLocales, + GlobalizationMode globalizationMode, + string extraProperties = "", + bool onlyPredefinedCultures=false, + string language = "en-US", + string icuFileName = "") + { + try + { + bool isBrowser = templateType == "wasmbrowser"; + (BuildArgs buildArgs, string projectFile) = CreateIcuProject( + config, templateType, aot, testedLocales, extraProperties, onlyPredefinedCultures); + string buildOutput = BuildIcuTest(buildArgs, isBrowser, globalizationMode, icuFileName); + string runOutput = isBrowser ? + await RunBrowser(buildArgs.Config, projectFile, language) : + RunConsole(buildArgs, language: language); + return $"{buildOutput}\n{runOutput}"; + } + catch(Exception ex) + { + Console.WriteLine($"Exception: {ex}; _testOutput={_testOutput}"); + throw; + } } } diff --git a/src/mono/wasm/Wasm.Build.Tests/ProjectProviderBase.cs b/src/mono/wasm/Wasm.Build.Tests/ProjectProviderBase.cs index 69b6d07ff2d6c..6f440be448144 100644 --- a/src/mono/wasm/Wasm.Build.Tests/ProjectProviderBase.cs +++ b/src/mono/wasm/Wasm.Build.Tests/ProjectProviderBase.cs @@ -364,12 +364,12 @@ public void AssertIcuAssets(AssertBundleOptionsBase assertOptions, BootJsonData expected.Add("icudt_hybrid.dat"); expected.Add("segmentation-rules.json"); break; - case GlobalizationMode.PredefinedIcu: - if (string.IsNullOrEmpty(assertOptions.PredefinedIcudt)) - throw new ArgumentException("WasmBuildTest is invalid, value for predefinedIcudt is required when GlobalizationMode=PredefinedIcu."); + case GlobalizationMode.Custom: + if (string.IsNullOrEmpty(assertOptions.CustomIcuFile)) + throw new ArgumentException("WasmBuildTest is invalid, value for Custom globalization mode is required when GlobalizationMode=Custom."); // predefined ICU name can be identical with the icu files from runtime pack - expected.Add(Path.GetFileName(assertOptions.PredefinedIcudt)); + expected.Add(Path.GetFileName(assertOptions.CustomIcuFile)); break; case GlobalizationMode.Sharded: // icu shard chosen based on the locale @@ -401,12 +401,12 @@ public void AssertIcuAssets(AssertBundleOptionsBase assertOptions, BootJsonData } AssertFileNames(expected, actual); - if (assertOptions.GlobalizationMode is GlobalizationMode.PredefinedIcu) + if (assertOptions.GlobalizationMode is GlobalizationMode.Custom) { - string srcPath = assertOptions.PredefinedIcudt!; + string srcPath = assertOptions.CustomIcuFile!; string runtimePackDir = BuildTestBase.s_buildEnv.GetRuntimeNativeDir(assertOptions.TargetFramework, assertOptions.RuntimeType); if (!Path.IsPathRooted(srcPath)) - srcPath = Path.Combine(runtimePackDir, assertOptions.PredefinedIcudt!); + srcPath = Path.Combine(runtimePackDir, assertOptions.CustomIcuFile!); TestUtils.AssertSameFile(srcPath, actual.Single()); } } diff --git a/src/mono/wasm/Wasm.Build.Tests/Templates/WasmTemplateTests.cs b/src/mono/wasm/Wasm.Build.Tests/Templates/WasmTemplateTests.cs index b32b95debd879..319130a3ec08f 100644 --- a/src/mono/wasm/Wasm.Build.Tests/Templates/WasmTemplateTests.cs +++ b/src/mono/wasm/Wasm.Build.Tests/Templates/WasmTemplateTests.cs @@ -15,98 +15,33 @@ namespace Wasm.Build.Tests { - public class WasmTemplateTests : BlazorWasmTestBase + public class WasmTemplateTests : WasmTemplateTestsBase { public WasmTemplateTests(ITestOutputHelper output, SharedBuildPerTestClassFixture buildContext) : base(output, buildContext) { } - - private string StringReplaceWithAssert(string oldContent, string oldValue, string newValue) - { - string newContent = oldContent.Replace(oldValue, newValue); - if (oldValue != newValue && oldContent == newContent) - throw new XunitException($"Replacing '{oldValue}' with '{newValue}' did not change the content '{oldContent}'"); - - return newContent; - } - - private void UpdateBrowserProgramCs() - { - var path = Path.Combine(_projectDir!, "Program.cs"); - string text = File.ReadAllText(path); - text = StringReplaceWithAssert(text, "while(true)", $"int i = 0;{Environment.NewLine}while(i++ < 10)"); - text = StringReplaceWithAssert(text, "partial class StopwatchSample", $"return 42;{Environment.NewLine}partial class StopwatchSample"); - File.WriteAllText(path, text); - } - - private void UpdateConsoleProgramCs() - { - string programText = """ + + private static string s_consoleProgramUpdateText = """ Console.WriteLine("Hello, Console!"); for (int i = 0; i < args.Length; i ++) Console.WriteLine ($"args[{i}] = {args[i]}"); """; - var path = Path.Combine(_projectDir!, "Program.cs"); - string text = File.ReadAllText(path); - text = StringReplaceWithAssert(text, @"Console.WriteLine(""Hello, Console!"");", programText); - text = StringReplaceWithAssert(text, "return 0;", "return 42;"); - File.WriteAllText(path, text); - } - - private void UpdateBrowserMainJs(string targetFramework, string runtimeAssetsRelativePath = DefaultRuntimeAssetsRelativePath) + private Dictionary consoleProgramReplacements = new Dictionary { - base.UpdateBrowserMainJs( - (mainJsContent) => - { - // .withExitOnUnhandledError() is available only only >net7.0 - mainJsContent = StringReplaceWithAssert( - mainJsContent, - ".create()", - (targetFramework == "net8.0" || targetFramework == "net9.0") - ? ".withConsoleForwarding().withElementOnExit().withExitCodeLogging().withExitOnUnhandledError().create()" - : ".withConsoleForwarding().withElementOnExit().withExitCodeLogging().create()" - ); - - // dotnet.run() is already used in <= net8.0 - if (targetFramework != "net8.0") - mainJsContent = StringReplaceWithAssert(mainJsContent, "runMain()", "dotnet.run()"); - - mainJsContent = StringReplaceWithAssert(mainJsContent, "from './_framework/dotnet.js'", $"from '{runtimeAssetsRelativePath}dotnet.js'"); - - return mainJsContent; - }, - targetFramework, - runtimeAssetsRelativePath - ); - } - - private void UpdateConsoleMainJs() + { "Console.WriteLine(\"Hello, Console!\");", s_consoleProgramUpdateText }, + { "return 0;", "return 42;" } + }; + private Dictionary browserProgramReplacements = new Dictionary { - string mainJsPath = Path.Combine(_projectDir!, "main.mjs"); - string mainJsContent = File.ReadAllText(mainJsPath); - - mainJsContent = StringReplaceWithAssert(mainJsContent, ".create()", ".withConsoleForwarding().create()"); - - File.WriteAllText(mainJsPath, mainJsContent); - } - - private void UpdateMainJsEnvironmentVariables(params (string key, string value)[] variables) + { "while(true)", $"int i = 0;{Environment.NewLine}while(i++ < 10)" }, + { "partial class StopwatchSample", $"return 42;{Environment.NewLine}partial class StopwatchSample" } + }; + private Dictionary consoleMainJSReplacements = new Dictionary { - string mainJsPath = Path.Combine(_projectDir!, "main.mjs"); - string mainJsContent = File.ReadAllText(mainJsPath); - - StringBuilder js = new(); - foreach (var variable in variables) - { - js.Append($".withEnvironmentVariable(\"{variable.key}\", \"{variable.value}\")"); - } - - mainJsContent = StringReplaceWithAssert(mainJsContent, ".create()", js.ToString() + ".create()"); - - File.WriteAllText(mainJsPath, mainJsContent); - } + { ".create()", ".withConsoleForwarding().create()" } + }; [Theory, TestCategory("no-fingerprinting")] [InlineData("Debug")] @@ -117,7 +52,7 @@ public void BrowserBuildThenPublish(string config) string projectFile = CreateWasmTemplateProject(id, "wasmbrowser"); string projectName = Path.GetFileNameWithoutExtension(projectFile); - UpdateBrowserProgramCs(); + UpdateProjectFile("Program.cs", browserProgramReplacements); UpdateBrowserMainJs(DefaultTargetFramework); var buildArgs = new BuildArgs(projectName, config, false, id, null); @@ -176,7 +111,7 @@ public void ConsoleBuildThenPublish(string config) string projectFile = CreateWasmTemplateProject(id, "wasmconsole"); string projectName = Path.GetFileNameWithoutExtension(projectFile); - UpdateConsoleMainJs(); + UpdateProjectFile("main.mjs", consoleMainJSReplacements); var buildArgs = new BuildArgs(projectName, config, false, id, null); buildArgs = ExpandBuildArgs(buildArgs); @@ -242,8 +177,8 @@ private void ConsoleBuildAndRun(string config, bool relinking, string extraNewAr string projectFile = CreateWasmTemplateProject(id, "wasmconsole", extraNewArgs, addFrameworkArg: addFrameworkArg); string projectName = Path.GetFileNameWithoutExtension(projectFile); - UpdateConsoleProgramCs(); - UpdateConsoleMainJs(); + UpdateProjectFile("Program.cs", consoleProgramReplacements); + UpdateProjectFile("main.mjs", consoleMainJSReplacements); if (relinking) AddItemsPropertiesToProject(projectFile, "true"); @@ -307,7 +242,7 @@ private async Task BrowserRunTwiceWithAndThenWithoutBuildAsync(string config, st string id = $"browser_{config}_{GetRandomId()}"; string projectFile = CreateWasmTemplateProject(id, "wasmbrowser"); - UpdateBrowserProgramCs(); + UpdateProjectFile("Program.cs", browserProgramReplacements); UpdateBrowserMainJs(DefaultTargetFramework); if (!string.IsNullOrEmpty(extraProperties)) @@ -341,8 +276,8 @@ private Task ConsoleRunWithAndThenWithoutBuildAsync(string config, string extraP string id = $"console_{config}_{GetRandomId()}"; string projectFile = CreateWasmTemplateProject(id, "wasmconsole"); - UpdateConsoleProgramCs(); - UpdateConsoleMainJs(); + UpdateProjectFile("Program.cs", consoleProgramReplacements); + UpdateProjectFile("main.mjs", consoleMainJSReplacements); if (!string.IsNullOrEmpty(extraProperties)) AddItemsPropertiesToProject(projectFile, extraProperties: extraProperties); @@ -399,8 +334,8 @@ public void ConsolePublishAndRun(string config, bool aot, bool relinking) string projectFile = CreateWasmTemplateProject(id, "wasmconsole"); string projectName = Path.GetFileNameWithoutExtension(projectFile); - UpdateConsoleProgramCs(); - UpdateConsoleMainJs(); + UpdateProjectFile("Program.cs", consoleProgramReplacements); + UpdateProjectFile("main.mjs", consoleMainJSReplacements); if (aot) { @@ -470,7 +405,7 @@ public async Task BrowserBuildAndRun(string extraNewArgs, string targetFramework CreateWasmTemplateProject(id, "wasmbrowser", extraNewArgs, addFrameworkArg: extraNewArgs.Length == 0); if (targetFramework != "net8.0") - UpdateBrowserProgramCs(); + UpdateProjectFile("Program.cs", browserProgramReplacements); UpdateBrowserMainJs(targetFramework, runtimeAssetsRelativePath); @@ -554,8 +489,8 @@ public void Test_WasmStripILAfterAOT(string stripILAfterAOT, bool expectILStripp string projectDirectory = Path.GetDirectoryName(projectFile)!; bool aot = true; - UpdateConsoleProgramCs(); - UpdateConsoleMainJs(); + UpdateProjectFile("Program.cs", consoleProgramReplacements); + UpdateProjectFile("main.mjs", consoleMainJSReplacements); string extraProperties = "true"; if (!string.IsNullOrEmpty(stripILAfterAOT)) diff --git a/src/mono/wasm/Wasm.Build.Tests/Templates/WasmTemplateTestsBase.cs b/src/mono/wasm/Wasm.Build.Tests/Templates/WasmTemplateTestsBase.cs new file mode 100644 index 0000000000000..0354ae1951dab --- /dev/null +++ b/src/mono/wasm/Wasm.Build.Tests/Templates/WasmTemplateTestsBase.cs @@ -0,0 +1,125 @@ +// 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 System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Xunit; +using Xunit.Abstractions; +using Xunit.Sdk; + +#nullable enable + +namespace Wasm.Build.Tests +{ + public class WasmTemplateTestsBase : BlazorWasmTestBase + { + public WasmTemplateTestsBase(ITestOutputHelper output, SharedBuildPerTestClassFixture buildContext) + : base(output, buildContext) + { + } + + private string StringReplaceWithAssert(string oldContent, string oldValue, string newValue) + { + string newContent = oldContent.Replace(oldValue, newValue); + if (oldValue != newValue && oldContent == newContent) + throw new XunitException($"Replacing '{oldValue}' with '{newValue}' did not change the content '{oldContent}'"); + + return newContent; + } + + protected void UpdateProjectFile(string pathRelativeToProjectDir, Dictionary replacements) + { + var path = Path.Combine(_projectDir!, pathRelativeToProjectDir); + string text = File.ReadAllText(path); + foreach (var replacement in replacements) + { + text = StringReplaceWithAssert(text, replacement.Key, replacement.Value); + } + File.WriteAllText(path, text); + } + + protected void RemoveContentsFromProjectFile(string pathRelativeToProjectDir, string afterMarker, string beforeMarker) + { + var path = Path.Combine(_projectDir!, pathRelativeToProjectDir); + string text = File.ReadAllText(path); + int start = text.IndexOf(afterMarker); + int end = text.IndexOf(beforeMarker, start); + if (start == -1 || end == -1) + throw new XunitException($"Start or end marker not found in '{path}'"); + start += afterMarker.Length; + text = text.Remove(start, end - start); + // separate the markers with a new line + text = text.Insert(start, "\n"); + File.WriteAllText(path, text); + } + + protected void UpdateBrowserMainJs(string targetFramework, string runtimeAssetsRelativePath = DefaultRuntimeAssetsRelativePath) + { + base.UpdateBrowserMainJs( + (mainJsContent) => + { + // .withExitOnUnhandledError() is available only only >net7.0 + mainJsContent = StringReplaceWithAssert( + mainJsContent, + ".create()", + (targetFramework == "net8.0" || targetFramework == "net9.0") + ? ".withConsoleForwarding().withElementOnExit().withExitCodeLogging().withExitOnUnhandledError().create()" + : ".withConsoleForwarding().withElementOnExit().withExitCodeLogging().create()" + ); + + // dotnet.run() is already used in <= net8.0 + if (targetFramework != "net8.0") + mainJsContent = StringReplaceWithAssert(mainJsContent, "runMain()", "dotnet.run()"); + + mainJsContent = StringReplaceWithAssert(mainJsContent, "from './_framework/dotnet.js'", $"from '{runtimeAssetsRelativePath}dotnet.js'"); + + return mainJsContent; + }, + targetFramework, + runtimeAssetsRelativePath + ); + } + + protected void UpdateMainJsEnvironmentVariables(params (string key, string value)[] variables) + { + string mainJsPath = Path.Combine(_projectDir!, "main.mjs"); + string mainJsContent = File.ReadAllText(mainJsPath); + + StringBuilder js = new(); + foreach (var variable in variables) + { + js.Append($".withEnvironmentVariable(\"{variable.key}\", \"{variable.value}\")"); + } + + mainJsContent = StringReplaceWithAssert(mainJsContent, ".create()", js.ToString() + ".create()"); + + File.WriteAllText(mainJsPath, mainJsContent); + } + + protected string RunConsole(BuildArgs buildArgs, int expectedExitCode = 42, string language = "en-US") + { + CommandResult res = new RunCommand(s_buildEnv, _testOutput) + .WithWorkingDirectory(_projectDir!) + .WithEnvironmentVariable("LANG", language) + .ExecuteWithCapturedOutput($"run --no-silent --no-build -c {buildArgs.Config}") + .EnsureExitCode(expectedExitCode); + return res.Output; + } + + protected async Task RunBrowser(string config, string projectFile, string language = "en-US") + { + using var runCommand = new RunCommand(s_buildEnv, _testOutput) + .WithWorkingDirectory(_projectDir!); + + await using var runner = new BrowserRunner(_testOutput); + var page = await runner.RunAsync(runCommand, $"run --no-silent -c {config} --no-build --project \"{projectFile}\" --forward-console", language: language); + await runner.WaitForExitMessageAsync(TimeSpan.FromMinutes(2)); + Assert.Contains("WASM EXIT 42", string.Join(Environment.NewLine, runner.OutputLines)); + return string.Join("\n", runner.OutputLines); + } + } +} diff --git a/src/mono/wasm/Wasm.Build.Tests/TestMainJsProjectProvider.cs b/src/mono/wasm/Wasm.Build.Tests/TestMainJsProjectProvider.cs index 4ca29b0f3298f..39b1effac5867 100644 --- a/src/mono/wasm/Wasm.Build.Tests/TestMainJsProjectProvider.cs +++ b/src/mono/wasm/Wasm.Build.Tests/TestMainJsProjectProvider.cs @@ -105,7 +105,7 @@ public void AssertBundle(BuildArgs buildArgs, BuildProjectOptions buildProjectOp MainJS: buildProjectOptions.MainJS ?? "test-main.js", GlobalizationMode: buildProjectOptions.GlobalizationMode, HasV8Script: buildProjectOptions.HasV8Script, - PredefinedIcudt: buildProjectOptions.PredefinedIcudt ?? string.Empty, + CustomIcuFile: buildProjectOptions.CustomIcuFile ?? string.Empty, IsBrowserProject: buildProjectOptions.IsBrowserProject, ExpectedFileType: expectedFileType, ExpectSymbolsFile: !buildArgs.AOT); diff --git a/src/mono/wasm/Wasm.Build.Tests/WasmSdkBasedProjectProvider.cs b/src/mono/wasm/Wasm.Build.Tests/WasmSdkBasedProjectProvider.cs index bd35644b0e8d0..16108e5af24ec 100644 --- a/src/mono/wasm/Wasm.Build.Tests/WasmSdkBasedProjectProvider.cs +++ b/src/mono/wasm/Wasm.Build.Tests/WasmSdkBasedProjectProvider.cs @@ -73,7 +73,7 @@ public void AssertBundle(BuildArgs buildArgs, BuildProjectOptions buildProjectOp IsPublish: buildProjectOptions.Publish, TargetFramework: buildProjectOptions.TargetFramework, BinFrameworkDir: buildProjectOptions.BinFrameworkDir ?? FindBinFrameworkDir(buildArgs.Config, buildProjectOptions.Publish, buildProjectOptions.TargetFramework), - PredefinedIcudt: buildProjectOptions.PredefinedIcudt, + CustomIcuFile: buildProjectOptions.CustomIcuFile, GlobalizationMode: buildProjectOptions.GlobalizationMode, AssertSymbolsFile: false, ExpectedFileType: buildProjectOptions.Publish && buildArgs.Config == "Release" ? NativeFilesType.Relinked : NativeFilesType.FromRuntimePack From c315bac8ae7cfc613cb446ca594a17d07d02aa3d Mon Sep 17 00:00:00 2001 From: Meri Khamoyan <96171496+mkhamoyan@users.noreply.github.com> Date: Fri, 27 Sep 2024 14:13:11 +0200 Subject: [PATCH 6/6] [iOS] Enable and disable tests for hybrid globalization on Apple (#108187) Enable and disable tests for hybrid globalization on Apple --- .../features/globalization-hybrid-mode.md | 6 ++ .../Common/tests/Tests/System/StringTests.cs | 76 ++++++++++--------- .../Data/SqlTypes/SqlStringSortingTest.cs | 5 +- .../GetStringComparerTests.cs | 3 +- .../CompareInfo/CompareInfoTests.HashCode.cs | 5 +- .../AssemblyNameTests.cs | 3 +- .../System/DateTimeTests.cs | 7 +- .../System/Text/RuneTests.cs | 4 +- 8 files changed, 61 insertions(+), 48 deletions(-) diff --git a/docs/design/features/globalization-hybrid-mode.md b/docs/design/features/globalization-hybrid-mode.md index bdbdb27b09e42..1dec94595b1c4 100644 --- a/docs/design/features/globalization-hybrid-mode.md +++ b/docs/design/features/globalization-hybrid-mode.md @@ -398,6 +398,12 @@ Affected public APIs: - String.Compare, - String.Equals. +Mapped to Apple Native API `compare:options:range:locale:`(https://developer.apple.com/documentation/foundation/nsstring/1414561-compare?language=objc) +This implementation uses normalization techniques such as `precomposedStringWithCanonicalMapping`, +which can result in behavior differences compared to other platforms. +Specifically, the use of precomposed strings and additional locale-based string folding can affect the results of comparisons. +Due to these differences, the exact result of string compariso on Apple platforms may differ. + The number of `CompareOptions` and `NSStringCompareOptions` combinations are limited. Originally supported combinations can be found [here for CompareOptions](https://learn.microsoft.com/dotnet/api/system.globalization.compareoptions) and [here for NSStringCompareOptions](https://developer.apple.com/documentation/foundation/nsstringcompareoptions). - `IgnoreSymbols` is not supported because there is no equivalent in native api. Throws `PlatformNotSupportedException`. diff --git a/src/libraries/Common/tests/Tests/System/StringTests.cs b/src/libraries/Common/tests/Tests/System/StringTests.cs index b756c06d87d6f..45c3bce3c343a 100644 --- a/src/libraries/Common/tests/Tests/System/StringTests.cs +++ b/src/libraries/Common/tests/Tests/System/StringTests.cs @@ -1011,7 +1011,6 @@ public static void MakeSureNoCompareToChecksGoOutOfRange_StringComparison() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/95338", typeof(PlatformDetection), nameof(PlatformDetection.IsHybridGlobalizationOnApplePlatform))] public static void CompareToNoMatch_StringComparison() { for (int length = 1; length < 150; length++) @@ -1035,24 +1034,29 @@ public static void CompareToNoMatch_StringComparison() var secondSpan = new ReadOnlySpan(second); Assert.True(0 > firstSpan.CompareTo(secondSpan, StringComparison.Ordinal)); - // Due to differences in the implementation, the exact result of CompareTo will not necessarily match with string.Compare. - // However, the sign will match, which is what defines correctness. - Assert.Equal( - Math.Sign(string.Compare(firstSpan.ToString(), secondSpan.ToString(), StringComparison.OrdinalIgnoreCase)), - Math.Sign(firstSpan.CompareTo(secondSpan, StringComparison.OrdinalIgnoreCase))); - - Assert.Equal( - string.Compare(firstSpan.ToString(), secondSpan.ToString(), StringComparison.CurrentCulture), - firstSpan.CompareTo(secondSpan, StringComparison.CurrentCulture)); - Assert.Equal( - string.Compare(firstSpan.ToString(), secondSpan.ToString(), StringComparison.CurrentCultureIgnoreCase), - firstSpan.CompareTo(secondSpan, StringComparison.CurrentCultureIgnoreCase)); - Assert.Equal( - string.Compare(firstSpan.ToString(), secondSpan.ToString(), StringComparison.InvariantCulture), - firstSpan.CompareTo(secondSpan, StringComparison.InvariantCulture)); - Assert.Equal( - string.Compare(firstSpan.ToString(), secondSpan.ToString(), StringComparison.InvariantCultureIgnoreCase), - firstSpan.CompareTo(secondSpan, StringComparison.InvariantCultureIgnoreCase)); + // On Apple platforms, string comparison is handled by native Apple functions, which apply normalization techniques + // like `precomposedStringWithCanonicalMapping`. This can lead to differences in behavior compared to other platforms. + if (PlatformDetection.IsNotHybridGlobalizationOnApplePlatform) + { + // Due to differences in the implementation, the exact result of CompareTo will not necessarily match with string.Compare. + // However, the sign will match, which is what defines correctness. + Assert.Equal( + Math.Sign(string.Compare(firstSpan.ToString(), secondSpan.ToString(), StringComparison.OrdinalIgnoreCase)), + Math.Sign(firstSpan.CompareTo(secondSpan, StringComparison.OrdinalIgnoreCase))); + + Assert.Equal( + string.Compare(firstSpan.ToString(), secondSpan.ToString(), StringComparison.CurrentCulture), + firstSpan.CompareTo(secondSpan, StringComparison.CurrentCulture)); + Assert.Equal( + string.Compare(firstSpan.ToString(), secondSpan.ToString(), StringComparison.CurrentCultureIgnoreCase), + firstSpan.CompareTo(secondSpan, StringComparison.CurrentCultureIgnoreCase)); + Assert.Equal( + string.Compare(firstSpan.ToString(), secondSpan.ToString(), StringComparison.InvariantCulture), + firstSpan.CompareTo(secondSpan, StringComparison.InvariantCulture)); + Assert.Equal( + string.Compare(firstSpan.ToString(), secondSpan.ToString(), StringComparison.InvariantCultureIgnoreCase), + firstSpan.CompareTo(secondSpan, StringComparison.InvariantCultureIgnoreCase)); + } } } } @@ -1286,7 +1290,6 @@ public static void ContainsMatchDifferentSpans_StringComparison() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/95338", typeof(PlatformDetection), nameof(PlatformDetection.IsHybridGlobalizationOnApplePlatform))] public static void ContainsNoMatch_StringComparison() { for (int length = 1; length < 150; length++) @@ -1312,19 +1315,24 @@ public static void ContainsNoMatch_StringComparison() Assert.False(firstSpan.Contains(secondSpan, StringComparison.OrdinalIgnoreCase)); - // Different behavior depending on OS - Assert.Equal( - firstSpan.ToString().StartsWith(secondSpan.ToString(), StringComparison.CurrentCulture), - firstSpan.Contains(secondSpan, StringComparison.CurrentCulture)); - Assert.Equal( - firstSpan.ToString().StartsWith(secondSpan.ToString(), StringComparison.CurrentCultureIgnoreCase), - firstSpan.Contains(secondSpan, StringComparison.CurrentCultureIgnoreCase)); - Assert.Equal( - firstSpan.ToString().StartsWith(secondSpan.ToString(), StringComparison.InvariantCulture), - firstSpan.Contains(secondSpan, StringComparison.InvariantCulture)); - Assert.Equal( - firstSpan.ToString().StartsWith(secondSpan.ToString(), StringComparison.InvariantCultureIgnoreCase), - firstSpan.Contains(secondSpan, StringComparison.InvariantCultureIgnoreCase)); + // On Apple platforms, string comparison is handled by native Apple functions, which apply normalization techniques + // like `precomposedStringWithCanonicalMapping`. This can lead to differences in behavior compared to other platforms. + if (PlatformDetection.IsNotHybridGlobalizationOnApplePlatform) + { + // Different behavior depending on OS + Assert.Equal( + firstSpan.ToString().StartsWith(secondSpan.ToString(), StringComparison.CurrentCulture), + firstSpan.Contains(secondSpan, StringComparison.CurrentCulture)); + Assert.Equal( + firstSpan.ToString().StartsWith(secondSpan.ToString(), StringComparison.CurrentCultureIgnoreCase), + firstSpan.Contains(secondSpan, StringComparison.CurrentCultureIgnoreCase)); + Assert.Equal( + firstSpan.ToString().StartsWith(secondSpan.ToString(), StringComparison.InvariantCulture), + firstSpan.Contains(secondSpan, StringComparison.InvariantCulture)); + Assert.Equal( + firstSpan.ToString().StartsWith(secondSpan.ToString(), StringComparison.InvariantCultureIgnoreCase), + firstSpan.Contains(secondSpan, StringComparison.InvariantCultureIgnoreCase)); + } } } } @@ -2113,7 +2121,6 @@ public static void EndsWithMatchDifferentSpans_StringComparison() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/95338", typeof(PlatformDetection), nameof(PlatformDetection.IsHybridGlobalizationOnApplePlatform))] public static void EndsWithNoMatch_StringComparison() { for (int length = 1; length < 150; length++) @@ -7379,7 +7386,6 @@ public static void StartsWithMatchDifferentSpans_StringComparison() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/95338", typeof(PlatformDetection), nameof(PlatformDetection.IsHybridGlobalizationOnApplePlatform))] public static void StartsWithNoMatch_StringComparison() { for (int length = 1; length < 150; length++) diff --git a/src/libraries/System.Data.Common/tests/System/Data/SqlTypes/SqlStringSortingTest.cs b/src/libraries/System.Data.Common/tests/System/Data/SqlTypes/SqlStringSortingTest.cs index 7fa294f8fe95d..7e2277986c5ac 100644 --- a/src/libraries/System.Data.Common/tests/System/Data/SqlTypes/SqlStringSortingTest.cs +++ b/src/libraries/System.Data.Common/tests/System/Data/SqlTypes/SqlStringSortingTest.cs @@ -37,8 +37,9 @@ public static class SqlStringSortingTest private static readonly UnicodeEncoding s_unicodeEncoding = new UnicodeEncoding(bigEndian: false, byteOrderMark: false, throwOnInvalidBytes: true); - [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotInvariantGlobalization))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/95338", typeof(PlatformDetection), nameof(PlatformDetection.IsHybridGlobalizationOnApplePlatform))] + // On Apple platforms, the string comparison implementation relies on native Apple functions which uses normalization techniques, which can result in behavior differences compared to other platforms. + // Specifically, the use of precomposed strings and additional locale-based string folding can affect the results of comparisons with certain options like `IgnoreKanaType`. + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotInvariantGlobalization), nameof(PlatformDetection.IsNotHybridGlobalizationOnApplePlatform))] [InlineData("ja-JP", 0x0411)] // Japanese - Japan [InlineData("ar-SA", 0x0401)] // Arabic - Saudi Arabia [InlineData("de-DE", 0x0407)] // German - Germany diff --git a/src/libraries/System.Runtime/tests/System.Globalization.Extensions.Tests/GetStringComparerTests.cs b/src/libraries/System.Runtime/tests/System.Globalization.Extensions.Tests/GetStringComparerTests.cs index 3046f50a98c3e..0735620db91af 100644 --- a/src/libraries/System.Runtime/tests/System.Globalization.Extensions.Tests/GetStringComparerTests.cs +++ b/src/libraries/System.Runtime/tests/System.Globalization.Extensions.Tests/GetStringComparerTests.cs @@ -19,8 +19,7 @@ public void GetStringComparer_Invalid() AssertExtensions.Throws("options", () => new CultureInfo("tr-TR").CompareInfo.GetStringComparer(CompareOptions.OrdinalIgnoreCase | CompareOptions.IgnoreCase)); } - [Theory] - [ActiveIssue("https://github.com/dotnet/runtime/issues/95338", typeof(PlatformDetection), nameof(PlatformDetection.IsHybridGlobalizationOnApplePlatform))] + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnApplePlatform))] [InlineData("hello", "hello", "fr-FR", CompareOptions.IgnoreCase, 0, 0)] [InlineData("hello", "HELLo", "fr-FR", CompareOptions.IgnoreCase, 0, 0)] [InlineData("hello", null, "fr-FR", CompareOptions.IgnoreCase, 1, 1)] diff --git a/src/libraries/System.Runtime/tests/System.Globalization.Tests/CompareInfo/CompareInfoTests.HashCode.cs b/src/libraries/System.Runtime/tests/System.Globalization.Tests/CompareInfo/CompareInfoTests.HashCode.cs index e05bcbf453e91..aa05f00d4b6e6 100644 --- a/src/libraries/System.Runtime/tests/System.Globalization.Tests/CompareInfo/CompareInfoTests.HashCode.cs +++ b/src/libraries/System.Runtime/tests/System.Globalization.Tests/CompareInfo/CompareInfoTests.HashCode.cs @@ -14,8 +14,9 @@ public class CompareInfoHashCodeTests : CompareInfoTestsBase { [OuterLoop] - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsIcuGlobalization))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/95338", typeof(PlatformDetection), nameof(PlatformDetection.IsHybridGlobalizationOnApplePlatform))] + // On Apple platforms, string comparison is handled by native Apple functions, which apply normalization techniques + // like `precomposedStringWithCanonicalMapping`. This can lead to differences in behavior compared to other platforms. + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsIcuGlobalization), nameof(PlatformDetection.IsNotHybridGlobalizationOnApplePlatform))] public void CheckHashingInLineWithEqual() { int additionalCollisions = 0; diff --git a/src/libraries/System.Runtime/tests/System.Reflection.Tests/AssemblyNameTests.cs b/src/libraries/System.Runtime/tests/System.Reflection.Tests/AssemblyNameTests.cs index 873d3d474545a..f5225adc18b78 100644 --- a/src/libraries/System.Runtime/tests/System.Reflection.Tests/AssemblyNameTests.cs +++ b/src/libraries/System.Runtime/tests/System.Reflection.Tests/AssemblyNameTests.cs @@ -221,8 +221,7 @@ public void CultureName_Set(AssemblyName assemblyName, string originalCultureNam Assert.Equal(new AssemblyName(expectedEqualString).FullName, assemblyName.FullName); } - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/95338", typeof(PlatformDetection), nameof(PlatformDetection.IsHybridGlobalizationOnApplePlatform))] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnApplePlatform))] public void CultureName_Set_Invalid_ThrowsCultureNotFoundException() { var assemblyName = new AssemblyName("Test"); diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/DateTimeTests.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/DateTimeTests.cs index 554d128a15933..ef51adaecc6a5 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/DateTimeTests.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/DateTimeTests.cs @@ -1982,7 +1982,10 @@ public static IEnumerable Parse_ValidInput_Succeeds_MemberData() yield return new object[] { "#2020-5-7T09:37:00.0000000+00:00#\0", CultureInfo.InvariantCulture, TimeZoneInfo.ConvertTimeFromUtc(new DateTime(2020, 5, 7, 9, 37, 0, DateTimeKind.Utc), TimeZoneInfo.Local) }; yield return new object[] { "2020-5-7T09:37:00.0000000+00:00", CultureInfo.InvariantCulture, TimeZoneInfo.ConvertTimeFromUtc(new DateTime(2020, 5, 7, 9, 37, 0, DateTimeKind.Utc), TimeZoneInfo.Local) }; - if (PlatformDetection.IsNotInvariantGlobalization) + // On Apple platforms, the handling calendars relies on native Apple APIs (NSCalendar). + // These APIs can cause differences in behavior when parsing or formatting dates compared to other platforms. + // Specifically, the way Apple handles calendar identifiers and date formats for cultures like "he-IL" may lead to variations in the output. + if (PlatformDetection.IsNotInvariantGlobalization && PlatformDetection.IsNotHybridGlobalizationOnApplePlatform) { DateTime today = DateTime.Today; var hebrewCulture = new CultureInfo("he-IL"); @@ -2003,7 +2006,6 @@ public static IEnumerable Parse_ValidInput_Succeeds_MemberData() } [Theory] - [ActiveIssue("https://github.com/dotnet/runtime/issues/95338", typeof(PlatformDetection), nameof(PlatformDetection.IsHybridGlobalizationOnApplePlatform))] [MemberData(nameof(Parse_ValidInput_Succeeds_MemberData))] public static void Parse_ValidInput_Succeeds(string input, CultureInfo culture, DateTime? expected) { @@ -2464,7 +2466,6 @@ public static IEnumerable ToString_MatchesExpected_MemberData() } [Theory] - [ActiveIssue("https://github.com/dotnet/runtime/issues/95338", typeof(PlatformDetection), nameof(PlatformDetection.IsHybridGlobalizationOnApplePlatform))] [MemberData(nameof(Parse_ValidInput_Succeeds_MemberData))] public static void Parse_Span_ValidInput_Succeeds(string input, CultureInfo culture, DateTime? expected) { diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Text/RuneTests.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Text/RuneTests.cs index f7e20fbfe8938..578d09f3eaad6 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Text/RuneTests.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Text/RuneTests.cs @@ -57,7 +57,8 @@ public static void Casing_Invariant(int original, int upper, int lower) Assert.Equal(new Rune(lower), Rune.ToLowerInvariant(rune)); } - [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsIcuGlobalizationAndNotHybridOnBrowser))] + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsIcuGlobalizationAndNotHybridOnBrowser), nameof(PlatformDetection.IsNotHybridGlobalizationOnApplePlatform))] + // HybridGlobalization on Apple mobile platforms has issues with casing dotless I // HybridGlobalization on Browser uses Invariant HashCode and SortKey, so its effect does not match this of ICU [InlineData('0', '0', '0')] [InlineData('a', 'A', 'a')] @@ -71,7 +72,6 @@ public static void Casing_Invariant(int original, int upper, int lower) [InlineData('\u0131', '\u0131', '\u0131')] // U+0131 LATIN SMALL LETTER DOTLESS I [InlineData(0x10400, 0x10400, 0x10428)] // U+10400 DESERET CAPITAL LETTER LONG I [InlineData(0x10428, 0x10400, 0x10428)] // U+10428 DESERET SMALL LETTER LONG I - [ActiveIssue("https://github.com/dotnet/runtime/issues/95338", typeof(PlatformDetection), nameof(PlatformDetection.IsHybridGlobalizationOnApplePlatform))] public static void ICU_Casing_Invariant(int original, int upper, int lower) { var rune = new Rune(original);