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

ResolvePackageFileConflicts Prohibits using DbgShim from NuGet package #90187

Closed
lordmilko opened this issue Jul 19, 2023 · 7 comments · Fixed by #90193
Closed

ResolvePackageFileConflicts Prohibits using DbgShim from NuGet package #90187

lordmilko opened this issue Jul 19, 2023 · 7 comments · Fixed by #90193

Comments

@lordmilko
Copy link

lordmilko commented Jul 19, 2023

When developing a .NET debugger capable of targeting .NET Core applications, a library called dbgshim is used to bootstrap the entire process. While dbgshim is included with the .NET Runtime, it is generally recommended to ship dbgshim with your application so that your debugger doesn't have to worry about locating dbgshim on the target system; simply load your dbgshim into your debugger, point it at a given process and ask "does this process host a CLR we can attach to?"

Microsoft has shipped a NuGet package for dbgshim that includes sub-packages with native dbgshim implementations for all of the various operating systems/architectures it supports.

If you attempt to reference this NuGet package however, neither dbgshim nor the runtimes directory inside the sub-packages are copied to your output folder, regardless of whether you are doing dotnet build or dotnet pack. For the purposes of developing an application, I expect these files should be copied to the output directory upon doing dotnet build (so that you can find and load them in your application)

After doing some debugging, I have identified that the reason dbgshim does not get copied to the output directory is that the version of dbgshim in the package is considered to conflict with the version of dbgshim included in the runtime, and thus the runtime always wins.

The sequence of events that lead up to the conflict being detected are as follows:

  1. The ResolvePackageFileConflicts task is run

  2. PlatformManifestReader reads a PlatformManifest.txt file for the current runtime

  3. As dbgshim is listed in this file, a conflict item is added for it

  4. ConflictResolver tries to resolve the platform dbgshim against the NuGet package dbgshim

  5. PackageOverrideResolver is called upon to see if there are potentially any overrides that could be taken advantage of

  6. As there are no overrides, the platform dbgshim wins by virtue of having a higher file version and you get the following error in the MSBuild log

    Encountered conflict between 'Platform:dbgshim.dll' and 'CopyLocal:C:\Users\user.nuget\packages\microsoft.diagnostics.dbgshim.win-x64\7.0.430602\runtimes\win-x64\native\dbgshim.dll'. Choosing 'Platform:dbgshim.dll' because file version '8.0.23.36403' is greater than '7.0.8.30602'. (TaskId:82)

If DbgShim is removed from PlatformManifest.txt, dbgshim and/or the runtimes folder is copied to the output directory upon building, as expected

In dotnet/sdk#2221 an issue similar to this is described, and there is a workaround in the code to deal with this, however it seems this workaround is predicated on having a "reference" of some kind to the conflicting assembly, which isn't the case when we've got a native, unreferenced library.

As a creative workaround, I attempted to add a target to "enhance" the list of package overrides with the goal of telling the conflict resolver to use my package as per this line

<Target Name="Hack" BeforeTargets="_HandlePackageFileConflicts">
    <ItemGroup>
        <PackageConflictOverrides Include="Microsoft.Diagnostics.DbgShim.win-x64">
            <!-- Use an arbitrarily high number so we always win -->
            <OverriddenPackages>
                Microsoft.NETCore.App.Ref|99.0
            </OverriddenPackages>
        </PackageConflictOverrides>
    </ItemGroup>
</Target>

Unfortunately, I found this does not work because the platform dbgshim has a null PackageVersion, hence the check in this if statement fails

I'm not sure whether I would recommend having dbgshim simply be omitted from PlatformManifest.txt, as this file may be used for other purposes as well. Arguably, dbgshim is kind of a special case, so potentially there could just be special rules for how to handle conflicts against dbgshim.dll, libdbgshim.so and libdbgshim.dylib

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jul 19, 2023
@marcpopMSFT marcpopMSFT self-assigned this Jul 24, 2023
@marcpopMSFT
Copy link
Member

@tommcdon do you happen to know who owns dbgshim on the runtime side and might have guidance on the best way to reverence it separately from the shared framework?

@tommcdon
Copy link
Member

@marcpopMSFT Dbgshim was removed from the 7.0 runtime and now lives in dotnet/diagnostics, shipping as an out-of-band nuget package. Given that it no longer ships with the runtime, it feels like we should remove it from PlatformManifest.txt. Adding @mikem8361 @hoyosjs for thoughts.

@marcpopMSFT
Copy link
Member

@mikem8361 @hoyosjs any thoughts on this and recommendations for the customer?

@hoyosjs
Copy link
Member

hoyosjs commented Aug 1, 2023

@jkoritzinsky to me it makes sense to remove from the platform manifest. Is there anything that breaks other than rollforward (from one that has dbgshim to one that doesn't will break)? Since all supported runtimes don't have it only .NET 5 and bellow will break when running on a 6.0+ runtime, but that's an OK tradeoff.

@jkoritzinsky
Copy link
Member

The main use case for the PlatformManifest is to ensure that in-box assets take precedence over packaged assets even if renamed. In this case, we've moved an in-box asset to out-of-box, so removing it from the manifest makes sense. This shouldn't break downlevel apps as the in-box asset will always be preferred for platforms like .NET 5.

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 8, 2023
@marcpopMSFT marcpopMSFT transferred this issue from dotnet/sdk Aug 8, 2023
@marcpopMSFT marcpopMSFT removed their assignment Aug 8, 2023
@marcpopMSFT
Copy link
Member

Per comments above, moving to runtime repo.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 8, 2023
@hoyosjs hoyosjs added area-Diagnostics-coreclr and removed untriaged New issue has not been triaged by the area owner needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 8, 2023
@ghost
Copy link

ghost commented Aug 8, 2023

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

When developing a .NET debugger capable of targeting .NET Core applications, a library called dbgshim is used to bootstrap the entire process. While dbgshim is included with the .NET Runtime, it is generally recommended to ship dbgshim with your application so that your debugger doesn't have to worry about locating dbgshim on the target system; simply load your dbgshim into your debugger, point it at a given process and ask "does this process host a CLR we can attach to?"

Microsoft has shipped a NuGet package for dbgshim that includes sub-packages with native dbgshim implementations for all of the various operating systems/architectures it supports.

If you attempt to reference this NuGet package however, neither dbgshim nor the runtimes directory inside the sub-packages are copied to your output folder, regardless of whether you are doing dotnet build or dotnet pack. For the purposes of developing an application, I expect these files should be copied to the output directory upon doing dotnet build (so that you can find and load them in your application)

After doing some debugging, I have identified that the reason dbgshim does not get copied to the output directory is that the version of dbgshim in the package is considered to conflict with the version of dbgshim included in the runtime, and thus the runtime always wins.

The sequence of events that lead up to the conflict being detected are as follows:

  1. The ResolvePackageFileConflicts task is run

  2. PlatformManifestReader reads a PlatformManifest.txt file for the current runtime

  3. As dbgshim is listed in this file, a conflict item is added for it

  4. ConflictResolver tries to resolve the platform dbgshim against the NuGet package dbgshim

  5. PackageOverrideResolver is called upon to see if there are potentially any overrides that could be taken advantage of

  6. As there are no overrides, the platform dbgshim wins by virtue of having a higher file version and you get the following error in the MSBuild log

    Encountered conflict between 'Platform:dbgshim.dll' and 'CopyLocal:C:\Users\user.nuget\packages\microsoft.diagnostics.dbgshim.win-x64\7.0.430602\runtimes\win-x64\native\dbgshim.dll'. Choosing 'Platform:dbgshim.dll' because file version '8.0.23.36403' is greater than '7.0.8.30602'. (TaskId:82)

If DbgShim is removed from PlatformManifest.txt, dbgshim and/or the runtimes folder is copied to the output directory upon building, as expected

In dotnet/sdk#2221 an issue similar to this is described, and there is a workaround in the code to deal with this, however it seems this workaround is predicated on having a "reference" of some kind to the conflicting assembly, which isn't the case when we've got a native, unreferenced library.

As a creative workaround, I attempted to add a target to "enhance" the list of package overrides with the goal of telling the conflict resolver to use my package as per this line

<Target Name="Hack" BeforeTargets="_HandlePackageFileConflicts">
    <ItemGroup>
        <PackageConflictOverrides Include="Microsoft.Diagnostics.DbgShim.win-x64">
            <!-- Use an arbitrarily high number so we always win -->
            <OverriddenPackages>
                Microsoft.NETCore.App.Ref|99.0
            </OverriddenPackages>
        </PackageConflictOverrides>
    </ItemGroup>
</Target>

Unfortunately, I found this does not work because the platform dbgshim has a null PackageVersion, hence the check in this if statement fails

I'm not sure whether I would recommend having dbgshim simply be omitted from PlatformManifest.txt, as this file may be used for other purposes as well. Arguably, dbgshim is kind of a special case, so potentially there could just be special rules for how to handle conflicts against dbgshim.dll, libdbgshim.so and libdbgshim.dylib

Author: lordmilko
Assignees: -
Labels:

area-Diagnostics-coreclr, untriaged, in-pr, needs-area-label

Milestone: -

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 22, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants