Message ID | eb560602d63a60bd6a541cd682b5bea50f2ed8c6.1708632642.git.juli@incana.org |
---|---|
State | New |
Headers | show |
Series | [bug#69313] gnu: tlpui: Fix broken package. | expand |
Hello, Juliana Sims <juli@incana.org> writes: > * gnu/packages/linux.scm (tlpui): Fix broken package. Thank you. I think the commit message could be expounded a bit, e.g., * gnu/packages/linux.scm (tlpui)[arguments]: Fix location for "defaults.conf". > (add-after 'unpack 'set-absolute-locations > - (lambda* (#:key inputs #:allow-other-keys) > - (let ((defaults.conf > - (search-input-file inputs "/share/tlp/defaults.conf")) > - (lspci (search-input-file inputs "/bin/lspci")) > - (lsusb (search-input-file inputs "/bin/lsusb")) > - (tlp-stat (search-input-file inputs "/bin/tlp-stat"))) > + (lambda _ > + (let ((defaults.conf #$(file-append tlp "/share/tlp/defaults.conf")) > + (lspci #$(file-append pciutils "/bin/lspci")) > + (lsusb #$(file-append usbutils "/bin/lsusb")) > + (tlp-stat #$(file-append tlp "/bin/tlp-stat"))) I'm a bit puzzled here: how does (search-input-file inputs "/share/tlp/defaults.conf") differ from #$(file-append tlp "/share/tlp/defaults.conf") ? And how does this relate to other changes in the patch (lsusb and lspci)? > (with-directory-excursion "tlpui" > (substitute* '("file.py" "settingshelper.py" "statui.py") > (("\"tlp-stat\"") > (string-append "'" tlp-stat "'")) > (("/usr/share/tlp/defaults.conf") > - (string-append "'" defaults.conf "'"))) > + defaults.conf)) OOC, would this single line suffice to fix the issue? On my side, this seems to be the case. Regards,
Juliana Sims <juli@incana.org> writes: >> how does this relate to other changes in the patch (lsusb and lspci)? > > It doesn't! I meant to split these into separate commits but was very tired. > Don't code sleepy, kids! :) >> I'm a bit puzzled here: how does >> >> (search-input-file inputs "/share/tlp/defaults.conf") >> >> differ from >> >> #$(file-append tlp "/share/tlp/defaults.conf") > > The latter directly accesses the input in question then joins its path with the > provided string and inserts the result where the `file-append` form was in the > code. The former searches each input's store directory at build time to find a > matching file. In other words, since we know exactly where to find these files > ahead of time, we simply tell the build dæmon where they are rather than making > it look for them. > > Because this is a quite minor performance improvement and code modernization, > rather than a requirement for the package to build, I've split it into a second > patch to be applied at the discretion of a commiter. I understand the performance improvement, but I'm dubious about the "code modernization" part. I've been out of the loop for a while, but I think using `search-input-file' is still the way to go. IIUC, the `file-append' way makes it more difficult to use package transformations, since you basically bind the executables to a fixed package. I let the issue open for you and others to comment about this. Meanwhile, I applied the first patch. tlpui, even outdated, now builds again! Thanks!
> I'm dubious about the "code modernization" part. ... IIUC, the > `file-append' way makes it more difficult to use package > transformations, since you basically bind the executables to a fixed > package. I used the phrase "modernization" just because `file-append` is a g-expression-related procedure, and my understanding is that gexprs are the new way to do things. I hadn't considered the fixing-to-specific-packages aspect. In theory one could bind a replacement package to the same name in the scope of g-expression expansion (package parameterization, for example, will enable this more simply if and when it's done), but that's kind of hacky atm and shouldn't be a burden on anyone inheriting from this package. I now understand why one might prefer `search-input-file` and, furthermore, think it should be preserved. If no one else has input on this in the next couple days, I'll try to remember to close this issue soon :) Thanks for helping me think about something in a new way! -Juli
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm index 8eb6d9b7d3..3aa02345b4 100644 --- a/gnu/packages/linux.scm +++ b/gnu/packages/linux.scm @@ -7868,18 +7868,17 @@ (define-public tlpui (substitute* "setup.py" (("/usr/") "")))) (add-after 'unpack 'set-absolute-locations - (lambda* (#:key inputs #:allow-other-keys) - (let ((defaults.conf - (search-input-file inputs "/share/tlp/defaults.conf")) - (lspci (search-input-file inputs "/bin/lspci")) - (lsusb (search-input-file inputs "/bin/lsusb")) - (tlp-stat (search-input-file inputs "/bin/tlp-stat"))) + (lambda _ + (let ((defaults.conf #$(file-append tlp "/share/tlp/defaults.conf")) + (lspci #$(file-append pciutils "/bin/lspci")) + (lsusb #$(file-append usbutils "/bin/lsusb")) + (tlp-stat #$(file-append tlp "/bin/tlp-stat"))) (with-directory-excursion "tlpui" (substitute* '("file.py" "settingshelper.py" "statui.py") (("\"tlp-stat\"") (string-append "'" tlp-stat "'")) (("/usr/share/tlp/defaults.conf") - (string-append "'" defaults.conf "'"))) + defaults.conf)) (substitute* "ui_config_objects/gtkusblist.py" (("\"lsusb\"") (string-append "'" lsusb "'")))