diff mbox series

[bug#57809] gnome-boxes: Patch 'qemu-img' path

Message ID 20220918204335.1734-1-kreyren@rixotstudio.cz
State Accepted
Headers show
Series [bug#57809] gnome-boxes: Patch 'qemu-img' path | expand

Checks

Context Check Description
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Jacob Hrbek Sept. 18, 2022, 8:43 p.m. UTC
---
 gnu/packages/gnome.scm | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

            gettext-minimal
@@ -12141,6 +12147,7 @@ (define-public gnome-boxes
            itstool
            pkg-config
            python
+           qemu-minimal                 ;for qemu-img
            vala))
     (inputs
      (list glib-networking              ;for TLS support
--
2.37.1

Comments

Jacob Hrbek Sept. 18, 2022, 9:05 p.m. UTC | #1
isn't that going to break if the user invokes `guix cg -D` though?

On Ne, zář 18 2022 at 20:43:43 +0000, Jacob Hrbek 
<kreyren@rixotstudio.cz> wrote:
> ---
>  gnu/packages/gnome.scm | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm
> index d6d22f8eb1..89f5999920 100644
> --- a/gnu/packages/gnome.scm
> +++ b/gnu/packages/gnome.scm
> @@ -12111,6 +12111,7 @@ (define-public polari
>      (home-page "<https://wiki.gnome.org/Apps/Polari>")
>      (license license:gpl2+)))
> 
> +;; NOTE(Krey): src/util-app.vala might need patched paths
>  (define-public gnome-boxes
>    (package
>      (name "gnome-boxes")
> @@ -12133,7 +12134,12 @@ (define-public gnome-boxes
>             #:phases #~(modify-phases %standard-phases
>                          (add-after 'unpack 
> 'disable-gtk-update-icon-cache
>                            (lambda _
> -                            (setenv "DESTDIR" "/"))))))
> +                            (setenv "DESTDIR" "/")))
> +                        (add-before 'configure 
> 'substitute-qemu-dependencies
> +                          (lambda _
> +                              (substitute* "src/installed-media.vala"
> +                                (("qemu-img")
> +                                 (search-input-file %build-inputs 
> "/bin/qemu-img"))))))))
>      (native-inputs
>       (list desktop-file-utils           ;for update-desktop-database
>             gettext-minimal
> @@ -12141,6 +12147,7 @@ (define-public gnome-boxes
>             itstool
>             pkg-config
>             python
> +           qemu-minimal                 ;for qemu-img
>             vala))
>      (inputs
>       (list glib-networking              ;for TLS support
> --
> 2.37.1
>
M Sept. 18, 2022, 9:14 p.m. UTC | #2
On 18-09-2022 23:05, Jacob Hrbek wrote:
> isn't that going to break if the user invokes `guix cg -D` though?

No, "guix gc" only deletes things that aren't (indirectly) referred to 
by a root (being a garbage collector), and by adding that substitution, 
the compiled gnome-boxes refers to qemu(-minimal).

(Unless the reference is obfuscated, but Vala has a C-like compilation 
model, so I wouldn't expect that.)

> +                              (substitute* "src/installed-media.vala"
> +                                (("qemu-img")
> +                                 (search-input-file %build-inputs "/bin/qemu-img"))))))))

I find (search-input-file inputs "/bin/qemu-img") clearer -- 'inputs' / 
'native-inputs' is sufficiently complicated on its own, no need to bring 
%build-inputs and friends in the equation when it can be avoided.

> +;; NOTE(Krey): src/util-app.vala might need patched paths

(1) the convention is TODO or FIXME, not NOTE
(2) who discovered the potential problem, does not need to be mentioned 
-- there is still "git blame" (which can be used for other things than 
blaming) and "git log" if it turns out to be somehow useful information 
after all (e.g. for attribution).

Greetings,
Maxime.
Jacob Hrbek Sept. 18, 2022, 9:32 p.m. UTC | #3
On Ne, zář 18 2022 at 23:14:12 +0200, Maxime Devos 
<maximedevos@telenet.be> wrote:
> I find (search-input-file inputs "/bin/qemu-img") clearer -- 'inputs' 
> /
> 'native-inputs' is sufficiently complicated on its own, no need to 
> bring
> %build-inputs and friends in the equation when it can be avoided.

That ain't working for me: <https://i.imgur.com/Co4Xsjl.png>

    unbound-variable #f "Unbound variable: ~S" (native-inputs) #f

 > (1) the convention is TODO or FIXME, not NOTE

It's not:
- TODO as in "To Do something" or "for something to be done"
- FIXME as in "Low priority thing or quality assurance something that 
needs to be fixed"

More like handing over note to the next guy about possible issue in the 
future and leaving the signature here so that they can just go on chat 
and ask instead of wasting ~3 min on average searching git blame/log 
which gets exponentially more unsustainable to track

At least that's how it works where i work O_o .. personally i find it 
better way than the generic TODO/FIXME
M Sept. 18, 2022, 9:50 p.m. UTC | #4
On 18-09-2022 23:32, Jacob Hrbek wrote:
> On Ne, zář 18 2022 at 23:14:12 +0200, Maxime Devos 
> <maximedevos@telenet.be> wrote:
>> I find (search-input-file inputs "/bin/qemu-img") clearer -- 'inputs' 
>> / 'native-inputs' is sufficiently complicated on its own, no need to 
>> bring %build-inputs and friends in the equation when it can be avoided. 
> 
> That ain't working for me: https://i.imgur.com/Co4Xsjl.png 
> <https://i.imgur.com/Co4Xsjl.png>

Quoting the relevant code would be sufficient, no need for image links. 
  Besides, text is more accessible than images of text, text would reach 
a wider audience.

>      unbound-variable #f "Unbound variable: ~S" (native-inputs) #f

(1) 'qemu-minimal' is in the wrong place, it needs to be in 'inputs', 
not 'native-inputs', otherwise it is compiled for the wrong 
architecture. (In both the phase and the inputs/native-inputs field)
(2) 'native-inputs' (or 'inputs'), is unbound.  For information on how 
to use search-input-file, inputs and phases, see the manual, in 
particular (guix)Build Utilities (index entry: search-input-file) and 
its example.

>> (1) the convention is TODO or FIXME, not NOTE  It's not: - TODO as in "To Do something" or "for something to be done" 
> - FIXME as in "Low priority thing or quality assurance something that 
> needs to be fixed"

A missing hardcoded file name can easily be a (usually low-priority) 
bug, so FIXME applies, it's also something to investigate and solve (if 
appropriate), so TODO applies.

> More like handing over note to the next guy about possible issue in the 
> future and leaving the signature here so that they can just go on chat

The required information is already in the "TODO: [stuff about file 
names]", they do not need to go 'on chat', and if they do anyway, 
hardcoding file names is a relatively well-known solution, so I don't 
expect a contact person to be needed.

If that's not sufficient, you can insert an (impersonal) link to the 
Guix issue <https://issues.guix.gnu.org/issue/57809>, which has all the 
relevant information (which is available even when you aren't available 
for chatting, and also when your IRC nick is unknown or if you're gone).

Also, there are no gender restrictions in Guix.

Greetings,
Maxime.
Jacob Hrbek Sept. 19, 2022, 4:28 a.m. UTC | #5
>> (2) 'native-inputs' (or 'inputs'), is unbound.  For information on how 

>> to use search-input-file, inputs and phases, see the manual, in particular (guix)Build Utilities (index entry: search-input-file) >> and 

>> its example.

I've been searching through that thing endlessly for almost 5 hours >_> it doesn't help much for that.. I can grep the code to look for usage though which is what I did here ^-^

>> If that's not sufficient, you can insert an (impersonal) link to the 

>> Guix issue

meh, imho takes too much time especially when it's from a newbie like me who needs guidance which adds bunch of useless info to the issue.

>> which has all the 

>> relevant information (which is available even when you aren't available 

>> for chatting, and also when your IRC nick is unknown or if you're gone).

The idea is that people are using chat/client that has chat history and sends notifications so that people can respond quickly or when they are available worst case sending an email..

-- Jacob "Kreyren" Hrbek

------- Original Message -------
On Sunday, September 18th, 2022 at 11:50 PM, Maxime Devos <maximedevos@telenet.be> wrote:


> 

> 

> On 18-09-2022 23:32, Jacob Hrbek wrote:
> 

> > On Ne, zář 18 2022 at 23:14:12 +0200, Maxime Devos
> > maximedevos@telenet.be wrote:
> > 

> > > I find (search-input-file inputs "/bin/qemu-img") clearer -- 'inputs'
> > > / 'native-inputs' is sufficiently complicated on its own, no need to
> > > bring %build-inputs and friends in the equation when it can be avoided.
> > 

> > That ain't working for me: https://i.imgur.com/Co4Xsjl.png
> > https://i.imgur.com/Co4Xsjl.png
> 

> 

> Quoting the relevant code would be sufficient, no need for image links.
> Besides, text is more accessible than images of text, text would reach
> a wider audience.
> 

> > unbound-variable #f "Unbound variable: ~S" (native-inputs) #f
> 

> 

> (1) 'qemu-minimal' is in the wrong place, it needs to be in 'inputs',
> not 'native-inputs', otherwise it is compiled for the wrong
> architecture. (In both the phase and the inputs/native-inputs field)
> (2) 'native-inputs' (or 'inputs'), is unbound. For information on how
> to use search-input-file, inputs and phases, see the manual, in
> particular (guix)Build Utilities (index entry: search-input-file) and
> its example.
> 

> > > (1) the convention is TODO or FIXME, not NOTE It's not: - TODO as in "To Do something" or "for something to be done"
> > > - FIXME as in "Low priority thing or quality assurance something that
> > > needs to be fixed"
> 

> 

> A missing hardcoded file name can easily be a (usually low-priority)
> bug, so FIXME applies, it's also something to investigate and solve (if
> appropriate), so TODO applies.
> 

> > More like handing over note to the next guy about possible issue in the
> > future and leaving the signature here so that they can just go on chat
> 

> 

> The required information is already in the "TODO: [stuff about file
> names]", they do not need to go 'on chat', and if they do anyway,
> hardcoding file names is a relatively well-known solution, so I don't
> expect a contact person to be needed.
> 

> If that's not sufficient, you can insert an (impersonal) link to the
> Guix issue https://issues.guix.gnu.org/issue/57809, which has all the
> 

> relevant information (which is available even when you aren't available
> for chatting, and also when your IRC nick is unknown or if you're gone).
> 

> Also, there are no gender restrictions in Guix.
> 

> Greetings,
> Maxime.
M Sept. 19, 2022, 1:35 p.m. UTC | #6
On 19-09-2022 06:28, Jacob Hrbek wrote:
>>> (2) 'native-inputs' (or 'inputs'), is unbound.  For information on how
> 
>>> to use search-input-file, inputs and phases, see the manual, in particular (guix)Build Utilities (index entry: search-input-file) >> and
> 
>>> its example.
> 
> I've been searching through that thing endlessly for almost 5 hours >_> it doesn't help much for that.. I can grep the code to look for usage though which is what I did here ^-^

Here are some proposed methods for finding it, which can be adapted for 
other searches as well.

  * "info guix" followed by 's' 'search-input-file'
    (free-form text search, good for finding examples)
  * "info guix" followed by 'i' 'search-input-file'
    (index entry, works well for procedure names)
  * "info guix" > "Programming Interface" > "Build Utilities", followed
     by some scrolling, then find 'search-input-file' there.
    (using the TOC)

The first two options only take a few seconds.  The latter can be quick 
or long, depending on whether your guess on where it's located is good 
or bad.

And a fourth option, since you seem to like grepping:

  * git grep -F search-input-file doc/
    (in a git checkout of Guix)

Greetings,
Maxime.
diff mbox series

Patch

diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm
index d6d22f8eb1..89f5999920 100644
--- a/gnu/packages/gnome.scm
+++ b/gnu/packages/gnome.scm
@@ -12111,6 +12111,7 @@  (define-public polari
     (home-page "https://wiki.gnome.org/Apps/Polari")
     (license license:gpl2+)))

+;; NOTE(Krey): src/util-app.vala might need patched paths
 (define-public gnome-boxes
   (package
     (name "gnome-boxes")
@@ -12133,7 +12134,12 @@  (define-public gnome-boxes
            #:phases #~(modify-phases %standard-phases
                         (add-after 'unpack 'disable-gtk-update-icon-cache
                           (lambda _
-                            (setenv "DESTDIR" "/"))))))
+                            (setenv "DESTDIR" "/")))
+                        (add-before 'configure 'substitute-qemu-dependencies
+                          (lambda _
+
    (substitute* "src/installed-media.vala"
+                                (("qemu-img")
+                                 (search-input-file %build-inputs "/bin/qemu-img"))))))))
     (native-inputs
      (list desktop-file-utils           ;for update-desktop-database