refactor(Repository): Revert changes to nestedRepositories

As was pointed out during the code review [1], there is now need to change `nestedRepositories` to support `Provenance`s, as only `git` is supported for `nestedRepositories`, which always contains `vcsInfo`. In order to reduce the amount of changes for this PR, any changes regarding `nestedRepositories` are now reverted. [1] oss-review-toolkit#8764 (comment) Signed-off-by: Jens Keim <[email protected]>
pepper-jk · Jul 1, 2024 · f32bdf5 · f32bdf5
1 parent 665ab20
commit f32bdf5
Show file tree

Hide file tree

Showing 16 changed files with 90 additions and 127 deletions.
diff --git a/analyzer/src/main/kotlin/Analyzer.kt b/analyzer/src/main/kotlin/Analyzer.kt
@@ -143,9 +143,7 @@ class Analyzer(private val config: AnalyzerConfiguration, private val labels: Ma
         }.orEmpty()
         val repository = Repository(
             provenance = RepositoryProvenance(vcs, revision),
-            nestedRepositories = nestedVcs.map {
-                it.key to RepositoryProvenance(it.value, it.value.revision)
-            }.toMap(),
+            nestedRepositories = nestedVcs,
             config = info.repositoryConfiguration
         )
 

diff --git a/cli/src/funTest/assets/git-repo-expected-output.yml b/cli/src/funTest/assets/git-repo-expected-output.yml
@@ -9,47 +9,35 @@ repository:
     resolved_revision: "31588aa8f8555474e1c3c66a359ec99e4cd4b1fa"
   nested_repositories:
     spdx-tools:
-      vcs_info:
-        type: "Git"
-        url: "https://github.com/spdx/tools"
-        revision: "e179fae47590eccedc46186ea0ce20cbade5fda7"
-        path: ""
-      resolved_revision: "e179fae47590eccedc46186ea0ce20cbade5fda7"
+      type: "Git"
+      url: "https://github.com/spdx/tools"
+      revision: "e179fae47590eccedc46186ea0ce20cbade5fda7"
+      path: ""
     submodules:
-      vcs_info:
-        type: "Git"
-        url: "https://github.com/oss-review-toolkit/ort-test-data-git-submodules"
-        revision: "fcea94bab5835172e826afddb9f6427274c983b9"
-        path: ""
-      resolved_revision: "fcea94bab5835172e826afddb9f6427274c983b9"
+      type: "Git"
+      url: "https://github.com/oss-review-toolkit/ort-test-data-git-submodules"
+      revision: "fcea94bab5835172e826afddb9f6427274c983b9"
+      path: ""
     submodules/commons-text:
-      vcs_info:
-        type: "Git"
-        url: "https://github.com/apache/commons-text.git"
-        revision: "7643b12421100d29fd2b78053e77bcb04a251b2e"
-        path: ""
-      resolved_revision: "7643b12421100d29fd2b78053e77bcb04a251b2e"
+      type: "Git"
+      url: "https://github.com/apache/commons-text.git"
+      revision: "7643b12421100d29fd2b78053e77bcb04a251b2e"
+      path: ""
     submodules/test-data-npm:
-      vcs_info:
-        type: "Git"
-        url: "https://github.com/oss-review-toolkit/ort-test-data-npm.git"
-        revision: "ad0367b7b9920144a47b8d30cc0c84cea102b821"
-        path: ""
-      resolved_revision: "ad0367b7b9920144a47b8d30cc0c84cea102b821"
+      type: "Git"
+      url: "https://github.com/oss-review-toolkit/ort-test-data-npm.git"
+      revision: "ad0367b7b9920144a47b8d30cc0c84cea102b821"
+      path: ""
     submodules/test-data-npm/isarray:
-      vcs_info:
-        type: "Git"
-        url: "https://github.com/juliangruber/isarray.git"
-        revision: "63ea4ca0a0d6b0574d6a470ebd26880c3026db4a"
-        path: ""
-      resolved_revision: "63ea4ca0a0d6b0574d6a470ebd26880c3026db4a"
+      type: "Git"
+      url: "https://github.com/juliangruber/isarray.git"
+      revision: "63ea4ca0a0d6b0574d6a470ebd26880c3026db4a"
+      path: ""
     submodules/test-data-npm/long.js:
-      vcs_info:
-        type: "Git"
-        url: "https://github.com/dcodeIO/long.js.git"
-        revision: "941c5c62471168b5d18153755c2a7b38d2560e58"
-        path: ""
-      resolved_revision: "941c5c62471168b5d18153755c2a7b38d2560e58"
+      type: "Git"
+      url: "https://github.com/dcodeIO/long.js.git"
+      revision: "941c5c62471168b5d18153755c2a7b38d2560e58"
+      path: ""
   config: {}
 analyzer:
   start_time: "1970-01-01T00:00:00Z"

diff --git a/helper-cli/src/main/kotlin/utils/Extensions.kt b/helper-cli/src/main/kotlin/utils/Extensions.kt
@@ -693,10 +693,8 @@ internal fun OrtResult.getScanResultFor(packageConfiguration: PackageConfigurati
 internal fun OrtResult.getRepositoryPaths(): Map<String, Set<String>> {
     val result = mutableMapOf(repository.vcsProcessed.url to mutableSetOf(""))
 
-    repository.nestedRepositories.mapValues { (path, provenance) ->
-        if (provenance is RepositoryProvenance) {
-            result.getOrPut(provenance.vcsInfo.url) { mutableSetOf() } += path
-        }
+    repository.nestedRepositories.mapValues { (path, vcsInfo) ->
+        result.getOrPut(vcsInfo.url) { mutableSetOf() } += path
     }
 
     // For some Git-repo projects `OrtResult.repository.nestedRepositories´ misses some nested repositories for Git

diff --git a/helper-cli/src/main/kotlin/utils/Utils.kt b/helper-cli/src/main/kotlin/utils/Utils.kt
@@ -25,10 +25,8 @@ import java.io.File
 
 import org.ossreviewtoolkit.downloader.VersionControlSystem
 import org.ossreviewtoolkit.model.Identifier
-import org.ossreviewtoolkit.model.KnownProvenance
 import org.ossreviewtoolkit.model.OrtResult
 import org.ossreviewtoolkit.model.PackageCuration
-import org.ossreviewtoolkit.model.RepositoryProvenance
 import org.ossreviewtoolkit.model.VcsInfo
 import org.ossreviewtoolkit.model.config.LicenseFindingCuration
 import org.ossreviewtoolkit.model.config.PathExclude
@@ -70,10 +68,8 @@ internal fun findRepositoryPaths(directory: File): Map<String, Set<String>> {
 
     val result = mutableMapOf<String, MutableSet<String>>()
 
-    findRepositories(directory).forEach { (path, provenance) ->
-        if (provenance is RepositoryProvenance) {
-            result.getOrPut(provenance.vcsInfo.url.replaceCredentialsInUri()) { mutableSetOf() } += path
-        }
+    findRepositories(directory).forEach { (path, vcs) ->
+        result.getOrPut(vcs.url.replaceCredentialsInUri()) { mutableSetOf() } += path
     }
 
     return result
@@ -83,17 +79,14 @@ internal fun findRepositoryPaths(directory: File): Map<String, Set<String>> {
  * Search the given [directory] for repositories and return a mapping from paths where each respective repository was
  * found to the corresponding [VcsInfo].
  */
-internal fun findRepositories(directory: File): Map<String, KnownProvenance> {
+internal fun findRepositories(directory: File): Map<String, VcsInfo> {
     require(directory.isDirectory)
 
     val workingTree = VersionControlSystem.forDirectory(directory)
-    val nestedVcs = workingTree?.getNested()?.filter { (path, _) ->
+    return workingTree?.getNested()?.filter { (path, _) ->
         // Only include nested VCS if they are part of the analyzed directory.
         workingTree.getRootPath().resolve(path).startsWith(directory)
     }.orEmpty()
-    return nestedVcs.map {
-        it.key to RepositoryProvenance(it.value, it.value.revision)
-    }.toMap()
 }
 
 /**
@@ -173,17 +166,15 @@ internal data class ProcessedCopyrightStatement(
  */
 internal fun getLicenseFindingCurationsByRepository(
     curations: Collection<LicenseFindingCuration>,
-    nestedRepositories: Map<String, KnownProvenance>
+    nestedRepositories: Map<String, VcsInfo>
 ): RepositoryLicenseFindingCurations {
     val result = mutableMapOf<String, MutableList<LicenseFindingCuration>>()
 
-    nestedRepositories.forEach { (path, provenance) ->
-        if (provenance is RepositoryProvenance) {
-            val pathExcludesForRepository = result.getOrPut(provenance.vcsInfo.url) { mutableListOf() }
-            curations.forEach { curation ->
-                curation.path.withoutPrefix("$path/")?.let {
-                    pathExcludesForRepository += curation.copy(path = it)
-                }
+    nestedRepositories.forEach { (path, vcs) ->
+        val pathExcludesForRepository = result.getOrPut(vcs.url) { mutableListOf() }
+        curations.forEach { curation ->
+            curation.path.withoutPrefix("$path/")?.let {
+                pathExcludesForRepository += curation.copy(path = it)
             }
         }
     }
@@ -196,17 +187,15 @@ internal fun getLicenseFindingCurationsByRepository(
  */
 internal fun getPathExcludesByRepository(
     pathExcludes: Collection<PathExclude>,
-    nestedRepositories: Map<String, KnownProvenance>
+    nestedRepositories: Map<String, VcsInfo>
 ): RepositoryPathExcludes {
     val result = mutableMapOf<String, MutableList<PathExclude>>()
 
-    nestedRepositories.forEach { (path, provenance) ->
-        if (provenance is RepositoryProvenance) {
-            val pathExcludesForRepository = result.getOrPut(provenance.vcsInfo.url) { mutableListOf() }
-            pathExcludes.forEach { pathExclude ->
-                pathExclude.pattern.withoutPrefix("$path/")?.let {
-                    pathExcludesForRepository += pathExclude.copy(pattern = it)
-                }
+    nestedRepositories.forEach { (path, vcs) ->
+        val pathExcludesForRepository = result.getOrPut(vcs.url) { mutableListOf() }
+        pathExcludes.forEach { pathExclude ->
+            pathExclude.pattern.withoutPrefix("$path/")?.let {
+                pathExcludesForRepository += pathExclude.copy(pattern = it)
             }
         }
     }

diff --git a/model/src/main/kotlin/OrtResult.kt b/model/src/main/kotlin/OrtResult.kt
@@ -194,9 +194,7 @@ data class OrtResult(
     }
 
     private val relativeProjectVcsPath: Map<Identifier, String?> by lazy {
-        getProjects().associateBy({ it.id }, {
-            repository.getRelativePath(RepositoryProvenance(it.vcsProcessed, it.vcsProcessed.revision))
-        })
+        getProjects().associateBy({ it.id }, { repository.getRelativePath(it.vcsProcessed) })
     }
 
     /**

diff --git a/model/src/main/kotlin/Repository.kt b/model/src/main/kotlin/Repository.kt
@@ -39,7 +39,7 @@ data class Repository(
      * nested repository relative to the root of the main repository.
      */
     @JsonInclude(JsonInclude.Include.NON_EMPTY)
-    val nestedRepositories: Map<String, KnownProvenance> = emptyMap(),
+    val nestedRepositories: Map<String, VcsInfo> = emptyMap(),
 
     /**
      * The configuration of the repository, parsed from [ORT_REPO_CONFIG_FILENAME].
@@ -73,12 +73,18 @@ data class Repository(
     }
 
     /**
-     * Return the path of [provenance] relative to [Repository.provenance], or null if [provenance] is neither
+     * Return the path of [vcs] relative to [Repository.provenance], or null if [vcs] is neither
      * [Repository.provenance] nor contained in [nestedRepositories].
      */
-    fun getRelativePath(provenance: KnownProvenance): String? {
-        if (this.provenance.matches(provenance)) return ""
+    fun getRelativePath(vcs: VcsInfo): String? {
+        if (this.provenance !is RepositoryProvenance) return null
 
-        return nestedRepositories.entries.find { (_, nestedProvenance) -> nestedProvenance.matches(provenance) }?.key
+        val normalizeVcs = vcs.normalize()
+
+        if (vcsProcessed.equalsDisregardingPath(normalizeVcs)) return ""
+
+        return nestedRepositories.entries.find { (_, nestedVcs) ->
+            nestedVcs.normalize().equalsDisregardingPath(vcs.normalize())
+        }?.key
     }
 }
diff --git a/model/src/main/kotlin/licenses/DefaultLicenseInfoProvider.kt b/model/src/main/kotlin/licenses/DefaultLicenseInfoProvider.kt
@@ -25,7 +25,6 @@ import java.util.concurrent.ConcurrentMap
 import org.ossreviewtoolkit.model.Identifier
 import org.ossreviewtoolkit.model.OrtResult
 import org.ossreviewtoolkit.model.Provenance
-import org.ossreviewtoolkit.model.RepositoryProvenance
 import org.ossreviewtoolkit.model.config.LicenseFindingCuration
 import org.ossreviewtoolkit.model.config.PathExclude
 import org.ossreviewtoolkit.model.utils.filterByVcsPath
@@ -107,9 +106,7 @@ class DefaultLicenseInfoProvider(val ortResult: OrtResult) : LicenseInfoProvider
             Configuration(
                 ortResult.repository.config.curations.licenseFindings,
                 ortResult.repository.config.excludes.paths,
-                ortResult.repository.getRelativePath(
-                    RepositoryProvenance(project.vcsProcessed, project.vcsProcessed.revision)
-                ).orEmpty()
+                ortResult.repository.getRelativePath(project.vcsProcessed).orEmpty()
             )
         } ?: ortResult.getPackageConfigurations(id, provenance).let { packageConfigurations ->
             Configuration(

diff --git a/model/src/main/kotlin/utils/OrtResultExtensions.kt b/model/src/main/kotlin/utils/OrtResultExtensions.kt
@@ -85,8 +85,11 @@ fun OrtResult.createLicenseInfoResolver(
  * Return the path where the repository given by [provenance] is linked into the source tree.
  */
 fun OrtResult.getRepositoryPath(provenance: RepositoryProvenance): String {
-    repository.nestedRepositories.forEach { (path, nestedProvenance) ->
-        if (nestedProvenance.matches(provenance)) {
+    repository.nestedRepositories.forEach { (path, vcsInfo) ->
+        if (vcsInfo.type == provenance.vcsInfo.type
+            && vcsInfo.url == provenance.vcsInfo.url
+            && vcsInfo.revision == provenance.resolvedRevision
+        ) {
             return "/$path/"
         }
     }

diff --git a/model/src/test/kotlin/OrtResultTest.kt b/model/src/test/kotlin/OrtResultTest.kt
@@ -108,8 +108,8 @@ class OrtResultTest : WordSpec({
                 Repository(
                     provenance = RepositoryProvenance(vcs, vcs.revision),
                     nestedRepositories = mapOf(
-                        "path/1" to RepositoryProvenance(nestedVcs1, nestedVcs1.revision),
-                        "path/2" to RepositoryProvenance(nestedVcs2, nestedVcs2.revision)
+                        "path/1" to nestedVcs1,
+                        "path/2" to nestedVcs2
                     )
                 ),
                 AnalyzerRun.EMPTY.copy(
@@ -136,7 +136,7 @@ class OrtResultTest : WordSpec({
                 Repository(
                     provenance = RepositoryProvenance(vcs, vcs.revision),
                     nestedRepositories = mapOf(
-                        "path/1" to RepositoryProvenance(nestedVcs1, nestedVcs1.revision)
+                        "path/1" to nestedVcs1
                     )
                 ),
                 AnalyzerRun.EMPTY.copy(

diff --git a/...ed-model/src/funTest/assets/evaluated-model-reporter-test-deduplicate-expected-output.yml b/...ed-model/src/funTest/assets/evaluated-model-reporter-test-deduplicate-expected-output.yml
@@ -1168,12 +1168,10 @@ repository:
     resolved_revision: "master"
   nested_repositories:
     sub/module:
-      vcs_info:
-        type: "Git"
-        url: "https://example.com/git"
-        revision: "master"
-        path: ""
-      resolved_revision: "master"
+      type: "Git"
+      url: "https://example.com/git"
+      revision: "master"
+      path: ""
   config:
     excludes:
       paths:

diff --git a/...ers/evaluated-model/src/funTest/assets/evaluated-model-reporter-test-expected-output.json b/...ers/evaluated-model/src/funTest/assets/evaluated-model-reporter-test-expected-output.json
@@ -1267,13 +1267,10 @@
     },
     "nested_repositories" : {
       "sub/module" : {
-        "vcs_info" : {
-          "type" : "Git",
-          "url" : "https://example.com/git",
-          "revision" : "master",
-          "path" : ""
-        },
-        "resolved_revision" : "master"
+        "type" : "Git",
+        "url" : "https://example.com/git",
+        "revision" : "master",
+        "path" : ""
       }
     },
     "config" : {

diff --git a/...ters/evaluated-model/src/funTest/assets/evaluated-model-reporter-test-expected-output.yml b/...ters/evaluated-model/src/funTest/assets/evaluated-model-reporter-test-expected-output.yml
@@ -1168,12 +1168,10 @@ repository:
     resolved_revision: "master"
   nested_repositories:
     sub/module:
-      vcs_info:
-        type: "Git"
-        url: "https://example.com/git"
-        revision: "master"
-        path: ""
-      resolved_revision: "master"
+      type: "Git"
+      url: "https://example.com/git"
+      revision: "master"
+      path: ""
   config:
     excludes:
       paths:

diff --git a/plugins/reporters/evaluated-model/src/funTest/assets/reporter-test-input.yml b/plugins/reporters/evaluated-model/src/funTest/assets/reporter-test-input.yml
@@ -9,12 +9,10 @@ repository:
     resolved_revision: "master"
   nested_repositories:
     sub/module:
-      vcs_info:
-        type: "Git"
-        url: "https://example.com/git"
-        revision: "master"
-        path: ""
-      resolved_revision: "master"
+      type: "Git"
+      url: "https://example.com/git"
+      revision: "master"
+      path: ""
   config:
     excludes:
       paths:

diff --git a/plugins/reporters/freemarker/src/test/kotlin/FreeMarkerTemplateProcessorTest.kt b/plugins/reporters/freemarker/src/test/kotlin/FreeMarkerTemplateProcessorTest.kt
@@ -96,12 +96,11 @@ private val PROJECT_VCS_INFO = VcsInfo(
     url = "ssh://git@host/manifests/repo?manifest=path/to/manifest.xml",
     revision = PROJECT_REVISION
 )
-private const val NESTED_REVISION = "0000000000000000000000000000000000000000"
 private val NESTED_VCS_INFO = VcsInfo(
     type = VcsType.GIT,
     url = "ssh://git@host/project/repo",
     path = "",
-    revision = NESTED_REVISION
+    revision = "0000000000000000000000000000000000000000"
 )
 
 private val idRootProject = Identifier("NPM:@ort:project-in-root-dir:1.0")
@@ -111,7 +110,7 @@ private val idNestedProject = Identifier("SpdxDocumentFile:@ort:project-in-neste
 private val ORT_RESULT = OrtResult(
     repository = Repository(
         provenance = RepositoryProvenance(PROJECT_VCS_INFO, PROJECT_REVISION),
-        nestedRepositories = mapOf("nested-vcs-dir" to RepositoryProvenance(NESTED_VCS_INFO, NESTED_REVISION))
+        nestedRepositories = mapOf("nested-vcs-dir" to NESTED_VCS_INFO)
     ),
     analyzer = AnalyzerRun.EMPTY.copy(
         result = AnalyzerResult.EMPTY.copy(

diff --git a/plugins/reporters/opossum/src/funTest/assets/reporter-test-input.yml b/plugins/reporters/opossum/src/funTest/assets/reporter-test-input.yml
@@ -9,12 +9,10 @@ repository:
     resolved_revision: "master"
   nested_repositories:
     sub/module:
-      vcs_info:
-        type: "Git"
-        url: "https://example.com/git"
-        revision: "master"
-        path: ""
-      resolved_revision: "master"
+      type: "Git"
+      url: "https://example.com/git"
+      revision: "master"
+      path: ""
   config:
     excludes:
       paths: