diff mbox series

[bug#39306] gnu: Add xsettingsd.

Message ID 87lfpp6n6v.fsf@daviwil.com
State Accepted
Headers show
Series [bug#39306] gnu: Add xsettingsd. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job

Commit Message

David Wilson Jan. 30, 2020, 2:20 p.m. UTC 
Hi Marius!

Marius Bakke <mbakke@fastmail.com> writes:

> I wonder if xdisorg.scm is better suited for this package, as it seems
> mostly unrelated to window management.  WDYT?

Agreed, that's a more appropriate place.

> gtest is provided by the 'googletest' package.  Can you try adding it?

Thanks!  I didn't think to check with that name, I've got tests enabled
using that now.

> Also, for margin comments, we only use one ';', without capitalizing the
> comment.

Margin comment no longer needed, but I'll keep this in mind for future
commits.

I've attached an updated patch file with the suggested changes.  The
only thing I'm unsure about is disabling a particular warning-as-error
that showed up in the gtest.h header:

---- SNIP ----
/gnu/store/bxapb1f1l8frjpbjckk3zdxhmcig3xzk-googletest-1.10.0/include/gtest/gtest.h:1527:11: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
   if (lhs == rhs) {
       ~~~~^~~~~~
cc1plus: all warnings being treated as errors
---- SNIP ----

Since this is a warning in gtest's own header file rather than the
package source, would it be OK to disable errors for it?

Thanks!

David

Comments

Marius Bakke Jan. 30, 2020, 2:46 p.m. UTC | #1 
David Wilson <david@daviwil.com> writes:

> I've attached an updated patch file with the suggested changes.  The
> only thing I'm unsure about is disabling a particular warning-as-error
> that showed up in the gtest.h header:
>
> ---- SNIP ----
> /gnu/store/bxapb1f1l8frjpbjckk3zdxhmcig3xzk-googletest-1.10.0/include/gtest/gtest.h:1527:11: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
>    if (lhs == rhs) {
>        ~~~~^~~~~~
> cc1plus: all warnings being treated as errors
> ---- SNIP ----
>
> Since this is a warning in gtest's own header file rather than the
> package source, would it be OK to disable errors for it?

I'm glad you asked.  :-)

It is definitively OK to disable warnings coming from dependencies.  In
fact, that is what we are supposed to do, and used to do until the
switch to GCC 7.

To clarify, when we switched to GCC 7, its search paths were changed
from C{,PLUS}_INCLUDE_PATH to CPATH.  The only[*] difference between
these search paths is that headers found on the former are treated as
"system headers", which disables warnings.

[*] Besides the fact that GCC 6 and later is very picky about the order
of entries in C_INCLUDE_PATH, which is why we had to switch; see
<https://issues.guix.gnu.org/issue/30756> for details.

So, LGTM, though I have a suggestion for the description:

> +    (synopsis "Minimal Xorg settings daemon")

Maybe s/Minimal //, as the description makes it clear that it is a
lightweight alternative to the GNOME and KDE approaches.

> +    (description "xsettingsd is a lightweight daemon that provides settings to
> +Xorg applications via the XSETTINGS specification.  It is used for defining
> +font and theme settings when a complete desktop environment (GNOME, KDE) is
> +not running.  With a simple .xsettingsd configuration file one can avoid
> +configuring visual settings in different UI toolkits separately.")

@command{xsettingsd} and @file{.xsettingsd} will make it look slightly
better/more readable in the various UIs.  :-)
diff mbox series

Patch

From b5ca2c0377d677c0e3b9e288a8229d54ccd89125 Mon Sep 17 00:00:00 2001
From: David Wilson <david@daviwil.com>
Date: Sun, 26 Jan 2020 05:55:47 -0800
Subject: [PATCH] gnu: Add xsettingsd.

* gnu/packages/xdisorg.scm (xsettingsd): New variable.
---
 gnu/packages/xdisorg.scm | 68 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/gnu/packages/xdisorg.scm b/gnu/packages/xdisorg.scm
index ecefab1dbb..501ef53d3e 100644
--- a/gnu/packages/xdisorg.scm
+++ b/gnu/packages/xdisorg.scm
@@ -30,6 +30,7 @@ 
 ;;; Copyright © 2019 Josh Holland <josh@inv.alid.pw>
 ;;; Copyright © 2019 Tanguy Le Carrour <tanguy@bioneland.org>
 ;;; Copyright © 2020 Guillaume Le Vaillant <glv@posteo.net>
+;;; Copyright © 2020 David Wilson <david@daviwil.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -57,6 +58,7 @@ 
   #:use-module (guix build-system glib-or-gtk)
   #:use-module (guix build-system meson)
   #:use-module (guix build-system python)
+  #:use-module (guix build-system scons)
   #:use-module (gnu packages)
   #:use-module (gnu packages documentation)
   #:use-module (gnu packages admin)
@@ -2005,3 +2007,69 @@  The cutbuffer and clipboard selection are always synchronized.")
 can optionally use some appearance settings from XSettings, tint2 and GTK.")
     (home-page "https://jgmenu.github.io/")
     (license license:gpl2)))
+
+(define-public xsettingsd
+  (package
+    (name "xsettingsd")
+    (version "1.0.0")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/derat/xsettingsd.git")
+             (commit (string-append "v" version))))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32
+         "05m4jlw0mgwp24cvyklncpziq1prr2lg0cq9c055sh4n9d93d07v"))))
+    (build-system scons-build-system)
+    (inputs
+     `(("libx11" ,libx11)))
+    (native-inputs
+     `(("pkg-config" ,pkg-config)
+       ("googletest" ,googletest)
+       ("googletest-source" ,(package-source googletest))))
+    (arguments
+     `(#:scons ,scons-python2
+       #:scons-flags
+       (list "CC=gcc")
+       #:phases
+       (modify-phases %standard-phases
+         (add-before 'build 'patch-sconstruct
+           (lambda* (#:key inputs #:allow-other-keys)
+             (substitute* "SConstruct"
+               ;; scons doesn't pick up environment variables automatically
+               ;; so it needs help to find path variables
+               (("env = Environment\\(")
+                "env = Environment(
+                         ENV = {
+                           'PATH': os.environ['PATH'],
+                           'CPATH': os.environ['CPATH'],
+                           'LIBRARY_PATH': os.environ['LIBRARY_PATH'],
+                           'PKG_CONFIG_PATH': os.environ['PKG_CONFIG_PATH']
+                         },")
+               ;; Update path to gtest source files used in tests
+               (("/usr/src/gtest") (string-append
+                                    (assoc-ref inputs "googletest-source")
+                                    "/googletest"))
+               ;; Exclude one warning that causes a build error
+               (("-Werror") "-Werror -Wno-error=sign-compare"))
+             #t))
+         ;; The SConstruct script doesn't configure installation so
+         ;; binaries must be copied to the output path directly
+         (replace 'install
+           (lambda* (#:key outputs #:allow-other-keys)
+             (let* ((out (assoc-ref outputs "out"))
+                    (bin (string-append out "/bin")))
+               (mkdir-p bin)
+               (install-file "xsettingsd" bin)
+               (install-file "dump_xsettings" bin)
+               #t))))))
+    (home-page "https://github.com/derat/xsettingsd")
+    (synopsis "Minimal Xorg settings daemon")
+    (description "xsettingsd is a lightweight daemon that provides settings to
+Xorg applications via the XSETTINGS specification.  It is used for defining
+font and theme settings when a complete desktop environment (GNOME, KDE) is
+not running.  With a simple .xsettingsd configuration file one can avoid
+configuring visual settings in different UI toolkits separately.")
+    (license license:bsd-3)))
-- 
2.24.1