lein uberjar fails on master branch

[tirkarthi]

Describe the bug

Trying to make uberjar fails

To Reproduce Steps to reproduce the behavior:

1. checkout master branch
2. Execute lein uberjar

Expected behavior

It should create an uberjar but it fails

Additional context

Build failure : https://travis-ci.com/vedang/farstar/builds/74003094

It's also reproducible locally. Since your README has lein uberjar to install the library I think it broke somewhere recently. I checked if it's an upstream issue in byte-streams library but the version 0.2.3 on git builds fine as a uberjar.

git bisect tells me something broke on https://github.com/vedang/clj_fdb/commit/459071ebe722e1aa9602fdebb48339ebe45fa467

git bisect start HEAD 80fcc9cfa3388cd14047a89453d9573e1f26fff1 and continue with lein uberjar as the test script

Failure - 2a05f82 Change fixture to remove indiscriminate deletion Failure - 459071e (refs/bisect/bad) Create a folder for internal files Success - 5ae76b3 Add wrapper fns for com.apple.foundationdb.FDB

Hope that helps.

[tirkarthi]

When I move the internal namespace out as usual before the bad commit (459071e) it works fine. Compiling files separately also works fine. Seems like an interesting bug or I am missing something here.

diff --git a/src/clj_fdb/core.clj b/src/clj_fdb/core.clj
index 918a918..b93bbc7 100644
--- a/src/clj_fdb/core.clj
+++ b/src/clj_fdb/core.clj
@@ -1,7 +1,7 @@
 (ns clj-fdb.core
   (:refer-clojure :exclude [get set])
   (:require [byte-streams :as bs]
-            [clj-fdb.internal.byte-conversions :refer [byte-array-class]]
+            [clj-fdb.byte-conversions :refer [byte-array-class]]
             [clj-fdb.transaction :as ftr])
   (:import [com.apple.foundationdb KeyValue Range Transaction TransactionContext]
            java.lang.IllegalArgumentException))

[vedang]

Yeah, this is an interesting bug.

• Compiling the file works
• Starting a repl and using the code on the repl works
• Uberjar fails 🤷‍♂️

I want to spend some time today investigating why this breaks. If I can't find anything, I'll move the file back outside.

[tirkarthi]

Thanks. Do add an update if you find anything. One more thing I would suggest is to add lein uberjar as a step in Travis so that this is caught if there are any regressions in the future.

[tirkarthi]

To add some more context I commented the whole file except for ns call with require, import and the statement (def byte-array-class (class (Utils/byteArray 0))) the uberjar worked. But when I uncomment even one of the calls to def-conversion it fails with the same error message and line number that has the definition. The stack error is also very confusing since it's saying along the lines of casting int to int as below :

Caused by: java.lang.ClassCastException: byte_streams.graph.Type cannot be cast to byte_streams.graph.Type
    at byte_streams.graph.ConversionGraph.assoc_conversion(graph.clj:122)
    at byte_streams.graph$fn__2606$G__2568__2612.invoke(graph.clj:91)
    at byte_streams.graph$fn__2606$G__2567__2619.invoke(graph.clj:91)
    at clojure.lang.AFn.applyToHelper(AFn.java:171)
    at clojure.lang.AFn.applyTo(AFn.java:144)
    at clojure.lang.Atom.swap(Atom.java:79)
    at clojure.core$swap_BANG_.invokeStatic(core.clj:2347)
    at clojure.core$swap_BANG_.doInvoke(core.clj:2337)
    at clojure.lang.RestFn.invoke(RestFn.java:529)
def-conversion source code line >> at clj_fdb.internal.byte_conversions$fn__3638.invokeStatic(byte_conversions.clj:10) 
    at clj_fdb.internal.byte_conversions$fn__3638.invoke(byte_conversions.clj:10)

Relevant line in byte-streams library where the exception occurs : https://github.com/ztellman/byte-streams/blob/0.2.3/src/byte_streams/graph.clj#L122

[tirkarthi]

I cloned byte-streams repo and commented out the type hints as below. I did lein install and then changed the co-ordinates in project.clj to make sure it uses the locally installed version. I did lein uberjar for clj_fdb and it worked. I guess some type hinting stuff is causing the error but I don't know why casting a type A to type A is causing the issue and also why it doesn't occur when it's present in a namespace one above the current one as per the bad commit.

Opened an upstream issue for help : https://github.com/ztellman/byte-streams/issues/34

byte-streams type-hint patch

diff --git a/src/byte_streams/graph.clj b/src/byte_streams/graph.clj
index fc62f1d..3c94b41 100644
--- a/src/byte_streams/graph.clj
+++ b/src/byte_streams/graph.clj
@@ -119,10 +119,10 @@
   (assoc-conversion [_ src dst f cost]
     (let [m' (assoc-in m [src dst] (Conversion. f cost))
           m' (if (and
-                   (nil? (.wrapper ^Type src))
-                   (nil? (.wrapper ^Type dst)))
-               (let [src (.type ^Type src)
-                     dst (.type ^Type dst)]
+                   (nil? (.wrapper src))
+                   (nil? (.wrapper dst)))
+               (let [src (.type src)
+                     dst (.type dst)]
                  (-> m'
                    (assoc-in [(Type. 'seq src) (Type. 'seq dst)]
                      (Conversion. (fn [x options] (map #(f % options) x)) cost))

[vedang]

My guess is that for some reason order of compilation matters to the byte-stream library.

This problem has been introduced in the top commit in this library - 90a376b - where I moved around some test util fns from individual test namespaces into clj-fdb.internal.util. This has changed the order in which dependencies are compiled, which is somehow causing this problem.

[tirkarthi]

Yes, I also noticed the change in the order of files the compilation is done when uberjar is created. But as you said I don't know why it matters for byte-stream library and also the fact type-casting throws exception due to this. I hope upstream issue gives some pointers on this.

Thanks

On Tue, May 22, 2018, 11:02 PM Vedang Manerikar notifications@github.com wrote:

My guess is that for some reason order of compilation matters to the byte-stream library.

This problem has been introduced in the top commit in this library - 90a376b https://github.com/vedang/clj_fdb/commit/90a376b3836b6f1a9284eeab14b24d94251c17d0

• where I moved around some test util fns from individual test namespaces into clj-fdb.internal.util. This has changed the order in which dependencies are compiled, which is somehow causing this problem.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/vedang/clj_fdb/issues/16#issuecomment-391076037, or mute the thread https://github.com/notifications/unsubscribe-auth/ADyc90SVKuLJpgQqfIRVjXob8pao93Ckks5t1EvJgaJpZM4UIA1f .

--

Regards, Karthikeyan S

[vedang]

I found a fix for the problem and will push it, but I don't understand what the underlying issue is.

The problem is this: with 90a376b, the uberjar task compiles clj-fdb.internal.util which requires clj-fdb.core -> clj-fdb.internal.byte-conversions which compiles everything in byte-streams and loads all the conversions that we have defined. Compilation of this namespace works fine. The compiler then tries AOT compilation of clj-fdb.internal.byte-conversions, which fails in the very first def-conversion.

This is because byte-streams internally defines a deftype called Type. Something in the AOT system is breaking / mis-compiling this deftype such that it can no longer be cast to what the clojure compiler thinks of as the Type class in the next pass.

Removing the :profiles {:uberjar {:aot :all}} option from the project.clj fixes this problem. This option is meant for apps and not libraries anyway, and was round due to the fact that initially this was a play repo for me.

[tirkarthi]

Okay, please add lein uberjar as a step in Travis as part of the build as a safe check to make sure it always works fine.

Thanks

On Wed, May 23, 2018, 12:12 AM Vedang Manerikar notifications@github.com wrote:

I found a fix for the problem and will push it, but I don't understand what the underlying issue is.

The problem is this: with 90a376b https://github.com/vedang/clj_fdb/commit/90a376b3836b6f1a9284eeab14b24d94251c17d0, the uberjar task compiles clj-fdb.internal.util which requires clj-fdb.core -> clj-fdb.internal.byte-conversions which compiles everything in byte-streams and loads all the conversions that we have defined. Compilation of this namespace works fine. The compiler then tries AOT compilation of clj-fdb.internal.byte-conversions, which fails in the very first def-conversion.

This is because byte-streams internally defines a deftype called Type. Something in the AOT system is breaking / mis-compiling this deftype such that it can no longer be cast to what the clojure compiler thinks of as the Type class in the next pass.

Removing the :profiles {:uberjar {:aot :all}} option from the project.clj fixes this problem. This option is meant for apps and not libraries anyway, and was round due to the fact that initially this was a play repo for me.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/vedang/clj_fdb/issues/16#issuecomment-391098246, or mute the thread https://github.com/notifications/unsubscribe-auth/ADyc9yg1xJkuTolYSKryuTE6H5KbIKGEks5t1FwEgaJpZM4UIA1f .

--

Regards, Karthikeyan S

[tirkarthi]

Also change can be incorporated in farstar with respect to clj_fdb installation. Ref : https://github.com/vedang/farstar/blob/master/.travis.yml#L9