GNU bug report logs - #69313
[PATCH] gnu: tlpui: Fix broken package.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 69313 in the body.
You can then email your comments to 69313 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Juliana Sims <juli <at> incana.org>
To: guix-patches <at> gnu.org
Cc: Juliana Sims <juli <at> incana.org>
Subject: [PATCH] gnu: tlpui: Fix broken package.
Date: Thu, 22 Feb 2024 15:10:42 -0500

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(-)

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

base-commit: c97de01740ad336efba5830def0907f3daa9c0b8
-- 
2.41.0

Message #8 received at 69313 <at> debbugs.gnu.org (full text, mbox):

From: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
To: Juliana Sims <juli <at> incana.org>
Cc: 69313 <at> debbugs.gnu.org, Tobias Geerinckx-Rice <me <at> tobias.gr>,
 Wilko Meyer <w <at> wmeyer.eu>, Leo Famulari <leo <at> famulari.name>
Subject: Re: [bug#69313] [PATCH] gnu: tlpui: Fix broken package.
Date: Thu, 22 Feb 2024 23:07:25 +0100

Hello,

Juliana Sims <juli <at> 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

Message #11 received at 69313 <at> debbugs.gnu.org (full text, mbox):

From: Juliana Sims <juli <at> incana.org>
To: 69313 <at> debbugs.gnu.org
Cc: Juliana Sims <juli <at> incana.org>
Subject: [PATCH v2 2/2] gnu: tlpui: Reference inputs directly during build.
Date: Thu, 22 Feb 2024 17:56:59 -0500

* gnu/packages/linux.scm (tlpui)[arguments]: Reference inputs directly during
build.

Change-Id: I4c71edff805b700ab3f07ecf88ca633081d75c21
---
 gnu/packages/linux.scm | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index ef225479ca..3aa02345b4 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -7868,12 +7868,11 @@ (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\"")
-- 
2.41.0

Message #14 received at 69313 <at> debbugs.gnu.org (full text, mbox):

From: Juliana Sims <juli <at> incana.org>
To: 69313 <at> debbugs.gnu.org
Cc: Juliana Sims <juli <at> incana.org>
Subject: [PATCH v2 0/2] gnu: tlpui: Fix location for "defaults.conf".
Date: Thu, 22 Feb 2024 17:56:57 -0500

> 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 think the commit message could be expounded a bit, e.g.,
>
>   * gnu/packages/linux.scm (tlpui)[arguments]: Fix location for "defaults.conf".

Good idea! I've modified the commit messages to be more precise :)

Thanks,
Juli

Juliana Sims (2):
  gnu: tlpui: Fix location for "defaults.conf".
  gnu: tlpui: Reference inputs directly during build.

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


base-commit: 34ce59bb0668e75d40f42b89e36dc9af6f548e4c
-- 
2.41.0

Message #17 received at 69313 <at> debbugs.gnu.org (full text, mbox):

From: Juliana Sims <juli <at> incana.org>
To: 69313 <at> debbugs.gnu.org
Cc: Juliana Sims <juli <at> incana.org>
Subject: [PATCH v2 1/2] gnu: tlpui: Fix location for "defaults.conf".
Date: Thu, 22 Feb 2024 17:56:58 -0500

* gnu/packages/linux.scm (tlpui)[arguments]: Fix location for "defaults.conf".

Change-Id: I047375f875492aa5c6ef3f9ea673e366f00d89ad
---
 gnu/packages/linux.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 8eb6d9b7d3..ef225479ca 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -7879,7 +7879,7 @@ (define-public tlpui
                     (("\"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 "'")))
-- 
2.41.0

Message #20 received at 69313 <at> debbugs.gnu.org (full text, mbox):

From: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
To: Juliana Sims <juli <at> incana.org>
Cc: 69313 <at> debbugs.gnu.org, Tobias Geerinckx-Rice <me <at> tobias.gr>,
 Wilko Meyer <w <at> wmeyer.eu>, Leo Famulari <leo <at> famulari.name>
Subject: Re: [bug#69313] [PATCH v2 0/2] gnu: tlpui: Fix location for
 "defaults.conf".
Date: Fri, 23 Feb 2024 08:37:38 +0100

Juliana Sims <juli <at> 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!

Message #23 received at 69313 <at> debbugs.gnu.org (full text, mbox):

From: Juliana Sims <juli <at> incana.org>
To: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
Cc: 69313 <at> debbugs.gnu.org
Subject: Re: [bug#69313] [PATCH v2 0/2] gnu: tlpui: Fix location for
 "defaults.conf".
Date: Tue, 27 Feb 2024 12:10:47 -0500

> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.