Fork me on GitHub
#cljs-dev
<
2017-06-13
>
danielcompton01:06:14

Not sure if anyone has looked at Zach Tellman's https://github.com/ztellman/collection-check? Seems like that might be a good tester for the new hashing?

Oliver George04:06:41

Feels like I'm close to being able to fix but I'm not familiar with the correct approach for normalising paths (e.g. windows \\ and linux /)

Oliver George04:06:19

Specifically (System/getProperty "user.dir") returns "C:\\Repos\\utas-webapps\\frontend\\" but I need "/C:/Repos/utas-webapps/frontend/" in order to strip the prefix.

Oliver George04:06:58

Or perhaps it's the url arg which is technically the bug in this case.

tmulvaney09:06:41

@favila I mentioned that all callers of equiv are IKVReduce in patch, but was probably did make that very clear. Yes the fallback is bad - that is indeed an undefined variable i introduced. Ouch! @theller the records no longer use equiv-map it uses a set of fields checks and then a regular = check on the extmap as of cljs-2079. Also, the call made to 'equiv-map' by PAMs are redundant, it is made in the else branch of the if statement: (if (map? other) (not (record? other)) .... (equiv-map coll other)) which is pointless as 'other' is known not be a map by this point so equiv-map will always return false.

tmulvaney10:06:01

I see my typos was fixed. Thanks @dnolen ! btw I can make a test case for the fallback case if it would be helpful.

rauh11:06:11

I remember calling private functions used to be an error. Eg, I once had to hack around it like (js/cljs.core.fn__GT_comparator ...). Did that change?

dnolen11:06:51

@olivergeorge thanks for the clarifications on the ticket, very helpful

Oliver George12:06:16

dnolen: Thanks! I'll do that.

Oliver George12:06:53

I'm still not clear on why the bug is exhibited specifically when using :preload which leads me to think there's something more to it. I'm struggling to find that and don't have time just now to get more familiar.

Oliver George12:06:06

This fix should address the immediate issue and seems safe.

dnolen11:06:57

@tmulvaney sure, go for it

dnolen11:06:37

@rauh yeah I was pretty sure no one had worked on that one

dnolen11:06:44

not really interested in error

rauh11:06:11

Hmm, I wonder where I got the error from. I could've sworn it didn't work using a normal (fn->comparator ...) call. Oh well...

dnolen13:06:45

@olivergeorge your patch looks OK but you need to follow the patch guidelines here https://clojurescript.org/community/patches

dnolen13:06:56

I also upgraded your JIRA permissions, you should be able to edit tickets now etc.

mfikes13:06:39

PSA: If you are authoring patches, you can now also include generative tests (using test.check). The first such test has landed, exercising the ability for this to properly span JVM and self-hosted.

favila13:06:33

should there be a separate test workflow for those? generative tests take much longer to run

dnolen14:06:25

what does Clojure do here?

dnolen14:06:41

they have test.check tests now too

mfikes15:06:27

Instances defspec seems to argue that Clojure hasn’t yet started to worry about test run time. (They are mixed in and there aren’t very many yet.)

dnolen15:06:02

yeah that’s what I thought - I’m not really concerned

dnolen15:06:21

I don’t think we should be running long generative tests anyway

dspiteself16:06:11

We are seeing 15-30% decrease in total script time after CLJS-2079 and CLJS-1977 from Chrome's performance tab.

dspiteself16:06:24

I know we have a strange use case, but this is a big deal for us. Kudos all around.

dnolen16:06:39

not at all strange, these are important perf tweaks

Alex Miller (Clojure team)16:06:53

the Clojure JVM build has both unit tests and generative tests (actually these are the old test.generative, not test.check) and it’s possible to run them independently or together. I usually run them both as the generative ones don’t actually take that long right now.

Alex Miller (Clojure team)16:06:18

we also have test.check tests but they are currently mingled into the unit test segment (because reasons)

dnolen17:06:27

@alexmiller thanks good to know

Oliver George23:06:26

Hi All. Possibly silly question. What's the recommended approach to building the clojurescript compiler on Windows? I used a docker vm to avoid the issue but perhaps there's a less fiddly way.

Oliver George23:06:46

I get errors running script/build using the Git bash install on windows.

Exception in thread "main" java.io.FileNotFoundException: Could not locate cljs/closure__init.class or cljs/closure.clj on classpath., compiling:(C:\repos\clojurescript\script\aot.clj:1:1)

Oliver George23:06:44

In case it's of interest this was my docker work around

docker run -v c:\repos\clojurescript\:/app -w /app -it clojure

darwin23:06:10

sounds like a problem between forward/backward slashes in file paths

darwin23:06:29

maybe you could try to run it in linux subsystem in windows (I have never tried that before)

Oliver George23:06:30

Interesting idea.

Oliver George23:06:50

I wonder if that's a likely future standard approach or more a novelty.

darwin23:06:43

I dunno, it is pretty new, but it should do the work for you by translating windows paths to unix paths transparently