Execute AdvertiseRefsHook only for protocol v0 and v1

Refs are not advertised as part of the protocol v2 capability
advertisement. Don't call AdvertiseRefsHook.

Noticed because many implementations of AdvertiseRefsHook read all
refs in order to call UploadPack#setAdvertisedRefs, causing the
capability advertisement to be as slow as a v0 ref advertisement with
some RefDatabase implementations.

Such an AdvertiseRefsHook is of dubious utility (a better place to
determine which refs are advertised is in the RefDatabase
implementation itself, as in Gerrit), but at any rate since it's not
bringing about any benefit here, we can skip the hook call.

TODO:
- call an appropriate hook instead (https://bugs.eclipse.org/534847)
- add tests

[jn: fleshed out commit message; added TODO notes]

Change-Id: I6eb60ccfb251a45432954467a9ae9c1079a8c8b5
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>

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 a1c222b19c427b2e8eb249f522fa8954e5835b73..3d4d279a51441ee6e3faf17421ab4406ccc6e4f5 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -1153,16 +1153,6 @@ public class UploadPack {
         */
        public void sendAdvertisedRefs(RefAdvertiser adv) throws IOException,
                        ServiceMayNotContinueException {
-               try {
-                       advertiseRefsHook.advertiseRefs(this);
-               } catch (ServiceMayNotContinueException fail) {
-                       if (fail.getMessage() != null) {
-                               adv.writeOne("ERR " + fail.getMessage()); //$NON-NLS-1$
-                               fail.setOutput();
-                       }
-                       throw fail;
-               }
-
                if (useProtocolV2()) {
                        // The equivalent in v2 is only the capabilities
                        // advertisement.
@@ -1173,6 +1163,16 @@ public class UploadPack {
                        return;
                }
 
+               try {
+                       advertiseRefsHook.advertiseRefs(this);
+               } catch (ServiceMayNotContinueException fail) {
+                       if (fail.getMessage() != null) {
+                               adv.writeOne("ERR " + fail.getMessage()); //$NON-NLS-1$
+                               fail.setOutput();
+                       }
+                       throw fail;
+               }
+
                adv.init(db);
                adv.advertiseCapability(OPTION_INCLUDE_TAG);
                adv.advertiseCapability(OPTION_MULTI_ACK_DETAILED);