[bug#55769] [PATCH] gnu: Add xwhite.

[bug#55769] [PATCH] gnu: Add xwhite.

[derekchuank]

[-- Attachment #1: Type: text/plain, Size: 1887 bytes --]

diff --git a/gnu/packages/xdisorg.scm b/gnu/packages/xdisorg.scm
index e5a98edb35..250860274d 100644
--- a/gnu/packages/xdisorg.scm
+++ b/gnu/packages/xdisorg.scm
@@ -54,6 +54,7 @@
 ;;; Copyright © 2021 jgart <jgart@dismail.de>
 ;;; Copyright © 2022 John Kehayias <john.kehayias@protonmail.com>
 ;;; Copyright © 2022 Jai Vetrivelan <jaivetrivelan@gmail.com>
+;;; Copyright © 2022 Derek Chuank <derekchuank@outlook.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1499,6 +1500,40 @@ (define-public redshift-wayland
 protocol.")
       (license license:gpl3+))))

+(define-public xwhite
+  (package
+    (name "xwhite")
+    (version "0.0.1")
+    (source
+     (origin
+       (method url-fetch)
+       (uri
+        (string-append "https://github.com/derekchuank/xwhite/"
+                       "releases/download/v" version
+                       "/xwhite-" version ".tar.gz"))
+       (sha256
+        (base32 "1gadgvl408zdl9drz6rb0dhq285a6w57bn94ql2sgcpp9mq7ym5q"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:tests? #f
+       #:phases
+       (modify-phases %standard-phases
+         (delete 'configure)
+         (replace 'install
+           (lambda* (#:key outputs #:allow-other-keys)
+             (let ((out (assoc-ref outputs "out")))
+               (install-file "xwhite" (string-append out "/bin"))
+               #t))))))
+    (inputs
+     (list libxrandr))
+    (home-page "https://github.com/derekchuank/xwhite")
+    (synopsis "Adjust color balance")
+    (description "xwhite is a command line tool for adjusting color
+balance of screen.  It is based on xrandr's gamma correction and
+brightness adjustment.  Typically used for tuning color balance
+while setting color temperature.")
+    (license license:gpl2)))
+
 (define-public gammastep
   (package
     (name "gammastep")


[-- Attachment #2: Type: text/html, Size: 3650 bytes --]

[bug#55769] [PATCH] gnu: Add xwhite.

[Maxime Devos]

[-- Attachment #1: Type: text/plain, Size: 2038 bytes --]

derekchuank@outlook.com schreef op do 02-06-2022 om 15:39 [+0000]:
> +    (arguments
> +     `(#:tests? #f

Always add a comment on why tests are disabled.
In this case, probably because there are no tests?

> +       #:phases
> +       (modify-phases %standard-phases
> +         (delete 'configure)
> +         (replace 'install
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (let ((out (assoc-ref outputs "out")))
> +               (install-file "xwhite" (string-append out "/bin"))
> +               #t))))))

Trailing #t haven't been necessary anymore since a long time

Also, a comment on the makefile.  In the makefile you do CC=gcc, but
that won't work when cross-compiling, because then TARGET-gcc (e.g. 
aarch64-linux-gnu-gcc) is required.  There are multiple ways to resolve
this, but given that you are upstream, I recommend switching to a build
system such as, say, meson[0].  meson (and autotools) take care of such
details for you (cross-compilation, also choosing the right
installation directory).

If you use meson or autotools, then you won't have to add any Guix
phases or do cross-compilation detection in the makefile, you then only
have to replace gnu-build-system by
meson-build-system.

A comment on the license: strictly speaking, you have licensed it as
‘any version of the GPL whatsoever’.  From the GPL itself:

> If the Program does not specify a version number of this License, you
> may choose any version ever published by the Free Software
> Foundation.

I doubt that's what you mean, so maybe document it in the README to be
GPL-2-or-later or GPL-2-only or GPL-2-OR-3-only?

A comment on the code itself: I recommend returning a non-zero value on
failure.  Also, all this is already implemented by another tool:

$ redshift -P -g 2:2:0.1 -O 10000

so to me there appears to be no need to write, maintain and package a
new tool.

[0] https://mesonbuild.com/

Greetings,
Maxime

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#55769] [PATCH] gnu: Add xwhite.

[derekchuank]

[-- Attachment #1: Type: text/plain, Size: 2254 bytes --]

I've fixed all the issues you mentioned: adopt meson build system, add GPTv2-only license in README, fix return values in code and a new description. Again, Thank you very much for your detailed guidance and patience.


 gnu/packages/xdisorg.scm | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/gnu/packages/xdisorg.scm b/gnu/packages/xdisorg.scm
index e5a98edb35..1c09128c17 100644
--- a/gnu/packages/xdisorg.scm
+++ b/gnu/packages/xdisorg.scm
@@ -54,6 +54,7 @@
 ;;; Copyright © 2021 jgart <jgart@dismail.de>
 ;;; Copyright © 2022 John Kehayias <john.kehayias@protonmail.com>
 ;;; Copyright © 2022 Jai Vetrivelan <jaivetrivelan@gmail.com>
+;;; Copyright © 2022 Derek Chuank <derekchuank@outlook.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1499,6 +1500,37 @@ (define-public redshift-wayland
 protocol.")
       (license license:gpl3+))))

+(define-public xwhite
+  (package
+   (name "xwhite")
+   (version "0.0.2")
+   (source
+    (origin
+     (method url-fetch)
+     (uri
+      (string-append "https://github.com/derekchuank/xwhite/"
+                     "releases/download/v" version
+                     "/xwhite-" version ".tar.gz"))
+     (sha256
+      (base32 "0jbnlj5a91ib4anprmylqqnbv9wa73cr7fsc1s54df0a0w5yq8sz"))))
+   (build-system meson-build-system)
+   (arguments
+    `(#:tests? #f)) ; No test suite.
+   (native-inputs
+    (list pkg-config))
+   (inputs
+    (list libxrandr))
+   (home-page "https://github.com/derekchuank/xwhite")
+   (synopsis "Adjust the color balance")
+   (description "@command{xwhite} is a command line tool for adjusting the colour
+balance of screen.  It is based on xrandr's gamma correction and brightness adjustment.
+As such, it can only be used for X displays and not Wayland displays.  It is typically
+used for tuning the color balance and color temperature.  It has a similar function as
+@command{redshift -P -g R:G:B -O temperature}, but @command{xwhite} is more flexible
+in that it does not keep the white color fixed, suitable for setting the white color
+to an arbitrary balanced color.")
+   (license license:gpl2)))
+
 (define-public gammastep
   (package
     (name "gammastep")


[-- Attachment #2: Type: text/html, Size: 4256 bytes --]

[bug#55769] [PATCH] gnu: Add xwhite.

[Maxime Devos]

[-- Attachment #1: Type: text/plain, Size: 774 bytes --]

user guix
usertags 55769 + reviewed looks-good reviewed-looks-good
thanks
(list available at <https://debbugs.gnu.org/cgi/pkgreport.cgi?tag=reviewed-looks-good&users=guix>)

derekchuank@outlook.com schreef op vr 03-06-2022 om 19:07 [+0000]:
> +     (uri
> +      (string-append "https://github.com/derekchuank/xwhite/"
> +                     "releases/download/v" version
> +                     "/xwhite-" version ".tar.gz"))
> +     (sha256
> +      (base32
> "0jbnlj5a91ib4anprmylqqnbv9wa73cr7fsc1s54df0a0w5yq8sz"))))


New package definition LGTM (looks good to me).
The hash matches, without any malware in the tarball.
Assuming it actually builds (I haven't checked) it should be fine to
apply.

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

bug#55769: [PATCH] gnu: Add xwhite.

[Ludovic Courtès]

Hi,

Maxime Devos <maximedevos@telenet.be> skribis:

> user guix
> usertags 55769 + reviewed looks-good reviewed-looks-good
> thanks
> (list available at <https://debbugs.gnu.org/cgi/pkgreport.cgi?tag=reviewed-looks-good&users=guix>)
>
> derekchuank@outlook.com schreef op vr 03-06-2022 om 19:07 [+0000]:
>> +     (uri
>> +      (string-append "https://github.com/derekchuank/xwhite/"
>> +                     "releases/download/v" version
>> +                     "/xwhite-" version ".tar.gz"))
>> +     (sha256
>> +      (base32
>> "0jbnlj5a91ib4anprmylqqnbv9wa73cr7fsc1s54df0a0w5yq8sz"))))
>
>
> New package definition LGTM (looks good to me).
> The hash matches, without any malware in the tarball.
> Assuming it actually builds (I haven't checked) it should be fine to
> apply.

Applied.  Thanks Derek and Maxime!

Ludo’.