Strong name the assemblies in this package #1467
Conversation
|
Sourcy and EnumerableAsyncProcessor are my libraries too so I can easily accept a pr to strong sign them and push out a new version of each. Trxparser isn't mine though. |
This reduces the dependencies we need to get strong name signed down to just `EnumerableAsyncProcessor`. Fortunately, no one in this repo uses InternalsVisibleTo to this project, or it would _have_ to be strong named.
This avoids warnings about non-strong named TUnit assemblies that come from previously published nuget packages. Once strong named assemblies are published to nuget.org and that newer version is consumed in this cycle, we *can* start strong naming this project if desired.
|
I was able to filter the problem to just |
AArnott
had a problem deploying
to
Pull Requests
GitHub Actions
Failure
AArnott
had a problem deploying
to
Pull Requests
GitHub Actions
Failure
AArnott
had a problem deploying
to
Pull Requests
GitHub Actions
Failure
AArnott
had a problem deploying
to
Pull Requests
GitHub Actions
Failure
|
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
The EnumerableAsyncProcessor issue is now resolved, but now we have two new ones (OneOf, and Vogen.SharedTypes) which I've added to the PR description. |
They're only used in the test example project. They aren't part of any packages. Should be okay? |
This eliminates the last two non-strong-named dependency warnings.
That can make it OK, but if you have InternalsVisibleTo to that test assembly from a strong named project, then the test project must also be strong named. (aside: I have InternalsVisibleTo so bad... for several reasons). In your case, it appears this restriction does not impact you, because you don't use IVT to this test project from a strong named project. Yay. I've pushed another update to remove the last two warnings. |
| @@ -13,6 +13,10 @@ | |||
| <NoWarn>NU1507;NU1903;CS9107</NoWarn> | |||
| <BuildTransitivePath>buildTransitive/$(TargetFramework)/</BuildTransitivePath> | |||
| <BuildPath>build/netstandard2.0</BuildPath> | |||
|
|
|||
| <SignAssembly>true</SignAssembly> | |||
There was a problem hiding this comment.
If this is in the targets file instead, you could do something like:
<SignAssembly>'$(IsLibraryPackage)' == 'true'</SignAssembly>
That variable is applied to the NuGet package projects, and everything else should remain unsigned.
There was a problem hiding this comment.
At least as of when the PR was new, some non-shipping projects did need to be signed because you had InternalsVisibleTo exposed to them. And as of yesterday when I refreshed the PR, I saw you still have IVT in use, though I didn't pay attention to which assemblies you exposed internals to.
If you feel confident that you don't use IVT from shipping to non-shipping assemblies, then yes, I can give your suggestion a try.
AArnott
temporarily deployed
to
Pull Requests
GitHub Actions
Inactive
AArnott
temporarily deployed
to
Pull Requests
GitHub Actions
Inactive
AArnott
temporarily deployed
to
Pull Requests
GitHub Actions
Inactive
AArnott
temporarily deployed
to
Pull Requests
GitHub Actions
Inactive
|
I've just refactored so the only main project IVT usage is Core > Engine |
|
Alright then. I'll take another look. |
Closes #520
Remaining issues
Strong naming requires that all referenced assemblies are also strong named. Failure to comply with this emits a warning from the C# compiler and an assembly load failure at runtime on .NET Framework.
This repo has references to a few assemblies that are not strong named. So to succeed at strong naming this repo, we must first get the upstream dependencies strong named:
While some of these are only dependencies of tests, and the tests would not strictly need to be strong named for your users, your repo uses InternalsVisibleTo, and a strong named assembly is only allowed to use InternalsVisibleTo to other strong named assemblies. So if you expose internals to your tests, you have to strong name them too.