tracy: fix wayland - remove legacy build, add deps

[widlarizer]

Description of changes

This fixes #308220 on my end, but consider this a draft PR. I am very sleepy rn and don't really know what I'm doing. Tagging @paveloom @mpickering

Tasks before merging

• To be checked whether this breaks Tracy for X11 users (I guess it does)
• Try reverting to capstone from capstone_4 (if it works it's probably left as it was before)

Things done

I remove LEGACY=1. I add wayland, libxkbcommon as deps and replace capstone with capstone_4 since the current default.nix explicitly references capstone 4.

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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/)
• 24.11 Release Notes (or backporting 23.11 and 24.05 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
• [ X Fits CONTRIBUTING.md.

[Aleksanaa]

Looks good but if some x11 users see this pls give some feedback

[paveloom]

It will break the app for X11 users since the support for it has been put behind the flag since version 0.9.1.

I'm fine with it now, but, uhm... yeah... Also, see wolfpld/tracy#505.

Updating to v5 of capstone should probably work since this is the version used in the AUR package.

[widlarizer]

If X11 requires different deps, how about we split the package between tracy and tracy-x11? I'm sure there's more than zero happy X11 tracy users out there and I don't want to fight them over the steering wheel on who gets to use this program.

[paveloom]

Yep, I also think this is the way to approach this.

I would make it somewhat similar to how it's done in the derivation for rio:

1. A separate block of alphabetically sorted inputs with , withWayland ? true at the start.
2. Optionally a separate block of buildInputs with lib.optionals withWayland.
3. A separate derivation in all-packages.nix called tracy-x11 with withWayland set to false.

[widlarizer]

I have implemented what you described, and tested with xeyes that tracy-x11 is an X program and tracy is a wayland program

[widlarizer]

No idea what that test failure is supposed to be

[paveloom]

Add a separate block of build inputs, like this:

, capstone
, darwin
, dbus
, freetype
, glfw
, hicolor-icon-theme
, pkg-config
, tbb

, withWayland ? stdenv.isLinux
, libxkbcommon
, wayland

Note that we should also stick to pulling in Wayland dependencies on Linux systems only.

[paveloom]

The LEGACY flag is about choosing between GLFW and Wayland backends on Linux systems. Meaning, as of right now, the legacy build works both on X11 and Wayland (or should I say, used to work before #308220), so the use of the flag is not really an issue of X11 vs. Wayland. In the mean time, GLFW is still the default on all other platforms (including Darwin, which we support).

I agreed with the creation of the x11 package before, but now I think that we probably shouldn't add the legacy build to the binary cache. This also leads us to having two identical packages on Darwin systems (since the LEGACY flag is ignored there). So, this line should be deleted, and the release note should mention the use of the flag instead.

[flokli]

I agreed with the creation of the x11 package before, but now I think that we probably shouldn't add the legacy build to the binary cache

I think it's valid to have both in there. We have both X11 and Wayland users, and shouldn't make it unnecessarily painful for X11 users. We build so much other less useful stuff, I think it's fine to build both flavours.

[paveloom]

As I described above, this is more of a GLFW vs. custom Wayland backend issue, rather than an X11 vs. Wayland issue. On Darwin, two identical packages will be cached with the current set of changes, and the -x11 postfix doesn't really mean anything on that platform.

I wouldn't say it's painful to override one attribute if the legacy build is needed, but... If @widlarizer still wants to create a separate package, then he needs to make conditional changes to pname (different name is required to cache a different derivation) and meta.platforms.

Theoretically, we would have the tracy-wayland package for Linux only and tracy-glfw for both Linux and Darwin (the latter is the way it is packaged now). Then, one of those would conditionally be chosen as tracy.

[flokli]

I'm not saying your assessment is wrong, I'm merely proposing to have that being dealt with in a followup (or as an additional commit to this PR).

The PR fixes a crash at runtime, and already did a lot of rounds. The changes requested again might not be respectful to the PR author, who was merely sending a fix for a crash while being "very sleepy rn and don't really know what [...] doing".

We can always juggle things around a bit in unstable, especially editing release notes or removing "redundant packages".

[widlarizer]

I like the proposed solution (the one starting with "Theoretically,"). Being an X11 user on Linux is still valid and on NixOS shouldn't require overrides. Only problem is, looking at all-packages.nix for inspiration, I don't see a way of having one or two flavors of a package depending on the system. I'll leave that bit to a potential followup by @paveloom

[flokli]

Tested this on Wayland, and the tracy-x11 attribute still has the same store path as before - so there's not really any way this could have regressed X11 users (other than having to switch to the other attribute, which is clearly documented). I don't think we need to keep this broken for longer ;-)

@widlarizer can you address the cosmetical changes, or should this be done in a followup PR?

[widlarizer]

Cosmetics addressed, feel free to merge if CI goes green