Support bootstrap ClojureScript

[mfikes]

With some mild changes, bootstrap ClojureScript can be supported by Specter, thus broadening its usability with that target environment.

The main thing that would need to be revised is allowing the macros namespace to be compiled as ClojureScript code, and the only thing that currently prevents this is the use of :use here https://github.com/nathanmarz/specter/blob/585637b4fe3529086f05ad09d07266da42463c6e/src/clj/com/rpl/specter/macros.clj#L2

It's tedious, but if this is instead revised to a :require with explicit :refers, then things work. Here's an experimental change I made in a fork: https://github.com/mfikes/specter/blob/48c6f7b4876e37c734489e1882c17dc9a293459e/src/clj/com/rpl/specter/macros.clj#L2-L21

With this, I am able to use Specter with Planck, and I suspect that Specter would be generally usable in any bootstrapped ClojureScript environment.

Here is an example:

$ planck -c target/specter-0.9.3.jar
Planck 1.11
ClojureScript 1.8.44
    Docs: (doc function-name-here)
          (find-doc "part-of-name-here")
  Source: (source function-name-here)
    Exit: Control+D or :cljs/quit or exit or quit
 Results: Stored in vars *1, *2, *3, an exception in *e

cljs.user=> (require '[com.rpl.specter :refer [transform ALL]])
nil
cljs.user=> (transform [ALL :a even?]
       #_=>               inc
       #_=>               [{:a 1} {:a 2} {:a 4} {:a 3}])
[{:a 1} {:a 3} {:a 5} {:a 3}]

(Note that, to do the above requires master of Planck, because the currently released version of Planck doesn't have support for the clojure.core.reducers namespace. Master of Planck is installable via brew install --HEAD planck.)

I also tried running Specter's unit tests using Planck. This is evidently currently not yet possible because of something that needs to be sorted with clojure.test.check for bootstrap ClojureScript, but I suspect that this issue is resolvable, and with that, it would be possible to ensure Specter formally passes its test for this environment.

[nathanmarz]

I'm 100% on board with this. I changed the code on master to alias the namespace for impl instead of doing a use. Is this now compatible with Planck? Also, very soon Specter will no longer be using clojure.core.reducers for the cljs version.

[mfikes]

Thanks @nathanmarz !

I took a quick look at your latest commit. I haven't tried it yet, but it looks like you are using prefix list notation in the ns form, which isn't supported in ClojureScript.

This should work though:

(ns com.rpl.specter.macros
  (:require [com.rpl.specter]
                 [com.rpl.specter.impl :as i]))

[mfikes]

@nathanmarz Following up on my last comment:

Yes, just to confirm, I tried Specter master and the prefix list notation causes an issue for ClojureScript:

cljs.user=> (require '[com.rpl.specter :refer [transform ALL]])
Could not eval com.rpl.specter.macros
Only :as alias and :refer (names) options supported in :require; offending spec: [com.rpl.specter [impl :as i]] at line 1 
nil

FWIW, the prefix-list ClojureScript constraint is covered here: https://github.com/clojure/clojurescript/wiki/Differences-from-Clojure#namespaces

More importantly, though, my comment above has an issue in that circularity exists. The [com.rpl.specter] spec can't be there otherwise the ns processing will go into a loop:

cljs.user=> (require '[com.rpl.specter :refer [transform ALL]])
Could not require com.rpl.specter
Maximum call stack size exceeded.

Removing it, causes things to work. Here is the exact change I tried, after updating to your upstream changes and then fixing things to not use prefix-list notation:

https://github.com/mfikes/specter/commit/eb31f0c96fb84f68f7be66c02f37f56605948373

[nathanmarz]

OK, I made the change in master. What's the easiest way to get the tests to run against bootstrap cljs? I'd like to make sure that continues to be supported as the project evolves.

[mfikes]

Currently Planck is the only environment I'm aware of that supports cljs.test in bootstrap mode (I had to make some minor changes to port that namespace.) I'm hoping we can revise the version of cljs.test that ships with the compiler, thus making it easier for projects like Specter to run their tests in bootstrap mode.

I also plan on sorting what is going on with test.check—this should lead to an ability to at least run Specter's unit tests with Planck in the interim.

This is not ideal—even the ClojureScript compiler currently can't run its own unit tests in bootstrap mode, while Planck can run the ClojureScript compiler's tests. So the goal would be to push this capability upstream so Planck need not be the only way to address testing.

[mfikes]

FYI, David Nolen expressed interest / approval in a patch that would make cljs.test usable for bootstrap environments. See http://dev.clojure.org/jira/browse/CLJS-1626 This would be a big step towards addressing Nathan's last comment.

[nathanmarz]

Thanks for the thorough explanation.

[nathanmarz]

OK, the master branch no longer uses core.reducers for the cljs version. Now all that's left for this is sorting out the test stuff.

[mfikes]

Cool. I confirmed that the released version of Planck (1.10) works with Specter master.

(As an aside, I had to tell Planck to enable :static-fns true by additionally passing -s to Planck. This has nothing really to do with Specter, but is a defect in JavaScriptCore, covered here http://dev.clojure.org/jira/browse/CLJS-910).

[mfikes]

A patch to make cljs.test usable from bootstrap has now landed. https://github.com/clojure/clojurescript/commit/28e040d41f3a83901b6391898f51c4bfc2622452

Also, a repo exists showing (at lest one way) how to use this downstream: https://github.com/mfikes/titrate

So, things remaining to get support for testing Specter are the ClojureScript support to be formally released, and sorting out things like test.check in bootstrap. :)

[mfikes]

Progress:

1. The cljs.test revisions for bootstrap have now been formally released with ClojureScript 1.8.51
2. test.check can be made to work with bootstrap, with work proceeding with maintainer of that lib "in the loop" via http://dev.clojure.org/jira/browse/TCHECK-105

[nathanmarz]

The code in master may have some breaking changes with regards to bootstrap support. In particular: https://github.com/nathanmarz/specter/blob/master/src/clj/com/rpl/specter/macros.clj#L369

• Will the hack to determine in the macro whether emitting for cljs or clj work in bootstrap?
• What should be used to retrieve a uuid string when run in cljs rather than java.util.UUID?

[mfikes]

Hi Nathan, yes, the hack works. In particular for this source:

(ns foo.core)

(defmacro testme []
  (if (contains? &env :locals) :cljs :clj))

at the Planck REPL:

cljs.user=> (require-macros 'foo.core)
nil
cljs.user=> (foo.core/testme)
:cljs

For UUIDs, while ClojureScript has a UUID type (supporting the tagged literal), it appears that people resort to random number generation. (I looked and didn't see anything in the Google Closure Library).

Since you are worried about birthday collisions, this SO http://stackoverflow.com/a/2117523 has some analysis that seems to imply a 1 in a million chance.

Easy enough to do in ClojureScript using the (private) js* special form:

cljs.user=> (js* "'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function(c) {var r = Math.random()*16|0,v=c=='x'?r:r&0x3|0x8;return v.toString(16);});")
"d371220d-229b-4259-b3e0-92f3559d06fb"

And, (it doesn't appear you need it), but there is a constructor in cljs.core:

cljs.user=> (uuid "d371220d-229b-4259-b3e0-92f3559d06fb")
#uuid "d371220d-229b-4259-b3e0-92f3559d06fb"

I tried Specter master in Planck and it hits the fact that this namespace is not available: [clojure.tools.macro :as m]

[mfikes]

I tried modifying Specter master to see how far I could get with Planck, which the experimental changes here https://github.com/mfikes/specter/commit/bdef52461970f8a8bf77b46065e0858370247cd7

• I copied name-with-attributes into the namespace to avoid the dep.
• Since walk/macroexpand-all doesn't exist in ClojureScript, I cobbled it together based on an idea in this post. Since this is just an experimental hack, this pulls in cljs.js as a dep, which makes the namespace specific to ClojureScript in the fork / hack.
• I hacked the lack of UUID issue by just generating a random string of 50 digits

With this I could at least load things into Planck, but not good with runtime behavior yet:

cljs.user=> (require-macros '[com.rpl.specter.macros :refer [transform]])
nil
cljs.user=> (require '[com.rpl.specter :refer [ALL]])
nil
cljs.user=> (transform [ALL :a even?]
       #_=>               inc
       #_=>               [{:a 1} {:a 2} {:a 4} {:a 3}])
undefined is not an object (evaluating 'clojure.lang.Cons') at line 1

I suppose the value in the above is not so much in helping identity fixes, but at least in pointing out the things that break with self-host.

[nathanmarz]

OK, good to know. I also pushed some new changes (bug fixes) that required me to use riddley.walk instead of clojure.walk, but that only affects the Clojure implementation. So using your walk/macroexpand-all replacement should be fine for ClojureScript.

I did a little googling on proper UUID's in javascript, and the results were not encouraging. The best approach might be to remove the reliance on UUID's for the ClojureScript version. David Nolen had an idea about doing dynamic calls to def and exists? for the inline caches, something that can't be done in Clojure.

Not sure what that evaluation error is all about – if you change the code to do an (i/spy ...) around the output of the com.rpl.specter.macros/path macro that could give some insight.

[mfikes]

I'll see what I can figure out using i/spy.

Another thing to consider is changing the macros.clj file to be conditional (cljx or cljc) so that the macroexpand aspect can be employed only when using self-hosted ClojureScript. (In regular ClojureScript there is no issue given the macros are compiled as Clojure.)

[mfikes]

The clojure.lang.Cons issue was fairly straightforward (and quick to pinpoint with a quick (pst) after it occurs): It is some Clojure-specific code in fn-invocation? that can be worked around:

https://github.com/mfikes/specter/commit/084be5cffed623aefef9a05157bb3ae8c5506a54

(Note that, with respect to the change above, (list? (cons 1 ())) yields true in ClojureScript.)

With this, things work, at least for this sequence :)

cljs.user=> (require-macros '[com.rpl.specter.macros :refer [transform]])
nil
cljs.user=> (require '[com.rpl.specter :refer [ALL]])
nil
cljs.user=> (transform [ALL :a even?] inc [{:a 1} {:a 2} {:a 4} {:a 3}])
[{:a 1} {:a 3} {:a 5} {:a 3}]

[nathanmarz]

OK, I made all these changes in Specter and the tests are still passing. Can you verify the tests pass under bootstrap?

The last remaining action item is handling the UUID issue properly. I'll look into this more – at the moment it's using your random string code.

There's also room for further improvement later on to improve the path macro when run in bootstrap environment vs. normal cljs environment. Since bootstrap has eval, the same strategy as used by the clj implementation can be used for inline caching. The end effect of this is improved performance. I'll open an issue for this.

Finally, to ensure bootstrap support is maintained for future releases, there needs to be an easy way to run the tests via bootstrap – like lein bootstrap test or something like that. What's the best way to do this?

[mfikes]

Yeah, my rough thoughts on tests:

1. A build of test.check needs to be available that supports bootstrap. (Either the patch in http://dev.clojure.org/jira/browse/TCHECK-105 needs to be accepted or a custom build needs to be made using that patch.)
2. Then scripts and a little bit of bootstrap / self-host code can be added to the Specter tree making it possible to run the Specter tests in a bootstrap environment.

The idea behind (2) is it causes the Specter code to actually be loaded and compiled by the bootstrapped ClojureScript compiler. This approach is being used, for example in a port I have of core.async that targets bootstrapped ClojureScript: https://github.com/mfikes/andare/commit/8bffb637081d6d1476d042be36fe75074e1a44c7 This same technique is used in the ClojureScript compiler itself to ensure its own tests work in bootstrap mode, and also in the test.check patch. A repo showing targeting all three of Clojure / ClojureScript JVM and bootstrap ClojureScript is at https://github.com/mfikes/titrate

The unfortunate aspect of this is that it requires you to (a) get the cljs.jar (since Specter itself targets an older version of ClojureScript), and (b) have Node.js. But once those annoying bits are addressed, it really boils down to another test script to run.

I'm happy to help put this stuff together. (Perhaps in a separate branch you could copy in, or as a PR, or whatever mechanism works best for you.)

I kind-of wish the test.check patch would go through... that's the main bit I think this is waiting on.

Then, of course, there might be issues with Specter itself, getting it to run through tests in this way—but that's I suppose what you want: To identify anything that might be broken in some corner of the code.

[mfikes]

Hey Nathan, I tried the latest (after your changes for bootstrap). One require/alias is needed: https://github.com/mfikes/specter/commit/7b6e32ff590c8f03a5a096fb63ff0a516ea71c9a

[mfikes]

Oh Nathan, another thing I forgot to mention regarding test.check: It's API (or at least its namespaces) has been revised with the latest releases (that's where the self-host patch is pending).

[nathanmarz]

I tried adding that require but now I get a "No such namespace: cljs.js" error when running the tests in the node repl.

[mfikes]

Ahh. OK. cljx must have different semantics than reader conditionals. (FWIW reader conditionals will take the :cljs branch in macro namespaces when being compiled under bootstrap, but the :clj branch when macro namespaces are being compiled for regular JVM-based ClojureScript.)

Given that, this works in bootstrap: https://github.com/mfikes/specter/commit/6df3fa016de2d89dc5d86bc18951533a7d89be1d

It relies on the fact that, by definition, if you are in bootstrap, then cljs.js has already been loaded. So, it is abusing the concept of using a namespace that hasn't been explicitly required, but banking on the implicit aspect.

[nathanmarz]

Made that change and now the tests run fine.

As for getting the tests running under bootstrap, a pull request with the requisite changes would be great (once all the test.check stuff you mentioned is sorted out). Will leave this issue open until those changes are made as that is the time when bootstrap compatibility can be easily enforced moving forward. Until then the bootstrap test runner will be an remote procedure call to @mfikes :)

[mfikes]

Cool. Do you think Specter can be updated to the latest test.check release and API? Or, does that cause an issue with Specter striving to be backwards compatible (and usable in older ClojureScript environments)?

[nathanmarz]

I actually don't know what versions of ClojureScript are important to keep supporting. So far I've been trying to support as many versions of Clojure and ClojureScript as possible – but now that 1.9.0 is on its way I'm open to dropping compatibility guarantees for older versions. Based on reading the test.check changelog it looks like I would have to upgrade to Clojure 1.7.0 (and I'm assuming ClojureScript 1.7.x as well?) Since you know a lot more about ClojureScript than I do, what are your thoughts on which ClojureScript versions are important to maintain compatibility with?

[mfikes]

Back around the time this post occurred http://swannodette.github.io/2014/12/31/the-old-way-the-new-way/ there were some breaking changes that causes a bit of difficulty with forwards and backwards compatibility. Since that time frame, it has essentially been a continuous (roughly monthly) sequence of releases without any real compatibility hiccups or important milestones that often come up as a compatibility discontinuity.

So, I'd suggest supporting back as far as possible. But to be honest, I don't know how to determine what that might be. For example, if you require Clojure 1.7.0, I think versions of ClojureScript prior to 1.7.28 still work.

In terms of supporting self-host, consumers of Specter will need 1.7.28 or later (http://swannodette.github.io/2015/07/29/clojurescript-17/) but to be honest, such users are likely to be on a much more recent version because a lot of fixes have occurred in self-host functionality. But, you can almost set up a specific self host profile or copy in a released cljs.jar, so I think the self-host aspect can be a bit decoupled from what you choose to put in profile.clj—like it has been so far.

[mfikes]

Speaking of UUIDs and test.check, this is interesting relevant prior art: https://github.com/clojure/test.check/blob/a9e15ab4d884097f2e0636e98d047f46a20b7cc8/src/main/clojure/clojure/test/check/generators.cljc#L1276-L1309

Edit: I suppose in test.check's use case, they need to honor a seed to generate the same sequence of UUIDs.

[nathanmarz]

Thanks for the info. I think I'll release 0.11.0 targeting the versions it always has, and for 0.12.0 I'll upgrade to Clojure 1.7.0 and the lowest version of ClojureScript that will enable everything we need with bootstrap.

Also, I removed the reliance on UUID's for the ClojureScript implementation so we don't have to worry about that anymore.

[mfikes]

Support for bootstrapped ClojureScript has landed in the test.check repo via TCHECK-105.

In preparation for when the JAR is available, I'm happy to help set up a script that can run Specter's unit tests completely in self-hosted mode (either under Node or Nashorn if you have a preference).

It also looks like Specter needs to be updated from test.check 0.7.0 to 0.9.x (which appears to involve some breaking changes in the the test.check API (I don't have a feel for what the work entails here, other than I know if you try to change the dep to the latest released test.check in Specter's project.clj file, things don't work).