-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
lein uberjar fails with type hints in graph.clj #34
Comments
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 In case we have two namespaces 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 |
@ztellman it's also reproducible in REPL using the repo I've linked in the previous message 😞 (cc @serzh):
It works completely fine if |
Minor addition: both compilation using lein and REPL work with with following dependencies in the test project
but stops working starting from the Newer |
Correction to my initial comment: after additional investigation it looks like the expanded part of |
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 🙂 |
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
<#34 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAUUHU2KFEUCCEGDDIEZJLVL5XVXANCNFSM4FBB4ZAQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
For posterity:
Probably CLJ-1650, which was introduced in 1.7 and fixed in 1.8 |
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 vedang/clj_fdb@459071e 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.
The text was updated successfully, but these errors were encountered: