-
Notifications
You must be signed in to change notification settings - Fork 10.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SE.Redis DC backend / HybridCache : add net8.0 TFM #56122
Conversation
…rnet on Aspire which targets LTS hence 8
probably also /cc @wtgodbe in case this is a "Known Bad Thing™, verboten" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable to me, but I'd let @wtgodbe take a look before merging.
src/Caching/StackExchangeRedis/src/PublicAPI/net8.0/PublicAPI.Shipped.txt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little wary of this given that it's something we haven't done before, but given that this is an OOB package & not a library that's in the SharedFx, it's probably not too risky. Approved modulo a few questions
Context: Aspire has Garnet support via
AddGarnet()
; Aspire targets LTS, so 8.0; we would like Aspire's Garnet support to work for more than just the rawIConnectionMultiplexer
usage; specifically, we would like at least some of the inbuilt cache implementations to work. Prior to 9.0, the redis-basedIDistributedCache
does not work with Garnet, because it uses Lua, which Garnet does not support. It is not currently planned to backport the necessary Garnet support to 8.0.In theory, the pre-existing NS targets on these libraries means that 8.0 apps can consume the 9.0 OOB package and work, but we have been advised to avoid the NS target here, as crossing the streams may cause some assembly load problems at runtime. The suggestion is to add an 8.0 TFM to the 9.0 build, which is unusual but should be valid. In particular, this applies to:
(note that the "runtime" piece i.e. Microsoft.Extensions.Caching.Abstractions already has an explicit 8.0 TFM in the 9.0 build)
Both of these already have down-level targets, so this is not "new", just a new explicit TFM in the existing matrix.
To avoid leaving 8.0 in hard-to-find places, I have also introduced
$(CurrentLtsTargetFramework)
viaeng/Versions.props
; I have validated that this continues to work correctly when it matches the$(DefaultNetCoreTargetFramework)
/cc @mitchdenny