From 1dda2563575c76804d1477165a8e47ee65ca0890 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Mon, 17 Mar 2025 16:11:45 +0100 Subject: [PATCH 01/10] filter out inactive branch tags in Mercurial fixes #4741 --- .../indexer/history/MercurialRepository.java | 11 +++++++---- .../history/MercurialRepositoryTest.java | 19 ++++++++++++++++--- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java index 9c69d789efd..656e18f507e 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java @@ -18,7 +18,7 @@ */ /* - * Copyright (c) 2006, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2006, 2025, Oracle and/or its affiliates. All rights reserved. * Portions Copyright (c) 2017, 2019, Chris Fraire . */ package org.opengrok.indexer.history; @@ -634,12 +634,15 @@ protected void buildTagList(File directory, CommandTimeoutType cmdType) { ArrayList argv = new ArrayList<>(); ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK); argv.add(RepoCommand); - argv.add("tags"); + argv.add("log"); + argv.add("-b"); + argv.add(getBranch()); + argv.add("--rev=reverse(tag())"); 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 + // separator within the revision string (which is not used in this output but better // safe than sorry). - argv.add("{rev}|{tag}\\n"); + argv.add("{latesttag % \"{rev}|{tag}\\n\"}"); Executor executor = new Executor(argv, directory, RuntimeEnvironment.getInstance().getCommandTimeout(cmdType)); diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java index 0a80bd78592..5c6925b288b 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java @@ -18,7 +18,7 @@ */ /* - * Copyright (c) 2009, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2009, 2025, Oracle and/or its affiliates. All rights reserved. * Portions Copyright (c) 2017, Chris Fraire . */ package org.opengrok.indexer.history; @@ -483,8 +483,9 @@ void testBuildTagListInitial() throws Exception { } /** - * Clone the original repository, add new tag, check that the extracted tags contain the pre-existing - * and new one. + * Clone the original repository, create branch and add tag to the branch, switch back to the original branch, + * add new tag, check that the extracted tags contain the pre-existing and new one + * but not the non-default branch tag. */ @Test void testBuildTagListOneMore() throws Exception { @@ -494,6 +495,18 @@ void testBuildTagListOneMore() throws Exception { // 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()); + + // Branch the repo and add one changeset. + runHgCommand(repositoryRoot, "unbundle", + Paths.get(getClass().getResource("/history/hg-branch.bundle").toURI()).toString()); + // Switch to the branch. + runHgCommand(repositoryRoot, "update", "mybranch"); + final String branchTagName = "branch_tag"; + runHgCommand(repositoryRoot, "tag", branchTagName); + + // Switch back to the default branch. + runHgCommand(repositoryRoot, "update", "default"); + MercurialRepository hgRepo = (MercurialRepository) RepositoryFactory.getRepository(repositoryRoot); assertNotNull(hgRepo); // Using double space on purpose to test the parsing of tags. From a11f279995c7e825714e2851ada29a508299e8cc Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Mon, 17 Mar 2025 16:27:18 +0100 Subject: [PATCH 02/10] adjust comment --- .../org/opengrok/indexer/history/MercurialRepositoryTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java index 5c6925b288b..b13435b8050 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java @@ -499,7 +499,7 @@ void testBuildTagListOneMore() throws Exception { // Branch the repo and add one changeset. runHgCommand(repositoryRoot, "unbundle", Paths.get(getClass().getResource("/history/hg-branch.bundle").toURI()).toString()); - // Switch to the branch. + // Switch to the branch and add tag. runHgCommand(repositoryRoot, "update", "mybranch"); final String branchTagName = "branch_tag"; runHgCommand(repositoryRoot, "tag", branchTagName); From c9406fdd0948e8583d7abac8297573fbecb172f1 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Mon, 17 Mar 2025 17:02:19 +0100 Subject: [PATCH 03/10] use longopt properly --- .../java/org/opengrok/indexer/history/MercurialRepository.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java index 656e18f507e..236e668658d 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java @@ -638,11 +638,10 @@ protected void buildTagList(File directory, CommandTimeoutType cmdType) { argv.add("-b"); argv.add(getBranch()); argv.add("--rev=reverse(tag())"); - 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 this output but better // safe than sorry). - argv.add("{latesttag % \"{rev}|{tag}\\n\"}"); + argv.add("--template={latesttag % \"{rev}|{tag}\\n\"}"); Executor executor = new Executor(argv, directory, RuntimeEnvironment.getInstance().getCommandTimeout(cmdType)); From 27c9fff96e53c64aacd138673a7bb7538461b4f5 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Mon, 17 Mar 2025 17:17:44 +0100 Subject: [PATCH 04/10] try adding backslash --- .../java/org/opengrok/indexer/history/MercurialRepository.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java index 236e668658d..a15b40e76f6 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java @@ -641,7 +641,7 @@ protected void buildTagList(File directory, CommandTimeoutType cmdType) { // Use '|' as a revision separator rather than ':' to avoid collision with the commonly used // separator within the revision string (which is not used in this output but better // safe than sorry). - argv.add("--template={latesttag % \"{rev}|{tag}\\n\"}"); + argv.add("--template={latesttag % \\\"{rev}|{tag}\\n\\\"}"); Executor executor = new Executor(argv, directory, RuntimeEnvironment.getInstance().getCommandTimeout(cmdType)); From 3ede8521a3d863661c86a352f5fb0b0aa68a2dcb Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Mon, 17 Mar 2025 17:22:49 +0100 Subject: [PATCH 05/10] use single quotes --- .../java/org/opengrok/indexer/history/MercurialRepository.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java index a15b40e76f6..33a8a4aba7f 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java @@ -641,7 +641,7 @@ protected void buildTagList(File directory, CommandTimeoutType cmdType) { // Use '|' as a revision separator rather than ':' to avoid collision with the commonly used // separator within the revision string (which is not used in this output but better // safe than sorry). - argv.add("--template={latesttag % \\\"{rev}|{tag}\\n\\\"}"); + argv.add("--template={latesttag % '{rev}|{tag}\\n'}"); Executor executor = new Executor(argv, directory, RuntimeEnvironment.getInstance().getCommandTimeout(cmdType)); From 86b5ae5f060f4149f950f62a6c761c43beb7a866 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Mon, 17 Mar 2025 18:44:19 +0100 Subject: [PATCH 06/10] include tags from the parent branch --- .../java/org/opengrok/indexer/history/MercurialRepository.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java index 33a8a4aba7f..1fe8d8b76fa 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java @@ -637,7 +637,7 @@ protected void buildTagList(File directory, CommandTimeoutType cmdType) { argv.add("log"); argv.add("-b"); argv.add(getBranch()); - argv.add("--rev=reverse(tag())"); + argv.add("--rev=reverse(0::branch(" + this.getBranch() + ") and tag())"); // Use '|' as a revision separator rather than ':' to avoid collision with the commonly used // separator within the revision string (which is not used in this output but better // safe than sorry). From 09ea7a144f592ae1508e8788d211a7d908ca64ec Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Mon, 17 Mar 2025 18:45:42 +0100 Subject: [PATCH 07/10] drop the -b option --- .../java/org/opengrok/indexer/history/MercurialRepository.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java index 1fe8d8b76fa..cbedf781c7f 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java @@ -635,8 +635,6 @@ protected void buildTagList(File directory, CommandTimeoutType cmdType) { ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK); argv.add(RepoCommand); argv.add("log"); - argv.add("-b"); - argv.add(getBranch()); argv.add("--rev=reverse(0::branch(" + this.getBranch() + ") and tag())"); // Use '|' as a revision separator rather than ':' to avoid collision with the commonly used // separator within the revision string (which is not used in this output but better From 1a399c0f158d76ce9e10f7d9612824ea08e7b66f Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Mon, 17 Mar 2025 21:53:47 +0100 Subject: [PATCH 08/10] add test for non-default branch tags --- .../history/MercurialRepositoryTest.java | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java index b13435b8050..3e51df93b40 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java @@ -499,8 +499,10 @@ void testBuildTagListOneMore() throws Exception { // Branch the repo and add one changeset. runHgCommand(repositoryRoot, "unbundle", Paths.get(getClass().getResource("/history/hg-branch.bundle").toURI()).toString()); + // Switch to the branch and add tag. - runHgCommand(repositoryRoot, "update", "mybranch"); + final String myBranch = "mybranch"; + runHgCommand(repositoryRoot, "update", myBranch); final String branchTagName = "branch_tag"; runHgCommand(repositoryRoot, "tag", branchTagName); @@ -509,7 +511,8 @@ void testBuildTagListOneMore() throws Exception { MercurialRepository hgRepo = (MercurialRepository) RepositoryFactory.getRepository(repositoryRoot); assertNotNull(hgRepo); - // Using double space on purpose to test the parsing of tags. + + // Add tag. 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); @@ -522,6 +525,23 @@ void testBuildTagListOneMore() throws Exception { assertEquals(List.of(7, 9), tags.stream().map(TagEntry::getRevision).collect(Collectors.toList())); List expectedTags = List.of("start_of_novel", newTagName); assertEquals(expectedTags, tags.stream().map(TagEntry::getTags).collect(Collectors.toList())); + + // Add another tag to the default branch. + runHgCommand(repositoryRoot, "tag", "another_tag"); + + // Switch back to the non-default branch, check tags. + runHgCommand(repositoryRoot, "update", myBranch); + // The repository object has to be recreated to reflect the branch switch. + hgRepo = (MercurialRepository) RepositoryFactory.getRepository(repositoryRoot); + assertNotNull(hgRepo); + hgRepo.buildTagList(new File(hgRepo.getDirectoryName()), CommandTimeoutType.INDEXER); + tags = hgRepo.getTagList(); + assertNotNull(tags); + assertEquals(3, tags.size()); + expectedTags = List.of("start_of_novel", newTagName, branchTagName); + assertEquals(expectedTags, tags.stream().map(TagEntry::getTags).collect(Collectors.toList())); + + // cleanup IOUtils.removeRecursive(repositoryRootPath); } } From a061d3bf7b52e4422f193b5c1f36c643eb0b8715 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Mon, 17 Mar 2025 22:54:07 +0100 Subject: [PATCH 09/10] adjust top-level comment, add branch assert --- .../indexer/history/MercurialRepositoryTest.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java index 3e51df93b40..b8d8290ae92 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java @@ -483,9 +483,14 @@ void testBuildTagListInitial() throws Exception { } /** - * Clone the original repository, create branch and add tag to the branch, switch back to the original branch, - * add new tag, check that the extracted tags contain the pre-existing and new one - * but not the non-default branch tag. + * 1. Clone the original repository + * 2. create branch and add tag to the branch + * 3. switch back to the original branch, add new tag + * 4. check that the extracted tags contain the pre-existing and new one but not the non-default branch tag. + * 5. add another tag + * 6. switch to the non-default branch + * 7. check that the extracted tags consist of the tags added to the default branch before the branch point + * and also the tags added in that branch */ @Test void testBuildTagListOneMore() throws Exception { @@ -534,6 +539,7 @@ void testBuildTagListOneMore() throws Exception { // The repository object has to be recreated to reflect the branch switch. hgRepo = (MercurialRepository) RepositoryFactory.getRepository(repositoryRoot); assertNotNull(hgRepo); + assertEquals(myBranch, hgRepo.getBranch()); hgRepo.buildTagList(new File(hgRepo.getDirectoryName()), CommandTimeoutType.INDEXER); tags = hgRepo.getTagList(); assertNotNull(tags); From 2d0ff2667a532f8337eab7f22eb4f0cfc9763041 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Mon, 17 Mar 2025 22:56:05 +0100 Subject: [PATCH 10/10] add one more branch assert --- .../org/opengrok/indexer/history/MercurialRepositoryTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java index b8d8290ae92..f82e544826d 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java @@ -516,6 +516,7 @@ void testBuildTagListOneMore() throws Exception { MercurialRepository hgRepo = (MercurialRepository) RepositoryFactory.getRepository(repositoryRoot); assertNotNull(hgRepo); + assertEquals("default", hgRepo.getBranch()); // Add tag. Using double space on purpose to test the parsing of tags. final String newTagName = "foo bar";