From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lukas Gradl Subject: Re: [Patch 1/10] Add pjproject. Date: Tue, 20 Sep 2016 00:39:07 -0500 Message-ID: <87oa3j870k.fsf@openmailbox.org> References: <87mvjc1quq.fsf@openmailbox.org> <87inu01qsj.fsf@openmailbox.org> <87shswmj48.fsf@elephly.net> Mime-Version: 1.0 Content-Type: multipart/signed; boundary="==-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:45274) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bmDmL-0006l4-B8 for guix-devel@gnu.org; Tue, 20 Sep 2016 01:39:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bmDmG-0007RI-RA for guix-devel@gnu.org; Tue, 20 Sep 2016 01:39:36 -0400 Received: from smtp23.openmailbox.org ([62.4.1.57]:60819 helo=smtp8.openmailbox.org) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bmDmG-0007Qw-A9 for guix-devel@gnu.org; Tue, 20 Sep 2016 01:39:32 -0400 In-Reply-To: <87shswmj48.fsf@elephly.net> (Ricardo Wurmus's message of "Mon, 19 Sep 2016 09:41:43 +0200") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Ricardo Wurmus Cc: guix-devel@gnu.org --==-=-= Content-Type: multipart/mixed; boundary="=-=-=" --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi Ricardo! Thank you for your review!=20=20 Ricardo Wurmus writes: >> From d6a6a5ded95071a58a160a435ccf56d6828148b0 Mon Sep 17 00:00:00 2001 >> From: Lukas Gradl >> 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 @@ >=20=20 >> (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)) >=20=20 >> (define-public commoncpp >> @@ -286,3 +288,182 @@ lists. All you need to join an existing conferenc= e 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.g= it") >> + (commit commit))) >> + (file-name (string-append "sfl-patches" "-" "0.0.0-1." >> + (string-take commit 7) "-checko= ut")) >> + (modules '((guix build utils) >> + (ice-9 ftw))) >> + (snippet >> + '(let ((files (scandir "." (lambda (file) >> + (if (or (string=3D? file ".") >> + (string=3D? file "..= ")) >> + #f >> + #t))))) >> + (mkdir-p "sfl-patches") >> + (copy-recursively "contrib/src/pjproject/" "sfl-patche= s") >> + (for-each delete-file-recursively files))) > > Why is this needed? My idea here is the following: The source tree downloaded here contains the complete source for libring, which includes the patches to pjproject. In this snippet I try to get rid of all the other libring files that are not needed and only keep the patches to pjproject. Then I try to make the directory tree that contains these patches as shallow as possible. I remember I had some problems with copying them to "." but trying again now it works. I attached an updated patch that does this. > >> + (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 'resampl= e'. >> + ; Pjproject uses some of the source files of resample and on= e own >> + ; header (resamplesubs.h) not included with resample to buil= d 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_S= oftware.html > > For line comments like this please use =E2=80=9C;;=E2=80=9D. Done. > Can we split off the fork of =E2=80=9Cresample=E2=80=9D into a separate p= ackage? > This package definition is already very big. I tried this a few weeks (months?) ago. The biggest roadblocks were the missing header added by pjproject and some dependencies of 'resample'. I will look into it again. > >> + (let ((third-party-directories >> + (list "BaseClasses" "bdsound" "bin" "g7221" "gsm" >> + "ilbc" "lib" "milenage" "mp3" "portaudio" >> + "speex" "srtp" ; Keep only resample, build a= nd >> + ; README.txt. >> + "build/baseclasses" "build/g7221" "build/gsm" >> + "build/ilbc" "build/milenage" "build/portaudi= o" >> + "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 +=3D " 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. This is a leftover from a previous attempt at a different version. I am very sorry about this. I will be more careful next time before submitting.=20 >> + (build-system gnu-build-system) >> + (inputs >> + `(("portaudio" ,portaudio))) ; It might be possible to remove th= is. > > Have you tried? :) > Yes, but so far without success. Sorry that this comment was still there. The configure phase checks for portaudio and fails if it is not available. I had a vague hunch at the time that portaudio is actually not being used although configure fails if it can not find it. I based this off the observations that there is no store reference to portaudio and that a lot of flags in the build log seem to disable portaudio. I will look more into this but I think that most likely portaudio can not be omitted. >> + (propagated-inputs ; These packages are referenced in the Libs fi= eld of >> + ; the pkg-config file that will be installed by >> + ; pjproject. > > We normally use line comments for longer blocks like this. OK, fixed. >> + `(("speex" ,speex) >> + ("libsrtp" ,libsrtp) >> + ("gnutls" ,gnutls) >> + ("util-linux" ,util-linux))) >> + (native-inputs >> + `(("sfl-patches" ,sfl-patches) ; These patches are distributed w= ith the >> + ; libring source. They contain v= arious >> + ; 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? Some of the patches introduce changes to '.in' and '.ac' files and the snippet changes "aconfigure.ac", so I think it is necessary to run autoconf to make sure these changes will be used. >> + ("pkg-config" ,pkg-config) >> + ("libtool" ,libtool))) >> + (arguments >> + `(#:test-target >> + "selftest" > > Please put them on the same line. Sure, Sorry about that. >> + #: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=3Dgnutls" >> + "--with-external-speex" >> + "--with-external-pa" >> + "--with-external-srtp") > > Why are so many features disabled? A comment would be nice. The version that is bundled with libring had these flags, so I think ring will probably not use them. I can try to enable more of these. >> + #:phases >> + (modify-phases %standard-phases >> + (add-after 'unpack 'apply-patches >> + (lambda* (#:key inputs #:allow-other-keys) >> + (begin > > No need for =E2=80=9Cbegin=E2=80=9D here. OK. I changed this section a little to work with the patches sitting in the top level of the 'sfl-patches' tarball instead of the 'sfl-patches' sub-directory. > >> + (mkdir "patch-dir") >> + (system* "tar" "-xvf" (assoc-ref inputs "sfl-patches") >> + "-C" "patch-dir" "--strip-components=3D1") >> + (let ((patch-dir >> + (string-append "patch-dir" "/sfl-patches")) >> + (patches >> + '("errno" "endianness" "gnutls" "ipv6" "ice_con= fig" >> + "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=E2=80=99t patch in build phases. We use the =E2=80=9C(p= atches=E2=80=A6)=E2=80=9D field > in the source definition instead. Would this be possible here as > well? I think it should, but I was not able to get it to work. I tried "(patches (list sfl-patches))" because I think that the 'patches' field should accept origin objects, however this just yielded the following: =2D--8<--- cut here -------------------- start --->8--- patch unexpectedly ends in middle of line /gnu/store/ni491r4ffm03v0cr70df12lwiq826das-patch-2.7.5/bin/patch: **** Onl= y garbage was found in the patch input. source is under 'pjproject-2.4' applying '/gnu/store/mjh1a1fjlz53psz4qxx0kh09qmbqj0jc-sfl-patches-0.0.0-1.t= ar.xz'... builder for `/gnu/store/kb35an4z14adc0kn592dawxqlq6dzyhd-pjproject-2.4.tar.= xz.drv' failed to produce output path `/gnu/store/lhrw7zwcxgginsi4w1pfgrkhp= ims1rza-pjproject-2.4.tar.xz' =2D--8<--- cut here -------------------- end ----->8--- It seems like it is trying to apply the tarball as a patch, so it is probably not as easy as "(patches (list sfl-patches))". Are there any packages that use an 'origin' object in the 'patches' field? Grepping with grep -r "(patches " gnu/packages | grep -v "search-patch" does not yield any results that were helpful to me. >> + (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? Yes, that is right. >> + (add-before 'patch-source-shebangs 'autoconf >> + (lambda _ >> + (zero? >> + (system* "autoconf" "-v" "-f" "-i" "-o" >> + "aconfigure" "aconfigure.ac")))) > > It would be better if we didn=E2=80=99t have to run autoconf. Is it poss= ible to > avoid it? The file "aconfigure.ac" gets changed by the patch "gnutls.patch", but that patch also changes "aconfigure", so it might not need to rerun autoconf. However, I also change this file in a snippet (to disable the requirement of building the third-party directory), so running autoconf can not be avoided here. > >> + (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=E2=80=99t use =E2=80=9Cstring-append=E2=80=9D here. You can j= ust break the string > literal. The comments would go on top then. OK, done. > >> + (string-append >> + "selftest: " >> + "pjlib-test " >> + "pjlib-util-test " >> + "pjmedia-test"))))) >> + (add-before 'autoconf 'set-flags >> + (lambda _ >> + (setenv "CFLAGS" >> + (string-append >> + "-DPJ_ICE_MAX_CAND=3D32" " " >> + "-DPJ_ICE_MAX_CHECKS=3D150" " " >> + "-DPJ_ICE_COMP_BITS=3D2"))))))) > > We can pass flags in #:make-flags or #:configure-flags. That=E2=80=99s b= etter > than using a build phase. Done. Sorry about that. I remember having problems with this but it worked just now. > ~~ Ricardo > Thank you for your detailed review! I apologize for the high number of trivial issues! I will review my own work more careful next time before submitting. The attached patch does not yet address the following: =2D Separate package for resample. =2D Apply patches in the (patches ...) field instead of a build phase. =2D Maybe remove portaudio from inputs. I will work on all of these, but I think some of these items might take some time. Thank you! Best, Lukas --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-gnu-Add-pjproject.patch Content-Transfer-Encoding: quoted-printable From=20060f49eb22788ee5a331ae12c5094066b341efcd Mon Sep 17 00:00:00 2001 From: Lukas Gradl Date: Wed, 20 Jul 2016 21:26:32 -0500 Subject: [PATCH] gnu: Add pjproject * gnu/packages/telephony.scm (pjproject-sfl): New variable. =2D-- gnu/packages/telephony.scm | 160 +++++++++++++++++++++++++++++++++++++++++= ++++ 1 file changed, 160 insertions(+) diff --git a/gnu/packages/telephony.scm b/gnu/packages/telephony.scm index d8a33dd..ce2034a 100644 =2D-- a/gnu/packages/telephony.scm +++ b/gnu/packages/telephony.scm @@ -23,6 +23,7 @@ =20 (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)) =20 (define-public commoncpp @@ -286,3 +288,161 @@ lists. All you need to join an existing conference i= s 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=3D? file ".") + (string=3D? file "..")) + #f + #t))))) + (copy-recursively "contrib/src/pjproject/" ".") + (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_Sof= tware.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 +=3D " dirs)) ""))) + third-party-dirs)) + (substitute* "aconfigure.ac" + (("third_party/build/portaudio/os-auto.mak") "")))) + (sha256 + (base32 + "0n90n1p41svf23d4fag8jqbjnv82fz14z6zchb8j1kldvap1b00h")))) + (build-system gnu-build-system) + (inputs + `(("portaudio" ,portaudio))) + (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 ;; The disabled features are not used by libri= ng. + (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=3Dgnutls" + "--with-external-speex" + "--with-external-pa" + "--with-external-srtp" + "CFLAGS=3D-DPJ_ICE_MAX_CAND=3D32 -DPJ_ICE_MAX_CHECKS=3D150 \ +-DPJ_ICE_COMP_BITS=3D2") + #:phases + (modify-phases %standard-phases + (add-after 'unpack 'apply-patches + (lambda* (#:key inputs #:allow-other-keys) + (let ((patch-dir "patch-dir") + (patches + '("errno" "endianness" "gnutls" "ipv6" "ice_config" + "multiple_listeners" "pj_ice_sess"))) + (mkdir patch-dir) + (system* "tar" "-xvf" (assoc-ref inputs "sfl-patches") + "-C" patch-dir "--strip-components=3D1") + (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 + ;; Three of the six test programs fail due to missing network + ;; access. + (lambda _ + (substitute* "Makefile" + (("selftest: pjlib-test pjlib-util-test pjnath-test \ +pjmedia-test pjsip-test pjsua-test") + "selftest: pjlib-test pjlib-util-test pjmedia-test")))))= )) + (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 + =2D-=20 2.9.0 --=-=-=-- --==-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJX4Mt9AAoJEFP7dyh+2DCRp54QALxtDzoSH04J42uvhfzOr0C/ rUBx0IObRcluAJlEflwPkKNviJhUIb0awN1H4ih0UxtGo5f7ydN/suA6JAzGB6sa Na3lYWs8jdrc8bqyOloCZn1SA7K3+33FPMgimp20Q6vaa9vrej9OsX/RXgGwIXts EIMJwuIryh69BiegBC50uuZQtAYA6eb29sIq465OSXFCl22ZzFFYULOBHQw8hz+8 2G7y2osje9HnmwHRDz2WzbC6aoFAXIyVGpthtwhbMvCLwmTe0RAgAGyiyt4wdtiP ydGYxwMJHL10a10TlsaJzsq6Vc3PPWEvGsoWe+TmZSUINavc9iAiIdlQCsSbyEQZ d+nHIG5iGn8NuLekmKSXil+ka5Ldismnnuo3WQ5KCwaconeFvVPD2CkfY1MidQny fGpFGCibnwhDJaDZFiULzn/xC+uMLyHfaNIzMDqH6eAZgEa6yq+c1VdA13kgh07J 2xpvJtQBekXfimgSFKxkoBp/lGBTCzvyLDhnCVAT9EjVrGKbGSm3hGV9dmRw0qmV UFS5zPIytsd9zKuPMseR2ovjPWThO2r+jM1e5M+9ehapUS4/4icuIao0Wp1tWS3S e8S+WeJ1bKS6baNOHedSqUF+hSlTIO8OvuVEp3sUsm7TJslm+Ls4g17qvgPuht2y ROJGW2KbY3HS7Hu9yK9f =BBfC -----END PGP SIGNATURE----- --==-=-=--