Skip to content
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

Add source artifacts to classpath #64

Closed
hugoduncan opened this issue May 16, 2014 · 43 comments
Closed

Add source artifacts to classpath #64

hugoduncan opened this issue May 16, 2014 · 43 comments

Comments

@hugoduncan
Copy link
Member

Is there interest in making the lein plugin automatically add source jars to the classpath?

One possibility is to add source coordinates to the :dependencies, but since most artifacts in the clojure world don't have source jars, that could be a problem. The second option is to add locally available source jars (and rely on lein pom; mvn dependency:sources; to download them).

Ritz contained code to add locally available source jars: https://github.com/pallet/ritz/blob/develop/lein-ritz/src/lein_ritz/add_sources.clj

The ritz code added the artifacts via a hooke, which is probably not the nicest way of doing it, as the source artifacts could end up in an uberjar.

@gtrak
Copy link
Contributor

gtrak commented May 16, 2014

I was thinking this would be a good idea. We would probably do it through
the project-middleware, not a hook.

We could add a task to download available source jars (with necessary
checks/fallbacks), and then use the locally available ones at runtime.

It would be up to the user to prevent them from getting included in the
jar. (add plugin to :dev/:user profile only).

@gtrak
Copy link
Contributor

gtrak commented May 16, 2014

Though looks like at least downloading the deps is trivial with maven, we
could skip that work:
http://stackoverflow.com/a/14395560/2559313

@hugoduncan
Copy link
Member Author

@gtrak not sure what to take from that link. Source jars are mainly for java libraries (eg, clojure itself, etc).

@gtrak
Copy link
Contributor

gtrak commented May 16, 2014

Ah, sorry, I didn't notice before now that you had already listed the
procedure for getting source deps through maven. That link's just
redundant.

@jeffvalk
Copy link
Contributor

I like the idea of automating this, but should point out that CLJ-1161 continues to be a stumbling block here. Essentially, the sources dep that will be most widely used (Clojure itself) has a bug that, until fixed, will cause the REPL on error on startup. Until this is addressed, we can't make automatically injecting this jar the default behavior -- it would break everybody's setups.

@hugoduncan
Copy link
Member Author

hugoduncan commented May 18, 2014

Might it be possible to work around this by ensuring the sources jar is
after the main jar on the classpath?

@jeffvalk
Copy link
Contributor

As long as the classloader in use respects that order, yes, that should work.

For the sake of clarity: The error originates here when the resource clojure/version.properties resolves to the invalid and erroneously included one in clojure-1.6.0-sources.jar instead of the intended one in clojure-1.6.0.jar. If we can guarantee the valid one is resolved, we avoid the error.

@gtrak
Copy link
Contributor

gtrak commented May 18, 2014

If we automate it at runtime instead of lein-time, loading the dependencies
automatically with aether or dynapath, then we can trade off that issue for
other ones :-).

@jeffvalk
Copy link
Contributor

@gtrak My thoughts exactly! :-)

@cichli
Copy link
Member

cichli commented Dec 14, 2015

Looks like CLJ-1161 will be fixed in 1.8, so we should look into this again.

@itegebo
Copy link

itegebo commented May 15, 2016

FWIW, adding Clojure's sources for 1.8 works for me:

:profiles {:dev {:dependencies [[org.clojure/clojure "1.8.0" :classifier "sources"]]}}

@Malabarba
Copy link
Member

I'm sorry for arriving late to the party, but don't we already support jumping to source out-of-the-box? What's being requested by this issue?

@expez
Copy link
Member

expez commented May 18, 2016

@Malabarba You can only jump to jars listed on the classpath. I opened this CIDER issue when I forgot about that, so I suspect others are confused too.

@Malabarba
Copy link
Member

@expez I understand the issue you had. It sort of looks like @hugoduncan was talking about sources for clojure artifacts. Which is strange to me, given that most clojure artifact already come with sources AFAIK.

@bbatsov
Copy link
Member

bbatsov commented May 18, 2016

Which is strange to me, given that most clojure artifact already come with sources AFAIK.

Not sure about Clojure artifacts, but this is definitely not the case with Java ones. There are always separate source artifacts for them.

@Malabarba
Copy link
Member

Is that this issue was is then? Trying to pull in java sources by default?
Sorry, I'm just trying to determine whether the issue is still relevant.

@bbatsov
Copy link
Member

bbatsov commented May 18, 2016

I'm not a mindreader, but I believe so. I remember that when I was a Java dev Eclipse and Intellij would do this for maven projects - automatically fetch the sources, so their xref functionality would work for third-party code. This was pretty handy.

@vise890
Copy link

vise890 commented Sep 20, 2017

Hi all, I'd really love to see this resolved.

Pomegranate (now used by leiningen) hints at the intention of adding support for downloading source/javadoc jars (and adding them to the classpath).

I've been using several java libs recently, and the lack of doc/jump to source has been causing me a lot of pain. So I've decided to hack together a proof of concept and, while it's far from perfect, it seems to work fine from the repl.

What would it take to get something like it added to cider-nrepl?

@expez
Copy link
Member

expez commented Sep 21, 2017

What would it take to get something like it added to cider-nrepl?

Make it generally useful and open a PR. Then be patient during a short period of intense nit-picking ;)

Fwiw this is one of the bigger pain points for me too these days.

@bbatsov
Copy link
Member

bbatsov commented Dec 10, 2017

@vise890 Still waiting for that PR. ;-)

@vise890
Copy link

vise890 commented Dec 10, 2017

@bbatsov I'm afraid I think this is a bit beyond my powers to tackle. If anyone is willing to babysit me a bit, I'm happy to continue working on it though!

I've made a leiningen plugin as a proof of concept: lein-pocketbook.

As far as I can tell, it works, but it's pretty hacky.

I haven't done much in terms of integrating it with emacs. From a clojure file, jumping to a java source works, but, once you're in java, you can't jump around anymore (I'm hoping this is reasonably easy to achieve).

@SevereOverfl0w
Copy link
Contributor

@vise890 jumping around in java could be left to another extension. Your pocketbook plugin works really well, with the caveats you mentioned considered. I was able to use them for jumping around stacktraces, even into the clojure source code: it was really magical!

@vise890
Copy link

vise890 commented Dec 10, 2017

@SevereOverfl0w I'm glad you enjoyed it!

I'm pretty sure the jumping around in Java just needs support on the Emacs side. Using M-x cider-find-var and inputting a fully qualified Java "symbol" already seems to be able to jump to pretty much anything.

Anyway. Do people want a PR to this repo? I can cook something up..

@jeffvalk
Copy link
Contributor

@vise890 If you just enable cider-mode on the Java buffer, you should be able to jump around as you like. This was how Java source support originally worked, and jumping around Java sources was a baked-in feature.

Of course, cider-mode has evolved quite a bit since then, so maybe a minimal minor mode to just support jumping would be the way to go.

dpsutton pushed a commit to dpsutton/cider-nrepl that referenced this issue Feb 14, 2019
@jumarko
Copy link

jumarko commented Aug 6, 2019

I think this is still a useful suggestion and would be great to have an easy way how to add source artifacts for all java deps on the classpath.
Especially for interop-heavy stuff it's invaluable - this is an area where Cursive is so much better.

@clojure-emacs clojure-emacs deleted a comment from MalloZup Aug 7, 2019
@futuro
Copy link

futuro commented Sep 6, 2019

Mulling over this problem, it seems like updating the dependencies coordinates before lein does dependency resolution is the simplest solution.

However, you can't just specify extra coords for the "sources" and "javadoc" classifiers, as any missing jars for those classifiers causes lein to throw an error.

I tried specifying :optional true for the deps that don't have source/javadoc jars, but that didn't stop lein from throwing errors of the form

Could not find artifact nrepl:nrepl:jar:sources:0.6.0 in central (https://repo1.maven.org/maven2/)
Could not find artifact nrepl:nrepl:jar:sources:0.6.0 in clojars (https://repo.clojars.org/)
Could not find artifact nrepl:nrepl:jar:javadoc:0.6.0 in central (https://repo1.maven.org/maven2/)
Could not find artifact nrepl:nrepl:jar:javadoc:0.6.0 in clojars (https://repo.clojars.org/)

I'm hoping someone might have thoughts on how to automatically handle this case, so that user intervention can be minimized. I'm also very curious how Eclipse and Cursive handle this problem. Perhaps they just try to resolve these deps every time 🤔

@Invertisment
Copy link

Invertisment commented Apr 18, 2020

It doesn't need to be out of the box. There could be a yes/no prompt or a variable to add the source jars if needed. If some people will want to save time on this, let they do it.

Also java disassembler would be handy too, but it would require more work.
IntelliJ disassembles JAR's byte code if there are no sources in the project dependencies.
They don't bother to download it. I can easily load and decompile minecraft.jar and they will display obfuscated source code because symbols are erased.

And jump to java source doesn't work for me even if I include them this way: #64 (comment). An issue related to this is closed: clojure-emacs/cider#1666. I guess nobody does this kind of development here. It's probably either ClojureScript or purity.

I use Spacemacs develop/5e037c889
Emacs 26.3
cider/cider-nrepl "0.25.0-SNAPSHOT"

The first thing that popped up after installing IntelliJ an putting the deps is whether I'd like that Gradle wrapper would decompile the JAR. Here you go.
And it downloaded the sources for the other JAR that had those in maven central.

@jumarko
Copy link

jumarko commented Apr 19, 2020

@Invertisment I essentially gave up and waiting if/until cider gets better support.
In the meantime, I keep myself saying that I'll switch to Cursive if absolutely necessary.
It turns out that I rather rarely absolutely need these things although it would be quite convenient to be able to explore Javadoc and sources without having to manually look them up on the Internet.

I also keep InteliJ IDEA opened and if I have to look at Java class I just open it there.

@yyoncho
Copy link

yyoncho commented Apr 19, 2020

Those looking for browsing java sources, disassembler, debugging java source might use: https://github.com/emacs-lsp/dap-mode/wiki/Clojure:-debugging-and-introspecting-Java-Code

@bbatsov
Copy link
Member

bbatsov commented Apr 19, 2020

@Invertisment @jumarko I understand your frustration, but you have to understand that this mostly depends on the tool used to start the REPL. To my knowledge there's no easy way to tell Lein/Boot/tools.deps to just fetch every source jar that exists. If we could do this that'd be the best solution. We can explore hot loading source jars, but we still can't know whether some tool publishes those or not.

@futuro Highlights the problem here very well:

However, you can't just specify extra coords for the "sources" and "javadoc" classifiers, as any missing jars for those classifiers causes lein to throw an error.

@jumarko
Copy link

jumarko commented Apr 19, 2020

@yyoncho thanks for the tip!
Would you mind providing more instructions? (a short video with a sample clojure/lein project would be awesome)
I tried it briefly in spacemacs with following config: https://develop.spacemacs.org/layers/+lang/java/README.html

     (java :variables java-backend 'lsp)

Then created a new sample lein project and generated pom.xml

lein new app lsp-java-clojure-demo
cd lsp-java-clojure-demo
lein pom

Then opened project.clj in spacemacs and run lsp.
After that I tried xref-find-apropos -> java.lang.String but it doesn't find anything; instead it prints

helm-M-x: The connected server(s) does not support method workspace/symbol
To find out what capabilities support your server use ‘M-x lsp-describe-session’ and expand the capabilities section.

The lsp-describe-session shows this:
image

I'm also interested in how do you combine this with Cider.
Do you just run both lsp and cider-jack-in and then do normal Clojure development with Cider and if you need to jump to a Java class then you do xref-find-apropos?

@Invertisment
Copy link

#64 (comment), I used IntelliJ and I inspected all bits of a library that I wanted. This works. Maybe it's the way to do it. At least for now.

@yyoncho
Copy link

yyoncho commented Apr 20, 2020

@jumarko - I think that you pretty much did it - the issue that you had is that you tried to perform xref-find-appropos in buffer that is not managed by the java language server. There are 2 solutions:

  1. Do C-u M-x lsp and pick jstls when you are in the clojure buffer. This will associate the current buffer with JDT LS.
  2. Use helm-lsp-global-workspace-symbol(or lsp-ivy equivalent) which searches in all workspaces.

I'm also interested in how do you combine this with Cider.
Do you just run both lsp and cider-jack-in and then do normal Clojure development with Cider and if you need to jump to a Java class then you do xref-find-apropos?

I use cider+jdtls in clojure+java projects and from time to time I use the debugger since I interact from clojure with our java codebase

@bbatsov
Copy link
Member

bbatsov commented Apr 20, 2020

@yyoncho It'd be nice if you submitted some instructions to CIDER's docs for using it together with LSP for Java. I think this topic comes up every now and then and it'd be nice if we just had some section in the docs where we could point people to.

@yyoncho
Copy link

yyoncho commented Apr 22, 2020

@yyoncho It'd be nice if you submitted some instructions to CIDER's docs for using it together with LSP for Java. I think this topic comes up every now and then and it'd be nice if we just had some section in the docs where we could point people to.

Sure. I will try to prepare something when I am in a mood for writing docs.

@vemv
Copy link
Member

vemv commented Dec 31, 2020

Hi,

I've implemented a Lein plugin that cleanly fetches all available Java sources/javadocs:

https://github.com/threatgrid/clj-experiments/tree/master/resolve-java-sources-and-javadocs

It's more complete than previous attempts like lein-pocketbook, lein-ubersource. It certainly works without issue in large, Java-heavy projects even in presence of advanced features like :managed-dependencies, and plugins like lein-sub, lein-monolith.

Feel free to give it a spin and let me know if there's anything to tweak for CIDER-friendliness :)

Happy New Year!

@bbatsov
Copy link
Member

bbatsov commented Jan 2, 2021

Well, I'm guessing if it fetches them and places them on the classpath everything should work just fine in CIDER. I'll test the plugin when I can.

@bbatsov
Copy link
Member

bbatsov commented Jan 3, 2021

@vemv Any plans to move this to a separate project repo? I'm guessing this way more people will be inclined to give it a go. :-)

@jumarko
Copy link

jumarko commented Jan 4, 2021

@vemv the plugin fails with Caused by: java.lang.ClassNotFoundException: java/sql/Timestamp when used with JDK 11 or 14 on my (Mac OS X) computer. It seems to work fine with an old JDK 8.
I saw a similar issue with leiningen a long time ago but thought it's completed solved now.

EDIT: I tried with leiningen 2.9.3 before and now, after an upgrade, with lein 2.9.5 - the issue persists.

@vemv
Copy link
Member

vemv commented Jan 4, 2021

@vemv the plugin fails with Caused by: java.lang.ClassNotFoundException: java/sql/Timestamp when used with JDK 11 or 14 on my (Mac OS X) computer. It seems to work fine with an old JDK 8.

Thanks! Yes I use JDK8 (and lein @ latest) so something might have slipped. I'll setup CI in the project sometime in the week and run it against a matrix ideally.

...Over the last couple days I've added https://github.com/threatgrid/clj-experiments/tree/master/resolve-java-sources-and-javadocs/integration-testing (25 real-world, large/significant projects to test against) so my confidence around the plugin has increased greatly.

Probably in a matter of days any remaining issues will be ironed out.

@jumarko
Copy link

jumarko commented Jan 15, 2021

@vemv do you have any update on this? I'm eager to try the plugin again once the issue is fixed.

@vemv
Copy link
Member

vemv commented Jan 15, 2021

Issue is fixed and CI'd. On top of that I've revamped the README.

So after quite a few iterations I consider the plugin ready for prime-time - I was simply waiting for some close people to leave feedback.

After that last round of feedback I'll extract the project to its own git repo :)

@bbatsov
Copy link
Member

bbatsov commented Jul 16, 2023

Now that enrich-classpath (https://github.com/clojure-emacs/enrich-classpath/) is quite mature, I think we can finally close this ticket.

@bbatsov bbatsov closed this as completed Jul 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests