Shallow fetch: Respect "shallow" lines
When fetching from a shallow clone, the client sends "have" lines
to tell the server about objects it already has and "shallow" lines
to tell where its local history terminates. In some circumstances,
the server fails to honor the shallow lines and fails to return
objects that the client needs.
UploadPack passes the "have" lines to PackWriter so PackWriter can
omit them from the generated pack. UploadPack processes "shallow"
lines by calling RevWalk.assumeShallow() with the set of shallow
commits. RevWalk creates and caches RevCommits for these shallow
commits, clearing out their parents. That way, walks correctly
terminate at the shallow commits instead of assuming the client has
history going back behind them. UploadPack converts its RevWalk to an
ObjectWalk, maintaining the cached RevCommits, and passes it to
PackWriter.
Unfortunately, to support shallow fetches the PackWriter does the
following:
if (shallowPack && !(walk instanceof DepthWalk.ObjectWalk))
walk = new DepthWalk.ObjectWalk(reader, depth);
That is, when the client sends a "deepen" line (fetch --depth=<n>)
and the caller has not passed in a DepthWalk.ObjectWalk, PackWriter
throws away the RevWalk that was passed in and makes a new one. The
cleared parent lists prepared by RevWalk.assumeShallow() are lost.
Fortunately UploadPack intends to pass in a DepthWalk.ObjectWalk.
It tries to create it by calling toObjectWalkWithSameObjects() on
a DepthWalk.RevWalk. But it doesn't work: because DepthWalk.RevWalk
does not override the standard RevWalk#toObjectWalkWithSameObjects
implementation, the result is a plain ObjectWalk instead of an
instance of DepthWalk.ObjectWalk.
The result is that the "shallow" information is thrown away and
objects reachable from the shallow commits can be omitted from the
pack sent when fetching with --depth from a shallow clone.
Multiple factors collude to limit the circumstances under which this
bug can be observed:
1. Commits with depth != 0 don't enter DepthGenerator's pending queue.
That means a "have" cannot have any effect on DepthGenerator unless
it is also a "want".
2. DepthGenerator#next() doesn't call carryFlagsImpl(), so the
uninteresting flag is not propagated to ancestors there even if a
"have" is also a "want".
3. JGit treats a depth of 1 as "1 past the wants".
Because of (2), the only place the UNINTERESTING flag can leak to a
shallow commit's parents is in the carryFlags() call from
markUninteresting(). carryFlags() only traverses commits that have
already been parsed: commits yet to be parsed are supposed to inherit
correct flags from their parent in PendingGenerator#next (which
doesn't happen here --- that is (2)). So the list of commits that have
already been parsed becomes relevant.
When we hit the markUninteresting() call, all "want"s, "have"s, and
commits to be unshallowed have been parsed. carryFlags() only
affects the parsed commits. If the "want" is a direct parent of a
"have", then it carryFlags() marks it as uninteresting. If the "have"
was also a "shallow", then its parent pointer should have been null
and the "want" shouldn't have been marked, so we see the bug. If the
"want" is a more distant ancestor then (2) keeps the uninteresting
state from propagating to the "want" and we don't see the bug. If the
"shallow" is not also a "have" then the shallow commit isn't parsed
so (2) keeps the uninteresting state from propagating to the "want
so we don't see the bug.
Here is a reproduction case (time flowing left to right, arrows
pointing to parents). "C" must be a commit that the client
reports as a "have" during negotiation. That can only happen if the
server reports it as an existing branch or tag in the first round of
negotiation:
A <-- B <-- C <-- D
First do
git clone --depth 1 <repo>
which yields D as a "have" and C as a "shallow" commit. Then try
git fetch --depth 1 <repo> B:refs/heads/B
Negotiation sets up: have D, shallow C, have C, want B.
But due to this bug B is marked as uninteresting and is not sent.
Change-Id: I6e14b57b2f85e52d28cdcf356df647870f475440
Signed-off-by: Terry Parker <tparker@google.com>
7 years ago |
123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251 |
- /*
- * Copyright (C) 2010, Garmin International
- * Copyright (C) 2010, Matt Fischer <matt.fischer@garmin.com>
- * and other copyright owners as documented in the project's IP log.
- *
- * This program and the accompanying materials are made available
- * under the terms of the Eclipse Distribution License v1.0 which
- * accompanies this distribution, is reproduced below, and is
- * available at http://www.eclipse.org/org/documents/edl-v10.php
- *
- * 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 Eclipse Foundation, Inc. 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.eclipse.jgit.revwalk;
-
- import java.io.IOException;
-
- import org.eclipse.jgit.errors.IncorrectObjectTypeException;
- import org.eclipse.jgit.errors.MissingObjectException;
- import org.eclipse.jgit.lib.AnyObjectId;
- import org.eclipse.jgit.lib.ObjectReader;
- import org.eclipse.jgit.lib.Repository;
-
- /** Interface for revision walkers that perform depth filtering. */
- public interface DepthWalk {
- /** @return Depth to filter to. */
- public int getDepth();
-
- /** @return flag marking commits that should become unshallow. */
- public RevFlag getUnshallowFlag();
-
- /** @return flag marking commits that are interesting again. */
- public RevFlag getReinterestingFlag();
-
- /** RevCommit with a depth (in commits) from a root. */
- public static class Commit extends RevCommit {
- /** Depth of this commit in the graph, via shortest path. */
- int depth;
-
- /** @return depth of this commit, as found by the shortest path. */
- public int getDepth() {
- return depth;
- }
-
- /**
- * Initialize a new commit.
- *
- * @param id
- * object name for the commit.
- */
- protected Commit(AnyObjectId id) {
- super(id);
- depth = -1;
- }
- }
-
- /** Subclass of RevWalk that performs depth filtering. */
- public class RevWalk extends org.eclipse.jgit.revwalk.RevWalk implements DepthWalk {
- private final int depth;
-
- private final RevFlag UNSHALLOW;
-
- private final RevFlag REINTERESTING;
-
- /**
- * @param repo Repository to walk
- * @param depth Maximum depth to return
- */
- public RevWalk(Repository repo, int depth) {
- super(repo);
-
- this.depth = depth;
- this.UNSHALLOW = newFlag("UNSHALLOW"); //$NON-NLS-1$
- this.REINTERESTING = newFlag("REINTERESTING"); //$NON-NLS-1$
- }
-
- /**
- * @param or ObjectReader to use
- * @param depth Maximum depth to return
- */
- public RevWalk(ObjectReader or, int depth) {
- super(or);
-
- this.depth = depth;
- this.UNSHALLOW = newFlag("UNSHALLOW"); //$NON-NLS-1$
- this.REINTERESTING = newFlag("REINTERESTING"); //$NON-NLS-1$
- }
-
- /**
- * Mark a root commit (i.e., one whose depth should be considered 0.)
- *
- * @param c
- * Commit to mark
- * @throws IOException
- * @throws IncorrectObjectTypeException
- * @throws MissingObjectException
- */
- public void markRoot(RevCommit c) throws MissingObjectException,
- IncorrectObjectTypeException, IOException {
- if (c instanceof Commit)
- ((Commit) c).depth = 0;
- super.markStart(c);
- }
-
- @Override
- protected RevCommit createCommit(AnyObjectId id) {
- return new Commit(id);
- }
-
- public int getDepth() {
- return depth;
- }
-
- public RevFlag getUnshallowFlag() {
- return UNSHALLOW;
- }
-
- public RevFlag getReinterestingFlag() {
- return REINTERESTING;
- }
-
- @Override
- public ObjectWalk toObjectWalkWithSameObjects() {
- ObjectWalk ow = new ObjectWalk(reader, depth);
- ow.objects = objects;
- ow.freeFlags = freeFlags;
- return ow;
- }
- }
-
- /** Subclass of ObjectWalk that performs depth filtering. */
- public class ObjectWalk extends org.eclipse.jgit.revwalk.ObjectWalk implements DepthWalk {
- private final int depth;
-
- private final RevFlag UNSHALLOW;
-
- private final RevFlag REINTERESTING;
-
- /**
- * @param repo Repository to walk
- * @param depth Maximum depth to return
- */
- public ObjectWalk(Repository repo, int depth) {
- super(repo);
-
- this.depth = depth;
- this.UNSHALLOW = newFlag("UNSHALLOW"); //$NON-NLS-1$
- this.REINTERESTING = newFlag("REINTERESTING"); //$NON-NLS-1$
- }
-
- /**
- * @param or Object Reader
- * @param depth Maximum depth to return
- */
- public ObjectWalk(ObjectReader or, int depth) {
- super(or);
-
- this.depth = depth;
- this.UNSHALLOW = newFlag("UNSHALLOW"); //$NON-NLS-1$
- this.REINTERESTING = newFlag("REINTERESTING"); //$NON-NLS-1$
- }
-
- /**
- * Mark a root commit (i.e., one whose depth should be considered 0.)
- *
- * @param o
- * Commit to mark
- * @throws IOException
- * @throws IncorrectObjectTypeException
- * @throws MissingObjectException
- */
- public void markRoot(RevObject o) throws MissingObjectException,
- IncorrectObjectTypeException, IOException {
- RevObject c = o;
- while (c instanceof RevTag) {
- c = ((RevTag) c).getObject();
- parseHeaders(c);
- }
- if (c instanceof Commit)
- ((Commit) c).depth = 0;
- super.markStart(o);
- }
-
- /**
- * Mark an element which used to be shallow in the client, but which
- * should now be considered a full commit. Any ancestors of this commit
- * should be included in the walk, even if they are the ancestor of an
- * uninteresting commit.
- *
- * @param c
- * Commit to mark
- * @throws MissingObjectException
- * @throws IncorrectObjectTypeException
- * @throws IOException
- */
- public void markUnshallow(RevObject c) throws MissingObjectException,
- IncorrectObjectTypeException, IOException {
- if (c instanceof RevCommit)
- c.add(UNSHALLOW);
- super.markStart(c);
- }
-
- @Override
- protected RevCommit createCommit(AnyObjectId id) {
- return new Commit(id);
- }
-
- public int getDepth() {
- return depth;
- }
-
- public RevFlag getUnshallowFlag() {
- return UNSHALLOW;
- }
-
- public RevFlag getReinterestingFlag() {
- return REINTERESTING;
- }
- }
- }
|