-
Notifications
You must be signed in to change notification settings - Fork 10k
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
[master] Update dependencies from dotnet/efcore #18247
[master] Update dependencies from dotnet/efcore #18247
Conversation
…109.3 - Microsoft.EntityFrameworkCore.Tools - 5.0.0-alpha.1.20059.3 - Microsoft.EntityFrameworkCore.SqlServer - 5.0.0-alpha.1.20059.3 - dotnet-ef - 5.0.0-alpha.1.20059.3 - Microsoft.EntityFrameworkCore - 5.0.0-alpha.1.20059.3 - Microsoft.EntityFrameworkCore.InMemory - 5.0.0-alpha.1.20059.3 - Microsoft.EntityFrameworkCore.Relational - 5.0.0-alpha.1.20059.3 - Microsoft.EntityFrameworkCore.Sqlite - 5.0.0-alpha.1.20059.3
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
…109.2 - Microsoft.EntityFrameworkCore.Tools - 5.0.0-alpha.1.20059.2 - Microsoft.EntityFrameworkCore.SqlServer - 5.0.0-alpha.1.20059.2 - dotnet-ef - 5.0.0-alpha.1.20059.2 - Microsoft.EntityFrameworkCore - 5.0.0-alpha.1.20059.2 - Microsoft.EntityFrameworkCore.InMemory - 5.0.0-alpha.1.20059.2 - Microsoft.EntityFrameworkCore.Relational - 5.0.0-alpha.1.20059.2 - Microsoft.EntityFrameworkCore.Sqlite - 5.0.0-alpha.1.20059.2
It looks like this is blocked on an update to get the ef tool to roll forward to netcoreapp5.0 like @dougbu suggests in #17947 (comment) |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
@halter73 @bricelam's fix for that (dotnet/efcore#19515) went in a couple of days ago and was included in this build. The problem now looks like something different, probably missing direct dependencies on packages that efcore is using old versions of. @JunTaoLuo @wtgodbe didn't we resolve those issues already? |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
716dacd
to
160af47
Compare
160af47
to
d427e2a
Compare
I'm trying to fix this by reapplying @wtgodbe's workaround to add direct dependencies to a bunch of EF projects. @wtgodbe do you think the change to ProjectTemplates.Tests.csproj to hard code netcoreapp3.1 for the DotNetEfFullPath is still necessary? |
The following shows the kind of build errors we were seeing before adding the direct dependencies:
|
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
I think this is still affected by not having updates from |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
That's correct, we still are blocked until runtime starts flowing.
Hopefully not, now that EF is using roll-forward. |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
@jaredpar thinks this will be resolved sometime today. Issue is tracked in dotnet/runtime#1129 and has risen to the level of their overall CI build health (dotnet/runtime#702). @dagood is on point for dotnet/runtime#1129. |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
…110.1 - Microsoft.EntityFrameworkCore.Tools - 5.0.0-alpha.1.20060.1 - Microsoft.EntityFrameworkCore.SqlServer - 5.0.0-alpha.1.20060.1 - dotnet-ef - 5.0.0-alpha.1.20060.1 - Microsoft.EntityFrameworkCore - 5.0.0-alpha.1.20060.1 - Microsoft.EntityFrameworkCore.InMemory - 5.0.0-alpha.1.20060.1 - Microsoft.EntityFrameworkCore.Relational - 5.0.0-alpha.1.20060.1 - Microsoft.EntityFrameworkCore.Sqlite - 5.0.0-alpha.1.20060.1
…111.1 - Microsoft.EntityFrameworkCore.Tools - 5.0.0-alpha.1.20061.1 - Microsoft.EntityFrameworkCore.SqlServer - 5.0.0-alpha.1.20061.1 - dotnet-ef - 5.0.0-alpha.1.20061.1 - Microsoft.EntityFrameworkCore - 5.0.0-alpha.1.20061.1 - Microsoft.EntityFrameworkCore.InMemory - 5.0.0-alpha.1.20061.1 - Microsoft.EntityFrameworkCore.Relational - 5.0.0-alpha.1.20061.1 - Microsoft.EntityFrameworkCore.Sqlite - 5.0.0-alpha.1.20061.1
…112.1 - Microsoft.EntityFrameworkCore.Tools - 5.0.0-alpha.1.20062.1 - Microsoft.EntityFrameworkCore.SqlServer - 5.0.0-alpha.1.20062.1 - dotnet-ef - 5.0.0-alpha.1.20062.1 - Microsoft.EntityFrameworkCore - 5.0.0-alpha.1.20062.1 - Microsoft.EntityFrameworkCore.InMemory - 5.0.0-alpha.1.20062.1 - Microsoft.EntityFrameworkCore.Relational - 5.0.0-alpha.1.20062.1 - Microsoft.EntityFrameworkCore.Sqlite - 5.0.0-alpha.1.20062.1
From what I can tell now, there are still template tests failing from an EF error:
@wtgodbe @dougbu should I apply the suggested workaround that @halter73 mentioned about to pin 3.1 for EF? |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
Coherency updates are not tied to a subscription, so there could be coherency updates in a PR even if no dependencies changed that were affected by the subscription. Since we changed the coherency algorithm recently, this could happen. IMO CPD is a bit of a hack. It has multiple interpretations depending on what you decide you want in any given situation. The good news is that once efcore is removed from the graph and tooling is folded into aspnetcore, you will be able to get rid of of them altogether. |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
I think we can get rid of the extra references now, and do #17949. I'll try both in this PR. |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
I removed the extra refs, I'll try removing the |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
@mmitche would it be better to have the CPD for the runtime dependencies be on an AspNetCore-Tooling package here? That way the EFCore updates into this repo wouldn't mess with any runtime dependencies. We already use an Aspnetcore-tooling dependency as the CPD for Extensions dependencies in this repo |
Yeah probably. Isn't EFCore getting removed from the graph soon? |
Auto-Merge StatusThis pull request has been merged because the following merge policies have succeeded.
|
We've been intending to pin the EF dependencies, or just have them be only in versions.props & update them manually - we don't have a concrete plan yet for this, though. I'll put up an issue for it, the actual change will be minimal once we decide what we want to do. |
We shouldn't have any CPD attributes pointing to EF packages at all. Where do you see something like that @wtgodbe❔ |
The issue is not that we list a CPD attribute pointing to EF, but that Maestro could choose EF as the parent for a CPD attribute on Microsoft.NetCore.App, since EF also depends on that. I believe @mmitche was describing that that was what caused us to downgrade our corefx dependencies to 4.7.0 in this PR. If that's true, listing a CPD on anything that EF depends on, could cause a similar problem (hence #18669 depending on Tooling rather than Extensions) |
That doesn't match up with my understanding of how CPD works- changing the parent of our Runtime dependencies doesn't remove Extensions from the graph, just chains it. Extensions depends on Runtime, Tooling gets runtime from Extensions, AspNetCore gets runtime from tooling. The outcome is the same dependencies, but without the risk of getting something from EFCore. I believe that even CPD-ing something from the same repo as one of EFCore's CPDs could break us (@mmitche could you confirm/deny any of these assumptions?) |
It's possible there are additional CPD issues (given that we are tweaking it right now), but basically what it will do is this:
In the case of #18669, I think what will happen is that it will simply choose the runtime that aspnetcore-tooling has, which will be whatever extensions has, because that's what tooling ties its verisons to: https://github.com/dotnet/aspnetcore-tooling/blob/master/eng/Version.Details.xml#L48 Chaining is actually not strictly necessary. In fact, I'm not entirely sure it's ever actually required. What it does is that it ensures that the head (or maybe it's the tail, I can't remember) of a chain is updated first, then the next item in the chain is updated after, so that may be able to come up with different results in some cases. The upshot of all of this is that CPD is confusing and open to interpretation depending on what you want, and I think generally indicates a problem with repository boundaries more than anything else. |
This pull request updates the following dependencies
From https://github.com/dotnet/efcore
Coherency Updates
The following updates ensure that dependencies with a CoherentParentDependency
attribute were produced in a build used as input to the parent dependency's build.
See Dependency Description Format