From d6db08f624e7a14bdef681d2eea5e1bcb6cec3ce Mon Sep 17 00:00:00 2001 From: Thomas Diewald Date: Mon, 12 Jun 2017 21:11:42 +0200 Subject: [PATCH 1/2] BugFix for Duplicate Library Conflicts. In case of multiple matching libraries (possible duplicates) only the required one is chosen, by making a guess based on the other, guaranteed, libraries in use. In a first pass, the guaranteed libraries are cached. In a second pass, unresolved package-library mappings are generated, based on pass 1. --- app/src/processing/app/Mode.java | 198 ++++++++++++++++++ java/src/processing/mode/java/JavaBuild.java | 42 +++- java/src/processing/mode/java/JavaEditor.java | 79 +++++-- .../mode/java/pdex/PreprocessingService.java | 46 ++-- 4 files changed, 324 insertions(+), 41 deletions(-) diff --git a/app/src/processing/app/Mode.java b/app/src/processing/app/Mode.java index c548844db1..ce33b0e36d 100644 --- a/app/src/processing/app/Mode.java +++ b/app/src/processing/app/Mode.java @@ -30,6 +30,7 @@ import java.io.*; import java.util.*; import java.util.List; +import java.util.Map.Entry; import javax.swing.*; import javax.swing.tree.*; @@ -395,6 +396,20 @@ public Library getCoreLibrary() { } + + + /** + * + * tries to return a library based on a given package name. + * In case of duplicates an exception is thrown. + * + * @param pkgName + * @return + * @throws SketchException in case of multiple library mappings (Duplicates) + * + * @deprecated use {@link #getLibraries(HashMap pkg_libs)} instead. + */ + @Deprecated public Library getLibrary(String pkgName) throws SketchException { List libraries = importToLibraryTable.get(pkgName); if (libraries == null) { @@ -418,6 +433,189 @@ public Library getLibrary(String pkgName) throws SketchException { return libraries.get(0); } } + + + + /** + * + * For each package declaration (= map keys) a matching library is searched for. + * and put into the map. + * + * In case of multiple matching libraries (possible duplicates) only the + * required one is chosen, by making a guess based on the other, guaranteed, + * libraries in use.
+ * + * This avoids the old "Duplicate Library" Conflict. + * + *

+ * the map can still contain null-values, in case no library was found at all. + * + * @param pkg_libs + */ + public void getLibraries(HashMap pkg_libs) { + + if(pkg_libs == null || pkg_libs.isEmpty()){ + return; + } + + + // stack, for imports that will we resolved in the second pass + List pkg_unresolved = new ArrayList(); + + // cache, for libraries that are picked with certainty + List lib_cache = new ArrayList(); + + // PASS 1 + // for each package declaration (= map keys) a matching library is searched for. + // in case of only one available library (standard case) its picked instantly + // and put into the lib_cache. + // In case of multiple candidates, the lib_cache is queried, if nothing is + // found so far, the package is put into pkg_unresolved (for the second pass) + +// System.out.println("Pass 1: find package-imports library mappings"); + + Iterator> iter = pkg_libs.entrySet().iterator(); + while (iter.hasNext()) { + + Map.Entry pkg_lib = iter.next(); + + String pgk_name = pkg_lib.getKey(); + + List lib_candidates = importToLibraryTable.get(pgk_name); + + // 0) no library found for this package, ... worst case + if(lib_candidates == null || lib_candidates.size() == 0) + { + System.err.printf("Error, no library found for package: \"%s\"\n", pgk_name); + pkg_lib.setValue(null); + } + + // 1) exactly one library found for this package, ... most of the time + else if(lib_candidates.size() == 1) + { + Library lib_picked = lib_candidates.get(0); + lib_cache.add(lib_picked); + pkg_lib.setValue(lib_picked); + } + + // 2) two or more libraries found for this package, ... can happen + else + { + + // warning msg + StringBuilder sb = new StringBuilder(); + sb.append(String.format("Warning, duplicate libraries found for package: \"%s\"\n", pgk_name)); + + // try to find a matching library that has been picked previously + Library lib_picked = null; + for (Library library : lib_candidates) { + + // if the cache contains one of these candidates, pick it + if(lib_picked == null && lib_cache.contains(library)){ + lib_picked = library; + } + + // candidate info + String lib_name = library.getName(); + String lib_path = library.getPath(); + String picked = (lib_picked == library) ? "[x]": "[ ]"; + sb.append(String.format(" %s %-20s %s\n", picked, lib_name, lib_path)); + } + + + if(lib_picked != null){ + // found a library + lib_cache.add(lib_picked); + pkg_lib.setValue(lib_picked); + // TODO: note sure what processings' best practice is with this + // informative warning. I'll just print it for the moment. + System.out.print(sb); + } else { + // still no match, but at least we have multiple candidates. + // --> resolve the conflict in the second pass + pkg_unresolved.add(pgk_name); + } + + } + + } + + + // PASS 2 + // +// System.out.println("Pass 2: fix unresolved mappings"); + + // resolve the other packages + for(String pgk_name : pkg_unresolved){ + + List lib_candidates = importToLibraryTable.get(pgk_name); + + // warning msg + StringBuilder sb = new StringBuilder(); + sb.append(String.format("Warning, duplicate libraries found for package: \"%s\"\n", pgk_name)); + + // try to find a matching library that has been picked previously + Library lib_picked = null; + for (Library library : lib_candidates) { + + // if the cache contains one of these candidates, pick it + if(lib_picked == null && lib_cache.contains(library)){ + lib_picked = library; + } + + // candidate info + String lib_name = library.getName(); + String lib_path = library.getPath(); + String picked = (lib_picked == library) ? "[x]": "[ ]"; + sb.append(String.format(" %s %-20s %s\n", picked, lib_name, lib_path)); + } + + + // still not found in cache, so this time we just take the first one + if(lib_picked == null){ + lib_picked = lib_candidates.get(0); + + // candidate info + String lib_name = lib_picked.getName(); + String lib_path = lib_picked.getPath(); + String picked = "[x]"; + sb.append(String.format(" %s %-20s %s\n", picked, lib_name, lib_path)); + } + + // found a library, finally + lib_cache.add(lib_picked); + pkg_libs.put(pgk_name, lib_picked); + // TODO: note sure what processings' best practice is with this + // informative warning. I'll just print it for the moment. + System.out.print(sb); + } + + + + // look what you have done +// iter = pkg_libs.entrySet().iterator(); +// int idx = 0; +// System.out.println("\nPackage/Library mapping"); +// while (iter.hasNext()) { +// Map.Entry pkg_lib = iter.next(); +// +// String pkg_name = pkg_lib.getKey(); +// Library pgk_lib = pkg_lib.getValue(); +// +// System.out.printf("[%2d] %-70s - %-25s - %s\n", idx++, pkg_name, pgk_lib.getName(), pgk_lib.getClassPath()); +// } + + + } + + + + + + + + + // . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . diff --git a/java/src/processing/mode/java/JavaBuild.java b/java/src/processing/mode/java/JavaBuild.java index 88f6fc3be0..1169f6bb90 100644 --- a/java/src/processing/mode/java/JavaBuild.java +++ b/java/src/processing/mode/java/JavaBuild.java @@ -27,7 +27,10 @@ import java.util.ArrayList; import java.util.Enumeration; import java.util.HashMap; +import java.util.Iterator; import java.util.List; +import java.util.Map; +import java.util.Map.Entry; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; import java.util.zip.ZipOutputStream; @@ -52,6 +55,7 @@ import processing.core.PConstants; import processing.data.StringList; import processing.data.XML; +import processing.mode.java.pdex.ImportStatement; import processing.mode.java.preproc.PdePreprocessor; import processing.mode.java.preproc.PreprocessorResult; import processing.mode.java.preproc.SurfaceInfo; @@ -389,7 +393,11 @@ public String preprocess(File srcFolder, javaLibraryPath += File.pathSeparator + core.getNativePath(); } -// System.out.println("extra imports: " + result.extraImports); + + + // init map by setting package imports as keys and libraries to null + HashMap pkg_libs = new HashMap(); + for (String item : result.extraImports) { // System.out.println("item = '" + item + "'"); // remove things up to the last dot @@ -397,7 +405,7 @@ public String preprocess(File srcFolder, // http://dev.processing.org/bugs/show_bug.cgi?id=1145 String entry = (dot == -1) ? item : item.substring(0, dot); // System.out.print(entry + " => "); - + if (item.startsWith("static ")) { // import static - https://github.com/processing/processing/issues/8 // Remove more stuff. @@ -405,11 +413,28 @@ public String preprocess(File srcFolder, entry = entry.substring(7, (dot2 == -1) ? entry.length() : dot2); // System.out.println(entry); } - -// System.out.println("library searching for " + entry); - Library library = mode.getLibrary(entry); -// System.out.println(" found " + library); - + + // add package import as key, library is null atm + pkg_libs.put(entry, null); + } + + + // for each package import, find a library that makes sense + // no more "duplicate library" conflicts" + mode.getLibraries(pkg_libs); + + + // checkout the generated mapping + // some imports are still mapped to null, which is the case if not + // a single library was found + Iterator> iter = pkg_libs.entrySet().iterator(); + while (iter.hasNext()) { + + Map.Entry pkg_lib = iter.next(); + + String entry = pkg_lib.getKey(); + Library library = pkg_lib.getValue(); + if (library != null) { if (!importedLibraries.contains(library)) { importedLibraries.add(library); @@ -436,7 +461,10 @@ public String preprocess(File srcFolder, System.err.println("No library found for " + entry); } } + } + + // PApplet.println(PApplet.split(libraryPath, File.pathSeparatorChar)); // Finally, add the regular Java CLASSPATH. This contains everything diff --git a/java/src/processing/mode/java/JavaEditor.java b/java/src/processing/mode/java/JavaEditor.java index 562efca8a6..7c0cd24da6 100644 --- a/java/src/processing/mode/java/JavaEditor.java +++ b/java/src/processing/mode/java/JavaEditor.java @@ -6,9 +6,11 @@ import java.io.*; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import javax.swing.*; import javax.swing.border.*; @@ -1817,39 +1819,74 @@ protected void downloadImports() { * @param importHeaders */ private List getNotInstalledAvailableLibs(ArrayList importHeadersList) { - Map importMap = - ContributionListing.getInstance().getLibrariesByImportHeader(); + + Map importMap = ContributionListing.getInstance().getLibrariesByImportHeader(); + List libList = new ArrayList<>(); + + + // init map by setting package imports as keys and libraries to null + + HashMap pkg_libs = new HashMap(); + HashMap pkg_contr = new HashMap(); + for (String importHeaders : importHeadersList) { int dot = importHeaders.lastIndexOf('.'); - String entry = (dot == -1) ? importHeaders : importHeaders.substring(0, - dot); + String entry = (dot == -1) ? importHeaders : importHeaders.substring(0, dot); if (entry.startsWith("java.") || entry.startsWith("javax.") || entry.startsWith("processing.")) { continue; } - - Library library = null; - try { - library = this.getMode().getLibrary(entry); - if (library == null) { - Contribution c = importMap.get(importHeaders); - if (c != null && c instanceof AvailableContribution) { - libList.add((AvailableContribution) c);// System.out.println(importHeaders - // + "not found"); - } - } - } catch (Exception e) { - // Not gonna happen (hopefully) - Contribution c = importMap.get(importHeaders); - if (c != null && c instanceof AvailableContribution) { - libList.add((AvailableContribution) c);// System.out.println(importHeaders - // + "not found"); + pkg_libs.put(entry, null); + pkg_contr.put(entry, importMap.get(entry)); + } + + // for each package import, find a library that makes sense + // no more "duplicate library" conflicts" + mode.getLibraries(pkg_libs); + + + // checkout the generated mapping + // some imports are still mapped to null, which is the case if not + // a single library was found. Exactly what we want here. + Iterator> iter = pkg_libs.entrySet().iterator(); + while (iter.hasNext()) { + + Map.Entry pkg_lib = iter.next(); + + String pkg = pkg_lib.getKey(); + Library lib = pkg_lib.getValue(); + + if(lib == null){ + Contribution contr = pkg_contr.get(pkg); + if (contr != null && contr instanceof AvailableContribution) { + libList.add((AvailableContribution) contr); } } } + + + +// for (String importHeaders : importHeadersList) { +// int dot = importHeaders.lastIndexOf('.'); +// String entry = (dot == -1) ? importHeaders : importHeaders.substring(0, dot); +// +// if (entry.startsWith("java.") || +// entry.startsWith("javax.") || +// entry.startsWith("processing.")) { +// continue; +// } +// +// if (pkg_libs.get(entry) == null) { +// Contribution c = importMap.get(importHeaders); +// if (c != null && c instanceof AvailableContribution) { +// libList.add((AvailableContribution) c); +// } +// } +// } + return libList; } diff --git a/java/src/processing/mode/java/pdex/PreprocessingService.java b/java/src/processing/mode/java/pdex/PreprocessingService.java index 5e9d3f0bf7..3553aaa9c8 100644 --- a/java/src/processing/mode/java/pdex/PreprocessingService.java +++ b/java/src/processing/mode/java/pdex/PreprocessingService.java @@ -31,9 +31,11 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.Map.Entry; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; import java.util.concurrent.CompletableFuture; @@ -575,19 +577,37 @@ static private List buildSketchLibraryClassPath(JavaMode mode, List programImports) { StringBuilder classPath = new StringBuilder(); - programImports.stream() - .map(ImportStatement::getPackageName) - .filter(pckg -> !ignorableImport(pckg)) - .map(pckg -> { - try { - return mode.getLibrary(pckg); - } catch (SketchException e) { - return null; - } - }) - .filter(lib -> lib != null) - .map(Library::getClassPath) - .forEach(cp -> classPath.append(File.pathSeparator).append(cp)); + + // init map by setting package imports as keys and libraries to null + HashMap pkg_libs = new HashMap(); + for(ImportStatement pgk : programImports){ + pkg_libs.put(pgk.getPackageName(), null); + } + + // for each package import, find a library that makes sense + // no more "duplicate library" conflicts" + mode.getLibraries(pkg_libs); + + // checkout the generated mapping + // some imports are still mapped to null, which is the case if not + // a single library was found + Iterator> iter = pkg_libs.entrySet().iterator(); + while (iter.hasNext()) { + Map.Entry pkg_lib = iter.next(); + + Library lib = pkg_lib.getValue(); + if(lib != null){ + classPath.append(File.pathSeparator).append(lib.getClassPath()); + } + } + +// programImports.stream() +// .map(ImportStatement::getPackageName) +// .filter(pckg -> !ignorableImport(pckg)) +// .map(pckg -> mode.getLibrary(pckg)) +// .filter(lib -> lib != null) +// .map(Library::getClassPath) +// .forEach(cp -> classPath.append(File.pathSeparator).append(cp)); return sanitizeClassPath(classPath.toString()); } From eb6bd560c2941926fbbe20a182f1e88cca1218bb Mon Sep 17 00:00:00 2001 From: Thomas Diewald Date: Mon, 19 Jun 2017 14:32:24 +0200 Subject: [PATCH 2/2] . --- .gitignore | 3 +++ core/methods/.classpath | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 528a5fbbbc..84c7fc50c5 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,6 @@ ._* *~ /build/shared/reference.zip +/bugfix_info/processing_bugfix_duplicateLibraryConflict_new.png +/bugfix_info/processing_bugfix_duplicateLibraryConflict_old.png +/build/ant_run.bat diff --git a/core/methods/.classpath b/core/methods/.classpath index d9132e9f49..c80e3eaa9f 100644 --- a/core/methods/.classpath +++ b/core/methods/.classpath @@ -2,6 +2,6 @@ - +