bug#28510: crash: guix build -S foo --with-source=bla

bug#28510: crash: guix build -S foo --with-source=bla

[Ricardo Wurmus]

The command “guix build -S guile-git --with-source=guile-git” crashes
instead of failing gracefully:

--8<---------------cut here---------------start------------->8---
rwurmus@bimsb-sys02 in code: git clone https://gitlab.com/guile-git/guile-git.git
Cloning into 'guile-git'...
remote: Counting objects: 906, done.        
remote: Compressing objects: 100% (268/268), done.        
remote: Total 906 (delta 634), reused 895 (delta 630)        
Receiving objects: 100% (906/906), 243.62 KiB | 136.00 KiB/s, done.
Resolving deltas: 100% (634/634), done.
rwurmus@bimsb-sys02 in code: cd guile-git
rwurmus@bimsb-sys02 in guile-git: git reset --hard 969514a
HEAD is now at 969514a switch to using set-pointer-finalizer!
rwurmus@bimsb-sys02 in guile-git: cd ..
rwurmus@bimsb-sys02 in code: guix build -S guile-git --with-source=guile-git
Backtrace:
           9 (primitive-load "/gnu/store/04gdnvw7k8mnpzs9ig5kwmblzvgkxm4a-guix-0.13.0-1.a6d7…")
In guix/ui.scm:
  1331:12  8 (run-guix-command _ . _)
In ice-9/boot-9.scm:
    837:9  7 (catch _ _ #<procedure 7f11d55d88c0 at guix/ui.scm:448:2 (key c)> _)
    837:9  6 (catch _ _ #<procedure 7f11d55d88d8 at guix/ui.scm:536:6 (key proc format-stri…> …)
In guix/scripts/build.scm:
   725:14  5 (_)
In guix/ui.scm:
    633:8  4 (show-what-to-build #<build-daemon 256.97 2832e10> ("/gnu/store/07g069ka7l45f…") …)
In guix/derivations.scm:
   307:18  3 (substitution-oracle #<build-daemon 256.97 2832e10> ("/gnu/store/07g069ka7l45…") …)
In srfi/srfi-1.scm:
   466:18  2 (fold #<procedure 4028de0 at guix/derivations.scm:307:24 (drv result)> () ("/g…"))
In guix/derivations.scm:
   308:38  1 (_ "/gnu/store/07g069ka7l45f91npb8qv4j1kvi0y7m7-guile-git" ())
    606:7  0 (derivation->output-paths "/gnu/store/07g069ka7l45f91npb8qv4j1kvi0y7m7-guile-git")

guix/derivations.scm:606:7: In procedure derivation->output-paths:
guix/derivations.scm:606:7: In procedure struct_vtable: Wrong type argument in position 1 (expecting struct): "/gnu/store/07g069ka7l45f91npb8qv4j1kvi0y7m7-guile-git"
rwurmus@bimsb-sys02 in code: 
--8<---------------cut here---------------end--------------->8---

-- Ricardo

bug#28510: crash: guix build -S foo --with-source=bla

[zimoun]

Hi,

On Tue, 19 Sep 2017 at 14:09, Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> wrote:
> The command “guix build -S guile-git --with-source=guile-git” crashes
> instead of failing gracefully:
>
> rwurmus@bimsb-sys02 in code: git clone https://gitlab.com/guile-git/guile-git.git

[...]

> rwurmus@bimsb-sys02 in code: guix build -S guile-git --with-source=guile-git
> Backtrace:

Still failing.  In ’compute-derivation’, this 

              (s
               (list (package-source-derivation store s)))))

returns a list of string instead of a derivation, so then

                          (for-each show-derivation-outputs drv)

fails because it expects a derivation or a derivation-input (see in
(guix ui)).

Well, I am not sure where the error should be handled.  In (guix ui) and
in (guix scripts build)?  WDYT?


Cheers,
simon

bug#28510: crash: guix build -S foo --with-source=bla

[Jérémy Korwin-Zmijowski]

[-- Attachment #1: Type: text/plain, Size: 2005 bytes --]

Hello,

Today, I followed this steps to try to reproduce :

jeko@slim guix ±|master ✗|→ git pull
jeko@slim guix ±|master ✗|→ guix shell -D guix help2man git strace --pure
[dev] jeko@slim guix ±|master ✗|→ make
[dev] jeko@slim guix ±|master ✗|→ git clone 
https://gitlab.com/guile-git/guile-git.git /tmp/guile-git
[dev] jeko@slim guix ±|master ✗|→ ./pre-inst-env guix build -S guile-git 
--with-source=/tmp/guile-git
Backtrace:
In ice-9/boot-9.scm:
   1752:10 16 (with-exception-handler _ _ #:unwind? _ # _)
In unknown file:
           15 (apply-smob/0 #<thunk 7f7b1c56f2e0>)
In ice-9/boot-9.scm:
     724:2 14 (call-with-prompt _ _ #<procedure default-prompt-handle?>)
In ice-9/eval.scm:
     619:8 13 (_ #(#(#<directory (guile-user) 7f7b1c574c80>)))
In guix/ui.scm:
    2263:7 12 (run-guix . _)
   2226:10 11 (run-guix-command _ . _)
In ice-9/boot-9.scm:
   1752:10 10 (with-exception-handler _ _ #:unwind? _ # _)
In guix/status.scm:
     815:4  9 (call-with-status-report _ _)
In ice-9/boot-9.scm:
   1752:10  8 (with-exception-handler _ _ #:unwind? _ # _)
In guix/store.scm:
    656:37  7 (thunk)
    1295:8  6 (call-with-build-handler _ _)
    1295:8  5 (call-with-build-handler #<procedure 7f7b19ae5c90 at g?> ?)
In guix/scripts/build.scm:
    808:26  4 (_)
In srfi/srfi-1.scm:
     634:9  3 (for-each #<procedure show-derivation-outputs (derivat?> ?)
In guix/ui.scm:
     907:2  2 (show-derivation-outputs _)
In ice-9/boot-9.scm:
   1685:16  1 (raise-exception _ #:continuable? _)
   1685:16  0 (raise-exception _ #:continuable? _)

ice-9/boot-9.scm:1685:16: In procedure raise-exception:
Throw to key `match-error' with args `("match" "no matching pattern" 
"/gnu/store/wgxcw7j7d45n3z3g5rswjj1hf393abl0-guile-git")'.

So, still failing but I don't get the same error… Is it valid according 
to the bug declaration ? I'm not sure haha

-- 
Jérémy Korwin-Zmijowski

GPG: 036B 4D54 B7B4 D6C8 DA62 2746 700F 5E0C CBB2 E2D1

[-- Attachment #2: Type: text/html, Size: 2894 bytes --]

bug#28510: crash: guix build -S foo --with-source=bla

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 531 bytes --]

On 19-09-2022 19:38, Jérémy Korwin-Zmijowski wrote:
> Hello,
> 
> Today, I followed this steps to try to reproduce : [...]'.
> 
> So, still failing but I don't get the same error… Is it valid according 
> to the bug declaration ? I'm not sure haha
> 

Here is a simpler reproducer for that error:

file a.scm:
(use-modules (gnu packages) (guix packages) (guix gexp))
(package
   (inherit (specification->package "hello"))
   (source (local-file "a.scm")))

guix build -f a.scm --source

Greetings,
Maxime.

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

bug#28510: crash: guix build -S foo --with-source=bla

[Josselin Poiret via Bug reports for GNU Guix]

Hi everyone,

Maxime Devos <maximedevos@telenet.be> writes:
> Here is a simpler reproducer for that error:
>
> file a.scm:
> (use-modules (gnu packages) (guix packages) (guix gexp))
> (package
>    (inherit (specification->package "hello"))
>    (source (local-file "a.scm")))
>
> guix build -f a.scm --source

The issue is that package-source-derivation in guix/packages.scm doesn't
actually always yield a derivation, since lower-object isn't guaranteed
to do that: here the gexp compiler for local-file only returns a string
denoting the file path of the interned store file.  `guix build` relies
on the (wrong) assumption that everything that it will build will end up
being a derivation in some way or another, so just calls
show-derivation-outputs on that, which then errors as above.

build-derivations, contrary to its name, can also pass simple file names
to build-things, and since the file will already be interned in the
store at that point, it won't need to do anything, and there won't be
any errors there.

The simple fix would be to add another band-aid cond at the
show-derivation-outputs call in build.scm, but it doesn't seem to be
enough in the long term.

What do people think?

-- 
Josselin Poiret

bug#28510: crash: guix build -S foo --with-source=bla

[zimoun]

Hi,

On Tue, 20 Sep 2022 at 11:19, Josselin Poiret via Bug reports for GNU Guix <bug-guix@gnu.org> wrote:

> The simple fix would be to add another band-aid cond at the
> show-derivation-outputs call in build.scm, but it doesn't seem to be
> enough in the long term.

Well, I would add an error handler; as proposed [1]. :-)  Because does
“guix build foo --source --with-source=bla” make sense?  What is the
use-case for such command?

1: <https://issues.guix.gnu.org/28510/#1>


Cheers,
simon

bug#28510: crash: guix build -S foo --with-source=bla

[Josselin Poiret via Bug reports for GNU Guix]

Hi Simon,

zimoun <zimon.toutoune@gmail.com> writes:

> Well, I would add an error handler; as proposed [1]. :-)  Because does
> “guix build foo --source --with-source=bla” make sense?  What is the
> use-case for such command?

My bad, I didn't see the previous discussion on the subject.  To me, the
lack of generality would be unexpected: if I have a package with a
source, I expect to be able to get that source, whatever the source may
be.  Maxime's minimal example shows that it could happen for a variety
of different reasons, not just a --with-source= flag.  I don't know what
a proper fix for it would be though, since we're passing things around
that we pretend are derivations but are not, and in many places.

Best,
-- 
Josselin Poiret

bug#28510: crash: guix build -S foo --with-source=bla

[jeremy]

[-- Attachment #1: Type: text/plain, Size: 1234 bytes --]

Le 2022-09-20 21:42, Josselin Poiret a écrit :
> Hi Simon,
> 
> zimoun <zimon.toutoune@gmail.com> writes:
> 
>> Well, I would add an error handler; as proposed [1]. :-)  Because does
>> “guix build foo --source --with-source=bla” make sense?  What is the
>> use-case for such command?
> 
> My bad, I didn't see the previous discussion on the subject.  To me, 
> the
> lack of generality would be unexpected: if I have a package with a
> source, I expect to be able to get that source, whatever the source may
> be.  Maxime's minimal example shows that it could happen for a variety
> of different reasons, not just a --with-source= flag.  I don't know 
> what
> a proper fix for it would be though, since we're passing things around
> that we pretend are derivations but are not, and in many places.
> 
> Best,

Hi !

I feel not able to take decision about the proper way right now.
So as a step forward I wrote a test to capture the behavior expected.
Please have a look at it as it's the starting point for me.
Next, I can implement the error handler, making the previous test to 
pass.
As the test checks the behaviour, it will let enough space to change 
implementation later without invalitating the test.

Cheers,
Jérémy

[-- Attachment #2: guix-scripts.scm --]
[-- Type: text/plain, Size: 930 bytes --]

(define-module (tests scripts build)
  #:use-module (srfi srfi-64)
  #:use-module (ice-9 exceptions)
  #:use-module (guix scripts build)
  #:use-module (srfi srfi-34))

(test-begin "scripts")

(define DUMMY_DEFINITION_PATH "/tmp/a.scm")

(define (create-dummy-definition)
  (call-with-output-file DUMMY_DEFINITION_PATH
    (lambda (port)
      (format port
              "(use-modules (gnu packages) (guix packages) (guix gexp)) (package (inherit (specification->package \"hello\")) (source (local-file \"a.scm\")))"))))

(define (delete-dummy-definition)
  (delete-file DUMMY_DEFINITION_PATH))

(test-group-with-cleanup "build"

  (create-dummy-definition)

  (test-equal "package definition as only source"
    "/gnu/store/7mnkrg9bmybgyf1dn5n67di6ng6qvhkz-a.scm"
    (guard
        (ex
         ((exception? ex) #f))
      (guix-build "-f" DUMMY_DEFINITION_PATH "--source")))

  (delete-dummy-definition))

(test-end "scripts")

bug#28510: crash: guix build -S foo --with-source=bla

[zimoun]

Hi Jérémy,

On Fri, 07 Oct 2022 at 10:41, jeremy@korwin-zmijowski.fr wrote:

> I feel not able to take decision about the proper way right now.
> So as a step forward I wrote a test to capture the behavior expected.
> Please have a look at it as it's the starting point for me.
> Next, I can implement the error handler, making the previous test to 
> pass.
> As the test checks the behaviour, it will let enough space to change 
> implementation later without invalitating the test.

Nice to start by the test! :-)

> (define-module (tests scripts build)
>   #:use-module (srfi srfi-64)
>   #:use-module (ice-9 exceptions)
>   #:use-module (guix scripts build)
>   #:use-module (srfi srfi-34))
>
> (test-begin "scripts")
>
> (define DUMMY_DEFINITION_PATH "/tmp/a.scm")
>
> (define (create-dummy-definition)
>   (call-with-output-file DUMMY_DEFINITION_PATH
>     (lambda (port)
>       (format port
>               "(use-modules (gnu packages) (guix packages) (guix gexp)) (package (inherit (specification->package \"hello\")) (source (local-file \"a.scm\")))"))))
>
> (define (delete-dummy-definition)
>   (delete-file DUMMY_DEFINITION_PATH))
>
> (test-group-with-cleanup "build"
>
>   (create-dummy-definition)
>
>   (test-equal "package definition as only source"
>     "/gnu/store/7mnkrg9bmybgyf1dn5n67di6ng6qvhkz-a.scm"
>     (guard
>         (ex
>          ((exception? ex) #f))
>       (guix-build "-f" DUMMY_DEFINITION_PATH "--source")))

Well, this test looks good to me.  However, I would lower what the
procedure under test.  For instance, package-source-derivation in
guix/packages.scm or show-derivation-outputs, as Josselin pointed [1].

1: <https://yhetil.org/guix/87k05y2yjc.fsf@jpoiret.xyz>

>
>   (delete-dummy-definition))
>
> (test-end "scripts")


Cheers,
simon