Add SuppressMetaPackage to break circular dependency during package restore #28042
Conversation
|
|
||
| <!-- this package is part of the implementation closure of NETStandard.Library | ||
| therefore it cannot reference NETStandard.Library --> | ||
| <SuppressMetaPackage Include="NETStandard.Library" /> |
There was a problem hiding this comment.
Are there other packages this should be added to, as well?
There was a problem hiding this comment.
From my discussion with @ericstj yesterday, this was the only package that did not have it and needed it.
Other packages, like System.Buffers, System.Diagnostics.DiagnosticSource, etc. all have it already (https://github.com/dotnet/corefx/search?utf8=%E2%9C%93&q=SuppressMetaPackage&type=)
The remaining packages (for example those under Reflection.*) no longer publish newer versions.
A potential candidate for this could be System.Memory but so far no packages that are OOB (or partially OOB) reference System.Memory.
If, for instance, System.Threading.Tasks.Extensions were to reference System.Memory, we would probably need to add the property to System.Memory as well.
The validation tests that Eric mentioned should discover circular dependencies that we might miss in the future once we can get it up and running again.
Interestingly, System.Security.Cryptography.Cng does reference System.Memory, but doesn't trigger the circular dependency:
From System.Memory nuspec:
<group targetFramework=".NETStandard1.1">
<dependency id="NETStandard.Library" version="1.6.1" />
<dependency id="System.Buffers" version="4.5.0-preview2-26313-0" exclude="Compile" />
<dependency id="System.Runtime.CompilerServices.Unsafe" version="4.5.0-preview2-26313-0" exclude="Compile" />
</group>This is most likely because nothing in NETStandard.Library depends on System.Memory (not part of the implementation closure of NETStandard.Library, along with System.Security.Cryptography.Cng). Please let me know if I am mistaken.
There was a problem hiding this comment.
Interestingly, System.Security.Cryptography.Cng does reference System.Memory, but doesn't trigger the circular dependency:
That's because CNG isn't in the package closure of NETStandard.Library. The problem comes about when something in the closure of NETStandard.Library starts referencing NETStandard.Library. For everything subsumed by NETStandard.Library this isn't an issue since we no longer build packages for them. There were, however, a handful of packages which were previously in the implementation closure of NETStandard.Library, but not added to the NETStandard2.0 surface area (not subsumed):
- System.Buffers
- System.Diagnostics.DiagnosticSource
- System.Threading.Tasks.Extensions
- System.Reflection.Emit
- System.Reflection.Emit.ILGeneration
- System.Reflection.Emit.Lightweight
- System.Reflection.TypeExtensions
- System.Security.Cryptography.OpenSsl
For any of these we have to make sure that newer versions of the packages don't bring in a transitive reference to NETStandard.Library, otherwise it will create a cycle. The one that did introduce this was System.Threading.Tasks.Extensions > System.Runtime.CompilerServices.Unsafe > NETStandard.Library.
There was a problem hiding this comment.
The one that did introduce this was System.Reflection.TypeExtensions > System.Runtime.CompilerServices.Unsafe > NETStandard.Library.
Shouldn't it be System.Threading.Tasks.Extensions?
|
Is this going to be necessary once we fix https://github.com/dotnet/corefx/issues/26145 ? |
Yes I do think this will still be needed after you fix #26145 as that only adds a netcoreapp configuration and the issue at hand is about a netstandard library, in partcular netstandard library less then v2. @ahsonkhan did you verify the diff of the package/nuspec looks correct? |
Before and after: |
|
OSX failure - https://github.com/dotnet/core-eng/issues/2913 I am not sure what is the cause of the Windows catastrophic failures (on Windows.81.Amd64.Open:Debug-x64 / Windows.7.Amd64.Open:Debug-x64). @dotnet/dnceng, is this a known issue?
@dotnet-bot test Windows x64 Debug Build |
|
@ahsonkhan That's just what the secret cycling error looks like on Windows. It's the same issue that's occurring on OSX stuff. It's "unable to download" bc it has improper auth tokens. |
|
The issue looks fixed across the board but OSX is heavily backed up, give it some time to catch up. |
|
@dotnet-bot test OSX x64 Debug Build |
1 similar comment
|
@dotnet-bot test OSX x64 Debug Build |
…estore (dotnet/corefx#28042) Commit migrated from dotnet/corefx@e75c019
See dotnet/corefxlab#2150 (comment) for context (where the issue was manifesting as a VS crash due to stackoverflow).
I tested this locally (by building and referencing the System.Threading.Tasks.Extensions/System.IO.Pipelines packages with this fix in a netstandard1.3 class library).
cc @weshaggard, @ericstj