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

nixos/sway: Extend the default configuration for NixOS #122605

Merged
merged 1 commit into from
May 13, 2021
Merged

nixos/sway: Extend the default configuration for NixOS #122605

merged 1 commit into from
May 13, 2021

Conversation

primeos
Copy link
Member

@primeos primeos commented May 11, 2021 •
edited
Loading

The default config.in template contains
"include @sysconfdir@/sway/config.d/*" but we've dropped it to better
support non-NixOS (which seems like a mistake in retrospect).
This restores that behaviour and extends the default configuration via
nixos.conf to fix #119445.

Note: The security configurations (security.d) where dropped entirely
(but maybe they'll return).

Motivation for this change

Note: This is currently just an idea/draft and I haven't tested it yet. Unfortunately it's somewhat hacky so an alternative would be to drop our support to easily use our Sway package outside of NixOS (we could ask around if anyone uses that and if it even works as expected).

TODOs: Extend the NixOS VM test for Sway to test pinentry (maybe an optional test/extension if this causes too many rebuilds when testing changes to core packages).

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@primeos
nixos/sway: Extend the default configuration for NixOS
00e8e5b 
The default config.in template contains
"include @sysconfdir@/sway/config.d/*" but we've dropped it to better
support non-NixOS (which seems like a mistake in retrospect).
This restores that behaviour and extends the default configuration via
nixos.conf to fix NixOS#119445.

Note: The security configurations (security.d) where dropped entirely
(but maybe they'll return).
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels May 11, 2021
@primeos primeos marked this pull request as draft May 11, 2021 16:59
@primeos
Copy link
Member Author

primeos commented May 11, 2021

Some resources regarding this:

@ofborg ofborg bot requested review from Synthetica9 and Ma27 May 11, 2021 18:17
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels May 11, 2021
@primeos primeos marked this pull request as ready for review May 11, 2021 19:36
@primeos
Copy link
Member Author

primeos commented May 11, 2021 •
edited
Loading

I just tested this with a default configuration and gpg --quick-generate-key - seems to work as expected.
The implementation is far from ideal but it might be nice to have this for NixOS 21.05 (but this will only help (new) users that use /etc/sway/config).
@Synthetica9 @Ma27 what do you think?

@GrahamcOfBorg test sway

@primeos primeos mentioned this pull request May 11, 2021
Closed
@r-rmcgibbo
Copy link

r-rmcgibbo commented May 11, 2021 •
edited
Loading

Result of nixpkgs-review pr 122605 at 00e8e5b run on aarch64-linux 1

4 packages built successfully:
  • sway
  • sway-contrib.grimshot
  • sway-unwrapped
  • waybar
3 suggestions:
  • warning: missing-patch-comment

    Consider adding a comment explaining the purpose of this patch on the line preceeding.
    Near pkgs/applications/window-managers/sway/default.nix:23:5:

       |
23 |     ./sway-config-no-nix-store-references.patch
   |     ^
  • warning: missing-patch-comment

    Consider adding a comment explaining the purpose of this patch on the line preceeding.
    Near pkgs/applications/window-managers/sway/default.nix:26:5:

       |
26 |     (substituteAll {
   |     ^
  • warning: missing-patch-comment

    Consider adding a comment explaining the purpose of this patch on the line preceeding.
    Near pkgs/applications/window-managers/sway/default.nix:24:5:

       |
24 |     ./load-configuration-from-etc.patch
   |     ^

Result of nixpkgs-review pr 122605 at 00e8e5b run on x86_64-linux 1

4 packages built successfully:
  • sway
  • sway-contrib.grimshot
  • sway-unwrapped
  • waybar
3 suggestions:
  • warning: missing-patch-comment

    Consider adding a comment explaining the purpose of this patch on the line preceeding.
    Near pkgs/applications/window-managers/sway/default.nix:24:5:

       |
24 |     ./load-configuration-from-etc.patch
   |     ^
  • warning: missing-patch-comment

    Consider adding a comment explaining the purpose of this patch on the line preceeding.
    Near pkgs/applications/window-managers/sway/default.nix:23:5:

       |
23 |     ./sway-config-no-nix-store-references.patch
   |     ^
  • warning: missing-patch-comment

    Consider adding a comment explaining the purpose of this patch on the line preceeding.
    Near pkgs/applications/window-managers/sway/default.nix:26:5:

       |
26 |     (substituteAll {
   |     ^

@primeos
Copy link
Member Author

primeos commented May 13, 2021

Let's merge this for NixOS 21.05. Having /etc/sway/config.d/* in the default configuration seems like an important "fix" and the current nixos.conf for Sway should make it much easier to use GPG and screen sharing (both still require additional configuration but that isn't Sway specific / affects all other compositors). The implementation can still be changed in the future.

@primeos primeos merged commit 60f2af5 into NixOS:master May 13, 2021
primeos added a commit to primeos/nixpkgs that referenced this pull request May 13, 2021
@primeos
nixos/tests/sway: test GPG's pinentry pop-up
0380854 
This test is important to confirm that $WAYLAND_DISPLAY is correctly
imported via "dbus-update-activation-environment --systemd" which is
done by default since NixOS#122605 (00e8e5b).
It ensures that the gnome3-pinentry pop-ups work as expected to avoid
regressions like NixOS#119445 (which also broke screen sharing).
This was referenced May 13, 2021
Merged
Open
@primeos primeos mentioned this pull request May 27, 2021
Open
22 tasks
@evils
Copy link
Member

evils commented May 27, 2021

i'm not clear on how adding stuff to the default config is "restoring behaviour"
shouldn't that involve removing the part where "we dropped that"?

@primeos
Copy link
Member Author

primeos commented May 27, 2021

@evils that part's a bit complicated due to NixOS vs. Nix outside of NixOS, Nix store paths (that get garbage collected), and copying Sway's default configuration file (which should not contain any Nix store paths that will get GCed). Together with the Git history that hopefully explains the reasoning at the time.

https://github.com/NixOS/nixpkgs/pull/122995/files#diff-635aedb425667b0a9cd9ecab374760ed0c32a1853a15e95d5fb86cdfc047da1f might also help explaining it.

@NilsIrl
Copy link
Member

NilsIrl commented Jun 12, 2021

Idk if this is linked but now when I run pass (which runs gpg behind) from an ssh session, I don't get the pinentry prompt in the shell but on my desktop. This is problematic as I am unable to then enter my passphrase without being physically at my computer.

@primeos
Copy link
Member Author

primeos commented Jun 12, 2021

@NilsIrl huh, that sounds very strange (unless you're SSHing to your own machine and the same user). Shouldn't be related though (and it's easy to test). I quickly tested running gpg --quick-gen-key via SSH on a remote server and that works as expected. Anyway, this should get a dedicated issue if it's really Nixpkgs related.

@NilsIrl
Copy link
Member

NilsIrl commented Jun 16, 2021

unless you're SSHing to your own machine and the same user

It seems to me that this is what I'm doing?

I'm SSHing into the machine that is running sway as the user that is running sway (the user account I use for everything)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Synthetica9 Synthetica9 Awaiting requested review from Synthetica9

@Ma27 Ma27 Awaiting requested review from Ma27

Assignees
No one assigned
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

sway (1.6) / wlroots (0.13.0) broke screen sharing and gnome3 pinentry popups
4 participants
@primeos @r-rmcgibbo @evils @NilsIrl