Add strong name signing to ParquetSharp

[DanSmith]

Implements strong name signing #296
Also strong names the test project, due to being a friend assembly

[mfkl]

Personally against this change for reasons, but I'm not a parquetsharp maintainer, these are just my 2 cents :-)

[adamreeve]

Apologies for the awkward format check that doesn't tell you what's wrong. You should be able to run the formatter locally to fix any issues:

dotnet tool restore
dotnet jb cleanupcode "csharp" "csharp.test" "csharp.benchmark" --profile="Built-in: Reformat Code" --settings="ParquetSharp.DotSettings"  --verbosity=WARN

[marcin-krystianc]

@DanSmith Apologies for realising this only now, but it seems that adding a strong name to the assembly is a breaking change. My impression was that only removing the strong name or changing the key used for signing is a breaking change, but adding a strong name breaks backward compatibility too.
Let's consider such example ConsoleAppParquetSharp.zip:

App --> ParquetSharp 8.0.0-beta2 (with the strong name)
  \
    --> Lib1 --> ParquetSharp 7.0.0 (without the strong name)

When we try to run such App, it fails with:

Unhandled Exception: System.IO.FileLoadException: Could not load file or assembly 'ParquetSharp, Version=7.0.0.0, Culture=neutral, PublicKeyToken=null' or one of its dependencies. The located assembly's manifest definition does not match the assembly reference. (Exception from HRESULT: 0x80131040)

For small solutions with small dependency trees it would be something that people would be able to deal with. However for large dependency tree, particularly for which some of dependencies to ParquetSharp come from external NuGet packages it would become unmanageable.

[DanSmith]

Hi Marcin,

Thanks for the example showing how this is a breaking change. Does this mean that this type of breaking change must be held up until a new major version, or grouped with other breaking changes that might come along in the future?

[marcin-krystianc]

Hi Marcin,

Thanks for the example showing how this is a breaking change. Does this mean that this type of breaking change must be held up until a new major version, or grouped with other breaking changes that might come along in the future?

This change is going to introduce diamond-dependency conflicts which will be extremely painful to deal with. Unfortunately, I don't see anything that we could do to minimize the negative impact for our users. As the amount of problems caused by this PR outweighs its benefits, we cannot accept it. Again, I would like to apologize for not noticing that sooner.

[adamreeve]

As Marcin said, I don't think there's any way to work around this being a breaking change and it's not something we'd be happy changing even for a major release so I'm going to close this. Apologies for misleading you and saying we would accept this, I wasn't aware of this issue originally.