Fork me on GitHub
#tools-build
<
2022-02-04
>
Jungin Kwon03:02:33

I am getting this error, when I build with clojure -T:build uber. I am using v0.6.7 tools.build. Sometimes the build succeeds and sometimes it fails with this error. Anyone know why?

Syntax error compiling at (clojure/tools/namespace/parse.cljc:55:19).
No such var: reader/read

hiredman03:02:01

What version of tools.deps? There was a code loading race condition that could manifest oddly like that

hiredman03:02:19

0.12.1098 and later should have the race I am thinking of fixed

Alex Miller (Clojure team)03:02:00

Yeah, I would bump to latest tools.build, that should be fixed

seancorfield03:02:15

FWIW, tools.build 0.6.7 brings in t.d.a 0.12.1071 so, yeah, use a later version to see if it helps @jungin.kwon1

Jungin Kwon03:02:10

Thank you! 👍

flowthing13:02:34

I'm migrating a project from Leiningen to deps.edn and tools.build. In my project.clj, there's this:

:profiles {:dev {:aot [db.migration.migrations]}}
My (possibly incorrect) understanding is that this tells Leiningen to AOT-compile db.migration.migrations such that the generated classes are in the classpath for REPL use. In other words, if db.migration.migrations changes, I don't have to manually recompile it before running lein repl — Leiningen will take care of that for me. With tools.build, I can define a task like this:
(defn compile-migrations
  [_]
  (build/compile-clj
    {:src-dirs ["src"]
     :basis basis
     :ns-compile ['db.migration.migrations]
     :filter-nses ['db.migration.migrations]
     :class-dir "classes"}))
Then do clj -T:build compile-migrations before starting a REPL (with "classes" in the classpath) to make the classes available for dev. This does mean that I need to run clj -T:build compile-migrations manually whenever db.migration.migrations changes, though. That's not a big deal, but I just wanted to make sure my understanding's correct and that this is indeed the way to do this with tools.build.

borkdude13:02:55

@flowthing why AOT-compile db-migrations at all for dev?

flowthing13:02:54

These migrations generate a bunch of Flyway classes via gen-class. I don't think there's an alternative to AOT-compiling them?

borkdude13:02:24

I guess you can always call compile manually from the REPL if those sources change

flowthing13:02:52

Right, but I'll have to restart the REPL after that, though.

borkdude13:02:18

I guess you could put this in your user.clj

borkdude13:02:43

(ns user)
(binding [*compile-path* "target/classes"]
  (compile 'db.migrations))

flowthing13:02:50

That was my first thought, but I'm not sure that there's a benefit over making a tools.build task that you need to run beforehand, since you need to restart the REPL anyway.

flowthing13:02:23

Also, :filter-nses is useful here, since I really don't want to AOT-compile anything except db.migration. migrations for dev.

borkdude13:02:26

the benefit would be preventing starting up 2 JVMs, so it will likely save 5 seconds or so of startup time

borkdude13:02:52

tools build runs in another JVM and then starts another JVM for compiling clj and then you start another JVM for your REPL

borkdude13:02:57

which can be a bit slow

flowthing13:02:01

But I only need to run the compile-migrations task (and spin up the second JVM) if the migrations change.

flowthing13:02:09

No need to run it every time I start a REPL.

borkdude13:02:06

Personally I would make a bb.edn with a bb dev task which checks if sources have changed using fs/modified-since which would then invoke tools build if necessary and then invokes your REPL

borkdude13:02:44

but you can do something similar in your user.clj as well

borkdude13:02:15

why use tools.build for this if you can just compile from the same dev REPL process, it's much cheaper to do so. (compile 'db.migration) is the same as :ns-compile '[db.migration]. Compilation is always a transitive process: all transitively required namespace will also be compiled, no matter what filter-nses is.

flowthing13:02:53

That's an interesting thought, although I'm not sure I want to bring in Babashka just for that. Also, there are many ways to start a REPL, so it's not something I'd be willing to bake into something like bb dev.

borkdude13:02:29

Sure, whatever tool you use to start the REPL (make, bash, manual invocation), it's conceptually the same idea: you need to do something, but only sometimes, before starting a REPL. Or you can do that work inside your REPL.

flowthing14:02:31

> why use tools.build for this if you can just compile from the same dev REPL process, it's much cheaper to do so. I don't know that it's cheaper since I need to restart the REPL anyway?

borkdude14:02:56

it's cheaper in terms of how much work is done. you're invoking 3 JVMs instead of just 1 REPL JVM.

borkdude14:02:52

but if that's not a problem, go for it ;)

flowthing14:02:12

Well, 3 vs. 2. 🙂 I'm not sure I'm convinced that the REPL approach will take up less time.

borkdude14:02:24

3 vs 1. Why do you think there's 2?

flowthing14:02:32

Need to restart the REPL.

borkdude14:02:56

Oh, then it's 4 vs 2.

flowthing14:02:30

1. Start REPL. 2. Eval (compile 'db.migration.migrations). Class files appear in classes/. 3. Shut down REPL. 4. Start new REPL with classes in classpath.

borkdude14:02:07

No :) Restart REPL. user.clj: check if sources have changed, then call compile then load the rest of your app. Just 1.

flowthing14:02:43

All right, I have no idea what's going on anymore...

borkdude14:02:20

To keep it simple, I think just a (compile 'db.migrations) in your user.clj as the first thing you do (regardless if anything's changed), will be sufficient.

borkdude14:02:27

When you change source, just restart your REPL and that's it.

borkdude14:02:58

Do not require any other namespaces from your app prior to that

flowthing14:02:22

Oh, right. That might be what's messing things up.

flowthing14:02:49

Just to be clear: do you mean having (compile 'db.migration.migrations) at the top level of user.clj?

flowthing14:02:31

Caused by: java.lang.RuntimeException: *compile-path* not set

flowthing14:02:42

If I do that.

borkdude14:02:48

(binding [*compile-path* "target/classes"] (compile '...))

flowthing14:02:09

Oh, I see. :thumbsup: Let me try that.

borkdude14:02:20

or whatever your classes dir is.

flowthing14:02:10

Cool, that seems to work! So now I have this:

(binding [*compile-path* "classes"]
  (compile 'db.migration.migrations))

(ns user (:require ,,,))

;; etc
I just need to ensure that classes/ exists. I guess the reason I'm confused is that I feel quite certain I tried a variant of this and couldn't get it to work, but maybe the key is having the (compile ,,,) call at the top level before the ns form? Another (entirely plausible) option is that I made a stupid mistake previously that left me under the impression that I would need to restart the REPL after compiling for the changes to take effect.

borkdude14:02:09

You can also write:

(ns user)

(binding [*compile-path* "classes"]
  (compile 'db.migration.migrations))

;; rest of your user.clj
(require ')

borkdude14:02:48

as an optimization you could do a check with fs/modified-since or similar to check if compilation is necessary, but I assume compilation itself doesn't take that long that it makes a huge difference

flowthing14:02:02

Yep, could use require instead. :thumbsup:

flowthing14:02:16

Yeah, compilation doesn't seem to take that long, but I'll keep that in mind.

borkdude14:02:52

To ensure classes exists: (fs/create-dirs "classes")

flowthing14:02:16

Well, I tried putting (binding [*compile-path* "classes"] (compile 'db.migration.migrations)) into (user/start) instead of at the top level and it still works. Better to have it at the top level so that the migrations are compiled even when I don't run (user/start), though. I have zero idea why it works now and didn't before, though, but I think Stupid User Error is the only possibility. 🙂

borkdude14:02:53

Congrats on getting it working

flowthing14:02:09

Nonetheless, many thanks for the help! This is much better than having the compile-migrations task. 🙂

flowthing15:02:28

Oh, not quite there yet, actually.

λ rm -rf classes/*
λ clj -T:build compile-migrations
λ ls classes
db
λ rm -rf classes/*
λ clj -M:dev -r # with (compile ,,,) in user.clj
Clojure 1.10.3
user=>

λ ls classes
clojure cprop   db      hugsql  myapp

flowthing15:02:22

Having other AOT-compiled classes than db.migration.migrations messes things up in dev. Well, need to revisit this tomorrow.

borkdude15:02:35

You cannot have (compile 'db.migration) only have the db space compiled. If it depends on other namespaces, those are also going to be compiled. This will be the same with tools.build. But what filter-nses does is that it only copies some of the compiled files from a temporary dir to the class dir.

borkdude15:02:12

You could have the same if you set compile-path to some tmp-dir and then only copy over the db dir to the class dir.

flowthing15:02:59

Yes, I figured it must be something like that. Well, need to think about this a bit.

borkdude15:02:01

but since these are dependencies, they shouldn't be changing, so maybe it doesn't hurt to have their compiled namespaces in here too.

borkdude15:02:33

if you remove them, clojure will compile them into memory bytecode anyway, again 🤷

borkdude15:02:19

some people use compile to make an AOT cache for their deps to have their REPL start up faster, which is basically what you're doing here, but only for the transitive deps of db.migrations

flowthing15:02:27

It does seem to hurt, but I don't know why yet. I evaluated (tools.namespace.repl/refresh ,,,) and got an exception related to one of the AOT-compiled namespaces, but I didn't have the time to look further into it yet.

borkdude15:02:47

omg, tools namespace refresh... let's not go there ;)

flowthing15:02:00

I don't want to use t.n.r, but that's the way the project is set up, unfortunately. 🙂

flowthing15:02:20

Guess I could try to rip it out, maybe...

borkdude15:02:24

ok, delete everything from the classes dir except db and then you should have a similar thing as with tools.builder filter-nses

flowthing15:02:51

Yes, I'll consider that, thanks. :thumbsup: