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

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

Closed
mrblumi opened this issue Jun 7, 2021 · 24 comments
Closed
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@mrblumi
Copy link

mrblumi commented Jun 7, 2021

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:

  • dotnet new console -n Test -o ./
  • changing target framework to net48 in csproj file
  • dotnet add package Haukcode.WkHtmlToPdfDotNet --version 1.5.8
  • dotnet publish --configuration=Release

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
Copy link
Author

mrblumi commented Jun 7, 2021

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
Copy link
Owner

HakanL commented Jun 7, 2021

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
Copy link
Author

mrblumi commented Jun 7, 2021

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
Copy link
Author

mrblumi commented Jun 8, 2021

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
Copy link
Owner

HakanL commented Jun 8, 2021

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
Copy link
Author

mrblumi commented Jun 8, 2021

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

@HakanL HakanL added bug Something isn't working help wanted Extra attention is needed labels Jun 8, 2021
@horizondave
Copy link

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
Copy link

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

@HakanL
Copy link
Owner

HakanL commented Oct 5, 2021

@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
Copy link
Author

mrblumi commented Oct 5, 2021

I can test this tomorrow afternoon :)

@horizondave
Copy link

Version 1.5.59 works fine for me, many thanks :)

@mrblumi
Copy link
Author

mrblumi commented Oct 7, 2021

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
Copy link
Owner

HakanL commented Oct 7, 2021

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
Copy link
Author

mrblumi commented Oct 7, 2021

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
Copy link
Owner

HakanL commented Oct 7, 2021

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
Copy link

nickalbrecht commented Nov 23, 2021

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
Copy link

nickalbrecht commented Nov 24, 2021

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
Copy link
Owner

HakanL commented Nov 24, 2021

@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
Copy link
Owner

HakanL commented Nov 24, 2021

@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
Copy link

nickalbrecht commented Nov 24, 2021

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
Copy link

nickalbrecht commented Nov 24, 2021

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
Copy link
Owner

HakanL commented Nov 24, 2021

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
Copy link
Owner

HakanL commented Nov 24, 2021

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
Copy link
Owner

HakanL commented May 16, 2022

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.

@HakanL HakanL closed this as completed May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants