-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[main] Update dependencies from dotnet/runtime #41793
[main] Update dependencies from dotnet/runtime #41793
Conversation
…0625.4 Microsoft.Extensions.DependencyModel , Microsoft.Extensions.FileSystemGlobbing , Microsoft.Extensions.Logging , Microsoft.Extensions.Logging.Abstractions , Microsoft.Extensions.Logging.Console , Microsoft.NET.HostModel , Microsoft.NET.ILLink.Tasks , Microsoft.NETCore.App.Host.win-x64 , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.Platforms , System.CodeDom , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.Security.Cryptography.ProtectedData , System.ServiceProcess.ServiceController , System.Text.Encoding.CodePages , VS.Redist.Common.NetCore.SharedFramework.x64.9.0 , VS.Redist.Common.NetCore.TargetingPack.x64.9.0 , Microsoft.SourceBuild.Intermediate.runtime.linux-x64 From Version 9.0.0-preview.6.24323.2 -> To Version 9.0.0-preview.7.24325.4
Notification for subscribed users from https://github.com/dotnet/runtime:@dotnet/dnr-codeflow Action requested: Please take a look at this failing automated dependency-flow pull request's checks; failures may be related to changes which originated in your repo.
|
@am11 source-build failed with
|
@akoeplinger dotnet/runtime#104012. Could you apply it as a patch here in the meanwhile, so we can see if it works? |
The CentOS8 non-portable build in runtime was passing https://dev.azure.com/dnceng-public/public/_build/results?buildId=720314&view=logs&jobId=fc3f098e-ab70-5b56-d12d-f5fa32e40725&j=fc3f098e-ab70-5b56-d12d-f5fa32e40725&t=de6336d7-76bb-5aba-e49e-d6aff7b10b1a bu CentOS9 non-portable build in SDK failed. cc @tmds maybe we should bring runtime CentOS8 leg to the same plan as CentOS9 in SDK to catch these issues early on. |
1 file changed, 1 insertion(+) | ||
|
||
diff --git a/eng/Subsets.props b/eng/Subsets.props | ||
index 68569344a6e..532928f67da 100644 |
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 this index is wrong, we need to create patch from the dotnet/dotnet to get the correct index: https://github.com/dotnet/source-build/blob/main/Documentation/patching-guidelines.md (I can't get the eng/vmr-sync.sh to succeed locally just remembered how @ViktorHofer does the patching 🙂 )
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.
Hmm I followed that doc and it doesn't say you need to do it from dotnet/dotnet...
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.
My understanding is this is about runtime project under dotnet/dotnet tree:
To create a repo patch file, first commit your changes to the source repo (e.g. runtime, aspnetcore) as normal,
but I could be wrong. Index mismatch is the only thing which I could make sense of. Maybe we need to checkout d0c8f836b5d46cd7c24fb7b7a8292b45d97f8e42
cd runtime
git checkout d0c8f836b5d46cd7c24fb7b7a8292b45d97f8e42
curl -sSL https://github.com/dotnet/runtime/pull/104012.patch | git apply -
git add eng && git commit -m "Also pass NETCoreSdkRuntimeIdentifier to runtime.pro"
git format-patch --zero-commit --no-signature -1
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.
Just tried it and it produces the same indices. So it's a mystery.
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.
Yes, you can create the patch from outside of dotnet/dotnet.
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.
akoeplinger reported this to me and it's a bug in the sync process which I am fixing now. There's a bug when adding a patch for currently changed lines of code. Should be fixed today.
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.
Thanks. I think we can revert this patch once dotnet/runtime#104012 is merged.
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.
Sorry, I was unable to merge the fix today. I will try to do it tomorrow.
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.
Ok, I have a PR with the fix: dotnet/arcade-services#3690
It turned out to be quite a nasty bug.
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.
Thanks, we have reverted this patch as it has flowed through the runtime. You fix will definitely help in future endeavors. 👌
I don't understand why there seems to be an issue with source-built NativeAOT as all the PRs have already made it in the vmr. Anyway, whether it makes sense to add a job in runtime that would catch this earlier depends on how much issues it would catch and how much time that saves vs cost for dealing with the runs (false positives, extra time, etc). I don't have an informed opinion. |
@tmds, this error is only showing up in SDK's centos leg but passing in runtime's centos leg. I was referring to that difference. Before we weren't passing properties to runtime.proj, so it was accidentally working. Now these properties are explicitly being passed down to the project. |
👍 Sounds like a good idea to update the runtime leg so it would have caught the issue. |
…0626.3 Microsoft.Extensions.DependencyModel , Microsoft.Extensions.FileSystemGlobbing , Microsoft.Extensions.Logging , Microsoft.Extensions.Logging.Abstractions , Microsoft.Extensions.Logging.Console , Microsoft.NET.HostModel , Microsoft.NET.ILLink.Tasks , Microsoft.NETCore.App.Host.win-x64 , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.Platforms , System.CodeDom , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.Security.Cryptography.ProtectedData , System.ServiceProcess.ServiceController , System.Text.Encoding.CodePages , VS.Redist.Common.NetCore.SharedFramework.x64.9.0 , VS.Redist.Common.NetCore.TargetingPack.x64.9.0 , Microsoft.SourceBuild.Intermediate.runtime.linux-x64 From Version 9.0.0-preview.7.24324.19 -> To Version 9.0.0-preview.7.24326.3
…meIdentifier-to-runtime.pro.patch
…0627.2 Microsoft.Extensions.DependencyModel , Microsoft.Extensions.FileSystemGlobbing , Microsoft.Extensions.Logging , Microsoft.Extensions.Logging.Abstractions , Microsoft.Extensions.Logging.Console , Microsoft.NET.HostModel , Microsoft.NET.ILLink.Tasks , Microsoft.NETCore.App.Host.win-x64 , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.Platforms , System.CodeDom , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.Security.Cryptography.ProtectedData , System.ServiceProcess.ServiceController , System.Text.Encoding.CodePages , VS.Redist.Common.NetCore.SharedFramework.x64.9.0 , VS.Redist.Common.NetCore.TargetingPack.x64.9.0 , Microsoft.SourceBuild.Intermediate.runtime.linux-x64 From Version 9.0.0-preview.7.24324.19 -> To Version 9.0.0-preview.7.24327.2
…0627.4 Microsoft.Extensions.DependencyModel , Microsoft.Extensions.FileSystemGlobbing , Microsoft.Extensions.Logging , Microsoft.Extensions.Logging.Abstractions , Microsoft.Extensions.Logging.Console , Microsoft.NET.HostModel , Microsoft.NET.ILLink.Tasks , Microsoft.NETCore.App.Host.win-x64 , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.Platforms , System.CodeDom , System.Reflection.MetadataLoadContext , System.Resources.Extensions , System.Security.Cryptography.ProtectedData , System.ServiceProcess.ServiceController , System.Text.Encoding.CodePages , VS.Redist.Common.NetCore.SharedFramework.x64.9.0 , VS.Redist.Common.NetCore.TargetingPack.x64.9.0 , Microsoft.SourceBuild.Intermediate.runtime.linux-x64 From Version 9.0.0-preview.7.24324.19 -> To Version 9.0.0-preview.7.24327.4
This pull request updates the following dependencies
From https://github.com/dotnet/runtime