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

Rename wrapGAppsHook to wrapGAppsHook3 #303921

Closed
DanielSidhion opened this issue Apr 13, 2024 · 2 comments
Closed

Rename wrapGAppsHook to wrapGAppsHook3 #303921

DanielSidhion opened this issue Apr 13, 2024 · 2 comments
Labels
0.kind: enhancement Add something new 6.topic: GNOME GNOME desktop environment and its underlying platform

Comments

@DanielSidhion
Copy link
Member

DanielSidhion commented Apr 13, 2024

Created from a comment in #302599 (comment).

There is a specific wrapGAppsHook4 for GTK 4. wrapGAppsHook brings GTK 3, so renaming it would make it easier for users to understand which version of the GTK they're getting with the hook they're using.

According to the comment in the PR above, it might be best to wait until #291339 is merged to avoid conflicts.


Add a 👍 reaction to issues you find important.

@DanielSidhion DanielSidhion added 0.kind: enhancement Add something new 6.topic: GNOME GNOME desktop environment and its underlying platform labels Apr 13, 2024
@uninsane
Copy link
Contributor

my understanding is that the eventual goal is to remove gtk{3,4} from wrapGAppsHook altogether. see the comments inside pkgs/build-support/setup-hooks/wrap-gapps-hook/default.nix:

  propagatedBuildInputs = [ makeWrapper ] ++ lib.optionals isGraphical [
    # TODO: remove this, packages should depend on GTK explicitly.
    gtk3
    librsvg
  ];

i'm not sure the precise path to get there has been elaborated. it could be:

  1. replace wrapGAppsHook with explicit wrapGAppsHook{3,4} calls.
    the gtk4 half of that has already been done. the gtk3 part is what this issue would be about.
  2. rename the existing wrapGAppsNoGuiHook -> wrapGAppsHook.
  3. port wrapGAppsHook{3,4} users back to wrapGAppsHook.
  4. delete the now unused wrapGAppsHook{3,4}.

@nyabinary
Copy link
Contributor

Done :3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: enhancement Add something new 6.topic: GNOME GNOME desktop environment and its underlying platform
Projects
None yet
Development

No branches or pull requests

3 participants