Merge pull request #98 from lucidsoftware/bfreestone-fixes-source-filepath-determinism-issue

Fix tasty file non-determinism with `sourceroot` argument in multiplex worker and `analysis_store.gz` changes

Author: blainefreestone

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)
+  }
+}

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 =

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"],
+)

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
+  }
+}

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