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

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

[Juliana Sims]

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

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

[Nicolas Goaziou via Guix-patches via]

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

[bug#69313] [PATCH v2 0/2] gnu: tlpui: Fix location for "defaults.conf".

[Juliana Sims]

> 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

[bug#69313] [PATCH v2 0/2] gnu: tlpui: Fix location for "defaults.conf".

[Nicolas Goaziou via Guix-patches via]

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!

[bug#69313] [PATCH v2 0/2] gnu: tlpui: Fix location for "defaults.conf".

[Juliana Sims]

> 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

[bug#69313] [PATCH v2 1/2] gnu: tlpui: Fix location for "defaults.conf".

[Juliana Sims]

* 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

[bug#69313] [PATCH v2 2/2] gnu: tlpui: Reference inputs directly during build.

[Juliana Sims]

* 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