Skip to content
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

[release/6.0] Fix gcc warnings during mono linux-x64 build (backport of #60675) #77505

Merged

Conversation

ayakael
Copy link
Contributor

@ayakael ayakael commented Oct 26, 2022

Backport of #60675 to release/6.0

Per @am11

main with Debug configuration: 822 warnings - http://sprunge.us/2GzrDE
PR with Debug configuration: 3 warnings related to deprecated sys/sysctl.h includes - http://sprunge.us/JuyA3K

after fixing Debug warnings, there were 13 additional warnings in Release configuration:
http://sprunge.us/PJCivP

PR with Release configuration: (same) 3 warnings - http://sprunge.us/NwKHNE

Used:

same docker image used in CI for coreclr gcc build: mcr.microsoft.com/dotnet-buildtools/prereqs:debian-11-gcc11-amd64-20210916125744-f42d766
build command: runtime/build.sh mono -gcc -c Debug (and later with -c Release)

Customer impact

Mono flavored-runtime will be less prone to errors

Testing

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

Risk

Low, as it is already in main, and only activates when cmake for linux test passes.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Oct 26, 2022
@teo-tsirpanis teo-tsirpanis added this to the 6.0.x milestone Oct 26, 2022
@ayakael
Copy link
Contributor Author

ayakael commented Nov 2, 2022

@am11 Is this something that looks good for you as far as a direct backport, or is there something missing on my implementation?

@vargaz
Copy link
Contributor

vargaz commented Nov 2, 2022

Why do these warning fixes need to be backported ?

@ayakael
Copy link
Contributor Author

ayakael commented Nov 2, 2022

Without this, build of dotnet6 on s390x has issues. This was first backported to Fedora's package, which Alpine Linux has then applied to our package in getting that up to speed. @omajid can fill in on the details.

@omajid
Copy link
Member

omajid commented Nov 2, 2022

The commit doesn't spell it out, but this also fixes build errors when building .NET with clang 15. We needed this to be able to build .NET 6 on Fedora 37.

@ayakael ayakael force-pushed the backports/pr-60675-to-release/6.0 branch from c22b0d0 to 6ad6fd0 Compare November 2, 2022 15:42
@am11
Copy link
Member

am11 commented Nov 2, 2022

This a partial backport of the linked PR. Looks good. Thanks!

The commit doesn't spell it out, but this also fixes build errors when building .NET with clang 15.

Understandable; 577a70a is a one year old commit, while clang-15 was released two months ago.

@carlossanlop
Copy link
Member

@vargaz @lambdageek @marek-safar 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.

@carlossanlop
Copy link
Member

Ping @lambdageek @marek-safar @vargaz. Also, the CI seems to be stuck, so it needs a restart. I'm closing and reopening.

@carlossanlop carlossanlop reopened this Nov 7, 2022
@lambdageek lambdageek added the Servicing-consider Issue for next servicing release review label Nov 8, 2022
@carlossanlop
Copy link
Member

@vargaz @lambdageek @marek-safar this has not been approved by Tactics yet. Can you please send the approval email? The deadline is the 14th.

@lambdageek
Copy link
Member

@vargaz @marek-safar @carlossanlop I emailed tactics

@vargaz vargaz added the Servicing-approved Approved for servicing release label Nov 11, 2022
@vargaz
Copy link
Contributor

vargaz commented Nov 11, 2022

Approved by tactics.

@marek-safar marek-safar removed the Servicing-consider Issue for next servicing release review label Nov 11, 2022
@carlossanlop
Copy link
Member

Signed off by area owner.
Approved by Tactics.
CI green.
No OOB package authoring changes needed for this.
Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit ce7aaec into dotnet:release/6.0 Nov 11, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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
Development

Successfully merging this pull request may close these issues.

8 participants