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-scanner: split from wayland #214906

Merged
merged 54 commits into from
Aug 12, 2024
Merged

Conversation

alyssais
Copy link
Member

@alyssais alyssais commented Feb 6, 2023

Description of changes

We've used a wayland-scanner = wayland.bin alias for ages, to make packages clearer and allow them to be independently overridden. Going the whole way into splitting them into separate packages is useful because it means we can have different meta.platforms attributes for libwayland and wayland-scanner.

There is no duplication in outputs between the two packages — they don't install any files in common.

Draft because I'm going to need to test the downstream packages and make sure none were relying on pulling wayland-scanner from the wayland attribute.

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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@wegank
Copy link
Member

wegank commented Feb 6, 2023

The remaining occurrences of wayland.withLibraries are in

  • pkgs/development/libraries/wayland/protocols.nix
  • pkgs/tools/graphics/mesa-demos/default.nix (well...)

and also wayland.bin in

  • pkgs/applications/networking/browsers/chromium/common.nix

@alyssais
Copy link
Member Author

alyssais commented Feb 6, 2023

Things that still need to be checked:

  • nix-review to catch packages that relied on wayland in buildInputs and strictDeps = false to get wayland-scanner
  • test cross compilation of all packages that directly depend on wayland-scanner (they might need pkg-config in depsBuildBuild now)

@alyssais alyssais added the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 7, 2023
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 7, 2023
@alyssais alyssais force-pushed the wayland-scanner branch 2 times, most recently from cf02f4c to 365caa5 Compare February 8, 2023 22:11
@ofborg ofborg bot requested a review from eadwu February 8, 2023 22:31
@ofborg ofborg bot requested a review from chvp February 8, 2023 23:16
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning this up

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 10, 2023
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 10, 2023
@alyssais alyssais force-pushed the wayland-scanner branch 2 times, most recently from 1d41c53 to 40cbc34 Compare February 10, 2023 13:14
@flokli flokli merged commit 0b09202 into NixOS:staging Aug 12, 2024
22 checks passed
@trofi
Copy link
Contributor

trofi commented Aug 18, 2024

Bisect says 921bd99 wayland-scanner: split from wayland broke aquamarine build in staging as:

$ nix build --no-link -f. -L aquamarine
...
aquamarine> Running phase: buildPhase
aquamarine> build flags: -j16 SHELL=/nix/store/srq8ja7vpm3b95p2rhw149mx9pbfaqrv-bash-5.2p32/bin/bash
aquamarine> [  3%] Generating /build/source/protocols/xdg-shell.cpp, /build/source/protocols/xdg-shell.hpp
aquamarine> [  7%] Generating /build/source/protocols/linux-dmabuf-v1.cpp, /build/source/protocols/linux-dmabuf-v1.hpp
aquamarine> [ 11%] Generating /build/source/protocols/wayland.cpp, /build/source/protocols/wayland.hpp
aquamarine> Couldn't load proto
aquamarine> make[2]: *** [CMakeFiles/aquamarine.dir/build.make:74: /build/source/protocols/wayland.cpp] Error 1
aquamarine> make[2]: *** Waiting for unfinished jobs....
aquamarine> make[1]: *** [CMakeFiles/Makefile2:85: CMakeFiles/aquamarine.dir/all] Error 2
aquamarine> make: *** [Makefile:136: all] Error 2

@alyssais alyssais deleted the wayland-scanner branch August 19, 2024 07:22
@flokli
Copy link
Contributor

flokli commented Aug 19, 2024

Thanks for the report, I'll take a close look once I rebuilt things until there :-)

The package recipe might need some slight adjustments.

@flokli flokli mentioned this pull request Aug 19, 2024
13 tasks
@flokli
Copy link
Contributor

flokli commented Aug 19, 2024

It was an issue with the upstream package picking from the wrong .pc file, which we exposed with the wayland-scanner split.

Fix in #335788.

@trofi
Copy link
Contributor

trofi commented Aug 19, 2024

Another casualty of 921bd99 wayland-scanner: split from wayland is wine64Packages.wayland on staging:

$ nix build --no-link -f. wine64Packages.wayland
...
wine64-wayland> checking for wayland-scanner... Package wayland-scanner was not found in the pkg-config search path.
wine64-wayland> Perhaps you should add the directory containing `wayland-scanner.pc'
wine64-wayland> to the PKG_CONFIG_PATH environment variable
wine64-wayland> No package 'wayland-scanner' found
wine64-wayland> no
...
wine64-wayland> configure: error: Wayland 64-bit development files not found or not new enough, the Wayland driver won't be supported.
wine64-wayland> This is an error since --with-wayland was requested.

@flokli
Copy link
Contributor

flokli commented Aug 19, 2024

Try adding wayland-scanner to buildInputs, so pkg-config can discover it?

@trofi
Copy link
Contributor

trofi commented Aug 20, 2024

Sounds good. Proposed the addition as:

@SomeoneSerge
Copy link
Contributor

Try adding wayland-scanner to buildInputs, so pkg-config can discover it?

Idk much about how wine uses wayland-scanner, but why buildInputs and not native? Are you sure this isn't just working because strictDeps are off?

@trofi
Copy link
Contributor

trofi commented Aug 20, 2024

wine's generic nix code is a bit unusual in how it pulls in pkgs due to multi-abi support. Bringing wayland-scanner the same way it used to be exposed sounds simplest just to make it work again.

@flokli
Copy link
Contributor

flokli commented Aug 20, 2024

wine's generic nix code is a bit unusual in how it pulls in pkgs due to multi-abi support. Bringing wayland-scanner the same way it used to be exposed sounds simplest just to make it work again.

Fair enough, cleaning this up can be a separate PR, that PR is not making it more messy than it was before.

@trofi
Copy link
Contributor

trofi commented Aug 21, 2024

hyprland is another impacted package:

$ nix build --no-link -f. -L hyprland
...
hyprland> cd /build/source && hyprwayland-scanner --wayland-enums //nix/store/k3wz91y35macps9c811s68z207vxymc7-wayland-1.23.0/share/wayland/wayland.xml /build/source/protocols/
hyprland> Couldn't load proto

@flokli
Copy link
Contributor

flokli commented Aug 22, 2024

hyprland is another impacted package:

$ nix build --no-link -f. -L hyprland
...
hyprland> cd /build/source && hyprwayland-scanner --wayland-enums //nix/store/k3wz91y35macps9c811s68z207vxymc7-wayland-1.23.0/share/wayland/wayland.xml /build/source/protocols/
hyprland> Couldn't load proto

This is the same issue as it was in hyprwm/aquamarine#55. I sent a PR at hyprwm/Hyprland#7467.

nixpkgs PR at #336512

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: cross-compilation Building packages on a different platform than they will be used on 6.topic: Enlightenment DE The Enlightenment Desktop Environment 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: python 6.topic: qt/kde 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 101-500 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants