adding a targetId to help grab correct instrument file at runtime from cp

Sameer Arora · Sameer Arora · commit 626e99e4f7ae · 2019-07-14T18:02:32.000-07:00
diff --git a/build.sbt b/build.sbt
@@ -71,7 +71,6 @@ lazy val runtime = CrossProject("scalac-scoverage-runtime", file("scalac-scovera
     .settings(appSettings: _*)
     .jvmSettings(
       fork in Test := true,
-      envVars in Test := Map("SCOVERAGE_MEASUREMENT_PATH" -> "scoverageMeasurementFiles"),
       libraryDependencies ++= Seq(
         "org.scalatest" %% "scalatest" % ScalatestVersion % "test"
       )
diff --git a/scalac-scoverage-plugin/src/main/scala/scoverage/Constants.scala b/scalac-scoverage-plugin/src/main/scala/scoverage/Constants.scala
@@ -1,13 +1,19 @@
 package scoverage
 
 object Constants {
-  // the file that contains the statement mappings
-  val CoverageFileName = "scoverage.coverage"
   // the final scoverage report
   val XMLReportFilename = "scoverage.xml"
   val XMLReportFilenameWithDebug = "scoverage-debug.xml"
   // directory that contains all the measurement data but not reports
   val DataDir = "scoverage-data"
+  /*********************************************************************
+   * If updating any of the files below, an update is also required to *
+   * the corresponding file name in Invoker.scala                      *
+   *********************************************************************/
+  // the file that contains the statement mappings
+  val CoverageFileName = "scoverage.coverage"
   // the prefix the measurement files have
   val MeasurementsPrefix = "scoverage.measurements."
+  //subdir to suffix to classpath when [writeToClasspath] option is in use"
+  val ClasspathSubdir = "META-INF/scoverage"
 }
diff --git a/scalac-scoverage-plugin/src/main/scala/scoverage/plugin.scala b/scalac-scoverage-plugin/src/main/scala/scoverage/plugin.scala
@@ -34,9 +34,7 @@ class ScoveragePlugin(val global: Global) extends Plugin {
         options.dataDir = opt.substring("dataDir:".length)
       } else if (opt.startsWith("extraAfterPhase:") || opt.startsWith("extraBeforePhase:")) {
         // skip here, these flags are processed elsewhere
-      } else if (opt.startsWith("classpathSubdir:")){
-        options.classpathSubdir = opt.substring("classpathSubdir:".length)
-      }else if (opt.startsWith("writeToClasspath:")) {
+      } else if (opt.startsWith("writeToClasspath:")) {
         try {
           options.writeToClasspath = opt.substring("writeToClasspath:".length).toBoolean
         } catch {
@@ -56,10 +54,10 @@ class ScoveragePlugin(val global: Global) extends Plugin {
   }
 
   override val optionsHelp: Option[String] = Some(Seq(
-    "-P:scoverage:writeToClasspath:<boolean>               if true, use the environment variable to store measurements" +
-                                                           "and store instruments to the output classpath where compiler emits classfiles." +
+    "-P:scoverage:writeToClasspath:<boolean>               if true, use the system property variable [scoverage_measurement_path]" +
+                                                           " to store measurements " +
+                                                           "and store instruments file to the output classpath where compiler emits classfiles. " +
                                                            "Overrides the datadir with the output classpath.",
-    "-P:scoverage:classpathSubdir:<pathtosubdir>           subdir to attach to classpath. Default: META-INF/scoverage",
 
     "-P:scoverage:dataDir:<pathtodatadir>                  where the coverage files should be written\n",
     "-P:scoverage:excludedPackages:<regex>;<regex>         semicolon separated list of regexs for packages to exclude",
@@ -101,13 +99,12 @@ class ScoverageOptions {
 
   // Adding additional option for https://github.com/scoverage/scalac-scoverage-plugin/issues/265
   // If the option is false, scoverage works the usual way.
-  // If the option is true, the instruments are stored on to the classpath and
-  // the measurements are written to the directory specified by the environment variable
-  // `SCOVERAGE_MEASUREMENT_PATH`. This means setting [writeToClasspath] to true overrides the [dataDir]
+  // If the option is true, the instrument files [scoverage.coverage] are stored on to the classpath and
+  // the measurement files are written to the directory specified by the system property
+  // `scoverage_measurement_path`. This means setting [writeToClasspath] to true overrides the [dataDir]
   // as the [dataDir] will now point to the output classpath of the compiler.
   // By default, the option is set to false.
   var writeToClasspath: Boolean = false
-  var classpathSubdir: String = "META-INF/scoverage"
 }
 
 class ScoverageInstrumentationComponent(val global: Global, extraAfterPhase: Option[String], extraBeforePhase: Option[String])
@@ -133,6 +130,27 @@ class ScoverageInstrumentationComponent(val global: Global, extraAfterPhase: Opt
   private var options: ScoverageOptions = new ScoverageOptions()
   private var coverageFilter: CoverageFilter = AllCoverageFilter
 
+  // This [targetId] is used below in case [writeToClasspath] option
+  // is true. It is used for:
+  // 1. Creating a unique subdir under the classpath to store the instrument files, i.e
+  //    path to instrument files will be `< classpath >/META-INF/scoverage/< targetId >/scoverage.coverage`
+  // 2. Instrumenting the binaries with the [targetId], i.e binaries will be instrumented with
+  //    `Invoker.invokedWriteToClasspath(< statement id >, < targetId >)`
+  //    Consequently, at runtime, we can get the "correct" instrument file located at `.../< targetId >/scoverage.coverage`
+  //    from the classpath and store it with its appropriate measurements file.
+  private var targetId: String = ""
+
+  // used for generating the targetId.
+  def md5HashString(s: String): String = {
+    import java.security.MessageDigest
+    import java.math.BigInteger
+    val md = MessageDigest.getInstance("MD5")
+    val digest = md.digest(s.getBytes)
+    val bigInt = new BigInteger(1,digest)
+    val hashedString = bigInt.toString(16)
+    hashedString
+  }
+
   def setOptions(options: ScoverageOptions): Unit = {
 
     this.options = options
@@ -142,7 +160,11 @@ class ScoverageInstrumentationComponent(val global: Global, extraAfterPhase: Opt
     // i.e to the directory where compiler is going to output the classfiles.
     if(options.writeToClasspath){
       settings.outputDirs.getSingleOutput match {
-        case Some(dest) => options.dataDir = s"${dest.toString()}/${options.classpathSubdir}"
+        case Some(dest) =>
+          targetId = md5HashString(s"${dest.toString()}")
+          // Setting [dataDir] to `< classpath >/META-INF/scoverage/< targetId >`.
+          // This is where the instrument file [scoverage.coverage] for this target will now be stored.
+          options.dataDir = s"${dest.toString()}/${Constants.ClasspathSubdir}/$targetId"
         case None => throw new RuntimeException("No output classpath specified.")
       }
     }
@@ -173,8 +195,8 @@ class ScoverageInstrumentationComponent(val global: Global, extraAfterPhase: Opt
         reporter.echo(s"Will write measurement data to [${ options.dataDir }]")
       }
       else {
-        reporter.echo(s"Will write measurements data to the directory specified by environment" +
-          s" variable SCOVERAGE_MEASUREMENT_PATH at runtime.")
+        reporter.echo(s"Will write measurements data to the directory specified by system" +
+          s" variable scoverage_measurement_path at runtime.")
       }
     }
   }
@@ -199,49 +221,41 @@ class ScoverageInstrumentationComponent(val global: Global, extraAfterPhase: Opt
     def safeLine(tree: Tree): Int = if (tree.pos.isDefined) tree.pos.line else -1
     def safeSource(tree: Tree): Option[SourceFile] = if (tree.pos.isDefined) Some(tree.pos.source) else None
 
-    def invokeCall(id: Int): Tree = {
+    var termName: String = ""
+    var secondArg: String = ""
+    if (options.writeToClasspath){
       /**
-       * We still pass in [options.dataDir] as one of the instruments with
-       * [invokedUseEnvironment] as it helps differentiating the source
-       * files at runtime, i.e helps in creating a unique subdir for each source file.
+       * We pass in [targetId] as one of the instruments with
+       * [invokedUseEnvironment] as it helps in grabbing the "correct" instruments
+       * file from the classpath at runtime. It also helps in
+       * differentiating the targets at runtime, i.e helps in creating a unique subdir for each target.
        */
-      if (options.writeToClasspath){
-        Apply(
+      termName = "invokedWriteToClasspath"
+      secondArg = targetId
+    }
+    else{
+      termName = "invoked"
+      secondArg = options.dataDir
+    }
+
+    def invokeCall(id: Int): Tree = {
+      Apply(
+        Select(
           Select(
-            Select(
-              Ident("scoverage"),
-              newTermName("Invoker")
-            ),
-            newTermName("invokedWriteToClasspath")
+            Ident("scoverage"),
+            newTermName("Invoker")
           ),
-          List(
-            Literal(
-              Constant(id)
-            ),
-            Literal(
-              Constant(options.dataDir)
-            )
-          )
-        )
-      }else {
-        Apply(
-          Select(
-            Select(
-              Ident("scoverage"),
-              newTermName("Invoker")
-            ),
-            newTermName("invoked")
+          newTermName(termName)
+        ),
+        List(
+          Literal(
+            Constant(id)
           ),
-          List(
-            Literal(
-              Constant(id)
-            ),
-            Literal(
-              Constant(options.dataDir)
-            )
+          Literal(
+            Constant(secondArg)
           )
         )
-      }
+      )
     }
 
     override def transform(tree: Tree) = process(tree)
diff --git a/scalac-scoverage-runtime/jvm/src/test/scala/scoverage/InvokerUseEnvironmentTest.scala b/scalac-scoverage-runtime/jvm/src/test/scala/scoverage/InvokerUseEnvironmentTest.scala
@@ -1,6 +1,5 @@
 package scoverage
 
-import java.nio.file.{Files, Paths}
 import org.scalatest.{BeforeAndAfter, FunSuite}
 import scoverage.Platform.File
 
@@ -10,58 +9,47 @@ import scoverage.Platform.File
 class InvokerUseEnvironmentTest extends FunSuite with BeforeAndAfter {
 
 
-  val instrumentsDir = Array(
-    new File("target/invoker-test.instrument0"),
-    new File("target/invoker-test.instrument1")
+  val targetIds = Array(
+    new File("target_id_0"),
+    new File("target_id_1")
   )
 
   before {
-    deleteInstrumentFiles()
-    instrumentsDir.foreach(_.mkdirs())
-    createNewInstruments()
+    System.setProperty("scoverage_measurement_path","scoverage_java_prop")
   }
 
-  test("calling Invoker.invokedUseEnvironments puts the data on env var SCOVERAGE_MEASUREMENT_PATH" +
-    " and copies coverage files from instruments directory.") {
+  test("calling Invoker.invokedUseEnvironments puts the data on sys property scoverage_measurement_path") {
 
     val testIds: Set[Int] = (1 to 10).toSet
 
-    testIds.map { i: Int => Invoker.invokedWriteToClasspath(i, instrumentsDir(i % 2).toString)}
+    testIds.map { i: Int => Invoker.invokedWriteToClasspath(i, targetIds(i % 2).toString)}
 
-    // Verify measurements went to correct directory under the environment variable.
-    val dir0 = s"${System.getenv("SCOVERAGE_MEASUREMENT_PATH")}/${Invoker.safe_name(instrumentsDir(0).toString)}"
+    // Verify that [Invoker.dataDir] has been set correctly.
+    assert(Invoker.dataDir == System.getProperty("scoverage_measurement_path"))
+
+    // Verify measurements went to correct directory under the system property.
+    val dir0 = s"${Invoker.dataDir}/${targetIds(0).toString}"
     val measurementFiles3 = Invoker.findMeasurementFiles(dir0)
     val idsFromFile3 = Invoker.invoked(measurementFiles3.toIndexedSeq)
     assert (idsFromFile3 == testIds.filter { i: Int => i % 2 == 0})
 
 
-    val dir1 = s"${System.getenv("SCOVERAGE_MEASUREMENT_PATH")}/${Invoker.safe_name(instrumentsDir(1).toString)}"
+    val dir1 = s"${Invoker.dataDir}/${targetIds(1).toString}"
     val measurementFiles4 = Invoker.findMeasurementFiles(dir1)
     val idsFromFile4 = Invoker.invoked(measurementFiles4.toIndexedSeq)
     assert (idsFromFile4 == testIds.filter { i: Int => i % 2 == 1})
 
-    // Verify that coverage files have been copied correctly.
-    assert(Files.exists(Paths.get(s"$dir0/scoverage.coverage")))
-    assert(Files.exists(Paths.get(s"$dir1/scoverage.coverage")))
   }
 
   after {
-    deleteInstrumentFiles()
-    instrumentsDir.foreach(_.delete())
     deleteMeasurementFolders()
   }
 
-  private def deleteInstrumentFiles(): Unit = {
-    instrumentsDir.foreach((md) => {
-      if (md.isDirectory)
-        md.listFiles().foreach(_.delete())
-    })
-  }
 
   private def deleteMeasurementFolders(): Unit = {
-    val d = s"${System.getenv("SCOVERAGE_MEASUREMENT_PATH")}"
-    instrumentsDir.foreach (i => {
-      val f = new File(s"$d/${Invoker.safe_name(i.toString)}")
+    val d = s"${Invoker.dataDir}"
+    targetIds.foreach (i => {
+      val f = new File(s"$d/${i.toString}")
       if (f.isDirectory)
         f.listFiles().foreach(_.delete())
     })
@@ -72,9 +60,5 @@ class InvokerUseEnvironmentTest extends FunSuite with BeforeAndAfter {
     f2.delete()
   }
 
-  private def createNewInstruments(): Unit = {
-    new File("target/invoker-test.instrument0/scoverage.coverage").createNewFile()
-    new File("target/invoker-test.instrument1/scoverage.coverage").createNewFile()
-  }
 
 }
diff --git a/scalac-scoverage-runtime/shared/src/main/scala/scoverage/Invoker.scala b/scalac-scoverage-runtime/shared/src/main/scala/scoverage/Invoker.scala

Original file line number	Diff line number	Diff line change
`@@ -71,7 +71,6 @@ lazy val runtime = CrossProject("scalac-scoverage-runtime", file("scalac-scovera`
`71`	`71`	`.settings(appSettings: _*)`
`72`	`72`	`.jvmSettings(`
`73`	`73`	`fork in Test := true,`
`74`		`- envVars in Test := Map("SCOVERAGE_MEASUREMENT_PATH" -> "scoverageMeasurementFiles"),`
`75`	`74`	`libraryDependencies ++= Seq(`
`76`	`75`	`"org.scalatest" %% "scalatest" % ScalatestVersion % "test"`
`77`	`76`	`)`