coreclr icon indicating copy to clipboard operation
coreclr copied to clipboard

[release/3.1] backports dotnet#67763: fix build with musl 1.2.3

Open ayakael opened this issue 2 years ago • 10 comments

Backports https://github.com/dotnet/runtime/pull/67772 to release/3.1 to fix build with musl 1.2.3

ayakael avatar Apr 25 '22 16:04 ayakael

@nekopsykose I see that you backported the musl 123 fix on Alpine's side. (thanks for that!) You removed whatever modifications in src/pal/src/init/pal.cpp. Why was that? Could this be the source of the failing tests on release/3.1 and release/5.0?

ayakael avatar Apr 25 '22 17:04 ayakael

afaik those don't change any semantics (NULL == nullptr), it just failed to apply cleanly and i was lazy for a mere backport :)

nekopsykose avatar Apr 25 '22 17:04 nekopsykose

afaik those don't change any semantics (NULL == nullptr), it just failed to apply cleanly and i was lazy for a mere backport :)

Gotcha, then for the upgrade to 3.1.418 I'll apply proper change - it's what upstream wants.

ayakael avatar Apr 25 '22 17:04 ayakael

Confirmed good build in Alpine pipelines

ayakael avatar Apr 25 '22 20:04 ayakael

@ayakael is this ready to merge? Can you get a sign-off from someone in your team?

carlossanlop avatar May 03 '22 22:05 carlossanlop

Ah, it's missing the servicing consider label. I added it. This needs to go through tactics.

carlossanlop avatar May 03 '22 23:05 carlossanlop

@janvorli can you do a code review? Same as https://github.com/dotnet/runtime/pull/67772

jeffschwMSFT avatar May 12 '22 17:05 jeffschwMSFT

@ayakael, could you update the PR body with a template we use to get servicing approval? Once I get it from you, I will get an approval. Please refer an example from https://github.com/dotnet/runtime/pull/68934 which includes:

  • What is the issue
  • What was the root cuase
  • What was the fix
  • Customer Impact
  • Testing
  • Risk (if it is low, why is it low?)

JulieLeeMSFT avatar May 12 '22 22:05 JulieLeeMSFT

Removing servicing consider until I hear back from @ayakael and get code review from @janvorli. cc @mangod9.

JulieLeeMSFT avatar May 13 '22 18:05 JulieLeeMSFT

@ayakael, I have a question to you. You put up servicing PRs for only .NET 3.1 and .NET 5. I don't see .NET 6 patch. What is your plan for .NET 6 patch?

When fixing issues, we need to fix all the supported releases between the lowest needed (in this case .NET 3.1) and the currently supported release (.NET 6). Since .NET 5 is going out of support, we will need to patch both .NET 3.1 and .NET 6 if you need to patch .NET 3.1.

JulieLeeMSFT avatar May 13 '22 20:05 JulieLeeMSFT

@ayakael the PR has been inactive for a few months. I'll close it. Feel free to reopen if you can answer the questions above.

carlossanlop avatar Sep 07 '22 20:09 carlossanlop