sanitize ciworkspace instead of using Path

DataDog · daniel-mohedano · May 9, 2025 · May 8, 2025 · May 8, 2025 · May 8, 2025
commit 9cc396e039b4c69e1ed66a0b3b032d3f2b6b1d7e
@@ -35,7 +35,6 @@
 import datadog.trace.civisibility.source.index.RepoIndexProvider;
 import datadog.trace.civisibility.source.index.RepoIndexSourcePathResolver;
 import datadog.trace.util.Strings;
-import java.io.File;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Map;
@@ -72,7 +71,7 @@ public class CiVisibilityRepoServices {
       LOGGER.info("PR detected: {}", pullRequestInfo);
     }
 
-    repoRoot = appendSlashIfNeeded(getRepoRoot(ciInfo, services.gitClientFactory));
+    repoRoot = getRepoRoot(ciInfo, services.gitClientFactory);
     moduleName = getModuleName(services.config, repoRoot, path);
     ciTags = new CITagsProvider().getCiTags(ciInfo, pullRequestInfo);
 
@@ -126,7 +125,7 @@ private static PullRequestInfo buildPullRequestInfo(
   }
 
   private static String getRepoRoot(CIInfo ciInfo, GitClient.Factory gitClientFactory) {
-    String ciWorkspace = ciInfo.getNormalizedCiWorkspace();
+    String ciWorkspace = ciInfo.getCiWorkspace();
     if (Strings.isNotBlank(ciWorkspace)) {
       return ciWorkspace;
 
@@ -146,14 +145,6 @@ private static String getRepoRoot(CIInfo ciInfo, GitClient.Factory gitClientFact
     }
   }
 
-  private static String appendSlashIfNeeded(String repoRoot) {
-    if (repoRoot != null && !repoRoot.endsWith(File.separator)) {
-      return repoRoot + File.separator;
-    } else {
-      return repoRoot;
-    }
-  }
-
   static String getModuleName(Config config, @Nullable String repoRoot, Path path) {
     // if parent process is instrumented, it will provide build system's module name
     String parentModuleName = config.getCiVisibilityModuleName();

@@ -210,19 +210,13 @@ public String getCiJobUrl() {
   }
 
   /**
-   * @deprecated This method is here only to satisfy CI spec tests. Use {@link
-   *     #getNormalizedCiWorkspace()}
+   * @return Workspace path without the trailing separator
    */
-  @Deprecated
   public String getCiWorkspace() {
-    return ciWorkspace;
-  }
-
-  public String getNormalizedCiWorkspace() {
     String realCiWorkspace = FileUtils.toRealPath(ciWorkspace);
-    return (realCiWorkspace == null || realCiWorkspace.endsWith(File.separator))
+    return (realCiWorkspace == null || !realCiWorkspace.endsWith(File.separator))
         ? realCiWorkspace
-        : (realCiWorkspace + File.separator);
+        : (realCiWorkspace.substring(0, realCiWorkspace.length() - 1));
   }
 
   public String getCiNodeName() {

@@ -22,7 +22,7 @@ public CITagsProvider() {
   }
 
   public Map<String, String> getCiTags(CIInfo ciInfo, PullRequestInfo pullRequestInfo) {
-    String repoRoot = ciInfo.getNormalizedCiWorkspace();
+    String repoRoot = ciInfo.getCiWorkspace();
     GitInfo gitInfo = gitInfoProvider.getGitInfo(repoRoot);
 
     return new CITagsBuilder()

@@ -1,6 +1,7 @@
 package datadog.trace.civisibility.source;
 
 import datadog.compiler.utils.CompilerUtils;
+import java.io.File;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
@@ -9,7 +10,7 @@ public class CompilerAidedSourcePathResolver implements SourcePathResolver {
   private final String repoRoot;
 
   public CompilerAidedSourcePathResolver(String repoRoot) {
-    this.repoRoot = repoRoot;
+    this.repoRoot = repoRoot.endsWith(File.separator) ? repoRoot : repoRoot + File.separator;
   }
 
   @Nullable

@@ -8,7 +8,7 @@ import java.nio.file.Files
 
 class CodeownersProviderTest extends Specification {
 
-  private static final String REPO_ROOT = "/repo/root/"
+  private static final String REPO_ROOT = "/repo/root"
 
   def "test codeowners loading: #path"() {
     setup:
@@ -30,10 +30,10 @@ class CodeownersProviderTest extends Specification {
 
     where:
     path << [
-      REPO_ROOT + "CODEOWNERS",
-      REPO_ROOT + ".github/CODEOWNERS",
-      REPO_ROOT + ".gitlab/CODEOWNERS",
-      REPO_ROOT + "docs/CODEOWNERS"
+      REPO_ROOT + "/CODEOWNERS",
+      REPO_ROOT + "/.github/CODEOWNERS",
+      REPO_ROOT + "/.gitlab/CODEOWNERS",
+      REPO_ROOT + "/docs/CODEOWNERS"
     ]
   }
 }
@@ -5,7 +5,7 @@ import spock.lang.Specification
 
 class CompilerAidedSourcePathResolverTest extends Specification {
 
-  public static final String REPO_ROOT = "/repo/root/"
+  public static final String REPO_ROOT = "/repo/root"
   public static final String SOURCE_PATH_VALUE = "/repo/root/path/to/AClassWithSourceInfoInjected.java"
   public static final String SOURCE_PATH_OUTSIDE_REPO_VALUE = "/outside/path/to/AClassWithSourceInfoInjected.java"
 

@@ -57,7 +57,7 @@ public GitInfo getGitInfo(@Nullable String repositoryPath) {
     }
 
     // normalize path to avoid creating two entries in the cache
-    return gitInfoCache.computeIfAbsent(Paths.get(repositoryPath).toString(), this::buildGitInfo);
+    return gitInfoCache.computeIfAbsent(repositoryPath, this::buildGitInfo);
   }
 
   private GitInfo buildGitInfo(String repositoryPath) {

diff --git a/internal-api/src/test/groovy/datadog/trace/api/git/GitInfoProviderTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/git/GitInfoProviderTest.groovy
@@ -341,25 +341,6 @@ class GitInfoProviderTest extends Specification {
     actualGitInfo.repositoryURL == "http://usefulUrl"
   }
 
-  def "test caches git info regardless of path normalization"() {
-    setup:
-    def gitInfo = new GitInfo("repoUrl", "branch", "tag", new CommitInfo("sha"))
-    def gitInfoBuilder = Mock(GitInfoBuilder)
-    gitInfoBuilder.order() >> 1
-    gitInfoBuilder.providerAsExpected() >> GitProviderExpected.USER_SUPPLIED
-    gitInfoBuilder.providerAsDiscrepant() >> GitProviderDiscrepant.USER_SUPPLIED
-
-    def gitInfoProvider = new GitInfoProvider()
-    gitInfoProvider.registerGitInfoBuilder(gitInfoBuilder)
-
-    when:
-    gitInfoProvider.getGitInfo(REPO_PATH)
-    gitInfoProvider.getGitInfo(REPO_PATH + File.separator)
-
-    then:
-    1 * gitInfoBuilder.build(REPO_PATH) >> gitInfo
-  }
-
   private GitInfoBuilder givenABuilderReturning(GitInfo gitInfo) {
     givenABuilderReturning(gitInfo, 1)
   }