Skip to content

[release/6.0] Fix KeepNativeSymbols to work on mono as well (backport of #66594)#77506

Merged
carlossanlop merged 1 commit intodotnet:release/6.0from
ayakael:backport/pr-66594-to-release/6.0
Nov 10, 2022
Merged

[release/6.0] Fix KeepNativeSymbols to work on mono as well (backport of #66594)#77506
carlossanlop merged 1 commit intodotnet:release/6.0from
ayakael:backport/pr-66594-to-release/6.0

Conversation

@ayakael
Copy link
Contributor

@ayakael ayakael commented Oct 26, 2022 •
edited
Loading

Backport of #66594

Per @omajid:

When packaging .NET on s390x through source-build, we want to keep debug
symbols enabled in the build. The package manager (debbuild, rpmbuild,
etc) generally strips the binaries to create distro-standard
-debug/-debuginfo packages. This was supported in coreclr via
--keepnativesymbols flag, but wasn't supported in mono (ie, s390x). Fix
that.

Customer impact

Source-build with mono-flavored runtime is problematic without this patch.

Testing

  • unit tests in ci
  • source-build build within Alpine environment passes

Risk

Low, as it is already in main, and only activates when KeepNativeSymbols!=true

@ayakael ayakael requested a review from marek-safar as a code owner October 26, 2022 19:26
@ghost ghost added area-Build-mono community-contribution Indicates that the PR has been added by a community member labels Oct 26, 2022
@ayakael ayakael changed the title Fix KeepNativeSymbols to work on mono as well (backport of #66594) [release/6.0] Fix KeepNativeSymbols to work on mono as well (backport of #66594) Oct 26, 2022
@ayakael ayakael mentioned this pull request Oct 26, 2022
Closed
18 tasks
@teo-tsirpanis teo-tsirpanis added this to the 6.0.x milestone Oct 26, 2022
@build-analysis build-analysis bot mentioned this pull request Oct 26, 2022
Closed
@marek-safar marek-safar requested a review from directhex October 27, 2022 10:23
@ayakael ayakael force-pushed the backport/pr-66594-to-release/6.0 branch from fab82ba to bc3084a Compare November 2, 2022 15:42
@carlossanlop
Copy link
Contributor

carlossanlop commented Nov 3, 2022

@marek-safar @directhex this needs to go through Tactics. When/if this is ready, please add the servicing-consider label and send an email to Tactics requesting merge approval.

@marek-safar marek-safar added the Servicing-consider Issue for next servicing release review label Nov 3, 2022
@rbhanda rbhanda modified the milestones: 6.0.x, 6.0.12 Nov 3, 2022
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Nov 3, 2022
@carlossanlop
Copy link
Contributor

carlossanlop commented Nov 3, 2022

@omajid @ayakael @directhex I see some CI failures that I haven't seen in any other backport PR. Are they related to this change?

@directhex can you please provide a code review sign-off?

@ayakael ayakael force-pushed the backport/pr-66594-to-release/6.0 branch from bc3084a to 3cec0ed Compare November 3, 2022 21:00
@build-analysis build-analysis bot mentioned this pull request Nov 4, 2022
Closed
2 tasks
@ayakael
Copy link
Contributor Author

ayakael commented Nov 4, 2022 •
edited
Loading

@omajid @ayakael @directhex I see some CI failures that I haven't seen in any other backport PR. Are they related to this change?

@directhex can you please provide a code review sign-off?

It really shouldn't be, as KeepNativeSymbols is really only used by source-build from memory, and the failures seem related to CoreCLR which this PR doesn't touch. Pushing a new rebase as to trigger another round of CI tests. Maybe it was just a fluke.

@ayakael ayakael force-pushed the backport/pr-66594-to-release/6.0 branch from 3cec0ed to 3b8eadb Compare November 4, 2022 20:21
@carlossanlop
Copy link
Contributor

carlossanlop commented Nov 7, 2022

Ping @directhex @marek-safar for a code review-sign off.

@carlossanlop carlossanlop mentioned this pull request Nov 10, 2022
Closed
@carlossanlop
Copy link
Contributor

carlossanlop commented Nov 10, 2022

CI failure confirmed offline to be unrelated. Signed off by area owners. Approved by Tactics. No OOB package authoring changes needed. Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit 86caa85 into dotnet:release/6.0 Nov 10, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@marek-safar marek-safar marek-safar approved these changes

+1 more reviewer

@directhex directhex directhex approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@marek-safar marek-safar

@directhex directhex

Labels

area-Build-mono community-contribution Indicates that the PR has been added by a community member Servicing-approved Approved for servicing release

Projects

None yet

Milestone

6.0.12

Development

Successfully merging this pull request may close these issues.

6 participants

@ayakael @carlossanlop @marek-safar @directhex @teo-tsirpanis @rbhanda