Merge changes Ie393fb8b,Ib11a077a

* changes:
  Push errors back over sideband when possible
  Report progress while updating references

8 files changed, 208 insertions, 42 deletions

diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java
index 192050a17c..0c856d4aa5 100644
--- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java
+++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java
@@ -64,6 +64,7 @@ import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.eclipse.jgit.errors.UnpackException;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.ReceivePack;
 import org.eclipse.jgit.transport.RefAdvertiser.PacketLineOutRefAdvertiser;
@@ -170,9 +171,18 @@ class ReceivePackServlet extends HttpServlet {
 			};
 			rp.receive(getInputStream(req), out, null);
 			out.close();
+		} catch (UnpackException e) {
+			// This should be already reported to the client.
+			getServletContext().log(
+					HttpServerText.get().internalErrorDuringReceivePack,
+					e.getCause());
+
 		} catch (IOException e) {
 			getServletContext().log(HttpServerText.get().internalErrorDuringReceivePack, e);
-			rsp.sendError(SC_INTERNAL_SERVER_ERROR);
+			if (!rsp.isCommitted()) {
+				rsp.reset();
+				rsp.sendError(SC_INTERNAL_SERVER_ERROR);
+			}
 			return;
 		}
 	}
diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java
index 2b9e81f1d3..178473c401 100644
--- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java
+++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java
@@ -68,6 +68,7 @@ import javax.servlet.http.HttpServletResponse;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.RefAdvertiser.PacketLineOutRefAdvertiser;
 import org.eclipse.jgit.transport.UploadPack;
+import org.eclipse.jgit.transport.UploadPackInternalServerErrorException;
 import org.eclipse.jgit.transport.UploadPackMayNotContinueException;
 import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException;
 import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException;
@@ -175,14 +176,23 @@ class UploadPackServlet extends HttpServlet {
 			out.close();
 
 		} catch (UploadPackMayNotContinueException e) {
-			if (!e.isOutput())
+			if (!e.isOutput() && !rsp.isCommitted()) {
+				rsp.reset();
 				rsp.sendError(SC_SERVICE_UNAVAILABLE);
+			}
 			return;
 
+		} catch (UploadPackInternalServerErrorException e) {
+			getServletContext().log(
+					HttpServerText.get().internalErrorDuringUploadPack,
+					e.getCause());
+
 		} catch (IOException e) {
 			getServletContext().log(HttpServerText.get().internalErrorDuringUploadPack, e);
-			rsp.reset();
-			rsp.sendError(SC_INTERNAL_SERVER_ERROR);
+			if (!rsp.isCommitted()) {
+				rsp.reset();
+				rsp.sendError(SC_INTERNAL_SERVER_ERROR);
+			}
 			return;
 		}
 	}
diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties
index 5a40edd7dc..570895e810 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties
@@ -224,6 +224,7 @@ indexSignatureIsInvalid=Index signature is invalid: {0}
 indexWriteException=Modified index could not be written
 integerValueOutOfRange=Integer value {0}.{1} out of range
 internalRevisionError=internal revision error
+internalServerError=internal server error
 interruptedWriting=Interrupted writing {0}
 inTheFuture=in the future
 invalidAdvertisementOf=invalid advertisement of {0}
@@ -470,6 +471,7 @@ unsupportedEncryptionVersion=Unsupported encryption version: {0}
 unsupportedOperationNotAddAtEnd=Not add-at-end: {0}
 unsupportedPackIndexVersion=Unsupported pack index version {0}
 unsupportedPackVersion=Unsupported pack version {0}.
+updatingReferences=Updating references
 updatingRefFailed=Updating the ref {0} to {1} failed. ReturnCode from RefUpdate.update() was {2}
 uriNotFound={0} not found
 userConfigFileInvalid=User config file {0} invalid {1}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java
index 02538a2dce..7054cf800f 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java
@@ -284,6 +284,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String indexWriteException;
 	/***/ public String integerValueOutOfRange;
 	/***/ public String internalRevisionError;
+	/***/ public String internalServerError;
 	/***/ public String interruptedWriting;
 	/***/ public String inTheFuture;
 	/***/ public String invalidAdvertisementOf;
@@ -530,6 +531,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String unsupportedOperationNotAddAtEnd;
 	/***/ public String unsupportedPackIndexVersion;
 	/***/ public String unsupportedPackVersion;
+	/***/ public String updatingReferences;
 	/***/ public String updatingRefFailed;
 	/***/ public String uriNotFound;
 	/***/ public String userConfigFileInvalid;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java
index 17c2d5f866..5f330308a0 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java
@@ -57,11 +57,13 @@ import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jgit.JGitText;
 import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.errors.NotSupportedException;
 import org.eclipse.jgit.errors.TransportException;
+import org.eclipse.jgit.lib.BatchingProgressMonitor;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ProgressMonitor;
@@ -183,10 +185,16 @@ class FetchProcess {
 
 		final RevWalk walk = new RevWalk(transport.local);
 		try {
+			if (monitor instanceof BatchingProgressMonitor) {
+				((BatchingProgressMonitor) monitor).setDelayStart(
+						250, TimeUnit.MILLISECONDS);
+			}
+			monitor.beginTask(JGitText.get().updatingReferences, localUpdates.size());
 			if (transport.isRemoveDeletedRefs())
 				deleteStaleTrackingRefs(result, walk);
 			for (TrackingRefUpdate u : localUpdates) {
 				try {
+					monitor.update(1);
 					u.update(walk);
 					result.add(u);
 				} catch (IOException err) {
@@ -195,6 +203,7 @@ class FetchProcess {
 							u.getLocalName(), err.getMessage()), err);
 				}
 			}
+			monitor.endTask();
 		} finally {
 			walk.release();
 		}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java
index f1f9b0f44d..40435b8a29 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java
@@ -62,6 +62,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jgit.JGitText;
 import org.eclipse.jgit.errors.MissingObjectException;
@@ -1026,8 +1027,20 @@ public class ReceivePack {
 
 	private void executeCommands() {
 		preReceive.onPreReceive(this, filterCommands(Result.NOT_ATTEMPTED));
-		for (final ReceiveCommand cmd : filterCommands(Result.NOT_ATTEMPTED))
+
+		List<ReceiveCommand> toApply = filterCommands(Result.NOT_ATTEMPTED);
+		ProgressMonitor updating = NullProgressMonitor.INSTANCE;
+		if (sideBand) {
+			SideBandProgressMonitor pm = new SideBandProgressMonitor(msgOut);
+			pm.setDelayStart(250, TimeUnit.MILLISECONDS);
+			updating = pm;
+		}
+		updating.beginTask(JGitText.get().updatingReferences, toApply.size());
+		for (ReceiveCommand cmd : toApply) {
+			updating.update(1);
 			execute(cmd);
+		}
+		updating.endTask();
 	}
 
 	private void execute(final ReceiveCommand cmd) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
index 0e50b937b6..41f64a163c 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -401,25 +401,62 @@ public class UploadPack {
 			}
 		}
 
-		recvWants();
-		if (wantIds.isEmpty()) {
-			preUploadHook.onBeginNegotiateRound(this, wantIds, 0);
-			preUploadHook.onEndNegotiateRound(this, wantIds, 0, 0, false);
-			return;
-		}
+		boolean sendPack;
+		try {
+			recvWants();
+			if (wantIds.isEmpty()) {
+				preUploadHook.onBeginNegotiateRound(this, wantIds, 0);
+				preUploadHook.onEndNegotiateRound(this, wantIds, 0, 0, false);
+				return;
+			}
 
-		if (options.contains(OPTION_MULTI_ACK_DETAILED)) {
-			multiAck = MultiAck.DETAILED;
-			noDone = options.contains(OPTION_NO_DONE);
-		} else if (options.contains(OPTION_MULTI_ACK))
-			multiAck = MultiAck.CONTINUE;
-		else
-			multiAck = MultiAck.OFF;
+			if (options.contains(OPTION_MULTI_ACK_DETAILED)) {
+				multiAck = MultiAck.DETAILED;
+				noDone = options.contains(OPTION_NO_DONE);
+			} else if (options.contains(OPTION_MULTI_ACK))
+				multiAck = MultiAck.CONTINUE;
+			else
+				multiAck = MultiAck.OFF;
+
+			sendPack = negotiate();
+		} catch (PackProtocolException err) {
+			reportErrorDuringNegotiate(err.getMessage());
+			throw err;
+
+		} catch (UploadPackMayNotContinueException err) {
+			if (!err.isOutput() && err.getMessage() != null) {
+				try {
+					pckOut.writeString("ERR " + err.getMessage() + "\n");
+					err.setOutput();
+				} catch (Throwable err2) {
+					// Ignore this secondary failure (and not mark output).
+				}
+			}
+			throw err;
+
+		} catch (IOException err) {
+			reportErrorDuringNegotiate(JGitText.get().internalServerError);
+			throw err;
+		} catch (RuntimeException err) {
+			reportErrorDuringNegotiate(JGitText.get().internalServerError);
+			throw err;
+		} catch (Error err) {
+			reportErrorDuringNegotiate(JGitText.get().internalServerError);
+			throw err;
+		}
 
-		if (negotiate())
+		if (sendPack)
 			sendPack();
 	}
 
+	private void reportErrorDuringNegotiate(String msg) {
+		try {
+			pckOut.writeString("ERR " + msg + "\n");
+		} catch (Throwable err) {
+			// Ignore this secondary failure.
+		}
+	}
+
 	/**
 	 * Generate an advertisement of available refs and capabilities.
 	 *
@@ -536,16 +573,7 @@ public class UploadPack {
 
 	private ObjectId processHaveLines(List<ObjectId> peerHas, ObjectId last)
 			throws IOException {
-		try {
-			preUploadHook.onBeginNegotiateRound(this, wantIds, peerHas.size());
-		} catch (UploadPackMayNotContinueException fail) {
-			if (fail.getMessage() != null) {
-				pckOut.writeString("ERR " + fail.getMessage() + "\n");
-				fail.setOutput();
-			}
-			throw fail;
-		}
-
+		preUploadHook.onBeginNegotiateRound(this, wantIds, peerHas.size());
 		if (peerHas.isEmpty())
 			return last;
 
@@ -576,7 +604,6 @@ public class UploadPack {
 					if (wantIds.contains(id)) {
 						String msg = MessageFormat.format(
 								JGitText.get().wantNotValid, id.name());
-						pckOut.writeString("ERR " + msg);
 						throw new PackProtocolException(msg, notFound);
 					}
 					continue;
@@ -591,7 +618,6 @@ public class UploadPack {
 					if (!advertised.contains(obj)) {
 						String msg = MessageFormat.format(
 								JGitText.get().wantNotValid, obj.name());
-						pckOut.writeString("ERR " + msg);
 						throw new PackProtocolException(msg);
 					}
 
@@ -689,17 +715,7 @@ public class UploadPack {
 			sentReady = true;
 		}
 
-		try {
-			preUploadHook.onEndNegotiateRound(this, wantAll, //
-					haveCnt, missCnt, sentReady);
-		} catch (UploadPackMayNotContinueException fail) {
-			if (fail.getMessage() != null) {
-				pckOut.writeString("ERR " + fail.getMessage() + "\n");
-				fail.setOutput();
-			}
-			throw fail;
-		}
-
+		preUploadHook.onEndNegotiateRound(this, wantAll, haveCnt, missCnt, sentReady);
 		peerHas.clear();
 		return last;
 	}
@@ -768,6 +784,49 @@ public class UploadPack {
 						"\\x" + Integer.toHexString(eof)));
 		}
 
+		if (sideband) {
+			try {
+				sendPack(true);
+			} catch (UploadPackMayNotContinueException noPack) {
+				// This was already reported on (below).
+				throw noPack;
+			} catch (IOException err) {
+				if (reportInternalServerErrorOverSideband())
+					throw new UploadPackInternalServerErrorException(err);
+				else
+					throw err;
+			} catch (RuntimeException err) {
+				if (reportInternalServerErrorOverSideband())
+					throw new UploadPackInternalServerErrorException(err);
+				else
+					throw err;
+			} catch (Error err) {
+				if (reportInternalServerErrorOverSideband())
+					throw new UploadPackInternalServerErrorException(err);
+				else
+					throw err;
+			}
+		} else {
+			sendPack(false);
+		}
+	}
+
+	private boolean reportInternalServerErrorOverSideband() {
+		try {
+			SideBandOutputStream err = new SideBandOutputStream(
+					SideBandOutputStream.CH_ERROR,
+					SideBandOutputStream.SMALL_BUF,
+					rawOut);
+			err.write(Constants.encode(JGitText.get().internalServerError));
+			err.flush();
+			return true;
+		} catch (Throwable cannotReport) {
+			// Ignore the reason. This is a secondary failure.
+			return false;
+		}
+	}
+
+	private void sendPack(final boolean sideband) throws IOException {
 		ProgressMonitor pm = NullProgressMonitor.INSTANCE;
 		OutputStream packOut = rawOut;
 		SideBandOutputStream msgOut = null;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPackInternalServerErrorException.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPackInternalServerErrorException.java
new file mode 100644
index 0000000000..0636984dcf
--- /dev/null
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPackInternalServerErrorException.java
@@ -0,0 +1,61 @@
+/*
+ * Copyright (C) 2011, Google Inc.
+ * 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.transport;
+
+import java.io.IOException;
+
+/** UploadPack has already reported an error to the client. */
+public class UploadPackInternalServerErrorException extends IOException {
+	private static final long serialVersionUID = 1L;
+
+	/**
+	 * Initialize a new exception.
+	 *
+	 * @param why
+	 *            root cause.
+	 */
+	public UploadPackInternalServerErrorException(Throwable why) {
+		super(why);
+	}
+}