-
Notifications
You must be signed in to change notification settings - Fork 511
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
[main] Update dependencies from dotnet/installer #11175
[main] Update dependencies from dotnet/installer #11175
Conversation
…210408.1 Microsoft.Dotnet.Sdk.Internal From Version 6.0.100-preview.3.21202.5 -> To Version 6.0.100-preview.4.21208.1
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
❌ Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View API diffTest results7 tests failed, 91 tests passed.Failed tests
Pipeline on Agent XAMBOT-1109' |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
We should have a build after the merge I just did, else something broke ;) |
…210409.4 Microsoft.Dotnet.Sdk.Internal From Version 6.0.100-preview.3.21202.5 -> To Version 6.0.100-preview.4.21209.4
/azp run |
Comment was made before the most recent commit for PR 11175 in repo xamarin/xamarin-macios |
Looks like this got hung yesterday |
…210410.1 Microsoft.Dotnet.Sdk.Internal From Version 6.0.100-preview.3.21202.5 -> To Version 6.0.100-preview.4.21210.1
but the ICU support was added based on P3 but merged after ^
link sdk failure might be something new in P4
monotouch-tests time outs did not show much clues |
…210412.5 Microsoft.Dotnet.Sdk.Internal From Version 6.0.100-preview.3.21202.5 -> To Version 6.0.100-preview.4.21212.5
…210413.70 Microsoft.Dotnet.Sdk.Internal From Version 6.0.100-preview.3.21202.5 -> To Version 6.0.100-preview.4.21213.70
…210414.14 Microsoft.Dotnet.Sdk.Internal From Version 6.0.100-preview.3.21202.5 -> To Version 6.0.100-preview.4.21214.14
@spouliot @dalexsoto as a heads up, when the next SDK version is brought in you'll need some runtime pack related changes. The runtime pack names now have a
We also saw something similar in P4 bump from earlier this week, but it seems to be fixed with our latest bump to |
Thanks @pjcollins for the heads up #11175 (comment)
@@ -18,6 +18,8 @@ | |||
|
|||
<!-- Don't include default Compile items for binding projects, because that would pick up ApiDefinition.cs and StructsAndEnums.cs --> | |||
<EnableDefaultCompileItems Condition=" '$(IsBindingProject)' == 'true' ">false</EnableDefaultCompileItems> | |||
|
|||
<UseMonoRuntime Condition=" '$(UseMonoRuntime)' == '' ">true</UseMonoRuntime> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still likely going to be problematic wrt.
<_PackageIdInfix Condition="'$(_XamarinRuntime)' != 'CoreCLR' And '$(_PlatformName)' == 'macOS'">Mono.</_PackageIdInfix> |
which is another variable that used to switch between the runtimes and applies the prefix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact it's the same place which causes the test build failures. _MonoNugetPackageId
is computed incorrectly which results in _MonoFrameworkReference
and _MonoRuntimePackPath
being incorrect leading to incorrect path for the AOT compiler eventually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should fix the build failures: filipnavara@8325f8d
(intentionally minimal fix; _XamarinRuntime
can be removed later and replaced with UseMonoRuntime
in separate PR)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, some of those dual variables can't be avoided (for legacy) but we should strive to remove all others
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is new (never released) so easy to avoid :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, better kill it before anyone gets emotionally attached to it 😄
The "monotouch-test/Mac [dotnet]/Debug [dotnet]" failure can be resolved by merging |
Those monotouch-test failure are weird since they affect non-dotnet runs :| |
I tried running the test locally. First, they interactively ask for quite a lot of permissions. The non-dotnet tests run with few failures for me. The dotnet one hangs on
It may actually be a bug in ICU where it doesn't release the mutex in error path (https://github.com/dotnet/icu/blob/032ffcab30edde31f2274f6e5613254b84cc7fa4/icu/icu4c/source/i18n/smpdtfmt.cpp#L4266). Since the time zone data are stripped in icudt.dat the error path will be hit. The dotnet/runtime builds for browser that use the same ICU data don't call these APIs because they take slightly different code path in the managed code for time zone names. /cc @steveisok UPD: ICU issue filed here and PR here. Even if that turns out to be valid fix we need another solution in the interim. The simplest one I can think of is to use the browser code paths in dotnet/runtime also for iOS/tvOS. |
<!-- download the runtime packs --> | ||
<PackageDownload Include="@(PackageRuntimeIdentifiers -> 'Microsoft.NETCore.App.Runtime.%(Identity)')" Version="[$(BundledNETCorePlatformsPackageVersion)]" /> | ||
<PackageDownload Include="@(PackageRuntimeIdentifiers -> 'Microsoft.NETCore.App.Runtime.Mono.%(Identity)')" Version="[$(BundledNETCorePlatformsPackageVersion)]" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this doesn't download the CoreCLR version anymore (which we need for macOS). However, that should make the build fail, which it apparently doesn't, so IDK?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
were any test are enabled ? maybe it silently fail ? 🤷♂️
…210418.6 Microsoft.Dotnet.Sdk.Internal From Version 6.0.100-preview.3.21202.5 -> To Version 6.0.100-preview.4.21218.6
@filipnavara that's normal for this test app, none should be blocking and the test checks the non-determined state (not allowed) in a few cases. Also thanks for filing the ICU issues. I'll rebuild (from clean) this morning since I did not hit the hangs. |
I can hit the hangs on The non-dotnet tests ask for ~3 permissions and I had to interact with them manually. Then 7 tests failed related to some bundle properties. I didn't find any issue related to it except some SO posts referring to certain properties returning |
Filtered changelog for the latest bump https://gist.github.com/spouliot/4da8c7853ac10fea76e00dfe90f7abc6
|
Something else must have changed... as the invariant change for ICU is in |
So with a clean build I get the expected (same as bot) deadlock for |
The ICU deadlock is triggered by a new code path introduced in dotnet/runtime#48931. At least one mystery solved. |
Nope, I don't get the timeouts for non-dotnet tests :( |
Ref: xamarin/xamarin-macios#11175 (comment), https://unicode-org.atlassian.net/browse/ICU-21591 There seems to be a bug in ICU that leads to deadlock when the time zone data are stripped. Since dotnet/icu uses the same stripping of data on all the platforms the time zone data are also not present on iOS/tvOS or any platform that consumes it. So even if the deadlock itself is resolved at some point it makes sense to use the same implementation for all the platforms that rely on the filtered app-local ICU data. I also enabled to code path on MacCatalyst to keep it consistent with iOS. I am open to change that. Android may need to be treated the same way too.
Fix satellite/location assemblies and some unit tests
@filipnavara looks like you were right and that the dotnet timeouts affected the other monotouch-tests (or were just very lucky this morning)
Failure is unrelated, known issue -> https://github.com/xamarin/maccore/issues/2411 |
This pull request updates the following dependencies
From https://github.com/dotnet/installer