[DONE; update documentation] publish Polly v6.0.0 as strong-named only
reisenberger opened this issue · 16 comments
CONSIDER publishing Polly v6.0.0 as strong-named only
Background
ASP.NET Core 2.1 is moving to consume Polly for HttpClient instances created through HttpClientFactory. This will allow the easy application of Polly resilience strategies to outbound calls through HttpClient.
This will significantly increase consumption of Polly's strong-named package (currently labelled Polly-Signed). That however will create conflicts for packages or projects that currently consume non-strong-named Polly: example; longer discussion.
There is no easy answer to this problem, as attested by discussion on numerous projects: octokit and the many projects that reference that issue.
Options:
Be purely non-strong-named: project cannot then be consumed in environments or by upstream packages mandating strong-naming; not an option for Polly due to ASP.NET Core.- Offer both strong-named and not (as Polly currently): but creates conflicts where a project ends up transiently referencing both.
Or:
Publish only strong-named
Some consensus among high-consumption projects now is around this, the NewtonSoft.Json strategy, as the least of all evils. @JeremySkinner of FluentValidation sums it up well here as does @sharwell elsewhere. I summarise also here.
TL;DR Switching to release Polly as strong-named only would be a one-time hit breaking change for consumers of Polly. But thereafter, having only the strong-named version of Polly in the market would prevent/reduce those conflicts (strong-named versus not) going forward.
(edited: for typos)
This is a proposal on which we would appreciate evidence-driven feedback. Particularly any consumption scenarios we may not be aware of. The proposed change is driven by the HttpClientFactory ASP.NET Core integration, which will drive many more Polly users to the strong-named version. ASP.NET Core has prior decision to be strong-named and thus consume strong-named assemblies.
I am leaning toward publishing only strong-named, starting with version 6.0. This would result in us ceasing the Polly-Signed lineage, and we'd need to make it very clear in several places that the assembly in the Polly package is strong-named.
I had initially thought of having a new namespace called something like PollyCore that is strongly signed, and maintain that separately like we do with Polly and Polly-Signed, in order to reduce conflicts. Then thought better of it as adding more confusion and then dealing with ambiguous method names, etc. when you bring in HttpClientFactory to an existing project that uses Polly already. The other downside is that you'd have two identical DLLs, but with different names. This is not ideal for various reasons.
I am leaning toward publishing only strong-named, starting with version 6.0. This would result in us ceasing the Polly-Signed lineage, and we'd need to make it very clear in several places that the assembly in the Polly package is strong-named.
.NET Framework treats changing the signing key on the assembly, including adding and removing one, as a different assembly. That means all libraries that depends on the package will break when someone upgrades to your new package until the library is recompiled.
That is fine for apps - people will recompile after upgrading packages - but other NuGet packages they use that depend your package will break until they are upgraded.
Read up on what happened when log4net changed its key.
This situation might have improved on .NET Core.
I commented over here that consistency in the ecosystem is probably the best outcome we can hope for. The push to signed policy created by asp.net HttpClientFactory is essentially a fait accompli here, we are all going to have to take the signed version. So it would be easier to create a pit of success.
Strong naming is problematic, but I won't solve that problem...
Briefly to add some stats around the dependent packages issue highlighted ^^ by @JamesNK (thanks!), I don't know if the following stats are accurate/complete, but libraries.io helpfully gives us some idea of the packages currently dependent on Polly unsigned:
- dependent packages - 163
- repos - 267
EDIT: To estimate the ripple out effect more: The number of dependent packages is as above, but the number of dependent packages that in turn have widespread consumption is small (ie projects with more than a handful of stars per libraries.io; some ad-hoc checks suggest this has correlation with high downloads).
Brighter project will depend on this change
BrighterCommand/Brighter#287
💭 Is adding a strong name a binary breaking change for applications that are not strong-named? It's been so long since I did anything with an assembly that wasn't strong named that I'm not sure the answer.
@sharwell Yes, see this Brighter issue where we are tracking: BrighterCommand/Brighter#286
But, we are happy to go this route. It seems to be the best path forward. We'll have to release as a 'breaking change' but we can live with that issue.
Thanks @iancooper and @rvanmaanen for highlighting this important issue.
x-ref aspnet/HttpClientFactory#108 where I am querying the ASPNET Core team on the timing for this.
Yes, see this Brighter issue where we are tracking: BrighterCommand/Brighter#286
This a different issue.¹ The situation I refer to would be the following:
- Create a library MyLibrary which does not have a strong name
- Create a command line application MyApp which does not have a strong name
- Recompile MyLibrary with a strong name
- Copy the binary from step (3) to the output directory from step (2), overwriting the binary in that directory originally from step (1)
- Attempt to run MyApp without recompiling it
If the above steps are successful, it would indicate that replacing a non-strong-name assembly with a strong-name assembly would not be a binary breaking change. The key insight leading to this conclusion is step (2) - specifically it's not possible to create MyApp with a strong name at this point.
Edit: I ran this test locally and found the following output in step (5):
Unhandled Exception: System.IO.FileLoadException: Could not load file or assembly 'StrongNameBreakingTest, Version=1.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)
¹ The linked issue differs from the case I am describing because the application in question references both the non-strong-name and the strong-name forms of the assembly at the same time.
@pardahlman (for RawRabbit); @TomPallister (for Ocelot); @fanliang11 (for surging): @RamjotSingh (for Microsoft.Bot.Connector.Teams)
libraries.io suggests you maintain libraries which have high downloads on nuget or high github stars, and consume Polly as a dependency. As a courtesy, please read this thread: Polly intends, from Polly v6.0.0, to switch to publish only as a strong-named version.
Polly v6.0.0 will be a release functionally identical to Polly v5.9.0, but a major-version bump (v6.0.0) as we switch to purely strong-naming.
Current (up to Polly v5.9.0)
| nuget package | kind |
|---|---|
| Polly | non strong-named |
| Polly-Signed | strong-named |
Polly v6.0.0 onwards
| nuget package | kind |
|---|---|
| Polly | strong-named |
Impact
Both non-strong-named DLLs and strong-named DLLs can reference strong-named DLLs.
Whether the DLLs in your package are themselves strong-named or not, you will be able to reference the new Polly v6.0.0 strong-named package.
Why is Polly doing this?
ASPNET Core 2.1 is moving to consume Polly for HttpClient instances created through HttpClientFactory. This will allow the easy application of Polly resilience strategies to outbound calls through HttpClient.
This is expected to significantly increase consumption of Polly's strong-named package (currently labelled Polly-Signed). That however will create conflicts which apparently have no solution for packages or projects that currently consume non-strong-named Polly (example) (good summary) and wish to use the new ASPNET Core 2.1 features.
If we leave both non-strong-named and strong-named Polly in the market, we prolong the possibility of these conflicts into future months and years. We prolong the possibility that we (and you) will have users who complain that products are unusable, because some product (perhaps consuming yours) requires strong-named Polly as a transitive dependency via one axis, and non-strong-named Polly as a transitive dependency via another axis.
If we switch to a single, strong-named Polly release, there is initial impact while products switch to strong-named Polly, but the duration of pain is limited by there being only a single version in the market going forward.
This strategy is the strategy used by many high-download nuget packages such as NewtonSoft.Json, FluentValidation, and Serilog, and is the strategy recommended by the Microsoft ASP.NET team.
Reducing onward impact
We are also adopting the so-called Newtonsoft.Json strategy described here to minimise onward impact. In this strategy:
- nuget package numbers follow full semver (Major.Minor.Patch increments) to differentiate bug-fixes, new-functionality-without-breaking-changes, and breaking-changes.
- Dlls partially follow Major.0.0 version numbering only (even if the true version is Major.Minor.Patch). This reduces the burden on projects which consume Polly of introducing excessive assembly binding redirects.
Edited: to fix links
Following community discussion, we believe this to be the best way forward (but comments and questions are welcome!).
We've just released Polly version 6.0.0-alpha to nuget.org. It has the following characteristics:
- Publish as strong-named package only (discontinue non-strong-named versions)
- Add .NetStandard 2.0 tfm
- Provide .NET4.5 support via .NetStandard 1.1 tfm
- Discontinue .NET4.0 support
- Remove methods marked as deprecated in v5.9.0
The first item is the one that addresses the primary issue. Because this is related to Polly and its relationship to ASP.NET Core 2.1,we're working on releasing Polly.Extensions.Http 2.0 alpha tomorrow or the next day, which follows the same single-strong-name package method and will reference this new Polly v6.0 package.
UPDATE: Polly.Extensions.Http 2.0 alpha has been published. This follows the single-strong-named package approach and references Polly v6.0-alpha.
Thanks for the heads up, Ocelot uses Polly so I will do whatever I need to fit in with you :)
Thanks for the amazing work and the Polly project!
Polly v6.0.1 is published, delivering the changes discussed in this thread.
Leaving the issue open as I intend to transition the discussion from this thread and aspnet/HttpClientFactory#108, aspnet/HttpClientFactory#105 into the Polly wiki, as documentation.
Documentation published to wiki