Merge pull request #2850 from PaulStoffregen/duplicate-library-detect

ffissore · ffissore · commit f76d1d567897 · 2015-04-01T09:31:10.000+02:00
Advise of duplicate libraries after compiling
diff --git a/arduino-core/src/cc/arduino/contributions/libraries/LibrariesIndexer.java b/arduino-core/src/cc/arduino/contributions/libraries/LibrariesIndexer.java
@@ -52,6 +52,7 @@ public class LibrariesIndexer {
 
   private LibrariesIndex index;
   private final LibraryList installedLibraries = new LibraryList();
+  private final LibraryList installedLibrariesWithDuplicates = new LibraryList();
   private List<File> librariesFolders;
   private final File indexFile;
   private final File stagingFolder;
@@ -92,15 +93,16 @@ public void setLibrariesFolders(List<File> _librariesFolders) {
   public void rescanLibraries() {
     // Clear all installed flags
     installedLibraries.clear();
+    installedLibrariesWithDuplicates.clear();
     for (ContributedLibrary lib : index.getLibraries())
       lib.setInstalled(false);
 
     // Rescan libraries
     for (File folder : librariesFolders)
-      scanInstalledLibraries(folder);
+      scanInstalledLibraries(folder, folder.equals(sketchbookLibrariesFolder));
   }
 
-  private void scanInstalledLibraries(File folder) {
+  private void scanInstalledLibraries(File folder, boolean isSketchbook) {
     File list[] = folder.listFiles(OnlyDirs.ONLY_DIRS);
     // if a bad folder or something like that, this might come back null
     if (list == null)
@@ -117,14 +119,14 @@ private void scanInstalledLibraries(File folder) {
       }
 
       try {
-        scanLibrary(subfolder);
+        scanLibrary(subfolder, isSketchbook);
       } catch (IOException e) {
         System.out.println(I18n.format(_("Invalid library found in {0}: {1}"), subfolder, e.getMessage()));
       }
     }
   }
 
-  private void scanLibrary(File folder) throws IOException {
+  private void scanLibrary(File folder, boolean isSketchbook) throws IOException {
     boolean readOnly = !FileUtils.isSubDirectory(sketchbookLibrariesFolder, folder);
 
     // A library is considered "legacy" if it doesn't contains
@@ -135,13 +137,23 @@ private void scanLibrary(File folder) throws IOException {
       LegacyUserLibrary lib = LegacyUserLibrary.create(folder);
       lib.setReadOnly(readOnly);
       installedLibraries.addOrReplace(lib);
+      if (isSketchbook) {
+        installedLibrariesWithDuplicates.add(lib);
+      } else {
+        installedLibrariesWithDuplicates.addOrReplace(lib);
+      }
       return;
     }
 
     // Create a regular library
     UserLibrary lib = UserLibrary.create(folder);
     lib.setReadOnly(readOnly);
     installedLibraries.addOrReplace(lib);
+    if (isSketchbook) {
+      installedLibrariesWithDuplicates.add(lib);
+    } else {
+      installedLibrariesWithDuplicates.addOrReplace(lib);
+    }
 
     // Check if we can find the same library in the index
     // and mark it as installed
@@ -170,6 +182,15 @@ public LibraryList getInstalledLibraries() {
     return installedLibraries;
   }
 
+  // Same as getInstalledLibraries(), but allow duplicates between
+  // builtin+package libraries and sketchbook installed libraries.
+  // However, do not report duplicates among builtin and packages, to
+  // allow any package to override builtin libraries without being
+  // reported as duplicates.
+  public LibraryList getInstalledLibrariesWithDuplicates() {
+    return installedLibrariesWithDuplicates;
+  }
+
   public File getStagingFolder() {
     return stagingFolder;
   }
diff --git a/arduino-core/src/processing/app/BaseNoGui.java b/arduino-core/src/processing/app/BaseNoGui.java
@@ -45,7 +45,7 @@ public class BaseNoGui {
   static private File toolsFolder;
 
   // maps #included files to their library folder
-  public static Map<String, UserLibrary> importToLibraryTable;
+  public static Map<String, LibraryList> importToLibraryTable;
 
   // maps library name to their library folder
   static private LibraryList libraries;
@@ -771,14 +771,24 @@ static private void createToolPreferences(ContributionsIndexer indexer) {
   }
 
   static public void populateImportToLibraryTable() {
-    // Populate importToLibraryTable
-    importToLibraryTable = new HashMap<String, UserLibrary>();
+    // Populate importToLibraryTable. Each header filename maps to
+    // a list of libraries. Compiler.java will use only the first
+    // library on each list. The others are used only to advise
+    // user of ambiguously matched and duplicate libraries.
+    importToLibraryTable = new HashMap<String, LibraryList>();
     for (UserLibrary lib : librariesIndexer.getInstalledLibraries()) {
       try {
         String headers[] = headerListFromIncludePath(lib.getSrcFolder());
         for (String header : headers) {
-          UserLibrary old = importToLibraryTable.get(header);
-          if (old != null) {
+          LibraryList list = importToLibraryTable.get(header);
+          if (list == null) {
+            // This is the first library found with this header
+            list = new LibraryList();
+            list.addFirst(lib);
+            importToLibraryTable.put(header, list);
+          } else {
+            UserLibrary old = list.peekFirst();
+            boolean useThisLib = true;
             // This is the case where 2 libraries have a .h header
             // with the same name.  We must decide which library to
             // use when a sketch has #include "name.h"
@@ -796,58 +806,81 @@ static public void populateImportToLibraryTable() {
             String oldName = old.getInstalledFolder().getName();  // just the library folder name
             String libName = lib.getInstalledFolder().getName();  // just the library folder name
             //System.out.println("name conflict: " + name);
-            //System.out.println("  old = " + oldName + "  ->  " + old.getFolder().getPath());
-            //System.out.println("  new = " + libName + "  ->  " + lib.getFolder().getPath());
+            //System.out.println(" old = " + oldName + " -> " + old.getInstalledFolder().getPath());
+            //System.out.println(" new = " + libName + " -> " + lib.getInstalledFolder().getPath());
             String name_lc = name.toLowerCase();
             String oldName_lc = oldName.toLowerCase();
             String libName_lc = libName.toLowerCase();
             // always favor a perfect name match
             if (libName.equals(name)) {
             } else if (oldName.equals(name)) {
-                continue;
+                useThisLib = false;
             // check for "-master" appended (zip file from github)
             } else if (libName.equals(name+"-master")) {
             } else if (oldName.equals(name+"-master")) {
-                continue;
+                useThisLib = false;
             // next, favor a match with other stuff appended
             } else if (libName.startsWith(name)) {
             } else if (oldName.startsWith(name)) {
-                continue;
+                useThisLib = false;
             // otherwise, favor a match with stuff prepended
             } else if (libName.endsWith(name)) {
             } else if (oldName.endsWith(name)) {
-                continue;
+                useThisLib = false;
             // as a last resort, match if stuff prepended and appended
             } else if (libName.contains(name)) {
             } else if (oldName.contains(name)) {
-                continue;
+                useThisLib = false;
             // repeat all the above tests, with case insensitive matching
             } else if (libName_lc.equals(name_lc)) {
             } else if (oldName_lc.equals(name_lc)) {
-                continue;
+                useThisLib = false;
             } else if (libName_lc.equals(name_lc+"-master")) {
             } else if (oldName_lc.equals(name_lc+"-master")) {
-                continue;
+                useThisLib = false;
             } else if (libName_lc.startsWith(name_lc)) {
             } else if (oldName_lc.startsWith(name_lc)) {
-                continue;
+                useThisLib = false;
             } else if (libName_lc.endsWith(name_lc)) {
             } else if (oldName_lc.endsWith(name_lc)) {
-                continue;
+                useThisLib = false;
             } else if (libName_lc.contains(name_lc)) {
             } else if (oldName_lc.contains(name_lc)) {
-                continue;
+                useThisLib = false;
             } else {
               // none of these tests matched, so just default to "libName".
             }
+            if (useThisLib) {
+              list.addFirst(lib);
+            } else {
+              list.addLast(lib);
+            }
           }
-          importToLibraryTable.put(header, lib);
         }
       } catch (IOException e) {
         showWarning(_("Error"), I18n
             .format("Unable to list header files in {0}", lib.getSrcFolder()), e);
       }
     }
+    // repeat for ALL libraries, to pick up duplicates not visible normally.
+    // any new libraries found here are NEVER used, but they are added to the
+    // end of already-found headers, to allow Compiler to report them if
+    // the sketch tries to use them.
+    for (UserLibrary lib : librariesIndexer.getInstalledLibrariesWithDuplicates()) {
+      try {
+        String headers[] = headerListFromIncludePath(lib.getSrcFolder());
+        for (String header : headers) {
+          LibraryList list = importToLibraryTable.get(header);
+          if (list != null) {
+            if (!(list.hasLibrary(lib))) {
+              list.addLast(lib);
+              //System.out.println(" duplicate lib: " + lib.getInstalledFolder().getPath());
+            }
+          }
+        }
+        } catch (IOException e) {
+      }
+    }
   }
 
   static public void initParameters(String args[]) {
diff --git a/arduino-core/src/processing/app/debug/Compiler.java b/arduino-core/src/processing/app/debug/Compiler.java
@@ -114,9 +114,16 @@ static public String build(SketchData data, String buildPath, File tempBuildFold
     
     // compile the program. errors will happen as a RunnerException
     // that will bubble up to whomever called build().
-    if (compiler.compile(verbose)) {
-      compiler.size(compiler.getBuildPreferences());
-      return primaryClassName;
+    try {
+      if (compiler.compile(verbose)) {
+        compiler.size(compiler.getBuildPreferences());
+        return primaryClassName;
+      }
+    } catch (RunnerException e) {
+      // when the compile fails, take this opportunity to show
+      // any helpful info possible before throwing the exception
+      compiler.adviseDuplicateLibraries();
+      throw e;
     }
     return null;
   }
@@ -428,10 +435,29 @@ public boolean compile(boolean _verbose) throws RunnerException, PreferencesMapE
 
     // Hook runs at End of Compilation
     runActions("hooks.postbuild", prefs);
+    adviseDuplicateLibraries();
 
     return true;
   }
 
+  private void adviseDuplicateLibraries() {
+    for (int i=0; i < importedDuplicateHeaders.size(); i++) {
+      System.out.println(I18n.format(_("Multiple libraries were found for \"{0}\""),
+        importedDuplicateHeaders.get(i)));
+      boolean first = true;
+      for (UserLibrary lib : importedDuplicateLibraries.get(i)) {
+        if (first) {
+          System.out.println(I18n.format(_(" Used: {0}"),
+            lib.getInstalledFolder().getPath()));
+          first = false;
+        } else {
+          System.out.println(I18n.format(_(" Not used: {0}"),
+            lib.getInstalledFolder().getPath()));
+        }
+      }
+    }
+  }
+
   private PreferencesMap createBuildPreferences(String _buildPath,
                                                 String _primaryClassName)
       throws RunnerException {
@@ -1166,10 +1192,19 @@ public void preprocess(String buildPath, PdePreprocessor preprocessor) throws Ru
     // grab the imports from the code just preproc'd
 
     importedLibraries = new LibraryList();
+    importedDuplicateHeaders = new ArrayList<String>();
+    importedDuplicateLibraries = new ArrayList<LibraryList>();
     for (String item : preprocessor.getExtraImports()) {
-      UserLibrary lib = BaseNoGui.importToLibraryTable.get(item);
-      if (lib != null && !importedLibraries.contains(lib)) {
-        importedLibraries.add(lib);
+      LibraryList list = BaseNoGui.importToLibraryTable.get(item);
+      if (list != null) {
+        UserLibrary lib = list.peekFirst();
+        if (lib != null && !importedLibraries.contains(lib)) {
+          importedLibraries.add(lib);
+          if (list.size() > 1) {
+            importedDuplicateHeaders.add(item);
+            importedDuplicateLibraries.add(list);
+          }
+        }
       }
     }
 
@@ -1201,6 +1236,8 @@ public void preprocess(String buildPath, PdePreprocessor preprocessor) throws Ru
    * List of library folders.
    */
   private LibraryList importedLibraries;
+  private List<String>      importedDuplicateHeaders;
+  private List<LibraryList> importedDuplicateLibraries;
 
   /**
    * Map an error from a set of processed .java files back to its location
diff --git a/arduino-core/src/processing/app/packages/LibraryList.java b/arduino-core/src/processing/app/packages/LibraryList.java
@@ -81,4 +81,11 @@ public LibraryList filterLibrariesInSubfolder(File subFolder) {
     }
     return res;
   }
+
+  public boolean hasLibrary(UserLibrary lib) {
+    for (UserLibrary l : this)
+      if (l == lib) return true;
+    return false;
+  }
 }
+

Original file line number	Diff line number	Diff line change
`@@ -81,4 +81,11 @@ public LibraryList filterLibrariesInSubfolder(File subFolder) {`
`81`	`81`	`}`
`82`	`82`	`return res;`
`83`	`83`	`}`
	`84`	`+`
	`85`	`+ public boolean hasLibrary(UserLibrary lib) {`
	`86`	`+ for (UserLibrary l : this)`
	`87`	`+ if (l == lib) return true;`
	`88`	`+ return false;`
	`89`	`+ }`
`84`	`90`	`}`
	`91`	`+`