# HG changeset patch # User Artem Tikhomirov # Date 1323379167 -3600 # Node ID 150500515714cb8879cfec37e4cf7b7bdec612ad # Parent 1d9bcab9c50f9f97ff779cd2b33cd3c05df6c631 Report non-critical errors during status operation to handler/inspector diff -r 1d9bcab9c50f -r 150500515714 cmdline/org/tmatesoft/hg/console/Main.java --- a/cmdline/org/tmatesoft/hg/console/Main.java Thu Dec 08 15:34:13 2011 +0100 +++ b/cmdline/org/tmatesoft/hg/console/Main.java Thu Dec 08 22:19:27 2011 +0100 @@ -640,6 +640,11 @@ } } + public void invalid(Path fname, Exception ex) { + System.out.printf("FAILURE: %s\n", fname); + ex.printStackTrace(System.out); + } + private void print(char status, Path fname) { if (!hideStatusPrefix) { System.out.print(status); diff -r 1d9bcab9c50f -r 150500515714 cmdline/org/tmatesoft/hg/console/Status.java --- a/cmdline/org/tmatesoft/hg/console/Status.java Thu Dec 08 15:34:13 2011 +0100 +++ b/cmdline/org/tmatesoft/hg/console/Status.java Thu Dec 08 22:19:27 2011 +0100 @@ -32,6 +32,7 @@ import org.tmatesoft.hg.core.HgStatus; import org.tmatesoft.hg.core.HgStatus.Kind; import org.tmatesoft.hg.core.HgStatusCommand; +import org.tmatesoft.hg.core.HgStatusHandler; import org.tmatesoft.hg.util.Path; /** @@ -68,7 +69,7 @@ // cmd.subrepo(cmdLineOpts.getBoolean("-S", "--subrepos")) final boolean noStatusPrefix = cmdLineOpts.getBoolean("-n", "--no-status"); final boolean showCopies = cmdLineOpts.getBoolean("-C", "--copies"); - class StatusHandler implements HgStatusCommand.Handler { + class StatusHandler implements HgStatusHandler { final EnumMap> data = new EnumMap>(HgStatus.Kind.class); final Map copies = showCopies ? new HashMap() : null; @@ -85,6 +86,11 @@ } } + public void handleError(Path file, org.tmatesoft.hg.util.Status s) { + System.out.printf("FAILURE: %s %s\n", s.getMessage(), file); + s.getException().printStackTrace(System.out); + } + public void dump() { sortAndPrint('M', data.get(Kind.Modified), null); sortAndPrint('A', data.get(Kind.Added), copies); diff -r 1d9bcab9c50f -r 150500515714 src/org/tmatesoft/hg/core/HgStatusCommand.java --- a/src/org/tmatesoft/hg/core/HgStatusCommand.java Thu Dec 08 15:34:13 2011 +0100 +++ b/src/org/tmatesoft/hg/core/HgStatusCommand.java Thu Dec 08 22:19:27 2011 +0100 @@ -30,6 +30,7 @@ import org.tmatesoft.hg.repo.HgStatusInspector; import org.tmatesoft.hg.repo.HgWorkingCopyStatusCollector; import org.tmatesoft.hg.util.Path; +import org.tmatesoft.hg.util.Status; /** * Command to obtain file status information, 'hg status' counterpart. @@ -164,7 +165,7 @@ * @throws IllegalArgumentException if handler is null * @throws ConcurrentModificationException if this command already runs (i.e. being used from another thread) */ - public void execute(Handler statusHandler) throws CancellationException, HgException, IOException { + public void execute(HgStatusHandler statusHandler) throws CancellationException, HgException, IOException { if (statusHandler == null) { throw new IllegalArgumentException(); } @@ -189,12 +190,21 @@ sc.walk(startRevision, endRevision, mediator); } } + } catch (HgCallbackTargetException.Wrap ex) { + // seems too general to catch RuntimeException, i.e. + // unless catch is for very narrow piece of code, it's better not to catch any RTE (which may happen elsewhere, not only in handler) + // XXX Perhaps, need more detailed explanation in handlers that are expected to throw Wrap/RTE (i.e. HgChangesetHandler) + throw new HgCallbackTargetException(ex).setRevisionNumber(endRevision); } finally { mediator.done(); } } - public interface Handler { + /** + * @deprecated replaced with {@link HgStatusHandler} + */ + @Deprecated + public interface Handler extends HgStatusHandler{ void handleStatus(HgStatus s); } @@ -207,13 +217,13 @@ boolean needClean; boolean needIgnored; boolean needCopies; - Handler handler; + HgStatusHandler handler; private ChangelogHelper logHelper; Mediator() { } - public void start(Handler h, ChangelogHelper changelogHelper) { + public void start(HgStatusHandler h, ChangelogHelper changelogHelper) { handler = h; logHelper = changelogHelper; } @@ -268,5 +278,9 @@ handler.handleStatus(new HgStatus(Ignored, fname, logHelper)); } } + + public void invalid(Path fname, Exception ex) { + handler.handleError(fname, new Status(Status.Kind.ERROR, "Failed to get file status", ex)); + } } } diff -r 1d9bcab9c50f -r 150500515714 src/org/tmatesoft/hg/core/HgStatusHandler.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/src/org/tmatesoft/hg/core/HgStatusHandler.java Thu Dec 08 22:19:27 2011 +0100 @@ -0,0 +1,42 @@ +/* + * Copyright (c) 2011 TMate Software Ltd + * + * This program 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; version 2 of the License. + * + * 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. + * + * For information on how to redistribute this software under + * the terms of a license other than GNU General Public License + * contact TMate Software at support@hg4j.com + */ +package org.tmatesoft.hg.core; + +import org.tmatesoft.hg.util.Path; +import org.tmatesoft.hg.util.Status; + +/** + * Callback to process {@link HgStatus} objects. + * @author Artem Tikhomirov + * @author TMate Software Ltd. + */ +public interface HgStatusHandler { + + /* XXX #next() as in HgChangesetHandler? + * perhaps, handle() is better name? If yes, rename method in HgChangesetHandler, too, to make them similar. + * void next(HgStatus s); + * XXX describe RTE and HgCallbackTargetException + */ + void handleStatus(HgStatus s); + + /** + * Report non-critical error processing single file during status operation + * @param file name of the file that caused the trouble + * @param s error description object + */ + void handleError(Path file, Status s); +} diff -r 1d9bcab9c50f -r 150500515714 src/org/tmatesoft/hg/repo/HgChangelog.java --- a/src/org/tmatesoft/hg/repo/HgChangelog.java Thu Dec 08 15:34:13 2011 +0100 +++ b/src/org/tmatesoft/hg/repo/HgChangelog.java Thu Dec 08 22:19:27 2011 +0100 @@ -238,6 +238,7 @@ } } + // FIXME internal class DataAccess as part of API public static RawChangeset parse(DataAccess da) { try { byte[] data = da.byteArray(); diff -r 1d9bcab9c50f -r 150500515714 src/org/tmatesoft/hg/repo/HgStatusCollector.java --- a/src/org/tmatesoft/hg/repo/HgStatusCollector.java Thu Dec 08 15:34:13 2011 +0100 +++ b/src/org/tmatesoft/hg/repo/HgStatusCollector.java Thu Dec 08 22:19:27 2011 +0100 @@ -271,9 +271,9 @@ inspector.added(copyTarget); } } catch (HgException ex) { - ex.printStackTrace(); - // FIXME perhaps, shall record this exception to dedicated mediator and continue + // record exception to a mediator and continue, // for a single file not to be irresolvable obstacle for a status operation + inspector.invalid(r2fname, ex); } } } @@ -329,6 +329,7 @@ public static class Record implements HgStatusInspector { private List modified, added, removed, clean, missing, unknown, ignored; private Map copied; + private Map failures; private int startRev, endRev; private HgStatusCollector statusHelper; @@ -402,6 +403,13 @@ public List getIgnored() { return proper(ignored); } + + public Map getInvalid() { + if (failures == null) { + return Collections.emptyMap(); + } + return Collections.unmodifiableMap(failures); + } private static List proper(List l) { if (l == null) { @@ -448,6 +456,13 @@ public void ignored(Path fname) { ignored = doAdd(ignored, fname); } + + public void invalid(Path fname, Exception ex) { + if (failures == null) { + failures = new LinkedHashMap(); + } + failures.put(fname, ex); + } private static List doAdd(List l, Path p) { if (l == null) { diff -r 1d9bcab9c50f -r 150500515714 src/org/tmatesoft/hg/repo/HgStatusInspector.java --- a/src/org/tmatesoft/hg/repo/HgStatusInspector.java Thu Dec 08 15:34:13 2011 +0100 +++ b/src/org/tmatesoft/hg/repo/HgStatusInspector.java Thu Dec 08 22:19:27 2011 +0100 @@ -35,7 +35,18 @@ void copied(Path fnameOrigin, Path fnameAdded); void removed(Path fname); void clean(Path fname); - void missing(Path fname); // aka deleted (tracked by Hg, but not available in FS any more + /** + * Reports file tracked by Mercurial, but not available in file system any more, aka deleted. + */ + void missing(Path fname); // void unknown(Path fname); // not tracked void ignored(Path fname); + /** + * Reports a single file error during status collecting operation. It's up to client to treat the whole operation as successful or not. + * The error reported is otherwise not critical for the status operation. + * + * @param fname origin of the error + * @param ex describes an error occurred while accessing the file, never null + */ + void invalid(Path fname, Exception ex); } \ No newline at end of file diff -r 1d9bcab9c50f -r 150500515714 src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java --- a/src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java Thu Dec 08 15:34:13 2011 +0100 +++ b/src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java Thu Dec 08 22:19:27 2011 +0100 @@ -149,7 +149,7 @@ } try { if (getDirstateImpl() == null) { - // XXX this is a hack to avoid declaring throws for the #walk() at the moment + // FIXME this is a hack to avoid declaring throws for the #walk() at the moment // once I decide whether to have mediator that collects errors or to use exceptions here // this hack shall be removed in favor of either severe error in mediator or a re-thrown exception. getDirstate(); @@ -339,8 +339,8 @@ return; } } catch (HgException ex) { - ex.printStackTrace(); - // FIXME report to a mediator, continue status collection + // report failure and continue status collection + inspector.invalid(fname, ex); } } else if ((r = getDirstateImpl().checkAdded(fname)) != null) { if (r.copySource() != null && baseRevNames.contains(r.copySource())) { diff -r 1d9bcab9c50f -r 150500515714 src/org/tmatesoft/hg/util/Status.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/src/org/tmatesoft/hg/util/Status.java Thu Dec 08 22:19:27 2011 +0100 @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2011 TMate Software Ltd + * + * This program 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; version 2 of the License. + * + * 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. + * + * For information on how to redistribute this software under + * the terms of a license other than GNU General Public License + * contact TMate Software at support@hg4j.com + */ +package org.tmatesoft.hg.util; + +import org.tmatesoft.hg.internal.Experimental; + +/** + * Success/failure descriptor. When exception is too much. + * + * @author Artem Tikhomirov + * @author TMate Software Ltd. + */ +@Experimental(reason="Accidental use, does not justify dedicated class, perhaps.") +public class Status { + // XXX perhaps private enum and factory method createError() and createOk()? + public enum Kind { + OK, ERROR; + } + + private final Kind kind; + private final String message; + private final Exception error; + + public Status(Kind k, String msg) { + this(k, msg, null); + } + + public Status(Kind k, String msg, Exception err) { + kind = k; + message = msg; + error = err; + } + + public boolean isOk() { + return kind == Kind.OK; + } + + public Kind getKind() { + return kind; + } + + public String getMessage() { + return message; + } + + public Exception getException() { + return error; + } +} diff -r 1d9bcab9c50f -r 150500515714 test/org/tmatesoft/hg/test/TestStatus.java --- a/test/org/tmatesoft/hg/test/TestStatus.java Thu Dec 08 15:34:13 2011 +0100 +++ b/test/org/tmatesoft/hg/test/TestStatus.java Thu Dec 08 22:19:27 2011 +0100 @@ -26,6 +26,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -38,12 +39,14 @@ import org.tmatesoft.hg.core.HgStatus; import org.tmatesoft.hg.core.HgStatus.Kind; import org.tmatesoft.hg.core.HgStatusCommand; +import org.tmatesoft.hg.core.HgStatusHandler; import org.tmatesoft.hg.internal.PathGlobMatcher; import org.tmatesoft.hg.repo.HgLookup; import org.tmatesoft.hg.repo.HgRepository; import org.tmatesoft.hg.repo.HgStatusCollector; import org.tmatesoft.hg.repo.HgWorkingCopyStatusCollector; import org.tmatesoft.hg.util.Path; +import org.tmatesoft.hg.util.Status; /** @@ -173,9 +176,10 @@ // TODO check not -A, but defaults()/custom set of modifications } - private static class StatusCollector implements HgStatusCommand.Handler { + private static class StatusCollector implements HgStatusHandler { private final Map> kind2names = new TreeMap>(); private final Map> name2kinds = new TreeMap>(); + private final Map name2error = new LinkedHashMap(); public void handleStatus(HgStatus s) { List l = kind2names.get(s.getKind()); @@ -191,6 +195,10 @@ k.add(s.getKind()); } + public void handleError(Path file, Status s) { + name2error.put(file, s); + } + public List get(Kind k) { List rv = kind2names.get(k); return rv == null ? Collections.emptyList() : rv; @@ -200,6 +208,10 @@ List rv = name2kinds.get(p); return rv == null ? Collections.emptyList() : rv; } + + public Map getErrors() { + return name2error; + } } /* @@ -213,6 +225,7 @@ HgStatusCommand cmd = new HgStatusCommand(repo); StatusCollector sc = new StatusCollector(); cmd.all().base(7).execute(sc); + assertTrue(sc.getErrors().isEmpty()); Path file5 = Path.create("dir/file5"); // shall not be listed at all assertTrue(sc.get(file5).isEmpty()); @@ -230,6 +243,7 @@ HgStatusCommand cmd = new HgStatusCommand(repo); StatusCollector sc = new StatusCollector(); cmd.all().execute(sc); + assertTrue(sc.getErrors().isEmpty()); final Path file2 = Path.create("file2"); assertTrue(sc.get(file2).contains(Modified)); assertTrue(sc.get(file2).size() == 1); @@ -246,17 +260,20 @@ HgStatusCommand cmd = new HgStatusCommand(repo); StatusCollector sc = new StatusCollector(); cmd.all().execute(sc); + assertTrue(sc.getErrors().isEmpty()); Path file4 = Path.create("dir/file4"); assertTrue(sc.get(file4).contains(Removed)); assertTrue(sc.get(file4).size() == 1); // // different code path (collect != null) cmd.base(3).execute(sc = new StatusCollector()); + assertTrue(sc.getErrors().isEmpty()); assertTrue(sc.get(file4).contains(Removed)); assertTrue(sc.get(file4).size() == 1); // // wasn't there in rev 2, shall not be reported at all cmd.base(2).execute(sc = new StatusCollector()); + assertTrue(sc.getErrors().isEmpty()); assertTrue(sc.get(file4).isEmpty()); } @@ -273,20 +290,24 @@ HgStatusCommand cmd = new HgStatusCommand(repo); StatusCollector sc = new StatusCollector(); cmd.all().execute(sc); + assertTrue(sc.getErrors().isEmpty()); final Path file3 = Path.create("dir/file3"); assertTrue(sc.get(file3).contains(Ignored)); assertTrue(sc.get(file3).size() == 1); // cmd.base(3).execute(sc = new StatusCollector()); + assertTrue(sc.getErrors().isEmpty()); assertTrue(sc.get(file3).contains(Ignored)); assertTrue(sc.get(file3).contains(Removed)); assertTrue(sc.get(file3).size() == 2); // cmd.base(5).execute(sc = new StatusCollector()); + assertTrue(sc.getErrors().isEmpty()); assertTrue(sc.get(file3).contains(Ignored)); assertTrue(sc.get(file3).size() == 1); // cmd.base(0).execute(sc = new StatusCollector()); + assertTrue(sc.getErrors().isEmpty()); assertTrue(sc.get(file3).contains(Ignored)); assertTrue(sc.get(file3).size() == 1); @@ -304,6 +325,7 @@ StatusCollector sc = new StatusCollector(); cmd.base(1); cmd.all().execute(sc); + assertTrue(sc.getErrors().isEmpty()); final Path file1 = Path.create("file1"); assertTrue(sc.get(file1).contains(Unknown)); assertTrue(sc.get(file1).contains(Removed)); @@ -311,6 +333,7 @@ // // no file1 in rev 2, shall be reported as unknown only cmd.base(2).execute(sc = new StatusCollector()); + assertTrue(sc.getErrors().isEmpty()); assertTrue(sc.get(file1).contains(Unknown)); assertTrue(sc.get(file1).size() == 1); } @@ -322,6 +345,7 @@ StatusCollector sc = new StatusCollector(); cmd.match(new PathGlobMatcher("*")); cmd.all().execute(sc); + assertTrue(sc.getErrors().isEmpty()); /* * C .hgignore * ? file1 @@ -336,6 +360,7 @@ assertTrue(sc.get(Modified).size() == 1); // cmd.match(new PathGlobMatcher("dir/*")).execute(sc = new StatusCollector()); + assertTrue(sc.getErrors().isEmpty()); /* * I dir/file3 * R dir/file4 @@ -414,6 +439,7 @@ cmd.match(new PathGlobMatcher("dir/*")); StatusCollector sc = new StatusCollector(); cmd.execute(sc); + assertTrue(sc.getErrors().isEmpty()); final Path file3 = Path.create("dir/file3"); final Path file4 = Path.create("dir/file4"); final Path file5 = Path.create("dir/file5"); @@ -464,6 +490,7 @@ } private void report(String what, StatusCollector r) { + assertTrue(r.getErrors().isEmpty()); reportNotEqual(what + "#MODIFIED", r.get(Modified), statusParser.getModified()); reportNotEqual(what + "#ADDED", r.get(Added), statusParser.getAdded()); reportNotEqual(what + "#REMOVED", r.get(Removed), statusParser.getRemoved());