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

Jump to bottom

sway: Switch to 1.0 #57414

Merged
merged 2 commits into from Mar 13, 2019
Merged

sway: Switch to 1.0 #57414

merged 2 commits into from Mar 13, 2019

Conversation

primeos
Copy link
Member

@primeos primeos commented Mar 11, 2019 •
edited

First commit (moves sway-beta to sway)

This is the result of executing:
git mv -f pkgs/applications/window-managers/sway/beta.nix pkgs/applications/window-managers/sway/default.nix
git mv -f nixos/modules/programs/sway-beta.nix nixos/modules/programs/sway.nix

And removing sway-beta from the following files:
pkgs/top-level/all-packages.nix
nixos/modules/module-list.nix

Second commit (manual changes)

Should be a separate commit to keep the history readable.

TODOs

  • Compare modules. Do we need a release-notes entry? -> There are a few breaking changes but Sway is probably not relevant enough and most users probably already use 1.0 or can easily update their configuration.
    • sway-beta introduced a package option.
    • The new module does not need any capabilities anymore and drops the sway group.
    • The keymap may now be specified withing Sway's configuration file (TODO: Does XKB_DEFAULT_* still work?)
    • WLC_REPEAT_* will most likely not work anymore.
    • We've removed i3status by default, this can break setups (should we re-add it?).
    • swaylock and swayidle are now separate packages.
    • From the upstream release notes:
      • We have completely overhauled the codebase. Some behaviors were changed to match i3 behavior, in breaking ways. There are likely many bugs we don't know about. Your millage may vary.
      • swaygrab has been removed. For screenshots, try grim, and for video capture try wlstream
      • Experimental Nvidia support has been deprecated since its inception, and has now been removed entirely. Please use nouveau and consider future purchases from GPU vendors that support open source. Other proprietary drivers are also unsupported.
      • swaylock is now distributed separately
      • swayidle, a new idle management daemon, is available separately
  • Backport to 19.03.
  • Test different setups / collect feedback - are our current defaults ok?
  • Wait for an official patch to fix the build. -> Official patch is now applied.
  • Verify that security.d/00-defaults works correctly and review sway-security. -> security.d is irrelevant ATM (source) and the sway-security man page is not in 1.0. (config.d is also empty.)
  • cc @gnidorah @Synthetica9 @colemickens
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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@tadeokondrak tadeokondrak mentioned this pull request Mar 11, 2019
Closed
10 tasks
@ghost
Copy link

ghost commented Mar 11, 2019

Cool! Thank you!

@primeos primeos marked this pull request as ready for review March 12, 2019 13:05
@primeos primeos requested a review from infinisil as a code owner March 12, 2019 13:05
@primeos
Copy link
Member Author

primeos commented Mar 12, 2019

A few things could still be improved/discussed but this should be ready now.

@xeji
Copy link
Contributor

xeji commented Mar 12, 2019

Please resolve the merge conflict.

@primeos
sway: Switch to 1.0
45004c6 
This is the result of executing:
git mv -f pkgs/applications/window-managers/sway/beta.nix pkgs/applications/window-managers/sway/default.nix
git mv -f nixos/modules/programs/sway-beta.nix nixos/modules/programs/sway.nix

And removing sway-beta from the following files:
pkgs/top-level/all-packages.nix
nixos/modules/module-list.nix
@primeos
Copy link
Member Author

primeos commented Mar 12, 2019 •
edited

Just wanted to avoid rebasing for now (that did not cause any conflicts because the relevant commit was already in my branch, automatic merging probably failed because the commit was cherry-picked and it couldn't find the correct common ancestor) because it makes reviewing the changes through GitHub harder (IMO). But it's ready to be merged via GitHub now and I've also squashed some commits (keeping two separate commits makes sense IMO to keep the history readable). The only final change was to drop the package option for now (I did not test this change yet but I can do this tomorrow if no one beats me to it).

Update: I tested the changes again to be sure everything still works.

colemickens
colemickens reviewed Mar 13, 2019
View reviewed changes
pkgs/applications/window-managers/sway/sway-config-no-nix-store-references.patch
-# Default wallpaper (more resolutions are available in @datadir@/backgrounds/sway/)
-output * bg @datadir@/backgrounds/sway/Sway_Wallpaper_Blue_1920x1080.png fill
+# Default wallpaper
+#output * bg ~/.config/sway/backgrounds/Sway_Wallpaper_Blue_1920x1080.png fill
Copy link
Member

Choose a reason for hiding this comment

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

Why is this done? The files are installed in the store path, right?

Copy link
Member Author

@primeos primeos Mar 13, 2019 •
edited

Choose a reason for hiding this comment

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

That's because most users (including me) will probably choose the default workflow from the wiki (https://github.com/swaywm/sway/wiki#configuration) which I also referred to in the documentation of the enable option:

The suggested location for the sway configuration file is ~/.config/sway/config. To begin configuring sway, create this directory and copy the default config.

mkdir -p ~/.config/sway
cp /etc/sway/config ~/.config/sway/
$EDITOR ~/.config/sway/config

Read the default config - it has comments that explain what each option does. Read man 5 sway for more information about each config command.

But then their copy won't get updated and the /nix/store/ paths will eventually getting garbage collected. And I also stopped installing the default wallpapers into /nix/store/ but we could revert that.

~/.config/sway/backgrounds/Sway_Wallpaper_Blue_1920x1080.png is not optimal but I kept it as an example.

An alternative would be to install the default wallpapers e.g. into /run/current-system/sw/share/sway/wallpapers if we want to keep them.
Edit: Or e.g. /run/current-system/sw/share/backgrounds/sway which would be the default location.

@primeos
nixos/sway: Improve the module and adapt it to NixOS
578fe3f 
This commit contains the following changes:
- nixos/sway: Remove the beta references
- sway: Drop buildDocs
- nixos/sway: Improve the documentation
- sway,nixos/sway: Adapt Sway to NixOS
  - Copy the default configuration file to /etc/sway/config (Sway will
    still load the identical file from the Nix store but this makes it
    easier to copy the default configuration file).
  - This will also remove all references to the Nix store from the
    default configuration file as they will eventually be garbage
    collected which is a problem if the user copies it.
  - I've also decided to drop the default wallpaper (alternatively we
    could copy it to a fixed location).
- nixos/sway: Drop the package option
@primeos primeos merged commit 9179a3a into NixOS:master Mar 13, 2019
@primeos primeos mentioned this pull request Mar 13, 2019
Open
22 tasks
@primeos
Copy link
Member Author

primeos commented Mar 13, 2019

I decided to merge it so that we have enough time to test it with different setups and get some feeback (I'm currently testing the backport for 19.03 and will hopefully be able to merge it soon).

I've opened #57602 for further discussion, feedback, etc.

@colemickens colemickens mentioned this pull request Mar 18, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@colemickens colemickens colemickens left review comments

@jtojnar jtojnar jtojnar left review comments

@L-as L-as L-as left review comments

@infinisil infinisil Awaiting requested review from infinisil

Assignees
No one assigned
Labels
6.topic: nixos 8.has: clean-up 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@primeos @xeji @grahamc @colemickens @jtojnar @L-as @GrahamcOfBorg