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

XMonad: configured recompile #107696

Merged
merged 9 commits into from Dec 28, 2020

Conversation

ivanbrennan
Copy link
Member

@ivanbrennan ivanbrennan commented Dec 27, 2020

Motivation for this change

In services.xserver.windowManager.xmonad, the xmonad binary was only given the ability to recompile from a local config (e.g. ~/.xmonad/xmonad.hs) if the config option was left unset. There was no clean way to set config and maintain the ability to recompile from a local config, making it difficult to try out xmonad config changes without a nixos-rebuild.

Adding ghc with the necessary packages (xmonad, xmonad-contrib, etc.) to environment.systemPackages, is undesirable because it places both ghc and an unconfigured xmonad on PATH:

$ ls -l $(which xmonad ghc)
lrwxrwxrwx ... /run/current-system/sw/bin/ghc -> /nix/store/...-ghc-8.10.2-with-packages/bin/ghc
lrwxrwxrwx ... /run/current-system/sw/bin/xmonad -> /nix/store/...-ghc-8.10.2-with-packages/bin/xmonad

The xmonad on PATH would differ from the one run by the NixOS module, and restarting xmonad would dump the user into the unconfigured version. If no local config was found, they'd be left in an unconfigured state.

Things done

Give xmonad runtime access to ghc even if the config option is set. This allows users to switch between the system config and a local config while avoiding the pitfalls of exposing ghc globally. (Additionally, include xmonad man page, which was previously only included if config was left unset.)

Since the xmonad on PATH is the same one run by the NixOS module, there's no danger a user will unwittingly restart into an unconfigured version, and because xmonad will refuse to recompile if no local config exists, there's no danger a user will unwittingly recompile into an unconfigured version.


If a local config exists, the recompile/restart behavior depends on two factors:

  • which entry point is used
    • XMonad.xmonad (default)
    • XMonad.launch (recommended in "config" option description)
  • what operation is triggered (i.e. via mod+q)
    • spawn "xmonad --recompile && xmonad --restart" (default)
    • restart "xmonad" True
    • custom function

If the default XMonad.xmonad entrypoint and default mod+q operation are used, hitting mod+q will compile and exec the local config, which will remain in use until next time the display manager is restarted.

If the entrypoint is changed to XMonad.launch and mod+q bound to restart "xmonad" True, hitting mod+q will restart from the system config. Another key can be bound to a custom function, e.g. compileRestart as show in the "config" option example, to compile and exec a local config. The user can use these two keys to switch between the system and local config at will.

  • 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.

When the "config" option isn't set, we use xmonad-with-packages to
provide xmonad with runtime access to an isolated ghc, ensuring it can
recompile and exec a user's local config (e.g. $HOME/.xmonad/xmonad.hs)
regardless of which ghc (if any) is on PATH.

When the "config" option is set, however, we compile a configured xmonad
executable upfront (during nixos-rebuild), and prior to this commit, it
was not provided with runtime access to an isolated ghc.

As a result, with the "config" option set, it was not possible
to recompile and exec a user's local config unless there was a
compatible version of ghc on PATH with the necessary packages (xmonad,
xmonad-contrib, etc.) in its package database. Adding such a ghc to
environment.systemPackages, e.g.

  (haskellPackages.ghcWithPackages (ps: with ps; [xmonad xmonad-contrib]))

is problematic because it adds both ghc and an unconfigured xmonad to
PATH, e.g.

  $ ls -l $(which xmonad ghc)
  lrwxrwxrwx ... /run/current-system/sw/bin/ghc -> /nix/store/...-ghc-8.10.2-with-packages/bin/ghc
  lrwxrwxrwx ... /run/current-system/sw/bin/xmonad -> /nix/store/...-ghc-8.10.2-with-packages/bin/xmonad

Having the unconfigured xmonad on PATH is particularly bad because
restarting xmonad will dump the user into the unconfigured version, and
if no local config exists (e.g. in $HOME/.xmonad/xmonad.hs), they'll be
left in this unconfigured state.

In this commmit, we give the configured xmonad runtime access to ghc
like xmonad-with-packages does for the unconfigured version. The aim
is to allow the user to switch between the nixos module's config and a
local config (e.g. $HOME/.xmonad/xmonad.hs) at will, so they can try out
config changes without performing a nixos-rebuild.

Since the xmonad on PATH is the configured executable, there's no
danger a user could unwittingly restart into the unconfigured version,
and because xmonad will refuse to recompile when no local config
exists, there's no danger a user could unwittingly recompile into an
unconfigured version.

Given that a local config exists, the recompile/restart behavior depends
on two factors:
- which entry point is used
  * 'XMonad.xmonad' (default)
  * 'XMonad.launch' (recommended in "config" option description)
- what operation is triggered (i.e. via mod+q)
  * `spawn "xmonad --recompile && xmonad --restart"` (default)
  * `restart "xmonad" True`
  * custom function

If the default 'XMonad.xmonad' entrypoint and default mod+q operation
are used, hitting mod+q will compile and exec the local config, which
will remain in use until next time the display manager is restarted.

If the entrypoint is changed to 'XMonad.launch' but mod+q left with its
default operation, hitting mod+q will have no visible effect. The logs
(as seen by running `journalctl --identifier xmonad --follow`) will show
an error,
  X Error of failed request:  BadAccess (attempt to access private resource denied)
which indicates that the shell was unable to start xmonad because
another window manager is already running (namely, the nixos-configured
xmonad).
https://wiki.haskell.org/Xmonad/Frequently_asked_questions#X_Error_of_failed_request:_BadAccess_.28attempt_to_access_private_resource_denied.29

Changing the mod+q operation to `restart "xmonad" True` (as recommended
in the "config" option's description) will allow a restart of the
nixos-configured xmonad to be triggeredy by hitting mod+q.

Finally, if the entrypoint is 'XMonad.launch', mod+q has been
bound to `restart "xmonad" True` and another key bound to a custom
recompile/restart function (e.g. `compileRestart` as shown in the
"config" option example), the user can switch between the nixos module's
config and their local config, with the custom key switching to the
local config and mod+q switching back.
Prior to this commit, man pages were not installed if the "config"
option was set.
Calling writeStateToFile prior to recompiling and restarting allows
state (workspaces, etc.) to be preserved across the restart.
@Lassulus
Copy link
Member

looks good.
So if I have a xmonad config specified, have a binding for restart "xmonad" True and I then rebuild my system. Will pressing the recompile button bring me to the xmonad before the rebuild or the one after the rebuild?

Copy link
Contributor

@xaverdh xaverdh left a comment

Choose a reason for hiding this comment

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

Generally looks good to me, I did some testing with my own config, and switching between config compiled from ~/.xmonad and global config appears to work fine!

@@ -108,7 +120,7 @@ in {
{ modMask = mod4Mask -- Use Super instead of Alt
, terminal = "urxvt" }
`additionalKeys`
[ ( (mod4Mask,xK_r), compileRestart )
[ ( (mod4Mask,xK_r), writeStateToFile >> compileRestart )
Copy link
Contributor

Choose a reason for hiding this comment

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

good catch adding writeStateToFile, I forgot about layout in my original pr..
But we probably want to add that in compileRestart itself instead of here, I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

I created bindings for compileRestart both with and without writeStateToFile since I occasionally (though very rarely) want to clear out the state (when I've made a mess of things while experimenting with my xmonad config).

Maybe it should accept a Bool parameter like restart does.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking of something like:

compileRestart :: X ()
compileRestart = whenX (recompile True)
  $ writeStateToFile *> catchIO ( do
     dir  <- getXMonadDataDir
     args <- getArgs
     executeFile (dir </> compiledConfig) False args Nothing )

Copy link
Member Author

Choose a reason for hiding this comment

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

In my opinion though, it's cleaner and clearer if compileRestart remains as it was and the call site handles writing (or skipping) the state file.

Copy link
Member Author

Choose a reason for hiding this comment

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

@xaverdh I took your suggestion and added a Bool resume parameter similar to the one used by restart. 88eee47

Copy link
Contributor

Choose a reason for hiding this comment

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

The commit you pushed is the cleanest solution I think, because its the same signature as restart "xmonad" :)

@ivanbrennan
Copy link
Member Author

So if I have a xmonad config specified, have a binding for restart "xmonad" True and I then rebuild my system. Will pressing the recompile button bring me to the xmonad before the rebuild or the one after the rebuild?

@Lassulus Pressing the key (triggering restart "xmonad" True) will bring you to the xmonad after the rebuild.

ivanbrennan added a commit to ivanbrennan/nixbox that referenced this pull request Dec 27, 2020
Enable switching xmonad between system config and local config at will.

I've temporarily disabled the existing xmonad NixOS module and swapped
in a local copy of the changes from
NixOS/nixpkgs#107696 to facilitate this. Once
that's made its way into the nixos-unstable channel, we can remove this
local copy.
@Lassulus Lassulus merged commit b90c5cb into NixOS:master Dec 28, 2020
@ivanbrennan ivanbrennan deleted the xmonad-configured-recompile branch December 28, 2020 16:33
ivanbrennan added a commit to ivanbrennan/nixbox that referenced this pull request Dec 31, 2020
Upstream now has the xmonad changes introduced by NixOS/nixpkgs#107696

After updating to the latest nixos-unstable channel, running `nix-info`
shows:

  system: "x86_64-linux", multi-user?: yes, version: nix-env (Nix) 2.3.10, channels(root): "nixos-21.03pre260232.733e537a8ad", nixpkgs: /nix/var/nix/profiles/per-user/root/channels/nixos/nixpkgs
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.

None yet

3 participants