-
Notifications
You must be signed in to change notification settings - Fork 132
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
macOS PR validation smoke-test failure: libicu not installed/detected in smoke-test #1685
Comments
This may not be flakiness... happened 3 times in a row on https://dev.azure.com/dnceng/public/_build/results?buildId=742764&view=results. Pool configuration change maybe? FYI @dseefeld @crummel |
@dotnet/dnceng Is this a known issue or configuration issue for OSX images? |
Taking a peek |
Couple things:
I need to talk to folks in AzDO about their timeline of moving to the "new" macs; this was supposed to be completed last week but is still ongoing. This is likely your problem as I see the same PR building on different days hitting the problem and not. |
I chatted with @safern and @tarekgh who may want to weigh in here. There has been code change in this area that you may be missing. Please see dotnet/runtime#39900, specifically this commit. |
Just to add to what @MattGal mentioned, the extended version of the fix in order to support building runtime in macOS Big Sur is: dotnet/runtime#39900 |
@safern will that be ported to 3.1 where we hit this error? Is there a tracking issue? |
I believe @ViktorHofer was porting that to downstream branches. |
A few more bits to connect the dots between this and dotnet/runtime#39900: it looks like originally this problem came up in https://github.com/dotnet/core-eng/issues/10304, which shows up exactly the same way it does for us. The initial fix was dotnet/runtime#39833:
Then dotnet/runtime#39900 looks like an improved fix in line with new macOS changes. |
@ViktorHofer ported it to coreclr3.1 which is the one that builds S.Globalization.Native in 3.1. dotnet/coreclr@d3a7344 |
Note that only the simple fix was ported. If we need to build on Big Sur for 3.1 we can also port the complete fix. |
The fix was backported into the 2.1, 2.2 and 3.1 branches. Here's the change for 3.1: dotnet/coreclr@d3a7344. |
Ah, expected some backlink to show up for that at least. 😄 For future reference: Thanks, we should uptake those changes eventually. When push comes to shove, source-build doesn't currently have a critical reason to build on macOS at all, so no worries about building on Big Sur as far as we're concerned. |
Responding to @asbjornu from dotnet/sdk#11795 (comment):
Not in the "correct fix" way, because there isn't a build of .NET Core yet that incorporates that fix. Any workaround we add now would have to be reverted once we get a build. (It's not feasible to rush a build.) You could lower your CMake version to emulate how we managed to build the last release on macOS before the CMake breaking change (from our project's PoV). Or you should be able to fairly easily incorporate the fix (dotnet/coreclr#28066) as a patch file in your own process. This might end up being a patch file (https://github.com/Homebrew/formula-patches) that in turn adds a patch file into source-build. (Straight up patch files would work for the source-build tarballs we produce, but IIRC you're using the the GitHub source archives, which don't include the CoreCLR source initially.) |
Thanks, @dagood. While I can indeed provide a patch in the formula, I feel patching the source of code that is not within the main repository is going to be difficult. What's the timeframe on a new release incorporating dotnet/coreclr#28066? |
One trick that I have used is to copy my own patch files to the |
I'd expect it at the earliest between 2-3 weeks from now. |
Ok, then I'll wait. |
I believe this is fixed now. The 3.1.108 release PR didn't hit this--its macOS job was green: #1732. |
Indeed, the build successfully completes locally now! With that, I'll wrap up the Homebrew formula and submit a PR as soon as possible. Thanks! |
@dagood, just a little note on the following:
I hope this view changes as soon as Homebrew/homebrew-core#60929 is merged. 😅🙏 |
Yeah, I think that being able to point at that high profile usage probably would have justified making our own patch to fix up our upstream (rather than waiting for its next release). |
https://dev.azure.com/dnceng/public/_build/results?buildId=742764&view=logs&j=dd0961f5-eb75-5263-969b-8fed4b0393f9&t=e3c3f94f-154c-5989-8736-91280ba7f044&l=75
This seems to be flakiness: the PR that hit this didn't change anything about this job, and the "Initialize job" step appears identical to an earlier PR validation run that failed. Time will tell.
The text was updated successfully, but these errors were encountered: