-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Update CoreClr, CoreFx, ProjectNTfs, ProjectNTfsTestILC, Standard to preview2-25227-01, preview2-25227-01, beta-25227-00, beta-25227-00, preview2-25227-01, respectively (release/2.0.0) #19024
Update CoreClr, CoreFx, ProjectNTfs, ProjectNTfsTestILC, Standard to preview2-25227-01, preview2-25227-01, beta-25227-00, beta-25227-00, preview2-25227-01, respectively (release/2.0.0) #19024
Conversation
b39cac2 to
84c0611
Compare
|
@brianrob, @weshaggard, this error is back here, too: |
10afed2 to
1e9ce26
Compare
dependencies.props
Outdated
| <ExternalCurrentRef>3b8a99621d89ad9877f053ba8af25e0f25dcd9d8</ExternalCurrentRef> | ||
| <ProjectNTfsCurrentRef>ff616742d18be3af7e9da9327d51502b79de859d</ProjectNTfsCurrentRef> | ||
| <ProjectNTfsTestILCCurrentRef>ff616742d18be3af7e9da9327d51502b79de859d</ProjectNTfsTestILCCurrentRef> | ||
| <ProjectNTfsCurrentRef>a84d35ed9b6b6da8749cf90a2c107f4adfc73a2b</ProjectNTfsCurrentRef> |
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.
@dagood any ideas why this is updating the ProjectN refs when they aren't part of the subscription triggers https://github.com/dotnet/versions/blob/master/Maestro/subscriptions.json#L282?
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.
Do we need to update the DependencyBranch property below in the release branch?
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.
The subscription triggers only trigger the update--they aren't passed into the auto-upgrade build at all. (All updates need to happen during any trigger so that the commit that's generated is fresh from GitHub's HEAD yet contains all updates.) To disable updating you need to get rid of the RemoteDependencyBuildInfo.
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 think updating DependencyBranch without removing ProjectN's RemoteDependencyBuildInfo would actually cause build errors, since ProjectN doesn't have a release/2.0.0 build-info.
EDIT: But in general, yes, DependencyBranch should be updated so CoreFX gets updates from CoreCLR/Standard/CoreFX release/2.0.0 rather than master.
…preview2-25227-01, preview2-25227-01, beta-25227-00, beta-25227-00, preview2-25227-01, respectively
1e9ce26 to
a3771c9
Compare
| <!-- | ||
| <XmlUpdateStep Include="External"> | ||
| <Path>$(MSBuildThisFileFullPath)</Path> | ||
| <ElementName>ExternalExpectedPrerelease</ElementName> |
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.
Good catch that these need to be commented out. I think they could be conditioned on whether the RemoteDependencyBuildInfo exists, but that seems more complicated than it's worth.
|
@weshaggard, I originally suspected that this was because I removed EventCounter.cs from System.Private.CoreLib and it took time to get sunk to the the UAP builds. Is this not the case? |
|
@brianrob that is what I thought as well but this is the release/2.0.0 branch which shouldn't be getting updates for those packages right now. So the fix is to stop those from updating. You might hit that issue in master though if you have changed it recently. |
|
Got it. I missed that this was into release. |
No description provided.