Closed tirkarthi closed 2 years ago
Here's a minimal reproducible case https://github.com/gsnewmark/byte-streams-cnf
It seems like under some conditions Leiningen compiles namespaces in incorrect order causing expanded part of byte-streams/def-conversion
to be executed twice. Second execution fails with the reported exception, because class of src
/ dst
(byte_streams.graph.Type
) is really a different one than the currently compiled one (not sure why/how though).
In case we have two namespaces core
and conversions
, conversions
contains def-conversion
's, and core
requires it, lein compile :all
firstly compiles conversions
, and then core
, expanded part of def-conversion
is executed once, and compilation finishes correctly. But if we add a third namespace lib
which also dependes on conversions
for some reason lein compiles this new ns lib
firstly, loads conversions
during its compilation and executes expanded part of def-conversion
successfully, but then lein tries to compile conversions
, executes expanded part once more and fails with ClassCastException
:woman_shrugging:
Following change avoids the exception, but it's clearly just a crude workaround, not a proper solution:
1 file changed, 19 insertions(+), 17 deletions(-)
src/byte_streams.clj | 36 +++++++++++++++++++-----------------
modified src/byte_streams.clj
@@ -91,7 +91,7 @@
:else
(g/type (class x))))
-(defn- normalize-type-descriptor [x]
+(defn normalize-type-descriptor [x]
(cond
(instance? Type x)
x
@@ -106,22 +106,24 @@
"Defines a conversion from one type to another."
[[src dst :as conversion] params & body]
(let [^Type src (normalize-type-descriptor src)
- dst (normalize-type-descriptor dst)]
- `(let [f#
- (fn [~(with-meta (first params)
- {:tag (when (and (instance? Class (.type src)) (not (.wrapper src)))
- (if (= src (normalize-type-descriptor 'bytes))
- 'bytes
- (.getName ^Class (.type src))))})
- ~(if-let [options (second params)]
- options
- `_#)]
- ~@body)
-
- cost#
- ~(get (meta conversion) :cost 1)]
- (swap! conversions g/assoc-conversion ~src ~dst f# cost#)
- (swap! inverse-conversions g/assoc-conversion ~dst ~src f# cost#))))
+ dst (normalize-type-descriptor dst)]
+ `(when (= ~src (normalize-type-descriptor ~src))
+ (let [f#
+ (fn [~(with-meta (first params)
+ {:tag (when (and (instance? Class (.type src)) (not (.wrapper src)))
+ (if (= src (normalize-type-descriptor 'bytes))
+ 'bytes
+ (.getName ^Class (.type src))))})
+ ~(if-let [options (second params)]
+ options
+ `_#)]
+ ~@body)
+
+ cost#
+ ~(get (meta conversion) :cost 1)]
+
+ (swap! conversions g/assoc-conversion ~src ~dst f# cost#)
+ (swap! inverse-conversions g/assoc-conversion ~dst ~src f# cost#)))))
(defmacro def-transfer
"Defines a byte transfer from one type to another."
Probably related https://dev.clojure.org/jira/browse/CLJ-1741
That's weird, the deftype+
macro being used in byte-streams is supposed to avoid this exact issue, at least in REPL-based situations. I'll look into it.
@ztellman it's also reproducible in REPL using the repo I've linked in the previous message :disappointed: (cc @serzh):
➜ byte-streams-cnf git:(master) lein repl
Retrieving byte-streams/byte-streams/0.2.4/byte-streams-0.2.4.pom from clojars
Retrieving primitive-math/primitive-math/0.1.6/primitive-math-0.1.6.pom from clojars
Retrieving manifold/manifold/0.1.8/manifold-0.1.8.pom from clojars
Retrieving manifold/manifold/0.1.8/manifold-0.1.8.jar from clojars
Retrieving primitive-math/primitive-math/0.1.6/primitive-math-0.1.6.jar from clojars
Retrieving byte-streams/byte-streams/0.2.4/byte-streams-0.2.4.jar from clojars
nREPL server started on port 53229 on host 127.0.0.1 - nrepl://127.0.0.1:53229
REPL-y 0.3.7, nREPL 0.2.12
Clojure 1.9.0
Java HotSpot(TM) 64-Bit Server VM 1.8.0_131-b11
Docs: (doc function-name-here)
(find-doc "part-of-name-here")
Source: (source function-name-here)
Javadoc: (javadoc java-object-or-class-here)
Exit: Control+D or (exit) or (quit)
Results: Stored in vars *1, *2, *3, an exception in *e
user=> (compile 'byte
byte byte-array bytes bytes?
user=> (compile 'byte-streams-cnf.lib)
byte-streams-cnf.lib
user=> (compile 'byte-streams-cnf.conversions)
ClassCastException byte_streams.graph.Type cannot be cast to byte_streams.graph.Type byte-streams.graph.ConversionGraph (graph.clj:122)
user=>
It works completely fine if compile
is not called though.
Minor addition: both compilation using lein and REPL work with with following dependencies in the test project
:dependencies [[org.clojure/clojure "1.6.0"]
[byte-streams "0.2.0-alpha1"]]
but stops working starting from the [org.clojure/clojure "1.7.0"]
, so it's definitely caused by some changes in compiler introduced in 1.7.0 :disappointed:
Newer byte-streams
versions don't work with 1.6.0 because they (transitively) rely on new features from 1.7.0.
Correction to my initial comment: after additional investigation it looks like the expanded part of deftype+
is not actually executed twice, but class is still somehow different, most probably it's really loaded by a different class loader (precisely what's described here).
I can't replicate this, neither with current clj_fdb, nor with the byte-streams-cnf test case. I tried downgrading the Clojure versions to 1.7.0 and 1.9.0, but still couldn't trigger the error. I also tried to verify it with the listed clj_fdb commit, 459071e, but one of the deps, fdb-java 5.1.7, is no longer available from Maven.
@tirkarthi @gsnewmark I'm inclined to close it, unless you think it should remain open.
I'd say go for it :slightly_smiling_face:
Closing for now, but @tirkarthi lmk if it needs to be reopened.
I am okay with closure as well since I am not using the library currently to replicate the issue. Thanks @KingMob
I removed byte-streams
from clj_fdb
, which is why the issue is not
reproducible.
Please close it for now.
On Thu, May 26, 2022, 8:51 AM Karthikeyan Singaravelan < @.***> wrote:
I am okay with closure as well since I am not using the library currently to replicate the issue. Thanks @KingMob https://github.com/KingMob
— Reply to this email directly, view it on GitHub https://github.com/clj-commons/byte-streams/issues/34#issuecomment-1138543446, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAUUHU2KFEUCCEGDDIEZJLVL5XVXANCNFSM4FBB4ZAQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>
I am using byte-streams library for clj_fdb. When I try to create a uberjar on the master I have the below error. I tried with the latest alpha release and also it throws the same error. I think it's due to some type hinting at graph.clj . I cloned the repo and removed the type hints and did lein install. Then I used the local version which works fine during uberjar creation.
The strange part was that it used to work fine except when the relevant file was moved not to another folder. I figured out the bad commit at https://github.com/vedang/clj_fdb/commit/459071ebe722e1aa9602fdebb48339ebe45fa467 where I move the namespace to another internal folder. When I revert the folder organization it works fine. I don't why moving it inside to
internal
folder causes the issue with type hinting while moving it one above works fine. Maybe I am missing something here.Another caveat is that compiling the file as a separate file works fine and starting the repl also works fine. This happens only when I have the namespace inside the folder while moving it out works fine.
Exception
byte-streams type-hint change that makes uberjar work
Thanks a lot for the library.