Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Globalization] Exclude not used ICU Files #20828

Closed
wants to merge 17 commits into from

Conversation

mkhamoyan
Copy link
Contributor

We don't use anymore icu .dat files.

Contributes to dotnet/runtime#99521

rolfbjarne and others added 16 commits July 1, 2024 18:25
This reverts commit f8552e9.

This change is only supposed to be released with .NET 9, and we might
release new .NET 8 updates from main. Thus we need to make sure these
changes are only in the net9.0 branch (they already are).
There's no need to support `RuntimeIdentifiers` (plural) for Hot Restart
(because we don't have any scenarios where multiple runtime identifiers
applies to iOS; a single runtime identifier can always be used).

Adding support would make our code base more complex, so just avoid it by
showing an early error if someone tries (which is likely to be accidental
anyways).

This way we show an actionable error message for a scenario customers will
probably be confused about (because the build would fail in rather
inexplicable ways) if they run into it.

Partial fix for #19262.
Disable the autoinjected governance checks in the tests templates since
they timeout on the mac.

ref: https://docs.opensource.microsoft.com/tools/cg/index.html
This way it can easily be overridden when building from the command line (to provide a different url if the normal url doesn't work for some temporary reason).
…or methods public. (#20808)

When finding an enumerator for the given code:

```cs
var collection = new NSSet<NSNumber> ();
foreach (var item in collection) {
	// ...
}
```

the C# compiler will first look for any `GetEnumerator` methods. The non-generic `NSSet` class defines a `IEnumerator<NSObject> GetEnumerator<NSObject> ()` method, which, since the generic `NSSet<T>` class doesn't define such a method, is selected.

The end result is that the type of the foreach element is `NSObject`
(`GetEnumerator`'s return type') - which is somewhat unexpected:

```cs
var collection = new NSSet<NSNumber> ();
foreach (var item in collection) {
	Console.WriteLine (item.LongValue); // error CS1061: 'NSObject' does not contain a definition for 'LongValue'
}
```

The fix is to define a  `IEnumerator<T> GetEnumerator<T> ()` method in the
generic `NSSet<T>` class, which the C# will find and choose over the base
class' method. Then the type of the foreach element is the correct type, and
the following code works:

```cs
var collection = new NSSet<NSNumber> ();
foreach (var item in collection) {
	Console.WriteLine (item.LongValue); // it works!
}
```

Do this for all our generic collection classes.

Also document these methods + all the other public `GetEnumerator` methods.
…m Protocol attributes in bindings to the generated code. (#20804)

Also fix the MustSetBackwardsCompatibleCodeGenerationToFalse test to skip
protocols that actually set BackwardsCompatibleCodeGeneration=false.
This pull request updates the following dependencies

## From https://github.com/dotnet/installer

- **Subscription**: 80cb9ffd-f92f-4fc8-9f8b-08dbca46abfb
- **Build**: 20240624.1
- **Date Produced**: June 24, 2024 9:52:55 PM UTC
- **Commit**: 03065cafae0f89b376fa983773e909e341db96c0
- **Branch**: refs/heads/release/8.0.1xx

- **Updates**:
  - **Microsoft.Dotnet.Sdk.Internal**: [from 8.0.107-servicing.24317.4 to 8.0.107-servicing.24324.1][14]

[14]: dotnet/installer@de7be3d...03065ca

---------

Co-authored-by: Manuel de la Pena <mandel@microsoft.com>
This pull request updates the following dependencies

## From https://github.com/dotnet/xharness

- **Subscription**: 601bc5e1-1cae-44b5-cf5f-08db9342aa2f
- **Build**: 20240626.1
- **Date Produced**: June 26, 2024 1:01:59 PM UTC
- **Commit**: c1a7044cbe36ea67281412766a417eece02fb3a5
- **Branch**: refs/heads/main

- **Updates**:
  - **Microsoft.DotNet.XHarness.iOS.Shared**: [from 9.0.0-prerelease.24312.3 to 9.0.0-prerelease.24326.1][2]

[2]: dotnet/xharness@6ce1531...c1a7044
The C# build server makes trouble for us, because:

* We're using parallel make, and parallel make will start a jobserver, managed
  by file descriptors, where these file descriptors must be closed in all
  subprocesses for make to realize it's done.
* 'dotnet build' might have started a build server
* The build server does not close any file descriptors it may have inherited
  when daemonizing itself.
* Thus the build server (which will still be alive after we're done building)
  might have a file descriptor open which make is waiting for.
* The proper fix is to fix the build server to close its file descriptors.
* The intermediate working is to shut down the build server instead. An
  alternative solution would be to pass /p:UseSharedCompilation=false to
  'dotnet pack' to disable the usage of the build server.
* Note that build server will exit automatically after 10 minutes of idle
  time, so the hang is only a 10 minute delay at works.

For the siminstaller, which builds using the system .NET, the simplest
solution is to just not use the build server.

This fixes a problem where the build would hang for 10 minutes after running
the system dependency check (which builds and runs siminstaller).
Apple says the bug on their side causing a runtime crash has been fixed since
macOS 12, so unignore the code and add a version check for macOS 12.

Fixes xamarin/maccore#2345.
…d TeamIdentifierPrefix/AppIdentifierPrefix without a provisioning profile. (#20759)

By including the key and the offending value in the warning message.
Convert all projects except xtro-sharpie.csproj to .NET projects.
xtro-sharpie.csproj can't be converted yet, because it depends on
Objective-Sharpie, which hasn't been converted yet.
Add a simple makefile to the src/bgen directory that only builds and
tests bgen.

This is very useful when working on bgen to make sure your changes are
at building and working.
@mkhamoyan mkhamoyan self-assigned this Jul 8, 2024
Comment on lines +167 to +168
<_GlobalizationDataFile Condition="'$(_PlatformName)' != 'macOS' And '$(InvariantGlobalization)' != 'true' And '$(HybridGlobalization)' != 'true' And '$(_GlobalizationDataFile)' == '' And $([MSBuild]::VersionLessThan($(TargetFrameworkVersion), '9.0'))">icudt.dat</_GlobalizationDataFile>
<_GlobalizationDataFile Condition="'$(_PlatformName)' != 'macOS' And '$(InvariantGlobalization)' != 'true' And '$(HybridGlobalization)' == 'true' And '$(_GlobalizationDataFile)' == '' And $([MSBuild]::VersionLessThan($(TargetFrameworkVersion), '9.0'))">icudt_hybrid.dat</_GlobalizationDataFile>
Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding the condition on .NET 9, you can just target the net9.0 branch.

That way I believe you can just remove these lines, and also any other lines related to the icu file:

$ git grep GlobalizationDataFile
dotnet/targets/Xamarin.Shared.Sdk.targets:              <_GlobalizationDataFile Condition="'$(_PlatformName)' != 'macOS' And '$(InvariantGlobalization)' != 'true' And '$(HybridGlobalization)' != 'true' And '$(_GlobalizationDataFile)' == ''">icudt.dat</_GlobalizationDataFile>
dotnet/targets/Xamarin.Shared.Sdk.targets:              <_GlobalizationDataFile Condition="'$(_PlatformName)' != 'macOS' And '$(InvariantGlobalization)' != 'true' And '$(HybridGlobalization)' == 'true' And '$(_GlobalizationDataFile)' == ''">icudt_hybrid.dat</_GlobalizationDataFile>
dotnet/targets/Xamarin.Shared.Sdk.targets:                      <_ArchitectureSpecificFiles Include="$(_GlobalizationDataFile)" Condition="'$(_GlobalizationDataFile)' != ''" />
dotnet/targets/Xamarin.Shared.Sdk.targets:              <Error Text="Can not set values for both InvariantGlobalization '$(InvariantGlobalization)' and _GlobalizationDataFile '$(_GlobalizationDataFile)'" Condition="'$(_GlobalizationDataFile)' != '' And '$(InvariantGlobalization)' == 'true'" />
dotnet/targets/Xamarin.Shared.Sdk.targets:                              GlobalizationDataFile=$(_GlobalizationDataFile)
dotnet/targets/Xamarin.Shared.Sdk.targets:                                          '%(ResolvedFileToPublish.Filename)%(ResolvedFileToPublish.Extension)' == '$(_GlobalizationDataFile)' And
msbuild/Xamarin.Shared/Xamarin.Shared.targets:                  <_GlobalizationDataFileAppBundleRelativePath Condition="'$(_AppContentsRelativePath)' == ''">$(_GlobalizationDataFile)</_GlobalizationDataFileAppBundleRelativePath>
msbuild/Xamarin.Shared/Xamarin.Shared.targets:                  <_GlobalizationDataFileAppBundleRelativePath Condition="'$(_AppContentsRelativePath)' != ''">$(_AppContentsRelativePath)\$(_GlobalizationDataFile)</_GlobalizationDataFileAppBundleRelativePath>
msbuild/Xamarin.Shared/Xamarin.Shared.targets:                  <RuntimeHostConfigurationOption Include="ICU_DAT_FILE_PATH" Value="$(_GlobalizationDataFileAppBundleRelativePath.Replace('\','/'))" />
tools/dotnet-linker/LinkerConfiguration.cs:             public string GlobalizationDataFile { get; private set; } = string.Empty;
tools/dotnet-linker/LinkerConfiguration.cs:                             case "GlobalizationDataFile":
tools/dotnet-linker/LinkerConfiguration.cs:                                     GlobalizationDataFile = value;

Looking at the runtime PR, it also looks like we'll have to stop linking with these libraries:

$ git grep -E 'icudata|icui18n|icuuc'
runtime/Makefile:DOTNET_iphonesimulator_DYLIB_FLAGS=-lmonosgen-2.0 -licudata -licui18n -licuuc -framework UIKit
runtime/Makefile:DOTNET_iphoneos_DYLIB_FLAGS=-lmonosgen-2.0 -licudata -licui18n -licuuc -framework UIKit
runtime/Makefile:DOTNET_tvsimulator_DYLIB_FLAGS=-lmonosgen-2.0 -licudata -licui18n -licuuc -framework UIKit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rolfbjarne by mistake I pushed 66e7a1a to net9.0 instead of my branch, could you please help me revert that?

Copy link
Member

Choose a reason for hiding this comment

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

I reverted that commit and pushed the revert.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a lot!

@mkhamoyan mkhamoyan changed the base branch from main to net9.0 July 8, 2024 10:07
@vs-mobiletools-engineering-service2
Copy link
Collaborator

📚 [CI Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [CI Build] Tests on macOS X64 - Mac Sonoma (14) passed 💻

All tests on macOS X64 - Mac Sonoma (14) passed.

Pipeline on Agent
Hash: 36aaaefdd854705189fea80560e4765de1ada99b [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [CI Build] Tests on macOS M1 - Mac Ventura (13) passed 💻

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

Pipeline on Agent
Hash: 36aaaefdd854705189fea80560e4765de1ada99b [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [CI Build] Tests on macOS M1 - Mac Monterey (12) passed 💻

All tests on macOS M1 - Mac Monterey (12) passed.

Pipeline on Agent
Hash: 36aaaefdd854705189fea80560e4765de1ada99b [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

❌ [CI Build] Windows Integration Tests failed ❌

❌ Failed ❌

Pipeline on Agent
Hash: 36aaaefdd854705189fea80560e4765de1ada99b [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

🚀 [CI Build] Test results 🚀

Test results

✅ All tests passed on VSTS: test results.

🎉 All 170 tests passed 🎉

Tests counts

✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (iOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (MacCatalyst): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (macOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (Multiple platforms): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (tvOS): All 1 tests passed. Html Report (VSDrops) Download
✅ framework: All 6 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 7 tests passed. Html Report (VSDrops) Download
✅ generator: All 2 tests passed. Html Report (VSDrops) Download
✅ install-source: All 1 tests passed. Html Report (VSDrops) Download
✅ interdependent-binding-projects: All 7 tests passed. Html Report (VSDrops) Download
✅ introspection: All 8 tests passed. 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 (iOS): All 11 tests passed. Html Report (VSDrops) Download
✅ monotouch (MacCatalyst): All 8 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 9 tests passed. Html Report (VSDrops) Download
✅ monotouch (tvOS): All 11 tests passed. Html Report (VSDrops) Download
✅ monotouch (watchOS): All 4 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: 36aaaefdd854705189fea80560e4765de1ada99b [PR build]

@mkhamoyan mkhamoyan closed this Jul 8, 2024
@mkhamoyan mkhamoyan deleted the dev/mkhamoyan/clean-icu-files branch July 8, 2024 16:06
rolfbjarne pushed a commit that referenced this pull request Jul 11, 2024
As discussed in
#20828 (comment)
removing icu related files.

ICU lib files were removed by
ed1b6e5.

Contributes to dotnet/runtime#99521
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.

5 participants