-
Notifications
You must be signed in to change notification settings - Fork 136
Update to 3.1.109 #1770
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
Update to 3.1.109 #1770
Conversation
Fixes smoke-test.
The bootstrap online-smoke-test was failing because it wasn't able to find the runtime packs in the public feeds: public feeds aren't included in Fixes #1669. (Although we didn't have this exact scenario in mind at the time.) |
To get a Microsoft-built SDK to compare against, I went here via URL manipulation: I got this: https://gist.github.com/dagood/5b523381aa7200fdfe5fd7e9635853f9 I scanned through and saw that the Microsoft-built SDK doesn't have a few DLLs that source-build does have: --- /work/testing-smoke/builtCli
+++ /work/microsoft-built-dotnet-sdk-3.1.109.tar.gz
@@ -1781,17 +2487,13 @@
./shared/Microsoft.AspNetCore.App/3.1.9/Microsoft.Extensions.WebEncoders.dll
./shared/Microsoft.AspNetCore.App/3.1.9/Microsoft.JSInterop.dll
./shared/Microsoft.AspNetCore.App/3.1.9/Microsoft.Net.Http.Headers.dll
-./shared/Microsoft.AspNetCore.App/3.1.9/Microsoft.Win32.Registry.dll
./shared/Microsoft.AspNetCore.App/3.1.9/Microsoft.Win32.SystemEvents.dll
./shared/Microsoft.AspNetCore.App/3.1.9/System.Diagnostics.EventLog.dll
./shared/Microsoft.AspNetCore.App/3.1.9/System.Drawing.Common.dll
./shared/Microsoft.AspNetCore.App/3.1.9/System.IO.Pipelines.dll
-./shared/Microsoft.AspNetCore.App/3.1.9/System.Security.AccessControl.dll
-./shared/Microsoft.AspNetCore.App/3.1.9/System.Security.Cryptography.Cng.dll
./shared/Microsoft.AspNetCore.App/3.1.9/System.Security.Cryptography.Pkcs.dll
./shared/Microsoft.AspNetCore.App/3.1.9/System.Security.Cryptography.Xml.dll
./shared/Microsoft.AspNetCore.App/3.1.9/System.Security.Permissions.dll
-./shared/Microsoft.AspNetCore.App/3.1.9/System.Security.Principal.Windows.dll
./shared/Microsoft.AspNetCore.App/3.1.9/System.Windows.Extensions.dll This lines up with the names mentioned in dotnet/aspnetcore#17973. But that's a 3.1.0 bug, and I'm not sure yet why we would be affected by it now. |
I think that in theory, these DLLs should be excluded from the ASP.NET Core sharedfx because they're in the base sharedfx. Not sure yet why that happens in the Microsoft build but not source-build. |
I checked 3.1.108, and the extra DLLs are there, too. So, this is not a regression and I don't think it should block the build. I looked at a Microsoft build binlog for aspnet, and it looks like the only difference is that the On our side, we rebuild This is yet another way that the Microsoft build reuse of old nupkgs is making a difference in source-build. My current granularity plan/proposal could help with this by testing that the file versions are the same between the old, reused Microsoft-built nupkg and the fresh bits that source-build produces. We already assume and rely on the DLLs being compatible, so making the file version the same so it's treated as part of the platform seems reasonable to me. Filed #1779 |
Now seeing this in the final bootstrap build, in dotnet/sdk:
The reference being from:
I think this might be along the lines of this problem in dotnet/source-build-reference-packages#164 (comment), @dseefeld can you confirm?
The build didn't hit that problem before, ~4 days ago. I think this is because of the floating SBRP branch reference, and some changes, in particular dotnet/source-build-reference-packages#165 which merged ~3 days ago. That PR put back the change that had the problems listed above. @dseefeld, what was done to resolve those issues in 5.0? (PR description is blank and there are no backlinks.) If this isn't quickly reconcilable between the branches, I could make the 3.1 build use a pinned reference to the SBRP repo for now to keep using an older version. |
The issues that I was seeing with the updates to SBRP was as described in the arcade patch in #1777. This is a temporary update to Arcade until RC1, when Microsoft.Bcl.AsyncInterfaces will be re-added to the product. This won't work to fix the issue that you're seeing above, but I'm in favor of having a release/3.1 branch of SBRP just for servicing. I don't think there will be many changes on that branch and it will prevent against issues like this in the future. |
Ah, right. Sure, I'll go with |
I found a deeper Msft/source-build diff with the package that I think is more clearly linked with what I ran into, filed at #1781. But, no need to fix that at this second--if the re-revert alone fixes things, that's fine. |
#1768