Skip to content

Commit 6826e67

Browse files
authored
Remove adding test options to the project/build target name hash (#3107)
* Remove adding test options to the project/build target name hash - this should not be put there. The hash in the build target name is used to introduce hashing when buildOptions are changed from the CLI. Otherwise, if options are changed from sources, Bloop recompiles the project anyway because the sources changed. Adding this new hash that comes from sources possibly creates new build targets every time somebody fiddles with some test scope options. It's just nonsense. Also look at the comments in the code, we've made a rule and then stopped following it. * Apply scalafix * Add test for the number of build targets created when changing options from cli and from sources * Format
1 parent 58f5a76 commit 6826e67

File tree

2 files changed

+52
-38
lines changed

2 files changed

+52
-38
lines changed

modules/build/src/main/scala/scala/build/Build.scala

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -200,22 +200,15 @@ object Build {
200200
*/
201201
def updateInputs(
202202
inputs: Inputs,
203-
options: BuildOptions,
204-
testOptions: Option[BuildOptions] = None
203+
options: BuildOptions
205204
): Inputs = {
206205

207206
// If some options are manually overridden, append a hash of the options to the project name
208207
// Using options, not options0 - only the command-line options are taken into account. No hash is
209208
// appended for options from the sources.
210-
val optionsHash = options.hash
211-
val testOptionsHash = testOptions.flatMap(_.hash)
212-
213-
inputs.copy(
214-
baseProjectName =
215-
inputs.baseProjectName
216-
+ optionsHash.map("_" + _).getOrElse("")
217-
+ testOptionsHash.map("_" + _).getOrElse("")
218-
)
209+
val optionsHash = options.hash
210+
211+
inputs.copy(baseProjectName = inputs.baseProjectName + optionsHash.fold("")("_" + _))
219212
}
220213

221214
private def allInputs(
@@ -278,6 +271,11 @@ object Build {
278271
overrideOptions: BuildOptions
279272
): Either[BuildException, NonCrossBuilds] = either {
280273

274+
val inputs0 = updateInputs(
275+
inputs,
276+
overrideOptions.orElse(options) // update hash in inputs with options coming from the CLI or cross-building, not from the sources
277+
)
278+
281279
val baseOptions = overrideOptions.orElse(sharedOptions)
282280

283281
val scopedSources = value(crossSources.scopedSources(baseOptions))
@@ -290,12 +288,6 @@ object Build {
290288
value(scopedSources.sources(Scope.Test, baseOptions, inputs.workspace, logger))
291289
val testOptions = testSources.buildOptions
292290

293-
val inputs0 = updateInputs(
294-
inputs,
295-
mainOptions, // update hash in inputs with options coming from the CLI or cross-building, not from the sources
296-
Some(testOptions).filter(_ != mainOptions)
297-
)
298-
299291
def doBuildScope(
300292
options: BuildOptions,
301293
sources: Sources,

modules/integration/src/test/scala/scala/cli/integration/CompileTestDefinitions.scala

Lines changed: 43 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -183,16 +183,7 @@ abstract class CompileTestDefinitions
183183

184184
test("no arg") {
185185
simpleInputs.fromRoot { root =>
186-
val projectFilePrefix = root.baseName + "_"
187186
os.proc(TestUtil.cli, "compile", extraOptions, ".").call(cwd = root)
188-
val projDirs = os.list(root / Constants.workspaceDirName)
189-
.filter(_.last.startsWith(projectFilePrefix))
190-
.filter(os.isDir(_))
191-
expect(projDirs.length == 1)
192-
val projDir = projDirs.head
193-
val projDirName = projDir.last
194-
val elems = projDirName.stripPrefix(projectFilePrefix).split("[-_]").toSeq
195-
expect(elems.length == 1)
196187
}
197188
}
198189

@@ -254,18 +245,6 @@ abstract class CompileTestDefinitions
254245
)
255246
expect(isDefinedTestPathInClassPath)
256247
checkIfCompileOutputIsCopied("Tests", tempOutput)
257-
258-
val projectFilePrefix = root.baseName + "_"
259-
260-
val projDirs = os.list(root / Constants.workspaceDirName)
261-
.filter(_.last.startsWith(projectFilePrefix))
262-
.filter(os.isDir(_))
263-
expect(projDirs.length == 1)
264-
val projDir = projDirs.head
265-
val projDirName = projDir.last
266-
val elems = projDirName.stripPrefix(projectFilePrefix).split("[-_]").toSeq
267-
expect(elems.length == 2)
268-
expect(elems.toSet.size == 2)
269248
}
270249
}
271250

@@ -697,4 +676,47 @@ abstract class CompileTestDefinitions
697676
expect(out.contains("Too small maximum heap"))
698677
}
699678
}
679+
680+
test("new build targets should only be created when CLI options change") {
681+
val filename = "Main.scala"
682+
val inputs = TestInputs(
683+
os.rel / filename ->
684+
"""object Main extends App {
685+
| println("Hello")
686+
|}
687+
|""".stripMargin,
688+
os.rel / "Test.test.scala" ->
689+
"""object Test extends App {
690+
| println("Hello")
691+
|}
692+
|""".stripMargin
693+
)
694+
inputs.fromRoot { root =>
695+
os.proc(TestUtil.cli, "compile", extraOptions :+ "--test", ".").call(cwd = root)
696+
697+
def buildTargetDirs = os.list(root / Constants.workspaceDirName)
698+
.filter(os.isDir)
699+
.filter(_.last != ".bloop")
700+
701+
expect(buildTargetDirs.size == 1)
702+
703+
os.write.over(
704+
root / filename,
705+
"""//> using dep com.lihaoyi::os-lib:0.9.1
706+
|
707+
|object Main extends App {
708+
| println("Hello")
709+
|}
710+
|""".stripMargin
711+
)
712+
713+
os.proc(TestUtil.cli, "compile", extraOptions :+ "--test", ".").call(cwd = root)
714+
expect(buildTargetDirs.size == 1)
715+
716+
os.proc(TestUtil.cli, "compile", extraOptions ++ Seq("--test", "-nowarn"), ".").call(cwd =
717+
root
718+
)
719+
expect(buildTargetDirs.size == 2)
720+
}
721+
}
700722
}

0 commit comments

Comments
 (0)