Skip to content

Commit

Permalink
Include library paths used by libraries in the compiler search path.
Browse files Browse the repository at this point in the history
  • Loading branch information
cmaglie committed Dec 9, 2013
1 parent 60c24ef commit 0502b94
Show file tree
Hide file tree
Showing 5 changed files with 229 additions and 34 deletions.
39 changes: 10 additions & 29 deletions app/src/processing/app/Base.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,11 @@
import processing.app.helpers.FileUtils;
import processing.app.helpers.PreferencesMap;
import processing.app.helpers.filefilters.OnlyDirs;
import processing.app.helpers.filefilters.OnlyFilesWithExtension;
import processing.app.javax.swing.filechooser.FileNameExtensionFilter;
import processing.app.packages.HeuristicResolver;
import processing.app.packages.Library;
import processing.app.packages.LibraryList;
import processing.app.packages.LibraryResolver;
import processing.app.tools.MenuScroller;
import processing.app.tools.ZipDeflater;
import processing.core.*;
Expand Down Expand Up @@ -105,7 +106,7 @@ public class Base {
static private LibraryList libraries;

// maps #included files to their library folder
static Map<String, Library> importToLibraryTable;
static private LibraryResolver libraryResolver;

// classpath for all known libraries for p5
// (both those in the p5/libs folder and those with lib subfolders
Expand Down Expand Up @@ -1326,20 +1327,9 @@ public void onBoardOrPortChange() {
String currentArch = Base.getTargetPlatform().getId();
libraries = libraries.filterByArchitecture(currentArch);

// Populate importToLibraryTable
importToLibraryTable = new HashMap<String, Library>();
for (Library lib : libraries) {
try {
String headers[] = headerListFromIncludePath(lib.getSrcFolder());
for (String header : headers) {
importToLibraryTable.put(header, lib);
}
} catch (IOException e) {
showWarning(_("Error"), I18n
.format("Unable to list header files in {0}", lib.getSrcFolder()), e);
}
}

// Create library resolver
libraryResolver = new HeuristicResolver(libraries, currentArch);

// Update editors status bar
for (Editor editor : editors)
editor.onBoardOrPortChange();
Expand Down Expand Up @@ -1736,19 +1726,6 @@ public void actionPerformed(ActionEvent event) {
}
}

/**
* Given a folder, return a list of the header files in that folder (but not
* the header files in its sub-folders, as those should be included from
* within the header files at the top-level).
*/
static public String[] headerListFromIncludePath(File path) throws IOException {
String[] list = path.list(new OnlyFilesWithExtension(".h"));
if (list == null) {
throw new IOException();
}
return list;
}

protected void loadHardware(File folder) {
if (!folder.isDirectory()) return;

Expand Down Expand Up @@ -2982,4 +2959,8 @@ public void handleAddLibrary() {
public static DiscoveryManager getDiscoveryManager() {
return discoveryManager;
}

public static LibraryResolver getLibraryResolver() {
return libraryResolver;
}
}
26 changes: 21 additions & 5 deletions app/src/processing/app/Sketch.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,16 @@
import processing.app.forms.PasswordAuthorizationDialog;
import processing.app.helpers.PreferencesMap;
import processing.app.helpers.FileUtils;
import processing.app.helpers.filefilters.OnlyFilesWithExtension;
import processing.app.packages.Library;
import processing.app.packages.LibraryList;
import processing.app.packages.LibraryResolver;
import processing.app.preproc.*;
import processing.core.*;
import static processing.app.I18n._;

import java.awt.*;
import java.io.*;
import java.util.*;
import java.util.List;

import javax.swing.*;

Expand Down Expand Up @@ -1116,7 +1116,7 @@ public void importLibrary(File jarPath) throws IOException {
// make sure the user didn't hide the sketch folder
ensureExistence();

String list[] = Base.headerListFromIncludePath(jarPath);
String list[] = jarPath.list(new OnlyFilesWithExtension(".h"));

// import statements into the main sketch file (code[0])
// if the current code is a .java file, insert into current
Expand Down Expand Up @@ -1389,13 +1389,29 @@ public void preprocess(String buildPath, PdePreprocessor preprocessor) throws Ru
// grab the imports from the code just preproc'd

importedLibraries = new LibraryList();
LibraryResolver resolver = Base.getLibraryResolver();
for (String item : preprocessor.getExtraImports()) {
Library lib = Base.importToLibraryTable.get(item);
Library lib = resolver.importToLibrary(item);
if (lib != null && !importedLibraries.contains(lib)) {
importedLibraries.add(lib);
}
}


// extend the import list with the library dependency tree
while (true) {
LibraryList dependencies = new LibraryList();
for (Library library : importedLibraries) {
for (Library dependency : library.getResolvedDependencies()) {

This comment has been minimized.

Copy link
@matthijskooijman

matthijskooijman Dec 9, 2013

It seems like this scans the dependencies of libraries in importedLibraries over and over again, which shouldn't be needed?

I think you can fix this by making the first forloop loop over "dependencies" instead of "importedLibraries, and initialize "dependencies" to the same contents as "importedLibraries" just above the while loop?

This comment has been minimized.

Copy link
@cmaglie

cmaglie Dec 9, 2013

Author Owner

Uhm... I'm a bit lost on your suggestion...

The loop could be read in pseudocode to something like this:

importedLibraries = libs imported from sketch
loop
   dependencies = importedLibraries.getDependencies()
   if dependencies is empy then exit loop
   add dependencies to importedLibraries

The external loop is needed to handle dependencies of dependencies.
Or probably is better if you show me how you would change it?

This comment has been minimized.

Copy link
@matthijskooijman

matthijskooijman Dec 9, 2013

I meant:

importedLibraries = libs imported from sketch
dependencies = importedLibraries
loop
   dependencies = dependencies.getDependencies()
   if dependencies is empy then exit loop
   add dependencies to importedLibraries

Which only does getDependencies on the next batch of dependencies, instead on every dependency already in importredLibraries (over and over again).

if (importedLibraries.contains(dependency))
continue;
dependencies.addOrReplace(dependency);
}
}
if (dependencies.size() == 0)
break;
importedLibraries.addAll(dependencies);
}

// 3. then loop over the code[] and save each .java file

for (SketchCode sc : code) {
Expand Down
150 changes: 150 additions & 0 deletions app/src/processing/app/packages/HeuristicResolver.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
/*
* This file is part of Arduino.
*
* Arduino is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*
* As a special exception, you may use this file as part of a free software
* library without restriction. Specifically, if other files instantiate
* templates or use macros or inline functions from this file, or you compile
* this file and link it with other files to produce an executable, this
* file does not by itself cause the resulting executable to be covered by
* the GNU General Public License. This exception does not however
* invalidate any other reasons why the executable file might be covered by
* the GNU General Public License.
*
* Copyright 2013 Arduino LLC (http://www.arduino.cc/)
*/

package processing.app.packages;

import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import processing.app.helpers.FileUtils;
import processing.app.helpers.filefilters.OnlyFilesWithExtension;
import processing.app.preproc.PdePreprocessor;

/**
* This resolver uses an heuristic approach to resolve dependencies between
* libraries without looking into libraries metadata.
*
* All libraries headers are inspected to search for #include lines, afterward
* import dependencies are searched in the same way we do for includes in
* sketches, i.e. looking for a library containing the requested headers.
*/
public class HeuristicResolver implements LibraryResolver {

private LibraryList libraries;
private Map<String, Library> importToLibrary;

public HeuristicResolver(LibraryList _libraries, String arch) {
libraries = _libraries;
importToLibrary = new HashMap<String, Library>();

// Populate importToLibrary table
for (Library library : libraries) {
File srcFolder = library.getSrcFolder();
for (String header : srcFolder.list(new OnlyFilesWithExtension(".h"))) {
importToLibrary.put(header, library);
}
}

// Resolve all libraries dependencies
for (Library library : libraries)
library.resolvedDependencies = resolve(library, arch);
}

/**
* Resolve dependencies for a library
*
* @param library
* @param arch
* @return A LibraryList containing the dependencies
*/
private LibraryList resolve(Library library, String arch) {

This comment has been minimized.

Copy link
@matthijskooijman

matthijskooijman Dec 9, 2013

The name of this method looks a bit short and general to me. Perhaps it should be renamed to "findLibraryDependencies" or something like that?

Same for resolveHeader below, perhaps that should be findHeaderDependencies?

This comment has been minimized.

Copy link
@cmaglie

cmaglie Dec 9, 2013

Author Owner

Yeah, it's still a bit unripe, I'll clean up the names...

List<File> headers = new ArrayList<File>();
for (File folder : library.getSrcFolders(arch)) {
List<File> files = FileUtils.listAllFilesWithExtension(folder, ".h",
".c", ".cpp");

This comment has been minimized.

Copy link
@matthijskooijman

matthijskooijman Dec 9, 2013

Won't this break for 1.0 libraries, which don't need recursive compiling? If they have a subdirectory with examples, for example, those would be included in this scan?

This comment has been minimized.

Copy link
@cmaglie

cmaglie Dec 9, 2013

Author Owner

And here you're absolutely right.
BTW the examples are not affected because they have .ino or .pde extension.

This comment has been minimized.

Copy link
@matthijskooijman

matthijskooijman Dec 9, 2013

Ah, right. The examples was just an example, the problem still exists even when no valid practical example can be found ;-)

headers.addAll(files);
}

LibraryList result = new LibraryList();
for (File header : headers)
result.addOrReplaceAll(resolveHeader(header, headers, library));
return result;
}

/**
* Inspect headerFile and search for dependencies
*
* @param headerFile
* @param exclusionList
* @param library
*/
private LibraryList resolveHeader(File headerFile, List<File> exclusionList,
Library library) {
LibraryList res = new LibraryList();

// Extract #includes from header file
List<String> imports;
try {
PdePreprocessor preprocessor = new PdePreprocessor();
String header = FileUtils.readFileToString(headerFile);
preprocessor.writePrefix(header);
imports = preprocessor.getExtraImports();
} catch (IOException e) {
e.printStackTrace();
return res;
}

// For every #include found...
for (String libImport : imports) {

// ...check if there is a matching library...
Library depLib = importToLibrary.get(libImport);
if (depLib == null || depLib == library)
continue;

// ...and check if the include is not in the exclusion list
boolean exclude = false;
for (File excluded : exclusionList) {
if (excluded.getName().equals(libImport))
exclude = true;
}
if (exclude)
continue;

This comment has been minimized.

Copy link
@matthijskooijman

matthijskooijman Dec 9, 2013

Why not put this continue inside the for loop? You consider that bad style, I presume? I consider the 3 extra lines needed now less readable ;-p

This comment has been minimized.

Copy link
@cmaglie

cmaglie Dec 9, 2013

Author Owner

This hit me too: the "continue" should restart the external loop (line 118), putting it inside the internal loop restarts the internal loop (line 127).

This comment has been minimized.

Copy link
@matthijskooijman

matthijskooijman Dec 9, 2013

Oh, of course! Missed that :-)


// add the dependency
res.addOrReplace(depLib);

System.out.println("Found dependency for " + library.getName());
System.out.println(" " + headerFile + " uses " + libImport + " -> " +
depLib.getName());
}

return res;
}

@Override
public Library importToLibrary(String h) {

This comment has been minimized.

Copy link
@matthijskooijman

matthijskooijman Dec 9, 2013

Perhaps this should be renamed to includeToLibrary since they're really includes, not imports like in java? Doesn't matter much, though.

This comment has been minimized.

Copy link
@cmaglie

cmaglie Dec 9, 2013

Author Owner

I prefer to keep this one like that to not lose the generality by changing the word "import" to "include" that is C/C++ specific. I know is a bit poor reason... :-)

This comment has been minimized.

Copy link
@matthijskooijman

matthijskooijman Dec 9, 2013

Well, it's a bit potato potato of course. Also, the LibraryResolver mechanism is potentially more generic than C++, so your argument makes sense :-)

return importToLibrary.get(h);
}

}
5 changes: 5 additions & 0 deletions app/src/processing/app/packages/Library.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public class Library {
private File folder, srcFolder, archFolder;
private List<String> architectures;
private boolean pre15Lib;
protected LibraryList resolvedDependencies;

private static final List<String> MANDATORY_PROPERTIES = Arrays
.asList(new String[] { "architectures", "author", "core-dependencies",
Expand Down Expand Up @@ -189,6 +190,10 @@ public List<String> getDependencies() {
return dependencies;
}

public LibraryList getResolvedDependencies() {
return resolvedDependencies;
}

public String getEmail() {
return email;
}
Expand Down
43 changes: 43 additions & 0 deletions app/src/processing/app/packages/LibraryResolver.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* This file is part of Arduino.
*
* Arduino is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*
* As a special exception, you may use this file as part of a free software
* library without restriction. Specifically, if other files instantiate
* templates or use macros or inline functions from this file, or you compile
* this file and link it with other files to produce an executable, this
* file does not by itself cause the resulting executable to be covered by
* the GNU General Public License. This exception does not however
* invalidate any other reasons why the executable file might be covered by
* the GNU General Public License.
*
* Copyright 2013 Arduino LLC (http://www.arduino.cc/)
*/

package processing.app.packages;

public interface LibraryResolver {

/**
* Returns the Library referenced by the include file name
*
* @param header
* The include file name, for example "SPI.h".
* @return The referenced library
*/
public abstract Library importToLibrary(String header);

}

0 comments on commit 0502b94

Please sign in to comment.