handle tags with multiple spaces

Author: vladak

opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java

@@ -635,6 +635,11 @@ protected void buildTagList(File directory, CommandTimeoutType cmdType) {
         ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK);
         argv.add(RepoCommand);
         argv.add("tags");
+        argv.add("--template");
+        // Use '|' as a revision separator rather than ':' to avoid collision with the commonly used
+        // separator within the revision string (which is not used in the 'hg tags' output but better
+        // safe than sorry).
+        argv.add("{rev}|{tag}\\n");
 
         Executor executor = new Executor(argv, directory,
                 RuntimeEnvironment.getInstance().getCommandTimeout(cmdType));

opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialTagParser.java

@@ -26,6 +26,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import java.util.NavigableSet;
 import java.util.TreeSet;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -49,7 +50,7 @@ public class MercurialTagParser implements Executor.StreamHandler {
      *
      * @return entries a set of tag entries
      */
-    public TreeSet<TagEntry> getEntries() {
+    public NavigableSet<TagEntry> getEntries() {
         return entries;
     }
 
@@ -59,35 +60,23 @@ public void processStream(InputStream input) throws IOException {
             try (BufferedReader in = new BufferedReader(new InputStreamReader(input))) {
                 String line;
                 while ((line = in.readLine()) != null) {
-                    String[] parts = line.split("  *");
+                    String[] parts = line.split("\\|");
                     if (parts.length < 2) {
                         LOGGER.log(Level.WARNING,
                                 "Failed to parse tag list: {0}",
                                 "Tag line contains more than 2 columns: " + line);
                         entries = null;
                         break;
                     }
-                    // Grrr, how to parse tags with spaces inside?
-                    // This solution will lose multiple spaces ;-/
-                    String tag = parts[0];
-                    for (int i = 1; i < parts.length - 1; ++i) {
-                        tag = tag.concat(" ");
-                        tag = tag.concat(parts[i]);
-                    }
+                    String rev = parts[0];
+                    String tag = parts[1];
+
                     // The implicit 'tip' tag only causes confusion so ignore it.
                     if (tag.contentEquals("tip")) {
                         continue;
                     }
-                    String[] revParts = parts[parts.length - 1].split(":");
-                    if (revParts.length != 2) {
-                        LOGGER.log(Level.WARNING,
-                                "Failed to parse tag list: {0}",
-                                "Mercurial revision parsing error: "
-                                        + parts[parts.length - 1]);
-                        entries = null;
-                        break;
-                    }
-                    TagEntry tagEntry = new MercurialTagEntry(Integer.parseInt(revParts[0]), tag);
+
+                    TagEntry tagEntry = new MercurialTagEntry(Integer.parseInt(rev), tag);
                     // Reverse the order of the list.
                     entries.add(tagEntry);
                 }

opengrok-indexer/src/main/java/org/opengrok/indexer/history/TagEntry.java

@@ -83,6 +83,10 @@ protected TagEntry(Date date, String tags) {
         this.tags = tags;
     }
 
+    public int getRevision() {
+        return revision;
+    }
+
     public String getTags() {
         return this.tags;
     }

opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java

@@ -481,17 +481,24 @@ void testBuildTagListOneMore() throws Exception {
         Path repositoryRootPath = Files.createDirectory(Path.of(RuntimeEnvironment.getInstance().getSourceRootPath(),
                 "addedTagTest"));
         File repositoryRoot = repositoryRootPath.toFile();
+        // Clone the internal repository because it will be modified.
+        // This avoids interference with other tests in this class.
         runHgCommand(this.repositoryRoot, "clone", this.repositoryRoot.toString(), repositoryRootPath.toString());
         MercurialRepository hgRepo = (MercurialRepository) RepositoryFactory.getRepository(repositoryRoot);
         assertNotNull(hgRepo);
-        runHgCommand(repositoryRoot, "tag", "foo");
+        // Using double space on purpose to test the parsing of tags.
+        final String newTagName = "foo  bar";
+        runHgCommand(repositoryRoot, "tag", newTagName);
         hgRepo.buildTagList(new File(hgRepo.getDirectoryName()), CommandTimeoutType.INDEXER);
         var tags = hgRepo.getTagList();
         assertNotNull(tags);
         assertEquals(2, tags.size());
-        Set<TagEntry> expectedTags = Set.of(new MercurialTagEntry(7, "start_of_novel"),
-                new MercurialTagEntry(9, "foo"));
-        assertEquals(expectedTags, tags);
+        // TagEntry has special semantics for comparing/equality which does not compare the tags at all,
+        // so using assertEquals() on two sets of TagEntry objects would not help.
+        // Instead, check the tags separately.
+        assertEquals(List.of(7, 9), tags.stream().map(TagEntry::getRevision).collect(Collectors.toList()));
+        List<String> expectedTags = List.of("start_of_novel", newTagName);
+        assertEquals(expectedTags, tags.stream().map(TagEntry::getTags).collect(Collectors.toList()));
         IOUtils.removeRecursive(repositoryRootPath);
     }
 }