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

Skip sys/sysctl.h on linux in mono #61110

Merged
merged 1 commit into from
Nov 3, 2021

Conversation

am11
Copy link
Member

@am11 am11 commented Nov 2, 2021

Fixes three warnings:

  [ 56%] Building C object mono/mini/CMakeFiles/monosgen-objects.dir/__/utils/mono-mmap.c.o
  In file included from /runtime/src/mono/mono/utils/mono-mmap.c:23:
  /usr/include/x86_64-linux-gnu/sys/sysctl.h:21:2: warning: #warning "The <sys/sysctl.h> header is deprecated and will be removed." [-Wcpp]
     21 | #warning "The <sys/sysctl.h> header is deprecated and will be removed."
        |  ^~~~~~~
# snip
  [ 59%] Building C object mono/mini/CMakeFiles/monosgen-objects.dir/__/utils/mono-time.c.o
  In file included from /runtime/src/mono/mono/utils/mono-proclib.c:42:
  /usr/include/x86_64-linux-gnu/sys/sysctl.h:21:2: warning: #warning "The <sys/sysctl.h> header is deprecated and will be removed." [-Wcpp]
     21 | #warning "The <sys/sysctl.h> header is deprecated and will be removed."
        |  ^~~~~~~
  [ 59%] Building C object mono/mini/CMakeFiles/monosgen-objects.dir/__/utils/mono-uri.c.o
  [ 59%] Building C object mono/mini/CMakeFiles/monosgen-objects.dir/__/utils/mono-poll.c.o
  In file included from /runtime/src/mono/mono/utils/mono-time.c:107:
  /usr/include/x86_64-linux-gnu/sys/sysctl.h:21:2: warning: #warning "The <sys/sysctl.h> header is deprecated and will be removed." [-Wcpp]
     21 | #warning "The <sys/sysctl.h> header is deprecated and will be removed."
        |  ^~~~~~~

With this PR: 0 warnings - http://sprunge.us/96kyq9

Like in src/libraries/Native, skip linux from sys/sysctl.h check. For other systems, check_include_files (instead of check_include_file (singular)) when a header depends on another one.

Also, simplified few conditions: "is linux" check supersedes "is android".

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Nov 2, 2021
@am11
Copy link
Member Author

am11 commented Nov 2, 2021

cc @akoeplinger

@Thefrank
Copy link
Contributor

Thefrank commented Nov 3, 2021

FYI: FreeBSD doesn't have a mono section in CI yet as #57487 is still pending.

I don't foresee the change breaking FreeBSD

@am11
Copy link
Member Author

am11 commented Nov 3, 2021

The introspection check for sysctl.h is ported from libs.native subset, so it is as good / safe. Harmonizing configs generally reduces the risk of breakage. :)

@am11
Copy link
Member Author

am11 commented Nov 3, 2021

It would be nice to replace ac_check_headers function with explicit check_include_files() calls (one line per header), like we have in coreclr, libs, corehost and tests. Some of the headers are unused, and some have dependencies (like sys/sysctl.h requires sys/types.h).
@akoeplinger, @vargaz, @lambdageek, wdyt? I can give it a go as part of this PR.

@akoeplinger akoeplinger merged commit 6aaee16 into dotnet:main Nov 3, 2021
@akoeplinger
Copy link
Member

I can give it a go as part of this PR.

I prefer to do it in a separate PR. We could also add parameter(s) for the dependent headers to ac_check_headers to keep the shared HAVE_* logic, but I don't feel strongly about it.

@am11 am11 deleted the feature/native/gcc/mono branch November 3, 2021 11:09
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2021
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants