libgit2 / libgit2sharp.nativebinaries

MIT License
28 stars 62 forks source link

#111: Move libgit2 PDB files to legacy symbol package #119

Closed rcdailey closed 3 years ago

rcdailey commented 3 years ago
rcdailey commented 3 years ago

Let me explain the testing I did for this. If additional testing is needed, please let me know and I'll see if I'm able to do that for you.

First, my build and test environment:

Steps:

  1. Follow the README steps for building the project and its packages. Utilized the changes in PR #118 for compatibility with my version of CMake for the 64-bit build of libgit2.

  2. Build the packages with .\buildpackage.ps1 2.1.0-test (latest stable release is 2.0.x, so I used 2.1 to ensure my local package took precedence if needed).

  3. Published the package to a local nuget repository using command .\Publish-Package.ps1 E:\code\nuget\repo\ E:\code\nuget\symbols\ (symbols and package go to different locations).

  4. Edit my %appdata%\NuGet\NuGet.Config file so that it includes my local nuget repository:

    <packageSources>
     <add key="local" value="E:\code\nuget\repo" />
     <add key="nuget.org" value="https://api.nuget.org/v3/index.json" protocolVersion="3" />
    </packageSources>
  5. In the project I have that consumes the libgit2sharp project, add an explicit dependency on the register binaries package to override the implicit dependency on the 2.0.x version. This generates some warnings at build time, but these can be ignored for the purposes of this test:

    <PackageReference Include="LibGit2Sharp" Version="0.*-*" />
    <PackageReference Include="LibGit2Sharp.NativeBinaries" Version="2.1.0-test" />
  6. Run the following command to build and publish a single executable binary of my project. Not all of these options may be necessary to reproduce the issue. This is intended to be comprehensive and representative of my specific scenario.

    dotnet publish src\Trash `
       --output publish `
       --configuration Release `
       --runtime win-x64 `
       --self-contained true `
       -p:PublishSingleFile=true `
       -p:PublishTrimmed=true `
       -p:IncludeNativeLibrariesForSelfExtract=true `
       -p:PublishReadyToRun=true `
       -p:PublishReadyToRunShowWarnings=true

    The full contents of the script where the above command was taken is visible in my public repository here.

  7. Visit the publish directory and observe that the git2-6777db8.pdb file is no longer present next to my executable.

  8. Execute my program and observe that it successfully runs now without errors related to the PDB file missing.

I'm obviously missing tests for the symbol package itself here. I did enough testing to verify that the symbol package is published, however:

  1. I'm not certain if there's even a real use for the symbols by the developers. It could be that the PDBs getting included in the build were collateral damage. If not, I'd love to learn more about the use case for these to ensure that the packages themselves are functional.
  2. My particular use case was only interested in removing the PDB files for reasons explained in

    111. My particular product was deployed with libgit2sharp included, but many people reported

    being unable to run it due to the missing PDB.

bording commented 3 years ago

Given https://github.com/libgit2/libgit2sharp.nativebinaries/issues/111#issuecomment-897316837 and the changes I made in #121, we're not going to need this PR.

Instead, I've just remove the PDBs from the package entirely, which should also solve the problem without needing to introduce symbol packages.

Thanks for the effort on this and the information about the workaround not being valid for .NET 5.

rcdailey commented 3 years ago

I'm glad my PR could help push us forward to a solution. Thank you for taking your time to review this!