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

improvement: fetch missing dependency sources #5819

Merged
merged 5 commits into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
add timeout to fetching dependency sources
  • Loading branch information
kasiaMarek committed Nov 22, 2023
commit 8778a3463c16e24a95a0f107f24527cc0ff1eaef
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ object ImportedBuild {
dependencySources: DependencySourcesResult,
javacOptions: JavacOptionsResult,
scalacOptions: ScalacOptionsResult,
)(implicit ec: ExecutionContext): Future[DependencySourcesResult] = Future {
)(implicit ec: ExecutionContext): Future[DependencySourcesResult] = {
val dependencySourcesItems = dependencySources.getItems().asScala.toList
val idsLookup = dependencySourcesItems.map(_.getTarget()).toSet
val classpaths = javacOptions
Expand All @@ -119,16 +119,21 @@ object ImportedBuild {
.asScala
.map(item => (item.getTarget(), item.getClasspath()))

val newItems =
classpaths.collect {
case (id, classpath) if !idsLookup(id) =>
val items = JarSourcesProvider.fetchSources(
classpath.asScala.filter(_.endsWith(".jar")).toSeq
)
new DependencySourcesItem(id, items.asJava)
val newItemsFuture =
Future.sequence {
classpaths.collect {
case (id, classpath) if !idsLookup(id) =>
for {
items <- JarSourcesProvider.fetchSources(
classpath.asScala.filter(_.endsWith(".jar")).toSeq
)
} yield new DependencySourcesItem(id, items.asJava)
}
}

new DependencySourcesResult((dependencySourcesItems ++ newItems).asJava)
newItemsFuture.map { newItems =>
new DependencySourcesResult((dependencySourcesItems ++ newItems).asJava)
}
}

def fromList(data: Seq[ImportedBuild]): ImportedBuild =
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
package scala.meta.internal.metals

import java.nio.file.Path
import java.util.concurrent.TimeUnit
import java.util.concurrent.TimeoutException

import scala.concurrent.ExecutionContext
import scala.concurrent.Future
import scala.util.Success
import scala.util.Try
import scala.util.control.NonFatal
import scala.xml.XML

import scala.meta.internal.metals.MetalsEnrichments._
Expand All @@ -12,12 +17,13 @@ import scala.meta.io.AbsolutePath

import coursierapi.Dependency
import coursierapi.Fetch
import coursierapi.error.CoursierError

object JarSourcesProvider {

private val sbtRegex = "sbt-(.*)".r

def fetchSources(jars: Seq[String]): Seq[String] = {
def fetchSources(jars: Seq[String])(implicit ec: ExecutionContext): Future[Seq[String]] = {
def sourcesPath(jar: String) = s"${jar.stripSuffix(".jar")}-sources.jar"

val (haveSources, toDownload) = jars.partition { jar =>
Expand All @@ -37,25 +43,33 @@ object JarSourcesProvider {
}.distinct

val fetchedSources =
dependencies.flatMap { dep =>
Try(fetchDependencySources(dep)).toEither match {
case Right(fetched) => fetched.map(_.toUri().toString())
case Left(error) =>
scribe.warn(
s"could not fetch dependency sources for $dep, error: $error"
)
None
Future.sequence {
dependencies.map{ dep =>
fetchDependencySources(dep)
.recover {
case error : CoursierError =>
scribe.warn(s"Could not fetch dependency sources for $dep, error: $error")
Nil
}
}
}
}.recover {
case _: TimeoutException =>
scribe.warn(s"Timeout when fetching dependency sources.")
Nil
case NonFatal(e) =>
scribe.warn(s"Could not fetch dependency sources, error: $e.")
Nil
}.map(_.flatten.map(_.toUri().toString()))

val existingSources = haveSources.map(sourcesPath)
fetchedSources ++ existingSources
fetchedSources.map(_ ++ existingSources)

}

private def sbtFallback(jar: AbsolutePath): Option[Dependency] = {
val filename = jar.filename.stripSuffix(".jar")
filename match {
case sbtRegex(versionStr) if Try().isSuccess =>
case sbtRegex(versionStr) =>
Try(SemVer.Version.fromString(versionStr)) match {
case Success(version) if version.toString == versionStr =>
Some(Dependency.of("org.scala-sbt", "sbt", versionStr))
Expand Down Expand Up @@ -91,7 +105,7 @@ object JarSourcesProvider {

private def fetchDependencySources(
dependency: Dependency
): List[Path] = {
)(implicit ec: ExecutionContext): Future[List[Path]] = Future {
Fetch
Copy link
Contributor

Choose a reason for hiding this comment

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

Forgot about it and I think we do it wrong in most places. Could we make sure we handle an issue if there is no internet available? Make sure it doesn't get stuck etc. Probably we should have some timeout and do it in a separate future

.create()
.withDependencies(dependency)
Expand All @@ -101,6 +115,6 @@ object JarSourcesProvider {
.asScala
.map(_.toPath())
.toList
}
}.withTimeout(5, TimeUnit.MINUTES)

}
22 changes: 15 additions & 7 deletions tests/unit/src/test/scala/tests/JarSourcesProviderSuite.scala
Original file line number Diff line number Diff line change
@@ -1,18 +1,26 @@
package tests

import scala.concurrent.ExecutionContext

import scala.meta.internal.metals.JarSourcesProvider
import scala.meta.internal.metals.MetalsEnrichments._

class JarSourcesProviderSuite extends BaseSuite {

private implicit val ctx: ExecutionContext = this.munitExecutionContext

test("download-deps") {
val downloadedSources =
JarSourcesProvider.fetchSources(Library.cats.map(_.toURI.toString()))
assert(downloadedSources.nonEmpty)
downloadedSources.foreach { pathStr =>
val path = pathStr.toAbsolutePath
assert(path.exists)
assert(path.filename.endsWith("-sources.jar"))
for {
downloadedSources <- JarSourcesProvider.fetchSources(
Library.cats.map(_.toURI.toString())
)
} yield {
assert(downloadedSources.nonEmpty)
downloadedSources.foreach { pathStr =>
val path = pathStr.toAbsolutePath
assert(path.exists)
assert(path.filename.endsWith("-sources.jar"))
}
}
}

Expand Down