HakanL / WkHtmlToPdf-DotNet

C# .NET Core wrapper for wkhtmltopdf library that uses Webkit engine to convert HTML pages to PDF.
GNU Lesser General Public License v3.0
367 stars 66 forks source link

Runtimes folder is not copied during publish-process on .NET Framework #39

Closed mrblumi closed 2 years ago

mrblumi commented 3 years ago

Publishing .NET Framework (4.8) Projects does not copy runtimes directory to target location. The problem occures with new csproj-format (i.e. console app) as well as with old csproj-format (i.e. asp.net application)...

Steps to reproduce for first example:

I have not tried to get a minimum example for legacy csproj format since I think it s the same problem with both formats...

mrblumi commented 3 years ago

Okey, I managed to get a hacky fix for this in asp.net framework project by adding these lines at the bottom of the publish profile

<Target Name="CustomCollectFiles">
    <ItemGroup>
      <_CustomFiles Include="$(SolutionDir)\packages\Haukcode.WkHtmlToPdfDotNet.1.5.8\runtimes\win-*\**\*" />
      <FilesForPackagingFromProject Include="%(_CustomFiles.Identity)">
        <DestinationRelativePath>bin\runtimes\%(RecursiveDir)%(Filename)%(Extension)</DestinationRelativePath>
      </FilesForPackagingFromProject>
    </ItemGroup>
  </Target>

  <PropertyGroup>
    <CopyAllFilesToSingleFolderForPackageDependsOn>
      CustomCollectFiles;
      $(CopyAllFilesToSingleFolderForPackageDependsOn);
    </CopyAllFilesToSingleFolderForPackageDependsOn>

    <CopyAllFilesToSingleFolderForMsdeployDependsOn>
      CustomCollectFiles;
      $(CopyAllFilesToSingleFolderForMsdeployDependsOn);
    </CopyAllFilesToSingleFolderForMsdeployDependsOn>
  </PropertyGroup>

But it is still nasty because one has to think about manually fixing package version while upgrading nuget dependency.

HakanL commented 3 years ago

Hmm, that's odd, it's in the build output, but not in publish. Hopefully we can find out a solution that goes into the Nuget package so we don't have to make any changes to the csproj of the consuming project. I don't know what that fix is though, happy to accept suggestions/PRs.

mrblumi commented 3 years ago

It s not realy a fix, it is an ugly hack appended inside the .pubxml file of our ASP.NET project which copies the required files into the publish folder, because otherwise we would forget to publish them manually every now and then ;)

I'm hoping to give you a working minimum example tomorrow

mrblumi commented 3 years ago

Created a minimal ASP.NET API and installed Haukcode.WkHtmlToPdfDotNET nuget package and finally added two visual studio publish profiles (one with above additions, one without) Demo

First I tried to get something working like this inside csproj file, but as I am not that familiar with legacy framework build techniques I had no success. But maybe this can be a starting point for a real fix...

HakanL commented 3 years ago

Thanks @mrblumi, I'm not sure what the real fix is either, we may have to leave this open for now to see if somebody else can come up with a fix. The best way would obviously be a fix in the source Nuget package.

mrblumi commented 3 years ago

I 100% agree with you. Adding some custom stuff to either csproj or pubxml files is alwas error prone...

horizondave commented 2 years ago

I'm not an expert in this by any means, but it looks like there are a couple of options for getting the nuget working properly with .net framework.

  1. Multi-target using <TargetFramework>
  2. Use the "lib" directory to host windows versions of the DLLs specifically for .net platform.

https://stackoverflow.com/questions/52397501/nuget-references-to-assemblies-in-runtimes-folder-not-added

horizondave commented 2 years ago

I've got this working on a local nuget feed OK. Sent a pull request for you to review.

HakanL commented 2 years ago

@mrblumi and @horizondave Can you please test with the latest Nuget package, v1.5.59 and see if it works now and report back here? Thanks!

mrblumi commented 2 years ago

I can test this tomorrow afternoon :)

horizondave commented 2 years ago

Version 1.5.59 works fine for me, many thanks :)

mrblumi commented 2 years ago

Works for new csproj based .NET Framework based projects (tested with dotnet publish) . Does not work for old style ASP.NET Framework projects... The files of wkhtmltox are copied flat into $publish folder but not into $publish/bin/runtimes/.../... where we would expect them. Sorry...

HakanL commented 2 years ago

Works for new csproj based .NET Framework based projects (tested with dotnet publish) . Does not work for old style ASP.NET Framework projects... The files of wkhtmltox are copied flat into $publish folder but not into $publish/bin/runtimes/.../... where we would expect them. Sorry...

Not sure what the fix is for that, or if it's even possible.

mrblumi commented 2 years ago

Well, this definitively fixes the bug i described in the initial statement of this issue.

But old ASP projects are still broken and one has to mess around with the build config to get this working. This was the only way i found to get the native dlls published into the correct location...

HakanL commented 2 years ago

We may have to just say that the old style csproj isn't supported. Unless someone can figure out a solution and post a PR. I think there may be some examples like SQLite that also has native libraries that could be looked into for ideas.

nickalbrecht commented 2 years ago

Not sure if my problem is the same issue or not (#61, I closed it when I THOUGHT I had fixed it, then found this issue), but I'm hitting a similar issue/symptom. When publishing (dotnet publish), I'm not getting the runtimes folder. In my case (after MUCH trial and error), it seems to be related to RuntimeIdentifier? If I leave out the RuntimeIdentifier, then I get all of the runtimes when I publish, which is overkill, and bloats my publish file's size. My webapp app only runs on Windows. So I specify win-x64 as my runtime identifier. But as soon as I do that, the runtimes folder disappears from the publish output. I do see a wkhtmltox.dll in the root folder with all the other dependencies, but the file's size seems to only matches the x86 version? Actually, looks like that dll is always there, even when I don't specify a runtime identifier. Is that supposed to be there at all? What's it for? However even if that's the correct file, it won't load it because the attributes in the ModuleFactory.cs file are hardcoded to only look in the runtimes folder.

Steps to reproduce

  1. dotnet new console -n Test
  2. dotnet add package Haukcode.WkHtmlToPdfDotNet
  3. dotnet publish --runtime win-x64 --self-contained false
nickalbrecht commented 2 years ago

This looks promising, though the Q/A is a few years old. Not sure if it's changed with the refactoring made to newer versions of .NET like, 5 and 6.

https://stackoverflow.com/questions/40104838/automatic-native-and-managed-dlls-extracting-from-nuget-package

HakanL commented 2 years ago

@nickalbrecht Hmm, no, there shouldn't be a file in the root, don't know where that's coming from. It may be some of the attempts to solve this issue, we do have some Nuget-related commits.

HakanL commented 2 years ago

@nickalbrecht I think the latest version has this resolved. I've also added some test projects in the main solution to test the various platforms. It works for me in .NET 4.6.1, .NET 4.7.1, .NET 4.8, .NET Core 3.1, .NET 5, .NET 6 and also .NET 4.7.1 in the old csproj format. @nickalbrecht and @mrblumi Can you please try version 1.5.64 to see if these issues are resolved?

nickalbrecht commented 2 years ago

I just tried it using the same repro steps I had above, it grabbed 1.5.66 which I'm guessing is you still trying to fix things. It does include a runtimes folder now no matter what I do, but it also included wkhtmltox.dll in the root folder with all the other DLLs (if I specify a runtime), which appeared to be the correct file size for the given runtime. I checked it for both win-x64 and win-x86, and it was the correct file size in both cases. I'm guessing that when specifying a specific runtime (either as a publish argument or just in the csproj file), it would forgo the nested runtimes folder and just copy the contents to the root bin folder. And when you don't specify a runtime, it leaves the resources inside of the runtimes folder. So with 1.5.66, if I specify win-x64, I'm seeing the correct file in the root folder, though, I know it won't be used, because the ModuleFactory has its DllImport attribute hardcoded to only look at the runtimes folder.

Would it be better to allow the runtimes folder to be excluded (as it seemed to be automatically originally, when an exact runtime is specified), and instead have ModuleFactory gracefully failover to looking for the dll in either the root folder (which should theoretically always be the right version) then look in the runtimes folder?

I don't know how this affects the full .NET Framework, as I've only been testing this with .NET 6 however (I'm guessing my observations would be the same with other versions of .NET Core SDK).

nickalbrecht commented 2 years ago

If it helps, when I used to use the DinkHtmlToPdf package (granted, this is on windows), I didn't load the wkhtmltox.dll directly via DllImport, so I didn't have to hardcode the path. Instead, I loaded it using this

internal static class NativeMethods
{
    [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true, EntryPoint = "LoadLibrary")]
    internal static extern IntPtr LoadLibrary(string libname);
}
var ptr = NativeMethods.LoadLibrary(path);
if (ptr == IntPtr.Zero)
{
    throw new Exception($"Error loading {path} (ErrorCode: {Marshal.GetLastWin32Error()})");
}

Which would allow things like checking System.IO.File.Exists()

HakanL commented 2 years ago

If it helps, when I used to use the DinkHtmlToPdf package (granted, this is on windows), I didn't load the wkhtmltox.dll directly via DllImport, so I didn't have to hardcode the path. Instead, I loaded it using this

internal static class NativeMethods
{
    [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true, EntryPoint = "LoadLibrary")]
    internal static extern IntPtr LoadLibrary(string libname);
}
var ptr = NativeMethods.LoadLibrary(path);
if (ptr == IntPtr.Zero)
{
    throw new Exception($"Error loading {path} (ErrorCode: {Marshal.GetLastWin32Error()})");
}

The issue is/was that NativeMethods.LoadLibrary is not available in .NET Core (now I think it's been added in .NET Core 3.0, so it may be an option now, but I forked Dink when it was .NET Core 2.0 and it wasn't available). And we have to make sure all this works in Windows, Mac and Linux/docker. But I'm open to rewrite it, but at the moment I don't have the time to spend to rewrite that part, but I'm happy to accept a PR, hint hint :)

HakanL commented 2 years ago

If there is a way to exclude the runtimes folder when doing publish that would be good, but for the portable publish the recommended way (going off memory here) is to have the runtimes folder for native libraries. I think I had the initial load code in the ModuleFactory to cover that scenario (when the dll is in the root), but at least running it from VS it seems to work for all the different platforms right now.

HakanL commented 2 years ago

I've tested the latest version with all the various client projects and they all seem to work. I'm closing this issue, feel free to re-open if anything related is still broken.