-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Fix official builds #104070
Fix official builds #104070
Conversation
This reverts commit 4e1ad9c.
@MichalStrehovsky i don't think non-portable build should block us in the first place. Just disable it for non-portable build as before rather than the revert? |
You mean the source build issue in the SDK repo (dotnet/sdk#41793)? That one is separate. We're having issues getting official builds of the dotnet/runtime repo out because there's an apparent race condition. |
In the original PR, I didn't touched cdacreader until I was asked. I think reverting src/native/managed/compile-native.proj should be enough for both problems. |
If you can make a PR, I can approve and merge and we'll see. (If I make a PR, we need to find someone else who can approve before we can merge and it's late night in the US). |
Closing in favor of #104072. I expect that one will sweep the MUSL issue back under the rug for someone else to discover again later (we need to fix the official build). |
@am11 I have to revert these again, sorry. Out of 5 last official build attempts, 4 failed in the native AOT MUSL x64 or arm64 legs with the
error that we've seen before. So it looks like it's still there, and still non-deterministic.
The command used is:
Or
I was digging in the binlogs (I don't think I can share them externally because it's from the official build machine and I don't know if there are secrets) and the problem seems to be:
Bad:
Good:
The cdacreader project is hooked up in a bit of an odd way so maybe it doesn't restore the host ILCompiler runtime pack and we just have a happy accident that something else does restore it before sometimes.
Looks like cdacreader was excluded from building on MUSL before this change and now we've centralized it and it's no longer excluded and we're running into this? Cc @lambdageek @jkoritzinsky