Hi, Ricardo Wurmus writes: > ng0 writes: > >> From ac578d27529cc2a5f39f66054b5991e44e65f0b9 Mon Sep 17 00:00:00 2001 >> From: ng0 >> Date: Tue, 9 Aug 2016 16:47:37 +0000 >> Subject: [PATCH] gnu: Add cbatticon. > >> * gnu/packages/admin.scm (cbatticon): New variable. >> --- >> gnu/packages/admin.scm | 46 +++++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 45 insertions(+), 1 deletion(-) ... >> + #:phases >> + (modify-phases %standard-phases >> + (delete 'configure) ; no configure script >> + (add-before 'build 'patch-paths-in-Makefile >> + (lambda* (#:key outputs #:allow-other-keys) >> + (lambda _ > > Why is this a lambda inside of another lambda? This means that the > substitution really doesn’t happen at build time. This build phase only > returns a function and then moves on. whoa.. okay I should've waited a day and review it myself before I send it. this is horrible work i've done, the only reason for this quality is that I've had anesthesia in hospital 2.5 hours before I wrote this package... Lesson learned. >> + (substitute* "Makefile" >> + (("msgfmt") (which "msgfmt")) >> + (("RM = rm -f") >> + (string-append "RM = " (which "rm") " -f"))))))))) > > These substitutions don’t seem necessary to me. (Considering that this > doesn’t get executed due to the nested lambda, maybe this is really not > needed.) > >> + (propagated-inputs >> + `(("libnotify" ,libnotify))) > > Why is this propagated? This shouldn’t be needed. Propagation is best > avoided. I was convinced this is needed as a runtime dependency and that propagated-inputs is equal to runtime dependencies as they are installed with the package. But you are right, it is not needed to be propagated. > Okay. Could you please send an updated patch? Thanks! > > ~~ Ricardo >