[JENKINS-40834] Probably safest to let the consumer of the API decide…

… what to do if the is no symbolc ref support
jenkinsci · Mar 22, 2017 · c5edec5 · c5edec5
1 parent 2cb2fc1
commit c5edec5
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 48 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java b/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java
@@ -2614,27 +2614,29 @@ public Map<String, ObjectId> getRemoteReferences(String url, String pattern, boo
     @Override
     public Map<String, String> getRemoteSymbolicReferences(String url, String pattern)
             throws GitException, InterruptedException {
-        ArgumentListBuilder args = new ArgumentListBuilder("ls-remote");
-        args.add("--symref");
-        args.add(url);
-        if (pattern != null) {
-            args.add(pattern);
-        }
+        Map<String, String> references = new HashMap<>();
+        if (isAtLeastVersion(2, 8, 0, 0)) {
+            // --symref is only understood by ls-remote starting from git 2.8.0
+            // https://github.com/git/git/blob/afd6726309/Documentation/RelNotes/2.8.0.txt#L72-L73
+            ArgumentListBuilder args = new ArgumentListBuilder("ls-remote");
+            args.add("--symref");
+            args.add(url);
+            if (pattern != null) {
+                args.add(pattern);
+            }
 
-        StandardCredentials cred = credentials.get(url);
-        if (cred == null) cred = defaultCredentials;
+            StandardCredentials cred = credentials.get(url);
+            if (cred == null) cred = defaultCredentials;
 
-        String result = launchCommandWithCredentials(args, null, cred, url);
+            String result = launchCommandWithCredentials(args, null, cred, url);
 
-        Map<String, String> references = new HashMap<>();
-        String[] lines = result.split("\n");
-        Pattern symRefPattern = Pattern.compile("^ref:\\s+([^ ]+)\\s+([^ ]+)$");
-        for (String line : lines) {
-            Matcher matcher = symRefPattern.matcher(line);
-            if (matcher.matches()) {
-                references.put(matcher.group(2), matcher.group(1));
-            } else  if (line.length() < 41) {
-                throw new GitException("unexpected ls-remote output " + line);
+            String[] lines = result.split("\n");
+            Pattern symRefPattern = Pattern.compile("^ref:\\s+([^ ]+)\\s+([^ ]+)$");
+            for (String line : lines) {
+                Matcher matcher = symRefPattern.matcher(line);
+                if (matcher.matches()) {
+                    references.put(matcher.group(2), matcher.group(1));
+                }
             }
         }
         return references;

diff --git a/src/main/java/org/jenkinsci/plugins/gitclient/GitClient.java b/src/main/java/org/jenkinsci/plugins/gitclient/GitClient.java
@@ -621,11 +621,13 @@ public interface GitClient {
 
     /**
      * List symbolic references in a remote repository. Equivalent to <tt>git ls-remote --symref &lt;repository&gt;
-     * [&lt;refs&gt;]</tt>.
+     * [&lt;refs&gt;]</tt>. Note: the response may be empty for multiple reasons
      *
      * @param remoteRepoUrl Remote repository URL.
      * @param pattern       Only references matching the given pattern are displayed.
-     * @return a map of references name and its underlying reference. Empty if none.
+     * @return a map of references name and its underlying reference. Empty if none or if the remote does not report
+     * symbolic references (i.e. Git 1.8.4 or earlier) or if the client does not support reporting symbolic references
+     * (e.g. command line Git prior to 2.8.0).
      * @throws hudson.plugins.git.GitException if underlying git operation fails.
      * @throws java.lang.InterruptedException  if interrupted.
      */

diff --git a/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java b/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java
@@ -840,38 +840,12 @@ public Map<String, String> getRemoteSymbolicReferences(String url, String patter
                             }
                         }
                     }
-                } catch (URISyntaxException | NotSupportedException | TransportException e) {
                     // ignore this is a total hack
                 }
             } catch (IllegalAccessException | NoSuchFieldException e) {
-                // ignore, we just have to try it the Git 1.8.4 way
+                // ignore, caller will just have to try it the Git 1.8.4 way, we'll return an empty map
             }
-
-            LsRemoteCommand lsRemote = new LsRemoteCommand(repo);
-            lsRemote.setRemote(url);
-            lsRemote.setCredentialsProvider(getProvider());
-            Map<String, Ref> refs = lsRemote.callAsMap();
-
-            Ref target = refs.get(Constants.HEAD);
-            if (target == null) {
-                return references;
-            }
-            Set<String> candidates = new HashSet<>();
-            for (Map.Entry<String, Ref> entry: refs.entrySet()) {
-                if (entry.getValue() == target) {
-                    continue;
-                }
-                if (entry.getValue().getObjectId().equals(target.getObjectId())) {
-                    candidates.add(entry.getKey());
-                }
-            }
-            if (candidates.size() == 1) {
-                references.put(Constants.HEAD, candidates.iterator().next());
-            } else if (candidates.contains(Constants.R_HEADS+Constants.MASTER)) {
-                // if multiple heads have the same object ID, git 1.8.4 and earlier would give priority to master
-                references.put(Constants.HEAD, Constants.R_HEADS + Constants.MASTER);
-            } // else we have an inconclusive resolution
-        } catch (GitAPIException | IOException e) {
+        } catch (IOException | URISyntaxException e) {
             throw new GitException(e);
         }
         return references;