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

[NuGet] WebView2Loader.dll is copied twice to output #2898

Closed
czdietrich opened this issue Oct 20, 2022 · 10 comments
Closed

[NuGet] WebView2Loader.dll is copied twice to output #2898

czdietrich opened this issue Oct 20, 2022 · 10 comments
Assignees
Labels
bug Something isn't working

Comments

@czdietrich
Copy link

czdietrich commented Oct 20, 2022

Description
The Microsoft.Web.WebView2 NuGet always creates a folder structure equal to runtimes\win-x64\native\WebView2Loader.dll in the output directory.

The reason for this is that there are copy instructions in the Common.targets file for managed projects.
There should be no need for copying manually any DLLs for managed projects, since NuGet has its own conventions for copying native DLLs into the output directory, which also makes use of the current RuntimeIdentifier. see NuGet spec

In .NET Core there should be no runtimes folder when publishing an application for a specific platform.

So, my request is that the NuGet should not copy manually any DLLs if NuGet semantics would already do it on its own. Or if there is an issue to correctly detect whether manual copying is needed, there should be a switch (property) to disable it.

Version
SDK: .NET Core 6.0.402
Runtime: win-x64
OS: Windows

Repro Steps

  1. Create a new .NET Core project
  2. Add Microsoft.Web.WebView2 NuGet as PackageReference
  3. Publish the application (e. g. dotnet publish -r win-x64 --no-self-contained)

The DLL can now be found twice in the output folder:
runtimes\win-x64\native\WebView2Loader.dll
WebView2Loader.dll

\bin\Debug\net6.0\win-x64\publish\runtimes
\bin\Debug\net6.0\win-x64\publish\Microsoft.Web.WebView2.Core.dll
\bin\Debug\net6.0\win-x64\publish\Microsoft.Web.WebView2.WinForms.dll
\bin\Debug\net6.0\win-x64\publish\Microsoft.Web.WebView2.Wpf.dll
\bin\Debug\net6.0\win-x64\publish\TestWebView2LoaderOutput.deps.json
\bin\Debug\net6.0\win-x64\publish\TestWebView2LoaderOutput.dll
\bin\Debug\net6.0\win-x64\publish\TestWebView2LoaderOutput.exe
\bin\Debug\net6.0\win-x64\publish\TestWebView2LoaderOutput.pdb
\bin\Debug\net6.0\win-x64\publish\TestWebView2LoaderOutput.runtimeconfig.json
\bin\Debug\net6.0\win-x64\publish\WebView2Loader.dll <--
\bin\Debug\net6.0\win-x64\publish\runtimes\win-x64
\bin\Debug\net6.0\win-x64\publish\runtimes\win-x64\native
\bin\Debug\net6.0\win-x64\publish\runtimes\win-x64\native\WebView2Loader.dll <--
@czdietrich czdietrich added the bug Something isn't working label Oct 20, 2022
@cremor
Copy link

cremor commented Oct 29, 2022

Related/more information: #1418 (comment)

@victorhuangwq
Copy link
Collaborator

@czdietrich does this bug still exist for you? or have you found a resolution for it.

@victorhuangwq victorhuangwq assigned victorhuangwq and unassigned yunate Dec 1, 2023
@czdietrich
Copy link
Author

@victorhuangwq
Hi, yes the issue is still present and as of now there is no work around.

@dotMorten
Copy link

This needs to be addressed. With WindowsAppSDK's recent dependency on this package, any library that takes dependency on it gets BROKEN, because it now also gets content into a class library, which means the .pri resource files gets references to files that aren't part of that SDK and then fails to build.

@czdietrich
Copy link
Author

Just in case someone is looking for a temporary fix.
We added the following code to our Directory.Build.targets file:

  <!-- Remove unnessecary second WebView2Loader.dll from runtimes folder -->
  <!-- See also: https://github.com/MicrosoftEdge/WebView2Feedback/issues/2898 -->
  <Target Name="RemoveDuplicatedWebView2LoaderDll" BeforeTargets="AssignTargetPaths">
    <ItemGroup>
      <Content Remove="@(Content)" Condition="%(Content.Link) == 'runtimes\win-x64\native\WebView2Loader.dll'" />
    </ItemGroup>
  </Target>

@oggy22 oggy22 self-assigned this Oct 16, 2024
@oggy22
Copy link
Member

oggy22 commented Oct 16, 2024

I believe this has been just fixed and here is an internal build I can provide. Can anyone please try with this SDK version and verify if the bug doesn't repro anymore? The zip file needs to be renamed to nuget file nupkg.

Microsoft.Web.WebView2.1.0.2887-prerelease.zip

@dotMorten
Copy link

dotMorten commented Oct 17, 2024

@oggy22 Just looking inside the package I see some issues.
Paths like this is wrong: \runtimes\win-arm64\native_uap. That's not a proper native path for deploying native runtimes. For uap it should be \runtimes\win10-arm64\native, but that would also apply to Win10 win32 apps, so better to put this in a separate package that would be referenced if the TFM is UAP. It's pretty typical to have a nuget with the reference libraries, and then sub-packages with the native runtimes for each platform, included based on the TFM.
I still see the \build\ folder adding the runtimes as content. That should only occur for .NET Framework apps, not for .NET 6+ apps.

There's also some odd automatic referencing WPF and WinForms DLLs which makes no sense if you're not actually using those frameworks (you should instead switch on UseWpf and UseWindowsForms to determine if those should be referenced).
I wonder if getting a review of the package from the NuGet team might help improve this package?

The fact that there's now a WebView2NeverCopyLoaderDllToOutputDirectory property to disable copying is a code-smell all by itself. Done right this shouldn't be needed.

@oggy22
Copy link
Member

oggy22 commented Oct 17, 2024

@dotMorten there are some custom paths in our package where native_uap is being one of them. We use it instead of native so that .NET builds don't try to use Microsoft.Web.WebView2.Core.dll from there. Another example is lib_manual that we introduced for supporting WinRT C# projection. Generally speaking supporting so many API surfaces from a single package is challenging task. I don't think we will make any big refactorings of the package.

I'd appreciate feedback on the package version I provided as I believe it may fix this problem. Thanks!

@dotMorten
Copy link

dotMorten commented Oct 17, 2024

@oggy22 but AFAIK anything you put in the runtimes folder will be copied to the app output, and you don't want native_uap files copied for non-uap apps I assume?

Generally speaking supporting so many API surfaces from a single package is challenging task

That's why it should be split up. There's one main package, which references the right native runtimes based on TFM, and there are packages that sit above that for instance adds WPF support etc. Trying to do this in one package is probably the reason for this franken-nuget and all the issues that it'll ultimately create.

@oggy22 oggy22 closed this as completed Dec 5, 2024
@dotMorten
Copy link

@oggy22 Can you share a little bit about what was done to address this and what version it'll be shipped in?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants