[PATCH] gnu: tlpui: Fix broken package.

  • Done
  • quality assurance status badge
Details
2 participants
  • Juliana Sims
  • Nicolas Goaziou
Owner
unassigned
Submitted by
Juliana Sims
Severity
normal
J
J
Juliana Sims wrote on 22 Feb 21:10 +0100
(address . guix-patches@gnu.org)(name . Juliana Sims)(address . juli@incana.org)
eb560602d63a60bd6a541cd682b5bea50f2ed8c6.1708632642.git.juli@incana.org
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(-)

Toggle diff (33 lines)
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
N
N
Nicolas Goaziou wrote on 22 Feb 23:07 +0100
(name . Juliana Sims)(address . juli@incana.org)
87il2gcfvm.fsf@nicolasgoaziou.fr
Hello,

Juliana Sims <juli@incana.org> writes:

Toggle quote (2 lines)
> * 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".


Toggle quote (13 lines)
> (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)?

Toggle quote (8 lines)
> (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
J
J
Juliana Sims wrote on 22 Feb 23:56 +0100
[PATCH v2 2/2] gnu: tlpui: Reference inputs directly during build.
(address . 69313@debbugs.gnu.org)(name . Juliana Sims)(address . juli@incana.org)
760260ba9ec62a79c7a52795280f7f158f6d8612.1708642619.git.juli@incana.org
* 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(-)

Toggle diff (24 lines)
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
J
J
Juliana Sims wrote on 22 Feb 23:56 +0100
[PATCH v2 0/2] gnu: tlpui: Fix location for "defaults.conf".
(address . 69313@debbugs.gnu.org)(name . Juliana Sims)(address . juli@incana.org)
cover.1708642619.git.juli@incana.org
Toggle quote (2 lines)
> 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!

Toggle quote (8 lines)
> 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.

Toggle quote (4 lines)
> 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
J
J
Juliana Sims wrote on 22 Feb 23:56 +0100
[PATCH v2 1/2] gnu: tlpui: Fix location for "defaults.conf".
(address . 69313@debbugs.gnu.org)(name . Juliana Sims)(address . juli@incana.org)
6d30d106283a0c75a963d97191b4d76678c0994c.1708642619.git.juli@incana.org
* 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(-)

Toggle diff (15 lines)
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
N
N
Nicolas Goaziou wrote on 23 Feb 08:37 +0100
Re: [bug#69313] [PATCH v2 0/2] gnu: tlpui: Fix location for "defaults.conf".
(name . Juliana Sims)(address . juli@incana.org)
87edd3d41p.fsf@nicolasgoaziou.fr
Juliana Sims <juli@incana.org> writes:

Toggle quote (5 lines)
>> 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!

:)

Toggle quote (19 lines)
>> 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!
J
J
Juliana Sims wrote on 27 Feb 18:10 +0100
(name . Nicolas Goaziou)(address . mail@nicolasgoaziou.fr)(address . 69313@debbugs.gnu.org)
ZDYI9S.PC6SWWBGLP8T3@incana.org
Toggle quote (5 lines)
> 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
J
J
Juliana Sims wrote on 7 Mar 21:49 +0100
close issue
(address . control@debbugs.gnu.org)
UIWZ9S.D65PTW1V9JZ02@incana.org
close 69313
?