Skip to content

Simplify icudat file lookup by specifying ICU_DAT_FILE_PATH as a RuntimeHostConfigurationOption#18914

Merged
ivanpovazan merged 3 commits intodotnet:net8.0from
ivanpovazan:new-find-icu-data
Sep 8, 2023
Merged

Simplify icudat file lookup by specifying ICU_DAT_FILE_PATH as a RuntimeHostConfigurationOption#18914
ivanpovazan merged 3 commits intodotnet:net8.0from
ivanpovazan:new-find-icu-data

Conversation

@ivanpovazan
Copy link
Copy Markdown
Member

Fixes #18471

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@ivanpovazan
Copy link
Copy Markdown
Member Author

There seems to be an issue with generator tests in Windows integration jobs, where it fails with:

"D:\Program Files\PowerShell\7\pwsh.exe" -NoLogo -NoProfile -NonInteractive -ExecutionPolicy Unrestricted -Command ". 'D:\AzDO\_work\_temp\96a6f4b1-a5c6-4e9b-b5ff-6d9e90781870.ps1'"
D:\AzDO\_work\1\s/xamarin-macios/tools/devops/automation/scripts/run-generator-tests-on-windows.ps1 : The term 'D:\AzDO\_work\1\s/xamarin-macios/tools/devops/automation/scripts/run-generator-tests-on-windows.ps1' is not recognized as a name of a cmdlet, function, script file, or executable program.

although it doesn't seem to be related to this change.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

contents.AppendLine ("static void xamarin_initialize_dotnet ()");
contents.AppendLine ("{");
if (Configuration.InvariantGlobalization) {
contents.AppendLine ("\tsetenv (\"DOTNET_SYSTEM_GLOBALIZATION_INVARIANT\", \"1\", 1);");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

for a future PR: this could be a runtime config option too: System.Globalization.Invariant (https://learn.microsoft.com/en-us/dotnet/core/runtime-config/globalization)

or actually we might not even need to do anything if the InvariantGlobalization msbuild property is set it should get turned into the runtime config automatically?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, this is from the early .NET 5 days, so the runtime config option probably didn't work then.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The DOTNET_SYSTEM_GLOBALIZATION_HYBRID environment variable is pretty new though:

3f6d43c

so maybe @mkhamoyan knows if these environment variables are really needed or not?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I added DOTNET_SYSTEM_GLOBALIZATION_HYBRID following to the logic done for Invariant mode. Not sure if these env variables are really needed or not.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

@akoeplinger akoeplinger Sep 6, 2023

Choose a reason for hiding this comment

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

Do you want me to remove setting up env variables explicitly in xamarin_initialize_dotnet?

we can do that in a separate PR after verifying that the switches do end up in runtimeconfig.bin etc. but yeah, it'd be nice to reduce custom logic

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I checked this by building MySingleView and inspecting MySingleView.runtimeconfig.json:

    "configProperties": {
      "Microsoft.Extensions.DependencyInjection.VerifyOpenGenericServiceTrimmability": true,
      "System.AggressiveAttributeTrimming": false,
      "System.ComponentModel.TypeConverter.EnableUnsafeBinaryFormatterInDesigntimeLicenseContextSerialization": false,
      "System.Diagnostics.Debugger.IsSupported": true,
      "System.Diagnostics.Tracing.EventSource.IsSupported": false,
      "System.Globalization.Invariant": false,
      "System.Globalization.Hybrid": false,
      "System.Net.Http.EnableActivityPropagation": false,
      "System.Net.Http.UseNativeHttpHandler": true,
      "System.Reflection.NullabilityInfoContext.IsSupported": false,
      "System.Resources.ResourceManager.AllowCustomResourceTypes": false,
      "System.Resources.UseSystemResourceKeys": false,
      "System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported": false,
      "System.Runtime.InteropServices.BuiltInComInterop.IsSupported": false,
      "System.Runtime.InteropServices.EnableConsumingManagedCodeFromNativeHosting": false,
      "System.Runtime.InteropServices.EnableCppCLIHostActivation": false,
      "System.Runtime.InteropServices.Marshalling.EnableGeneratedComInterfaceComImportInterop": false,
      "System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization": false,
      "System.StartupHookProvider.IsSupported": false,
      "System.Text.Encoding.EnableUnsafeUTF7Encoding": false,
      "System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault": true,
      "System.Threading.Thread.EnableAutoreleasePool": true,
      "System.Linq.Expressions.CanEmitObjectArrayDelegate": false,
      "ICU_DAT_FILE_PATH": "icudt.dat"
    }

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I have included the clean up in this PR, will see what the tests say

Comment thread msbuild/Xamarin.Shared/Xamarin.Shared.targets
@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

✅ API diff for current PR / commit

Legacy Xamarin (No breaking changes)
  • iOS (no change detected)
  • tvOS (no change detected)
  • watchOS (no change detected)
  • macOS (no change detected)
NET (empty diffs)
  • iOS: (empty diff detected)
  • tvOS: (empty diff detected)
  • MacCatalyst: (empty diff detected)
  • macOS: (empty diff detected)

✅ API diff vs stable

Legacy Xamarin (No breaking changes)
.NET (No breaking changes)
Legacy Xamarin (stable) vs .NET

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 1e06cef1e74bc16815aeceb2ee51d89ee3dc3a3f [PR build]

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

🔥 [PR Build] Build failed 🔥

Build failed for the job 'Build macOS tests'

Pipeline on Agent
Hash: 1e06cef1e74bc16815aeceb2ee51d89ee3dc3a3f [PR build]

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

💻 [CI Build] Windows Integration Tests passed 💻

All Windows Integration Tests passed.

Pipeline on Agent
Hash: 1e06cef1e74bc16815aeceb2ee51d89ee3dc3a3f [PR build]

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

All tests on macOS M1 - Mac Big Sur (11.5) passed.

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻

All tests on macOS M1 - Mac Ventura (13.0) passed.

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

🚀 [CI Build] Test results 🚀

Test results

✅ All tests passed on VSTS: simulator tests.

🎉 All 233 tests passed 🎉

Tests counts

✅ bcl: All 69 tests passed. Html Report (VSDrops) Download
✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests: All 1 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 7 tests passed. Html Report (VSDrops) Download
✅ framework: All 8 tests passed. Html Report (VSDrops) Download
✅ generator: All 2 tests passed. Html Report (VSDrops) Download
✅ interdependent_binding_projects: All 7 tests passed. Html Report (VSDrops) Download
✅ install_source: All 1 tests passed. Html Report (VSDrops) Download
✅ introspection: All 8 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ linker: All 65 tests passed. Html Report (VSDrops) Download
✅ mac_binding_project: All 1 tests passed. Html Report (VSDrops) Download
✅ mmp: All 2 tests passed. Html Report (VSDrops) Download
✅ mononative: All 6 tests passed. Html Report (VSDrops) Download
✅ monotouch: All 39 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ mtouch: All 1 tests passed. Html Report (VSDrops) Download
✅ xammac: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 8 tests passed. Html Report (VSDrops) Download
✅ xtro: All 2 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: 1e06cef1e74bc16815aeceb2ee51d89ee3dc3a3f [PR build]

@SamMonoRT
Copy link
Copy Markdown
Member

/cc @directhex fyi

@ivanpovazan ivanpovazan merged commit bf77022 into dotnet:net8.0 Sep 8, 2023
@ivanpovazan ivanpovazan deleted the new-find-icu-data branch September 13, 2023 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants