Filter resource file names to avoid including the same file multiple times. (#883)

* Filter resource file names to avoid including the same file multiple times.
Addresses issue #882.

* Use a Set instead of a Map to filter Verilog files.

* Use canonical paths for file name comparison and unify name generation.
Provide a common method for copying resources to a directory to ensure the same resource ends up with the same name if it's copied by multiple clients.

* Reduce confusion - another absolute -> canonical switch.
Use the canonical path on the verilator command line for the filter additional Verilog sources.

2 files changed, 45 insertions, 14 deletions

diff --git a/src/main/scala/firrtl/transforms/BlackBoxSourceHelper.scala b/src/main/scala/firrtl/transforms/BlackBoxSourceHelper.scala
index 1e6fa7ea..182accf2 100644
--- a/src/main/scala/firrtl/transforms/BlackBoxSourceHelper.scala
+++ b/src/main/scala/firrtl/transforms/BlackBoxSourceHelper.scala
@@ -70,12 +70,7 @@ class BlackBoxSourceHelper extends firrtl.Transform {
 
     val resourceFiles: ListSet[File] = annos.collect {
       case BlackBoxResourceAnno(_, resourceId) =>
-        val name = resourceId.split("/").last
-        val outFile = new File(targetDir, name)
-        (resourceId, outFile)
-    }.map { case (res, file) =>
-      BlackBoxSourceHelper.copyResourceToFile(res, file)
-      file
+        BlackBoxSourceHelper.writeResourceToDirectory(resourceId, targetDir)
     }
 
     val inlineFiles: ListSet[File] = annos.collect {
@@ -97,6 +92,19 @@ object BlackBoxSourceHelper {
   /**
     * finds the named resource and writes into the directory
     * @param name the name of the resource
+    * @param dir the directory in which to write the file
+    * @return the closed File object
+    */
+  def writeResourceToDirectory(name: String, dir: File): File = {
+    val fileName = name.split("/").last
+    val outFile = new File(dir, fileName)
+    copyResourceToFile(name, outFile)
+    outFile
+  }
+
+  /**
+    * finds the named resource and writes into the directory
+    * @param name the name of the resource
     * @param file the file to write it into
     */
   def copyResourceToFile(name: String, file: File) {
@@ -113,11 +121,13 @@ object BlackBoxSourceHelper {
 
   def writeFileList(files: ListSet[File], targetDir: File) {
     if (files.nonEmpty) {
-      // We need the absolute path here (or strip targetDir from the file path),
-      //  so verilator will create a path to the file that works from the targetDir.
-      //  Otherwise, when make tries to determine dependencies based on the *__ver.d file, we end up with errors like:
-      //    make[1]: *** No rule to make target `test_run_dir/examples.AccumBlackBox_PeekPokeTest_Verilator345491158/AccumBlackBox.v', needed by `.../chisel-testers/test_run_dir/examples.AccumBlackBox_PeekPokeTest_Verilator345491158/VAccumBlackBoxWrapper.h'.  Stop.
-      writeTextToFile(files.map(_.getAbsolutePath).mkString("\n"), new File(targetDir, fileListName))
+      // We need the canonical path here, so verilator will create a path to the file that works from the targetDir,
+      //  and, so we can compare the list of files automatically included, with an explicit list provided by the client
+      //  and reject duplicates.
+      // If the path isn't canonical, when make tries to determine dependencies based on the *__ver.d file, we end up with errors like:
+      //  make[1]: *** No rule to make target `test_run_dir/examples.AccumBlackBox_PeekPokeTest_Verilator345491158/AccumBlackBox.v', needed by `.../chisel-testers/test_run_dir/examples.AccumBlackBox_PeekPokeTest_Verilator345491158/VAccumBlackBoxWrapper.h'.  Stop.
+      //  or we end up including the same file multiple times.
+      writeTextToFile(files.map(_.getCanonicalPath).mkString("\n"), new File(targetDir, fileListName))
     }
   }
 
diff --git a/src/main/scala/firrtl/util/BackendCompilationUtilities.scala b/src/main/scala/firrtl/util/BackendCompilationUtilities.scala
index d3d34e87..bd5d2a9a 100644
--- a/src/main/scala/firrtl/util/BackendCompilationUtilities.scala
+++ b/src/main/scala/firrtl/util/BackendCompilationUtilities.scala
@@ -21,7 +21,10 @@ trait BackendCompilationUtilities {
     format.format(now)
   }
 
-  /** Copy the contents of a resource to a destination file.
+  /**
+    * Copy the contents of a resource to a destination file.
+    * @param name the name of the resource
+    * @param file the file to write it into
     */
   def copyResourceToFile(name: String, file: File) {
     val in = getClass.getResourceAsStream(name)
@@ -79,6 +82,12 @@ trait BackendCompilationUtilities {
     * all the files which are not included elsewhere. If multiple ones exist,
     * the compilation will fail.
     *
+    * If the file BlackBoxSourceHelper.fileListName exists in the output directory,
+    * it contains a list of source files to be included. Filter out any files in the vSources
+    * sequence that are in this file so we don't include the same file multiple times.
+    * This complication is an attempt to work-around the fact that clients used to have to
+    * explicitly include additional Verilog sources. Now, more of that is automatic.
+    *
     * @param dutFile name of the DUT .v without the .v extension
     * @param dir output directory
     * @param vSources list of additional Verilog sources to compile
@@ -94,8 +103,8 @@ trait BackendCompilationUtilities {
 
     val topModule = dutFile
 
+    val list_file = new File(dir, firrtl.transforms.BlackBoxSourceHelper.fileListName)
     val blackBoxVerilogList = {
-      val list_file = new File(dir, firrtl.transforms.BlackBoxSourceHelper.fileListName)
       if(list_file.exists()) {
         Seq("-f", list_file.getAbsolutePath)
       }
@@ -104,12 +113,24 @@ trait BackendCompilationUtilities {
       }
     }
 
+    // Don't include the same file multiple times.
+    // If it's in BlackBoxSourceHelper.fileListName, don't explicitly include it on the command line.
+    // 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
+      }
+      else {
+        Set.empty
+      }
+    }
+    val vSourcesFiltered = vSources.filterNot(f => blackBoxHelperFiles.contains(f.getCanonicalPath))
     val command = Seq(
       "verilator",
       "--cc", s"${dir.getAbsolutePath}/$dutFile.v"
     ) ++
       blackBoxVerilogList ++
-      vSources.flatMap(file => Seq("-v", file.getAbsolutePath)) ++
+      vSourcesFiltered.flatMap(file => Seq("-v", file.getCanonicalPath)) ++
       Seq("--assert",
         "-Wno-fatal",
         "-Wno-WIDTH",