Remove deps cached only when deps in mix file changed #345
Conversation
There was a problem hiding this comment.
Nice investigative work in #235 (comment) !
At first I wasn't sure about removing the prev_deps = cached_deps() line, but according to the docs Mix.Dep.load_on_environment doesn't change the cache so I think this change is good.
@lukaszsamson does this look good to you as well?
| Mix.Dep.load_on_environment([]) != prev_deps, | ||
| do: fetch_deps() | ||
| Mix.Dep.load_on_environment([]) != cached_deps() do | ||
| :ok = Mix.Project.clear_deps_cache() |
There was a problem hiding this comment.
Can we add an explanatory comment about why this needs to be within the if statement?
There was a problem hiding this comment.
@axelson NOTE comment added.
|
It looks reasonable but TBH I don't know this code very well |
|
@axelson @lukaszsamson To be honest, I don't understand the reason why we need to clear deps cache every time before fetching deps. :D |
|
It's seems test was failing with debugger. I'm not sure why. |
|
@wingyplus the test output (some of that debugger issue) was improved in #347 and if I can clean up #352 the test suite can become green again. So we don't need to worry about it for this PR. I plan to do a bit more testing on this and get it merged soon, hopefully this weekend, but if I take longer feel free to ping me. |
|
@axelson That's ok. What can I help? |
Remove deps cached every build can cause deps cached disappear in :ets. Fixes by move calling Mix.Project.clear_deps_cache/0 only when deps in mix.exs was change. Fixes elixir-lsp#235
|
Test passed. 🎉 |
|
@axelson I see an error about debugger in the CI https://travis-ci.org/github/elixir-lsp/elixir-ls/jobs/720334814#L261. Is it intention? |
|
Yes, it’s intentional. The log message should be captured though. |
|
@lukaszsamson noted. Thanks |
|
Thanks for rebasing! Merging now. |
Remove deps cached every build can cause deps cached disappear in :ets.
Fixes by move calling Mix.Project.clear_deps_cache/0 only when deps in
mix.exs was change.
Fixes #235