Babashka compatibility about fipp

[borkdude]

Here's a screenshot with puget + the above changes made in a local checkout of fipp:

[borkdude]

Note that zprint already works with bb:

(ns zprint
  (:require [babashka.deps :as deps]))

(deps/add-deps '{:deps {org.babashka/spec.alpha {:git/url "https://github.com/babashka/spec.alpha"
                                                 :git/sha "1a841c4cc1d4f6dab7505a98ed2d532dd9d56b78"}
                        zprint/zprint {:mvn/version "1.2.3"}}})

(require '[zprint.core :as zp])

(println (zp/zprint-file-str (slurp *file*) "yolo" {:color? true :style :dark-color-map}))

but some users would like to use fipp as well.

[borkdude]

Submitted a PR to address point 2: #82
I think that change would be good in general, not only for bb.

[brandonbloom]

Hey @borkdude, thanks for the detailed writeup and PR #82 (merged!).

In general, I'm willing to make improvements (cleanup, fixes, etc) that also extend compatibility, but would like to avoid accreting compatibility cruft. On that front, I'm disinclined to eliminate the dependency on rrb vectors & would instead encourage you to upstream the compatibility fixes there.

Perhaps worth mentioning is that Fipp used to use finger-trees for ClojureScript (See a2bd18a), but rrb-vectors grew support there. I might also consider an alternative dependency that provides a dequeue interface more abstractly.

[borkdude]

On that front, I'm disinclined to eliminate the dependency on rrb vectors

Just to be clear, I'm not asking to eliminate the dependency, the change would be hidden behind reader conditionals for bb. clj + cljs would still be using rrb. But I understand that you would not like to deal with compatibility for more than clj and cljs.

[borkdude]

I haven't looked too deeply into this library, but Java also comes with a Deque implementation:

https://docs.oracle.com/javase/7/docs/api/java/util/Deque.html

Perhaps you've already considered that.

[brandonbloom]

Needs a functional/persistent dequeue.

I didn't mean to cause you to close this issue. Still open to small changes to support Babashka, but my hope is that can be as easy as bumping an rrb-vector dependency.

[borkdude]

A few thoughts:

• I don't think it makes a lot of sense to make rrb compatible from source so we can run rrb from source with babashka. Performance of code executed with babashka is several times slower than executed with the JVM. Falling back on regular vector will likely make things faster in babashka than executing rrb from source to optimize concatenation of vectors. Of course I haven't measured this and I might be wrong.

• Babashka has built-in dependencies which execute as fast (since they're just using the compiled code). Babashka could decide to include rrb as a built-in dep. But so far, I haven't seen many other libraries that need rrb, other than those depending on puget / fipp.

• Maintaining a fork of fipp is still an option. Maintaining a separate repo that creates a "polyfill" for rrb that falls back on clojure.core is another option. The latter option seems to be the least intrusive for fipp. I could give that a try first.

[brandonbloom]

Babashka could decide to include rrb as a built-in dep.

Looking at this patch here: babashka/babashka#1241 (comment)

I don't know anything about Babashka, but do you have some sort of library/namespace/function stub/patch mechanism? Seems like you could stub either fipp.deque or clojure.core.rrb-vector and you only need that one catvec function?

[borkdude]

@brandonbloom Babashka is able to "fake" any Clojure function via the SCI (interpreter) configuration, so that's certainly an option to try. I will try that soon.

[borkdude]

I have a development version of babashka here (for macOS):

https://output.circle-artifacts.com/output/job/319f007f-70c2-4b1b-af39-e7fb78269d33/artifacts/0/release/babashka-0.8.2-SNAPSHOT-macos-amd64.tar.gz

which can run fipp (the current version from master).

What I've done is include rrb vector proper, but only the catvec function. This adds 350kb to the binary. Might be worth it, but I'll wait for feedback in greglook/puget#56, to see if I can keep it at that for compatibility for fipp + puget.

$ rlwrap ./bb
Babashka v0.8.2-SNAPSHOT REPL.
Use :repl/quit or :repl/exit to quit the REPL.
Clojure rocks, Bash reaches.

user=> (babashka.deps/add-deps '{:deps {fipp/fipp {:deps/manifest :deps :git/url "https://github.com/brandonbloom/fipp" :git/sha "efdf87f22a6fcc08cef5017c14769df29fc37850"}}})

nil
user=> (require '[fipp.edn :as fipp])
nil
user=> (fipp/pprint (range 45))
(0
 1
 2
 3
 4

[borkdude]

Finally got to the point that hashp is working in babashka (current master version)

;; add fipp from master
(babashka.deps/add-deps '{:deps {fipp/fipp {:deps/manifest :deps :git/url "https://github.com/brandonbloom/fipp" :git/sha "efdf87f22a6fcc08cef5017c14769df29fc37850"}}})
;; add newest zprint which is already compatible with bb (hashp also loads zprint)
(babashka.deps/add-deps '{:deps {zprint/zprint {:mvn/version "1.2.3"}}})
;; add babashka fork of clojure spec
(babashka.deps/add-deps '{:deps {org.babashka/spec.alpha {:git/url "https://github.com/babashka/spec.alpha"
                         :git/sha "1a841c4cc1d4f6dab7505a98ed2d532dd9d56b78"}}})
;; add hashp
(babashka.deps/add-deps '{:deps {hashp/hashp {:mvn/version "0.2.1"}}})

(require '[hashp.core])
(set! *data-readers* {'p hashp.core/p*})

[borkdude]

If you can release a new version of fipp, then we don't need the git/sha stuff anymore and then I think we can close this issue.

[brandonbloom]

What I've done is include rrb vector proper, but only the catvec function. This adds 350kb to the binary.

I'm not sure I follow, but I am curious: What does it mean to add it "proper" but only one function?

[brandonbloom]

deployed: https://clojars.org/fipp/versions/0.6.26

[borkdude]

@brandonbloom It means I've only exposed the catvec function to the interpreter:

https://github.com/babashka/babashka/blob/master/feature-rrb-vector/babashka/impl/rrb_vector.clj

GraalVM native-image will analyze what is used and will not include other vars in the native image, if they are not reachable.

[borkdude]

Thanks!

[brandonbloom]

It means I've only exposed the catvec function to the interpreter

Since catvec returns an object of the rrb vector class, presumably that means a fair amount of code is then included?

[borkdude]

@brandonbloom Yes, that fair amount of code yields about 350kb.