Fork me on GitHub
#cljs-dev
<
2017-05-05
>
vikeri11:05:59

I have an interesting issue, when I get an circular dependency through a macro it won’t be detected when using :parallel-build true. If I disable parallel build it detects it. Tested with 1.9.521

vikeri11:05:15

What happens instead is that the process just freezes

dnolen17:05:51

@vikeri “Circular dependency through a macro” what does that mean?

vikeri09:05:22

dnolen: In my macro I call a function say my.ns/myfn and then I use that macro in my.ns2 on which my.ns depends without noticing it. First the compiler will complain that my.ns does not exist in my.ns2. I then require it at the top. Then I've created a circular dependency but I never call a function in my.ns explicitly in my.ns2 but only through the macro. This circular dependency was not picked up by the compiler when compiling in parallel. Instead the compilation just deadlocks and I have to kill the process. Not sure if it's clearer now? Would like to create a minimal reproduceable version of it but I'm quite swamped with work so atm I don't have time unfortunately.

dnolen17:05:21

right but this isn’t supported anyway

dnolen17:05:32

remove the circular dependency

vikeri16:05:41

The issue is not that the circular dependency is not supported, it’s that the compiler won’t tell me that I have a circular dependency as it normally does but it will just freeze up instead. The expected behaviour would be to throw an error saying that I have a circular dependency (like it does without parallel compilation) but instead it just freezes without giving me a hint what the issue is.

dnolen12:05:50

the circular dependency check we have for explicit cases

dnolen12:05:12

you have an implicit case

dnolen12:05:57

an issue for that is welcome, but we’ll need a minimal repro in the ticket which will probably be somewhat challenging to construct

vikeri16:05:46

Exactly, ok when I get time for it I’ll try to flesh out a minimal repro and file a ticket

dnolen17:05:30

Clojure nor ClojureScript support circular deps anyway

frank18:05:26

@vikeri how does one reproduce this?

anmonteiro18:05:53

@tmulvaney In Clojure,

(type (find {:a 1} :a))
clojure.lang.MapEntry

anmonteiro18:05:14

does your recent patch also return a MapEntry for find?

dnolen18:05:16

I just changed cljs.spec to cljs.spec.alpha now would be a good time to try master - would like to cut a release for this

anmonteiro19:05:02

@dnolen so you didn’t delete .spec namespaces? e.g. cljs.spec.impl.gen

anmonteiro19:05:21

or cljs.spec.test

anmonteiro19:05:42

we’ll have cljs.spec.test and cljs.spec.test.alpha?

dnolen19:05:58

no those should be gone

dnolen19:05:52

@anmonteiro oops your right fixed now

anmonteiro19:05:54

@dnolen another question is: you added cljs/spec/impl/gen because there was a cljs.spec/gen var.

anmonteiro19:05:02

should we change it back or keep impl?

anmonteiro19:05:23

I don’t think it’s a pressing issue, just thought I’d start the discussion

dnolen19:05:35

oh right, we can change that

dnolen19:05:31

changing it now thanks

dnolen19:05:59

changes applied to master

mfikes20:05:47

Guessing simply cljs.spec.alpha$macros/speced-vars

anmonteiro20:05:08

putting together a patch

mfikes21:05:17

With ^, the spec -> alpha changes LGTM