search

microsoft / playwright-dotnet

.NET version of the Playwright testing and automation library.
https://playwright.dev/dotnet/
MIT License
2.47k stars 235 forks source link

[Proposal] Fix strong name/assembly signing #1570

Closed avodovnik closed 3 years ago

avodovnik commented 3 years ago

After investigating https://github.com/microsoft/playwright-dotnet/issues/1176, and reading up on documentation I'm unconvinced there is a benefit in providing a strong name for our assemblies.

Especially considering we mainly target .NET Core (even though we support netstandard2.0 we know the majority of users will be using core), where the docs state "For .NET Core, strong-named assemblies do not provide material benefits.".

The other benefits/reasons would be (for .NET framework):

You want to enable your assemblies to be referenced by strong-named assemblies, or you want to give friend access to your assemblies from other strong-named assemblies.

While this was considered for a moment, this would only be needed for internal usage (i.e. testing tools), and honestly, we'd be better staying away from this and implementing that differently.

An app needs access to different versions of the same assembly. This means you need different versions of an assembly to load side by side in the same app domain without conflict. For example, if different extensions of an API exist in assemblies that have the same simple name, strong-naming provides a unique identity for each version of the assembly.

We do not. In fact, it's unlikely to work correctly, due to the fact they'd both reference the same .playwright folder and then start crashing on the driver.

You do not want to negatively affect performance of apps using your assembly, so you want the assembly to be domain neutral. This requires strong-naming because a domain-neutral assembly must be installed in the global assembly cache.

Same point as above - it's unlikely we'd perform well, being installed in the GAC. Equally, the performance penalty of not doing this would be negligible compared to the fact we're actually launching a browser in the back - that is to say, this library is not performance critical enough to consider this a (significant) drawback.

You want to centralize servicing for your app by applying publisher policy, which means the assembly must be installed in the global assembly cache.

~We do not have that requirement, nor do I foresee it happening.~ This might be the one reason we'd consider doing this "right" (i.e. not use PublicSign).

avodovnik commented 3 years ago

I believe we have two options, but both are contingent on us dropping PublicSign:

1) We simply remove strong name, as it seems to be a source of problems 2) We use proper strong name signing for as long as we support the full framework.

avodovnik commented 3 years ago

https://github.com/microsoft/playwright-dotnet/pull/1572 - I've got a draft PR up, that uses the full snk to sign the assemblies

KirillOsenkov commented 3 years ago

Yup, that is best, just fully strong name since you have the private key.