git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Shawn O. Pearce" <spearce@spearce.org>
To: Robin Rosenberg <robin.rosenberg@dewire.com>
Cc: git@vger.kernel.org
Subject: [JGIT PATCH 1/3] Check object connectivity during fetch if fsck is enabled
Date: Thu, 30 Oct 2008 10:46:23 -0700	[thread overview]
Message-ID: <1225388785-26818-2-git-send-email-spearce@spearce.org> (raw)
In-Reply-To: <1225388785-26818-1-git-send-email-spearce@spearce.org>

If we are fetching over a pack oriented connection and we are doing
object-level fsck validation we need to also verify the graph is
fully connected after the fetch is complete.  This additional check
is necessary to ensure the peer didn't omit objects that we don't
have, but which are listed as needing to be present.

On the walk style fetch connection we can bypass this check, as the
connectivity was implicitly verified by the walker as it downloaded
objects and built its queue of things to fetch.  Native pack and
bundle transports however do not have this check built into them,
and require that we execute the work ourselves.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../jgit/transport/BasePackFetchConnection.java    |    4 +++
 .../spearce/jgit/transport/FetchConnection.java    |   22 ++++++++++++++++++++
 .../org/spearce/jgit/transport/FetchProcess.java   |   13 ++++++++++-
 .../spearce/jgit/transport/TransportBundle.java    |    4 +++
 .../jgit/transport/WalkFetchConnection.java        |    4 +++
 5 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
index a542eb7..542a8a9 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
@@ -146,6 +146,10 @@ public boolean didFetchIncludeTags() {
 		return false;
 	}
 
+	public boolean didFetchTestConnectivity() {
+		return false;
+	}
+
 	protected void doFetch(final ProgressMonitor monitor,
 			final Collection<Ref> want) throws TransportException {
 		try {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/FetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/FetchConnection.java
index 9d25b0d..d93972d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/FetchConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/FetchConnection.java
@@ -111,4 +111,26 @@ public void fetch(final ProgressMonitor monitor, final Collection<Ref> want)
 	 *         false if tags were not implicitly obtained.
 	 */
 	public boolean didFetchIncludeTags();
+
+	/**
+	 * Did the last {@link #fetch(ProgressMonitor, Collection)} validate graph?
+	 * <p>
+	 * Some transports walk the object graph on the client side, with the client
+	 * looking for what objects it is missing and requesting them individually
+	 * from the remote peer. By virtue of completing the fetch call the client
+	 * implicitly tested the object connectivity, as every object in the graph
+	 * was either already local or was requested successfully from the peer. In
+	 * such transports this method returns true.
+	 * <p>
+	 * Some transports assume the remote peer knows the Git object graph and is
+	 * able to supply a fully connected graph to the client (although it may
+	 * only be transferring the parts the client does not yet have). Its faster
+	 * to assume such remote peers are well behaved and send the correct
+	 * response to the client. In such tranports this method returns false.
+	 * 
+	 * @return true if the last fetch had to perform a connectivity check on the
+	 *         client side in order to succeed; false if the last fetch assumed
+	 *         the remote peer supplied a complete graph.
+	 */
+	public boolean didFetchTestConnectivity();
 }
\ No newline at end of file
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/FetchProcess.java b/org.spearce.jgit/src/org/spearce/jgit/transport/FetchProcess.java
index 654572d..bb2d051 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/FetchProcess.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/FetchProcess.java
@@ -118,7 +118,7 @@ else if (tagopt == TagOpt.FETCH_TAGS)
 
 			final boolean includedTags;
 			if (!askFor.isEmpty() && !askForIsComplete()) {
-				conn.fetch(monitor, askFor.values());
+				fetchObjects(monitor);
 				includedTags = conn.didFetchIncludeTags();
 
 				// Connection was used for object transfer. If we
@@ -143,7 +143,7 @@ else if (tagopt == TagOpt.FETCH_TAGS)
 				if (!askFor.isEmpty() && (!includedTags || !askForIsComplete())) {
 					reopenConnection();
 					if (!askFor.isEmpty())
-						conn.fetch(monitor, askFor.values());
+						fetchObjects(monitor);
 				}
 			}
 		} finally {
@@ -171,6 +171,15 @@ else if (tagopt == TagOpt.FETCH_TAGS)
 		}
 	}
 
+	private void fetchObjects(final ProgressMonitor monitor)
+			throws TransportException {
+		conn.fetch(monitor, askFor.values());
+		if (transport.isCheckFetchedObjects()
+				&& !conn.didFetchTestConnectivity() && !askForIsComplete())
+			throw new TransportException(transport.getURI(),
+					"peer did not supply a complete object graph");
+	}
+
 	private void closeConnection() {
 		if (conn != null) {
 			conn.close();
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java
index 5b321a0..7d38b02 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java
@@ -165,6 +165,10 @@ private String readLine(final byte[] hdrbuf) throws IOException {
 			return RawParseUtils.decode(Constants.CHARSET, hdrbuf, 0, lf);
 		}
 
+		public boolean didFetchTestConnectivity() {
+			return false;
+		}
+
 		@Override
 		protected void doFetch(final ProgressMonitor monitor,
 				final Collection<Ref> want) throws TransportException {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java
index 5638454..d089f7b 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java
@@ -189,6 +189,10 @@ WalkFetchConnection(final WalkTransport wt, final WalkRemoteObjectDatabase w) {
 		workQueue = new LinkedList<ObjectId>();
 	}
 
+	public boolean didFetchTestConnectivity() {
+		return true;
+	}
+
 	@Override
 	protected void doFetch(final ProgressMonitor monitor,
 			final Collection<Ref> want) throws TransportException {
-- 
1.6.0.3.756.gb776d

  reply	other threads:[~2008-10-30 17:47 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-30 17:46 [JGIT PATCH 0/3] Improved object validation Shawn O. Pearce
2008-10-30 17:46 ` Shawn O. Pearce [this message]
2008-10-30 17:46   ` [JGIT PATCH 2/3] Add --[no-]thin and --[no-]fsck optiosn to fetch command line tool Shawn O. Pearce
2008-10-30 17:46     ` [JGIT PATCH 3/3] Don't permit '.' or '..' in tree entries Shawn O. Pearce
2008-10-31  0:01 ` [JGIT PATCH 0/3] Improved object validation Robin Rosenberg
2008-10-31 14:55   ` Shawn O. Pearce

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1225388785-26818-2-git-send-email-spearce@spearce.org \
    --to=spearce@spearce.org \
    --cc=git@vger.kernel.org \
    --cc=robin.rosenberg@dewire.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).