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

wayland: fix build on darwin #122475

Merged
merged 1 commit into from
May 12, 2021
Merged

Conversation

stephank
Copy link
Contributor

@stephank stephank commented May 10, 2021

Motivation for this change

ZHF: #122042
cc @NixOS/nixos-release-managers

Hydra failure: https://hydra.nixos.org/build/141980180/nixlog/2

There appears to be no direct job in Hydra for wayland on Darwin, and I'm not sure how to fix that. Regardless, this will unblock the build of retroarch, which is several dozen jobs. 🙃

Alternatively, retroarch shouldn't depend on wayland on Darwin, and I'll open a separate PR to make that change.

Upstream merge request: https://gitlab.freedesktop.org/wayland/wayland/-/merge_requests/136

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@Synthetica9
Copy link
Member

Synthetica9 commented May 10, 2021

That's a lotta rebuilds on non-darwin! Maybe instead of checking the platform in the patch itself, use stdenv.isDarwin to only include the patch on darwin?

@r-rmcgibbo
Copy link

r-rmcgibbo commented May 10, 2021

Result of nixpkgs-review pr 122475 at 5266288b run on aarch64-linux 1

612 packages marked as broken and skipped:
  • adoptopenjdk-hotspot-bin-13
  • adoptopenjdk-hotspot-bin-14
  • adoptopenjdk-jre-hotspot-bin-13
  • adoptopenjdk-jre-hotspot-bin-14
  • agdaPackages.iowa-stdlib
  • aldor
  • aqemu
  • asymptote
  • avxsynth
  • belle-sip
  • ...
3642 packages skipped due to time constraints:
  • AusweisApp2
  • DisnixWebService
  • MIDIVisualizer
  • R
  • SDL2_gfx
  • SDL2_net
  • SDL_gpu
  • _1oom
  • _20kly
  • _90secondportraits
  • ...
56 packages built successfully:
  • SDL2
  • SDL2_image
  • SDL2_mixer
  • SDL2_ttf
  • adoptopenjdk-bin (adoptopenjdk-hotspot-bin-11 ,openjdk11-bootstrap)
  • openjdk16-bootstrap (adoptopenjdk-hotspot-bin-15)
  • openjdk8-bootstrap (adoptopenjdk-hotspot-bin-8)
  • appstream-glib
  • cdogs-sdl
  • clickclack
  • czkawka
  • devilutionx
  • geoclue2
  • glib-networking (gnome.glib-networking ,gnome.glib_networking)
  • gnome.gnome-desktop (gnome.gnome_desktop)
  • gnome.gnome-session-ctl
  • gtk3 (gnome.gtk ,gtk3-x11)
  • gssdp
  • gst_all_1.gst-plugins-base
  • gst_all_1.gst-plugins-ugly
  • gtkspell3
  • gupnp
  • gupnp-igd
  • kodelife
  • libcanberra-gtk3
  • libdbusmenu-gtk3
  • libdmapsharing
  • libnice
  • libproxy
  • libva
  • libva-minimal
  • libwnck3
  • mpdevil
  • myxer
  • networkmanager
  • nice-dcv-client
  • nxengine-evo
  • openconnect (openconnect_gnutls)
  • pantheon.elementary-icon-theme
  • pantheon.granite
  • prs
  • python38Packages.gst-python
  • qrencode
  • rofi
  • sdlpop
  • smpeg2
  • stoken
  • tela-icon-theme
  • usbview
  • vulkan-loader
  • wayland-protocols
  • wayland-scanner
  • wdomirror
  • wl-clipboard
  • xfce.xfce4-dev-tools (xfce.xfce4_dev_tools)
  • yapesdl
2 suggestions:
  • warning: missing-patch-comment

    Consider adding a comment explaining the purpose of this patch on the line preceeding.
    Near pkgs/development/libraries/wayland/default.nix:40:5:

       |
    40 |     (substituteAll {
       |     ^
    
  • warning: missing-patch-comment

    Consider adding a comment explaining the purpose of this patch on the line preceeding.
    Near pkgs/development/libraries/wayland/default.nix:44:5:

       |
    44 |     ./0002-fix-darwin-build.patch
       |     ^
    

Result of nixpkgs-review pr 122475 at 5266288b run on x86_64-linux 1

576 packages marked as broken and skipped:
  • adoptopenjdk-hotspot-bin-13
  • adoptopenjdk-hotspot-bin-14
  • adoptopenjdk-jre-hotspot-bin-13
  • adoptopenjdk-jre-hotspot-bin-14
  • adoptopenjdk-jre-openj9-bin-13
  • adoptopenjdk-jre-openj9-bin-14
  • adoptopenjdk-openj9-bin-13
  • adoptopenjdk-openj9-bin-14
  • agdaPackages.iowa-stdlib
  • aldor
  • ...
1 package failed to build:
4417 packages skipped due to time constraints:
  • AusweisApp2
  • DisnixWebService
  • MIDIVisualizer
  • OSCAR
  • R
  • SDL2_gfx
  • SDL2_net
  • SDL_gpu
  • Sylk
  • _1oom
  • ...
64 packages built successfully:
  • SDL2
  • SDL2_image
  • SDL2_mixer
  • SDL2_ttf
  • adoptopenjdk-bin (adoptopenjdk-hotspot-bin-11 ,openjdk11-bootstrap)
  • openjdk16-bootstrap (adoptopenjdk-hotspot-bin-15)
  • openjdk8-bootstrap (adoptopenjdk-hotspot-bin-8)
  • bluejeans-gui
  • cdogs-sdl
  • clickclack
  • czkawka
  • devilutionx
  • geoclue2
  • glib-networking (gnome.glib-networking ,gnome.glib_networking)
  • gnome.gnome-desktop (gnome.gnome_desktop)
  • gnome.gnome-session-ctl
  • gtk3 (gnome.gtk ,gtk3-x11)
  • gnome.zenity
  • googleearth
  • gssdp
  • gst_all_1.gst-plugins-base
  • gst_all_1.gst-plugins-ugly
  • gtkspell3
  • gupnp
  • gupnp-igd
  • hubstaff
  • libappindicator (libappindicator-gtk3)
  • libcanberra (libcanberra_kde)
  • libcanberra-gtk3
  • libdbusmenu-gtk3
  • libdmapsharing
  • libindicator (libindicator-gtk3)
  • libnice
  • libproxy
  • libva
  • libva-minimal
  • libwnck3
  • mailspring
  • methane
  • mpdevil
  • myxer
  • networkmanager
  • nxengine-evo
  • openconnect (openconnect_gnutls)
  • pantheon.elementary-icon-theme
  • pantheon.granite
  • postman
  • prs
  • python38Packages.gst-python
  • qrencode
  • rofi
  • sdlpop
  • smpeg2
  • stoken
  • sunvox
  • upwork
  • viber
  • vulkan-loader
  • wayland-protocols
  • wayland-scanner
  • wdomirror
  • wrapGAppsHook
  • xfce.xfce4-dev-tools (xfce.xfce4_dev_tools)
  • yapesdl
5 suggestions:
  • warning: missing-patch-comment

    Consider adding a comment explaining the purpose of this patch on the line preceeding.
    Near pkgs/development/libraries/wayland/default.nix:40:5:

       |
    40 |     (substituteAll {
       |     ^
    
  • warning: missing-patch-comment

    Consider adding a comment explaining the purpose of this patch on the line preceeding.
    Near pkgs/development/libraries/wayland/default.nix:44:5:

       |
    44 |     ./0002-fix-darwin-build.patch
       |     ^
    
  • warning: build-tools-in-build-inputs

    docbook-xsl-nons is a build tool so it likely goes to nativeBuildInputs, not buildInputs.

    Near pkgs/development/libraries/wayland/default.nix:78:3:

       |
    78 |   buildInputs = [
       |   ^
    
  • warning: build-tools-in-build-inputs

    docbook_xml_dtd_42 is a build tool so it likely goes to nativeBuildInputs, not buildInputs.

    Near pkgs/development/libraries/wayland/default.nix:78:3:

       |
    78 |   buildInputs = [
       |   ^
    
  • warning: build-tools-in-build-inputs

    docbook_xml_dtd_45 is a build tool so it likely goes to nativeBuildInputs, not buildInputs.

    Near pkgs/development/libraries/wayland/default.nix:78:3:

       |
    78 |   buildInputs = [
       |   ^
    

Note that build failures may predate this PR, and could be nondeterministic or hardware dependent.
Please exercise your independent judgement. Does something look off? Please file an issue or reach out on IRC.

@stephank
Copy link
Contributor Author

Whoops! I did not consider that. 😅

I've also added a note about another upstream merge request which intends to fix this with a completely different mechanism. I'm not sure if we should swap in that patch (it does work for me) or keep this one. Argument for is that it's upstream supported, argument against is that it's a larger diff.

@ofborg ofborg bot added 10.rebuild-linux: 0 This PR does not cause any packages to rebuild and removed 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ labels May 10, 2021
@SuperSandro2000
Copy link
Member

That's a lotta rebuilds on non-darwin! Maybe instead of checking the platform in the patch itself, use stdenv.isDarwin to only include the patch on darwin?

This means the patch will rot and will most likely broken with the next update.

Also #102230

@ofborg ofborg bot added 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ and removed 10.rebuild-linux: 0 This PR does not cause any packages to rebuild labels May 11, 2021
@stephank
Copy link
Contributor Author

I suppose we should move this to staging, then?

@primeos primeos changed the base branch from master to staging May 11, 2021 10:17
@primeos
Copy link
Member

primeos commented May 11, 2021

@stephank yes, I've just moved it for you (requires rebasing onto git merge-base origin/staging origin/master to prevent CI from "spamming" people when switching the base branch on GH).

@primeos
Copy link
Member

primeos commented May 11, 2021

@GrahamcOfBorg build pkgsCross.aarch64-multiplatform.wayland
@GrahamcOfBorg test sway

@SuperSandro2000
Copy link
Member

@GrahamcOfBorg build pkgsCross.aarch64-multiplatform.wayland
@GrahamcOfBorg test sway

Please use @ofborg

Copy link
Member

@primeos primeos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran the Sway test manually to be absolutely sure and it succeeded.
Thanks for the PR!

Please use @ofborg

Force of habit... :o Thanks :)

@primeos primeos merged commit a605e03 into NixOS:staging May 12, 2021
@stephank stephank deleted the fix-wayland-darwin branch May 12, 2021 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants