Skip to content

Conversation

@alexarchambault
Copy link
Collaborator

No description provided.

alexarchambault and others added 30 commits July 5, 2022 10:58
Previously, Bloop's `CompilerCache` could return instances of
`JavaCompiler` relying on using the local JDK Java compiler API
("unforked" compilation) in cases where the javac options included
options expected to be passed to the runtime system (options starting
with `-J`, eg. `-J-Dfoo=bar`).

This happened, because the compiler instances are cached and the path to
the `javac` binary was used as cache key. This means that if 2 projects
A and B share the same path to the `javac` binary, but only project B
has javac options that should be passed to the runtime system, then
Bloop could use a local compiler to compile project B, because the cache
would have been populated with the local compiler used to compile
project A. See the test cases included in this commit.

This commit fixes this issue by encoding in the cache key whether a
local compiler can be used.
* Cache Scala.js linkers for incremental linking

* Use TrieMap instead of mutable.Map

* Avoid caching fullLinkJS linker to save memory

* Cache only one linker per target path

* Use `withClosureCompiler` instead of `withClosureCompilerIfAvailable`

* Avoid redundant `withOptimizer(true)`

* Use SoftReference instead of WeakReference

Co-authored-by: Sébastien Doeraene <sjrdoeraene@gmail.com>
They mainly depend on bloop-config and jsoniter-scala, which should be
stable enough.
…ter#1764)

I'm noticed that this outputStream might produce an unhandled OOM error when it used from Metals (~80MB in memory).
Can be reproduced on long running session with a lot of diagnostics.
…mat (scalacenter#1766)

This is to enable usage of Bloop/Metals in Enterprise environments where only the valid-POM format is accepted for JAR downloads (meaning most SBT plug-ins don't work -- currently making Scala unusable from VS Code at organizations with stricter security policies), for thousands of developers.

--

How this is tested locally:

`publishM2`, then in the target project that uses sbt-bloop, in its `project/metals.sbt` add, e.g.:

```
libraryDependencies += "ch.epfl.scala" % "sbt-bloop_2.12_1.0" % "1.5.2-13-15ded987-20220726-1436"
// only needed if published to local .m2 via publishM2
resolvers += Resolver.mavenLocal
```
…management-voln

Upgrade sbt-librarymanagement to 1.1.5 due to CVEs in its 1.0.x tree.
Do it in the same way how it's implemeted in zinc - https://github.com/sbt/zinc/blob/189d33bcf6678fcc86e7e3e438c551d6cc51be8a/internal/zinc-core/src/main/scala/sbt/internal/inc/IncrementalCommon.scala#L396-L405

Technically it shouldn't be an issue, if user don't touch clasees then
there is no need to check `removedProducts`.

However, sometimes when I do a lot of switches between branches in
combination with Metals by some reason I'm getting an invalid state of project where some products were
removed and their sources aren't presented in changed files.
fix: check if products were removed in inc compilation
 Conflicts:
	build.sbt
	project/project/build.sbt
 Conflicts:
	.github/workflows/sbt-dependency-graph.yml
 Conflicts:
	integrations/maven-bloop/src/main/scala/bloop/integrations/maven/MojoImplementation.scala
	integrations/maven-bloop/src/test/scala/bloop/integrations/maven/MavenConfigGenerationSuite.scala
 Conflicts:
	project/Dependencies.scala
 Conflicts:
	.github/workflows/sbt-dependency-graph.yml
 Conflicts:
	.github/workflows/ci.yml
	benchmark-bridge
	build.sbt
	frontend/src/test/resources/compiler-plugin-allowlist/project/plugins.sbt
	frontend/src/test/resources/cross-test-build-scala-native-0.4/project/plugins.sbt
	frontend/src/test/resources/cross-test-build-scalajs-0.6/project/plugins.sbt
	frontend/src/test/resources/cross-test-build-scalajs-1.0/project/plugins.sbt
	frontend/src/test/resources/cross-test-build-scalajs-1.x/project/plugins.sbt
	frontend/src/test/resources/custom-test-framework/project/plugins.sbt
	frontend/src/test/resources/no-test-frameworks/project/plugins.sbt
	frontend/src/test/resources/scala-seed-project/project/plugins.sbt
	frontend/src/test/resources/simple-build/project/plugins.sbt
	project/build.sbt
	project/project/build.sbt
 Conflicts:
	launcher-core/src/main/scala/bloop/launcher/bsp/BspBridge.scala
 Conflicts:
	docs/build-tools/sbt.md
	project/build.sbt
 Conflicts:
	project/Dependencies.scala
 Conflicts:
	integrations/gradle-bloop/src/main/scala/bloop/integrations/gradle/model/BloopConverter.scala
	integrations/gradle-bloop/src/main/scala/bloop/integrations/gradle/tasks/BloopInstallTask.scala
	integrations/gradle-bloop/src/test/scala/bloop/integrations/gradle/ConfigGenerationSuite.scala
@alexarchambault alexarchambault marked this pull request as ready for review August 16, 2022 13:36
@alexarchambault alexarchambault merged commit a8b6296 into scala-cli:main Aug 16, 2022
@alexarchambault alexarchambault deleted the merge-upstream branch August 16, 2022 13:36
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.

9 participants