| Message ID | 60c20787457b3c531db6948f2b9773e79214b1f4.1683929239.git.firestone.preston@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#63473] gnu: praat: Update to 6.3.10. | expand |
Hi all, Bumping this patch to hopefully get attention. The praat package currently in the main distribution is broken and has been for several weeks; it would be wonderful if this patch could be applied. Preston
Hi Preston, Am Freitag, dem 12.05.2023 um 17:09 -0500 schrieb Preston M. Firestone: > * gnu/packages/language.scm (praat): Update to 6.3.10. > [arguments]: Use make-flags to set CC to gcc. Add tests and remove > broken > one. Remove final #t from all lambdas. You're mixing functional and style changes in ways that aren't really necessary. IMHO, it's better to split the patch in two. Either way, separate this log into one log per argument, i.e. one for #:tests?, one for #:make-flags and one for #:phases. “Remove final #t…” ought to be “Drop trailing #t.” > [inputs]: Update to gtk+3 and reformat. Should be “Replace gtk+-2 with gtk+.” > --- > gnu/packages/language.scm | 34 +++++++++++++++++++++------------- > 1 file changed, 21 insertions(+), 13 deletions(-) > > diff --git a/gnu/packages/language.scm b/gnu/packages/language.scm > index 36968ad11c..530842d2c8 100644 > --- a/gnu/packages/language.scm > +++ b/gnu/packages/language.scm > @@ -850,37 +850,45 @@ (define-public link-grammar > (define-public praat > (package > (name "praat") > - (version "6.1.30") > + (version "6.3.10") > (source (origin > (method git-fetch) > (uri (git-reference > - (url "https://github.com/praat/praat") > - (commit (string-append "v" version)))) > + (url "https://github.com/praat/praat") > + (commit (string-append "v" version)))) > (file-name (git-file-name name version)) > (sha256 > (base32 > - > "1pjfifyv3wjn68l3i2dr83xm75nf2kxvfxrk9qqbmwz58p183jw4")))) > + > "0kwv0p2bn2x5h0c61rymm87icqqwnbj699awgc5afl4qp53azci8")))) > (build-system gnu-build-system) > (arguments > - `(#:tests? #f ; no test target > + '(#:make-flags > + ;; For some reason this variable doesn't properly percolate > down to the > + ;; recursive subcalls of make despite being defined in > makefile.defs, > + ;; which is sourced by all the other makefiles. Setting it as > a flag > + ;; guarantees that the right compiler is called every time. > + '("-E" "CC = gcc") You should use (cc-for-target) as CC here. Don't bother with the spaces between the variable and it's values, those are more likely to introduce faults than to prevent them. > #:phases > (modify-phases %standard-phases > + (add-after 'unpack 'remove-problem-tests > + (lambda _ > + ;; FIXME: Why doesn't this test pass? > + (delete-file "test/sys/graphicsText.praat"))) The canonical name for such a phase is ‘delete-failing-tests’ > (replace 'configure > (lambda _ > - (copy-file "makefiles/makefile.defs.linux.pulse" > "makefile.defs") > - #t)) > + (copy-file "makefiles/makefile.defs.linux.pulse" > "makefile.defs"))) > + (replace 'check > + (lambda _ > + ;; Run only the tests that don't require a GUI. > + (invoke "./praat" "--run" > "test/runAllTests_batch.praat"))) > (replace 'install > (lambda* (#:key outputs #:allow-other-keys) > (let* ((out (assoc-ref outputs "out")) > (bin (string-append out "/bin"))) > (mkdir-p bin) > - (copy-file "praat" (string-append bin "/praat"))) > - #t))))) > + (copy-file "praat" (string-append bin > "/praat")))))))) > (inputs > - `(("alsa-lib" ,alsa-lib) > - ("gtk" ,gtk+-2) > - ("jack" ,jack-1) > - ("publesaudio" ,pulseaudio))) > + (list alsa-lib gtk+ jack-1 pulseaudio)) > (native-inputs > (list pkg-config)) > (home-page "https://www.fon.hum.uva.nl/praat/") Cheers PS: Sorry for the second mail, I misspelled debbugs on the first.
Hi Preston, Would it be possible to split some of these changes into separate patches? For example, rewriting the style of the inputs field and removing the #t's could be in their own patch. Ideally any changes not strictly related to the update should be in a separate patch from the changes strictly necessary for the update, and each patch should contain one logically-related set of changes. Related, were you able to fix the build of the old version? If so, could you commit that fix as the first in a patch series? > + ;; FIXME: Why doesn't this test pass? It seems like this is answered later by > + ;; Run only the tests that don't require a GUI. and if that's the case the first comment could be changed. Also, you can get X during build with xorg-server-for-tests as a native-input and a build stage like this one from the geary package defintion: ``` (add-before 'check 'setup-xvfb (lambda _ (system "Xvfb :1 &") (setenv "DISPLAY" ":1"))) ``` While I don't know that such an addition will resolve the failing test, it might be worth trying. Everything else seems to look fine. You may want to run guix style -S arguments to port it to g-expressions while you're here, but I don't think that's strictly necessary, just preferred. All the best, Juli
Hi Juli and Liliana, Thank you for the feedback and help. I have reformatted the patch as a series. Let me know whether there's anything else that needs fixing. Thanks, Preston
Hi Preston, Preston Miller Firestone <firestone.preston@gmail.com> skribis: > Thank you for the feedback and help. I have reformatted the patch as a > series. Let me know whether there's anything else that needs fixing. Finally applied, thank you! Ludo’.
diff --git a/gnu/packages/language.scm b/gnu/packages/language.scm index 36968ad11c..530842d2c8 100644 --- a/gnu/packages/language.scm +++ b/gnu/packages/language.scm @@ -850,37 +850,45 @@ (define-public link-grammar (define-public praat (package (name "praat") - (version "6.1.30") + (version "6.3.10") (source (origin (method git-fetch) (uri (git-reference - (url "https://github.com/praat/praat") - (commit (string-append "v" version)))) + (url "https://github.com/praat/praat") + (commit (string-append "v" version)))) (file-name (git-file-name name version)) (sha256 (base32 - "1pjfifyv3wjn68l3i2dr83xm75nf2kxvfxrk9qqbmwz58p183jw4")))) + "0kwv0p2bn2x5h0c61rymm87icqqwnbj699awgc5afl4qp53azci8")))) (build-system gnu-build-system) (arguments - `(#:tests? #f ; no test target + '(#:make-flags + ;; For some reason this variable doesn't properly percolate down to the + ;; recursive subcalls of make despite being defined in makefile.defs, + ;; which is sourced by all the other makefiles. Setting it as a flag + ;; guarantees that the right compiler is called every time. + '("-E" "CC = gcc") #:phases (modify-phases %standard-phases + (add-after 'unpack 'remove-problem-tests + (lambda _ + ;; FIXME: Why doesn't this test pass? + (delete-file "test/sys/graphicsText.praat"))) (replace 'configure (lambda _ - (copy-file "makefiles/makefile.defs.linux.pulse" "makefile.defs") - #t)) + (copy-file "makefiles/makefile.defs.linux.pulse" "makefile.defs"))) + (replace 'check + (lambda _ + ;; Run only the tests that don't require a GUI. + (invoke "./praat" "--run" "test/runAllTests_batch.praat"))) (replace 'install (lambda* (#:key outputs #:allow-other-keys) (let* ((out (assoc-ref outputs "out")) (bin (string-append out "/bin"))) (mkdir-p bin) - (copy-file "praat" (string-append bin "/praat"))) - #t))))) + (copy-file "praat" (string-append bin "/praat")))))))) (inputs - `(("alsa-lib" ,alsa-lib) - ("gtk" ,gtk+-2) - ("jack" ,jack-1) - ("publesaudio" ,pulseaudio))) + (list alsa-lib gtk+ jack-1 pulseaudio)) (native-inputs (list pkg-config)) (home-page "https://www.fon.hum.uva.nl/praat/")