Hi Leo,

Thanks for reviewing this patch and helping me do this right!

I have worked through the issues you brought up, and it should at least be
better now. Maybe even good.

There is no indication of GPL v2 or later in the project though.

Best,
Petter

On 2016-11-15 20:35, Leo Famulari wrote:
On Mon, Nov 14, 2016 at 05:05:55PM +0100, Petter wrote:
Hi,

I just made a recipe for xcalib, with help from iyzsong. This is the first time I've created one so please be extra alert while reviewing, and don't
hesitate to inform me of mistakes.

Thanks Petter and iyzsong!

I think the package should go in the (gnu packages xdisorg) module
instead of its own module.

From 00ca66ec205baf52dc340faa61fd8ac38122e273 Mon Sep 17 00:00:00 2001
From: Petter <pet...@mykolab.ch>
Date: Mon, 14 Nov 2016 16:56:33 +0100
Subject: [PATCH] Added package xcalib

The commit title should be "gnu: Add xcalib.".

Also, please add a "changelog" line describing the code changes. In this
case, it should be:

* gnu/packages/xdisorg (xcalib): New variable.

This is the GNU convention, and we mention it in the manual, section 8.5 Submitting Patches [0]. Please refer to earlier Git commits or ask for help
on IRC if you are unsure about it.

[0]
https://www.gnu.org/software/guix/manual/html_node/Submitting-Patches.html

+(define-public xcalib
+  (package
+    (name "xcalib")
+    (version "0.8")
+    (source (origin
+              (method url-fetch)
+ (uri (string-append "http://downloads.sourceforge.net/xcalib/"; + "xcalib-source-" version ".tar.gz"))

We have a SourceForge "mirror" URL that should be used. Grep for
'mirror://sourceforge' in gnu/packages for some examples of how to use
it.

+    (arguments
+     '(#:make-flags '("CC=gcc")
+       #:phases (modify-phases %standard-phases
+                  (delete 'configure)
+                  (delete 'check)

Please add a comment explaining why we skip the tests. If there is no
test suite, just use "No test suite". Also, we prefer to skip tests by
setting #:tests? #f instead of deleting the check phase.

+                  (replace 'install
+                    (lambda* (#:key outputs #:allow-other-keys)
+                      (let* ((out (assoc-ref outputs "out"))
+                             (bin (string-append out "/bin")))
+                        (mkdir-p bin)
+                        (install-file "xcalib" bin)))))))

Okay. Please check if there is any documentation that could also be
installed. You can add a build phase before 'install that just fails:
(lambda _ #f), build with --keep-failed, and then look in the build
directory for man pages, READMEs, Info pages, etc.

+    (license gpl2)))

Please double-check if the source files contain the "or later" text in
the license headers. If so, it's gpl2+.
From 9deb4d7360af358a5ef9e280b443b3df8f85143a Mon Sep 17 00:00:00 2001
From: Petter <pet...@mykolab.ch>
Date: Wed, 16 Nov 2016 01:14:36 +0100
Subject: [PATCH] gnu: Add xcalib.

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

diff --git a/gnu/packages/xdisorg.scm b/gnu/packages/xdisorg.scm
index a26c716..faf1a4d 100644
--- a/gnu/packages/xdisorg.scm
+++ b/gnu/packages/xdisorg.scm
@@ -15,6 +15,7 @@
 ;;; Copyright © 2016 Leo Famulari <l...@famulari.name>
 ;;; Copyright © 2016 Alex Kost <alez...@gmail.com>
 ;;; Copyright © 2016 Marius Bakke <mba...@fastmail.com>
+;;; Copyright © 2016 Petter <pet...@mykolab.ch>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1062,3 +1063,44 @@ XCB util-xrm module provides the following libraries:
 
 - xrm: utility functions for the X resource manager.")
     (license license:x11)))
+
+(define-public xcalib
+  (package
+    (name "xcalib")
+    (version "0.8")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append "mirror://sourceforge/xcalib/xcalib/" version
+                                  "/xcalib-source-" version ".tar.gz"))
+              (sha256
+               (base32
+                "1rh6xb51c5xz926dnn82a2fn643g0sr4a8z66rn6yi7523kjw4ca"))))
+    (build-system gnu-build-system)
+    (arguments
+     '(#:make-flags '("CC=gcc")
+       #:tests? #f   ; No test suite
+       #:phases (modify-phases %standard-phases
+                  (delete 'configure)
+                  (replace 'install
+                    (lambda* (#:key outputs #:allow-other-keys)
+                      (let* ((out (assoc-ref outputs "out"))
+                             (bin (string-append out "/bin")))
+                        (mkdir-p bin)
+                        (install-file "xcalib" bin))))
+                  (add-after 'install 'install-doc
+                    (lambda* (#:key outputs #:allow-other-keys)
+                      (let ((doc (string-append(assoc-ref outputs "out")
+                                               "/share/doc/")))
+                        (install-file "README" doc)
+                        ;; Avoid unspecified return value.
+                        #t))))))
+    (inputs `(("libx11", libx11)
+              ("libxext", libxext)
+              ("libxxf86vm", libxxf86vm)))
+    (synopsis "Tiny monitor calibration loader for XFree86 (or X.org)")
+    (description "xcalib is a tiny tool to load the content of vcgt-Tags in ICC
+profiles to the video card's gamma ramp.  It does work with most video card
+drivers except the generic VESA driver.  Alter brightness, contrast, RGB, and
+invert colors on a specific display/screen.")
+    (home-page "http://xcalib.sourceforge.net/";)
+    (license license:gpl2)))
-- 
2.10.1

Reply via email to