Support "sideband-all" in protocol v2 fetch

Allow the client to specify "sideband-all" in a fetch v2 request,
indicating that the whole response is to be multiplexed (with a sideband
indicator on every non-flush and non-delim pkt) instead of only the
packfile being multiplexed. This allows, for example, progress messages
to be sent at any point in the response.

This implements the "sideband-all" feature documented in
Documentation/technical/protocol-v2.txt in Git.

Change-Id: I3e7f21c88ff0982b1b7ebb09c9ad6c742c4483c8
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
This commit is contained in:
Jonathan Tan 2019-01-22 13:17:12 -08:00
parent c1ed69de4a
commit 79d776429e
7 changed files with 132 additions and 4 deletions

View File

@ -594,6 +594,12 @@ public void testV2CapabilitiesRefInWantNotAdvertisedIfUnallowed() throws Excepti
checkUnadvertisedIfUnallowed("ref-in-want");
}
@Test
public void testV2CapabilitiesAllowSidebandAll() throws Exception {
checkAdvertisedIfAllowed("uploadpack", "allowsidebandall", "sideband-all");
checkUnadvertisedIfUnallowed("sideband-all");
}
@Test
public void testV2CapabilitiesRefInWantNotAdvertisedIfAdvertisingForbidden() throws Exception {
server.getConfig().setBoolean("uploadpack", null, "allowrefinwant", true);
@ -1874,6 +1880,13 @@ public void testV2FetchWantRefIfNotAllowed() throws Exception {
"unexpected want-ref refs/heads/one");
}
@Test
public void testV2FetchSidebandAllIfNotAllowed() throws Exception {
checkV2FetchWhenNotAllowed(
"sideband-all\n",
"unexpected sideband-all");
}
@Test
public void testV2FetchWantRef() throws Exception {
RevCommit one = remote.commit().message("1").create();
@ -2057,6 +2070,51 @@ public void testV2FetchMissingShallow() throws Exception {
assertTrue(client.getObjectDatabase().has(three.toObjectId()));
}
@Test
public void testV2FetchSidebandAllNoPackfile() throws Exception {
RevCommit fooParent = remote.commit().message("x").create();
RevCommit fooChild = remote.commit().message("x").parent(fooParent).create();
RevCommit barParent = remote.commit().message("y").create();
RevCommit barChild = remote.commit().message("y").parent(barParent).create();
remote.update("branch1", fooChild);
remote.update("branch2", barChild);
server.getConfig().setBoolean("uploadpack", null, "allowsidebandall", true);
ByteArrayInputStream recvStream = uploadPackV2(
"command=fetch\n",
PacketLineIn.DELIM,
"sideband-all\n",
"want " + fooChild.toObjectId().getName() + "\n",
"want " + barChild.toObjectId().getName() + "\n",
"have " + fooParent.toObjectId().getName() + "\n",
PacketLineIn.END);
PacketLineIn pckIn = new PacketLineIn(recvStream);
assertThat(pckIn.readString(), is("\001acknowledgments"));
assertThat(pckIn.readString(), is("\001ACK " + fooParent.getName()));
assertTrue(PacketLineIn.isEnd(pckIn.readString()));
}
@Test
public void testV2FetchSidebandAllPackfile() throws Exception {
RevCommit commit = remote.commit().message("x").create();
remote.update("master", commit);
server.getConfig().setBoolean("uploadpack", null, "allowsidebandall", true);
ByteArrayInputStream recvStream = uploadPackV2("command=fetch\n",
PacketLineIn.DELIM,
"want " + commit.getName() + "\n",
"sideband-all\n",
"done\n",
PacketLineIn.END);
PacketLineIn pckIn = new PacketLineIn(recvStream);
assertThat(pckIn.readString(), is("\001packfile"));
parsePack(recvStream);
}
@Test
public void testGetPeerAgentProtocolV0() throws Exception {
RevCommit one = remote.commit().message("1").create();

View File

@ -72,6 +72,8 @@ public final class FetchV2Request extends FetchRequest {
@NonNull
private final List<String> serverOptions;
private final boolean sidebandAll;
FetchV2Request(@NonNull List<ObjectId> peerHas,
@NonNull List<String> wantedRefs,
@NonNull Set<ObjectId> wantIds,
@ -79,7 +81,8 @@ public final class FetchV2Request extends FetchRequest {
@NonNull List<String> deepenNotRefs, int depth,
@NonNull FilterSpec filterSpec,
boolean doneReceived, @NonNull Set<String> clientCapabilities,
@Nullable String agent, @NonNull List<String> serverOptions) {
@Nullable String agent, @NonNull List<String> serverOptions,
boolean sidebandAll) {
super(wantIds, depth, clientShallowCommits, filterSpec,
clientCapabilities, deepenSince,
deepenNotRefs, agent);
@ -87,6 +90,7 @@ public final class FetchV2Request extends FetchRequest {
this.wantedRefs = requireNonNull(wantedRefs);
this.doneReceived = doneReceived;
this.serverOptions = requireNonNull(serverOptions);
this.sidebandAll = sidebandAll;
}
/**
@ -127,6 +131,13 @@ public List<String> getServerOptions() {
return serverOptions;
}
/**
* @return true if "sideband-all" was received
*/
boolean getSidebandAll() {
return sidebandAll;
}
/** @return A builder of {@link FetchV2Request}. */
static Builder builder() {
return new Builder();
@ -159,6 +170,8 @@ static final class Builder {
final List<String> serverOptions = new ArrayList<>();
boolean sidebandAll;
private Builder() {
}
@ -317,6 +330,15 @@ Builder addServerOption(@NonNull String value) {
return this;
}
/**
* @param value true if client sent "sideband-all"
* @return this builder
*/
Builder setSidebandAll(boolean value) {
sidebandAll = value;
return this;
}
/**
* @return Initialized fetch request
*/
@ -324,7 +346,8 @@ FetchV2Request build() {
return new FetchV2Request(peerHas, wantedRefs, wantIds,
clientShallowCommits, deepenSince, deepenNotRefs,
depth, filterSpec, doneReceived, clientCapabilities,
agent, Collections.unmodifiableList(serverOptions));
agent, Collections.unmodifiableList(serverOptions),
sidebandAll);
}
}
}

View File

@ -173,6 +173,14 @@ public final class GitProtocolConstants {
*/
public static final String OPTION_WANT_REF = "want-ref"; //$NON-NLS-1$
/**
* The client requested that the whole response be multiplexed, with
* each non-flush and non-delim pkt prefixed by a sideband designator.
*
* @since 5.5
*/
public static final String OPTION_SIDEBAND_ALL = "sideband-all"; //$NON-NLS-1$
/**
* The client supports atomic pushes. If this option is used, the server
* will update all refs within one atomic transaction.

View File

@ -74,6 +74,8 @@ public class PacketLineOut {
private boolean flushOnEnd;
private boolean useSideband;
/**
* Create a new packet line writer.
*
@ -97,6 +99,16 @@ public void setFlushOnEnd(boolean flushOnEnd) {
this.flushOnEnd = flushOnEnd;
}
/**
* When writing packet lines, use the first byte of each non-flush and
* non-delim packet as a sideband designator.
*
* @since 5.5
*/
public void useSidebandFormat() {
this.useSideband = true;
}
/**
* Write a UTF-8 encoded string as a single length-delimited packet.
*
@ -139,8 +151,14 @@ public void writePacket(byte[] packet) throws IOException {
* @since 4.5
*/
public void writePacket(byte[] buf, int pos, int len) throws IOException {
formatLength(len + 4);
out.write(lenbuffer, 0, 4);
if (useSideband) {
formatLength(len + 5);
out.write(lenbuffer, 0, 4);
out.write(1);
} else {
formatLength(len + 4);
out.write(lenbuffer, 0, 4);
}
out.write(buf, pos, len);
if (log.isDebugEnabled()) {
String s = RawParseUtils.decode(UTF_8, buf, pos, len);

View File

@ -49,6 +49,7 @@
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_NO_PROGRESS;
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_OFS_DELTA;
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SERVER_OPTION;
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SIDEBAND_ALL;
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SIDE_BAND_64K;
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_THIN_PACK;
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_WANT_REF;
@ -210,6 +211,9 @@ FetchV2Request parseFetchRequest(PacketLineIn pckIn)
filterReceived = true;
reqBuilder.setFilterSpec(FilterSpec.fromFilterLine(
line2.substring(OPTION_FILTER.length() + 1)));
} else if (transferConfig.isAllowSidebandAll()
&& line2.equals(OPTION_SIDEBAND_ALL)) {
reqBuilder.setSidebandAll(true);
} else {
throw new PackProtocolException(MessageFormat
.format(JGitText.get().unexpectedPacketLine, line2));

View File

@ -131,6 +131,7 @@ static ProtocolVersion parse(@Nullable String name) {
private final boolean allowTipSha1InWant;
private final boolean allowReachableSha1InWant;
private final boolean allowFilter;
private final boolean allowSidebandAll;
final @Nullable ProtocolVersion protocolVersion;
final String[] hideRefs;
@ -210,6 +211,8 @@ public TransferConfig(Config rc) {
"uploadpack", "allowfilter", false);
protocolVersion = ProtocolVersion.parse(rc.getString("protocol", null, "version"));
hideRefs = rc.getStringList("uploadpack", null, "hiderefs");
allowSidebandAll = rc.getBoolean(
"uploadpack", "allowsidebandall", false);
}
/**
@ -291,6 +294,14 @@ public boolean isAllowRefInWant() {
return allowRefInWant;
}
/**
* @return true if clients are allowed to specify a "sideband-all" line
* @since 5.5
*/
public boolean isAllowSidebandAll() {
return allowSidebandAll;
}
/**
* Get {@link org.eclipse.jgit.transport.RefFilter} respecting configured
* hidden refs.

View File

@ -61,6 +61,7 @@
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_NO_PROGRESS;
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_OFS_DELTA;
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SHALLOW;
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SIDEBAND_ALL;
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SIDE_BAND;
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SIDE_BAND_64K;
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_THIN_PACK;
@ -1122,6 +1123,10 @@ private void fetchV2(PacketLineOut pckOut) throws IOException {
protocolV2Hook.onFetch(req);
if (req.getSidebandAll()) {
pckOut.useSidebandFormat();
}
// TODO(ifrade): Refactor to pass around the Request object, instead of
// copying data back to class fields
List<ObjectId> deepenNots = new ArrayList<>();
@ -1263,6 +1268,7 @@ private List<String> getV2CapabilityAdvertisement() {
COMMAND_FETCH + '=' +
(transferConfig.isAllowFilter() ? OPTION_FILTER + ' ' : "") + //$NON-NLS-1$
(advertiseRefInWant ? CAPABILITY_REF_IN_WANT + ' ' : "") + //$NON-NLS-1$
(transferConfig.isAllowSidebandAll() ? OPTION_SIDEBAND_ALL + ' ' : "") + //$NON-NLS-1$
OPTION_SHALLOW);
caps.add(CAPABILITY_SERVER_OPTION);
return caps;