Skip to content

Comments

libglvnd: fix build with newer clang#234117

Merged
wegank merged 1 commit intoNixOS:stagingfrom
reckenrode:libglvnd-darwin-sandbox
May 26, 2023
Merged

libglvnd: fix build with newer clang#234117
wegank merged 1 commit intoNixOS:stagingfrom
reckenrode:libglvnd-darwin-sandbox

Conversation

@reckenrode
Copy link
Contributor

Description of changes

This was discovered while working on #229786. This change silences new warnings regarding integer to pointer conversions. Targeting staging due to the number of rebuilds.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot requested a review from primeos May 26, 2023 03:24
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 1001-2500 This PR causes many rebuilds on Darwin and should most likely target the staging branches. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels May 26, 2023
@wegank wegank merged commit 41b60d7 into NixOS:staging May 26, 2023
@reckenrode reckenrode deleted the libglvnd-darwin-sandbox branch May 26, 2023 14:57
@primeos
Copy link
Member

primeos commented May 27, 2023

Thanks. One note and a question:

This was discovered while working on #229786. This change silences new warnings regarding integer to pointer conversions. Targeting staging due to the number of rebuilds.

Would've been lovely to have that information in the Git commit message as well (so that it's properly documented in the Git history).

Is the Git commit message subject correct? Does this really fix the build or does it only silence some noisy warnings (as I'd expect from the diff)?

@reckenrode
Copy link
Contributor Author

Would've been lovely to have that information in the Git commit message as well (so that it's properly documented in the Git history).

I’ll keep that in mind for the next round of PRs fixing other breakage, but shouldn’t the commit be linked back to the PR by GitHub?

Is the Git commit message subject correct? Does this really fix the build or does it only silence some noisy warnings (as I'd expect from the diff)?

It fails with the following errors, which the additional flag suppresses.

libegl.c:257:38: error: incompatible integer to pointer conversion passing 'EGLNativeDisplayType' (aka 'int') to parameter of type 'void *' [-Wint-conversion]
    if (gbmSupported && IsGbmDisplay(display_id)) {
                                     ^~~~~~~~~~
libegl.c:161:38: note: passing argument to parameter 'native_display' here
static EGLBoolean IsGbmDisplay(void *native_display)
                                     ^
libegl.c:260:46: error: incompatible integer to pointer conversion passing 'EGLNativeDisplayType' (aka 'int') to parameter of type 'void *' [-Wint-conversion]
    if (waylandSupported && IsWaylandDisplay(display_id)) {
                                             ^~~~~~~~~~
libegl.c:201:42: note: passing argument to parameter 'native_display' here
static EGLBoolean IsWaylandDisplay(void *native_display)
                                         ^
libegl.c:263:38: error: incompatible integer to pointer conversion passing 'EGLNativeDisplayType' (aka 'int') to parameter of type 'void *' [-Wint-conversion]
    if (x11Supported && IsX11Display(display_id)) {
                                     ^~~~~~~~~~
libegl.c:177:38: note: passing argument to parameter 'dpy' here
static EGLBoolean IsX11Display(void *dpy)
                                     ^
libegl.c:389:55: error: incompatible integer to pointer conversion passing 'EGLNativeDisplayType' (aka 'int') to parameter of type 'void *' [-Wint-conversion]
            return GetPlatformDisplayCommon(platform, display_id, NULL, "eglGetDisplay");
                                                      ^~~~~~~~~~
libegl.c:271:15: note: passing argument to parameter 'native_display' here
        void *native_display, const EGLAttrib *attrib_list,
              ^
libegl.c:396:51: error: incompatible integer to pointer conversion passing 'EGLNativeDisplayType' (aka 'int') to parameter of type 'void *' [-Wint-conversion]
        return GetPlatformDisplayCommon(EGL_NONE, display_id, NULL, "eglGetDisplay");
                                                  ^~~~~~~~~~
libegl.c:271:15: note: passing argument to parameter 'native_display' here
        void *native_display, const EGLAttrib *attrib_list,
              ^
libegl.c:405:47: error: incompatible integer to pointer conversion passing 'EGLNativeDisplayType' (aka 'int') to parameter of type 'void *' [-Wint-conversion]
    return GetPlatformDisplayCommon(platform, display_id, NULL, "eglGetDisplay");
                                              ^~~~~~~~~~
libegl.c:271:15: note: passing argument to parameter 'native_display' here
        void *native_display, const EGLAttrib *attrib_list,
              ^

@primeos
Copy link
Member

primeos commented May 31, 2023

I’ll keep that in mind for the next round of PRs fixing other breakage, but shouldn’t the commit be linked back to the PR by GitHub?

Yes, but that link isn't part of the Git repository (information that could get lost, etc.) and then one has to invest extra time for looking everything up via GitHub - which is very inconvenient for Git power users.

It fails with the following errors, which the additional flag suppresses.

Thanks :)
Strange that the -Wno-error flag doesn't seem to apply here...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 1001-2500 This PR causes many rebuilds on Darwin and should most likely target the staging branches. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants