Strong naming

[nathan-alden-sr]

According to Microsoft guidelines, public libraries should be strong-named. Today, I posted a similar discussion on the Stylet repo and had Tanner chime in with specifics. Please take a look at his comments there.

I started playing around in my fork with adding strong naming. Unfortunately, some of the NuGet packages that Silk.NET relies on are not strong-named, meaning Silk.NET itself cannot be strong-named in those cases. Unfortunately, the Silk.NET solution is so huge and relies on things I just can't get to work properly like Android SDKs that I am unable to build the entire solution to test my changes (the thousands of warnings during build do not help). I would have to submit a PR and then analyze the CI build to see if the changes work.

See the changes in my fork (ignore the accidental solution file changes as they are not required): https://github.com/nathan-alden-sr/Silk.NET/commit/da3c192be295861b07e5956d5ae22a8fe0728a58 (fork recreated) https://github.com/nathan-alden-sr/Silk.NET/commit/a38c88b693ca18c6c7d305e6dbab24d35cb1f05f.

I took the approach that a src/Directory.Build.props file was the best place to configure the strong-naming properties. This is how I do it in several of my own projects. This avoids unnecessarily strong-naming test and sample projects. I added a Condition to the <PropertyGroup> that restricts strong naming to net5.0 and above. We may want to change this due to mobile TFMs; not sure.

After making my changes, I built Silk.NET.Maths with this command:

dotnet build --configuration Debug /p:TargetFramework=net5.0

Then, I verified the strong name with this command:

> sn -v Silk.NET.Maths.dll

Microsoft (R) .NET Framework Strong Name Utility  Version 4.0.30319.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Assembly 'Silk.NET.Maths.dll' is valid

I also verified that Silk.NET.Maths was not strong-named when built with the netstandard2.0 TFM:

> sn -v Silk.NET.Maths.dll

Microsoft (R) .NET Framework Strong Name Utility  Version 4.0.30319.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Silk.NET.Maths.dll does not represent a strongly named assembly

[HurricanKai]

I would hope that we can do strong naming, and I think src/Directory.Build.props is the wrong place to configure this.
We only need to strong name the bindings, Core, Maths, Windowing, Input and maybe SilkTouch.
BuildTools, NUKE, Lab, etc. don't need to be strong named, as they aren't user facing.
Have a look at build/props/common.props, putting the configuration there should reach all projects that are published, as this is also where we configure stuff like NuGet version.

[nathan-alden-sr]

I initially put it there, but I honestly wasn't sure what it applied to. The way you guys do MSBuild is different than I'm used to (relying on Directory.Build.* and folder structures).

Would you like me to put the configuration there and then submit a PR so we can see what CI does with it?

[nathan-alden-sr]

PR submitted (primarily as a CI check).

[nathan-alden-sr]

@Perksey Check out the two links at the top of my original post here. They link to the relevant Microsoft documentation that says public APIs should be strong-named, and also a comment from Tanner on another library that is also not strongly-named, Stylet.

As far as the TFMs to strong-name, if you want me to include all of them then I will remove the Condition. However, I think it might be risky to include them in any .NET Framework TFMs since it could affect existing apps' binding redirects. Tanner might be able to provide some wisdom here.

[nathan-alden-sr]

Ah, I remember now: some projects reference other libraries that are not strongly-named, so I couldn't strongly-name those TFMs. An example of this is Ultz.Bcl.Half.

[nathan-alden-sr]

I guess I never explicitly said this in the original post, but I want to strong name Vorpal Engine, which means I can't use Silk.NET otherwise. Strong-named libraries can only reference other strong-named libraries.

[tannergooding]

The benefits are primarily for .NET Framework and therefore for libraries that are targeting .NET Framework or .NET Standard.

As per the docs the SNK isn't really used in .NET Core or .NET 5+. It will still show up in things like the full assembly name, but its not used in the same context or consideration for things like binding anymore.
As such, while there is no real benefit to using SNK for .NET Core/.NET 5+, there should be no downside either and I do it simply for consistency.

[nathan-alden-sr]

I think I may abandon this for now. Perhaps a better time to pick this up is during or after you complete the net6.0 cleanup work. Unfortunately, not being able to reliably build the solution is hindering my ability to test all the scenarios. Additionally, it seems that Silk.NET makes use of many more NuGet packages than I had thought, with at least a couple of them not being strong-named.

I don't mind picking this back up in the future when it is easier to interact with the solution.