This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2022-04-02
Channels
- # asami (5)
- # aws (16)
- # babashka (41)
- # babashka-sci-dev (44)
- # beginners (157)
- # biff (3)
- # cider (1)
- # clj-commons (1)
- # cljdoc (22)
- # clojure (7)
- # clojure-dev (5)
- # clojure-europe (13)
- # clojure-nl (1)
- # clojure-uk (1)
- # clojurescript (17)
- # core-typed (13)
- # cursive (14)
- # datascript (10)
- # events (1)
- # fulcro (2)
- # graalvm (2)
- # gratitude (1)
- # jobs (3)
- # lsp (229)
- # pathom (2)
- # pedestal (3)
- # portal (53)
- # re-frame (7)
- # remote-jobs (1)
- # spacemacs (14)
- # xtdb (6)
@borkdude Saw you merged the uberjar support PR. I'm happy to say more about the arg parsing fns refactor if you want me to. Still on my road trip but I get back to real life tonight.
Feel free to say more when you get back :) I'll try to get the uberscript stuff in too, so we can prepare for a release hopefully mid next week
@cap10morgan Did we implement caching for :path
pods or not? If you're developing a pod in the JVM it can be pretty slow to load, and loading it an extra time takes a long time... I notice that when running tests :)
I noticed this takes pretty long on my machine:
(deftest uberjar-with-pods-test
(testing "jar contains bb.edn w/ only :pods when bb.edn has :pods"
(let [tmp-file (java.io.File/createTempFile "uber" ".jar")
path (.getPath tmp-file)]
(.deleteOnExit tmp-file)
(let [config {:paths ["test-resources/babashka/uberjar/src"]
:deps '{local/deps {:local/root "test-resources/babashka/uberjar"}}
:pods (cond-> '{org.babashka/go-sqlite3 {:version "0.1.0"}}
(not (fs/windows?)) (assoc 'pod/test-pod {:path "test-resources/pod"
#_:here-> :cache true}))}]
(tu/with-config config
(time (tu/bb nil "uberjar" path "-m" "my.main-pod"))
(let [bb-edn-entry (get-entry tmp-file "bb.edn")
bb-edn (-> path JarFile. (.getInputStream bb-edn-entry)
InputStreamReader. PushbackReader. edn/read)]
(is (= #{:pods} (-> bb-edn keys set)))
(is (= (:pods config) (:pods bb-edn))))
(is (str/includes? (tu/bb nil "--jar" path) "3")))))))
it's weird actually, creating the uberjar takes a long time... why, it doesn't actually have to load the pods at all I guess
but anyway, it seems caching also didn't work, so two issues: ⢠cache for local path does not seem to do anything? ⢠loading of pods for namespaces can be prevented when creating an uberjar
Maybe the second one is not important, since we usually do downloading of deps etc too in the first usage of whatever. So it's mostly: why wasn't the cache working I guess
Yeah different dir is the likely culprit
I can work on optimizing that test a bit if you don't beat me to it :)
Otherwise local pod caching should be enabled when you don't opt out of it
And look into not loading pods when building an uberjar
But might be good double checking if the caching is still ok in both registry and local pods
There are many things that do not need deps loading and pod caching, but we do it anyway, so I think it's best to leave it
@highpressurecarsalesm š I saw you were typing. Welcome back :)
I'm messing around with the :name
metadata for #1223, and it seems like probably the most straightforward way to do it is to add :name
in sci's new-var functions (there are a handful of accompanying tweaks in bb to use those functions more). My digging and testing on that has led me to a question that I'd like to confirm to make a better informed decision/proposal about an approach:
I think that the intent is that a SciVar
's sym field should be a namespace-qualified symbol - is that correct? That does seem to be the case in evaluator, but I think a lot of the copying ones use just a symbol name.
I don't even know why we have sym to be honest. We could get the name from the metadata
Maybe you could get an overview where we need the sym field and how it's typically used, in what functions
Then we could decide to either get rid of it and do it via the metadata or fix the field properly
from what I've seen, the actual sym field looks like it's primarily (maybe entirely?) used within the deftype
Well, I've got all the sci tests passing with no references to the sym
field (all the direct references are in the methods defined in the deftype), so it's doable entirely via metadata (notwithstanding potential performance concerns). My primary concern would be babashka, which, in a couple spots, creates SciVars without metadata (and quite possibly other consumers of Sci that might be calling the SciVar constructor directly instead of going through new-var
.
Have you found out what potential performance implications might be? When is .getName called, typically? And what happens in the absence of the symbol?
One should not create SciVars directly, but always via the sci.core
namespace, so I'll accept that breakage
I only see one call to getName on a SciVar, and that's on an exception path to throw when someone tries to change a built-in var. All the uses of sym
appear to be when getting a symbol/string to print a var name.
I'm convincing myself more and more that this is a good idea to go after:
before:
(->> (find-ns 'rewrite-clj.zip) ns-publics (take 5))
([child-sexprs #'child-sexprs] [tag #'tag] [insert-space-left #'insert-space-left] [postwalk #'postwalk] [whitespace? #'whitespace?])
after:
(->> (find-ns 'rewrite-clj.zip) ns-publics (take 5))
([child-sexprs #'rewrite-clj.zip/child-sexprs] [tag #'rewrite-clj.zip/tag] [insert-space-left #'rewrite-clj.zip/insert-space-left] [postwalk #'rewrite-clj.zip/postwalk] [whitespace? #'rewrite-clj.zip/whitespace?])
... with assoc'ing :name
meta in new-var
and removing namespaces from the names getting passed to it, the only outlier I see is *input*
, and that should be pretty easy to address@cap10morgan I think this is mostly it for uberscript: https://github.com/babashka/babashka/commit/ba6048450fb3d0a8a79df5c6a12b8331ce98132a?w=1
Great!
One issue there is that multiple namespaces might load a pod namespace so I got to keep track of those
@cap10morgan Merged the uberscript support now. Declarative pods in bb.edn v1 is done I think :)