programs.sway-beta: module init (temporary until sway-beta becomes sway-1.0) #48916

colemickens · 2018-10-23T20:30:34Z

Motivation for this change

Now that Sway-1.0-beta is in nixpkgs (#48829), we should discuss how to update the sway program module.

I have been running something like this with Sway 1.0-pre for sometime and it's been working well. Many of these changes were made due to suggestions from @srhb (and emily from IRC, I believe? Sorry for not remembering better to give credit and thanks!)

I'm looking for suggestions on how to handle this. Do we want a "sway-beta" program module? Seems messy in terms of upgrading/deprecations in the future, but also it may be too early to merge the change as-is now, since we still have older sway packaged as sway.

Thoughts? cc: @primeos @srhb @Synthetica9 @xeji

(I'm sending this out for initial review/testing. I'm still working on rebasing my local branches to test out everything with the sway-beta that was packaged earlier.)

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
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 nox --run "nox-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)
Fits CONTRIBUTING.md.

colemickens · 2018-10-23T20:45:08Z

nixos/modules/programs/sway.nix

-      type = types.lines;
-      default = "";
-      example = ''
-        # Define a keymap (US QWERTY is the default)


It might be nice to keep this since some users will need to set various XKB_ env vars before starting sway, but OTOH, if we remove it we can remove the sway wrapper entirely...

thoughts?

We can remove this for Sway 1.0 because it can/should be configured in the sway configuration, see https://github.com/swaywm/sway/wiki#keyboard-layout, but we should make that obvious (e.g. print an error message with instructions, if this option is set after the switch to 1.0).

Oh great, I'd totally misread the docs and thought they were still env-var-based in 1.0. Thanks.

Synthetica9 · 2018-10-23T20:49:37Z

nixos/modules/programs/sway.nix

-      '';
+    package = mkOption {
+      type = types.package;
+      default = pkgs.sway-beta;


I think the default should still be sway until 1.0.0 is officially released.

I think that makes sense. Should I do a "split" module sort of thing where I keep the setcap wrapper unless programs.sway.beta = true or something?

You could consider duplicating the module so we (temporarily) have a sway and a sway-beta module like we do for the package. Once 1.0.0 is released, the sway-beta module and package will replace the previous sway. Maybe that's easier to maintain.

@xeji seems simpler, yes, I'd be fine with it. Are we worried about introducing sway-beta and then removing it weeks later, from a deprecation, backward-compat standpoint? Maybe we don't care since none of this is likely to slip into 18.09 in this form?

I wouldn't care at this point. It's called beta for a reason, so users should expect it to break.
sway-beta shouldn't be backported to 18.09 anyway. But it might eventually end up in 19.03 unless sway 1.0.0 is released first...
@primeos what do you think?

I agree with @xeji, introducing a sway-beta module until the final release of Sway 1.0 sounds like the best idea. We could also use the description to mention that this module is only temporary.

matthewbauer · 2018-10-23T21:10:21Z

At this point can’t we just ask users to put sway in systemPackages? That seems like an easier thing here.

colemickens · 2018-10-23T21:12:01Z

@matthewbauer I personally think there's a bit of value in having the bits there for swaylock.

colemickens · 2018-10-24T00:05:23Z

Comments addressed, please take a look. Thanks for the feedback @Synthetica9 @primeos @xeji !

Synthetica9

LGTM

primeos · 2018-10-24T15:52:10Z

nixos/modules/programs/sway-beta.nix

+in {
+  options.programs.sway-beta = {
+    enable = mkEnableOption ''
+      The i3-compatible Wayland compositor. (temporary beta package)


The description isn't that relevant, but if possible you should start with a lowercase character (the result will be "Whether to enable .") and package might be a bit misleading here.

Maybe something like: "Sway, the i3-compatible tiling Wayland compositor. This module will be removed after the final release of Sway 1.0"?

Other than that it looks good to me :)

primeos · 2018-10-24T15:58:18Z

nixos/modules/programs/sway-beta.nix

+  };
+
+  config = mkIf cfg.enable {
+    environment.systemPackages = [ swayPackage ];


IIRC xwayland is required in environment.systemPackages (or PATH) for XWayland support (didn't test this but yesterday I got an error message without it). Maybe we should keep extraPackages (we could also suggest termite, rofi, ...) via the examples)?

colemickens · 2018-10-24T21:25:24Z

Updated with the suggested changes, including the description, extraPackages, xwayland, etc. Though I have rofi the example text for extraPackages, I added dmenu by default so that the user might have a shot at doing something with the default sway config. (default sway without dmenu or urxvt can be a bit limiting.)

primeos

LGTM
@xeji @matthewbauer If you don't have any complaints I'd merge it tomorrow (or feel free to merge it yourself if you want).

At this point can’t we just ask users to put sway in systemPackages? That seems like an easier thing here.

@matthewbauer Good point, the module was pretty minimal at the time of your comment. But IMO the current version is already helpful and makes it easier to try out Sway and it's also possible that the module will grow until the final release of Sway 1.0 (the documentation isn't completely up-to-date yet).

primeos · 2018-10-24T21:38:19Z

nixos/modules/programs/sway-beta.nix

+in {
+  options.programs.sway-beta = {
+    enable = mkEnableOption ''
+      Sway, the i3-compatible tiling Wayland compositor. This module will be removed after the final release of Sway 1.0"


If you have time for it you might want to remove the last double quotes (...Sway 1.0"), as that might be visible in the manual, but we can also change that when merging ;)

good eye, fixed. thanks.

GrahamcOfBorg added 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Oct 23, 2018

colemickens commented Oct 23, 2018

View reviewed changes

Synthetica9 reviewed Oct 23, 2018

View reviewed changes

colemickens force-pushed the sway-module branch from 19aeae0 to 45a3f14 Compare October 23, 2018 22:18

colemickens changed the title ~~programs.sway: module overhaul for 1.0~~ programs.sway-beta: module init (temporary until sway-beta becomes sway-1.0) Oct 23, 2018

colemickens force-pushed the sway-module branch 2 times, most recently from 324b432 to 32146eb Compare October 24, 2018 00:04

colemickens force-pushed the sway-module branch 3 times, most recently from d7cf07f to 491afa6 Compare October 24, 2018 00:51

Synthetica9 approved these changes Oct 24, 2018

View reviewed changes

primeos reviewed Oct 24, 2018

View reviewed changes

colemickens force-pushed the sway-module branch 2 times, most recently from c166bc0 to 5cb931b Compare October 24, 2018 21:24

primeos approved these changes Oct 24, 2018

View reviewed changes

sway-beta: module init

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.

Learn about vigilant mode

da960bb

colemickens force-pushed the sway-module branch from 5cb931b to da960bb Compare October 24, 2018 21:56

primeos merged commit 163adc5 into NixOS:master Oct 25, 2018

colemickens deleted the sway-module branch October 27, 2018 11:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

programs.sway-beta: module init (temporary until sway-beta becomes sway-1.0) #48916

programs.sway-beta: module init (temporary until sway-beta becomes sway-1.0) #48916

colemickens commented Oct 23, 2018

colemickens Oct 23, 2018

primeos Oct 23, 2018

colemickens Oct 23, 2018

Synthetica9 Oct 23, 2018

colemickens Oct 23, 2018

xeji Oct 23, 2018

colemickens Oct 23, 2018

xeji Oct 23, 2018

primeos Oct 23, 2018

matthewbauer commented Oct 23, 2018

colemickens commented Oct 23, 2018

colemickens commented Oct 24, 2018

Synthetica9 left a comment

primeos Oct 24, 2018

primeos Oct 24, 2018

colemickens commented Oct 24, 2018

primeos left a comment

primeos Oct 24, 2018

colemickens Oct 24, 2018

programs.sway-beta: module init (temporary until sway-beta becomes sway-1.0) #48916

programs.sway-beta: module init (temporary until sway-beta becomes sway-1.0) #48916

Conversation

colemickens commented Oct 23, 2018

Motivation for this change

Things done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewbauer commented Oct 23, 2018

colemickens commented Oct 23, 2018

colemickens commented Oct 24, 2018

Synthetica9 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colemickens commented Oct 24, 2018

primeos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment