UploadPack: move writing of "packfile" header

In a subsequent patch, in some cases, PackWriter#writePack will be
responsible for both the "packfile-uris" and "packfile" sections,
meaning that (in these cases) it must write the "packfile" section
header itself.

In preparation for that patch, move the writing of the "packfile"
section header closer to the invocation of PackWriter#writePack when the
entire fetch response is configured to use the sideband. This means that
"packfile" is written *after* objects are counted (and progress messages
sent to the client in sideband 2) when the "sideband-all" feature is
used (whether "packfile-uris" is used or not), and written *before*
objects are counted otherwise.

Having code to write "packfile" in two places is unfortunate but
necessary. When "sideband-all" is not used, object counting has to
happen after "packfile" is written, because "packfile" activates the
sideband that allows counting progress to be transmitted. When
"packfile-uris" is used, object counting has to happen before "packfile"
is written, because object counting determines whether to send
"packfile-uris" or "packfile". When "sideband-all" is used but
"packfile-uris" is not used, either way works; this commit uses
"packfile-uris" behavior in this case.

Also make the naming of the sideband-activating methods in PacketLineOut
more consistent.

Change-Id: Ifbfd26cc26af10c41b77758168833702d6983df1
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java

@@ -2138,7 +2138,14 @@ public class UploadPackTest {
 				PacketLineIn.END);
 		PacketLineIn pckIn = new PacketLineIn(recvStream);
 
-		assertThat(pckIn.readString(), is("\001packfile"));
+		String s;
+		// When sideband-all is used, object counting happens before
+		// "packfile" is written, and object counting outputs progress
+		// in sideband 2. Skip all these lines.
+		for (s = pckIn.readString(); s.startsWith("\002"); s = pckIn.readString()) {
+			// do nothing
+		}
+		assertThat(s, is("\001packfile"));
 		parsePack(recvStream);
 	}
 

org.eclipse.jgit/src/org/eclipse/jgit/transport/PacketLineOut.java

@@ -74,7 +74,7 @@ public class PacketLineOut {
 
 	private boolean flushOnEnd;
 
-	private boolean useSideband;
+	private boolean usingSideband;
 
 	/**
 	 * Create a new packet line writer.
@@ -100,13 +100,25 @@ public class PacketLineOut {
 	}
 
 	/**
-	 * When writing packet lines, use the first byte of each non-flush and
+	 * @return whether to add a sideband designator to each non-flush and
-	 * non-delim packet as a sideband designator.
+	 *     non-delim packet
-	 *
+	 * @see #setUsingSideband
+	 * @since 5.5
+	 */
+	public boolean isUsingSideband() {
+		return usingSideband;
+	}
+
+	/**
+	 * @param value If true, when writing packet lines, add, as the first
+	 *     byte, a sideband designator to each non-flush and non-delim
+	 *     packet. See pack-protocol.txt and protocol-v2.txt from the Git
+	 *     project for more information, specifically the "side-band" and
+	 *     "sideband-all" sections.
 	 * @since 5.5
 	 */
-	public void useSidebandFormat() {
+	public void setUsingSideband(boolean value) {
-		this.useSideband = true;
+		this.usingSideband = value;
 	}
 
 	/**
@@ -151,7 +163,7 @@ public class PacketLineOut {
 	 * @since 4.5
 	 */
 	public void writePacket(byte[] buf, int pos, int len) throws IOException {
-		if (useSideband) {
+		if (usingSideband) {
 			formatLength(len + 5);
 			out.write(lenbuffer, 0, 4);
 			out.write(1);

org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java

@@ -1136,7 +1136,7 @@ public class UploadPack {
 		protocolV2Hook.onFetch(req);
 
 		if (req.getSidebandAll()) {
-			pckOut.useSidebandFormat();
+			pckOut.setUsingSideband(true);
 		}
 
 		// TODO(ifrade): Refactor to pass around the Request object, instead of
@@ -1224,7 +1224,11 @@ public class UploadPack {
 
 			if (sectionSent)
 				pckOut.writeDelim();
+			if (!pckOut.isUsingSideband()) {
+				// sendPack will write "packfile\n" for us if sideband-all is used.
+				// But sideband-all is not used, so we have to write it ourselves.
 				pckOut.writeString("packfile\n"); //$NON-NLS-1$
+			}
 			sendPack(new PackStatistics.Accumulator(),
 					req,
 					req.getClientCapabilities().contains(OPTION_INCLUDE_TAG)
@@ -2293,6 +2297,9 @@ public class UploadPack {
 				}
 			}
 
+			if (pckOut.isUsingSideband()) {
+				pckOut.writeString("packfile\n"); //$NON-NLS-1$
+			}
 			pw.writePack(pm, NullProgressMonitor.INSTANCE, packOut);
 
 			if (msgOut != NullOutputStream.INSTANCE) {