-
Notifications
You must be signed in to change notification settings - Fork 763
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
Use 8.0 era dependencies for non net9.0 TFMs #5470
Conversation
src/Libraries/Microsoft.AspNetCore.HeaderParsing/Microsoft.AspNetCore.HeaderParsing.csproj
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 have a feeling we should go back to the original R9 organisation - i.e., have eng/packages/General.props contain only the general packages, and then have eng/packages/net80.props and eng/packages/net90.props that would contain TFM-specific packages.
Fair enough, I'm not sure how that will work for projects that are not multi-targeting as TFM is not always set at the right time, but I can try doing this and see how it behaves. |
Similar to https://github.com/dotnet/r9/tree/main/eng/Packages. |
@joperezr, what's the schedule for getting this in? We're trying to work through what to do for the various M.E.AI packages. |
src/Libraries/Microsoft.AspNetCore.HeaderParsing/Microsoft.AspNetCore.HeaderParsing.csproj
Outdated
Show resolved
Hide resolved
I'm working on this now, trying to get this in today, or early next week at the latest. |
Thanks. Since you created this, we added the M.E.AI packages. We should be able to have the M.E.AI.Abstractions library on the same plan as what you're creating here, and with a few tweaks, most of the others. The M.E.AI one, though, will need to be exempt for now. |
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.csproj
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.csproj
Outdated
Show resolved
Hide resolved
...ies/Microsoft.Extensions.AI.AzureAIInference/Microsoft.Extensions.AI.AzureAIInference.csproj
Outdated
Show resolved
Hide resolved
Ok, I think this is ready to go and I've resolved all conversations. Marking as auto-merge so that this can go in with the next sign-off. |
@stephentoub looks like the correctness warnings we are getting now are around nullability likely caused by the downgrade of S.T.J and missing annotations. Assuming that in this case the right thing is to suppress the warnings? |
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 not sure splitting the PackageVersions into 2 files helps that much, since the conditions across the 2 files need to line up. But I don't feel strongly it needs to be changed back.
LGTM. We need to make this change for 9.0.0.
Not sure why this triggers nullability warnings, but go ahead and revert the azure inference change and I'll fix it in a subsequent pr. |
Thanks a lot @stephentoub and @eerhardt for helping pushing this through. |
@stephentoub this change has upset the CI build:
|
Well, I agree it was related, it was just against the source index portion of the build. I'm looking into this and will disable that if required. |
Why didn't it fail CI as part of this PR? Regardless, putting this back might fix it: |
There are steps that can only be run on the internal infra. |
Fixes #5462
cc: @eerhardt
This change makes it such that packages produced from this repository will depend on 8.0 era packages when consuming them from a project that targets net8.0 (and net462), and on 9.0 era packages when consuming them from a project that targets net9.0.
This is in favor of allowing customers that can only consume LTS versions of the rest of .NET to not be lifted when referencing packages from dotnet/extensions.
FYI: @ericstj @stephentoub @sebastienros @DamianEdwards @RussKie
Microsoft Reviewers: Open in CodeFlow