Bring hotload-dependency back #301
Conversation
|
@benedekfazekas this seems to fail due to the |
|
Now that the other PR is merged, I guess you can rebase this one on top of |
|
I'd try to replicate this test as part of this PR: https://github.com/clojure-emacs/orchard/blob/dbe1bef72697a89843763dd112d151a9f365e5c8/test/orchard/java/classpath_test/third_party_compat_test.clj - else I'd fear that users of this feature could have issues that could manifest themselves in third-party libs. (in this case the |
Done
Thanks. That's a good idea. I read some of the discussion you linked yesterday, but it's not entirely clear what this test is testing. (deftest works
(is (seq (clojure.java.classpath/classpath-directories))
"The presence of `clojure.java` does not affect third-party libraries"))If you get stuff back it works, otherwise you got an exception as the failure mode? |
The side-effects of using dynapath were causing the result of |
|
will have a look |
|
Are you married to Adding |
|
It's been a while since I ran a snapshot locally and I'm not sure why it isn't working. I did: (setq cider-jack-in-lein-plugins
'(("refactor-nrepl/refactor-nrepl" "3.0.0-SNAPSHOT" :predicate cljr--inject-middleware-p)
("cider/cider-nrepl" "0.26.0-SNAPSHOT")))And that gave me: The REPL starts, but with a warning: Checking manually, the middleware isn't responding to the version check and when I inspect the classpath no I can't see any errors in any of the nrepl-buffers. Does that mean it just crashed silently on startup? Suggestions? |
I don't care about the reflection warnings, but many people do. If we disable this when we develop, then they'll suffer when we ship. |
|
Specifically, it can be a pain when one invokes i.e. even if performance might mostly be unaffected, it's pretty noisy. |
|
Good points! Hadn't considered that it'll look real bad when someone turns on Guess it might be prudent to fix these reflection warnings in edit: I'll hold off on doing that until we get this working, though. |
|
The only reason I know that people care about this is because people are constantly complaining when there are oversights in Orchard and cider-nrepl. 😆 |
|
@bbatsov I think something went wrong when you built the latest snapshot. It's almost half the size of the 2.5.1 release. When I inspect it all the
Or maybe that's the new normal with the latest mranderson release? |
|
I did a "make clean deploy", as I always do. Still, it seems the deps weren't properly inlined. |
expez
force-pushed
the
hotload-deps
branch
2 times, most recently
from
f27a97a to
12ab7b0
Compare
|
Time to rebase here I guess. |
expez
force-pushed
the
hotload-deps
branch
6 times, most recently
from
46b42b1 to
ebaffdf
Compare
|
Rebased. The lint run fails and says this: Yet locally I get no conflicting deps: I tried just taking the suggestion of adding What's going on here? |
check out how :pedantic is set. You can use
Yeah :pedantic doesn't always make an optimal suggestion. In this case, instead of an exclusion I'd add an explicit dependency on p.s. I self-assigned a couple issues from our tracker, I assume no overlap? |
Yeah, just trying to figure out why the build is failing :) |
|
we don't really have a mranderson problem atm, do we? |
|
(There's https://github.com/jafingerhut/dolly as well if anything else fails 😄, it's doing a decent job for Eastwood) |
When I start a REPL using the jar you published the REPL comes up without refactor-nrepl on the classpath. I can't see any errors anywhere so I've no idea what's wrong. The mrandersoned tests also run fine on the CI server. Pretty frustrating failure mode. |
oh i see your comment above now. would be interesting to see the line how your REPL is started up, what is injected etc. there could be a problem there not with the actual jar |
|
@benedekfazekas posted that too above, but here it is again: |
you did, sorry. and it looks fine too. will check whats in that jar |
|
only had time for a quick check now but doing this on a random project from CLI starts up the REPL fine. i might not be up to date on cider-nrepl etc... |
|
I don't think there is a problem with I jacked in with the following params: and successfully run find-usages on my local machine. let me know if I missed anything here... |
|
On ubuntu 20.04, with openjdk 16. I tested the equivalent command on win 10 with jdk8 and jdk 17 and got the same result. |
|
this is what i see in the ns decl of parse only occurs as an alias prefix in the body. my only guess is that you tried to build the jar locally and it went wrong, or that there was a previous version of the same jar that was broken. maybe try to redownload the jar?
|
I hadn't built the jar locally and I've downloaded the jar on two different machines. I deleted it manually just to be sure one last time, with the same result. I also tried building this branch locally, without inlining Pomegranate, and after installing the jar locally (under a different name 😛) I get a repl without any refactor-nrepl deps on the classpath again. I must be doing something wrong, but I'm not sure what that is at this point. Maybe someone else can take this across the finish line... |
|
It works for me with JDK 14. I'll install and test 16 later. |
|
Same with JDK 16. I ran: And then did a |
|
Thanks for testing @bbatsov. I still can't get a REPL going in emacs, but connecting to this: does work, if I don't do it in the context of the
Might spend some more time on this tomorrow, but kind of stuck again. The dependency resolution fails deep int the java code of the maven-resolver AFAICT. |
Previously this op was built on top of alembic, but that lib no longer seems to be maintained at all so we're moving to pomegranate. `tools.deps` would've been another alternative by their `add-lib` operation has yet to be merged to the main branch. Once it's merged and has proven itself in the wild nothing prevents us from moving over, though, if that solution proves to be better in some way.
as a workaround for the error that happens when inlining dependencies. this way javax.* packages won't get prefixed
`get-clojars-versions` threw a different exception than expected so it made sense to return nil when nothing found so the expected IAE would be thrown
|
I was at a bit of a loss as to how I could make I got it working at the REPL, but after packaging it up it fails because ;; Load extensions
(load "/clojure/tools/deps/alpha/extensions/maven")
(load "/clojure/tools/deps/alpha/extensions/local")
(load "/clojure/tools/deps/alpha/extensions/git")
(load "/clojure/tools/deps/alpha/extensions/deps")
(load "/clojure/tools/deps/alpha/extensions/pom")Because e.g. the maven extension is now called Since the Oh, and I also had to edit one line in It'll probably be a week or two until I have time to look at this again 😞 If anyone wants to pick up the torch, feel free to push directly to this branch, |
|
Hi everyone! Are there any updates on this topic? |
Nope. I've had no time to work on this. IIRC I ran into some problems with I believe this feature is sorely missed, so if you want to get involved that would be most welcome! |
|
It's also a pretty difficult problem to solve in general - I'd wager that once we would get past mranderson issues we'd get into a new world of JDK/classpath issues 😄 We can always wait for other libs to solve it and eventually integrate those. |
|
For what it's worth we've taken a new take on hot reloading of dependencies with lambdaisland/classpath, and expanded on it in launchpad. This will watch Not saying it's flawless, as mentioned there are a lot of tricky edge cases because a lot of JVM stuff doesn't expect you to mess with the classpath mid-flight. E.g. adding ClojureScript libs still requires a restart. But it's been working pretty well for us. I also did a blog posts about this stuff. It's hairy: The Classpath is a lie. |
Previously this op was built on top of alembic, but that lib no longer
seems to be maintained at all so we're moving to pomegranate.
tools.depswould've been another alternative by theiradd-liboperation has yet to be merged to the main branch. Once it's merged and
has proven itself in the wild nothing prevents us from moving over,
though, if that solution proves to be better in some way.