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

bsp: Implementing buildTarget/cleanCache #6638

Merged
merged 4 commits into from Sep 6, 2021

Conversation

hmemcpy
Copy link
Contributor

@hmemcpy hmemcpy commented Aug 27, 2021

Fixes #6569

This command is invoked by IntelliJ whenever a Rebuild operation is called. Unfortunately, because it's not implemented, this causes bsp-imported projects in IntelliJ to hang when asking to rebuild.

It works fine in bloop-imported projects because bloop implements this API.

@lightbend-cla-validator

Hi @hmemcpy,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

https://www.lightbend.com/contribute/cla

@hmemcpy
Copy link
Contributor Author

hmemcpy commented Aug 27, 2021

@adpi2 hello, sorry for the ping. Unfortunately, this isn't actually working, and I really have no idea why :(
The test passes, but when I actually try to test this in a local test project, when I call cleanCache from IntelliJ (or even send the json payload via sbt -bsp) I immediately get a shutdown.

I really don't know what I'm missing... any tips appreciated!

@hmemcpy
Copy link
Contributor Author

hmemcpy commented Aug 27, 2021

Update! I got it working... somehow. I made sure to clean everything up, do a publishLocal, followed by a "reboot dev" in my test project... and voilla!
Will finalize the PR soon.
image

@hmemcpy hmemcpy marked this pull request as ready for review August 27, 2021 08:07
@adpi2
Copy link
Member

adpi2 commented Aug 27, 2021

@hmemcpy Thanks for pinging me! It is great that you are working on it.

I tested it locally and it seems to work. Make sure you clean the .sbt cache after running publishLocal with:

rm -Rf ~/.sbt/boot/scala-2.12.14/org.scala-sbt/sbt/1.5.5-SNAPSHOT

Otherwise the launcher loads the old cached jars rather than the new ones.

Another way to test the buildTarget/cleanCache request is to add a test in BuildServerTest.

You can run it with:

sbt:sbt> serverTestProj/testOnly testpkg.BuildServerTest

@hmemcpy
Copy link
Contributor Author

hmemcpy commented Aug 27, 2021

@adpi2 I guess initially I had old jars in the cache. I'll test this a bit more and will mark the PR as done. Thanks again for the help!

@hmemcpy
Copy link
Contributor Author

hmemcpy commented Aug 27, 2021

And it's good to go!

Copy link
Member

@adpi2 adpi2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @hmemcpy!

Most of the comments are suggestions. But I think it is important to check in the test that buildTarget/cleanCache empties the class directory.

@hmemcpy
Copy link
Contributor Author

hmemcpy commented Aug 27, 2021

Thanks for the comments! My copy-pasting was obvious here :)
I'll make the changes and will figure out how to check for empty directories!

@hmemcpy hmemcpy force-pushed the buildTarget/cleanCache branch 2 times, most recently from 3527b2a to 71a52d2 Compare August 27, 2021 14:03
@hmemcpy hmemcpy requested a review from adpi2 August 27, 2021 14:05
adpi2
adpi2 previously approved these changes Aug 27, 2021
Copy link
Member

@adpi2 adpi2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hmemcpy hmemcpy marked this pull request as draft August 28, 2021 08:26
@hmemcpy
Copy link
Contributor Author

hmemcpy commented Aug 28, 2021

Oops, temporarily marking this as draft. It looks like the results.size == targets.size isn't a good check. I just noticed that out of the 3 targets, I get only 2 results back. The sent targets are:

  "targets": [
    {
      "uri": "file:/Users/hmemcpy/git/sbt-playground/#root-build"
    },
    {
      "uri": "file:/Users/hmemcpy/git/sbt-playground/#root/Compile"
    },
    {
      "uri": "file:/Users/hmemcpy/git/sbt-playground/#root/Test"
    }

And I get back just two. Unfortunately I don't know what they are, since it's a Unit result. Is, perhaps, root-build not a "real" target?

Update: I changed Keys.clean to Keys.bspBuildTarget and indeed, the two results I get back are Compile and Test, the -build is not present.

I understand this somewhat a synthetic target, perhaps I can filter it out when doing the comparison?

@hmemcpy
Copy link
Contributor Author

hmemcpy commented Aug 28, 2021

Alright... I managed to implement a solution for both "Rebuild Project" (which includes the root build), as well as a regular "Rebuild module" (which includes the exact targets to rebuild).

Would love any suggestions to improve this!

@hmemcpy hmemcpy marked this pull request as ready for review August 28, 2021 11:23
@hmemcpy hmemcpy requested a review from adpi2 September 1, 2021 11:27
Copy link
Member

@adpi2 adpi2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any obvious way to clean the build.sbt target. So this solution seems ok to me.

@adpi2 adpi2 merged commit d197b5b into sbt:develop Sep 6, 2021
@hmemcpy hmemcpy deleted the buildTarget/cleanCache branch September 6, 2021 10:26
@hmemcpy
Copy link
Contributor Author

hmemcpy commented Sep 6, 2021

TYVM! :)

@eed3si9n eed3si9n added this to the 1.6.0 milestone Sep 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BSP: support workspace/cleanCache
4 participants