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.

Sign up for GitHub

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

Jump to bottom

tracy: fix wayland - remove legacy build, add deps #315205

Merged
merged 1 commit into from
Jun 16, 2024
+14 −2
Merged

tracy: fix wayland - remove legacy build, add deps #315205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions nixos/doc/manual/release-notes/rl-2411.section.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@
services.portunus.ldap.package = pkgs.openldap.override { libxcrypt = pkgs.libxcrypt-legacy; };
```

- The `tracy` package no longer works on X11, since it's moved to Wayland
support, which is the intended default behavior by Tracy maintainers.
X11 users have to switch to the new package `tracy-x11`.

## Other Notable Changes {#sec-release-24.11-notable-changes}

<!-- To avoid merge conflicts, consider adding your item at an arbitrary place in the list instead. -->
Expand Down
0 ...1-remove-unifiedtypeidentifiers-framework → ...1-remove-unifiedtypeidentifiers-framework
View file
Open in desktop
File renamed without changes.
10 changes: 9 additions & 1 deletion pkgs/development/tools/tracy/default.nix → pkgs/by-name/tr/tracy/package.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
, hicolor-icon-theme
, pkg-config
, tbb

, withWayland ? stdenv.isLinux
, libxkbcommon
, wayland
}:

stdenv.mkDerivation rec {
Expand All @@ -33,6 +37,9 @@ stdenv.mkDerivation rec {
capstone
freetype
glfw
] ++ lib.optionals (stdenv.isLinux && withWayland) [
libxkbcommon
wayland
] ++ lib.optionals stdenv.isLinux [
dbus
hicolor-icon-theme
Expand Down Expand Up @@ -60,7 +67,8 @@ stdenv.mkDerivation rec {
make -j $NIX_BUILD_CORES -C csvexport/build/unix release
make -j $NIX_BUILD_CORES -C import-chrome/build/unix release
make -j $NIX_BUILD_CORES -C library/unix release
make -j $NIX_BUILD_CORES -C profiler/build/unix release LEGACY=1
make -j $NIX_BUILD_CORES -C profiler/build/unix release \
${lib.optionalString (stdenv.isLinux && !withWayland) "LEGACY=1"}
make -j $NIX_BUILD_CORES -C update/build/unix release
runHook postBuild
Expand Down
2 changes: 1 addition & 1 deletion pkgs/top-level/all-packages.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7584,7 +7584,7 @@ with pkgs;

tracker = callPackage ../development/libraries/tracker { };

tracy = callPackage ../development/tools/tracy { };
tracy-x11 = callPackage ../by-name/tr/tracy/package.nix { withWayland = false; };
Copy link
Member

@paveloom paveloom Jun 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Member

@paveloom paveloom Jun 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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


trivy = callPackage ../tools/admin/trivy { };

Expand Down