This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2022-02-07
Channels
- # announcements (38)
- # asami (14)
- # beginners (35)
- # biff (3)
- # calva (29)
- # cider (20)
- # clj-kondo (7)
- # cljdoc (38)
- # clojure (64)
- # clojure-art (2)
- # clojure-australia (1)
- # clojure-dev (6)
- # clojure-europe (30)
- # clojure-nl (4)
- # clojure-spec (4)
- # clojure-uk (9)
- # clojured (3)
- # clojurescript (87)
- # cursive (17)
- # datahike (2)
- # datomic (10)
- # defnpodcast (2)
- # emacs (2)
- # events (1)
- # fulcro (25)
- # gratitude (1)
- # introduce-yourself (1)
- # jobs-discuss (21)
- # lsp (103)
- # malli (41)
- # meander (8)
- # minecraft (3)
- # missionary (3)
- # nextjournal (20)
- # off-topic (10)
- # pedestal (1)
- # polylith (15)
- # portal (6)
- # releases (2)
- # ring (1)
- # ring-swagger (2)
- # sci (4)
- # shadow-cljs (5)
- # spacemacs (3)
- # sql (11)
- # xtdb (3)
@ericdallo At nextjournal there is a project on which lsp constantly consumes 1 core and doesn't get any further. I can reproduce this from the command line:
borkdude@MBP2019 ~/nj/viewers (elide*?) $ clojure-lsp diagnostics
[ 0%] clojure-lsp
The project is actually public: https://github.com/nextjournal/viewersI can reproduce this also with the JVM version:
clojure -Sdeps '{:deps {com.github.clojure-lsp/clojure-lsp {:mvn/version "2022.02.01-20.02.32"}}}' -X clojure-lsp.api/diagnostics
@U04V15CAJ I suspect it's related with a infinte loop when scanning local/roots
perhaps @U6T7M9DBR is also using local roots?
why would clojure-lsp itself loop through local roots? just doing clojure -Spath gives you all the paths already?
@U04V15CAJ for now we can add a configuration to fix it on .lsp/config.edn
{:source-paths ["modules/viewers/src"
"modules/cljs-extensions/src"
"modules/markdown/src"
"modules/log/src"
"modules/devcards/src"
"modules/inspector/src"
"modules/command-bar/src"
"modules/view/src"
"modules/devdocs/src"
"modules/ui/src"]}
For some reason, clojure-lsp is entering in a infinte loop when resolving a local-root that points to other previous root or something like that, I'll take a closer look later, but that config should fix it, it's what clojure-lsp would find automatically
the "cyclic" deps local/root depedency is:
deps.edn
-> viewer
-> devcards
-> viewer
-> devcards
...
yeah, somehow it works on deps.edn, maybe it's statically only when it uses the deps or something, not sure
@U04V15CAJ how deps.clj handle that kind of cyclic local-roots?
actually clojure-lsp has two things:
• classpath scan, this one we totally delegate to clojure
, lein
etc
• source-paths discovery, where we find what are the source paths from deps.edn manually scanning it
Created a sample that kind of repro the issue: https://github.com/ericdallo/clojure-sample/tree/cyclic-deps
clojure-lsp logs this:
2022-02-07T18:36:26.482Z WARN [clojure-lsp.source-paths:78] - Max deps source-paths resolve level found 500 , maybe a cyclic dependency?
2022-02-07T18:36:26.522Z INFO [clojure-lsp.source-paths:194] - Empty deps.edn source-paths, using default source-paths: #{"src" "test"}
so we already have a ciclyc dependency check> I do think such mutually recursive deps are allowed in deps.edn? Mutually recursive doesn't sound right. I would verify in #tools-deps if this is supported, but it sounds like a bad idea to begin with. Dependencies should form an acyclic graph/tree.
If you look at the output of clojure -Stree
in viewers you can see that it ignores certain parts of the mutual recursiveness because the dep was declared at the top and it always chooses the top dependency
this is probably a sign that the mutual recursive deps both depend on something which should be a third dep
so we’d need to handle this case, correct? https://github.com/clojure/tools.deps.alpha/blob/5412484d8b33b10941681cf5d07190ec81f7c6ec/src/main/clojure/clojure/tools/deps/alpha.clj#L331-L333
is there a reason to rebuild this functionality in lsp or could we also rely on tools.deps.alpha to compute the source path?
I'm not sure what lsp is doing here. a local/root dependency is not a source path, it's a dependency
I mean, the only difference conceptually between local/root and git deps is that they come from disk or a network. But both are just deps, not project sources.
so I would treat them like that. if users like to add more sources to their lsp config, they should do it manually probably
Thank you so much for this workaround, I was banging my head trying to figure out why LSP was stuck at 0%, I also have a deps.edn with local/root dependencies. 🙇
Btw, if you really want to get the directories from local/roots (still don't know why) you could just get the classpath using clojure
and then filter out the directories that fall within the project root.
@U04V15CAJ I was trying to avoid shelling out, maybe deps.clj can help with that
But since Clojure uses classpath caching both things shouldn't really matter on the second invocation and it will simplify your logic a lot
to get all :paths/:extra-paths of subprojects if any, since users open the mono-repo as root, clojure-lsp needs to know all project source-code
to when refactoring a var know that there is somewhere using, to navigation work and things like that
imagine a project root use local/root for subprojects, we would check the subprojects to get the paths
otherwise we would only know the paths of the root deps.edn, which usually is none for mono-repos
I still don't understand why getting the classpath via clojure is problematic. e.g. babashka does this every time you invoke it when you have deps in bb.edn and it's bloody fast if the classpath has already been calculated
yeah, it could work, I was just avoiding to add something that consumes startup time, we already have too much time spent on startup
I don't understand how this is different for users: it's only a change of what you're doing behind the scenes?
ATM users need to configure properly both project-specs if the default doesn't work for they project and the source-paths/source-aliases if changing the default
former is related with analysis, and the other on how other features would consider project source code
yes, but your local/root inspection thing vs walking through the dirs of the classpath and filter dirs that fall within the project, is an implementation detail - I don't get how changing that would affect users.
Would not affect negatively users, it won't work for users that configured source-aliases
or source-paths
but not project-specs
, but I don't see a problem since that would not be consistent already
I think I could make a POC testing your idea @U04V15CAJ, the tradeoffs are good and check how that affect startup time for big projects