unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] Add powertabeditor.
@ 2015-06-06 10:08 Ricardo Wurmus
  2015-06-09 13:32 ` Ludovic Courtès
  0 siblings, 1 reply; 4+ messages in thread
From: Ricardo Wurmus @ 2015-06-06 10:08 UTC (permalink / raw)
  To: Guix-devel

[-- Attachment #1: 0001-gnu-Add-withershins.patch --]
[-- Type: text/x-patch, Size: 3943 bytes --]

From da77c25e8c32243ca2bd77bd76de41312aafaac1 Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Mon, 25 May 2015 22:13:27 +0200
Subject: [PATCH 1/6] gnu: Add withershins.

* gnu/packages/code.scm (withershins): New variable.
---
 gnu/packages/code.scm | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/gnu/packages/code.scm b/gnu/packages/code.scm
index 9d2bde8..63a360e 100644
--- a/gnu/packages/code.scm
+++ b/gnu/packages/code.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2015 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2015 Andreas Enge <andreas@enge.fr>
+;;; Copyright © 2015 Ricardo Wurmus <rekado@elephly.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -22,9 +23,12 @@
   #:use-module (guix download)
   #:use-module ((guix licenses) #:prefix license:)
   #:use-module (guix build-system gnu)
+  #:use-module (guix build-system cmake)
+  #:use-module (gnu packages base)
   #:use-module (gnu packages compression)
   #:use-module (gnu packages databases)
   #:use-module (gnu packages emacs)
+  #:use-module (gnu packages gcc)
   #:use-module (gnu packages pcre)
   #:use-module (gnu packages pkg-config)
   #:use-module (gnu packages perl)
@@ -225,3 +229,61 @@ COCOMO model or user-provided parameters.")
 files, but compared to grep is much faster and respects files like .gitignore,
 .hgignore, etc.")
     (license license:asl2.0)))
+
+(define-public withershins
+  (package
+    (name "withershins")
+    (version "0.1")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append
+                    "https://github.com/cameronwhite/withershins/archive/v"
+                    version ".tar.gz"))
+              (file-name (string-append name "-" version ".tar.gz"))
+              (sha256
+               (base32
+                "08z3lyvswx7sad10637vfpwglbcbgzzcpfihw0x8lzr74f3b70bh"))))
+    (build-system cmake-build-system)
+    (arguments
+     `(#:out-of-source? #f
+       #:modules ((guix build utils)
+                  (guix build cmake-build-system)
+                  (ice-9 popen)
+                  (ice-9 rdelim))
+       #:phases
+       (modify-phases %standard-phases
+         (add-after
+          'unpack 'find-libiberty
+          (lambda _
+            (let ((plugin (let* ((port (open-input-pipe
+                                        "gcc -print-file-name=plugin"))
+                                 (str  (read-line port)))
+                            (close-pipe port)
+                            str)))
+              (substitute* "cmake/FindIberty.cmake"
+                (("/usr/include") (string-append plugin "/include"))
+                (("libiberty.a iberty") (string-append "NAMES libiberty.a iberty\nPATHS \""
+                                                       (assoc-ref %build-inputs "gcc")
+                                                       "/lib" "\"")))
+              #t)))
+         (replace
+          'install
+          (lambda* (#:key outputs #:allow-other-keys)
+            (let ((out (assoc-ref outputs "out")))
+              (mkdir-p (string-append out "/lib"))
+              (mkdir (string-append out "/include"))
+              (copy-file "src/withershins.hpp"
+                         (string-append out "/include/withershins.hpp"))
+              (copy-file "src/libwithershins.a"
+                         (string-append out "/lib/libwithershins.a")))
+            #t)))))
+    (home-page "https://github.com/cameronwhite/withershins")
+    (inputs
+     `(("gcc" ,gcc-4.8 "lib") ;for libiberty.a
+       ("binutils" ,binutils) ;for libbfd
+       ("zlib" ,zlib)))
+    (synopsis "C++11 library for generating stack traces")
+    (description
+     "Withershins is a simple cross-platform C++11 library for generating
+stack traces.")
+    (license license:expat)))
-- 
2.2.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-gnu-Add-RapidJSON.patch --]
[-- Type: text/x-patch, Size: 1533 bytes --]

From d426c47462be1dcc5ff4bbe9d1b154bbbb0761b9 Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Mon, 25 May 2015 22:14:39 +0200
Subject: [PATCH 2/6] gnu: Add RapidJSON.

* gnu/packages/web.scm (rapidjson): New variable.
---
 gnu/packages/web.scm | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/gnu/packages/web.scm b/gnu/packages/web.scm
index e77bad7..6660c3f 100644
--- a/gnu/packages/web.scm
+++ b/gnu/packages/web.scm
@@ -259,6 +259,27 @@ easily construct JSON objects in C, output them as JSON formatted strings and
 parse JSON formatted strings back into the C representation of JSON objects.")
     (license l:x11)))
 
+(define-public rapidjson
+  (package
+    (name "rapidjson")
+    (version "1.0.2")
+    (source (origin
+             (method url-fetch)
+             (uri (string-append
+                   "https://github.com/miloyip/rapidjson/archive/v"
+                   version ".tar.gz"))
+             (file-name (string-append name "-" version ".tar.gz"))
+             (sha256
+              (base32
+               "0rl6s0vg5y1dhh9vfl1lqay3sxf69sxjh0czxrjmasn7ng91wwf3"))))
+    (build-system cmake-build-system)
+    (home-page "https://github.com/miloyip/rapidjson")
+    (synopsis "JSON parser/generator for C++ with both SAX/DOM style API")
+    (description
+     "RapidJSON is a fast JSON parser/generator for C++ with both SAX/DOM
+style API.")
+    (license l:expat)))
+
 (define-public libwebsockets
   (package
     (name "libwebsockets")
-- 
2.2.1


[-- Attachment #3: 0003-gnu-Add-pugixml.patch --]
[-- Type: text/x-patch, Size: 2759 bytes --]

From 337f0790e7917f7ae2b394310c8543256756f0fe Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Thu, 28 May 2015 09:43:53 +0200
Subject: [PATCH 3/6] gnu: Add pugixml.

* gnu/packages/xml.scm (pugixml): New variable.
---
 gnu/packages/xml.scm | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/gnu/packages/xml.scm b/gnu/packages/xml.scm
index 8a4d2fb..a7925d9 100644
--- a/gnu/packages/xml.scm
+++ b/gnu/packages/xml.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2013, 2014, 2015 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2013, 2015 Andreas Enge <andreas@enge.fr>
 ;;; Copyright © 2015 Eric Bavier <bavier@member.fsf.org>
+;;; Copyright © 2015 Ricardo Wurmus <rekado@elephly.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -30,6 +31,7 @@
   #:use-module ((guix licenses) #:prefix license:)
   #:use-module (guix packages)
   #:use-module (guix download)
+  #:use-module (guix build-system cmake)
   #:use-module (guix build-system gnu)
   #:use-module (guix build-system perl)
   #:use-module (gnu packages linux))
@@ -358,6 +360,41 @@ from XML::Parser.  It parses XML strings or files and builds a data structure
 that conforms to the API of the Document Object Model.")
     (home-page "http://search.cpan.org/~tjmather/XML-DOM-1.44/lib/XML/DOM.pm")))
 
+(define-public pugixml
+  (package
+    (name "pugixml")
+    (version "1.6")
+    (source
+     (origin
+      (method url-fetch)
+      (uri (string-append "https://github.com/zeux/pugixml/archive/v"
+                          version ".tar.gz"))
+      (file-name (string-append name "-" version ".tar.gz"))
+      (sha256
+       (base32
+        "0czbcv9aqf2rw3s9cljz2wb1f4zbhd07wnj7ykklklccl0ipfnwi"))))
+    (build-system cmake-build-system)
+    (arguments
+     `(#:tests? #f
+       #:out-of-source? #f
+       #:phases (modify-phases %standard-phases
+                  (add-before
+                   'configure 'chdir
+                   (lambda _
+                     (chdir "scripts")
+                     #t)))))
+    (home-page "http://pugixml.org")
+    (synopsis "Light-weight, simple and fast XML parser for C++ with XPath support")
+    (description
+     "pugixml is a C++ XML processing library, which consists of a DOM-like
+interface with rich traversal/modification capabilities, a fast XML parser
+which constructs the DOM tree from an XML file/buffer, and an XPath 1.0
+implementation for complex data-driven tree queries.  Full Unicode support is
+also available, with Unicode interface variants and conversions between
+different Unicode encodings which happen automatically during
+parsing/saving.")
+    (license license:expat)))
+
 (define-public xmlto
   (package
     (name "xmlto")
-- 
2.2.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0004-gnu-Add-rtmidi.patch --]
[-- Type: text/x-patch, Size: 2675 bytes --]

From 67ec5348f611d58299ae2ab0b8575e817e0f1272 Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Thu, 28 May 2015 09:44:30 +0200
Subject: [PATCH 4/6] gnu: Add rtmidi.

* gnu/packages/audio.scm (rtmidi): New variable.
---
 gnu/packages/audio.scm | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/gnu/packages/audio.scm b/gnu/packages/audio.scm
index fdc783a..3b2d4e1 100644
--- a/gnu/packages/audio.scm
+++ b/gnu/packages/audio.scm
@@ -1031,6 +1031,57 @@ and ALSA.")
 tempo and pitch of an audio recording independently of one another.")
     (license license:gpl2+)))
 
+(define-public rtmidi
+  (package
+    (name "rtmidi")
+    (version "2.1.0")
+    (source (origin
+              (method url-fetch)
+              (uri
+               (string-append "https://github.com/powertab/rtmidi/archive/"
+                              version ".tar.gz"))
+              (file-name (string-append name "-" version ".tar.gz"))
+              (sha256
+               (base32
+                "0d49lapnmdgmjxh4vw57h6xk74nn5r0zwysv7jbd7m8kqhpq5rjj"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:tests? #f ;no "check" target
+       #:phases (modify-phases %standard-phases
+                  (add-before
+                   'configure 'autoconf
+                   (lambda _ (zero? (system* "autoreconf" "-vfi"))))
+                  (add-before
+                   'build 'fix-makefile
+                   (lambda _
+                     (substitute* "Makefile"
+                       (("/bin/ln") "ln")
+                       (("RtMidi.h RtError.h") "RtMidi.h"))
+                     #t))
+                  (add-before
+                   'install 'make-target-dirs
+                   (lambda _
+                     (let ((out (assoc-ref %outputs "out")))
+                       (mkdir-p (string-append out "/bin"))
+                       (mkdir (string-append out "/lib"))
+                       (mkdir (string-append out "/include")))
+                     #t)))))
+    (inputs
+     `(("jack" ,jack-1)
+       ("alsa-lib" ,alsa-lib)))
+    (native-inputs
+     `(("autoconf" ,autoconf)
+       ("automake" ,automake)
+       ("libtool" ,libtool)
+       ("pkg-config" ,pkg-config)))
+    (home-page "https://github.com/powertab/rtmidi")
+    (synopsis "Cross-platform MIDI library for C++")
+    (description
+     "RtMidi is a set of C++ classes (RtMidiIn, RtMidiOut, and API specific
+classes) that provide a common cross-platform API for realtime MIDI
+input/output.")
+    (license license:expat)))
+
 (define-public sratom
   (package
     (name "sratom")
-- 
2.2.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0005-gnu-catch-framework-Update-to-1.1.3.patch --]
[-- Type: text/x-patch, Size: 1528 bytes --]

From ab277b32cd5ace5ab257ec31abd719b2ee2470dd Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Wed, 3 Jun 2015 22:53:56 +0200
Subject: [PATCH 5/6] gnu: catch-framework: Update to 1.1.3.

* gnu/packages/check.scm (catch-framework): Update to 1.1.3.
---
 gnu/packages/check.scm | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/gnu/packages/check.scm b/gnu/packages/check.scm
index 5349ede..e0ee7c4 100644
--- a/gnu/packages/check.scm
+++ b/gnu/packages/check.scm
@@ -113,18 +113,17 @@ supervised tests.")
 (define-public catch-framework
   (package
     (name "catch")
-    (version "1.0.53")                  ;Sub-minor is the build number
+    (version "1.1.3")                  ;Sub-minor is the build number
     (source (origin
               (method git-fetch)
               (uri (git-reference
                     (url "https://github.com/philsquared/Catch")
-                    ;; Semi-arbitrary.  Contains mostly documentation fixes
-                    ;; since build 53.
-                    (commit "b9ec8a1")))
+                    ;; Semi-arbitrary.
+                    (commit "c51e86819d")))
               (file-name (string-append name "-" version))
               (sha256
                (base32
-                "05iijiwjwcjbza7qamwd32d0jypi0lpywmilmmj2xh280mcl4dbd"))))
+                "0kgi7wxxysgjbpisqfj4dj0k19cyyai92f001zi8gzkybd4fkgv5"))))
     (build-system trivial-build-system)
     (arguments
      `(#:modules ((guix build utils))
-- 
2.2.1


[-- Attachment #6: 0006-gnu-Add-powertabeditor.patch --]
[-- Type: text/x-patch, Size: 5642 bytes --]

From 98e2ab304ef345178ab1caad27d6e4412d19c476 Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Thu, 4 Jun 2015 10:01:11 +0200
Subject: [PATCH 6/6] gnu: Add powertabeditor.

* gnu/packages/music.scm (powertabeditor): New variable.
---
 gnu/packages/music.scm | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 96 insertions(+)

diff --git a/gnu/packages/music.scm b/gnu/packages/music.scm
index 5795ecb..a35a113 100644
--- a/gnu/packages/music.scm
+++ b/gnu/packages/music.scm
@@ -22,9 +22,15 @@
   #:use-module (guix download)
   #:use-module ((guix licenses) #:prefix license:)
   #:use-module (guix build-system gnu)
+  #:use-module (guix build-system cmake)
   #:use-module (gnu packages)
   #:use-module (gnu packages audio)
+  #:use-module (gnu packages base) ;libbdf
+  #:use-module (gnu packages boost)
   #:use-module (gnu packages bison)
+  #:use-module (gnu packages code)
+  #:use-module (gnu packages check)
+  #:use-module (gnu packages compression)
   #:use-module (gnu packages docbook)
   #:use-module (gnu packages flex)
   #:use-module (gnu packages fonts)
@@ -45,9 +51,11 @@
   #:use-module (gnu packages perl)
   #:use-module (gnu packages pkg-config)
   #:use-module (gnu packages python)
+  #:use-module (gnu packages qt)
   #:use-module (gnu packages rsync)
   #:use-module (gnu packages texinfo)
   #:use-module (gnu packages texlive)
+  #:use-module (gnu packages web)
   #:use-module (gnu packages xml)
   #:use-module (gnu packages xiph)
   #:use-module (gnu packages zip))
@@ -224,6 +232,94 @@ sessions.  Solfege is also designed to be extensible so you can easily write
 your own lessons.")
     (license license:gpl3+)))
 
+(define-public powertabeditor
+  (package
+    (name "powertabeditor")
+    (version "2.0.0-alpha7")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append
+                    "https://github.com/powertab/powertabeditor/archive/"
+                    version ".tar.gz"))
+              (file-name (string-append name "-" version ".tar.gz"))
+              (sha256
+               (base32
+                "1yp6ck2r72c2pfq31z1kpw1j639rndrifj85l3cbj2kdf8rdzhkk"))))
+    (build-system cmake-build-system)
+    (arguments
+     `(#:tests? #f ; no "check" target
+       #:modules ((guix build cmake-build-system)
+                  (guix build utils)
+                  (ice-9 match))
+       #:configure-flags
+       (list (string-append "-DCMAKE_INSTALL_RPATH='"
+                            (string-join (map (match-lambda
+                                                ((name . directory)
+                                                 (string-append directory "/lib")))
+                                              %build-inputs)
+                                         ";")
+                            "'"))
+       #:phases
+       (modify-phases %standard-phases
+         (add-before
+          'configure 'remove-third-party-libs
+          (lambda _
+            (substitute* "CMakeLists.txt"
+              (("include_directories\\(\\$\\{PROJECT_SOURCE_DIR\\}/external/.*") "")
+              ;; TODO: tests cannot be built:
+              ;; test/test_main.cpp:28:12: error: ‘Session’ is not a member of ‘Catch’
+              (("add_subdirectory\\(test\\)") "")
+              (("add_subdirectory\\(external\\)") ""))
+            (substitute* "source/CMakeLists.txt"
+              (("target_link_libraries\\(powertabeditor")
+               (string-append "target_link_libraries(powertabeditor "
+                              (assoc-ref %build-inputs "binutils")
+                              "/lib/libbfd.a "
+                              (assoc-ref %build-inputs "gcc")
+                              "/lib/libiberty.a "
+                              "dl")))
+            (substitute* "test/CMakeLists.txt"
+              (("include_directories\\(\\$\\{PROJECT_SOURCE_DIR\\}/external/.*") "")
+              (("target_link_libraries\\(pte_tests")
+               (string-append "target_link_libraries(pte_tests "
+                              (assoc-ref %build-inputs "binutils")
+                              "/lib/libbfd.a "
+                              (assoc-ref %build-inputs "gcc")
+                              "/lib/libiberty.a "
+                              "dl")))
+            (delete-file-recursively "external")
+            #t))
+         (add-after
+          'unpack 'add-install-target
+          (lambda _
+            (substitute* "source/CMakeLists.txt"
+              (("qt5_use_modules")
+               "install(TARGETS powertabeditor RUNTIME DESTINATION ${CMAKE_INSTALL_PREFIX}/bin)
+install(FILES data/tunings.json DESTINATION ${CMAKE_INSTALL_PREFIX}/share/powertabeditor/)
+qt5_use_modules"))
+            #t)))))
+    (inputs
+     `(("boost" ,boost)
+       ("alsa-lib" ,alsa-lib)
+       ("qt" ,qt)
+       ("withershins" ,withershins)
+       ("gcc" ,gcc-4.8 "lib") ;for libiberty.a (for withershins)
+       ("binutils" ,binutils) ;for -lbfd and -liberty (for withershins)
+       ("timidity" ,timidity++)
+       ("pugixml" ,pugixml)
+       ("rtmidi" ,rtmidi)
+       ("rapidjson" ,rapidjson)
+       ("zlib" ,zlib)))
+    (native-inputs
+     `(("catch" ,catch-framework)
+       ("pkg-config" ,pkg-config)))
+    (home-page "http://powertabs.net")
+    (synopsis "Guitar tablature editor")
+    (description
+     "PTE2.0 is the successor to the famous Power Tab Editor.  It is
+compatible with PTE1.7 and Guitar Pro.")
+    (license license:gpl3+)))
+
 (define-public tuxguitar
   (package
     (name "tuxguitar")
-- 
2.2.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Add powertabeditor.
  2015-06-06 10:08 [PATCH] Add powertabeditor Ricardo Wurmus
@ 2015-06-09 13:32 ` Ludovic Courtès
  2015-06-12 17:17   ` Ricardo Wurmus
  0 siblings, 1 reply; 4+ messages in thread
From: Ludovic Courtès @ 2015-06-09 13:32 UTC (permalink / raw)
  To: Ricardo Wurmus; +Cc: Guix-devel

Ricardo Wurmus <rekado@elephly.net> skribis:

> From da77c25e8c32243ca2bd77bd76de41312aafaac1 Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <rekado@elephly.net>
> Date: Mon, 25 May 2015 22:13:27 +0200
> Subject: [PATCH 1/6] gnu: Add withershins.
>
> * gnu/packages/code.scm (withershins): New variable.

[...]

> +    (inputs
> +     `(("gcc" ,gcc-4.8 "lib") ;for libiberty.a
> +       ("binutils" ,binutils) ;for libbfd
> +       ("zlib" ,zlib)))
> +    (synopsis "C++11 library for generating stack traces")
> +    (description
> +     "Withershins is a simple cross-platform C++11 library for generating
> +stack traces.")
> +    (license license:expat)))

BFD is GPLv3+ so the whole thing and its users are GPLv3+ once combined.
I guess ‘license’ should be gpl3+, with a comment saying that the source
is Expat?

Otherwise LGTM.

> From d426c47462be1dcc5ff4bbe9d1b154bbbb0761b9 Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <rekado@elephly.net>
> Date: Mon, 25 May 2015 22:14:39 +0200
> Subject: [PATCH 2/6] gnu: Add RapidJSON.
>
> * gnu/packages/web.scm (rapidjson): New variable.

OK.

> From 337f0790e7917f7ae2b394310c8543256756f0fe Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <rekado@elephly.net>
> Date: Thu, 28 May 2015 09:43:53 +0200
> Subject: [PATCH 3/6] gnu: Add pugixml.
>
> * gnu/packages/xml.scm (pugixml): New variable.

OK.

> From 67ec5348f611d58299ae2ab0b8575e817e0f1272 Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <rekado@elephly.net>
> Date: Thu, 28 May 2015 09:44:30 +0200
> Subject: [PATCH 4/6] gnu: Add rtmidi.
>
> * gnu/packages/audio.scm (rtmidi): New variable.

OK.

> +     `(("autoconf" ,autoconf)
> +       ("automake" ,automake)
> +       ("libtool" ,libtool)

Too bad they don’t provide a ‘make dist’-generated tarball.

> From ab277b32cd5ace5ab257ec31abd719b2ee2470dd Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <rekado@elephly.net>
> Date: Wed, 3 Jun 2015 22:53:56 +0200
> Subject: [PATCH 5/6] gnu: catch-framework: Update to 1.1.3.
>
> * gnu/packages/check.scm (catch-framework): Update to 1.1.3.

OK.

> From 98e2ab304ef345178ab1caad27d6e4412d19c476 Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <rekado@elephly.net>
> Date: Thu, 4 Jun 2015 10:01:11 +0200
> Subject: [PATCH 6/6] gnu: Add powertabeditor.
>
> * gnu/packages/music.scm (powertabeditor): New variable.

[...]

> +    (name "powertabeditor")
> +    (version "2.0.0-alpha7")

I suppose the stable version is way too old or non-existent?

> +       #:configure-flags
> +       (list (string-append "-DCMAKE_INSTALL_RPATH='"
> +                            (string-join (map (match-lambda
> +                                                ((name . directory)
> +                                                 (string-append directory "/lib")))
> +                                              %build-inputs)
> +                                         ";")
> +                            "'"))

Could you add a comment explaining why this is needed?  Ideally this
would be handled by ‘cmake-build-system’.

I think the single quotes aren’t needed, are they?  Also, the semicolon
is surprising here.

> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-before
> +          'configure 'remove-third-party-libs
> +          (lambda _
> +            (substitute* "CMakeLists.txt"
> +              (("include_directories\\(\\$\\{PROJECT_SOURCE_DIR\\}/external/.*") "")
> +              ;; TODO: tests cannot be built:
> +              ;; test/test_main.cpp:28:12: error: ‘Session’ is not a member of ‘Catch’
> +              (("add_subdirectory\\(test\\)") "")
> +              (("add_subdirectory\\(external\\)") ""))

Shouldn’t this and...

> +            (delete-file-recursively "external")

... this, and possibly...

> +            #t))
> +         (add-after
> +          'unpack 'add-install-target
> +          (lambda _
> +            (substitute* "source/CMakeLists.txt"
> +              (("qt5_use_modules")
> +               "install(TARGETS powertabeditor RUNTIME DESTINATION ${CMAKE_INSTALL_PREFIX}/bin)
> +install(FILES data/tunings.json DESTINATION ${CMAKE_INSTALL_PREFIX}/share/powertabeditor/)
> +qt5_use_modules"))

... this be done in a snippet?

> +    (description
> +     "PTE2.0 is the successor to the famous Power Tab Editor.  It is
> +compatible with PTE1.7 and Guitar Pro.")

Isn’t “PTE” and “Power Tab Editor” the same thing?  Furthermore, the
package name is ‘powertabeditor’, not ‘pte’.

Otherwise LGTM!

Thanks,
Ludo’.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Add powertabeditor.
  2015-06-09 13:32 ` Ludovic Courtès
@ 2015-06-12 17:17   ` Ricardo Wurmus
  2015-06-12 19:39     ` Ludovic Courtès
  0 siblings, 1 reply; 4+ messages in thread
From: Ricardo Wurmus @ 2015-06-12 17:17 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Guix-devel

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


Ludovic Courtès <ludo@gnu.org> writes:

> Ricardo Wurmus <rekado@elephly.net> skribis:
>
>> From da77c25e8c32243ca2bd77bd76de41312aafaac1 Mon Sep 17 00:00:00 2001
>> From: Ricardo Wurmus <rekado@elephly.net>
>> Date: Mon, 25 May 2015 22:13:27 +0200
>> Subject: [PATCH 1/6] gnu: Add withershins.
>>
>> * gnu/packages/code.scm (withershins): New variable.

[...]

> BFD is GPLv3+ so the whole thing and its users are GPLv3+ once combined.
> I guess ‘license’ should be gpl3+, with a comment saying that the source
> is Expat?

Okay, I added a comment and set the license to gpl3+.

>> From 98e2ab304ef345178ab1caad27d6e4412d19c476 Mon Sep 17 00:00:00 2001
>> From: Ricardo Wurmus <rekado@elephly.net>
>> Date: Thu, 4 Jun 2015 10:01:11 +0200
>> Subject: [PATCH 6/6] gnu: Add powertabeditor.
>>
>> * gnu/packages/music.scm (powertabeditor): New variable.
>
> [...]
>
>> +    (name "powertabeditor")
>> +    (version "2.0.0-alpha7")
>
> I suppose the stable version is way too old or non-existent?

There is no stable version.  Power Tab Editor 2.0 is intended as the
successor to the old Windows-only Power Tab Editor 1.7, whose announced
successor turned out to be vapourware.  PTE2.0 will be the first version
of the independently developed successor.  The latest release is
2.0.0-alpha7.

>> +       #:configure-flags
>> +       (list (string-append "-DCMAKE_INSTALL_RPATH='"
>> +                            (string-join (map (match-lambda
>> +                                                ((name . directory)
>> +                                                 (string-append directory "/lib")))
>> +                                              %build-inputs)
>> +                                         ";")
>> +                            "'"))
>
> Could you add a comment explaining why this is needed?  Ideally this
> would be handled by ‘cmake-build-system’.

It should not be needed in theory, but without it CMake would forget the
RUNPATH by the time the validation is run.  Setting the
CMAKE_INSTALL_RPATH variable explicitly was the only way I could make it
retain the RUNPATH.

> I think the single quotes aren’t needed, are they?  Also, the semicolon
> is surprising here.

You are right.  The semicolon is permitted according to CMake
documentation (I didn't see it mention the more common colon, but it
seems to be permitted as well).

>> +       #:phases
>> +       (modify-phases %standard-phases
>> +         (add-before
>> +          'configure 'remove-third-party-libs
>> +          (lambda _
>> +            (substitute* "CMakeLists.txt"
>> +              (("include_directories\\(\\$\\{PROJECT_SOURCE_DIR\\}/external/.*") "")
>> +              ;; TODO: tests cannot be built:
>> +              ;; test/test_main.cpp:28:12: error: ‘Session’ is not a member of ‘Catch’
>> +              (("add_subdirectory\\(test\\)") "")
>> +              (("add_subdirectory\\(external\\)") ""))
>
> Shouldn’t this and...
>
>> +            (delete-file-recursively "external")
>
> ... this, and possibly...
>
>> +            #t))
>> +         (add-after
>> +          'unpack 'add-install-target
>> +          (lambda _
>> +            (substitute* "source/CMakeLists.txt"
>> +              (("qt5_use_modules")
>> +               "install(TARGETS powertabeditor RUNTIME DESTINATION ${CMAKE_INSTALL_PREFIX}/bin)
>> +install(FILES data/tunings.json DESTINATION ${CMAKE_INSTALL_PREFIX}/share/powertabeditor/)
>> +qt5_use_modules"))
>
> ... this be done in a snippet?

In that case everything I'm doing in the 'remove-third-party-libs phase
(except for the substitutions depending on build inputs) should be done
in a snippet, I think.  The only reason why these substitutions are
required is because we're not using the bundled sources.

>> +    (description
>> +     "PTE2.0 is the successor to the famous Power Tab Editor.  It is
>> +compatible with PTE1.7 and Guitar Pro.")
>
> Isn’t “PTE” and “Power Tab Editor” the same thing?  Furthermore, the
> package name is ‘powertabeditor’, not ‘pte’.

I updated the description.  Attached is a new patch.
Thank you for the review!


[-- Attachment #2: 0001-gnu-Add-powertabeditor.patch --]
[-- Type: text/x-patch, Size: 6037 bytes --]

From 18eaadd221e098dfd769cc6aa9c8920730c2674f Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Thu, 4 Jun 2015 10:01:11 +0200
Subject: [PATCH] gnu: Add powertabeditor.

* gnu/packages/music.scm (powertabeditor): New variable.
---
 gnu/packages/music.scm | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/gnu/packages/music.scm b/gnu/packages/music.scm
index 5795ecb..a42d5c3 100644
--- a/gnu/packages/music.scm
+++ b/gnu/packages/music.scm
@@ -22,9 +22,15 @@
   #:use-module (guix download)
   #:use-module ((guix licenses) #:prefix license:)
   #:use-module (guix build-system gnu)
+  #:use-module (guix build-system cmake)
   #:use-module (gnu packages)
   #:use-module (gnu packages audio)
+  #:use-module (gnu packages base) ;libbdf
+  #:use-module (gnu packages boost)
   #:use-module (gnu packages bison)
+  #:use-module (gnu packages code)
+  #:use-module (gnu packages check)
+  #:use-module (gnu packages compression)
   #:use-module (gnu packages docbook)
   #:use-module (gnu packages flex)
   #:use-module (gnu packages fonts)
@@ -45,9 +51,11 @@
   #:use-module (gnu packages perl)
   #:use-module (gnu packages pkg-config)
   #:use-module (gnu packages python)
+  #:use-module (gnu packages qt)
   #:use-module (gnu packages rsync)
   #:use-module (gnu packages texinfo)
   #:use-module (gnu packages texlive)
+  #:use-module (gnu packages web)
   #:use-module (gnu packages xml)
   #:use-module (gnu packages xiph)
   #:use-module (gnu packages zip))
@@ -224,6 +232,96 @@ sessions.  Solfege is also designed to be extensible so you can easily write
 your own lessons.")
     (license license:gpl3+)))
 
+(define-public powertabeditor
+  (package
+    (name "powertabeditor")
+    (version "2.0.0-alpha7")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append
+                    "https://github.com/powertab/powertabeditor/archive/"
+                    version ".tar.gz"))
+              (file-name (string-append name "-" version ".tar.gz"))
+              (sha256
+               (base32
+                "1yp6ck2r72c2pfq31z1kpw1j639rndrifj85l3cbj2kdf8rdzhkk"))
+              (modules '((guix build utils)))
+              (snippet
+               '(begin
+                  ;; Remove bundled sources for external libraries
+                  (delete-file-recursively "external")
+                  (substitute* "CMakeLists.txt"
+                    (("include_directories\\(\\$\\{PROJECT_SOURCE_DIR\\}/external/.*") "")
+                    ;; TODO: tests cannot be built:
+                    ;; test/test_main.cpp:28:12: error: ‘Session’ is not a member of ‘Catch’
+                    (("add_subdirectory\\(test\\)") "")
+                    (("add_subdirectory\\(external\\)") ""))
+                  (substitute* "test/CMakeLists.txt"
+                    (("include_directories\\(\\$\\{PROJECT_SOURCE_DIR\\}/external/.*") ""))
+
+                  ;; Add install target
+                  (substitute* "source/CMakeLists.txt"
+                    (("qt5_use_modules")
+                     (string-append
+                      "install(TARGETS powertabeditor "
+                      "RUNTIME DESTINATION ${CMAKE_INSTALL_PREFIX}/bin)\n"
+                      "install(FILES data/tunings.json DESTINATION "
+                      "${CMAKE_INSTALL_PREFIX}/share/powertabeditor/)\n"
+                      "qt5_use_modules")))
+                  #t))))
+    (build-system cmake-build-system)
+    (arguments
+     `(#:tests? #f ; no "check" target
+       #:modules ((guix build cmake-build-system)
+                  (guix build utils)
+                  (ice-9 match))
+       #:configure-flags
+       ;; CMake appears to lose the RUNPATH for some reason, so it has to be
+       ;; explicitly set with CMAKE_INSTALL_RPATH.
+       (list (string-append "-DCMAKE_INSTALL_RPATH="
+                            (string-join (map (match-lambda
+                                                ((name . directory)
+                                                 (string-append directory "/lib")))
+                                              %build-inputs) ";")))
+       #:phases
+       (modify-phases %standard-phases
+         (add-before
+          'configure 'remove-third-party-libs
+          (lambda* (#:key inputs #:allow-other-keys)
+            ;; Link with required static libraries, because we're not
+            ;; using the bundled version of withershins.
+            (substitute* '("source/CMakeLists.txt"
+                           "test/CMakeLists.txt")
+              (("target_link_libraries\\((powertabeditor)" _ target)
+               (string-append "target_link_libraries(" target " "
+                              (assoc-ref inputs "binutils")
+                              "/lib/libbfd.a "
+                              (assoc-ref inputs "gcc")
+                              "/lib/libiberty.a "
+                              "dl")))
+            #t)))))
+    (inputs
+     `(("boost" ,boost)
+       ("alsa-lib" ,alsa-lib)
+       ("qt" ,qt)
+       ("withershins" ,withershins)
+       ("gcc" ,gcc-4.8 "lib") ;for libiberty.a (for withershins)
+       ("binutils" ,binutils) ;for -lbfd and -liberty (for withershins)
+       ("timidity" ,timidity++)
+       ("pugixml" ,pugixml)
+       ("rtmidi" ,rtmidi)
+       ("rapidjson" ,rapidjson)
+       ("zlib" ,zlib)))
+    (native-inputs
+     `(("catch" ,catch-framework)
+       ("pkg-config" ,pkg-config)))
+    (home-page "http://powertabs.net")
+    (synopsis "Guitar tablature editor")
+    (description
+     "Power Tab Editor 2.0 is the successor to the famous original Power Tab
+Editor.  It is compatible with Power Tab Editor 1.7 and Guitar Pro.")
+    (license license:gpl3+)))
+
 (define-public tuxguitar
   (package
     (name "tuxguitar")
-- 
2.2.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Add powertabeditor.
  2015-06-12 17:17   ` Ricardo Wurmus
@ 2015-06-12 19:39     ` Ludovic Courtès
  0 siblings, 0 replies; 4+ messages in thread
From: Ludovic Courtès @ 2015-06-12 19:39 UTC (permalink / raw)
  To: Ricardo Wurmus; +Cc: Guix-devel

Ricardo Wurmus <rekado@elephly.net> skribis:

> From 18eaadd221e098dfd769cc6aa9c8920730c2674f Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <rekado@elephly.net>
> Date: Thu, 4 Jun 2015 10:01:11 +0200
> Subject: [PATCH] gnu: Add powertabeditor.
>
> * gnu/packages/music.scm (powertabeditor): New variable.
> ---
>  gnu/packages/music.scm | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 98 insertions(+)

LGTM!

> +       ;; CMake appears to lose the RUNPATH for some reason, so it has to be
> +       ;; explicitly set with CMAKE_INSTALL_RPATH.

When you have time, it would be good to look at the build log to see all
the -rpath arguments that are passed.  It’s a bit scary because it seems
CMake cannot remain under control, somehow.  ;-)

Thank you!

Ludo’.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-06-12 19:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-06 10:08 [PATCH] Add powertabeditor Ricardo Wurmus
2015-06-09 13:32 ` Ludovic Courtès
2015-06-12 17:17   ` Ricardo Wurmus
2015-06-12 19:39     ` Ludovic Courtès

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).