-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[release/9.0-preview4] Update dependencies from dotnet/winforms #9055
[release/9.0-preview4] Update dependencies from dotnet/winforms #9055
Conversation
…40425.2 Microsoft.Dotnet.WinForms.ProjectTemplates , Microsoft.Private.Winforms , System.Drawing.Common From Version 9.0.0-preview.4.24222.2 -> To Version 9.0.0-preview.4.24225.2 Dependency coherency updates System.Reflection.MetadataLoadContext,System.Windows.Extensions,Microsoft.NETCore.Platforms,System.Resources.Extensions,Microsoft.NETCore.App.Ref,Microsoft.NETCore.App.Runtime.win-x64,VS.Redist.Common.NetCore.SharedFramework.x64.9.0,Microsoft.Win32.Registry.AccessControl,Microsoft.Win32.SystemEvents,System.CodeDom,System.ComponentModel.Composition,System.Configuration.ConfigurationManager,System.Data.Odbc,System.Data.OleDb,System.Diagnostics.EventLog,System.Diagnostics.PerformanceCounter,System.DirectoryServices.AccountManagement,System.DirectoryServices.Protocols,System.DirectoryServices,System.IO.Packaging,System.IO.Ports,System.Management,System.Reflection.Context,System.Runtime.Caching,System.Security.Cryptography.Pkcs,System.Security.Cryptography.ProtectedData,System.Security.Cryptography.Xml,System.Security.Permissions,System.ServiceModel.Syndication,System.ServiceProcess.ServiceController,System.Speech,System.Text.Encoding.CodePages,System.Threading.AccessControl,System.ComponentModel.Composition.Registration From Version 9.0.0-preview.4.24219.3 -> To Version 9.0.0-preview.4.24225.7 (parent: Microsoft.Private.Winforms
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.
Go, you big red fire engine!
@singhashish-wpf can you take a look? |
Could this be caused by dotnet/runtime#95830 ? |
Yes. I beleive this requires reaction in downstream repos. /cc @carlossanlop |
I was trying to fix this by changing the types from ICollection to IEnumerable in the LocalizedStrings.h and related files, however there is a problem with changing those types that it doesn't fit well with the IDictionary interface where ICollection is expected. Checking more on this. Also I am unclear on the effects of this change and the potential issues which could arise due to this, we change it this way. |
The general issue is the same as would exist if you had implemented both C++/CLI doesn't support covariant returns and doesn't properly understand DIMs, so you need to explicitly implement the conflicting member. Simply doing something similar to the following will resolve the source break property System::Collections::Generic::IEnumerable<CultureInfo^>^ Keys2
{
virtual System::Collections::Generic::IEnumerable<CultureInfo^>^ get() sealed =
System::Collections::Generic::IReadOnlyDictionary<CultureInfo^, String^>::Keys::get;
}
property System::Collections::Generic::IEnumerable<String^>^ Values2
{
virtual System::Collections::Generic::IEnumerable<String^>^ get() sealed =
System::Collections::Generic::IReadOnlyDictionary<CultureInfo^, String^>::Values::get;
} |
Thanks @tannergooding property System::Collections::Generic::IEnumerable<CultureInfo^>^ Keys2
{
virtual System::Collections::Generic::IEnumerable<CultureInfo^>^ get() sealed =
System::Collections::Generic::IReadOnlyDictionary<CultureInfo^, String^>::Keys::get;
}
property System::Collections::Generic::IEnumerable<String^>^ Values2
{
virtual System::Collections::Generic::IEnumerable<String^>^ get() sealed =
System::Collections::Generic::IReadOnlyDictionary<CultureInfo^, String^>::Values::get;
}
Also added the below to LocalizedStrings.cpp IEnumerable<CultureInfo^>^ LocalizedStrings::Keys2::get()
{
return (IEnumerable<CultureInfo^>^)KeysArray;
}
IEnumerable<String^>^ LocalizedStrings::Values2::get()
{
return (IEnumerable<String^>^)ValuesArray;
} Currently facing multiple instances of below error:
Looks like I am missing something here, Any further pointers? |
These can be resolved by doing something similar to C++/CLI doesn't like that both |
I do not think that this is acceptable workaround. We should revert the change that introduced the break in dotnet/runtime. |
I think it’s worth explicitly calling out that there’s seemingly 3 lines needing changed here, for what is very likely temporary workaround here (C++/CLI has taken tie breaking improvements several times over the past few years) and that the change is inline with how regular C++ expects such ambiguities to be resolved (this type of workaround is not unique to C++/CLI) The alternative is backing out one of the more highly requested changes, which goes back to .NET Framework 4.5 when the new interfaces were introduced, and where we will miss the biggest opportunity to get early feedback on it from the broader ecosystem. Given the years of deliberating taken to even try it this time, backing it out may substantially hinder future efforts (either to try this ask again or to look at similar asks in the future) Given that, I personally think it’s worth at least a little further discussion around this, the general impact, and the likelihood of a new tie breaker being added to avoid the issue by the time November rolls around |
As you know from internal discussions, we have cut corners with C/C++ validation and dismissed concerns that some people involved had about it. The learning for future efforts like this is that cutting corners is not acceptable. It has very high chance of firing back. It is not the first time we have learned it with efforts like this. We just keep re-learning it. |
The offending change was reverted in dotnet/runtime: dotnet/runtime#101645. We need to wait for the new build to flow into this PR. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
…40429.5 Microsoft.Dotnet.WinForms.ProjectTemplates , Microsoft.Private.Winforms , System.Drawing.Common From Version 9.0.0-preview.4.24222.2 -> To Version 9.0.0-preview.4.24229.5 Dependency coherency updates System.Reflection.MetadataLoadContext,System.Windows.Extensions,Microsoft.NETCore.Platforms,System.Resources.Extensions,Microsoft.NETCore.App.Ref,Microsoft.NETCore.App.Runtime.win-x64,VS.Redist.Common.NetCore.SharedFramework.x64.9.0,Microsoft.Win32.Registry.AccessControl,Microsoft.Win32.SystemEvents,System.CodeDom,System.ComponentModel.Composition,System.Configuration.ConfigurationManager,System.Data.Odbc,System.Data.OleDb,System.Diagnostics.EventLog,System.Diagnostics.PerformanceCounter,System.DirectoryServices.AccountManagement,System.DirectoryServices.Protocols,System.DirectoryServices,System.IO.Packaging,System.IO.Ports,System.Management,System.Reflection.Context,System.Runtime.Caching,System.Security.Cryptography.Pkcs,System.Security.Cryptography.ProtectedData,System.Security.Cryptography.Xml,System.Security.Permissions,System.ServiceModel.Syndication,System.ServiceProcess.ServiceController,System.Speech,System.Text.Encoding.CodePages,System.Threading.AccessControl,System.ComponentModel.Composition.Registration From Version 9.0.0-preview.4.24219.3 -> To Version 9.0.0-preview.4.24227.4 (parent: Microsoft.Private.Winforms
This pull request updates the following dependencies
Coherency Updates
The following updates ensure that dependencies with a CoherentParentDependency
attribute were produced in a build used as input to the parent dependency's build.
See Dependency Description Format
From https://github.com/dotnet/winforms
Microsoft Reviewers: Open in CodeFlow