Index: src/uk/me/parabola/mkgmap/combiners/GmapsuppBuilder.java =================================================================== --- src/uk/me/parabola/mkgmap/combiners/GmapsuppBuilder.java (revision 4654) +++ src/uk/me/parabola/mkgmap/combiners/GmapsuppBuilder.java (working copy) @@ -60,8 +60,6 @@ * @author Steve Ratcliffe */ public class GmapsuppBuilder implements Combiner { - private static final Logger log = Logger.getLogger(GmapsuppBuilder.class); - private static final String GMAPSUPP = "gmapsupp.img"; private final Map files = new LinkedHashMap<>(); @@ -121,7 +119,7 @@ ImgChannel chan = imgFs.create(imgname); mdrBuilder.initForDevice(chan, sort, mdrConfig); } catch (FileExistsException e) { - System.err.println("Could not create duplicate MDR file"); + Logger.defaultLogger.error("Could not create duplicate MDR file"); } mdrBuilderMap.put(familyId, mdrBuilder); @@ -140,11 +138,9 @@ } } else { if (prevSort.getCodepage() != sort.getCodepage()) - System.err.printf("WARNING: input file '%s' has a different code page (%d rather than %d)\n", - info.getFilename(), sort.getCodepage(), prevSort.getCodepage()); + Logger.defaultLogger.warn("Input file '" + info.getFilename() + "' has a different code page (" + sort.getCodepage() + " rather than " + prevSort.getCodepage() + ")"); if (info.hasSortOrder() && prevSort.getSortOrderId() != sort.getSortOrderId()) - System.err.printf("WARNING: input file '%s' has a different sort order (%x rather than %x\n", - info.getFilename(), sort.getSortOrderId(), prevSort.getSortOrderId()); + Logger.defaultLogger.warn("Input file '" + info.getFilename() + "' has a different sort order (" + sort.getSortOrderId() + " rather than " + prevSort.getSortOrderId() + ")"); } } @@ -186,8 +182,7 @@ writeMpsFile(); } catch (FileNotWritableException e) { - log.warn("Could not create gmapsupp file"); - System.err.println("Could not create gmapsupp file"); + Logger.defaultLogger.error("Could not create gmapsupp file"); } finally { Utils.closeFile(imgFs); } @@ -215,7 +210,7 @@ // Do not close srtFile here } catch (FileExistsException e) { // well it shouldn't exist! - log.error("could not create SRT file as it exists already"); + Logger.defaultLogger.error("could not create SRT file as it exists already"); throw new FileNotWritableException("already existed", e); } } @@ -286,7 +281,7 @@ ((FileLink)chan).link(sf, sync); } catch (FileExistsException e) { - log.warn("Could not copy " + sf.getName(), e); + Logger.defaultLogger.warn("Could not copy " + sf.getName(), e); } } } @@ -299,7 +294,7 @@ ImgChannel chan = outfs.create(createImgFilename(filename)); ((FileLink) chan).link(info.subFiles().get(0), fc.file(chan)); } catch (FileExistsException e) { - log.warn("Counld not copy " + filename, e); + Logger.defaultLogger.warn("Could not copy " + filename, e); } } @@ -319,7 +314,7 @@ mpsFile.addProduct(b); mr.close(); } catch (IOException e) { - log.error("Could not read MPS file from gmapsupp", e); + Logger.defaultLogger.error("Could not read MPS file from gmapsupp", e); } } @@ -340,7 +335,7 @@ return new MpsFile(channel); } catch (FileExistsException e) { // well it shouldn't exist! - log.error("could not create MPS file as it already exists"); + Logger.defaultLogger.error("could not create MPS file as it already exists"); throw new FileNotWritableException("already existed", e); } } Index: src/uk/me/parabola/mkgmap/CommandArgsReader.java =================================================================== --- src/uk/me/parabola/mkgmap/CommandArgsReader.java (revision 4654) +++ src/uk/me/parabola/mkgmap/CommandArgsReader.java (working copy) @@ -17,7 +17,6 @@ package uk.me.parabola.mkgmap; import java.io.File; -import java.io.IOException; import java.util.ArrayList; import java.util.Formatter; import java.util.Iterator; @@ -47,6 +46,7 @@ private final ArgumentProcessor proc; private boolean mapnameWasSet; + private boolean hasFiles = false; private final ArgList arglist = new ArgList(); @@ -109,6 +109,7 @@ } else { log.debug("adding filename:", arg); + hasFiles = true; add(new Filename(arg)); } } @@ -125,6 +126,9 @@ proc.endOptions(new CommandArgs(this.args)); } + public boolean getHasFiles() { + return hasFiles; + } /** * Add an option based on the option and value separately. @@ -202,6 +206,7 @@ case "input-file": if (value != null){ log.debug("adding filename", value); + hasFiles = true; add(new Filename(value)); } break; Index: src/uk/me/parabola/mkgmap/main/Main.java =================================================================== --- src/uk/me/parabola/mkgmap/main/Main.java (revision 4654) +++ src/uk/me/parabola/mkgmap/main/Main.java (working copy) @@ -81,6 +81,7 @@ */ public class Main implements ArgumentProcessor { private static final Logger log = Logger.getLogger(Main.class); + private static final Date StartTime = new Date(); // Final .img file combiners. private final List combiners = new ArrayList<>(); @@ -103,6 +104,7 @@ private volatile int programRC = 0; private final Map combinerMap = new HashMap<>(); + private boolean informationDisplayed = false; /** * Used for unit tests @@ -126,7 +128,7 @@ */ private static int mainStart(String... args) { Instant start = Instant.now(); - System.out.println("Time started: " + new Date()); + // We need at least one argument. if (args.length < 1) { printUsage(); @@ -137,18 +139,19 @@ Main mm = new Main(); int numExitExceptions = 0; + CommandArgsReader commandArgs = new CommandArgsReader(mm); try { // Read the command line arguments and process each filename found. - CommandArgsReader commandArgs = new CommandArgsReader(mm); commandArgs.setValidOptions(getValidOptions(System.err)); commandArgs.readArgs(args); } catch (OutOfMemoryError e) { ++numExitExceptions; - System.err.println(e); + String message = "Out of memory.\r\n"; if (mm.maxJobs > 1) - System.err.println("Try using the mkgmap --max-jobs option with a value less than " + mm.maxJobs + " to reduce the memory requirement, or use the Java -Xmx option to increase the available heap memory."); + message += "Try using the mkgmap --max-jobs option with a value less than " + mm.maxJobs + " to reduce the memory requirement, or use the Java -Xmx option to increase the available heap memory."; else - System.err.println("Try using the Java -Xmx option to increase the available heap memory."); + message += "Try using the Java -Xmx option to increase the available heap memory."; + Logger.defaultLogger.error(message); } catch (MapFailedException | ExitException e) { // one of the combiners failed ++numExitExceptions; @@ -158,32 +161,33 @@ message += "\r\n" + cause.toString(); cause = cause.getCause(); } - System.err.println(message); + Logger.defaultLogger.error(message); } - - System.out.println("Number of ExitExceptions: " + numExitExceptions); - - System.out.println("Time finished: " + new Date()); - Duration duration = Duration.between(start, Instant.now()); - long seconds = duration.getSeconds(); - if (seconds > 0) { - long hours = seconds / 3600; - seconds -= hours * 3600; - long minutes = seconds / 60; - seconds -= minutes * 60; - System.out.println("Total time taken: " + - (hours > 0 ? hours + (hours > 1 ? " hours " : " hour ") : "") + - (minutes > 0 ? minutes + (minutes > 1 ? " minutes " : " minute ") : "") + - (seconds > 0 ? seconds + (seconds > 1 ? " seconds" : " second") : "")); + + if(commandArgs.getHasFiles()) { + Logger.defaultLogger.write("Number of ExitExceptions: " + numExitExceptions); + + Logger.defaultLogger.write("Time finished: " + new Date()); + Duration duration = Duration.between(start, Instant.now()); + long seconds = duration.getSeconds(); + if (seconds > 0) { + long hours = seconds / 3600; + seconds -= hours * 3600; + long minutes = seconds / 60; + seconds -= minutes * 60; + Logger.defaultLogger.write("Total time taken: " + + (hours > 0 ? hours + (hours > 1 ? " hours " : " hour ") : "") + + (minutes > 0 ? minutes + (minutes > 1 ? " minutes " : " minute ") : "") + + (seconds > 0 ? seconds + (seconds > 1 ? " seconds" : " second") : "")); + } + else + Logger.defaultLogger.write("Total time taken: " + duration.getNano() / 1000000 + " ms"); } - else - System.out.println("Total time taken: " + duration.getNano() / 1000000 + " ms"); - if (numExitExceptions > 0 || mm.getProgramRC() != 0){ - return 1; - } - return 0; + else if (numExitExceptions == 0 && !mm.informationDisplayed) + System.err.println("The command line does not appear to require mkgmap to do anything."); + return (numExitExceptions > 0 || mm.getProgramRC() != 0) ? 1 : 0; } - + private static void printUsage (){ System.err.println("Usage: mkgmap [options...] "); } @@ -318,6 +322,7 @@ break; case "help": + informationDisplayed = true; printHelp(System.out, getLang(), (!val.isEmpty()) ? val : "help"); break; case "style-file": @@ -331,9 +336,11 @@ verbose = true; break; case "list-styles": + informationDisplayed = true; listStyles(); break; case "check-styles": + informationDisplayed = true; checkStyles(); break; case "max-jobs": @@ -342,14 +349,16 @@ else { maxJobs = Integer.parseInt(val); if (maxJobs < 1) { - log.warn("max-jobs has to be at least 1"); + Logger.defaultLogger.warn("max-jobs has to be at least 1"); maxJobs = 1; } if (maxJobs > Runtime.getRuntime().availableProcessors()) - log.warn("It is recommended that max-jobs be no greater that the number of processor cores"); + Logger.defaultLogger.warn("It is recommended that max-jobs be no greater that the number of processor cores"); } break; case "version": + informationDisplayed = true; + System.err.println("Mkgmap version " + Version.VERSION); System.err.println(Version.VERSION); System.exit(0); } @@ -458,7 +467,7 @@ try { style = new StyleImpl(styleFile, name, new EnhancedProperties(), performChecks); } catch (SyntaxException e) { - System.err.println("Error in style: " + e.getMessage()); + Logger.defaultLogger.error("Error in style: " + e.getMessage()); } catch (FileNotFoundException e) { log.debug("could not find style", name); try { @@ -465,7 +474,7 @@ searchedStyleName = new File(styleFile).getName(); style = new StyleImpl(styleFile, null, new EnhancedProperties(), performChecks); } catch (SyntaxException e1) { - System.err.println("Error in style: " + e1.getMessage()); + Logger.defaultLogger.error("Error in style: " + e1.getMessage()); } catch (FileNotFoundException e1) { log.debug("could not find style", styleFile); } @@ -485,9 +494,13 @@ public void endOptions(CommandArgs args) { fileOptions(args); + int taskCount = futures.size(); + if (taskCount > 0) { + Logger.defaultLogger.write("Mkgmap version " + Version.VERSION); + Logger.defaultLogger.write("Time started: " + StartTime); + } log.info("Start tile processors"); int threadCount = maxJobs; - int taskCount = futures.size(); Runtime runtime = Runtime.getRuntime(); if (threadPool == null) { if (threadCount == 0) { @@ -511,7 +524,7 @@ } threadCount = Math.max(threadCount, 1); threadCount = Math.min(threadCount, runtime.availableProcessors()); - System.out.println("Setting max-jobs to " + threadCount); + Logger.defaultLogger.warn("Setting max-jobs to " + threadCount); } } @@ -566,14 +579,14 @@ throw new ExitException("Exiting - if you want to carry on regardless, use the --keep-going option"); } } catch (Exception e) { - e.printStackTrace(); + Logger.defaultLogger.error("Unexpected error", e); throw new ExitException("Exiting due to unexpected error"); } } } - System.out.println("Number of MapFailedExceptions: " + numMapFailedExceptions); + Logger.defaultLogger.write("Number of MapFailedExceptions: " + numMapFailedExceptions); if ((taskCount > threadCount + 1) && (maxJobs == 0) && (threadCount < runtime.availableProcessors())) { - System.out.println("To reduce the run time, consider increasing the amnount of memory available for use by mkgmap by using the Java -Xmx flag to set the memory to more than " + 100* (1 + ((runtime.maxMemory() * runtime.availableProcessors()) / (threadCount * 1024 * 1024 * 100))) + " MB, providing this is less than the amount of physical memory installed."); + Logger.defaultLogger.warn("To reduce the run time, consider increasing the amnount of memory available for use by mkgmap by using the Java -Xmx flag to set the memory to more than " + 100* (1 + ((runtime.maxMemory() * runtime.availableProcessors()) / (threadCount * 1024 * 1024 * 100))) + " MB, providing this is less than the amount of physical memory installed."); } if (combiners.isEmpty()) @@ -589,7 +602,7 @@ hasFiles = true; } if (!hasFiles){ - log.warn("nothing to do for combiners."); + log.info("nothing to do for combiners."); return; } log.info("Combining maps"); @@ -679,7 +692,7 @@ if (f.exists() && f.isFile()) { try { Files.delete(f.toPath()); - log.warn("removed " + f); + log.info("removed " + f); } catch (IOException e) { log.warn("removing " + f + "failed with " + e.getMessage()); } Index: src/uk/me/parabola/mkgmap/main/MapMaker.java =================================================================== --- src/uk/me/parabola/mkgmap/main/MapMaker.java (revision 4654) +++ src/uk/me/parabola/mkgmap/main/MapMaker.java (working copy) @@ -51,7 +51,7 @@ public String makeMap(CommandArgs args, String filename) { if (new File(filename).isDirectory()) { - System.err.println("Need a single file, not a directory: " + filename); + Logger.defaultLogger.error("Need a single file, not a directory: " + filename); return filename; } try { @@ -69,11 +69,10 @@ } return makeMap(args, src, ""); } catch (FormatException e) { - System.err.println("Bad file format: " + filename); - System.err.println(e.getMessage()); + Logger.defaultLogger.error("Bad file format: " + filename); return filename; } catch (FileNotFoundException e) { - System.err.println("Could not open file: " + filename); + Logger.defaultLogger.error("Could not open file: " + filename); return filename; } } @@ -120,14 +119,14 @@ map.close(); return outName; } catch (FileExistsException e) { - log.error("File exists already"); + Logger.defaultLogger.error(e.getMessage()); throw new MapFailedException("File exists already", e); } catch (FileNotWritableException e) { - log.error("Could not create or write to file"); + Logger.defaultLogger.error(e.getMessage()); throw new MapFailedException("Could not create or write to file", e); } catch (MapFailedException e) { - log.error(e.getMessage()); // make sure the filename is logged + Logger.defaultLogger.error(e.getMessage()); // make sure the filename is logged throw e; } } Index: test/func/ArgsTest.java =================================================================== --- test/func/ArgsTest.java (revision 4654) +++ test/func/ArgsTest.java (working copy) @@ -38,7 +38,7 @@ public class ArgsTest extends Base { @Test public void testHelp() { - Outputs outputs = TestUtils.run("--help"); + Outputs outputs = TestUtils.runAsProcess("--help"); outputs.checkOutput("--help=options", "--help=links"); outputs.checkNoError(); checkNoStdFile(); @@ -46,7 +46,7 @@ @Test public void testHelpOptions() { - Outputs outputs = TestUtils.run("--help=options"); + Outputs outputs = TestUtils.runAsProcess("--help=options"); outputs.checkNoError(); outputs.checkOutput("--mapname=name", "--latin1", "--list-styles"); checkNoStdFile(); @@ -54,7 +54,7 @@ @Test public void testHelpUnknown() { - Outputs outputs = TestUtils.run("--help=unknown-help-option"); + Outputs outputs = TestUtils.runAsProcess("--help=unknown-help-option"); outputs.checkNoError(); outputs.checkOutput("Could not find", "unknown-help-option"); checkNoStdFile(); @@ -62,7 +62,7 @@ @Test public void testListStyles() { - Outputs op = TestUtils.run("--style-file=test/resources/teststyles", "--list-styles"); + Outputs op = TestUtils.runAsProcess("--style-file=test/resources/teststyles", "--list-styles"); op.checkNoError(); op.checkOutput("empty", "main", "simple", "derived", "2.2: A simple test style"); checkNoStdFile(); @@ -70,7 +70,7 @@ @Test public void testListStylesVerbose() { - Outputs op = TestUtils.run("--style-file=test/resources/teststyles", + Outputs op = TestUtils.runAsProcess("--style-file=test/resources/teststyles", "--verbose", "--list-styles"); op.checkNoError(); op.checkOutput("empty", "main", "simple", "derived", @@ -83,9 +83,9 @@ TestUtils.registerFile("osmmap.img"); int pri = 42; - Outputs op = TestUtils.run("--draw-priority=" + pri, + Outputs op = TestUtils.runAsProcess("--draw-priority=" + pri, Args.TEST_RESOURCE_OSM + "uk-test-1.osm.gz"); - op.checkNoError(); + op.checkError("Number of ExitExceptions: 0"); FileSystem fs = openFs(Args.DEF_MAP_FILENAME); ImgChannel chan = fs.open(Args.DEF_MAP_ID + ".TRE", "r"); @@ -96,7 +96,7 @@ @Test public void testNoDescription() { - Outputs op = TestUtils.run("--description", Args.TEST_RESOURCE_OSM + "uk-test-1.osm.gz"); - op.checkNoError(); + Outputs op = TestUtils.runAsProcess("--description", Args.TEST_RESOURCE_OSM + "uk-test-1.osm.gz"); + op.checkError("Number of ExitExceptions: 0"); } } Index: test/func/files/GmapsuppTest.java =================================================================== --- test/func/files/GmapsuppTest.java (revision 4654) +++ test/func/files/GmapsuppTest.java (working copy) @@ -401,7 +401,7 @@ "--latin1", Args.TEST_RESOURCE_OSM + "uk-test-2.osm.gz"); - Outputs outputs = TestUtils.run(Args.TEST_STYLE_ARG, + Outputs outputs = TestUtils.runAsProcess(Args.TEST_STYLE_ARG, "--gmapsupp", "--index", Index: test/func/files/IndexTest.java =================================================================== --- test/func/files/IndexTest.java (revision 4654) +++ test/func/files/IndexTest.java (working copy) @@ -36,7 +36,7 @@ f.delete(); assertFalse("does not pre-exist", f.exists()); - Outputs outputs = TestUtils.run( + Outputs outputs = TestUtils.runAsProcess( Args.TEST_STYLE_ARG, "--index", "--latin1", @@ -45,7 +45,7 @@ Args.TEST_RESOURCE_IMG + "63240001.img", Args.TEST_RESOURCE_IMG + "63240002.img" ); - outputs.checkNoError(); + outputs.checkError("Number of ExitExceptions: 0"); TestUtils.registerFile(MDR_IMG); TestUtils.registerFile(OVERVIEW_NAME+".tdb"); Index: test/func/lib/TestUtils.java =================================================================== --- test/func/lib/TestUtils.java (revision 4654) +++ test/func/lib/TestUtils.java (working copy) @@ -16,12 +16,16 @@ */ package func.lib; +import java.io.BufferedReader; import java.io.ByteArrayOutputStream; import java.io.Closeable; import java.io.File; import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.InputStreamReader; import java.io.OutputStream; import java.io.PrintStream; +import java.lang.ProcessBuilder.Redirect; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Arrays; @@ -28,6 +32,7 @@ import java.util.Collections; import java.util.Deque; import java.util.List; +import java.util.concurrent.TimeUnit; import uk.me.parabola.imgfmt.Utils; import uk.me.parabola.mkgmap.general.LevelInfo; @@ -136,6 +141,52 @@ } /** + * Run with the given args as a new process. Some standard arguments are added first. + * + * @param in The arguments to use. + */ + public static Outputs runAsProcess(String ... in) { + List args = new ArrayList<>(Arrays.asList(in)); + args.add(0, Args.TEST_STYLE_ARG); + args.add(0, "dist\\mkgmap.jar"); + args.add(0, "-jar"); + args.add(0, "java.exe"); + + ProcessBuilder pb = new ProcessBuilder(args); + StringBuilder outBuilder = new StringBuilder(); + StringBuilder errBuilder = new StringBuilder(); + try { + Process process = pb.start(); + try (BufferedReader errorStream = new BufferedReader(new InputStreamReader(process.getErrorStream())); + BufferedReader outputStream = new BufferedReader(new InputStreamReader(process.getInputStream()))) { + while (true) { + getStreamOutput(outputStream, outBuilder); + getStreamOutput(errorStream, errBuilder); + if (process.waitFor(1, TimeUnit.SECONDS)) { + getStreamOutput(outputStream, outBuilder); + getStreamOutput(errorStream, errBuilder); + break; + } + } + } + } catch (IOException e) { + // do nothing + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + return new Outputs(outBuilder.toString(), errBuilder.toString()); + } + + private static void getStreamOutput(BufferedReader stream, StringBuilder stringBuilder) throws IOException { + char[] buff = new char[100000]; + while (stream.ready()) { + int count = stream.read(buff); + if (count > 0) + stringBuilder.append(buff, 0, count); + } + } + + /** * Create a rule set out of a string. The string is processed * as if it were in a file and the levels spec had been set. */