From 57af7550263b6d2e1b808dac75d1cf6f41457296 Mon Sep 17 00:00:00 2001 From: Blaine Freestone Date: Mon, 21 Jul 2025 12:22:46 -0600 Subject: [PATCH 1/2] add sourceroot flag with absolute root path of the sandbox worker in order to make the filepath relative in the tasty file this should not invalidate the bazel cache every time because of the path mapping set in the bazel rule config remove sourceroot flag from analysis_store file content (it's not deterministic) add -sourceroot conditionally (it was introduced in Scala 3); change from --sourceroot to -sourceroot !fixup add -sourceroot !fixup add -sourceroot !fixup add -sourceroot add comment explaining change --- .../workers/zinc/compile/FilteredSetup.scala | 44 +++++++++++++++++++ .../workers/zinc/compile/ZincRunner.scala | 33 ++++++++++---- 2 files changed, 69 insertions(+), 8 deletions(-) create mode 100644 src/main/scala/higherkindness/rules_scala/workers/zinc/compile/FilteredSetup.scala diff --git a/src/main/scala/higherkindness/rules_scala/workers/zinc/compile/FilteredSetup.scala b/src/main/scala/higherkindness/rules_scala/workers/zinc/compile/FilteredSetup.scala new file mode 100644 index 00000000..f90623be --- /dev/null +++ b/src/main/scala/higherkindness/rules_scala/workers/zinc/compile/FilteredSetup.scala @@ -0,0 +1,44 @@ +package sbt.internal.inc + +import xsbti.compile.{CompileOptions, MiniSetup} +import java.nio.file.{Path, Paths} + +/** + * Zinc's MiniSetup contains scalacOptions which include the -sourceroot flag that references absolute sandbox paths. + * These paths are non-deterministic across builds because the sandbox directory changes (e.g., __sandbox/4/_main vs + * __sandbox/8/_main), making the analysis files non-deterministic. + * + * This class filters out the -sourceroot option from the scalacOptions to ensure deterministic analysis files. + * + * TODO: Consider if there's a better way to handle this upstream in Zinc + */ + +object FilteredSetup { + private val sourcerootFlag = "-sourceroot" + + def getFilteredSetup(setup: MiniSetup): MiniSetup = { + val options = setup.options() + // Filter out the -sourceroot option and its value + val filteredScalacOptions = { + val originalOptions = options.scalacOptions() + val filtered = scala.collection.mutable.ArrayBuffer[String]() + var i = 0 + while (i < originalOptions.length) { + val option = originalOptions(i) + if (option == sourcerootFlag) { + // Skip both the flag and its value (next argument) + i += 2 + } else { + filtered += option + i += 1 + } + } + filtered.toArray + } + + // Create new CompileOptions with filtered scalac options + val newOptions = options.withScalacOptions(filteredScalacOptions) + // Create new MiniSetup with filtered options + setup.withOptions(newOptions) + } +} diff --git a/src/main/scala/higherkindness/rules_scala/workers/zinc/compile/ZincRunner.scala b/src/main/scala/higherkindness/rules_scala/workers/zinc/compile/ZincRunner.scala index 7f7d9654..0481bbc8 100644 --- a/src/main/scala/higherkindness/rules_scala/workers/zinc/compile/ZincRunner.scala +++ b/src/main/scala/higherkindness/rules_scala/workers/zinc/compile/ZincRunner.scala @@ -18,7 +18,7 @@ import net.sourceforge.argparse4j.impl.Arguments as Arg import net.sourceforge.argparse4j.inf.{ArgumentParserException, Namespace} import sbt.internal.inc.classpath.ClassLoaderCache import sbt.internal.inc.caching.ClasspathCache -import sbt.internal.inc.{Analysis, AnalyzingCompiler, CompileFailed, FilteredInfos, FilteredRelations, IncrementalCompilerImpl, Locate, PlainVirtualFile, PlainVirtualFileConverter, ZincUtil} +import sbt.internal.inc.{Analysis, AnalyzingCompiler, CompileFailed, FilteredInfos, FilteredRelations, FilteredSetup, IncrementalCompilerImpl, Locate, PlainVirtualFile, PlainVirtualFileConverter, ZincUtil} import scala.jdk.CollectionConverters.* import scala.util.Try import scala.util.control.NonFatal @@ -212,17 +212,31 @@ object ZincRunner extends WorkerMain[ZincRunnerWorkerConfig] { isWorker, ) + // Check if we should include the -sourceroot flag in the compiler options + // We only include it for Scala 3 and later versions, as it is not supported + // in Scala 2.x versions. + // We include this so that the TASTy file generated by the Scala compiler + // will be deterministic across machines and directories Bazel uses for + // multiplexed sandbox execution. + val shouldIncludeSourceRoot = !scalaInstance.actualVersion.startsWith("0.") && + scalaInstance.actualVersion.startsWith("3") + + val scalacOptions = + workRequest.plugins.view.map(p => s"-Xplugin:$p").toArray ++ + workRequest.compilerOptions ++ + workRequest.compilerOptionsReferencingPaths.toArray ++ + (if (shouldIncludeSourceRoot) + Array("-sourceroot", task.workDir.toAbsolutePath().toString) + else + Array.empty[String]) + val compileOptions = CompileOptions.create .withSources(sources.view.map(source => PlainVirtualFile(source.toAbsolutePath().normalize())).toArray) .withClasspath((classesOutputDir +: deps.view.map(_.classpath)).map(path => PlainVirtualFile(path)).toArray) .withClassesDirectory(classesOutputDir) .withJavacOptions(workRequest.javaCompilerOptions) - .withScalacOptions( - workRequest.plugins.view.map(p => s"-Xplugin:$p").toArray ++ - workRequest.compilerOptions ++ - workRequest.compilerOptionsReferencingPaths.toArray, - ) + .withScalacOptions(scalacOptions) val compilers = { val scalaCompiler = ZincUtil @@ -335,14 +349,17 @@ object ZincRunner extends WorkerMain[ZincRunnerWorkerConfig] { ) } + // Filter out non-deterministic --sourceroot paths from the setup + val filteredSetup = FilteredSetup.getFilteredSetup(compileResult.setup) + val analysisStoreText = AnalysisUtil.getAnalysisStore( new File(pathString.substring(0, pathString.length() - 3) + ".text.gz"), true, readWriteMappers, ) - analysisStoreText.set(AnalysisContents.create(resultAnalysis, compileResult.setup)) - analysisStore.set(AnalysisContents.create(resultAnalysis, compileResult.setup)) + analysisStoreText.set(AnalysisContents.create(resultAnalysis, filteredSetup)) + analysisStore.set(AnalysisContents.create(resultAnalysis, filteredSetup)) // create used deps val usedDeps = From 0fb2ec3bd9bde7e41e312db783c5800b603c9362 Mon Sep 17 00:00:00 2001 From: Blaine Freestone Date: Mon, 28 Jul 2025 16:53:48 -0600 Subject: [PATCH 2/2] add automated test for TASTy file determinism !fixup add automated --- tests/determinism/BUILD | 7 +++ tests/determinism/SampleClass.scala | 17 ++++++ tests/determinism/test | 95 +++++++++++++++++++++++++++++ 3 files changed, 119 insertions(+) create mode 100644 tests/determinism/BUILD create mode 100644 tests/determinism/SampleClass.scala create mode 100755 tests/determinism/test diff --git a/tests/determinism/BUILD b/tests/determinism/BUILD new file mode 100644 index 00000000..49c1a654 --- /dev/null +++ b/tests/determinism/BUILD @@ -0,0 +1,7 @@ +load("@rules_scala_annex//rules:scala.bzl", "scala_library") + +# Sample library to test TASTy determinism +scala_library( + name = "tasty_test_lib", + srcs = ["SampleClass.scala"], +) diff --git a/tests/determinism/SampleClass.scala b/tests/determinism/SampleClass.scala new file mode 100644 index 00000000..de6b88c2 --- /dev/null +++ b/tests/determinism/SampleClass.scala @@ -0,0 +1,17 @@ +package determinism.test + +class SampleClass { + def method1(): String = "hello" + + def method2(x: Int, y: String): Boolean = { + x > 0 && y.nonEmpty + } + + private val field = 42 + + case class InnerCase(name: String, value: Int) + + object InnerObject { + def compute(): Int = field * 2 + } +} diff --git a/tests/determinism/test b/tests/determinism/test new file mode 100755 index 00000000..8a2705b8 --- /dev/null +++ b/tests/determinism/test @@ -0,0 +1,95 @@ +#!/bin/bash -e +. "$(dirname "$0")"/../common.sh + +# Test that verifies TASTy files can be generated deterministically +echo "Testing TASTy file generation with Scala 3..." + +# Build the Scala 3 target, explicitly using Scala 3 toolchain +echo "Building target with Scala 3 toolchain..." +bazel build --@rules_scala_annex//rules/scala:scala-toolchain=test_zinc_3 --keep_going --remote_executor= --remote_cache= --disk_cache= :tasty_test_lib + +# Get the generated jar file +bazel_bin=$(bazel info bazel-bin) +jar_file="$bazel_bin/determinism/tasty_test_lib.jar" + +echo "Extracting TASTy files from build..." +temp_dir=$(mktemp -d) + +cleanup() { + exit_code=$? + for dir in "${temp_dirs[@]}"; do + rm -r "$dir" 2>/dev/null || true + done + finish $exit_code +} +trap cleanup EXIT + +# Extract all .tasty files from the jar +unzip -j "$jar_file" "*.tasty" -d "$temp_dir" 2>/dev/null || { + echo "No .tasty files found in jar, checking if they exist at all..." + unzip -l "$jar_file" | grep -i tasty || { + echo "ERROR: No TASTy files found in the generated jar" + echo "Jar contents:" + unzip -l "$jar_file" + exit 1 + } +} + +# Verify TASTy files were generated +tasty_files=$(find "$temp_dir" -name "*.tasty" | wc -l) +if [ "$tasty_files" -eq 0 ]; then + echo "ERROR: No TASTy files were extracted" + exit 1 +fi + +echo "SUCCESS: Found $tasty_files TASTy file(s) generated by Scala 3 compiler" + +# Show what files were found +echo "Generated TASTy files:" +find "$temp_dir" -name "*.tasty" -exec basename {} \; + +# Test determinism by rebuilding multiple times +echo "Testing determinism by rebuilding 5 times..." + +# Array to store temp directories for each build +temp_dirs=("$temp_dir") + +# Perform 4 additional builds (we already have one) +for i in $(seq 2 5); do + echo "Build $i/5..." + bazel clean + bazel build --@rules_scala_annex//rules/scala:scala-toolchain=test_zinc_3 --remote_executor= --remote_cache= --disk_cache= :tasty_test_lib + + # Create temp directory for this build + temp_dir_n=$(mktemp -d) + temp_dirs+=("$temp_dir_n") + + # Extract TASTy files from this build + unzip -j "$jar_file" "*.tasty" -d "$temp_dir_n" 2>/dev/null || { + echo "ERROR: Failed to extract TASTy files from build $i" + exit 1 + } +done + +# Compare all builds against the first one +echo "Comparing TASTy files across all 5 builds for determinism..." +all_identical=true + +for i in $(seq 2 5); do + build_num=$((i-1)) + echo "Comparing build 1 with build $i..." + diff_output=$(diff -r "${temp_dirs[0]}" "${temp_dirs[$build_num]}" 2>&1) + if [ $? -ne 0 ]; then + echo "ERROR: TASTy files differ between build 1 and build $i - not deterministic" + echo "Differences found:" + echo "$diff_output" + all_identical=false + fi +done + +if [ "$all_identical" = true ]; then + echo "SUCCESS: TASTy files are identical across all 5 builds - build is deterministic!" +else + echo "ERROR: TASTy files differ between builds - not deterministic" + exit 1 +fi \ No newline at end of file