:import of records breaks

[stathissideris]

I have the following project.clj

(defproject positano "0.3.0-SNAPSHOT"
  :description "Provenace system for Clojure"
  :url "https://github.com/stathissideris/positano"
  :license {:name "Eclipse Public License"
            :url "http://www.eclipse.org/legal/epl-v10.html"}
  :dependencies [[org.clojure/clojure "1.8.0"]
                 ^:source-dep [datascript "0.15.0"]
                 ^:source-dep [org.clojure/core.async "0.2.385"]
                 ^:source-dep [org.clojure/tools.analyzer.jvm "0.6.10"]]

  :plugins [[thomasa/mranderson "0.4.7"]]
  :profiles {:dev {:source-paths ["src" "dev"]
                   :dependencies [[org.clojure/tools.namespace "0.2.11"]]}})

Running the tests with mranderson, results in this exception:

➜  positano lein with-profile +plugin.mranderson/config test
Exception in thread "main" java.lang.ClassNotFoundException: datascript.pull_parser.PullSpec, compiling:(mranderson047/datascript/v0v15v0/datascript/pull_api.cljc:1:1)

PullSpec is a record defined in a different namespace of datascript:

https://github.com/tonsky/datascript/blob/master/src/datascript/pull_parser.cljc

You can try my project here:

https://github.com/stathissideris/positano

[benedekfazekas]

give a try to just released 0.4.9 if still relevant

[danielcompton]

This is still present in 0.5.0 in re-frame-10x when creating source dependencies for garden.

[benedekfazekas]

can you point me to what exactly broke perhaps in your commit where you manually fixed this @danielcompton ?

[danielcompton]

@benedekfazekas it looks like you've fixed this now, do you still need me to track it down?

[benedekfazekas]

should be ok now, pls check with latest snapshot

[danielcompton]

I'm still having issues with importing garden records. It looks like the namespace needs to be changed from dashes to underscores when importing the record.

After running 0.5.1-SNAPSHOT against re-frame-10x (with ./source-deps.sh), I get this diff and more that are the same issue. The working code is the --- diff, the 0.5.1-SNAPSHOT is the +++ diff.

The change I'm most concerned about is the import field. It would be nice to have strings and comments not rewritten, but that's not such a big deal.

 ;; Compiler flags
diff --git a/gen-src/day8/re_frame_10x/inlined_deps/garden/v1v3v3/garden/def.clj b/gen-src/day8/re_frame_10x/inlined_deps/garden/v1v3v3/garden/def.clj
index b97b484..4c490c8 100644
--- a/gen-src/day8/re_frame_10x/inlined_deps/garden/v1v3v3/garden/def.clj
+++ b/gen-src/day8/re_frame_10x/inlined_deps/garden/v1v3v3/garden/def.clj
@@ -2,8 +2,8 @@
   (:require [day8.re-frame-10x.inlined-deps.garden.v1v3v3.garden.types]
             [day8.re-frame-10x.inlined-deps.garden.v1v3v3.garden.util :as util]
             [day8.re-frame-10x.inlined-deps.garden.v1v3v3.garden.core])
-  (:import day8.re_frame_10x.inlined_deps.garden.v1v3v3.garden.types.CSSFunction
-           day8.re_frame_10x.inlined_deps.garden.v1v3v3.garden.types.CSSAtRule))
+  (:import day8.re-frame-10x.inlined-deps.garden.v1v3v3.garden.types.CSSFunction
+           day8.re-frame-10x.inlined-deps.garden.v1v3v3.garden.types.CSSAtRule))

 (defmacro defstyles
   "Convenience macro equivalent to `(def name (list styles*))`."
@@ -45,7 +45,7 @@
   defcssfn
   "Define a function for creating custom CSS functions. The generated
   function will automatically create an instance of
-  `garden.types.CSSFunction` of which the `:args` field will be set
+  `day8.re-frame-10x.inlined-deps.garden.v1v3v3.garden.types.CSSFunction` of which the `:args` field will be set
   to whatever the return value of the original function is. The
   `:function` field will be set to `(str name)`.

@@ -57,10 +57,10 @@
       ;; => #'user/url

       (url \"http://fonts.googleapis.com/css?family=Lato\")
-      ;; => #garden.types.CSSFunction{:function \"url\", :args \"http://fonts.googleapis.com/css?family=Lato\"}
+      ;; => #day8.re-frame-10x.inlined-deps.garden.v1v3v3.garden.types.CSSFunction{:function \"url\", :args \"http://fonts.googleapis.com/css?family=Lato\"}

       (css (url \"http://fonts.googleapis.com/css?family=Lato\"))
-      ;; => url(http://fonts.googleapis.com/css?family=Lato)
+      ;; => url(http://fonts.googleapis.com/css?family=Lato)

[benedekfazekas]

new snapshot version with a proper fix is on clojars. please retest

re. comments and strings: there was an interim version of 0.5.0-SNAPSHOT where I parsed the whole ns with rewrite-clj but that is just not really an option due to performance issues -- to be more blunt it gets horribly slow this way. So now only the ns macro is parsed with rewrite-clj the ns body is processed simply as text. This gives the necessary performance boost and is possible because of the relatively simpler cases in the ns body. Downside is obviously that it would be much harder to implement these exceptions of strings, comments with regexps..

[danielcompton]

This is getting pretty close, there seems to be one case remaining, again --- is the hand fixed one, and +++ is the mranderson SNAPSHOT 3 changes. You can reproduce this for yourself by running ./source-deps.sh, found in re-frame-10x's repo.

diff --git a/gen-src/day8/re_frame_10x/inlined_deps/garden/v1v3v3/garden/units.cljc b/gen-src/day8/re_frame_10x/inlined_deps/garden/v1v3v3/garden/units.cljc
index 4b28a28..3f8e280 100644
--- a/gen-src/day8/re_frame_10x/inlined_deps/garden/v1v3v3/garden/units.cljc
+++ b/gen-src/day8/re_frame_10x/inlined_deps/garden/v1v3v3/garden/units.cljc
@@ -6,14 +6,14 @@
        [day8.re-frame-10x.inlined-deps.garden.v1v3v3.garden.types :as types]
        [day8.re-frame-10x.inlined-deps.garden.v1v3v3.garden.util :as util])
       (:import
-        (day8.re_frame_10x.inlined_deps.garden.v1v3v3.garden.types CSSUnit))])
+       [day8.re_frame_10x.inlined_deps.garden.v1v3v3.garden.types CSSUnit])])
   #?@(:cljs
       [(:require
         [cljs.reader :refer [read-string]]
-        [day8.re-frame-10x.inlined-deps.garden.v1v3v3.garden.types :as types :refer [CSSUnit]]
-        [day8.re-frame-10x.inlined-deps.garden.v1v3v3.garden.util :as util])
+        [day8.re_frame_10x.inlined_deps.garden.v1v3v3.garden.types :as types :refer [CSSUnit]]
+        [day8.re_frame_10x.inlined_deps.garden.v1v3v3.garden.util :as util])
        (:require-macros
-        [day8.re-frame-10x.inlined-deps.garden.v1v3v3.garden.units :refer [defunit]])]))
+        [day8.re_frame_10x.inlined_deps.garden.v1v3v3.garden.units :refer [defunit]])]))

 ;;;; ## Unit families

[benedekfazekas]

eh this is annoying, sorry. will have a look.

[benedekfazekas]

after looking at this realised that this is a slightly different edge case here. not trying to explain away the bug but apparently ns macro for units.cljc altho valid is a bit wasteful. it repeats requires for types and util that is not really necessary, those requires could go outside the reader macros.

that said I suppose I need to handle the case where there are two :require sections in the ns macro due to the reader macros

[benedekfazekas]

new snapshot is on clojars. just to eat my words again the problem was not with the multiple requires but my yesterday's fix was buggy. hopefully this does it -- famous last words ;)

[danielcompton]

Yep, looking good, can confirm this fixes it. Thanks!

[benedekfazekas]

nice one! thanks for the patience :)

does this mean you can get rid of the patching step in your workflow?

[danielcompton]

Yep, it’s all 100% automatic now.

[stathissideris]

I'm happy that this was fixed, many thanks! I'll give it another go next time I work on positano (which could be a while!)

[benedekfazekas]

@stathissideris let me know what you find when you get there, cheers