From 338939d4644ca6817a773f48a3eb910db0d42bb9 Mon Sep 17 00:00:00 2001 From: Atemu Date: Sat, 15 Feb 2020 20:05:02 +0100 Subject: [PATCH 1/5] xinit: use the system xserverrc Previously it'd try to use the one under its output path which is read-only of course --- pkgs/servers/x11/xorg/overrides.nix | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkgs/servers/x11/xorg/overrides.nix b/pkgs/servers/x11/xorg/overrides.nix index 02dfb7ee1f55..2678cd358c88 100644 --- a/pkgs/servers/x11/xorg/overrides.nix +++ b/pkgs/servers/x11/xorg/overrides.nix @@ -761,6 +761,9 @@ self: super: prePatch = '' sed -i 's|^defaultserverargs="|&-logfile \"$HOME/.xorg.log\"|p' startx.cpp ''; + postFixup = '' + substituteInPlace $out/bin/startx --replace $out/etc/X11/xinit/xserverrc /etc/X11/xinit/xserverrc + ''; }); xf86videointel = super.xf86videointel.overrideAttrs (attrs: { From c72c02ab2677601fd206925f48e87e753e9ca80b Mon Sep 17 00:00:00 2001 From: Atemu Date: Sat, 15 Feb 2020 21:31:07 +0100 Subject: [PATCH 2/5] nixos/startx: provide xserverArgs via xserverrc Fixes #80198 --- nixos/modules/services/x11/display-managers/startx.nix | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/nixos/modules/services/x11/display-managers/startx.nix b/nixos/modules/services/x11/display-managers/startx.nix index 3980203b9457..0b70154fe0d4 100644 --- a/nixos/modules/services/x11/display-managers/startx.nix +++ b/nixos/modules/services/x11/display-managers/startx.nix @@ -39,6 +39,10 @@ in displayManager.lightdm.enable = lib.mkForce false; }; systemd.services.display-manager.enable = false; + # Implement xserverArgs via xinit's system-wide xserverrc + environment.etc."X11/xinit/xserverrc".source = pkgs.writeShellScript "xserverrc" '' + exec ${pkgs.xorg.xorgserver}/bin/X ${toString config.services.xserver.displayManager.xserverArgs} "$@" + ''; environment.systemPackages = with pkgs; [ xorg.xinit ]; }; From a5ea1b6fb3e4906802e5d01e7fa08913d5b4f235 Mon Sep 17 00:00:00 2001 From: Atemu Date: Sun, 3 May 2020 19:09:43 +0200 Subject: [PATCH 3/5] xorgserver: set log-dir to /var/log instead of the Nix store Xorg creates the log-dir in its output path because X crashes if it can't write to its logfile. On a regular distro, this dir would be installed into the root to prevent that from happening but with Nix, it sits in the read-only Nix store. Ironically, when Xorg tries to write here, it fails and crashes. To make Xorg log to /var/log, we have to stop the build script from trying to create the log-dir as the sandbox doesn't (and shouldn't) have access to /var. This creates a runtime dependency on /var when running as root but that should exist on any Linux system (on NixOS, journald always creates /var/log). Previously, the startx displayManager required some workarounds for logfiles which are obsolete now. patchPhase -> postPatch because overriding the patchPhase prevents patches from being applied --- .../dont-create-logdir-during-build.patch | 32 +++++++++++++++++++ pkgs/servers/x11/xorg/overrides.nix | 9 +++++- 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 pkgs/servers/x11/xorg/dont-create-logdir-during-build.patch diff --git a/pkgs/servers/x11/xorg/dont-create-logdir-during-build.patch b/pkgs/servers/x11/xorg/dont-create-logdir-during-build.patch new file mode 100644 index 000000000000..3675292f9c99 --- /dev/null +++ b/pkgs/servers/x11/xorg/dont-create-logdir-during-build.patch @@ -0,0 +1,32 @@ +--- + hw/xfree86/Makefile.am | 1 - + hw/xfree86/Makefile.in | 1 - + 2 files changed, 2 deletions(-) + +diff --git a/hw/xfree86/Makefile.am b/hw/xfree86/Makefile.am +index 9aeaea1..dcca3b8 100644 +--- a/hw/xfree86/Makefile.am ++++ b/hw/xfree86/Makefile.am +@@ -100,7 +100,6 @@ EXTRA_DIST = xorgconf.cpp + + # Without logdir, X will post an error on the terminal and will not start + install-data-local: +- $(AM_V_GEN)$(MKDIR_P) $(DESTDIR)$(logdir) + if CYGWIN + $(INSTALL_DATA) libXorg.exe.a $(DESTDIR)$(libdir)/libXorg.exe.a + endif +diff --git a/hw/xfree86/Makefile.in b/hw/xfree86/Makefile.in +index c4fceee..74da8f1 100644 +--- a/hw/xfree86/Makefile.in ++++ b/hw/xfree86/Makefile.in +@@ -1161,7 +1161,6 @@ uninstall-am: uninstall-binPROGRAMS uninstall-local \ + + # Without logdir, X will post an error on the terminal and will not start + install-data-local: +- $(AM_V_GEN)$(MKDIR_P) $(DESTDIR)$(logdir) + @CYGWIN_TRUE@ $(INSTALL_DATA) libXorg.exe.a $(DESTDIR)$(libdir)/libXorg.exe.a + + install-exec-hook: +-- +2.25.4 + diff --git a/pkgs/servers/x11/xorg/overrides.nix b/pkgs/servers/x11/xorg/overrides.nix index 2678cd358c88..ec75a52d5929 100644 --- a/pkgs/servers/x11/xorg/overrides.nix +++ b/pkgs/servers/x11/xorg/overrides.nix @@ -571,7 +571,7 @@ self: super: attrs = if (abiCompat == null || lib.hasPrefix abiCompat version) then attrs_passed // { - buildInputs = attrs_passed.buildInputs ++ [ libdrm.dev ]; patchPhase = '' + buildInputs = attrs_passed.buildInputs ++ [ libdrm.dev ]; postPatch = '' for i in dri3/*.c do sed -i -e "s|#include |#include |" $i @@ -626,6 +626,12 @@ self: super: if (!isDarwin) then { outputs = [ "out" "dev" ]; + patches = [ + # The build process tries to create the specified logdir when building. + # + # We set it to /var/log which can't be touched from inside the sandbox causing the build to hard-fail + ./dont-create-logdir-during-build.patch + ]; buildInputs = commonBuildInputs ++ [ libdrm mesa ]; propagatedBuildInputs = attrs.propagatedBuildInputs or [] ++ [ libpciaccess epoxy ] ++ commonPropagatedBuildInputs ++ lib.optionals stdenv.isLinux [ udev @@ -642,6 +648,7 @@ self: super: "--with-xkb-bin-directory=${self.xkbcomp}/bin" "--with-xkb-path=${self.xkeyboardconfig}/share/X11/xkb" "--with-xkb-output=$out/share/X11/xkb/compiled" + "--with-log-dir=/var/log" "--enable-glamor" ] ++ lib.optionals stdenv.hostPlatform.isMusl [ "--disable-tls" From 693a31ab7b53de423d4ed87f35a6a43604d02039 Mon Sep 17 00:00:00 2001 From: Atemu Date: Sat, 15 Feb 2020 22:53:56 +0100 Subject: [PATCH 4/5] nixos/xserver: make logFile configurable It makes sense for it to be /dev/null for all the displayManagers but startx, it needs a different logFile configuration. --- nixos/modules/services/x11/xserver.nix | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/nixos/modules/services/x11/xserver.nix b/nixos/modules/services/x11/xserver.nix index 9e971671c474..eb8c4c17e987 100644 --- a/nixos/modules/services/x11/xserver.nix +++ b/nixos/modules/services/x11/xserver.nix @@ -518,6 +518,19 @@ in ''; }; + logFile = mkOption { + type = types.nullOr types.str; + default = "/dev/null"; + example = "/var/log/Xorg.0.log"; + description = '' + Controls the file Xorg logs to. + + The default of /dev/null is set so that systemd services (like displayManagers) only log to the journal and don't create their own log files. + + Setting this to null will not pass the -logfile argument to Xorg which allows it to log to its default logfile locations instead (see man Xorg). You probably only want this behaviour when running Xorg manually (e.g. via startx). + ''; + }; + verbose = mkOption { type = types.nullOr types.int; default = 3; @@ -692,11 +705,10 @@ in services.xserver.displayManager.xserverArgs = [ "-config ${configFile}" "-xkbdir" "${cfg.xkbDir}" - # Log at the default verbosity level to stderr rather than /var/log/X.*.log. - "-logfile" "/dev/null" ] ++ optional (cfg.display != null) ":${toString cfg.display}" ++ optional (cfg.tty != null) "vt${toString cfg.tty}" ++ optional (cfg.dpi != null) "-dpi ${toString cfg.dpi}" + ++ optional (cfg.logFile != null) "-logfile ${toString cfg.logFile}" ++ optional (cfg.verbose != null) "-verbose ${toString cfg.verbose}" ++ optional (!cfg.enableTCP) "-nolisten tcp" ++ optional (cfg.autoRepeatDelay != null) "-ardelay ${toString cfg.autoRepeatDelay}" From d3113a62b872286369b76c7a73998d1e84f0354d Mon Sep 17 00:00:00 2001 From: Atemu Date: Sat, 15 Feb 2020 23:53:49 +0100 Subject: [PATCH 5/5] nixos/startx: send Xorg log to the default location This partially reverts bf3d3dd19b48c432dd83aa0385b47dbe84aa647b. I don't know why we weren't getting a default logfile back then but Xorg definitely provides one now ($XDG_DATA_HOME for regular users and /var/log for root, see `man Xorg`) --- nixos/modules/services/x11/display-managers/startx.nix | 8 ++++++++ pkgs/servers/x11/xorg/overrides.nix | 3 --- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/nixos/modules/services/x11/display-managers/startx.nix b/nixos/modules/services/x11/display-managers/startx.nix index 0b70154fe0d4..6cd46cdf9649 100644 --- a/nixos/modules/services/x11/display-managers/startx.nix +++ b/nixos/modules/services/x11/display-managers/startx.nix @@ -39,6 +39,14 @@ in displayManager.lightdm.enable = lib.mkForce false; }; systemd.services.display-manager.enable = false; + + # Other displayManagers log to /dev/null because they're services and put + # Xorg's stdout in the journal + # + # To send log to Xorg's default log location ($XDG_DATA_HOME/xorg/), we do + # not specify a log file when running X + services.xserver.logFile = mkDefault null; + # Implement xserverArgs via xinit's system-wide xserverrc environment.etc."X11/xinit/xserverrc".source = pkgs.writeShellScript "xserverrc" '' exec ${pkgs.xorg.xorgserver}/bin/X ${toString config.services.xserver.displayManager.xserverArgs} "$@" diff --git a/pkgs/servers/x11/xorg/overrides.nix b/pkgs/servers/x11/xorg/overrides.nix index ec75a52d5929..ddf0aacdf07d 100644 --- a/pkgs/servers/x11/xorg/overrides.nix +++ b/pkgs/servers/x11/xorg/overrides.nix @@ -765,9 +765,6 @@ self: super: ]; propagatedBuildInputs = attrs.propagatedBuildInputs or [] ++ [ self.xauth ] ++ lib.optionals isDarwin [ self.libX11 self.xorgproto ]; - prePatch = '' - sed -i 's|^defaultserverargs="|&-logfile \"$HOME/.xorg.log\"|p' startx.cpp - ''; postFixup = '' substituteInPlace $out/bin/startx --replace $out/etc/X11/xinit/xserverrc /etc/X11/xinit/xserverrc '';