Merge pull request #100774 from Atemu/startx-improvements

Startx improvements

nixos/modules/services/x11/display-managers/startx.nix

@@ -39,6 +39,18 @@ 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} "$@"
+    '';
     environment.systemPackages =  with pkgs; [ xorg.xinit ];
   };
 

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 <literal>/dev/null</literal> is set so that systemd services (like <literal>displayManagers</literal>) only log to the journal and don't create their own log files.
+
+          Setting this to <literal>null</literal> will not pass the <literal>-logfile</literal> argument to Xorg which allows it to log to its default logfile locations instead (see <literal>man Xorg</literal>). You probably only want this behaviour when running Xorg manually (e.g. via <literal>startx</literal>).
+        '';
+      };
+
       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}"

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
+

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 <drm_fourcc.h>|#include <libdrm/drm_fourcc.h>|" $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"
@@ -758,8 +765,8 @@ self: super:
     ];
     propagatedBuildInputs = attrs.propagatedBuildInputs or [] ++ [ self.xauth ]
                          ++ lib.optionals isDarwin [ self.libX11 self.xorgproto ];
-    prePatch = ''
+    postFixup = ''
-      sed -i 's|^defaultserverargs="|&-logfile \"$HOME/.xorg.log\"|p' startx.cpp
+      substituteInPlace $out/bin/startx --replace $out/etc/X11/xinit/xserverrc /etc/X11/xinit/xserverrc
     '';
   });