Skip to content

Conversation

@ericstj
Copy link
Member

@ericstj ericstj commented May 7, 2021

The AvoidRestoreCycleOnSelfReference workaround was causing
DiagnosticSource to get the wrong PackageID in the assets file when
referenced by other projects. This wasn't a problem when using pkgproj
since we'd calculate dependencies from assembly references, ignoring
the assets file. This is a problem now that we're using csproj pack,
since that gets dependencies from the assets file.

Fixes #52459

The `AvoidRestoreCycleOnSelfReference` workaround was causing
DiagnosticSource to get the wrong PackageID in the assets file when
referenced by other projects.  This wasn't a problem when using pkgproj
since we'd calculate dependencies from assembly references, ignoring
the assets file.  This is a problem now that we're using csproj pack,
since that gets dependencies from the assets file.
@ghost
Copy link

ghost commented May 7, 2021

Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

The AvoidRestoreCycleOnSelfReference workaround was causing
DiagnosticSource to get the wrong PackageID in the assets file when
referenced by other projects. This wasn't a problem when using pkgproj
since we'd calculate dependencies from assembly references, ignoring
the assets file. This is a problem now that we're using csproj pack,
since that gets dependencies from the assets file.

Fixes #52459

Author: ericstj
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -

<Nullable>enable</Nullable>
<TargetFrameworks>$(NetCoreAppCurrent);net5.0;netstandard1.1;netstandard1.3;net45;net46;netstandard2.0</TargetFrameworks>
<ExcludeCurrentNetCoreAppFromPackage>true</ExcludeCurrentNetCoreAppFromPackage>
<AvoidRestoreCycleOnSelfReference>true</AvoidRestoreCycleOnSelfReference>
Copy link
Member Author

@ericstj ericstj May 7, 2021

Choose a reason for hiding this comment

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

We need to remove this hack anyhow for when we eventually convert this project to CSProj packing.

I looked into completely removing https://github.com/dotnet/runtime/blob/main/eng/AvoidRestoreCycleOnSelfReference.targets but it's still used here:

<AvoidRestoreCycleOnSelfReference>true</AvoidRestoreCycleOnSelfReference>

And that can't be removed in the same way (we could refactor the project into a separate task project to fix it). I'm not doing that now since the current issue is blocking consumption of dotnet/runtime.

@ericstj
Copy link
Member Author

ericstj commented May 7, 2021

Two test failures are #52464, one was a pass but an issue with helix reporting results https://github.com/dotnet/core-eng/issues/13026

@ericstj ericstj merged commit 04e4a22 into dotnet:main May 8, 2021
Copy link
Member

@ViktorHofer ViktorHofer left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the quick fix.

Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

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

LGTM.

@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Microsoft.Extensions.Logging has broken dependencies

5 participants