Fork me on GitHub
#cljs-dev
<
2015-10-29
>
juhoteperi19:10:07

Hmh, managed to fix the problem with recompile-dependents on cljs-1437 but now I found another problem

juhoteperi19:10:29

Analyzer locate-src presumes there is single source root: https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/analyzer.cljc#L1640 and this means that it can't find some namespaces which are only available in the input directories and not classpath

dnolen20:10:15

@juhoteperi is classpath case covered by cljs.util/ns->source ?

dnolen20:10:52

then what’s the problem? locate-src calls ns->source

juhoteperi20:10:19

A namespace which on input directories but not on classpath

juhoteperi20:10:15

Like (inputs "src" "src2") where classpath only includes src

dnolen20:10:16

right but that’s not a really a case we have ever cared that much about

dnolen20:10:29

everything really does need to be on the classpath

dnolen20:10:02

the file based support is just a historical thing

dnolen20:10:12

that we haven’t removed primarily for backwards compatibility

dnolen20:10:31

but I would say in modern ClojureScript dev everyone just uses the classpath

dnolen20:10:41

there are too many cases now where you get burned if you don't

dnolen20:10:46

3rd party REPLs, 3rd party tools

juhoteperi20:10:19

Is it problem if we break backwards compatibility now? Because I'm currently not setting (:root <@U095WMJNR>/*compiler*) to anything

dnolen20:10:52

I don’t see why we need to do that

dnolen20:10:11

if we don’t need to break backwards compatibility we just don’t do it

juhoteperi20:10:37

Well, it would be easy to set it when build is only passed a file/dir/url etc. but when passed a cljs.build.api/inputs or other it won't be possible to set it to meaningful value

juhoteperi20:10:49

The problem can be reproduced with https://github.com/mneise/cljs-1437-test and by running touch src/cljs_1437_test/core.cljs && ./scripts/build

juhoteperi20:10:56

Core.cljs depends on namespace foo2 from a directory which is not on classpath and because locate-src doesn't work analysis cache for foo2 won't be loaded

dnolen21:10:34

@juhoteperi: I just don’t understand the issue, is there something about your patch that makes it not possible to support this anymore?

juhoteperi21:10:58

@dnolen: Yes. There is no longer a single root directory.

juhoteperi21:10:14

Before the patch compile-root was called per input directory, like (inputs "src" "src2") called (compile-root "src") and (compile-root "src2") so (:root <@U095WMJNR>/*compiler*) was set to one directory at a time. Now that all files are compiled on one pass instead of per directory it's not possible to set the (:root <@U095WMJNR>/*compiler*) to a single directory.

juhoteperi21:10:58

If we want to support this case, I have some ideas how to implement locate-src so that it can search for sources in given input directories

dnolen22:10:00

ah, ok I think we should pursue that, I think losing this would come as big surprise to existing users

dnolen22:10:27

even though intuitively people know they have to specify the classpath

dnolen22:10:40

changing this could break a lot of builds where this was accidentally not the case before