From cfebad058c7786e3d584669868ee74c2d7118e1f Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Thu, 12 Mar 2009 19:07:42 -0700 Subject: [PATCH] Avoid incorrect output of UNINTERESTING commits when clock skew occurs The prior commit added functionality to scan a few extra commits when we otherwise would have aborted due to everything left being colored UNINTERESTING. When that happens we may wind up coloring a commit that we already produced to the caller, giving the caller an incorrect result set. If we insert a fully buffered generator in the pipeline, such as that used for RevSort.TOPO or RevSort.REVERSE, we can easily filter these late-colored commits out before we show them to the application. But otherwise we delay the output by 6 commits, just long enough to give PendingGenerator a reasonable chance at getting the coloring right. This is only an approximation. It is still possible for commits to produce when they should be UNINTERESTING, such as if more than the OVER_SCAN limit had clock skew between two branches and the common merge base, even if we are fully buffering our output. Signed-off-by: Shawn O. Pearce Signed-off-by: Robin Rosenberg --- .../org/spearce/jgit/revwalk/DelayRevQueue.java | 92 ++++++++++++++++++++++ .../jgit/revwalk/FixUninterestingGenerator.java | 77 ++++++++++++++++++ .../org/spearce/jgit/revwalk/PendingGenerator.java | 2 +- .../org/spearce/jgit/revwalk/StartGenerator.java | 23 +++++- 4 files changed, 189 insertions(+), 5 deletions(-) create mode 100644 org.spearce.jgit/src/org/spearce/jgit/revwalk/DelayRevQueue.java create mode 100644 org.spearce.jgit/src/org/spearce/jgit/revwalk/FixUninterestingGenerator.java diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/DelayRevQueue.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/DelayRevQueue.java new file mode 100644 index 00000000..1eb7064e --- /dev/null +++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/DelayRevQueue.java @@ -0,0 +1,92 @@ +/* + * Copyright (C) 2009, Google Inc. + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Git Development Community nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.spearce.jgit.revwalk; + +import java.io.IOException; + +import org.spearce.jgit.errors.IncorrectObjectTypeException; +import org.spearce.jgit.errors.MissingObjectException; + +/** + * Delays commits to be at least {@link PendingGenerator#OVER_SCAN} late. + *

+ * This helps to "fix up" weird corner cases resulting from clock skew, by + * slowing down what we produce to the caller we get a better chance to ensure + * PendingGenerator reached back far enough in the graph to correctly mark + * commits {@link RevWalk#UNINTERESTING} if necessary. + *

+ * This generator should appear before {@link FixUninterestingGenerator} if the + * lower level {@link #pending} isn't already fully buffered. + */ +final class DelayRevQueue extends Generator { + private static final int OVER_SCAN = PendingGenerator.OVER_SCAN; + + private final Generator pending; + + private final FIFORevQueue delay; + + private int size; + + DelayRevQueue(final Generator g) { + pending = g; + delay = new FIFORevQueue(); + } + + @Override + int outputType() { + return pending.outputType(); + } + + @Override + RevCommit next() throws MissingObjectException, + IncorrectObjectTypeException, IOException { + while (size < OVER_SCAN) { + final RevCommit c = pending.next(); + if (c == null) + break; + delay.add(c); + size++; + } + + final RevCommit c = delay.next(); + if (c == null) + return null; + size--; + return c; + } +} diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/FixUninterestingGenerator.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/FixUninterestingGenerator.java new file mode 100644 index 00000000..6a9ba613 --- /dev/null +++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/FixUninterestingGenerator.java @@ -0,0 +1,77 @@ +/* + * Copyright (C) 2009, Google Inc. + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Git Development Community nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.spearce.jgit.revwalk; + +import java.io.IOException; + +import org.spearce.jgit.errors.IncorrectObjectTypeException; +import org.spearce.jgit.errors.MissingObjectException; + +/** + * Filters out commits marked {@link RevWalk#UNINTERESTING}. + *

+ * This generator is only in front of another generator that has fully buffered + * commits, such that we are called only after the {@link PendingGenerator} has + * exhausted its input queue and given up. It skips over any uninteresting + * commits that may have leaked out of the PendingGenerator due to clock skew + * being detected in the commit objects. + */ +final class FixUninterestingGenerator extends Generator { + private final Generator pending; + + FixUninterestingGenerator(final Generator g) { + pending = g; + } + + @Override + int outputType() { + return pending.outputType(); + } + + @Override + RevCommit next() throws MissingObjectException, + IncorrectObjectTypeException, IOException { + for (;;) { + final RevCommit c = pending.next(); + if (c == null) + return null; + if ((c.flags & RevWalk.UNINTERESTING) == 0) + return c; + } + } +} diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/PendingGenerator.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/PendingGenerator.java index 4e244314..8e3d6bc9 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/PendingGenerator.java +++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/PendingGenerator.java @@ -69,7 +69,7 @@ class PendingGenerator extends Generator { * constant to 1 additional commit due to the use of a pre-increment * operator when accessing the value. */ - private static final int OVER_SCAN = 5 + 1; + static final int OVER_SCAN = 5 + 1; /** A commit near the end of time, to initialize {@link #last} with. */ private static final RevCommit INIT_LAST; diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/StartGenerator.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/StartGenerator.java index bf7067a9..3535250d 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/StartGenerator.java +++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/StartGenerator.java @@ -90,6 +90,7 @@ class StartGenerator extends Generator { return mbg.next(); } + final boolean uninteresting = q.anybodyHasFlag(RevWalk.UNINTERESTING); boolean boundary = walker.hasRevSort(RevSort.BOUNDARY); if (!boundary && walker instanceof ObjectWalk) { @@ -99,7 +100,7 @@ class StartGenerator extends Generator { // boundary = true; } - if (boundary && !q.anybodyHasFlag(RevWalk.UNINTERESTING)) { + if (boundary && !uninteresting) { // If we were not fed uninteresting commits we will never // construct a boundary. There is no reason to include the // extra overhead associated with that in our pipeline. @@ -107,16 +108,19 @@ class StartGenerator extends Generator { boundary = false; } + final DateRevQueue pending; int pendingOutputType = 0; - if (!(q instanceof DateRevQueue)) - q = new DateRevQueue(q); + if (q instanceof DateRevQueue) + pending = (DateRevQueue)q; + else + pending = new DateRevQueue(q); if (tf != TreeFilter.ALL) { rf = AndRevFilter.create(rf, new RewriteTreeFilter(w, tf)); pendingOutputType |= HAS_REWRITE | NEEDS_REWRITE; } walker.queue = q; - g = new PendingGenerator(w, (DateRevQueue) q, rf, pendingOutputType); + g = new PendingGenerator(w, pending, rf, pendingOutputType); if (boundary) { // Because the boundary generator may produce uninteresting @@ -143,6 +147,17 @@ class StartGenerator extends Generator { g = new LIFORevQueue(g); if (boundary) g = new BoundaryGenerator(w, g); + else if (uninteresting) { + // Try to protect ourselves from uninteresting commits producing + // due to clock skew in the commit time stamps. Delay such that + // we have a chance at coloring enough of the graph correctly, + // and then strip any UNINTERESTING nodes that may have leaked + // through early. + // + if (pending.peek() != null) + g = new DelayRevQueue(g); + g = new FixUninterestingGenerator(g); + } w.pending = g; return g.next(); -- 2.11.4.GIT