diff mbox series

[bug#69313] gnu: tlpui: Fix broken package.

Message ID eb560602d63a60bd6a541cd682b5bea50f2ed8c6.1708632642.git.juli@incana.org
State New
Headers show
Series [bug#69313] gnu: tlpui: Fix broken package. | expand

Commit Message

Juliana Sims Feb. 22, 2024, 8:10 p.m. UTC 
Hello,

Before this patch, tlpui failed to launch with an error about failing to find
defaults.conf. This is a relatively simple patch to fix that.

Note that tlpui is also somewhat outdated. I started in on updating it, but
several dependencies were updated beyond what we have in Guix and I didn't feel
like navigating that and the changes in build and testing in the later releases.

Thanks,
Juli

* gnu/packages/linux.scm (tlpui): Fix broken package.

Change-Id: I451ba405fb6287b76e449dc6a63718dd36f623ff
---
 gnu/packages/linux.scm | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)


base-commit: c97de01740ad336efba5830def0907f3daa9c0b8

Comments

Nicolas Goaziou Feb. 22, 2024, 10:07 p.m. UTC | #1 
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,
Nicolas Goaziou Feb. 23, 2024, 7:37 a.m. UTC | #2 
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!
Juliana Sims Feb. 27, 2024, 5:10 p.m. UTC | #3 
> 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 mbox series

Patch

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 "'")))