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

[PoC] Add multimodule support to Scala CLI ⚠️ #3085

Draft
wants to merge 36 commits into
base: main
Choose a base branch
from

Conversation

MaciejG604
Copy link
Contributor

@MaciejG604 MaciejG604 commented Aug 8, 2024

This PR is posted here to get some opinions on whether this is valid at all, not really in the code-sense but rather in the "Scala CLI as the official Scala runner"-sense.

This is a proof of concept PR stemming from previous work on scala-compose repo. The goal is to add support for projects containing multiple modules, each with separate classpath, that can depend on each other. Presented changes introduce BSP support that seems to work with Metals and a little worse with IntelliJ. This is still a work in progress and there are some things to figure out before this could be merged.

The main idea is to reuse the code that we already have for single module projects and simply loop over modules, calling functions same as before. This, in most parts avoids touching critical elements of the logic. Also, it allows for the behaviour to stay exactly the same when operating on single module projects.

The outline of the changes done to the code base:

  1. Commits before [NIT: Just to make reviewing easier:...] are just random improvements I've spotted while studying the code.
  2. Changing HasGeneratedSources to ManagesBuildTargets - the old HasGeneratedSourcesImpl assumed that the project contains only one module and operated on its Scopes keeping a map of those, new implementation keeps the same information by the ProjectName key which allows to store the Main and Test build targets same as before, but can handle mutliple of those. We're mostly relying on Bloop to keep the BSP standard while comunicating with the IDE client, so supporting mutliple build targets is ready out of the box if we can keep track of them. This change seems really safe - only one test blew up due to some race condition that I was not able to track down 😞.
  3. So while we're able to have BSP support for multiple build targets we don't have any way of introducing more than one. The best candidate for starting up modules is our Inputs class that aggregates information about files that get fed into the build process and also validates them. To multiply the instances of Inputs (renamed to ModuleInputs and later to Modules) I introduced the InputsComposer class that analyzes the configuration from modules.toml and creates instances of Inputs with the correct links between them. Putting this code into bsp and setup-ide commands is quite simple - it basically requires to switch to working on Seq[Inputs] instead of Inputs.
  4. I've also hacked some multi-module support for run command which resulted in introducing a new trait with the old name Inputs that kept track of which module is the "target" of the command and allows to compute the compile order of modules (also keep some universal stuff like workspace path to not call Seq[Inputs].head.workspace everywhere).

Things that still need figuring out:

  1. For modules that depend on some "other" add whole classpath from the "other", right now there's only a dependency on .scala-build/target-123/classes/main which does not contain external sources like libraries, this will require some reorganization of the build procedure so that we stop after computing each build target's Project, link them together and the proceed with calling Bloop for compilation.
  2. Creating multiple build targets when starting BSP is quite messy with BuildOptions - doing what common sense suggested was right, resulted in a very unexpected test failing. Not doing that however seems to have weird effect on the resulting classpaths of the build targets.

@MaciejG604 MaciejG604 added discussion Community help wanted Issues that idicate features that are nice-to-have but core team does not have time to work on it experimental Tickets tied to experimental features. labels Aug 8, 2024
@MaciejG604 MaciejG604 force-pushed the dreams-of-multi-module-scala-cli branch from dc40778 to ea0acc0 Compare August 20, 2024 18:08
- copy HasGeneratedSources.scala into ManagesBuildTargets.scala
- copy HasGeneratedSourcesImpl.scala into ManagesBuildTargetsImpl.scala.
Use ProjectName class instead of plain String
@bishabosha
Copy link
Contributor

Without diving into the changes, how could this PR address scenarios like full-stack apps - e.g. where you need to bundle some compiled scala-js into a route from a scala jvm server

@MaciejG604
Copy link
Contributor Author

@bishabosha I haven't given much thought to Scala JS, when preparing those changes and I'm not sure if building with Scala JS and depending on other modules will work out of the box with the changes prepared here.
What is your experience with scala-compose? Is simple extra classpath from the module dependencies enough to correclty build a Scala JS module?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community help wanted Issues that idicate features that are nice-to-have but core team does not have time to work on it discussion experimental Tickets tied to experimental features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants