bug#57121: clojure-build-system fails to compile -- backtrace from language/tree-il/peval.scm

bug#57121: clojure-build-system fails to compile -- backtrace from language/tree-il/peval.scm

[Maxime Devos]

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

I started with a guix checkout at commit 
d519305d83d08058e4def2c4d72fe62102d9599d. Everything was compiled, 
according to "make".

Then I switched to current master: b21d05d232ec0aba5abec20e83cc52c1d5163cc3.

Now I get the following error:

> In language/tree-il/peval.scm:
>     799:6 19 (loop _ #<vhash 7f81d96446e0 10 pairs> #f effect)
>   1661:48 18 (loop _ _ _ effect)
> In srfi/srfi-1.scm:
>    586:17 17 (map1 (#<tree-il (lexical install #{ g377}#)> #<tree-il 
> (const documentation)> #<tree-il (const "Standard 'install' phase for 
> clojure-build-system.")>))
> In language/tree-il/peval.scm:
>    853:11 16 (loop _ _ #f value)
>    346:22 15 (visit-operand #<<operand> var: #<<var> name: install 
> gensym: #{ g377}# refcount: 2 set?: #f> sym: #{install 412}# visit: 
> #<procedure 7f81d96447a0 at language/tree-il/peval.scm:977:40 (exp 
> counter ctx)> source: #<tree-il (primcall values (call (@ (guix build 
> java-utils) install-jars) (const "./")))> visit-count: 1 use-count: 0 
> copyable?: #t residual-value: #f constant-value: #f alias: #f> _ value 
> _ _)
>   1286:21 14 (loop _ #<vhash 7f81d96242c0 8 pairs> #<<counter> effort: 
> #<variable 7f81d9876ae0 value: 0> size: #<variable 7f81d9876ad0 value: 
> 0> continuation: #<procedure abort ()> recursive?: #t data: 
> #<<operand> var: #<<var> name: install gensym: #{ g377}# refcount: 2 
> set?: #f> sym: #{install 412}# visit: #<procedure 7f81d96447a0 at 
> language/tree-il/peval.scm:977:40 (exp counter ctx)> source: #<tree-il 
> (primcall values (call (@ (guix build java-utils) install-…> …)
> In srfi/srfi-1.scm:
>    586:17 13 (map1 (#<tree-il (call (@ (guix build java-utils) 
> install-jars) (const "./"))>))
> In language/tree-il/peval.scm:
>   1625:20 12 (loop _ #<vhash 7f81d96242c0 8 pairs> #<<counter> effort: 
> #<variable 7f81d9876ae0 value: 0> size: #<variable 7f81d9876ad0 value: 
> 0> continuation: #<procedure abort ()> recursive?: #t data: 
> #<<operand> var: #<<var> name: install gensym: #{ g377}# refcount: 2 
> set?: #f> sym: #{install 412}# visit: #<procedure 7f81d96447a0 at 
> language/tree-il/peval.scm:977:40 (exp counter ctx)> source: #<tree-il 
> (primcall values (call (@ (guix build java-utils) install-…> …)
>    981:20 11 (loop _ #<vhash 7f81d96242c0 8 pairs> #<<counter> effort: 
> #<variable 7f81d9876710 value: 65> size: #<variable 7f81d9876700 
> value: 20> continuation: #<procedure abort ()> recursive?: #f data: 
> #<tree-il (lambda ((name . install-jars)) (lambda-case 
> (((jar-directory) #f #f #f () (t413)) (lambda () (lambda-case ((() #f 
> #f (#t (#:outputs outputs #f)) ((const #f)) (t414)) (let (share) 
> (t415) ((call (@ (guile) string-append) (call (@ (guile) assoc-ref) 
> (lex…> …)
>     799:6 10 (loop #<tree-il (lambda () (lambda-case ((() #f #f (#t 
> (#:outputs outputs #f)) ((const #f)) (t414)) (let (share) (t415) 
> ((call (@ (guile) string-append) (call (@ (guile) assoc-ref) (lexical 
> outputs t414) (const "out")) (const "/share/java"))) (seq (call (@ 
> (guile) for-each) (lambda () (lambda-case (((f) #f #f #f () (t416)) 
> (call (@ (guix build utils) install-file) (lexical f t416) (lexical 
> share t415))))) (call (@ (guix build utils) find-files) (lex…> …)
>   1699:60  9 (loop _ #<vhash 7f81d964b200 10 pairs> #<<counter> 
> effort: #<variable 7f81d9876710 value: 65> size: #<variable 
> 7f81d9876700 value: 20> continuation: #<procedure abort ()> 
> recursive?: #f data: #<tree-il (lambda ((name . install-jars)) 
> (lambda-case (((jar-directory) #f #f #f () (t413)) (lambda () 
> (lambda-case ((() #f #f (#t (#:outputs outputs #f)) ((const #f)) 
> (t414)) (let (share) (t415) ((call (@ (guile) string-append) (call (@ 
> (guile) assoc-ref) (le…> …)
> In srfi/srfi-1.scm:
>    586:17  8 (map1 (#f))
> In language/tree-il/peval.scm:
>   1691:38  7 (new-sym _)
> In ice-9/boot-9.scm:
>   1685:16  6 (raise-exception _ #:continuable? _)
>   1685:16  5 (raise-exception _ #:continuable? _)
>   1780:13  4 (_ #<&compound-exception components: 
> (#<&assertion-failure> #<&origin origin: "cdr"> #<&message message: 
> "Wrong type argument in position 1 (expecting pair): ~S"> #<&irritants 
> irritants: (#f)> #<&exception-with-kind-and-args kind: wrong-type-arg 
> args: ("cdr" "Wrong type argument in position 1 (expecting pair): ~S" 
> (#f) (#f))>)>)
> In guix/build/compile.scm:
>     191:6  3 (_ _ . _)
> In ice-9/boot-9.scm:
>   1747:15  2 (with-exception-handler #<procedure 7f81dac67270 at 
> ice-9/boot-9.scm:1831:7 (exn)> _ #:unwind? _ #:unwind-for-type _)
> In guix/build/compile.scm:
>    194:22  1 (_)
> In unknown file:
>            0 (make-stack #t)
Looking at language/tree-il/peval.scm 1691:38, it appears that somehow 
the 'old' symbol does not appear in the environment.

For a minimal test case, remove everything but the 'define-module' and 
the '(define-with-docs install [...])'.

Looks like something is going wrong with 'define-with-docs' (without 
-with-docs and without the docstring, it compiles fine).

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#57121: clojure-build-system fails to compile -- backtrace from language/tree-il/peval.scm

[Maxime Devos]

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

Can't reproduce after touching java-utils.scm.

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#57121: clojure-build-system fails to compile -- backtrace from language/tree-il/peval.scm

[Maxim Cournoyer]

Hi Maxime.

Maxime Devos <maximedevos@telenet.be> writes:

> Can't reproduce after touching java-utils.scm.

I got this issue a couple times too.  I attributed it (without analysis)
to Guile's failure to keep track of changes to macro expanded code.

More like something to track on the side of Guile, I would think.

I'd suggest to close it here, since it's not reproducible.

Thanks,

Maxim

bug#57121: clojure-build-system fails to compile -- backtrace from language/tree-il/peval.scm

[Maxime Devos]

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

On 19-08-2022 22:37, Maxim Cournoyer wrote:

> Hi Maxime.
>
> Maxime Devos <maximedevos@telenet.be> writes:
>
>> Can't reproduce after touching java-utils.scm.
> I got this issue a couple times too.  I attributed it (without analysis)
> to Guile's failure to keep track of changes to macro expanded code.

It might be inlining. I don't see how macro expansion matters here. I 
think it's dependency tracking in general.

> More like something to track on the side of Guile, I would think.

I had a patch for build-aux/compile-all.scm that adds a form of 
dependency tracking: <https://issues.guix.gnu.org/50384>. If we teach it 
about (define-module (foo) #:use-module (bar)) --> (bar) is a dependency 
of (foo) (using parts of source-module-closure?) (and drop the 
search-patch things), then it seems solved to me.

As there is a known path to a solution, I wouldn't close this.

These patches are for Guix' build system.  I don't see anything that 
could be done on the Guile side, except for eventually migrating some 
dependency tracking stuff over to Guile -- "gcc" has an -M option to use 
in combination with "make", maybe Guile could have something similar.  
Actually acting on the dependency information (which is part of the 
patches) isn't something Guile can do, that seems more something for 
"make" (when using Autotools and not like how Guix uses Autotools), 
guile-build-system or build-aux/compile-all.scm to me.

> I'd suggest to close it here, since it's not reproducible.

Just wait a few months or something, it keeps popping up -- I have 
encountered this one some time in the past.

Also, non-determinism failures (which is a cause of irreproducibility) 
are bugs in Guix.

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#57121: clojure-build-system fails to compile -- backtrace from language/tree-il/peval.scm

[Maxim Cournoyer]

Hi Maxim,

Maxime Devos <maximedevos@telenet.be> writes:

> On 19-08-2022 22:37, Maxim Cournoyer wrote:
>
>> Hi Maxime.
>>
>> Maxime Devos <maximedevos@telenet.be> writes:
>>
>>> Can't reproduce after touching java-utils.scm.
>> I got this issue a couple times too.  I attributed it (without analysis)
>> to Guile's failure to keep track of changes to macro expanded code.
>
> It might be inlining. I don't see how macro expansion matters here. I
> think it's dependency tracking in general.

Dependency tracking is probably what I meant.

>> More like something to track on the side of Guile, I would think.
>
> I had a patch for build-aux/compile-all.scm that adds a form of
> dependency tracking: <https://issues.guix.gnu.org/50384>. If we teach
> it about (define-module (foo) #:use-module (bar)) --> (bar) is a
> dependency of (foo) (using parts of source-module-closure?) (and drop
> the search-patch things), then it seems solved to me.
>
> As there is a known path to a solution, I wouldn't close this.
>
> These patches are for Guix' build system.  I don't see anything that
> could be done on the Guile side, except for eventually migrating some
> dependency tracking stuff over to Guile

If a module imports a different module, and that module changes, even if
it's macro, Guile should not blindly reuse the stale .go like it
currently does.  It should complain and evaluate from source instead.

That would cover the base and avoid breakage.  After, if it known how to
do that, yes, it seems it'd be useful to have something similar to 'gcc
-M' to provide the needed intelligence to the build system.

Does that make sense?

Thanks,

Maxim

bug#57121: clojure-build-system fails to compile -- backtrace from language/tree-il/peval.scm

[Maxime Devos]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 811 bytes --]


On 22-08-2022 17:32, Maxim Cournoyer wrote:
>> These patches are for Guix' build system.  I don't see anything that
>> could be done on the Guile side, except for eventually migrating some
>> dependency tracking stuff over to Guile
> If a module imports a different module, and that module changes, even if
> it's macro, Guile should not blindly reuse the stale .go like it
> currently does.  It should complain and evaluate from source instead.
>
> That would cover the base and avoid breakage.  After, if it known how to
> do that, yes, it seems it'd be useful to have something similar to 'gcc
> -M' to provide the needed intelligence to the build system.
>
> Does that make sense?

Sounds reasonable, though we could go for something less general in Guix 
first.

Greetings,
Maxime.


[-- Attachment #1.1.1.2: Type: text/html, Size: 1296 bytes --]

[-- 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#57121: clojure-build-system fails to compile -- backtrace from language/tree-il/peval.scm

[Maxim Cournoyer]

Hi Maxime,

Maxime Devos <maximedevos@telenet.be> writes:

> On 22-08-2022 17:32, Maxim Cournoyer wrote:
>>> These patches are for Guix' build system.  I don't see anything that
>>> could be done on the Guile side, except for eventually migrating some
>>> dependency tracking stuff over to Guile
>> If a module imports a different module, and that module changes, even if
>> it's macro, Guile should not blindly reuse the stale .go like it
>> currently does.  It should complain and evaluate from source instead.
>>
>> That would cover the base and avoid breakage.  After, if it known how to
>> do that, yes, it seems it'd be useful to have something similar to 'gcc
>> -M' to provide the needed intelligence to the build system.
>>
>> Does that make sense?
>
> Sounds reasonable, though we could go for something less general in
> Guix first.

I'd rather avoiding adding more complexity in Guix if it can be fixed
upstream; where it'd benefit everyone most.

Thanks,

Maxim

bug#57121: clojure-build-system fails to compile -- backtrace from language/tree-il/peval.scm

[Maxime Devos]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 2417 bytes --]


On 23-08-2022 06:06, Maxim Cournoyer wrote:
> Hi Maxime,
>
> Maxime Devos<maximedevos@telenet.be>  writes:
>
>> On 22-08-2022 17:32, Maxim Cournoyer wrote:
>>>> These patches are for Guix' build system.  I don't see anything that
>>>> could be done on the Guile side, except for eventually migrating some
>>>> dependency tracking stuff over to Guile
>>> If a module imports a different module, and that module changes, even if
>>> it's macro, Guile should not blindly reuse the stale .go like it
>>> currently does.  It should complain and evaluate from source instead.
>>>
>>> That would cover the base and avoid breakage.  After, if it known how to
>>> do that, yes, it seems it'd be useful to have something similar to 'gcc
>>> -M' to provide the needed intelligence to the build system.
>>>
>>> Does that make sense?
>> Sounds reasonable, though we could go for something less general in
>> Guix first.
> I'd rather avoiding adding more complexity in Guix if it can be fixed
> upstream; where it'd benefit everyone most.

It cannot be solved upstream, this is not a pure Guile matter but also a 
build system matter. Even if this the 'if that module changes, it ... 
should evaluate from source' was implemented, the build system still 
needs to know to compile the module.

More concretely, build-aux/compile-all.scm uses the following to 
determine what needs to be recompiled:

> (define (file-needs-compilation? file)
>   (let ((go (scm->go file)))
>     (or (not (file-exists? go))
>         (file-mtime<? go file))))
--- just interpreting imported modules that have changed doesn't cause 
the importing module to be recompiled.

Also, at least one Guile maintainer wants dependency tracking to be 
orthogonal:

> <https://issues.guix.gnu.org/50960#59>
> I guess I could live with an 80% dependency tracking solution. However,
> my criteria for it would be that (1) it should be orthogonal (e.g., not
> requiring explicit calls to ‘notice-dependency’ in many places), and (2)
> it should be self-contained and reasonably small. Perhaps having
> ‘load*’ or similar instrument ‘open-file’ or similar could help achieve
> that?

Your proposal to do in Guile does not appear to match that criterium.

The notice-dependency could be eventually upstreamed, but the issue does 
not appear to be fixable upstream.

Greetings,
Maxime.

[-- Attachment #1.1.1.2: Type: text/html, Size: 3823 bytes --]

[-- 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#57121: clojure-build-system fails to compile -- backtrace from language/tree-il/peval.scm

[Maxim Cournoyer]

Hi Maxime,

Maxime Devos <maximedevos@telenet.be> writes:

> On 23-08-2022 06:06, Maxim Cournoyer wrote:
>> Hi Maxime,
>>
>> Maxime Devos<maximedevos@telenet.be>  writes:
>>
>>> On 22-08-2022 17:32, Maxim Cournoyer wrote:
>>>>> These patches are for Guix' build system.  I don't see anything that
>>>>> could be done on the Guile side, except for eventually migrating some
>>>>> dependency tracking stuff over to Guile
>>>> If a module imports a different module, and that module changes, even if
>>>> it's macro, Guile should not blindly reuse the stale .go like it
>>>> currently does.  It should complain and evaluate from source instead.
>>>>
>>>> That would cover the base and avoid breakage.  After, if it known how to
>>>> do that, yes, it seems it'd be useful to have something similar to 'gcc
>>>> -M' to provide the needed intelligence to the build system.
>>>>
>>>> Does that make sense?
>>> Sounds reasonable, though we could go for something less general in
>>> Guix first.
>> I'd rather avoiding adding more complexity in Guix if it can be fixed
>> upstream; where it'd benefit everyone most.
>
> It cannot be solved upstream, this is not a pure Guile matter but also
> a build system matter. Even if this the 'if that module changes, it
> ... should evaluate from source' was implemented, the build system
> still needs to know to compile the module.

OK.  I'd welcome fixing as much of it as possible in Guile, then adding
the Guix-specific bits on top of it.  We already carry a bit too many
things in Guix that could be in Guile proper, in my opinion.

> More concretely, build-aux/compile-all.scm uses the following to
> determine what needs to be recompiled:
>
>> (define (file-needs-compilation? file)
>>   (let ((go (scm->go file)))
>>     (or (not (file-exists? go))
>>         (file-mtime<? go file))))
> --- just interpreting imported modules that have changed doesn't cause
>     the importing module to be recompiled.
>

[...]

OK, I see that since we already implement such mechanism in Guix, it's
on our shoulders to fix it there.  I'm changing my mind but with what I
said above (as much of the fix to go in Guile, the rest on top in Guix).

Thank you,

Maxim