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

Fix export failing on input duplicates #2098

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
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
29 changes: 29 additions & 0 deletions modules/build/src/main/scala/scala/build/CollectionOps.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package scala.build
import scala.collection.mutable
object CollectionOps {
extension [T](items: Seq[T]) {

/** Works the same standard lib's `distinct`, but only differentiates based on the key extracted
* by the passed function. If more than one value exists for the same key, only the first one
* is kept, the rest is filtered out.
*
* @param f
* function to extract the key used for distinction
* @tparam K
* type of the key used for distinction
* @return
* the sequence of items with distinct [[items]].map(f)
*/
def distinctBy[K](f: T => K): Seq[T] =
if items.length == 1 then items
else
val seen = mutable.HashSet.empty[K]
items.filter { item =>
val key = f(item)
if seen(key) then false
else
seen += key
true
}
}
}
17 changes: 15 additions & 2 deletions modules/build/src/main/scala/scala/build/CrossSources.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package scala.build

import java.io.File

import scala.build.CollectionOps.*
import scala.build.EitherCps.{either, value}
import scala.build.Ops.*
import scala.build.Positioned
Expand Down Expand Up @@ -266,9 +267,21 @@ object CrossSources {
inputs.withElements(elements = filteredElements)
)

val preprocessedSources =
val preprocessedSources: Seq[PreprocessedSource] =
(preprocessedInputFromArgs ++ preprocessedSourcesFromDirectives).distinct
.pipe(sources => value(validateExcludeDirectives(sources, allInputs.workspace)))
.pipe { sources =>
val validatedSources: Seq[PreprocessedSource] =
value(validateExcludeDirectives(sources, allInputs.workspace))
val distinctSources = validatedSources.distinctBy(_.distinctPathOrSource)
val diff = validatedSources.diff(distinctSources)
if diff.nonEmpty then
val diffString = diff.map(_.distinctPathOrSource).mkString(s"${System.lineSeparator} ")
logger.message(
s"""[${Console.YELLOW}warn${Console.RESET}] Skipped duplicate sources:
| $diffString""".stripMargin
)
distinctSources
}

val scopedRequirements = preprocessedSources.flatMap(_.scopedRequirements)
val scopedRequirementsByRoot = scopedRequirements.groupBy(_.path.root)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,15 @@ sealed abstract class PreprocessedSource extends Product with Serializable {
def optionsWithTargetRequirements: List[WithBuildRequirements[BuildOptions]]
def requirements: Option[BuildRequirements]
def mainClassOpt: Option[String]

def scopedRequirements: Seq[Scoped[BuildRequirements]]
def scopePath: ScopePath
def directivesPositions: Option[Position.File]
def distinctPathOrSource: String = this match {
case PreprocessedSource.OnDisk(p, _, _, _, _, _, _) => p.toString
case PreprocessedSource.InMemory(op, rp, _, _, _, _, _, _, _, _, _) => s"$op; $rp"
case PreprocessedSource.UnwrappedScript(p, _, _, _, _, _, _, _, _, _) => p.toString
case PreprocessedSource.NoSourceCode(_, _, _, _, p) => p.toString
}
}

object PreprocessedSource {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package scala.build.tests

import com.eed3si9n.expecty.Expecty.expect

import scala.build.CollectionOps.distinctBy
class DistinctByTests extends munit.FunSuite {
case class Message(a: String, b: Int)
val distinctData = Seq(
Message(a = "1", b = 4),
Message(a = "2", b = 3),
Message(a = "3", b = 2),
Message(a = "4", b = 1)
)
val repeatingData = Seq(
Message(a = "1", b = 4),
Message(a = "1", b = 44),
Message(a = "2", b = 3),
Message(a = "22", b = 3),
Message(a = "3", b = 22),
Message(a = "33", b = 2),
Message(a = "4", b = 1),
Message(a = "4", b = 11)
)

test("distinctBy where data is already distinct") {
val distinctByA = distinctData.distinctBy(_.a)
val distinctByB = distinctData.distinctBy(_.b)
val generalDistinct = distinctData.distinct
expect(distinctData == generalDistinct)
expect(distinctData == distinctByA)
expect(distinctData == distinctByB)
}

test("distinctBy doesn't change data order") {
val expectedData = Seq(
Message(a = "1", b = 4),
Message(a = "2", b = 3),
Message(a = "22", b = 3),
Message(a = "3", b = 22),
Message(a = "33", b = 2),
Message(a = "4", b = 1)
)
expect(repeatingData.distinctBy(_.a) == expectedData)
}
}
42 changes: 39 additions & 3 deletions modules/build/src/test/scala/scala/build/tests/SourcesTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@ import coursier.util.{Artifact, Task}
import dependency.*

import scala.build.Ops.*
import scala.build.Sources
import scala.build.{CrossSources, Position, Sources, UnwrappedCrossSources}
import scala.build.internal.ObjectCodeWrapper
import scala.build.CrossSources
import scala.build.Position
import scala.build.errors.{UsingDirectiveValueNumError, UsingDirectiveWrongValueTypeError}
import scala.build.input.ScalaCliInvokeData
import scala.build.options.{BuildOptions, Scope, SuppressWarningOptions}
Expand Down Expand Up @@ -536,4 +534,42 @@ class SourcesTests extends munit.FunSuite {
}
}

test("CrossSources.forInputs respects the order of inputs passed") {
val inputArgs @ Seq(project, main, abc, message) =
Seq("project.scala", "Main.scala", "Abc.scala", "Message.scala")
val testInputs = TestInputs(
os.rel / project ->
"""//> using dep "com.lihaoyi::os-lib::0.8.1"
|//> using file "Message.scala"
|""".stripMargin,
os.rel / main ->
"""object Main extends App {
| println(Message(Abc.hello))
|}
|""".stripMargin,
os.rel / abc ->
"""object Abc {
| val hello = "Hello"
|}
|""".stripMargin,
os.rel / message ->
"""case class Message(value: String)
|""".stripMargin
)
testInputs.withInputs { (_, inputs) =>
val crossSourcesResult =
CrossSources.forInputs(
inputs,
preprocessors,
TestLogger(),
SuppressWarningOptions()
)
assert(crossSourcesResult.isRight)
val Right(CrossSources(onDiskSources, _, _, _, _)) =
crossSourcesResult.map(_._1.withWrappedScripts(BuildOptions()))
val onDiskPaths = onDiskSources.map(_.value._1.last)
expect(onDiskPaths == inputArgs)
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ object Export extends ScalaCommand[ExportOptions] {

logger.log("Preparing build")

val (crossSources, _) = value {
val (crossSources: UnwrappedCrossSources, _) = value {
CrossSources.forInputs(
inputs,
Sources.defaultPreprocessors(
Expand All @@ -56,8 +56,9 @@ object Export extends ScalaCommand[ExportOptions] {

val wrappedScriptsSources = crossSources.withWrappedScripts(buildOptions)

val scopedSources = value(wrappedScriptsSources.scopedSources(buildOptions))
val sources = scopedSources.sources(scope, wrappedScriptsSources.sharedOptions(buildOptions))
val scopedSources: ScopedSources = value(wrappedScriptsSources.scopedSources(buildOptions))
val sources: Sources =
scopedSources.sources(scope, wrappedScriptsSources.sharedOptions(buildOptions))

if (verbosity >= 3)
pprint.err.log(sources)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import java.nio.charset.Charset
import scala.util.Properties

trait ExportCommonTestDefinitions { _: ScalaCliSuite & TestScalaVersionArgs =>
protected lazy val extraOptions: Seq[String] = scalaVersionArgs ++ TestUtil.extraOptions
protected lazy val extraOptions: Seq[String] =
scalaVersionArgs ++ TestUtil.extraOptions ++ Seq("--suppress-experimental-warning")

protected def runExportTests: Boolean = Properties.isLinux

Expand Down Expand Up @@ -61,6 +62,17 @@ trait ExportCommonTestDefinitions { _: ScalaCliSuite & TestScalaVersionArgs =>
expect(output.contains("Hello"))
}

def extraSourceFromDirectiveWithExtraDependency(inputs: String*): Unit =
prepareTestInputs(
ExportTestProjects.extraSourceFromDirectiveWithExtraDependency(actualScalaVersion)
).fromRoot { root =>
exportCommand(inputs*).call(cwd = root, stdout = os.Inherit)
val res = buildToolCommand(root, runMainArgs*)
.call(cwd = root / outputDir)
val output = res.out.trim(Charset.defaultCharset())
expect(output.contains(root.toString))
}

if (runExportTests) {
test("JVM") {
jvmTest()
Expand All @@ -74,5 +86,11 @@ trait ExportCommonTestDefinitions { _: ScalaCliSuite & TestScalaVersionArgs =>
test("Ensure test framework NPE is not thrown when depending on logback") {
logbackBugCase()
}
test("extra source from a directive introducing a dependency") {
extraSourceFromDirectiveWithExtraDependency("Main.scala")
}
test("extra source passed both via directive and from command line") {
extraSourceFromDirectiveWithExtraDependency(".")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -279,4 +279,19 @@ object ExportTestProjects {
|//> using lib "ch.qos.logback:logback-classic:1.4.5"
|println("Hello")
|""".stripMargin)

def extraSourceFromDirectiveWithExtraDependency(scalaVersion: String): TestInputs =
TestInputs(
os.rel / "Main.scala" ->
s"""//> using scala "$scalaVersion"
|//> using file "Message.scala"
|object Main extends App {
| println(Message(value = os.pwd.toString).value)
|}
|""".stripMargin,
os.rel / "Message.scala" ->
s"""//> using dep "com.lihaoyi::os-lib:0.9.1"
|case class Message(value: String)
|""".stripMargin
)
}