diff mbox

[1/10] Add pjproject.

Message ID 87inu01qsj.fsf@openmailbox.org
State New
Headers show

Commit Message

Lukas Gradl Sept. 13, 2016, 2:23 a.m. UTC
None

Comments

Ricardo Wurmus Sept. 19, 2016, 7:41 a.m. UTC | #1
Hi Lukas,

thanks for the patches!

> From d6a6a5ded95071a58a160a435ccf56d6828148b0 Mon Sep 17 00:00:00 2001
> From: Lukas Gradl <lgradl@openmailbox.org>
> Date: Wed, 20 Jul 2016 21:26:32 -0500
> Subject: [PATCH 01/10] gnu: Add pjproject

> * gnu/packages/telephony.scm (pjproject-sfl): New variable.
> ---
>  gnu/packages/telephony.scm | 181 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 181 insertions(+)

> diff --git a/gnu/packages/telephony.scm b/gnu/packages/telephony.scm
> index d8a33dd..4893dbd 100644
> --- a/gnu/packages/telephony.scm
> +++ b/gnu/packages/telephony.scm
> @@ -23,6 +23,7 @@
 
>  (define-module (gnu packages telephony)
>    #:use-module (gnu packages)
> +  #:use-module (gnu packages audio)
>    #:use-module (gnu packages autotools)
>    #:use-module (gnu packages gnupg)
>    #:use-module (gnu packages linux)
> @@ -34,6 +35,7 @@
>    #:use-module (guix licenses)
>    #:use-module (guix packages)
>    #:use-module (guix download)
> +  #:use-module (guix git-download)
>    #:use-module (guix build-system gnu))
 
>  (define-public commoncpp
> @@ -286,3 +288,182 @@ lists.  All you need to join an existing conference is the host name or IP
>  address of one of the participants.")
>      (home-page "http://holdenc.altervista.org/seren/")
>      (license gpl3+)))
> +
> +(define-public pjproject-sfl
> +  (let ((sfl-patches
> +         (let ((commit "3dbedf53e9cceebb1eed155b5143026f6d253cb8"))
> +           (origin
> +             (method git-fetch)
> +             (uri
> +              (git-reference
> +               (url
> +                "https://gerrit-ring.savoirfairelinux.com/ring-daemon.git")
> +               (commit commit)))
> +             (file-name (string-append "sfl-patches" "-" "0.0.0-1."
> +                                       (string-take commit 7)  "-checkout"))
> +             (modules '((guix build utils)
> +                        (ice-9 ftw)))
> +             (snippet
> +              '(let ((files (scandir "." (lambda (file)
> +                                           (if (or (string=? file ".")
> +                                                   (string=? file ".."))
> +                                               #f
> +                                               #t)))))
> +                 (mkdir-p "sfl-patches")
> +                 (copy-recursively "contrib/src/pjproject/" "sfl-patches")
> +                 (for-each delete-file-recursively files)))

Why is this needed?

> +             (sha256
> +              (base32
> +               "1xnvkv0h24zr1dcmp7djjhqgzvrwic242jy4hb3m5qv71azvcsqg"))))))
> +    (package
> +      (name "pjproject")
> +      (version "2.4")
> +      (source
> +       (origin
> +         (method url-fetch)
> +         (uri (string-append
> +               "http://www.pjsip.org/release/"
> +               version "/" name "-" version ".tar.bz2"))
> +         (modules '((guix build utils)))
> +         (snippet
> +          '(begin
> +           ; The following removes bundled packages except for 'resample'.
> +           ; Pjproject uses some of the source files of resample and one own
> +           ; header (resamplesubs.h) not included with resample to build a
> +           ; shared library.  Upstream resample does not build this
> +           ; library. The license of resample is the LGPL2+
> +           ; The homepage of resample is at:
> +           ; https://ccrma.stanford.edu/~jos/resample/Free_Resampling_Software.html

For line comments like this please use “;;”.  Can we split off the fork
of “resample” into a separate package?  This package definition is
already very big.

> +             (let ((third-party-directories
> +                    (list "BaseClasses" "bdsound" "bin" "g7221" "gsm"
> +                          "ilbc" "lib" "milenage" "mp3" "portaudio"
> +                          "speex" "srtp"  ; Keep only resample, build and
> +                                        ; README.txt.
> +                          "build/baseclasses" "build/g7221" "build/gsm"
> +                          "build/ilbc" "build/milenage" "build/portaudio"
> +                          "build/samplerate" "build/speex" "build/srtp")))
> +                          ; Keep only Makefiles related to resample.
> +               (for-each (lambda (file)
> +                           (delete-file-recursively
> +                            (string-append "third_party/" file)))
> +                         third-party-directories)
> +               #t)
> +             (let ((third-party-dirs
> +                    (list "gsm" "ilbc" "speex" "portaudio"
> +                          "g7221" "srtp")))
> +               (for-each
> +                (lambda (dirs)
> +                  (substitute* "third_party/build/os-linux.mak"
> +                    (((string-append "DIRS += " dirs)) "")))
> +                third-party-dirs))
> +             (substitute* "aconfigure.ac"
> +               (("third_party/build/portaudio/os-auto.mak") ""))))
> +         (sha256
> +          (base32
> +           "0n90n1p41svf23d4fag8jqbjnv82fz14z6zchb8j1kldvap1b00h"))))
> +           ;"13fx7kpf1sswj7r0zl7fqkzg3rl5xz3dl96wdnv15qxfviq5wvq8")))) 2.4.5

Please remove the extra hash.

> +      (build-system gnu-build-system)
> +      (inputs
> +       `(("portaudio" ,portaudio))) ; It might be possible to remove this.

Have you tried? :)

> +      (propagated-inputs ; These packages are referenced in the Libs field of
> +                         ; the pkg-config file that will be installed by
> +                         ; pjproject.

We normally use line comments for longer blocks like this.

> +       `(("speex" ,speex)
> +         ("libsrtp" ,libsrtp)
> +         ("gnutls" ,gnutls)
> +         ("util-linux" ,util-linux)))
> +      (native-inputs
> +       `(("sfl-patches" ,sfl-patches) ; These patches are distributed with the
> +                                      ; libring source.  They contain various
> +                                      ; nontrivial changes such as the use of
> +                                      ; gnutls instead of openssl.
> +         ("autoconf" ,autoconf)
> +         ("automake" ,automake)

Why are the autotools needed?  Is this tarball not bootstrapped?

> +         ("pkg-config" ,pkg-config)
> +         ("libtool" ,libtool)))
> +      (arguments
> +       `(#:test-target
> +         "selftest"

Please put them on the same line.

> +         #:configure-flags
> +         (list "--disable-oss"
> +               "--disable-sound"
> +               "--disable-video"
> +               "--enable-ext-sound"
> +               "--disable-g711-codec"
> +               "--disable-l16-codec"
> +               "--disable-gsm-codec"
> +               "--disable-g722-codec"
> +               "--disable-g7221-codec"
> +               "--disable-ilbc-codec"
> +               "--disable-opencore-amr"
> +               "--disable-sdl"
> +               "--disable-ffmpeg"
> +               "--disable-v4l2"
> +               "--enable-ssl=gnutls"
> +               "--with-external-speex"
> +               "--with-external-pa"
> +               "--with-external-srtp")

Why are so many features disabled?  A comment would be nice.

> +         #:phases
> +         (modify-phases %standard-phases
> +           (add-after 'unpack 'apply-patches
> +             (lambda* (#:key inputs #:allow-other-keys)
> +               (begin

No need for “begin” here.

> +                 (mkdir "patch-dir")
> +                 (system* "tar" "-xvf" (assoc-ref inputs "sfl-patches")
> +                          "-C" "patch-dir" "--strip-components=1")
> +                 (let ((patch-dir
> +                        (string-append "patch-dir" "/sfl-patches"))
> +                       (patches
> +                        '("errno" "endianness" "gnutls" "ipv6" "ice_config"
> +                          "multiple_listeners" "pj_ice_sess")))
> +                   (for-each
> +                    (lambda (file)
> +                      (zero?
> +                       (system* "patch" "--force" "-p1" "-i"
> +                                (string-append patch-dir "/"
> +                                               file ".patch"))))
> +                    patches)))))

Normally, we don’t patch in build phases.  We use the “(patches…)” field
in the source definition instead.  Would this be possible here as well?

> +           (add-before 'build 'build-dep
> +             (lambda _
> +               (zero?
> +                (system* "make" "dep"))))

The lambda can go on one line.  Is this to build the remaining bundled
library?

> +           (add-before 'patch-source-shebangs 'autoconf
> +             (lambda _
> +               (zero?
> +                (system* "autoconf" "-v" "-f" "-i" "-o"
> +                         "aconfigure" "aconfigure.ac"))))

It would be better if we didn’t have to run autoconf.  Is it possible to
avoid it?

> +           (add-before 'autoconf 'disable-some-tests
> +             (lambda _
> +               (substitute* "Makefile"
> +                 (((string-append
> +                    "selftest: "
> +                    "pjlib-test "
> +                    "pjlib-util-test "
> +                    "pjnath-test "  ; This test fails.
> +                    "pjmedia-test "
> +                    "pjsip-test " ; This test fails.
> +                    "pjsua-test")) ; This test fails.  This test would need
> +                                        ; python.

Please don’t use “string-append” here.  You can just break the string
literal.  The comments would go on top then.

> +                  (string-append
> +                   "selftest: "
> +                   "pjlib-test "
> +                   "pjlib-util-test "
> +                   "pjmedia-test")))))
> +           (add-before 'autoconf 'set-flags
> +             (lambda _
> +               (setenv "CFLAGS"
> +                       (string-append
> +                        "-DPJ_ICE_MAX_CAND=32" " "
> +                        "-DPJ_ICE_MAX_CHECKS=150" " "
> +                        "-DPJ_ICE_COMP_BITS=2")))))))

We can pass flags in #:make-flags or #:configure-flags.  That’s better
than using a build phase.

~~ Ricardo
diff mbox

Patch

From d6a6a5ded95071a58a160a435ccf56d6828148b0 Mon Sep 17 00:00:00 2001
From: Lukas Gradl <lgradl@openmailbox.org>
Date: Wed, 20 Jul 2016 21:26:32 -0500
Subject: [PATCH 01/10] gnu: Add pjproject

* gnu/packages/telephony.scm (pjproject-sfl): New variable.
---
 gnu/packages/telephony.scm | 181 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 181 insertions(+)

diff --git a/gnu/packages/telephony.scm b/gnu/packages/telephony.scm
index d8a33dd..4893dbd 100644
--- a/gnu/packages/telephony.scm
+++ b/gnu/packages/telephony.scm
@@ -23,6 +23,7 @@ 
 
 (define-module (gnu packages telephony)
   #:use-module (gnu packages)
+  #:use-module (gnu packages audio)
   #:use-module (gnu packages autotools)
   #:use-module (gnu packages gnupg)
   #:use-module (gnu packages linux)
@@ -34,6 +35,7 @@ 
   #:use-module (guix licenses)
   #:use-module (guix packages)
   #:use-module (guix download)
+  #:use-module (guix git-download)
   #:use-module (guix build-system gnu))
 
 (define-public commoncpp
@@ -286,3 +288,182 @@  lists.  All you need to join an existing conference is the host name or IP
 address of one of the participants.")
     (home-page "http://holdenc.altervista.org/seren/")
     (license gpl3+)))
+
+(define-public pjproject-sfl
+  (let ((sfl-patches
+         (let ((commit "3dbedf53e9cceebb1eed155b5143026f6d253cb8"))
+           (origin
+             (method git-fetch)
+             (uri
+              (git-reference
+               (url
+                "https://gerrit-ring.savoirfairelinux.com/ring-daemon.git")
+               (commit commit)))
+             (file-name (string-append "sfl-patches" "-" "0.0.0-1."
+                                       (string-take commit 7)  "-checkout"))
+             (modules '((guix build utils)
+                        (ice-9 ftw)))
+             (snippet
+              '(let ((files (scandir "." (lambda (file)
+                                           (if (or (string=? file ".")
+                                                   (string=? file ".."))
+                                               #f
+                                               #t)))))
+                 (mkdir-p "sfl-patches")
+                 (copy-recursively "contrib/src/pjproject/" "sfl-patches")
+                 (for-each delete-file-recursively files)))
+             (sha256
+              (base32
+               "1xnvkv0h24zr1dcmp7djjhqgzvrwic242jy4hb3m5qv71azvcsqg"))))))
+    (package
+      (name "pjproject")
+      (version "2.4")
+      (source
+       (origin
+         (method url-fetch)
+         (uri (string-append
+               "http://www.pjsip.org/release/"
+               version "/" name "-" version ".tar.bz2"))
+         (modules '((guix build utils)))
+         (snippet
+          '(begin
+           ; The following removes bundled packages except for 'resample'.
+           ; Pjproject uses some of the source files of resample and one own
+           ; header (resamplesubs.h) not included with resample to build a
+           ; shared library.  Upstream resample does not build this
+           ; library. The license of resample is the LGPL2+
+           ; The homepage of resample is at:
+           ; https://ccrma.stanford.edu/~jos/resample/Free_Resampling_Software.html
+             (let ((third-party-directories
+                    (list "BaseClasses" "bdsound" "bin" "g7221" "gsm"
+                          "ilbc" "lib" "milenage" "mp3" "portaudio"
+                          "speex" "srtp"  ; Keep only resample, build and
+                                        ; README.txt.
+                          "build/baseclasses" "build/g7221" "build/gsm"
+                          "build/ilbc" "build/milenage" "build/portaudio"
+                          "build/samplerate" "build/speex" "build/srtp")))
+                          ; Keep only Makefiles related to resample.
+               (for-each (lambda (file)
+                           (delete-file-recursively
+                            (string-append "third_party/" file)))
+                         third-party-directories)
+               #t)
+             (let ((third-party-dirs
+                    (list "gsm" "ilbc" "speex" "portaudio"
+                          "g7221" "srtp")))
+               (for-each
+                (lambda (dirs)
+                  (substitute* "third_party/build/os-linux.mak"
+                    (((string-append "DIRS += " dirs)) "")))
+                third-party-dirs))
+             (substitute* "aconfigure.ac"
+               (("third_party/build/portaudio/os-auto.mak") ""))))
+         (sha256
+          (base32
+           "0n90n1p41svf23d4fag8jqbjnv82fz14z6zchb8j1kldvap1b00h"))))
+           ;"13fx7kpf1sswj7r0zl7fqkzg3rl5xz3dl96wdnv15qxfviq5wvq8")))) 2.4.5
+      (build-system gnu-build-system)
+      (inputs
+       `(("portaudio" ,portaudio))) ; It might be possible to remove this.
+      (propagated-inputs ; These packages are referenced in the Libs field of
+                         ; the pkg-config file that will be installed by
+                         ; pjproject.
+       `(("speex" ,speex)
+         ("libsrtp" ,libsrtp)
+         ("gnutls" ,gnutls)
+         ("util-linux" ,util-linux)))
+      (native-inputs
+       `(("sfl-patches" ,sfl-patches) ; These patches are distributed with the
+                                      ; libring source.  They contain various
+                                      ; nontrivial changes such as the use of
+                                      ; gnutls instead of openssl.
+         ("autoconf" ,autoconf)
+         ("automake" ,automake)
+         ("pkg-config" ,pkg-config)
+         ("libtool" ,libtool)))
+      (arguments
+       `(#:test-target
+         "selftest"
+         #:configure-flags
+         (list "--disable-oss"
+               "--disable-sound"
+               "--disable-video"
+               "--enable-ext-sound"
+               "--disable-g711-codec"
+               "--disable-l16-codec"
+               "--disable-gsm-codec"
+               "--disable-g722-codec"
+               "--disable-g7221-codec"
+               "--disable-ilbc-codec"
+               "--disable-opencore-amr"
+               "--disable-sdl"
+               "--disable-ffmpeg"
+               "--disable-v4l2"
+               "--enable-ssl=gnutls"
+               "--with-external-speex"
+               "--with-external-pa"
+               "--with-external-srtp")
+         #:phases
+         (modify-phases %standard-phases
+           (add-after 'unpack 'apply-patches
+             (lambda* (#:key inputs #:allow-other-keys)
+               (begin
+                 (mkdir "patch-dir")
+                 (system* "tar" "-xvf" (assoc-ref inputs "sfl-patches")
+                          "-C" "patch-dir" "--strip-components=1")
+                 (let ((patch-dir
+                        (string-append "patch-dir" "/sfl-patches"))
+                       (patches
+                        '("errno" "endianness" "gnutls" "ipv6" "ice_config"
+                          "multiple_listeners" "pj_ice_sess")))
+                   (for-each
+                    (lambda (file)
+                      (zero?
+                       (system* "patch" "--force" "-p1" "-i"
+                                (string-append patch-dir "/"
+                                               file ".patch"))))
+                    patches)))))
+           (add-before 'build 'build-dep
+             (lambda _
+               (zero?
+                (system* "make" "dep"))))
+           (add-before 'patch-source-shebangs 'autoconf
+             (lambda _
+               (zero?
+                (system* "autoconf" "-v" "-f" "-i" "-o"
+                         "aconfigure" "aconfigure.ac"))))
+           (add-before 'autoconf 'disable-some-tests
+             (lambda _
+               (substitute* "Makefile"
+                 (((string-append
+                    "selftest: "
+                    "pjlib-test "
+                    "pjlib-util-test "
+                    "pjnath-test "  ; This test fails.
+                    "pjmedia-test "
+                    "pjsip-test " ; This test fails.
+                    "pjsua-test")) ; This test fails.  This test would need
+                                        ; python.
+                  (string-append
+                   "selftest: "
+                   "pjlib-test "
+                   "pjlib-util-test "
+                   "pjmedia-test")))))
+           (add-before 'autoconf 'set-flags
+             (lambda _
+               (setenv "CFLAGS"
+                       (string-append
+                        "-DPJ_ICE_MAX_CAND=32" " "
+                        "-DPJ_ICE_MAX_CHECKS=150" " "
+                        "-DPJ_ICE_COMP_BITS=2")))))))
+      (home-page "www.pjsip.org")
+      (synopsis "Session Initiation Protocol (SIP) stack")
+      (description "PJProject provides an implementation of the Session
+Initiation Protocol (SIP) and a multimedia framework.
+
+This package is intended for use with libring.  There are several custom
+patches, most notably the use of gnutls instead of openssl for encryption.")
+      (license '(gpl2+ ; pjproject
+                 gpl3+ ; sfl-patches
+                 lgpl2+))))) ; bundled resample
+
-- 
2.9.0