Skip to content

programs/gnome-shell: init#1795

Closed
tadfisher wants to merge 1 commit intonix-community:masterfrom
tadfisher:gnome-shell
Closed

programs/gnome-shell: init#1795
tadfisher wants to merge 1 commit intonix-community:masterfrom
tadfisher:gnome-shell

Conversation

@tadfisher
Copy link
Copy Markdown
Contributor

Description

Fixes #284.

Adds programs.gnome-shell for customizing Gnome Shell extensions and theme.

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

  • If this PR adds a new module

    • Added myself as module maintainer. See example.

    • Added myself and the module files to .github/CODEOWNERS.

@tadfisher tadfisher requested a review from rycee as a code owner February 11, 2021 00:35
@tadfisher tadfisher marked this pull request as draft February 13, 2021 00:21
@tadfisher
Copy link
Copy Markdown
Contributor Author

This is a draft until #1797 is addressed, as it would be cleaner to add extension paths to XDG_DATA_DIRS instead of symlinking directories.

@terlar
Copy link
Copy Markdown
Contributor

terlar commented Feb 13, 2021

I agree, glad we came to the same conclusion, other than that I think this should be good to go.

@piegamesde
Copy link
Copy Markdown
Contributor

Can you please explain why you install the extensions to ./local/share? Simply adding the extensions to home.packages works fine, and is IMO a better solution. The only required thing is to set xdg.enable = true in order to have the correct environment variables.

@terlar
Copy link
Copy Markdown
Contributor

terlar commented May 14, 2021

Now that #1797 is merged we can do the change to rely on the XDG_DATA_DIRS instead of the symlinking and then I think we can get this merged as well.

@piegamesde a bit delayed answer. It only holds true if the XDG_DATA_DIRS includes the path where the packages are installed. This was not the case for non-NixOS before (at least for Wayland, that didn’t get the environment from home session variables correctly). Hence the comment above about waiting for that support so we don’t have to symlink anymore.

@tadfisher
Copy link
Copy Markdown
Contributor Author

I have the updated module ready, just waiting on #2001 so we can properly test.

@tadfisher tadfisher marked this pull request as ready for review May 16, 2021 22:11
@tadfisher
Copy link
Copy Markdown
Contributor Author

Updated; running this now on my system, and extensions are loading correctly.

@piegamesde
Copy link
Copy Markdown
Contributor

What's the deal with the .uuid attribute of extension packages? Do all extensions have one? Because I think wrapping all my extensions in { package = …; } doesn't look that good. It'd be nicer to have the option be a simple list of packages, and deal with that problem in nixpkgs.

@tadfisher
Copy link
Copy Markdown
Contributor Author

@piegamesde There isn't always a 1:1 correspondence between package and extension; for example, the gnome-shell-extensions package includes several extensions in $out/share/gnome-shell/extensions. I feel like this is simpler than coming up with a way to both obtain UUIDs from a package and allowing users to specify which extensions to enable.

@piegamesde
Copy link
Copy Markdown
Contributor

Oh, I see. How would using pkgs.gnome-shell-extensions with the current work? Would I do { package = pkgs.gnome-shell-extensions; id = …} with each of the uuids that it contains?

Also, what about enabling extensions that are not managed through home-manager? I think this is a use case that should be supported. Wouldn't the current implementation simply disable all non-managed extensions? One could probably work around this by passing a dummy (empty) package, but eeh.

@tadfisher
Copy link
Copy Markdown
Contributor Author

Oh, I see. How would using pkgs.gnome-shell-extensions with the current work? Would I do { package = pkgs.gnome-shell-extensions; id = …} with each of the uuids that it contains?

Correct.

Also, what about enabling extensions that are not managed through home-manager? I think this is a use case that should be supported. Wouldn't the current implementation simply disable all non-managed extensions? One could probably work around this by passing a dummy (empty) package, but eeh.

I'm conflicted as to whether we should support this use case; if you have non-nix-managed shell extensions, you should probably not configure extensions with home-manager. If you really want to make extensions break when generating a new config without also having those extensions installed manually, you can use:

{
  dconf.settings."org/gnome/shell".enabled-extensions = [ "additional-extension-uuid" "another-extension-uuid" ];
}

That will work with or without this module enabled, at the cost of you having to populate ~/.local/share/gnome-shell/extensions with the additional packages, which is kind of what home-manager is intended to solve. If you don't do this, then the failure mode is for gnome-shell to launch in "safe mode" with all extensions disabled.

@tadfisher tadfisher marked this pull request as draft May 23, 2021 22:00
@tadfisher
Copy link
Copy Markdown
Contributor Author

Moving back to draft; turns out that I still had extensions linked in ~/.local/share/gnome-shell/extensions. With those gone, just adding the extensions' $out/share dirs to XDG_DATA_DIRS results in them not being loaded by gnome-shell. Digging in further.

@tadfisher
Copy link
Copy Markdown
Contributor Author

I can't figure out what is resetting XDG_DATA_DIRS; I suspect it's something in the GDM session machinery. Updated this to add theme and extension packages to home.packages which seems to work.

@tadfisher tadfisher marked this pull request as ready for review June 13, 2021 21:45
@piegamesde
Copy link
Copy Markdown
Contributor

I'm sorry for the inconvenience, but I fear this PR should best be blocked on NixOS/nixpkgs#124315. Things are a bit moving around right now in nixpkgs, and this also affects the uuid attribute associated with extensions.

@berbiche
Copy link
Copy Markdown
Member

I'm sorry for the inconvenience, but I fear this PR should best be blocked on NixOS/nixpkgs#124315. Things are a bit moving around right now in nixpkgs, and this also affects the uuid attribute associated with extensions.

Sounds fair.
Ping me when this is ready for a review.

@terlar
Copy link
Copy Markdown
Contributor

terlar commented Jul 23, 2021

The nixpkgs PR has now been merged, but I guess it still makes sense to support both uuid and extensionUuid since people might still run an older version of nixpkgs. In my config I did:

config = { id = mkDefault config.package.uuid or config.package.extensionUuid or null; };

I guess we could do something similar here? One note is that you get a quite confusing error message for those packages that don't have a uuid. Perhaps we should adress that? Having a validation that checks the final config doesn't have id as null.

The error message you get is this:

error: A definition for option `dconf.settings.org/gnome/shell.enabled-extensions.[definition 1-entry 3]' is not of type `GVariant value'. Definition values:
       - In `<unknown-file>': null

@berbiche
Copy link
Copy Markdown
Member

Hi, is there any interest in rebasing the work on the latest changes?

@stale
Copy link
Copy Markdown

stale bot commented Jan 11, 2022

Thank you for your contribution! I marked this pull request as stale due to inactivity. If this remains inactive for another 7 days, I will close this PR. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously, when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the issue

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@stale stale bot added the status: stale label Jan 11, 2022
@stale stale bot closed this Jan 18, 2022
@terlar
Copy link
Copy Markdown
Contributor

terlar commented Jan 18, 2022

I can try to open a new PR with rebase, when I have time

@piegamesde
Copy link
Copy Markdown
Contributor

Ping us in here when you do.

@terlar terlar mentioned this pull request Mar 26, 2022
7 tasks
@terlar
Copy link
Copy Markdown
Contributor

terlar commented Mar 26, 2022

@piegamesde Finally got around to do this now, see #2843

Sorry for the delay.

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.

GNOME Shell extensions support

4 participants