From 89fa1c4bbd0adc2ba510c4ac2b611d2132248be4 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 25 Oct 2013 12:39:15 +0200 Subject: [PATCH 01/15] Don't re-set board preference when changing custom suboptions Since the custom suboptions are only visible when their associated board is the currently selected one, there is no point in re-setting the current board when a suboption is selected. --- app/src/processing/app/Base.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/app/src/processing/app/Base.java b/app/src/processing/app/Base.java index 21a765726ac..274027af593 100644 --- a/app/src/processing/app/Base.java +++ b/app/src/processing/app/Base.java @@ -1373,23 +1373,12 @@ public void actionPerformed(ActionEvent actionevent) { @SuppressWarnings("serial") Action subAction = new AbstractAction(_(boardCustomMenu.get(customMenuOption))) { public void actionPerformed(ActionEvent e) { - Preferences.set("target_package", (String) getValue("package")); - Preferences.set("target_platform", (String) getValue("platform")); - Preferences.set("board", (String) getValue("board")); Preferences.set("custom_" + menuId, getValue("board") + "_" + getValue("custom_menu_option")); - - filterVisibilityOfSubsequentBoardMenus((String) getValue("board"), currentIndex); - - onBoardOrPortChange(); Sketch.buildSettingChanged(); - rebuildImportMenu(Editor.importMenu); - rebuildExamplesMenu(Editor.examplesMenu); } }; subAction.putValue("board", boardId); subAction.putValue("custom_menu_option", customMenuOption); - subAction.putValue("package", packageName); - subAction.putValue("platform", platformName); if (!buttonGroupsMap.containsKey(menuId)) { buttonGroupsMap.put(menuId, new ButtonGroup()); From 46c930c841422841f95ad18a490a830d1e5e267c Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 25 Oct 2013 13:40:28 +0200 Subject: [PATCH 02/15] Pass TargetBoard objects around instead of strings Previously, strings containing the board id, or a joined version of the package, platform and board id were passed around. Since comparing objects is easier than strings and since parsing strings can be fragile, it's better to just pass the TargetBoard objects. There is one case where string parsing is still required: when parsing the --board commandline option. However, the parsing is now done in the right place, when handling the commandline, instead of in a generic selectBoard method. --- app/src/processing/app/Base.java | 49 ++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/app/src/processing/app/Base.java b/app/src/processing/app/Base.java index 274027af593..7407eb3a7a7 100644 --- a/app/src/processing/app/Base.java +++ b/app/src/processing/app/Base.java @@ -395,8 +395,20 @@ public Base(String[] args) throws Exception { Thread.sleep(10); // Do board selection if requested - if (selectBoard != null) - selectBoard(selectBoard); + if (selectBoard != null) { + String[] split = selectBoard.split(":"); + + TargetBoard targetBoard = getTargetPlatform(split[0], split[1]).getBoard(split[2]); + selectBoard(targetBoard); + + if (split.length > 3) { + String[] customsParts = split[3].split(","); + for (String customParts : customsParts) { + String[] keyValue = customParts.split("="); + Preferences.set("custom_" + keyValue[0].trim(), targetBoard.getId() + "_" + keyValue[1].trim()); + } + } + } if (doUpload) { // Build and upload @@ -1347,10 +1359,10 @@ private JRadioButtonMenuItem createBoardMenusAndCustomMenus( @SuppressWarnings("serial") Action action = new AbstractAction(board.getName()) { public void actionPerformed(ActionEvent actionevent) { - selectBoard((String) getValue("b")); + selectBoard((TargetBoard)getValue("b")); } }; - action.putValue("b", packageName + ":" + platformName + ":" + boardId); + action.putValue("b", board); JRadioButtonMenuItem item = new JRadioButtonMenuItem(action); @@ -1373,11 +1385,11 @@ public void actionPerformed(ActionEvent actionevent) { @SuppressWarnings("serial") Action subAction = new AbstractAction(_(boardCustomMenu.get(customMenuOption))) { public void actionPerformed(ActionEvent e) { - Preferences.set("custom_" + menuId, getValue("board") + "_" + getValue("custom_menu_option")); + Preferences.set("custom_" + menuId, ((TargetBoard)getValue("board")).getId() + "_" + getValue("custom_menu_option")); Sketch.buildSettingChanged(); } }; - subAction.putValue("board", boardId); + subAction.putValue("board", board); subAction.putValue("custom_menu_option", customMenuOption); if (!buttonGroupsMap.containsKey(menuId)) { @@ -1399,12 +1411,12 @@ public void actionPerformed(ActionEvent e) { return item; } - private static void filterVisibilityOfSubsequentBoardMenus(String boardID, int fromIndex) { + private static void filterVisibilityOfSubsequentBoardMenus(TargetBoard board, int fromIndex) { for (int i = fromIndex; i < Editor.boardsMenus.size(); i++) { JMenu menu = Editor.boardsMenus.get(i); for (int m = 0; m < menu.getItemCount(); m++) { JMenuItem menuItem = menu.getItem(m); - menuItem.setVisible(menuItem.getAction().getValue("board").equals(boardID)); + menuItem.setVisible(menuItem.getAction().getValue("board").equals(board)); } menu.setVisible(ifThereAreVisibleItemsOn(menu)); @@ -1477,22 +1489,15 @@ private static JMenuItem selectFirstEnabledMenuItem(JMenu menu) { } - private void selectBoard(String selectBoard) { - String[] split = selectBoard.split(":"); - Preferences.set("target_package", split[0]); - Preferences.set("target_platform", split[1]); - String boardId = split[2]; - Preferences.set("board", boardId); + private void selectBoard(TargetBoard targetBoard) { + TargetPlatform targetPlatform = targetBoard.getContainerPlatform(); + TargetPackage targetPackage = targetPlatform.getContainerPackage(); - if (split.length > 3) { - String[] customsParts = split[3].split(","); - for (String customParts : customsParts) { - String[] keyValue = customParts.split("="); - Preferences.set("custom_" + keyValue[0].trim(), boardId + "_" + keyValue[1].trim()); - } - } + Preferences.set("target_package", targetPackage.getId()); + Preferences.set("target_platform", targetPlatform.getId()); + Preferences.set("board", targetBoard.getId()); - filterVisibilityOfSubsequentBoardMenus(boardId, 1); + filterVisibilityOfSubsequentBoardMenus(targetBoard, 1); onBoardOrPortChange(); Sketch.buildSettingChanged(); From 0ea5509fcf52c71942ae1b67f7bdf5a45f7a64d2 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 25 Oct 2013 14:27:08 +0200 Subject: [PATCH 03/15] Handle errors in the --board parameter Previously, it would just raise nullpointer or index out of bounds exceptions when the --board paramater was wrong. --- app/src/processing/app/Base.java | 74 +++++++++++++++++++++++++------- 1 file changed, 59 insertions(+), 15 deletions(-) diff --git a/app/src/processing/app/Base.java b/app/src/processing/app/Base.java index 7407eb3a7a7..aa9de756f10 100644 --- a/app/src/processing/app/Base.java +++ b/app/src/processing/app/Base.java @@ -395,21 +395,8 @@ public Base(String[] args) throws Exception { Thread.sleep(10); // Do board selection if requested - if (selectBoard != null) { - String[] split = selectBoard.split(":"); - - TargetBoard targetBoard = getTargetPlatform(split[0], split[1]).getBoard(split[2]); - selectBoard(targetBoard); - - if (split.length > 3) { - String[] customsParts = split[3].split(","); - for (String customParts : customsParts) { - String[] keyValue = customParts.split("="); - Preferences.set("custom_" + keyValue[0].trim(), targetBoard.getId() + "_" + keyValue[1].trim()); - } - } - } - + processBoardArgument(selectBoard); + if (doUpload) { // Build and upload if (selectPort != null) @@ -444,6 +431,54 @@ public Base(String[] args) throws Exception { } } + protected void processBoardArgument(String selectBoard) { + // No board selected? Nothing to do + if (selectBoard == null) + return; + + String[] split = selectBoard.split(":", 4); + + if (split.length < 3) { + showError(null, I18n.format(_("{0}: Invalid board name, it should be of the form \"package:arch:board\" or \"package:arch:board:options\""), selectBoard), null); + } + + TargetPackage targetPackage = getTargetPackage(split[0]); + if (targetPackage == null) { + showError(null, I18n.format(_("{0}: Unknown package"), split[0]), null); + } + + TargetPlatform targetPlatform = targetPackage.get(split[1]); + if (targetPlatform == null) { + showError(null, I18n.format(_("{0}: Unknown architecture"), split[1]), null); + } + + TargetBoard targetBoard = targetPlatform.getBoard(split[2]); + if (targetBoard == null) { + showError(null, I18n.format(_("{0}: Unknown board"), split[2]), null); + } + + selectBoard(targetBoard); + + if (split.length > 3) { + String[] options = split[3].split(","); + for (String option : options) { + String[] keyValue = option.split("=", 2); + + if (keyValue.length != 2) + showError(null, I18n.format(_("{0}: Invalid option, should be of the form \"name=value\""), option, targetBoard.getId()), null); + String key = keyValue[0].trim(); + String value = keyValue[1].trim(); + + if (!targetBoard.hasMenu(key)) + showError(null, I18n.format(_("{0}: Invalid option for board \"{1}\""), key, targetBoard.getId()), null); + if (targetBoard.getMenuLabel(key, value) == null) + showError(null, I18n.format(_("{0}: Invalid option for \"{1}\" option for board \"{2}\""), value, key, targetBoard.getId()), null); + + Preferences.set("custom_" + key, targetBoard.getId() + "_" + value); + } + } + } + public Map> getBoardsViaNetwork() { return new HashMap>(boardsViaNetwork); } @@ -2002,6 +2037,15 @@ static public String getAvrBasePath() { return path; } + /** + * Returns a specific TargetPackage + * + * @param packageName + * @return + */ + static public TargetPackage getTargetPackage(String packageName) { + return packages.get(packageName); + } /** * Returns the currently selected TargetPlatform. From e32eafe48f151c05243389c52ccce17a369be342 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 25 Oct 2013 15:01:29 +0200 Subject: [PATCH 04/15] Handle sketch with only invalid code filenames Previously, this would error out with an index out of bounds exception. Now, an IOException is thrown, which is properly handled further up the call chain. --- app/src/processing/app/Sketch.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/src/processing/app/Sketch.java b/app/src/processing/app/Sketch.java index 3f38f80ca25..33f4111ef5c 100644 --- a/app/src/processing/app/Sketch.java +++ b/app/src/processing/app/Sketch.java @@ -151,7 +151,7 @@ public Sketch(Editor editor, String path) throws IOException { * Another exception is when an external editor is in use, * in which case the load happens each time "run" is hit. */ - protected void load() { + protected void load() throws IOException { codeFolder = new File(folder, "code"); dataFolder = new File(folder, "data"); @@ -193,6 +193,10 @@ protected void load() { } } } + + if (codeCount == 0) + throw new IOException(_("No valid code files found")); + // Remove any code that wasn't proper code = (SketchCode[]) PApplet.subset(code, 0, codeCount); @@ -1258,7 +1262,7 @@ public static void buildSettingChanged() { * When running from the editor, take care of preparations before running * the build. */ - public void prepare() { + public void prepare() throws IOException { // make sure the user didn't hide the sketch folder ensureExistence(); From 4ba80e37155932dd77eed906f160501299d3dad1 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 25 Oct 2013 15:21:04 +0200 Subject: [PATCH 05/15] Show an error on unknown options --- app/src/processing/app/Base.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/src/processing/app/Base.java b/app/src/processing/app/Base.java index aa9de756f10..d6bbb89d9db 100644 --- a/app/src/processing/app/Base.java +++ b/app/src/processing/app/Base.java @@ -357,6 +357,9 @@ public Base(String[] args) throws Exception { currentDirectory = args[i]; continue; } + if (args[i].startsWith("--")) + showError(null, I18n.format(_("unknown option: {0}"), args[i]), null); + String path = args[i]; // Fix a problem with systems that use a non-ASCII languages. Paths are // being passed in with 8.3 syntax, which makes the sketch loader code From 400ae7fdfdfafe62025f6106595b85453ea9fe78 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 25 Oct 2013 15:27:41 +0200 Subject: [PATCH 06/15] Slightly delay opening of files specified on the commandline Instead of opening up files during argument processing, the filenames are now stored in a list and opened only after all commandline arguments have been processed. This commit in itself shouldn't change any behaviour, but it prepares for improved error reporting in the next commits. --- app/src/processing/app/Base.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/src/processing/app/Base.java b/app/src/processing/app/Base.java index d6bbb89d9db..05b173b2a1c 100644 --- a/app/src/processing/app/Base.java +++ b/app/src/processing/app/Base.java @@ -325,6 +325,8 @@ public Base(String[] args) throws Exception { String selectBoard = null; String selectPort = null; String currentDirectory = System.getProperty("user.dir"); + List filenames = new LinkedList(); + // Check if any files were passed in on the command line for (int i = 0; i < args.length; i++) { if (args[i].equals("--upload")) { @@ -360,14 +362,17 @@ public Base(String[] args) throws Exception { if (args[i].startsWith("--")) showError(null, I18n.format(_("unknown option: {0}"), args[i]), null); - String path = args[i]; + filenames.add(args[i]); + } + + for (String path: filenames) { // Fix a problem with systems that use a non-ASCII languages. Paths are // being passed in with 8.3 syntax, which makes the sketch loader code // unhappy, since the sketch folder naming doesn't match up correctly. // http://dev.processing.org/bugs/show_bug.cgi?id=1089 if (isWindows()) { try { - File file = new File(args[i]); + File file = new File(path); path = file.getCanonicalPath(); } catch (IOException e) { e.printStackTrace(); From 0a3b82af8f5c605892b243c2dbcd3395981248b4 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 25 Oct 2013 15:38:28 +0200 Subject: [PATCH 07/15] Improve error handling for files on the commandline Previously, any files that were specified on the commandline but could not be opened were silently ignored. Only if --verify and --upload was specified and _all_ files failed to open, a generic error message was shown. Additionally, if multiple files were specified with --verify or --upload, only the first would be acted on (the others would be openened and shown in the GUI, but not actually verified or uploaded). Now, whenever a file fails to open, an error message is shown (fatal with --verify or --upload, non-fatal otherwise). Furthermore, with --verify or --upload an error is shown when there is not exactly one file on the commandline. Finally, instead of keeping an "opened" variable, the code now just checks the size of "editors" to see if a blank sketch should be opened. --- app/src/processing/app/Base.java | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/app/src/processing/app/Base.java b/app/src/processing/app/Base.java index 05b173b2a1c..92d54f0206a 100644 --- a/app/src/processing/app/Base.java +++ b/app/src/processing/app/Base.java @@ -318,7 +318,6 @@ public Base(String[] args) throws Exception { // Setup board-dependent variables. onBoardOrPortChange(); - boolean opened = false; boolean doUpload = false; boolean doVerify = false; boolean doVerbose = false; @@ -365,6 +364,9 @@ public Base(String[] args) throws Exception { filenames.add(args[i]); } + if ((doUpload || doVerify) && filenames.size() != 1) + showError(null, _("Must specify exactly one sketch file"), null); + for (String path: filenames) { // Fix a problem with systems that use a non-ASCII languages. Paths are // being passed in with 8.3 syntax, which makes the sketch loader code @@ -378,20 +380,22 @@ public Base(String[] args) throws Exception { e.printStackTrace(); } } + if (!new File(path).isAbsolute()) { path = new File(currentDirectory, path).getAbsolutePath(); } - if (handleOpen(path) != null) { - opened = true; + + if (handleOpen(path) == null) { + String mess = I18n.format(_("Failed to open sketch: \"{0}\""), path); + // Open failure is fatal in upload/verify mode + if (doUpload || doVerify) + showError(null, mess, null); + else + showWarning(null, mess, null); } } if (doUpload || doVerify) { - if (!opened) { - System.out.println(_("Can't open source sketch!")); - System.exit(2); - } - // Set verbosity for command line build Preferences.set("build.verbose", "" + doVerbose); Preferences.set("upload.verbose", "" + doVerbose); @@ -425,11 +429,10 @@ public Base(String[] args) throws Exception { } // Check if there were previously opened sketches to be restored - if (restoreSketches()) - opened = true; + restoreSketches(); // Create a new empty window (will be replaced with any files to be opened) - if (!opened) { + if (editors.isEmpty()) { handleNew(); } From f502c9b53cdab10a00500f067bfbe0f0e161ab19 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Sun, 27 Oct 2013 16:48:55 +0100 Subject: [PATCH 08/15] Error out when a required commandline argument is left out --- app/src/processing/app/Base.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/src/processing/app/Base.java b/app/src/processing/app/Base.java index 92d54f0206a..885322e5e68 100644 --- a/app/src/processing/app/Base.java +++ b/app/src/processing/app/Base.java @@ -344,18 +344,24 @@ public Base(String[] args) throws Exception { i++; if (i < args.length) selectBoard = args[i]; + else + showError(null, "Argument required for --board", null); continue; } if (args[i].equals("--port")) { i++; if (i < args.length) selectPort = args[i]; + else + showError(null, "Argument required for --port", null); continue; } if (args[i].equals("--curdir")) { i++; if (i < args.length) currentDirectory = args[i]; + else + showError(null, "Argument required for --curdir", null); continue; } if (args[i].startsWith("--")) From 57551b9e79eaf47b4f2f8adbfae106232deaf0fe Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Sun, 27 Oct 2013 17:03:59 +0100 Subject: [PATCH 09/15] Add --pref option This allows setting preferences through the commandline. --- app/src/processing/app/Base.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/app/src/processing/app/Base.java b/app/src/processing/app/Base.java index 885322e5e68..184eedc94f8 100644 --- a/app/src/processing/app/Base.java +++ b/app/src/processing/app/Base.java @@ -364,6 +364,14 @@ public Base(String[] args) throws Exception { showError(null, "Argument required for --curdir", null); continue; } + if (args[i].equals("--pref")) { + i++; + if (i < args.length) + processPrefArgument(args[i]); + else + showError(null, "Argument required for --pref", null); + continue; + } if (args[i].startsWith("--")) showError(null, I18n.format(_("unknown option: {0}"), args[i]), null); @@ -496,6 +504,14 @@ protected void processBoardArgument(String selectBoard) { } } + protected void processPrefArgument(String arg) { + String[] split = arg.split("=", 2); + if (split.length != 2 || split[0].isEmpty()) + showError(null, I18n.format(_("{0}: Invalid argument to --pref, should be of the form \"pref=value\""), arg), null); + + Preferences.set(split[0], split[1]); + } + public Map> getBoardsViaNetwork() { return new HashMap>(boardsViaNetwork); } From beac88e039be8b406db8011ec90f4192cbc3ca13 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 29 Oct 2013 14:40:49 +0100 Subject: [PATCH 10/15] Pass arguments to the Compiler constructor Previously, these arguments would be passed to the compile method. However, passing them to the constructor makes sure that the build preferences are created sooner, so they can be used by Sketch before calling the compile method. This commit shouldn't change any behaviour, but prepares for the next commits. --- app/src/processing/app/Sketch.java | 4 ++-- app/src/processing/app/debug/Compiler.java | 22 +++++++++++++--------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/app/src/processing/app/Sketch.java b/app/src/processing/app/Sketch.java index 33f4111ef5c..3376ee18a18 100644 --- a/app/src/processing/app/Sketch.java +++ b/app/src/processing/app/Sketch.java @@ -1542,8 +1542,8 @@ public String build(String buildPath, boolean verbose) throws RunnerException { // compile the program. errors will happen as a RunnerException // that will bubble up to whomever called build(). - Compiler compiler = new Compiler(); - if (compiler.compile(this, buildPath, primaryClassName, verbose)) { + Compiler compiler = new Compiler(this, buildPath, primaryClassName); + if (compiler.compile(verbose)) { size(compiler.getBuildPreferences()); return primaryClassName; } diff --git a/app/src/processing/app/debug/Compiler.java b/app/src/processing/app/debug/Compiler.java index 7cbba74c503..5bdb7d2e989 100644 --- a/app/src/processing/app/debug/Compiler.java +++ b/app/src/processing/app/debug/Compiler.java @@ -62,26 +62,30 @@ public class Compiler implements MessageConsumer { private String targetArch; private RunnerException exception; - + /** - * Compile sketch. - * + * Create a new Compiler * @param _sketch Sketch object to be compiled. * @param _buildPath Where the temporary files live and will be built from. * @param _primaryClassName the name of the combined sketch file w/ extension - * @return true if successful. - * @throws RunnerException Only if there's a problem. Only then. */ - public boolean compile(Sketch _sketch, String _buildPath, - String _primaryClassName, boolean _verbose) + public Compiler(Sketch _sketch, String _buildPath, String _primaryClassName) throws RunnerException { sketch = _sketch; + prefs = createBuildPreferences(_buildPath, _primaryClassName); + } + + /** + * Compile sketch. + * + * @return true if successful. + * @throws RunnerException Only if there's a problem. Only then. + */ + public boolean compile(boolean _verbose) throws RunnerException { verbose = _verbose || Preferences.getBoolean("build.verbose"); sketchIsCompiled = false; objectFiles = new ArrayList(); - prefs = createBuildPreferences(_buildPath, _primaryClassName); - // 0. include paths for core + all libraries sketch.setCompilingProgress(20); List includePaths = new ArrayList(); From 7b7f447a4adca2ea9edcd11a1036e57c9a59450f Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 29 Oct 2013 14:57:31 +0100 Subject: [PATCH 11/15] Move the definition of primaryClassName in Sketch Instead of defining in the preprocess method and returning, just define it in the build method. This makes sure the name is available before preprocessing, which is important for the upcoming commits. This commit should not change behaviour, only prepare for the next commits. --- app/src/processing/app/Sketch.java | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/app/src/processing/app/Sketch.java b/app/src/processing/app/Sketch.java index 3376ee18a18..95feb0cb5f6 100644 --- a/app/src/processing/app/Sketch.java +++ b/app/src/processing/app/Sketch.java @@ -1312,11 +1312,11 @@ public void prepare() throws IOException { * @param buildPath Location to copy all the .java files * @return null if compilation failed, main class name if not */ - public String preprocess(String buildPath) throws RunnerException { - return preprocess(buildPath, new PdePreprocessor()); + public void preprocess(String buildPath) throws RunnerException { + preprocess(buildPath, new PdePreprocessor()); } - public String preprocess(String buildPath, PdePreprocessor preprocessor) throws RunnerException { + public void preprocess(String buildPath, PdePreprocessor preprocessor) throws RunnerException { // make sure the user didn't hide the sketch folder ensureExistence(); @@ -1372,18 +1372,12 @@ public String preprocess(String buildPath, PdePreprocessor preprocessor) throws // 2. run preproc on that code using the sugg class name // to create a single .java file and write to buildpath - String primaryClassName = null; - try { // Output file File streamFile = new File(buildPath, name + ".cpp"); FileOutputStream outputStream = new FileOutputStream(streamFile); preprocessor.write(outputStream); outputStream.close(); - - // store this for the compiler and the runtime - primaryClassName = name + ".cpp"; - } catch (FileNotFoundException fnfe) { fnfe.printStackTrace(); String msg = _("Build folder disappeared or could not be written"); @@ -1432,7 +1426,6 @@ public String preprocess(String buildPath, PdePreprocessor preprocessor) throws sc.addPreprocOffset(headerOffset); } } - return primaryClassName; } @@ -1538,7 +1531,8 @@ public String build(boolean verbose) throws RunnerException { public String build(String buildPath, boolean verbose) throws RunnerException { // run the preprocessor editor.status.progressUpdate(20); - String primaryClassName = preprocess(buildPath); + String primaryClassName = name + ".cpp"; + preprocess(buildPath); // compile the program. errors will happen as a RunnerException // that will bubble up to whomever called build(). From 4592acc213d01bc7ff118bcd49ff137e72de0566 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 29 Oct 2013 17:50:58 +0100 Subject: [PATCH 12/15] Change the logic deciding when to do a full rebuild Previously, a full cleanup of the work directory (and thus a full rebuild) was done on the first build after: - startup, or - a change in the board or board suboption. This did not cooperate nicely with commandline compilation using --verify. Using the build.path option a persistent build path could be used, but the actual files in that path would never be reused. Now, each build saves the preferences used for building in a file "buildprefs.txt" inside the build directory. Subsequent builds will read this file to see if any build options changed and re-use the existing files if the build options are identical. Because the main .cpp file is not handled by Compiler::build, but by Sketch::preprocess, it is still always regenerated, even if the Sketch itself didn't change. This could be fixed later, though it is probably not a problem. When writing buildprefs.txt, only the build preferences starting with "build.", "compiler." or "recipes." are used. These should be enough to ensure files are always rebuilt when needed (probably also sometimes when not needed, when change build.verbose for example). Using all build preferences would cause the files to be rebuild too often, and because of last.ide.xxx.daterun, they would still rebuild on _every_ invocation... This approach is perhaps not ideal, but improving it would require putting more structure in the preferences instead of piling them all together into the build preferences. Because of this new mechanism, the old buildSettingsChanged()/deleteFilesOnNextBuild could be removed. --- app/src/processing/app/Base.java | 2 - app/src/processing/app/Editor.java | 2 - app/src/processing/app/Sketch.java | 88 +++++++++++++++++++++++------- 3 files changed, 69 insertions(+), 23 deletions(-) diff --git a/app/src/processing/app/Base.java b/app/src/processing/app/Base.java index 184eedc94f8..700d3b0563c 100644 --- a/app/src/processing/app/Base.java +++ b/app/src/processing/app/Base.java @@ -1454,7 +1454,6 @@ public void actionPerformed(ActionEvent actionevent) { Action subAction = new AbstractAction(_(boardCustomMenu.get(customMenuOption))) { public void actionPerformed(ActionEvent e) { Preferences.set("custom_" + menuId, ((TargetBoard)getValue("board")).getId() + "_" + getValue("custom_menu_option")); - Sketch.buildSettingChanged(); } }; subAction.putValue("board", board); @@ -1568,7 +1567,6 @@ private void selectBoard(TargetBoard targetBoard) { filterVisibilityOfSubsequentBoardMenus(targetBoard, 1); onBoardOrPortChange(); - Sketch.buildSettingChanged(); rebuildImportMenu(Editor.importMenu); rebuildExamplesMenu(Editor.examplesMenu); } diff --git a/app/src/processing/app/Editor.java b/app/src/processing/app/Editor.java index a6b6442eeea..278b548409e 100644 --- a/app/src/processing/app/Editor.java +++ b/app/src/processing/app/Editor.java @@ -2007,8 +2007,6 @@ public void internalCloseRunner() { try { stopHandler.run(); } catch (Exception e) { } - - sketch.cleanup(); } diff --git a/app/src/processing/app/Sketch.java b/app/src/processing/app/Sketch.java index 95feb0cb5f6..c8a2967ec0f 100644 --- a/app/src/processing/app/Sketch.java +++ b/app/src/processing/app/Sketch.java @@ -31,6 +31,7 @@ import processing.app.debug.Compiler; import processing.app.forms.PasswordAuthorizationDialog; import processing.app.helpers.PreferencesMap; +import processing.app.helpers.FileUtils; import processing.app.packages.Library; import processing.app.packages.LibraryList; import processing.app.preproc.*; @@ -96,6 +97,12 @@ public class Sketch { */ private LibraryList importedLibraries; + /** + * File inside the build directory that contains the build options + * used for the last build. + */ + static final String BUILD_PREFS_FILE = "buildprefs.txt"; + /** * path is location of the main .pde file, because this is also * simplest to use when opening the file from the finder/explorer. @@ -1182,12 +1189,12 @@ protected void setCurrentCode(String findName) { /** * Cleanup temporary files used during a build/run. */ - protected void cleanup() { + protected void cleanup(boolean force) { // if the java runtime is holding onto any files in the build dir, we // won't be able to delete them, so we need to force a gc here System.gc(); - if (deleteFilesOnNextBuild) { + if (force) { // delete the entire directory and all contents // when we know something changed and all objects // need to be recompiled, or if the board does not @@ -1199,8 +1206,6 @@ protected void cleanup() { // work because the build dir won't exist at startup, so the classloader // will ignore the fact that that dir is in the CLASSPATH in run.sh Base.removeDescendants(tempBuildFolder); - - deleteFilesOnNextBuild = false; } else { // delete only stale source files, from the previously // compiled sketch. This allows multiple windows to be @@ -1251,13 +1256,6 @@ protected void cleanup() { */ //protected String compile() throws RunnerException { - // called when any setting changes that requires all files to be recompiled - public static void buildSettingChanged() { - deleteFilesOnNextBuild = true; - } - - private static boolean deleteFilesOnNextBuild = true; - /** * When running from the editor, take care of preparations before running * the build. @@ -1286,12 +1284,6 @@ public void prepare() throws IOException { load(); } - // in case there were any boogers left behind - // do this here instead of after exiting, since the exit - // can happen so many different ways.. and this will be - // better connected to the dataFolder stuff below. - cleanup(); - // // handle preprocessing the main file's code // return build(tempBuildFolder.getAbsolutePath()); } @@ -1518,6 +1510,46 @@ public String build(boolean verbose) throws RunnerException { return build(tempBuildFolder.getAbsolutePath(), verbose); } + /** + * Check if the build preferences used on the previous build in + * buildPath match the ones given. + */ + protected boolean buildPreferencesChanged(File buildPrefsFile, String newBuildPrefs) { + // No previous build, so no match + if (!buildPrefsFile.exists()) + return true; + + String previousPrefs; + try { + previousPrefs = FileUtils.readFileToString(buildPrefsFile); + } catch (IOException e) { + System.err.println(_("Could not read prevous build preferences file, rebuilding all")); + return true; + } + + if (!previousPrefs.equals(newBuildPrefs)) { + System.out.println(_("Build options changed, rebuilding all")); + return true; + } else { + return false; + } + } + + /** + * Returns the build preferences of the given compiler as a string. + * Only includes build-specific preferences, to make sure unrelated + * preferences don't cause a rebuild (in particular preferences that + * change on every start, like last.ide.xxx.daterun). */ + protected String buildPrefsString(Compiler compiler) { + PreferencesMap buildPrefs = compiler.getBuildPreferences(); + String res = ""; + SortedSet treeSet = new TreeSet(buildPrefs.keySet()); + for (String k : treeSet) { + if (k.startsWith("build.") || k.startsWith("compiler.") || k.startsWith("recipes.")) + res += k + " = " + buildPrefs.get(k) + "\n"; + } + return res; + } /** * Preprocess and compile all the code for this sketch. @@ -1529,14 +1561,32 @@ public String build(boolean verbose) throws RunnerException { * @return null if compilation failed, main class name if not */ public String build(String buildPath, boolean verbose) throws RunnerException { + String primaryClassName = name + ".cpp"; + Compiler compiler = new Compiler(this, buildPath, primaryClassName); + File buildPrefsFile = new File(buildPath, BUILD_PREFS_FILE); + String newBuildPrefs = buildPrefsString(compiler); + + // Do a forced cleanup (throw everything away) if the previous + // build settings do not match the previous ones + boolean prefsChanged = buildPreferencesChanged(buildPrefsFile, newBuildPrefs); + cleanup(prefsChanged); + + if (prefsChanged) { + try { + PrintWriter out = new PrintWriter(buildPrefsFile); + out.print(newBuildPrefs); + out.close(); + } catch (IOException e) { + System.err.println(_("Could not write build preferences file")); + } + } + // run the preprocessor editor.status.progressUpdate(20); - String primaryClassName = name + ".cpp"; preprocess(buildPath); // compile the program. errors will happen as a RunnerException // that will bubble up to whomever called build(). - Compiler compiler = new Compiler(this, buildPath, primaryClassName); if (compiler.compile(verbose)) { size(compiler.getBuildPreferences()); return primaryClassName; From c6795dde73c873a9bf77b4425adf63270c15af52 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 29 Oct 2013 19:01:18 +0100 Subject: [PATCH 13/15] In EditorConsole::write(), use all arguments When System.(out|err).print was used before there was a visible EditorConsole, the message was written to the stderr/stdout by this instead of the EditorConsole. However, the write(data, offset, length) version would not pass on its offset and length parameters to the stdout/stderr stream, causing (parts of) a message to be printed multiple times. This commit makes sure the parameters are all properly passed to the real stream. For some reason the write(int) and write(byte[], int, int) methods in PrintStream do not throw an IOException like the write(byte[]) version, so the try block has to go. --- app/src/processing/app/EditorConsole.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/app/src/processing/app/EditorConsole.java b/app/src/processing/app/EditorConsole.java index 786fd8a16e2..bfd20e152ae 100644 --- a/app/src/processing/app/EditorConsole.java +++ b/app/src/processing/app/EditorConsole.java @@ -333,13 +333,11 @@ public void write(byte b[], int offset, int length) { if (currentConsole != null) { currentConsole.write(b, offset, length, err); } else { - try { - if (err) { - systemErr.write(b); - } else { - systemOut.write(b); - } - } catch (IOException e) { } // just ignore, where would we write? + if (err) { + systemErr.write(b, offset, length); + } else { + systemOut.write(b, offset, length); + } } OutputStream echo = err ? stderrFile : stdoutFile; From 9196a8d943cb1f2f66b466b1a7055dec444d3dd8 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 29 Oct 2013 19:10:43 +0100 Subject: [PATCH 14/15] Don't show the GUI on --verify or --upload These are intended to be ran from the commandline, so showing the GUI doesn't make so much sense. This is not quite the perfect solution yet, because an Editor object and all kinds of GUI objects are still created. This commit only prevents them from being visible, which is a nice first step, but not quite pretty yet. However, to do it properly, some code should be moved out of the Editor class, so that's a bit more work. Additionally, any messages shown with Base::showError and friends still create a popup, they probably shouldn't do this either. --- app/src/processing/app/Base.java | 15 ++++++--------- app/src/processing/app/EditorStatus.java | 6 ++++-- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/app/src/processing/app/Base.java b/app/src/processing/app/Base.java index 700d3b0563c..aa9c17b69b4 100644 --- a/app/src/processing/app/Base.java +++ b/app/src/processing/app/Base.java @@ -399,7 +399,7 @@ public Base(String[] args) throws Exception { path = new File(currentDirectory, path).getAbsolutePath(); } - if (handleOpen(path) == null) { + if (handleOpen(path, nextEditorLocation(), !(doUpload || doVerify)) == null) { String mess = I18n.format(_("Failed to open sketch: \"{0}\""), path); // Open failure is fatal in upload/verify mode if (doUpload || doVerify) @@ -416,10 +416,6 @@ public Base(String[] args) throws Exception { Editor editor = editors.get(0); - // Wait until editor is initialized - while (!editor.status.isInitialized()) - Thread.sleep(10); - // Do board selection if requested processBoardArgument(selectBoard); @@ -574,7 +570,7 @@ protected boolean restoreSketches() throws Exception { location = nextEditorLocation(); } // If file did not exist, null will be returned for the Editor - if (handleOpen(path, location) != null) { + if (handleOpen(path, location, true) != null) { opened++; } } @@ -888,11 +884,11 @@ public void handleOpenPrompt() throws Exception { * @throws Exception */ public Editor handleOpen(String path) throws Exception { - return handleOpen(path, nextEditorLocation()); + return handleOpen(path, nextEditorLocation(), true); } - protected Editor handleOpen(String path, int[] location) throws Exception { + protected Editor handleOpen(String path, int[] location, boolean showEditor) throws Exception { // System.err.println("entering handleOpen " + path); File file = new File(path); @@ -960,7 +956,8 @@ protected Editor handleOpen(String path, int[] location) throws Exception { // now that we're ready, show the window // (don't do earlier, cuz we might move it based on a window being closed) - editor.setVisible(true); + if (showEditor) + editor.setVisible(true); // System.err.println("exiting handleOpen"); diff --git a/app/src/processing/app/EditorStatus.java b/app/src/processing/app/EditorStatus.java index 5dd330b1a9b..e09b5b0f975 100644 --- a/app/src/processing/app/EditorStatus.java +++ b/app/src/processing/app/EditorStatus.java @@ -113,7 +113,8 @@ public void empty() { public void notice(String message) { mode = NOTICE; this.message = message; - copyErrorButton.setVisible(false); + if (copyErrorButton != null) + copyErrorButton.setVisible(false); //update(); repaint(); } @@ -126,7 +127,8 @@ public void unnotice(String unmessage) { public void error(String message) { mode = ERR; this.message = message; - copyErrorButton.setVisible(true); + if (copyErrorButton != null) + copyErrorButton.setVisible(true); repaint(); } From 0029e97b5a83fae997f5e33f6eedfc184884e6ad Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 28 Nov 2013 22:52:09 +0100 Subject: [PATCH 15/15] Return different exit codes with --verify or --upload Previous commits made all failures return 1, even though originally an unknown sketch file would return 2. This restores the previous behaviour and adds return code 3 to mean invalid options specified. The return codes are now: 0: Success 1: Build failed or upload failed 2: Sketch not found 3: Invalid commandline options --- app/src/processing/app/Base.java | 42 +++++++++++++++++++------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/app/src/processing/app/Base.java b/app/src/processing/app/Base.java index aa9c17b69b4..39592dff6f0 100644 --- a/app/src/processing/app/Base.java +++ b/app/src/processing/app/Base.java @@ -345,7 +345,7 @@ public Base(String[] args) throws Exception { if (i < args.length) selectBoard = args[i]; else - showError(null, "Argument required for --board", null); + showError(null, "Argument required for --board", 3); continue; } if (args[i].equals("--port")) { @@ -353,7 +353,7 @@ public Base(String[] args) throws Exception { if (i < args.length) selectPort = args[i]; else - showError(null, "Argument required for --port", null); + showError(null, "Argument required for --port", 3); continue; } if (args[i].equals("--curdir")) { @@ -361,7 +361,7 @@ public Base(String[] args) throws Exception { if (i < args.length) currentDirectory = args[i]; else - showError(null, "Argument required for --curdir", null); + showError(null, "Argument required for --curdir", 3); continue; } if (args[i].equals("--pref")) { @@ -369,17 +369,17 @@ public Base(String[] args) throws Exception { if (i < args.length) processPrefArgument(args[i]); else - showError(null, "Argument required for --pref", null); + showError(null, "Argument required for --pref", 3); continue; } if (args[i].startsWith("--")) - showError(null, I18n.format(_("unknown option: {0}"), args[i]), null); + showError(null, I18n.format(_("unknown option: {0}"), args[i]), 3); filenames.add(args[i]); } if ((doUpload || doVerify) && filenames.size() != 1) - showError(null, _("Must specify exactly one sketch file"), null); + showError(null, _("Must specify exactly one sketch file"), 3); for (String path: filenames) { // Fix a problem with systems that use a non-ASCII languages. Paths are @@ -403,7 +403,7 @@ public Base(String[] args) throws Exception { String mess = I18n.format(_("Failed to open sketch: \"{0}\""), path); // Open failure is fatal in upload/verify mode if (doUpload || doVerify) - showError(null, mess, null); + showError(null, mess, 2); else showWarning(null, mess, null); } @@ -460,22 +460,22 @@ protected void processBoardArgument(String selectBoard) { String[] split = selectBoard.split(":", 4); if (split.length < 3) { - showError(null, I18n.format(_("{0}: Invalid board name, it should be of the form \"package:arch:board\" or \"package:arch:board:options\""), selectBoard), null); + showError(null, I18n.format(_("{0}: Invalid board name, it should be of the form \"package:arch:board\" or \"package:arch:board:options\""), selectBoard), 3); } TargetPackage targetPackage = getTargetPackage(split[0]); if (targetPackage == null) { - showError(null, I18n.format(_("{0}: Unknown package"), split[0]), null); + showError(null, I18n.format(_("{0}: Unknown package"), split[0]), 3); } TargetPlatform targetPlatform = targetPackage.get(split[1]); if (targetPlatform == null) { - showError(null, I18n.format(_("{0}: Unknown architecture"), split[1]), null); + showError(null, I18n.format(_("{0}: Unknown architecture"), split[1]), 3); } TargetBoard targetBoard = targetPlatform.getBoard(split[2]); if (targetBoard == null) { - showError(null, I18n.format(_("{0}: Unknown board"), split[2]), null); + showError(null, I18n.format(_("{0}: Unknown board"), split[2]), 3); } selectBoard(targetBoard); @@ -486,14 +486,14 @@ protected void processBoardArgument(String selectBoard) { String[] keyValue = option.split("=", 2); if (keyValue.length != 2) - showError(null, I18n.format(_("{0}: Invalid option, should be of the form \"name=value\""), option, targetBoard.getId()), null); + showError(null, I18n.format(_("{0}: Invalid option, should be of the form \"name=value\""), option, targetBoard.getId()), 3); String key = keyValue[0].trim(); String value = keyValue[1].trim(); if (!targetBoard.hasMenu(key)) - showError(null, I18n.format(_("{0}: Invalid option for board \"{1}\""), key, targetBoard.getId()), null); + showError(null, I18n.format(_("{0}: Invalid option for board \"{1}\""), key, targetBoard.getId()), 3); if (targetBoard.getMenuLabel(key, value) == null) - showError(null, I18n.format(_("{0}: Invalid option for \"{1}\" option for board \"{2}\""), value, key, targetBoard.getId()), null); + showError(null, I18n.format(_("{0}: Invalid option for \"{1}\" option for board \"{2}\""), value, key, targetBoard.getId()), 3); Preferences.set("custom_" + key, targetBoard.getId() + "_" + value); } @@ -503,7 +503,7 @@ protected void processBoardArgument(String selectBoard) { protected void processPrefArgument(String arg) { String[] split = arg.split("=", 2); if (split.length != 2 || split[0].isEmpty()) - showError(null, I18n.format(_("{0}: Invalid argument to --pref, should be of the form \"pref=value\""), arg), null); + showError(null, I18n.format(_("{0}: Invalid argument to --pref, should be of the form \"pref=value\""), arg), 3); Preferences.set(split[0], split[1]); } @@ -2418,12 +2418,20 @@ static public void showWarning(String title, String message, Exception e) { } + static public void showError(String title, String message, Throwable e) { + showError(title, message, e, 1); + } + + static public void showError(String title, String message, int exit_code) { + showError(title, message, null, exit_code); + } + /** * Show an error message that's actually fatal to the program. * This is an error that can't be recovered. Use showWarning() * for errors that allow P5 to continue running. */ - static public void showError(String title, String message, Throwable e) { + static public void showError(String title, String message, Throwable e, int exit_code) { if (title == null) title = _("Error"); if (commandLine) { @@ -2434,7 +2442,7 @@ static public void showError(String title, String message, Throwable e) { JOptionPane.ERROR_MESSAGE); } if (e != null) e.printStackTrace(); - System.exit(1); + System.exit(exit_code); }