Fork me on GitHub
#cljs-dev
<
2018-03-03
>
dnolen00:03:25

@mfikes hrm this REPL issue seems specific to Safari unfortunately

mfikes01:03:14

@dnolen Here is timing info on a real-world Figwheel Project

mfikes01:03:39

We need good names for the 3 numbers; I hope they are obvious enough

dnolen01:03:44

@mfikes that’s pretty good

dnolen01:03:49

so 40% faster for cold

dnolen01:03:03

or a bit more than that I guess

mfikes01:03:10

Yeah, the middle number, for lack of a better way to describe it is after a lein clean

dnolen01:03:17

that’s great

dnolen01:03:41

@mfikes and this is with parallel?

mfikes01:03:21

The actual numbers in the graph are 36.5 s, 20.8 s, 17.3 s (I had to graph them though to get a better feel)

mfikes01:03:41

I'll try parallel now

dnolen01:03:41

@mfikes actually that graph is pretty nice, it shows that time on shared stuff is almost gone?

dnolen01:03:19

this Safari thing is really crazy I must say

dnolen01:03:23

seems like a Safari caching bug

mfikes01:03:32

Yeah, one say to look at it is the the difference between using the local cache and needing to copy in from the global cache is only about 3 seconds

dnolen01:03:37

even if I sleep between compiled file write, before send the JS for the load - Safari errors out

dnolen01:03:51

sleep for 3 seconds

mfikes01:03:06

This is a Safari bug that impacts the shared AOT cache logic?

mfikes01:03:56

(My numbers with :parallel-build true look pretty much the same for this project—both of the bars on the right are a little taller, but nothing to glean from it.)

mfikes01:03:27

Ahh, the Safari bug provokes the race

dnolen01:03:35

yeah I think parallel when you only have analysis caches is more overhead

dnolen01:03:09

@mfikes no it’s just a race like before, if we compile something to out and we’re already connected Safari balks about the file not being there for some reason

dnolen02:03:15

ok I have a fix which is just add a random timestamp, it’ll open a new tab window but this doesn’t seem like a big deal - Chrome already works that way

mfikes02:03:40

Ahh, so it is Safari's internal cache that needs busting... hrm.

mfikes02:03:09

Or perhaps something else, since it thinks the file isn't even there

dnolen02:03:39

if you empty the cache it works actually

dnolen02:03:59

but Safari doesn’t really seem to respect cache control headers - so there doesn’t seem to be anyway to force the issue

dnolen02:03:03

but I think this is fine

dnolen02:03:38

probably avoids other potential caching issues in other situations

dnolen02:03:05

clj -m cljs.main -e "(+ 3 4)"

dnolen02:03:19

takes 3.7 seconds on my machine

dnolen02:03:38

not too shabby given Clojure launch, ClojureScript load, browser launch, and REPL exec

dnolen02:03:22

@mfikes and I take it you didn’t see anything weird with AOT jar or global cache in your project?

mfikes02:03:58

No... I only tried compiling, I didn't go so far as to actually verify proper operation

mfikes02:03:35

Oh, I can try building Planck with the updated compiler; it has a few unit tests...

dnolen02:03:21

time clj -m cljs.main -e "(require 'cljs.core.async)"

dnolen02:03:26

real 0m10.022s

dnolen02:03:33

time clj -m cljs.main -e "(require 'cljs.core.async)"

dnolen02:03:39

real 0m4.707s

dnolen02:03:30

rm -rf out

dnolen02:03:36

time clj -m cljs.main -e "(require 'cljs.core.async)"

dnolen02:03:40

real 0m6.181s

dnolen02:03:15

might be time after this release to really dig into improving serial perf

mfikes02:03:02

I got what looks like a compiler trip-up when compiling Planck https://gist.github.com/mfikes/3ee861e6d558d0c6e3fd5d7d1a3f5bc5 Good thing is that it happened the first time so hopefully it is relatively reproducible.

dnolen02:03:39

clj -m cljs.main -v -d \mktemp -d\` -r` this is a neat way to try stuff quick

mfikes02:03:14

What? I've never heard of this command? Wow.

mfikes02:03:46

I feel like the time when I learned the caffeinate command 🙂

dnolen02:03:00

erg anyways, you know what I mean 🙂

mfikes02:03:11

Markdown ugh

dnolen02:03:50

ok that’s interesting, yeah will try to use this on a few things this weekend and try to iron out the obvious issues

mfikes02:03:49

The Planck build failure is fortunately easily reproducible, so I can see if I can find out what is going on

mfikes02:03:22

Now Canary will pay off big time 🙂

dnolen02:03:23

I expect stuff to break

mfikes02:03:29

I just kicked the Canary to see what happens

dnolen02:03:40

I must say, it’s quite cool that it’s so easy to start a ClojureScript REPL now, first time I feel like we have some competitive with my first experience with Clojure

dnolen02:03:00

and with deps.edn I fee like we finally have something that feels “easy” a la Node.js

mfikes02:03:03

When I first started coding in ClojureScript, I didn't even have a working REPL. Wow we've come a long way.

mfikes02:03:33

REPLs worked back then, I think it was because I was trying to target iOS

dnolen02:03:59

yeah oh well, the new people will be none the wiser 🙂

mfikes02:03:45

Ahh, I recall now, I coded for maybe a month or two until Colin Fleming pointed out I could probably just host JavaScriptCore in a WebView for dev and use https://github.com/tomjakubowski/weasel Hah!

dnolen02:03:24

ok done for the evening, but I think we’re in a good spot to do a lot of testing and shoot for a release not next Monday but the following Monday

dnolen02:03:40

I can take a stab at revising the Quick Start if someone else doesn’t take it

richiardiandrea03:03:33

Unbeaten path, reported a problem with socket REPL: https://dev.clojure.org/jira/browse/CLJS-2598

mfikes13:03:23

The built ClojureScript JARs no longer embed a

META-INF/maven/org.clojure/clojurescript/pom.xml
so I’ve updated Canary to no longer manipulate that file. (It would add a distinguishing build hash to the end of the version number, like 1.10.102-a0b9521.) I suppose a consequence might be that subsequent nightly Canary builds of the same ClojureScript version might collide (maybe overwrite). I haven’t looked up the details of this, but sharing in case strangeness occurs. As an aside, Coal Mine fetches ClojureScript as a git dep (using $CANARY_CLOJURESCRIPT_REV as the :sha, so it built without any changes.

Alex Miller (Clojure team)13:03:38

I noticed this when I modified the build and spent a bit of time trying to make Maven include it but did not succeed. If it becomes an issue, I’m happy to look at it again.

darwin13:03:24

@mfikes I think we will have to install the jar with first form of the command as described here: https://maven.apache.org/guides/mini/guide-3rd-party-jars-local.html and not rely on pom.xml

mfikes13:03:13

@darwin Ahh, cool, I’ll try adding our custom version to the -Dversion value

darwin13:03:49

or alternatively we could synthetise our own pom.xml at that point and use it instead of the original embedded one, not sure, I’m a maven/java noob

mfikes13:03:22

Yeah, I don’t know the consequences of any of this either.

darwin13:03:47

I’m not sure what pom.xml contains, I was under impression there could be also dependencies clojurescript itself needs

mfikes13:03:14

Oh, right. Perhaps that speaks to a larger issue.

darwin13:03:22

so installing it without proper pom.xml could give us broken clojurescript (without dependencies installed)

darwin13:03:13

what about changing install-canary.sh to work similar way as your Coal Mine? I’m not sure about consequences tough, e.g. would it be slower because it would not use AOT-ed version?

darwin13:03:08

dunno, will be back in 10mins

mfikes13:03:51

Coal Mine itself uses the clojure tool and can thus use git deps easily. Other projects (based on lein for example) really want a JAR to depend upon.

mfikes13:03:20

We might shortly see about broken dependencies with the latest job I submitted. (Hey, that’s the point of Canary, right?)

mfikes13:03:23

Changing it to install using -D stuff for now to see what happens…

darwin13:03:25

Thanks for trying it, but I’m afraid -D won’t be enough. Looking at a pom.xml from some previous ClojureScript jars and it contains quite some important deps: https://gist.github.com/darwin/6e41b44bc9f488c37fa2691c761e6e96#file-pom-xml-L24-L79

mfikes13:03:12

Yes, I agree. I think there will be a larger issue. The Canary builds will likely confirm that.

dnolen13:03:29

I’m looking at why it isn’t included now

Alex Miller (Clojure team)13:03:21

It’s due to how it’s built with the assembly plugin

mfikes13:03:34

Thanks! I'll revert the last couple of commits I have in Canary after sorted so we can re-confirm things work end-to-end.

Alex Miller (Clojure team)13:03:46

I’m curious why you are needing the pom inside the jar?

mfikes13:03:10

We may not. TBH neither Darwin nor I understand the underpinnings.

Alex Miller (Clojure team)13:03:29

Maven typically stores the Pom alongside the jar in a maven repo and that’s what the maven api uses

mfikes13:03:38

OK. That might be sufficient...

darwin13:03:01

I need to install custom-built clojurescript jar into local maven repo. Where should I get the pom.xml which maven requires?

Alex Miller (Clojure team)13:03:44

It’s just the pom.xml where you built the jar

Alex Miller (Clojure team)13:03:40

Presuming you are running script/build

darwin13:03:02

so it should be sitting next to produced jar file, somewhere here, right? https://github.com/cljs-oss/canary/blob/master/runner/scripts/build_compiler.sh#L186

mfikes13:03:56

I don't see it in target after a build

Alex Miller (Clojure team)13:03:00

No, it will be at the root

mfikes13:03:28

Confirmed, I see it there.

Alex Miller (Clojure team)13:03:38

It’s git ignored there

Alex Miller (Clojure team)13:03:52

build could copy it to target though

darwin13:03:55

ok, great, then we are all set, we upload pom.xml alongside our jar to github releases and then let canary-install.sh download it, instead looking for it inside the jar

mfikes13:03:37

(As an aside, Canary has confirmed that our current setup is insufficient, the Planck build complains about com.google.javascript.jscomp.CompilerOptions as you would expect.)

mfikes13:03:40

Yeah, seems like our "manual" Canary dep fetch just needs to be revised to deal with 2 files instead of 1 and when installing into maven use that pom.xml

darwin13:03:59

btw. embedding pom.xml into the jar still might be a good idea, if someone wanted to install it this way (method 3): https://maven.apache.org/guides/mini/guide-3rd-party-jars-local.html

mfikes13:03:29

We can also patch the pom.xml directly and when installing use the 2nd step here https://maven.apache.org/guides/mini/guide-3rd-party-jars-local.html

mfikes13:03:52

@darwin If you wanted to make any of these Canary revisions, feel free. Otherwise I might have some time later on.

Alex Miller (Clojure team)13:03:13

I can look at this again if it would be useful to include

Alex Miller (Clojure team)14:03:02

The other missing file is the build.properties which is probably less critical but is also available to include

darwin14:03:04

I see META-INF/maven/org.clojure/clojurescript/pom.properties in the older jars

Alex Miller (Clojure team)14:03:16

Sorry, that’s what I meant

darwin14:03:09

sure, no problem, you decide what you want to do and we will adapt to your solution in canary, thanks for all the info

Alex Miller (Clojure team)14:03:35

I can fix this - gimme a few to take care of dogs and children

dnolen14:03:00

I think this is because the assembly part of the pom.xml doesn’t have an archive section? If this doesn’t work I’ll defer to @alexmiller

Alex Miller (Clojure team)14:03:42

I messed with that without much success

dnolen14:03:09

ok, yeah not working for me

darwin14:03:39

I think keeping META-INF with maven meta info (even if generated by other means) in the official jar is less disruptive and allows easier standalone jar installation via maven install plugin - and that also would mean zero work for us in canary .-)

Alex Miller (Clojure team)14:03:21

there is almost certainly some better way to do it, but this works :)

dnolen14:03:59

it works for me

Alex Miller (Clojure team)14:03:09

@mfikes et al: fyi, new version of clj is out and it fixes the bug where it would swipe a single -h main arg and also enables use of the classpath cache with -Sdeps (which it turned off in the past). in case that effects any timing you’re reporting that include -Sdeps

mfikes14:03:31

Cool. Thanks I indeed noticed that -Sdeps turned off caching. I thought it was intentional given that -Sdeps might be thought of as "ephemeral", but caching is probably a good thing in that case as well.

mfikes14:03:47

OK @darwin given this I'll just revert my last two commits in Canary and give it a shot.

mfikes14:03:54

Thanks all. A new Canary job is now in flight.

darwin15:03:02

I would not patch it without reason

mfikes15:03:24

Right... I need to revert one more change I had made

darwin15:03:46

btw. you can test it with job only=cljs-devtools to just exercise it and when it will work we can run full round

mfikes15:03:06

Right. I already kicked on off. Will do that next time

darwin15:03:48

ok, no problem, I should add a comment explaining why we want to patch the version in the pom.xml

darwin15:03:09

also maybe explaining the idea behind appending sha to build version

mfikes15:03:10

Feel free to add one

darwin15:03:41

it is because we want to support forks and those can have same version numbers but different shas

darwin15:03:13

@mfikes it failed again because the release jar is already present, so it was NOT re-uploaded

darwin15:03:51

it should work next time when clojurescript repo gets new commits and canary will actually upload new jar

richiardiandrea16:03:11

David thanks I see why you say they do not compose - at the moment no socket accept fn receives repl-env and opts. It is a killer feature though, now almost all editors need an additional command for bootstrapping the (socket) Repls after startup. We could get rid of that burden.

darwin16:03:07

@dnolen just hit one weird corner case in the build script: I had a pre-1.10 fork of clojurescript repo, I did a pull to get current latest from official clojurescript repo, and pushed to my fork to get it up-to-date, that didn’t push tags by default, and the build script started failing at this line: https://github.com/clojure/clojurescript/blob/master/script/build#L28 should we attempt to make it more robust? from the error message is it not obvious what is wrong - git complains fatal: No names found, cannot describe anything.

dnolen16:03:10

@darwin I don’t think we need to do anything about that right?

dnolen16:03:30

you need to push tags

darwin16:03:35

I would add a simple bash test for failure and echo a possible explanation to instruct people to update tags in git repo

dnolen16:03:18

but who is this for other than a couple of people?

dnolen16:03:35

if you want to submit a patch for this - that’s fine by me

darwin16:03:36

ok, I agree that it really is a very rare case

dnolen16:03:47

just it seems very minor

mfikes16:03:59

@dnolen https://dev.clojure.org/jira/browse/CLJS-2601 captures the same as the Planck build failure I mentioned yesterday (but as a minimal repro)

dnolen16:03:17

@mfikes ok will take a look but it seems quite strange

mfikes16:03:39

Yeah, I hadn't looked deeply, just glad it is captured.

dnolen16:03:47

why is it looking for .cljc here?

mfikes16:03:09

Perhaps related to self-hosting. The provoking namespace requires cljs.js

dnolen16:03:16

oh sorry right

dnolen16:03:22

I see this is just a self-host issue

mfikes17:03:12

I'm putting 2018-03-12 in draft clojurescript-site content related to the release; can change if needed.

dnolen18:03:44

thanks to Tim Pote’s handshake bit, was pretty straightforward

dnolen18:03:05

anything that prints before browser REPL actually connects will still get sent over once the connection happens

richiardiandrea19:03:32

Ah nice, I think both figwheel and the boot cljs repl implement a similar queuing solution, good job!

mfikes19:03:51

If ClojureScript is an unbuilt dep, even though any artifacts produced for JAR deps are put in the shared cache, when you go to use them, since cljs.core will need to be compiled in the case of an empty output directory, this cascades a requirement to rebuild all of the shared cached artifacts (because they ultimately depend on cljs.core). A consequence, it seems, is that artifacts cached based on unbuilt ClojureScript deps are not really usable. You could argue that we add conditional logic not to cache them in the first place. But, if we ultimately end up supporting shared cached artifacts built from git deps, then if ClojureScript is itself a git dep, then things will work out in that case in the future.

dnolen19:03:51

@mfikes we probably want to open a issue for that, not sure if we’ll address before release since it doesn’t directly affect most people

dnolen19:03:42

just landed new main opt

dnolen19:03:59

-s --serve to launch a simple webserver

dnolen19:03:10

just reuses the browser REPL bits

mfikes20:03:16

@dnolen The issue above doesn’t break anything, but I’ll write a JIRA so we can take a decision on it.

dnolen20:03:48

the Quick Start is really, really short

dnolen20:03:13

still needs more work, but so far it’s mostly been about deleting stuff

mfikes20:03:05

LGTM. I was wondering whether the “`cljs.main` tour” news post would replicate a lot of what would ultimately be in Quick Start. I can now see that Quick Start has a focus on ClojureScript itself, and the use of cljs.main is just an ancillary / inherent thing that new users would have no reason to even notice.

mfikes20:03:40

I do like how it looks much shorter, and much less intimidating with lots of accidental complexity evaporating compared to the previous version.

mfikes20:03:52

And maybe one day in the future you will be able to just type

cljs -O advanced -c hello-world.core

dnolen20:03:58

@mfikes yes I think the announce and Quick Start serve different purposes, that they might overlap a bit is not a big deal

dnolen21:03:54

built-in browser now gzips JS when run standalone

dnolen21:03:11

makes it quite clear what advanced compiled ClojureScript is supposed to look like

dnolen21:03:02

with copy & paste you can definitely get through it under 10 minutes

dnolen21:03:30

@mfikes how did you do your screen shots your PRs?