Fix NuGet package publishing [9.0.0-beta8]

[nenadvicentic]

Does this issue relate to a new feature or an existing bug?

• Bug

What version of Serilog.Sinks.Elasticsearch is affected? Please list the related NuGet package.
9.0.0-beta8

Please describe the current behavior?
After updating .NET Framework web project from Serilog.Sinks.Elasticsearch version 8.4.1 to verion 9.0.0-beta8 compilation of project works, but on the startup assembly binding error occurs:

Could not load file or assembly 'Serilog.Formatting.Elasticsearch' or one of its dependencies. Strong name signature could not be verified. The assembly may have been tampered with, or it was delay signed but not fully signed with the correct private key. (Exception from HRESULT: 0x80131045)

Please describe the expected behavior?
Website should start/run without assembly-binding issues.

If the current behavior is a bug, please provide the steps to reproduce the issue and if possible a minimal demo of the problem
Update .NET Framework 4.8 project from earlier stable version of Serilog.Sinks.Elasticsearch (e.g. 8.4.1) to 9.0.0-beta8 and run the project.

[nenadvicentic]

Upon comparison of 9.0.0-beta8 packages with version 8.4.1 following has been found:

• For Serilog.Sinks.Elasticsearch:
  • DLL inside of NuGet package 8.4.1 has version of 8.4.1.0
  • DLL inside of NuGet package 9.0.0-beta8 has version of 6.0.0.0 (which is hard-coded in AssemlyInfo.cs file).
• For Serilog.Formatting.Elasticsearch NuGet package:
  • DLL inside of NuGet package 8.4.1 has version of 0.0.0.0, targeting net451, netstandard1.3 and netstandard2.0
  • DLL inside of NuGet package 9.0.0-beta8 has version of 0.0.0.0, targeting netstandard2.0 only.

Most likely cause of the problem with .NET Framework projects upgrade is that assembly from both versions of NuGet package Serilog.Formatting.Elasticsearch has version 0.0.0.0 and same public key. However, when .NET Framework 4.8 project uses older, 8.x version of Serilog.Sinks.Elasticsearch, it will pick up Serilog.Formatting.Elasticsearch targeting net451. After the NuGet package update to 9.0.0-beta8, assembly will have same version 0.0.0.0, same public key, but, since net451 target has been removed, project/msbuild will pick up assembly .NET Standard 2.0. Wherever caching of dlls occurs, we won't be sure any more which dll has been picked up.

More details about the differences between 8.x and 9.0.0-beta8 NuGet packages can be seen in the screenshots bellow.

Serilog.Sinks.Elasticsearch 9.0.0-beta8 package:

Serilog.Formatting.Elasticsearch 9.0.0-beta8 package:

Serilog.Sinks.Elasticsearch 8.4.1 package:

Serilog.Formatting.Elasticsearch 8.4.1 package::

[nenadvicentic]

Possible solution for this problem could be to overwrite assembly version of the dlls during the build.

To determine correct version and informational version of the dlls, GitTools/Action GitVersion can be used. There is an example here listing all version+git-branch variables.

    steps:
      - name: Checkout git repository
        uses: actions/checkout@v3
        with:
          fetch-depth: 0

      - name: Determine Version
          uses: gittools/actions/gitversion/[email protected]
          id: gitversion

And the build action can be modified to something like:

      - name: Build solution
        run: dotnet build --no-restore -c Release -p:Version=${{ steps.gitversion.outputs.majorMinorPatch  }}.$GITHUB_RUN_ID -p:InformationalVersion=${{ steps.gitversion.outputs.informationalVersion}}

I have used $GITHUB_RUN_ID variable from the documentation here as example for "Patch" part of assembly version. Perhaps better approach can be used.

For the current branch e3876b, running GitVersion.exe gives following results:

{
  "Major": 9,
  "Minor": 0,
  "Patch": 0,
  "PreReleaseTag": "beta.8",
  "PreReleaseTagWithDash": "-beta.8",
  "PreReleaseLabel": "beta",
  "PreReleaseLabelWithDash": "-beta",
  "PreReleaseNumber": 8,
  "WeightedPreReleaseNumber": 8,
  "BuildMetaData": null,
  "BuildMetaDataPadded": "",
  "FullBuildMetaData": "Branch.dev.Sha.e3876b1da84fc734febb4ac8ba2d486efb5a6292",
  "MajorMinorPatch": "9.0.0",
  "SemVer": "9.0.0-beta.8",
  "LegacySemVer": "9.0.0-beta8",
  "LegacySemVerPadded": "9.0.0-beta0008",
  "AssemblySemVer": "9.0.0.0",
  "AssemblySemFileVer": "9.0.0.0",
  "FullSemVer": "9.0.0-beta.8",
  "InformationalVersion": "9.0.0-beta.8+Branch.dev.Sha.e3876b1da84fc734febb4ac8ba2d486efb5a6292",
  "BranchName": "dev",
  "EscapedBranchName": "dev",
  "Sha": "e3876b1da84fc734febb4ac8ba2d486efb5a6292",
  "ShortSha": "e3876b1",
  "NuGetVersionV2": "9.0.0-beta0008",
  "NuGetVersion": "9.0.0-beta0008",
  "NuGetPreReleaseTagV2": "beta0008",
  "NuGetPreReleaseTag": "beta0008",
  "VersionSourceSha": "e3876b1da84fc734febb4ac8ba2d486efb5a6292",
  "CommitsSinceVersionSource": 0,
  "CommitsSinceVersionSourcePadded": "0000",
  "UncommittedChanges": 0,
  "CommitDate": "2023-01-25"
}

[mivano]

I tried to set it with

- name: Create Release NuGet package
  run: |
    arrTag=(${GITHUB_REF//\// })
    VERSION="${arrTag[2]}"
    VERSION="${VERSION//v}"
    dotnet pack -v normal -c Release --include-symbols --include-source -p:PackageVersion=$VERSION -p:Version=$VERSION -p:InformationalVersion=$VERSION -o ./nuget

So that should set the version, but no luck yet. It still shows 6.0.0 :-(
The packaging happens when a new release is created, it will pick the tag and use that as its version.
Tried to delete the assembly info values, but no luck as well.
I will need to dive into it more when I have some time available.

[nenadvicentic]

@mivano I made a pull request where I removed AssemblyInfo.cs. Also wrote local commands that produce properly version package. Those will have to be replaced by CI variables, as you wrote in the example.

Just a note regarding:

dotnet pack -v normal -c Release --include-symbols --include-source -p:PackageVersion=$VERSION -p:Version=$VERSION -p:InformationalVersion=$VERSION -o ./nuget

Part -p:PackageVersion=$VERSION is not needed.

Update: actually, I just discovered that -p:InformationalVersion is not needed as well. -p:Version=$VERSION will do all the magic.

I assume that this will be enough:

- name: Create Release NuGet package
  run: |
    arrTag=(${GITHUB_REF//\// })
    VERSION="${arrTag[2]}"
    VERSION="${VERSION//v}"
    dotnet pack -v normal -c Release --include-symbols --include-source -p:Version=$VERSION -o ./nuget

[mivano]

Thanks for your investigation! I merged the PR (also fixed that annoying test report issue) and changed the pipeline as you indicated. The latest beta11 package now shows 9.0.0, so I hope we are good now.

[nenadvicentic]

All this looks really good now! Thanks also to you for the support.

I will test the package with .NET (Core) and .NET Framework projects tomorrow during the day and let you know how it went.

[nenadvicentic]

@mivano With 9.0.0-beta11 I ran into the same issue like before (can't verify assembly signature).

After some reading and testing new package builds on my dev machine, it seems that the issue is caused by property in *.csproj:

<PublicSign Condition=" '$(OS)' != 'Windows_NT' ">true</PublicSign>

Now it's explicitly set to false, no matter on which operating system is NuGet package built. I only tested building a new version of NuGet package with this setting on Windows 10, where everything works fine, after I pull new package into my projects (.NET Framework), but I hope it works on Linux build server as well. Change is in this PR: #504

[mivano]

Keeps an interesting issue that signing :-) I just pushed the beta12 to nuget.

[mivano]

Looks like the serilog main library still has the same (https://github.com/serilog/serilog/blob/dev/Directory.Build.props) although it uses appveyor as its build agent.

[nenadvicentic]

@mivano The version 9.0.0-beta12 is working well. The issue with assembly binding is gone.

Looks like the serilog main library still has the same (https://github.com/serilog/serilog/blob/dev/Directory.Build.props) although it uses appveyor as its build agent.

I assume that they are using Windows machine for CI build, while we are using Linux. I can only guess that they disabled signing on Linux because it did not work at the time, or they had some specific scenario when development is done on Linux box (or inside of Linux Docker container).

There is only one little thingy, NuGet "thinks" that 9.0.0-beta9 is newer that 9.0.0-beta12 (newer version is decided alphabetically), so it can happen that update on Serilog.Formatting.Elasticsearch 9.0.0-beta9 gets picked up, if "Update" is run.

I would suggest two things:

• Unrelated to this, but to be more precise in ReadMe.md. Something along the lines of "Version 9.0.0 of the sink targets netstandard2.0 and therefore can be run on any .NET runtime that supports it (.NET Core versions and old .NET Framework), however, we are focused on testing it with .NET 6.0 to make the maintenance simpler.". I would not discourage people on .NET Framework 4.8 from upgrading, since current version 8.4.1 creates issues with typeName. I am testing it now with both .NET Framework 4.8 and .NET Core 6/7.
• Back to the package itself, I would suggest you create version 9.0.0-rc1 from the same code (perhaps with ReadMe updated), so we avoid this issue of alphabetical sorting. I would, then, update couple of apps with 9.0.0-rc1 (.NET Framework 4.8, .NET Core 6 and 7) and run it in a production for a few days. If everything woks fine, we can release final version.

[mivano]

Great!

I updated the readme with the suggested text to make it make clear.

For the package version; I had not anticipated that we would reach 12 otherwise I would have upped the version or prefixed it :-). I will create a rc release now.

[mivano]

Package is available: https://www.nuget.org/packages/Serilog.Sinks.Elasticsearch/9.0.0-rc1

[nenadvicentic]

@mivano I put 4 apps on prod with RC1. Two .NET Framework 4.8, two .NET 7.0. All go against Elasticsearch 7.x server. From my point of view, this is mainly to test if everything is OK under a load and it looks good so far. Let's give it another day.

[nenadvicentic]

@mivano The logging is working fine on production with RC1. Tested with 5 different apps against Elasticsearch 7.x. One of the apps is logging quite intensively. There were no issues.

From my point of view, we can release of 9.0.0 version.

[mivano]

That is great news! I will create the release package.

[mivano]

And the final v9.0.0 version: https://www.nuget.org/packages/Serilog.Sinks.Elasticsearch/9.0.0

https://github.com/serilog-contrib/serilog-sinks-elasticsearch/releases/tag/v9.0.0

Thanks for all your efforts @nenadvicentic !

[nenadvicentic]

@mivano Awesome! Really glad we did it!

Thank you, as well! Without your support it would not be possible to make such a good progress!