diff mbox

Add glfw 3.1.2 to GNU Guix

Message ID CAKKYfmGOaEdtetLWr1h=3Y03dp1XrsRni=KzfwRMPMhza8HvoA@mail.gmail.com
State New
Headers show

Commit Message

Dennis Mungai May 26, 2016, 8:02 p.m. UTC
Hello,

This patch adds glfw 3.1.2 to GNU Guix.

Thanks and regards,

Dennis Mungai.

Comments

Ricardo Wurmus May 27, 2016, 8:26 a.m. UTC | #1
Hi Dennis,

> This patch adds glfw 3.1.2 to GNU Guix.

Thank you for your patch!

> From 6d93bf068efe283940952cb31da1f0c1ba82a0cd Mon Sep 17 00:00:00 2001
> From: brainiarc7 <dmngaie@gmail.com>
> Date: Thu, 26 May 2016 22:49:25 +0300
> Subject: [PATCH] Add glfw to GNU Guix

> ---
>  gnu/packages/gl.scm | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)

The commits should have a summar in a conventional format.  In this case
it should be something like this:

~~~~~~~~~~~~~~~~
gnu: Add glfw.

* gnu/packages/gl.scm (glfw): New variable.
~~~~~~~~~~~~~~~~


> +(define-public glfw
> +  (package
> +    (name "glfw")
> +    (version "3.1.2")
> +    (source (origin
> +             (method url-fetch)
> +             (uri (string-append "https://github.com/glfw/glfw/archive/"
> +                                 version ".tar.gz"))

Since the tarball doesn’t contain the name of the package, we usually
override its name by adding this:

    (file-name (string-append name "-" version ".tar.gz"))

> +             (sha256
> +              (base32
> +               "08pixv8hd5xsccf7l8cqcijjqaq4k4da8qbp77wggal2fq445ika"))))
> +    (build-system cmake-build-system)
> +    (arguments `(#:configure-flags '("-DBUILD_SHARED_LIBS=ON") 
> +                 #:tests? #f))

Could you please add a comment explaining why the tests are disabled?
If there are no tests it is enough to add “; no tests” on the same line.

> +    (native-inputs `(("autoconf" ,autoconf)
> +        ("automake" ,automake)

The indentation here is off.  When we have input lists with more than
one item we usually start them on the next line.

    (native-inputs
     `(("autoconf" ,autoconf)
       ("automake" ,automake)
       ...))

> +        ("cmake" ,cmake)

As you are using the “cmake-build-system” you don’t need to add “cmake”
explicitly.

> +        ("git" ,git)

Why is git needed?  This is not a git checkout.  Does the build fail
without git?

> +        ("libtool" ,libtool)
> +        ("libpthread-stubs" ,libpthread-stubs)
> +        ("pkg-config" ,pkg-config)))
> +    (inputs `(("curl" ,curl)

The above comment on indenting lists applies here too.

> +       ("dbus" ,dbus)
> +       ("enca" ,enca)
> +       ("eudev" ,eudev)
> +       ("glew" ,glew)
> +       ("libcap" ,libcap)
> +       ("libjpeg" ,libjpeg)
> +       ("libltdl" ,libltdl)
> +       ("libtiff" ,libtiff)
> +       ("mesa-utils" ,mesa-utils)
> +       ("randrproto" ,randrproto)
> +       ("libxrandr" ,libxrandr)
> +       ("xineramaproto" ,xineramaproto)
> +       ("libxinerama" ,libxinerama)
> +       ("libxcursor" ,libxcursor)
> +       ("python" ,python-2)))

That’s a long list of inputs and I haven’t checked if all of them are
necessary.  Python is a pretty big input, though.  Is it really
necessary?  Or is this for additional Python bindings?  If so, could we
move the Python bindings to a separate output?

> +    (home-page "http://www.glfw.org/")
> +    (synopsis "glfw is an Open Source, multi-platform library for
> creating windows with OpenGL contexts and receiving input and
> events.")

The synopsis is too long.  How about:

   “Library for windows with OpenGL contexts and event handling”

> +    (description "glfw is an Open Source, multi-platform library for
> creating windows with OpenGL contexts and receiving input and
> events.")

We don’t use the term “Open Source”.  All packages in Guix are free
software, so we don’t explicitly mention this.

> +    (license (list l:gpl2
> +                   l:zlib))))

Is it GPLv2 only or is it GPLv2+?  What part of this package is under
the zlib license?  Whenever lists are used it is good to add a comment
explaining what it means.

Could you please send an updated patch?

Thanks!

~~ Ricardo
diff mbox

Patch

From 6d93bf068efe283940952cb31da1f0c1ba82a0cd Mon Sep 17 00:00:00 2001
From: brainiarc7 <dmngaie@gmail.com>
Date: Thu, 26 May 2016 22:49:25 +0300
Subject: [PATCH] Add glfw to GNU Guix

---
 gnu/packages/gl.scm | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/gnu/packages/gl.scm b/gnu/packages/gl.scm
index 907e3bf..8b7a39d 100644
--- a/gnu/packages/gl.scm
+++ b/gnu/packages/gl.scm
@@ -545,3 +545,46 @@  OpenGL graphics API.")
      "SOIL is a tiny C library used primarily for uploading textures into
 OpenGL.")
     (license l:public-domain)))
+    
+(define-public glfw
+  (package
+    (name "glfw")
+    (version "3.1.2")
+    (source (origin
+             (method url-fetch)
+             (uri (string-append "https://github.com/glfw/glfw/archive/"
+                                 version ".tar.gz"))
+             (sha256
+              (base32
+               "08pixv8hd5xsccf7l8cqcijjqaq4k4da8qbp77wggal2fq445ika"))))
+    (build-system cmake-build-system)
+    (arguments `(#:configure-flags '("-DBUILD_SHARED_LIBS=ON") 
+                 #:tests? #f))
+    (native-inputs `(("autoconf" ,autoconf)
+        ("automake" ,automake)
+        ("cmake" ,cmake)
+        ("git" ,git)
+        ("libtool" ,libtool)
+        ("libpthread-stubs" ,libpthread-stubs)
+        ("pkg-config" ,pkg-config)))
+    (inputs `(("curl" ,curl)
+       ("dbus" ,dbus)
+       ("enca" ,enca)
+       ("eudev" ,eudev)
+       ("glew" ,glew)
+       ("libcap" ,libcap)
+       ("libjpeg" ,libjpeg)
+       ("libltdl" ,libltdl)
+       ("libtiff" ,libtiff)
+       ("mesa-utils" ,mesa-utils)
+       ("randrproto" ,randrproto)
+       ("libxrandr" ,libxrandr)
+       ("xineramaproto" ,xineramaproto)
+       ("libxinerama" ,libxinerama)
+       ("libxcursor" ,libxcursor)
+       ("python" ,python-2)))       
+    (home-page "http://www.glfw.org/")
+    (synopsis "glfw is an Open Source, multi-platform library for creating windows with OpenGL contexts and receiving input and events.")
+    (description "glfw is an Open Source, multi-platform library for creating windows with OpenGL contexts and receiving input and events.")
+    (license (list l:gpl2
+                   l:zlib))))
-- 
2.7.4