From 54e67889f10b39323fb729808b8cd22b4c832910 Mon Sep 17 00:00:00 2001
From: chick
Date: Thu, 1 Aug 2019 09:21:52 -0700
Subject: Followup to PR #1142 Fixes a threading bug in where lazy reading of
 file caused a problem for multithreaded access to the that was read.

Changes all uses of io.Source to use new API
getText
getLines
getTextResource
getLinesResouce

Make style to only import FileUtils and not its methods
So code is more explicit as e.g. FileUtils.getText()
---
 src/main/scala/firrtl/Driver.scala                               | 4 ++--
 src/main/scala/firrtl/FileUtils.scala                            | 1 +
 src/main/scala/firrtl/options/phases/GetIncludes.scala           | 3 ++-
 src/main/scala/firrtl/passes/memlib/YamlUtils.scala              | 3 ++-
 src/main/scala/firrtl/stage/phases/DriverCompatibility.scala     | 3 ++-
 src/main/scala/firrtl/util/BackendCompilationUtilities.scala     | 3 ++-
 .../firrtl/stage/phases/tests/DriverCompatibilitySpec.scala      | 2 --
 src/test/scala/firrtlTests/AnnotationTests.scala                 | 6 +++---
 src/test/scala/firrtlTests/DriverSpec.scala                      | 8 ++++----
 src/test/scala/firrtlTests/FirrtlSpec.scala                      | 1 -
 src/test/scala/firrtlTests/MultiThreadingSpec.scala              | 9 +++++----
 src/test/scala/firrtlTests/ProtoBufSpec.scala                    | 7 +++++--
 src/test/scala/firrtlTests/ReplSeqMemTests.scala                 | 4 ++--
 src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala            | 4 ++--
 .../scala/firrtlTests/transforms/BlackBoxSourceHelperSpec.scala  | 5 ++---
 src/test/scala/firrtlTests/transforms/TopWiringTest.scala        | 1 -
 16 files changed, 34 insertions(+), 30 deletions(-)

(limited to 'src')

diff --git a/src/main/scala/firrtl/Driver.scala b/src/main/scala/firrtl/Driver.scala
index f68a2035..642e7f3f 100644
--- a/src/main/scala/firrtl/Driver.scala
+++ b/src/main/scala/firrtl/Driver.scala
@@ -14,7 +14,7 @@ import firrtl.stage.{FirrtlExecutionResultView, FirrtlStage}
 import firrtl.stage.phases.DriverCompatibility
 import firrtl.options.{StageUtils, Phase, Viewer}
 import firrtl.options.phases.DeletedWrapper
-import firrtl.FileUtils.getText
+import firrtl.FileUtils
 
 
 /**
@@ -112,7 +112,7 @@ object Driver {
       JsonProtocol.deserializeTry(file).recoverWith { case jsonException =>
         // Try old protocol if new one fails
         Try {
-          val yaml = getText(file).parseYaml
+          val yaml = FileUtils.getText(file).parseYaml
           val result = yaml.convertTo[List[LegacyAnnotation]]
           val msg = s"$file is a YAML file!\n" +
                     (" "*9) + "YAML Annotation files are deprecated! Use JSON"
diff --git a/src/main/scala/firrtl/FileUtils.scala b/src/main/scala/firrtl/FileUtils.scala
index b859073d..74919f20 100644
--- a/src/main/scala/firrtl/FileUtils.scala
+++ b/src/main/scala/firrtl/FileUtils.scala
@@ -149,6 +149,7 @@ object FileUtils {
   def getLinesResource(resourceName: String): Seq[String] = {
     val inputStream = getClass.getResourceAsStream(resourceName)
     val text = io.Source.fromInputStream(inputStream).getLines().toSeq
+    text.length  // This forces lazy buffer to reify, please suggest a better solution
     inputStream.close()
     text
   }
diff --git a/src/main/scala/firrtl/options/phases/GetIncludes.scala b/src/main/scala/firrtl/options/phases/GetIncludes.scala
index 9e198c61..cb9bb840 100644
--- a/src/main/scala/firrtl/options/phases/GetIncludes.scala
+++ b/src/main/scala/firrtl/options/phases/GetIncludes.scala
@@ -8,6 +8,7 @@ import firrtl.AnnotationSeq
 import firrtl.annotations.{AnnotationFileNotFoundException, JsonProtocol, LegacyAnnotation}
 import firrtl.annotations.AnnotationYamlProtocol._
 import firrtl.options.{InputAnnotationFileAnnotation, Phase, StageUtils}
+import firrtl.FileUtils
 
 import java.io.File
 
@@ -27,7 +28,7 @@ class GetIncludes extends Phase {
     JsonProtocol.deserializeTry(file).recoverWith { case jsonException =>
       // Try old protocol if new one fails
       Try {
-        val yaml = io.Source.fromFile(file).getLines().mkString("\n").parseYaml
+        val yaml = FileUtils.getText(file).parseYaml
         val result = yaml.convertTo[List[LegacyAnnotation]]
         val msg = s"$file is a YAML file!\n" + (" "*9) + "YAML Annotation files are deprecated! Use JSON"
         StageUtils.dramaticWarning(msg)
diff --git a/src/main/scala/firrtl/passes/memlib/YamlUtils.scala b/src/main/scala/firrtl/passes/memlib/YamlUtils.scala
index eab1fe37..a43adfe2 100644
--- a/src/main/scala/firrtl/passes/memlib/YamlUtils.scala
+++ b/src/main/scala/firrtl/passes/memlib/YamlUtils.scala
@@ -4,6 +4,7 @@ package firrtl.passes
 package memlib
 import net.jcazevedo.moultingyaml._
 import java.io.{CharArrayWriter, File, PrintWriter}
+import firrtl.FileUtils
 
 
 object CustomYAMLProtocol extends DefaultYamlProtocol {
@@ -23,7 +24,7 @@ case class Config(pin: Pin, source: Source, top: Top)
 class YamlFileReader(file: String) {
   def parse[A](implicit reader: YamlReader[A]) : Seq[A] = {
     if (new File(file).exists) {
-      val yamlString = scala.io.Source.fromFile(file).getLines.mkString("\n")
+      val yamlString = FileUtils.getText(file)
       yamlString.parseYamls flatMap (x =>
         try Some(reader read x)
         catch { case e: Exception => None }
diff --git a/src/main/scala/firrtl/stage/phases/DriverCompatibility.scala b/src/main/scala/firrtl/stage/phases/DriverCompatibility.scala
index f0f1c067..665861ef 100644
--- a/src/main/scala/firrtl/stage/phases/DriverCompatibility.scala
+++ b/src/main/scala/firrtl/stage/phases/DriverCompatibility.scala
@@ -6,6 +6,7 @@ import firrtl.stage._
 
 import firrtl.{AnnotationSeq, EmitAllModulesAnnotation, EmitCircuitAnnotation, FirrtlExecutionResult, Parser}
 import firrtl.annotations.NoTargetAnnotation
+import firrtl.FileUtils
 import firrtl.proto.FromProto
 import firrtl.options.{InputAnnotationFileAnnotation, OptionsException, Phase,
   StageOptions, StageUtils}
@@ -89,7 +90,7 @@ object DriverCompatibility {
           annotations.collectFirst{ case FirrtlFileAnnotation(f) =>
             FirrtlStageUtils.getFileExtension(f) match {
               case ProtoBufFile => FromProto.fromFile(f).main
-              case FirrtlFile   => Parser.parse(io.Source.fromFile(f).getLines().mkString("\n")).main } } )))
+              case FirrtlFile   => Parser.parse(FileUtils.getText(f)).main } } )))
 
   /** Determine the target directory with the following precedence (highest to lowest):
     *  - Explicitly from the user-specified [[firrtl.options.TargetDirAnnotation TargetDirAnnotation]]
diff --git a/src/main/scala/firrtl/util/BackendCompilationUtilities.scala b/src/main/scala/firrtl/util/BackendCompilationUtilities.scala
index e0341bf1..beb44ad0 100644
--- a/src/main/scala/firrtl/util/BackendCompilationUtilities.scala
+++ b/src/main/scala/firrtl/util/BackendCompilationUtilities.scala
@@ -7,6 +7,7 @@ import java.nio.file.Files
 import java.text.SimpleDateFormat
 import java.util.Calendar
 
+import firrtl.FileUtils
 
 import scala.sys.process.{ProcessBuilder, ProcessLogger, _}
 
@@ -121,7 +122,7 @@ trait BackendCompilationUtilities {
     // Build a set of canonical file paths to use as a filter to exclude already included additional Verilog sources.
     val blackBoxHelperFiles: Set[String] = {
       if(list_file.exists()) {
-        io.Source.fromFile(list_file).getLines.toSet
+        FileUtils.getLines(list_file).toSet
       }
       else {
         Set.empty
diff --git a/src/test/scala/firrtl/stage/phases/tests/DriverCompatibilitySpec.scala b/src/test/scala/firrtl/stage/phases/tests/DriverCompatibilitySpec.scala
index 77316feb..d06344af 100644
--- a/src/test/scala/firrtl/stage/phases/tests/DriverCompatibilitySpec.scala
+++ b/src/test/scala/firrtl/stage/phases/tests/DriverCompatibilitySpec.scala
@@ -4,8 +4,6 @@ package firrtl.stage.phases.tests
 
 import org.scalatest.{FlatSpec, Matchers, PrivateMethodTester}
 
-import scala.io.Source
-
 import java.io.File
 
 import firrtl._
diff --git a/src/test/scala/firrtlTests/AnnotationTests.scala b/src/test/scala/firrtlTests/AnnotationTests.scala
index 6b492148..7b944470 100644
--- a/src/test/scala/firrtlTests/AnnotationTests.scala
+++ b/src/test/scala/firrtlTests/AnnotationTests.scala
@@ -7,6 +7,7 @@ import java.io.{File, FileWriter, Writer}
 import firrtl.annotations.AnnotationYamlProtocol._
 import firrtl.annotations._
 import firrtl._
+import firrtl.FileUtils
 import firrtl.transforms.OptimizableExtModuleAnnotation
 import firrtl.passes.InlineAnnotation
 import firrtl.passes.memlib.PinAnnotation
@@ -472,8 +473,7 @@ class LegacyAnnotationTests extends AnnotationTests {
     Annotation(ModuleName(mod, CircuitName("Top")), classOf[Transform], "some value")
 
   "LegacyAnnotations" should "be readable from file" in {
-    val annotationStream = getClass.getResourceAsStream("/annotations/SampleAnnotations.anno")
-    val annotationsYaml = scala.io.Source.fromInputStream(annotationStream).getLines().mkString("\n").parseYaml
+    val annotationsYaml = FileUtils.getTextResource("/annotations/SampleAnnotations.anno").parseYaml
     val annotationArray = annotationsYaml.convertTo[Array[LegacyAnnotation]]
     annotationArray.length should be (9)
     annotationArray(0).targetString should be ("ModC")
@@ -538,7 +538,7 @@ class JsonAnnotationTests extends AnnotationTests with BackendCompilationUtiliti
     writer.write(JsonProtocol.serialize(annos))
     writer.close()
 
-    val text = io.Source.fromFile(annoFile).getLines().mkString("\n")
+    val text = FileUtils.getText(annoFile)
     annoFile.delete()
 
     val readAnnos = JsonProtocol.deserializeTry(text).get
diff --git a/src/test/scala/firrtlTests/DriverSpec.scala b/src/test/scala/firrtlTests/DriverSpec.scala
index 1cb991c8..b4f61dfd 100644
--- a/src/test/scala/firrtlTests/DriverSpec.scala
+++ b/src/test/scala/firrtlTests/DriverSpec.scala
@@ -9,10 +9,10 @@ import firrtl.passes.{InlineAnnotation, InlineInstances}
 import firrtl.passes.memlib.{InferReadWrite, InferReadWriteAnnotation, ReplSeqMem, ReplSeqMemAnnotation}
 import firrtl.transforms.BlackBoxTargetDirAnno
 import firrtl._
+import firrtl.FileUtils
 import firrtl.annotations._
 import firrtl.util.BackendCompilationUtilities
 
-import scala.io.Source
 import scala.util.{Failure, Success, Try}
 
 class ExceptingTransform extends Transform {
@@ -307,7 +307,7 @@ class DriverSpec extends FreeSpec with Matchers with BackendCompilationUtilities
     copyResourceToFile(s"/annotations/$annoFilename", annotationsTestFile)
 
     import net.jcazevedo.moultingyaml._
-    val text = io.Source.fromFile(annotationsTestFile).mkString
+    val text = FileUtils.getText(annotationsTestFile)
     val yamlAnnos = text.parseYaml match {
       case YamlArray(xs) => xs
     }
@@ -467,8 +467,8 @@ class DriverSpec extends FreeSpec with Matchers with BackendCompilationUtilities
       Driver.execute(args)
     }
     "Both paths do the same thing" in {
-      val s1 = Source.fromFile(verilogFromFir).mkString
-      val s2 = Source.fromFile(verilogFromPb).mkString
+      val s1 = FileUtils.getText(verilogFromFir)
+      val s2 = FileUtils.getText(verilogFromPb)
       s1 should equal (s2)
     }
   }
diff --git a/src/test/scala/firrtlTests/FirrtlSpec.scala b/src/test/scala/firrtlTests/FirrtlSpec.scala
index c110109c..efbb27de 100644
--- a/src/test/scala/firrtlTests/FirrtlSpec.scala
+++ b/src/test/scala/firrtlTests/FirrtlSpec.scala
@@ -11,7 +11,6 @@ import scala.sys.process._
 import org.scalatest._
 import org.scalatest.prop._
 
-import scala.io.Source
 import firrtl._
 import firrtl.ir._
 import firrtl.Parser.{IgnoreInfo, UseInfo}
diff --git a/src/test/scala/firrtlTests/MultiThreadingSpec.scala b/src/test/scala/firrtlTests/MultiThreadingSpec.scala
index 444faebc..72c66e93 100644
--- a/src/test/scala/firrtlTests/MultiThreadingSpec.scala
+++ b/src/test/scala/firrtlTests/MultiThreadingSpec.scala
@@ -2,10 +2,11 @@
 
 package firrtlTests
 
-import firrtl.{ChirrtlForm, CircuitState, Compiler, annotations}
+import firrtl.FileUtils
+import firrtl.{ChirrtlForm, CircuitState}
 
-import scala.concurrent.{Future, Await, ExecutionContext}
 import scala.concurrent.duration.Duration
+import scala.concurrent.{Await, ExecutionContext, Future}
 
 class MultiThreadingSpec extends FirrtlPropSpec {
 
@@ -27,8 +28,8 @@ class MultiThreadingSpec extends FirrtlPropSpec {
     val numThreads = 64 // arbitrary
 
     // Begin the actual test
-    val inputStream = getClass().getResourceAsStream(inputFilePath)
-    val inputStrings = io.Source.fromInputStream(inputStream).getLines().toList
+
+    val inputStrings = FileUtils.getLinesResource(inputFilePath)
 
     import ExecutionContext.Implicits.global
     try { // Use try-catch because error can manifest in many ways
diff --git a/src/test/scala/firrtlTests/ProtoBufSpec.scala b/src/test/scala/firrtlTests/ProtoBufSpec.scala
index 7a0c3eeb..3d46e291 100644
--- a/src/test/scala/firrtlTests/ProtoBufSpec.scala
+++ b/src/test/scala/firrtlTests/ProtoBufSpec.scala
@@ -30,8 +30,11 @@ class ProtoBufSpec extends FirrtlFlatSpec {
 
   for (FirrtlResourceTest(name, dir) <- firrtlResourceTests) {
     s"$name" should "work with Protobuf serialization and deserialization" in {
-      val stream = getClass.getResourceAsStream(s"$dir/$name.fir")
-      val circuit = parse(scala.io.Source.fromInputStream(stream).getLines.mkString("\n"))
+//      val stream = getClass.getResourceAsStream(s"$dir/$name.fir")
+//      val circuit = parse(scala.io.Source.fromInputStream(stream).getLines.mkString("\n"))
+//      stream.close()
+
+      val circuit = parse(FileUtils.getTextResource(s"$dir/$name.fir"))
 
       // Test ToProto and FromProto
       val protobuf = proto.ToProto.convert(circuit)
diff --git a/src/test/scala/firrtlTests/ReplSeqMemTests.scala b/src/test/scala/firrtlTests/ReplSeqMemTests.scala
index a1f27958..9a5a2f72 100644
--- a/src/test/scala/firrtlTests/ReplSeqMemTests.scala
+++ b/src/test/scala/firrtlTests/ReplSeqMemTests.scala
@@ -7,6 +7,7 @@ import firrtl.ir._
 import firrtl.passes._
 import firrtl.transforms._
 import firrtl.passes.memlib._
+import firrtl.FileUtils
 import annotations._
 import FirrtlCheckers._
 
@@ -29,8 +30,7 @@ class ReplSeqMemSpec extends SimpleTransformSpec {
 
   def checkMemConf(filename: String, mems: Set[MemConf]) {
     // Read the mem conf
-    val file = scala.io.Source.fromFile(filename)
-    val text = try file.mkString finally file.close()
+    val text = FileUtils.getText(filename)
     // Verify that this does not throw an exception
     val fromConf = MemConf.fromString(text)
     // Verify the mems in the conf are the same as the expected ones
diff --git a/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala b/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala
index 0d1120fc..e6129550 100644
--- a/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala
+++ b/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala
@@ -6,7 +6,7 @@ import org.scalatest.{FeatureSpec, GivenWhenThen, Matchers}
 
 import java.io.{File, PrintWriter}
 
-import scala.io.Source
+import firrtl.FileUtils
 
 import firrtl.stage.FirrtlMain
 import firrtl.util.BackendCompilationUtilities
@@ -312,7 +312,7 @@ class FirrtlMainSpec extends FeatureSpec with GivenWhenThen with Matchers with f
 
       Then("the implicit annotation file should NOT be read")
       val annoFileOut = new File(td.dir + "/Top.out.anno.json")
-      val annotationJson = Source.fromFile(annoFileOut).mkString
+      val annotationJson = FileUtils.getText(annoFileOut)
       annotationJson should not include ("InlineInstances")
 
       And("no warning should be printed")
diff --git a/src/test/scala/firrtlTests/transforms/BlackBoxSourceHelperSpec.scala b/src/test/scala/firrtlTests/transforms/BlackBoxSourceHelperSpec.scala
index 089e837a..6e63317b 100644
--- a/src/test/scala/firrtlTests/transforms/BlackBoxSourceHelperSpec.scala
+++ b/src/test/scala/firrtlTests/transforms/BlackBoxSourceHelperSpec.scala
@@ -6,6 +6,7 @@ import firrtl.annotations.{Annotation, CircuitName, ModuleName}
 import firrtl.transforms._
 import firrtl.{FIRRTLException, Transform, VerilogCompiler, VerilogEmitter}
 import firrtlTests.{HighTransformSpec, LowTransformSpec}
+import firrtl.FileUtils
 import org.scalacheck.Test.Failed
 import org.scalatest.{FreeSpec, Matchers, Succeeded}
 
@@ -132,9 +133,7 @@ class BlacklBoxSourceHelperTransformSpec extends LowTransformSpec {
     //  but our file list should not include the verilog header file.
     val fileListFile = new java.io.File(s"test_run_dir/${BlackBoxSourceHelper.defaultFileListName}")
     fileListFile.exists should be (true)
-    val fileListFileSource = io.Source.fromFile(fileListFile)
-    val fileList = fileListFileSource.getLines.mkString
-    fileListFileSource.close()
+    val fileList = FileUtils.getText(fileListFile)
     fileList.contains("ParameterizedViaHeaderAdderExtModule.v") should be (true)
     fileList.contains("VerilogHeaderFile.vh") should be (false)
   }
diff --git a/src/test/scala/firrtlTests/transforms/TopWiringTest.scala b/src/test/scala/firrtlTests/transforms/TopWiringTest.scala
index 16dffd66..9dd290f8 100644
--- a/src/test/scala/firrtlTests/transforms/TopWiringTest.scala
+++ b/src/test/scala/firrtlTests/transforms/TopWiringTest.scala
@@ -6,7 +6,6 @@ package transforms
 import org.scalatest.FlatSpec
 import org.scalatest.Matchers
 import org.scalatest.junit.JUnitRunner
-import scala.io.Source
 import java.io._
 
 import firrtl._
-- 
cgit v1.2.3


From 2bf399c240938ba51069348f986fa5d65135a808 Mon Sep 17 00:00:00 2001
From: chick
Date: Thu, 1 Aug 2019 17:04:45 -0700
Subject: Followup to PR #1142 - use scala.io.Source instead of io.Source -
 .toList cleaner way to force stream to be read. - clear old commented out
 code in ProtoBufSpec

---
 src/main/scala/firrtl/FileUtils.scala         | 15 ++++++++-------
 src/test/scala/firrtlTests/ProtoBufSpec.scala |  6 ------
 2 files changed, 8 insertions(+), 13 deletions(-)

(limited to 'src')

diff --git a/src/main/scala/firrtl/FileUtils.scala b/src/main/scala/firrtl/FileUtils.scala
index 74919f20..1007eb2a 100644
--- a/src/main/scala/firrtl/FileUtils.scala
+++ b/src/main/scala/firrtl/FileUtils.scala
@@ -98,7 +98,7 @@ object FileUtils {
     * @param fileName The file to read
     */
   def getLines(fileName: String): Seq[String] = {
-    val source = io.Source.fromFile(fileName)
+    val source = scala.io.Source.fromFile(fileName)
     val lines = source.getLines()
     source.close()
     lines.toSeq
@@ -110,7 +110,7 @@ object FileUtils {
     * @param file a java File to be read
     */
   def getLines(file: File): Seq[String] = {
-    val source = io.Source.fromFile(file)
+    val source = scala.io.Source.fromFile(file)
     val lines = source.getLines()
     source.close()
     lines.toSeq
@@ -122,7 +122,7 @@ object FileUtils {
     * @param fileName The file to read
     */
   def getText(fileName: String): String = {
-    val source = io.Source.fromFile(fileName)
+    val source = scala.io.Source.fromFile(fileName)
     val text = source.mkString
     source.close()
     text
@@ -134,7 +134,7 @@ object FileUtils {
     * @param file a java File to be read
     */
   def getText(file: File): String = {
-    val source = io.Source.fromFile(file)
+    val source = scala.io.Source.fromFile(file)
     val text = source.mkString
     source.close()
     text
@@ -148,8 +148,9 @@ object FileUtils {
     */
   def getLinesResource(resourceName: String): Seq[String] = {
     val inputStream = getClass.getResourceAsStream(resourceName)
-    val text = io.Source.fromInputStream(inputStream).getLines().toSeq
-    text.length  // This forces lazy buffer to reify, please suggest a better solution
+    // the .toList at the end is critical to force stream to be read.
+    // Without it the lazy evaluation can cause failure in MultiThreadingSpec
+    val text = scala.io.Source.fromInputStream(inputStream).getLines().toList
     inputStream.close()
     text
   }
@@ -162,7 +163,7 @@ object FileUtils {
     */
   def getTextResource(resourceName: String): String = {
     val inputStream = getClass.getResourceAsStream(resourceName)
-    val text = io.Source.fromInputStream(inputStream).mkString
+    val text = scala.io.Source.fromInputStream(inputStream).mkString
     inputStream.close()
     text
   }
diff --git a/src/test/scala/firrtlTests/ProtoBufSpec.scala b/src/test/scala/firrtlTests/ProtoBufSpec.scala
index 3d46e291..526a194c 100644
--- a/src/test/scala/firrtlTests/ProtoBufSpec.scala
+++ b/src/test/scala/firrtlTests/ProtoBufSpec.scala
@@ -2,8 +2,6 @@
 
 package firrtlTests
 
-import java.io.{ByteArrayInputStream, ByteArrayOutputStream}
-
 import firrtl.FirrtlProtos.Firrtl
 import firrtl._
 import firrtl.ir._
@@ -30,10 +28,6 @@ class ProtoBufSpec extends FirrtlFlatSpec {
 
   for (FirrtlResourceTest(name, dir) <- firrtlResourceTests) {
     s"$name" should "work with Protobuf serialization and deserialization" in {
-//      val stream = getClass.getResourceAsStream(s"$dir/$name.fir")
-//      val circuit = parse(scala.io.Source.fromInputStream(stream).getLines.mkString("\n"))
-//      stream.close()
-
       val circuit = parse(FileUtils.getTextResource(s"$dir/$name.fir"))
 
       // Test ToProto and FromProto
-- 
cgit v1.2.3