Skip to content

Commit ac06be1

Browse files
pepper-jkpepper-jk
committed
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]>
1 parent 665ab20 commit ac06be1

File tree

16 files changed

+90
-127
lines changed

16 files changed

+90
-127
lines changed

‎analyzer/src/main/kotlin/Analyzer.kt

+1-3
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,7 @@ class Analyzer(private val config: AnalyzerConfiguration, private val labels: Ma
143143
}.orEmpty()
144144
val repository = Repository(
145145
provenance = RepositoryProvenance(vcs, revision),
146-
nestedRepositories = nestedVcs.map {
147-
it.key to RepositoryProvenance(it.value, it.value.revision)
148-
}.toMap(),
146+
nestedRepositories = nestedVcs,
149147
config = info.repositoryConfiguration
150148
)
151149

‎cli/src/funTest/assets/git-repo-expected-output.yml

+24-36
Original file line numberDiff line numberDiff line change
@@ -9,47 +9,35 @@ repository:
99
resolved_revision: "31588aa8f8555474e1c3c66a359ec99e4cd4b1fa"
1010
nested_repositories:
1111
spdx-tools:
12-
vcs_info:
13-
type: "Git"
14-
url: "https://github.com/spdx/tools"
15-
revision: "e179fae47590eccedc46186ea0ce20cbade5fda7"
16-
path: ""
17-
resolved_revision: "e179fae47590eccedc46186ea0ce20cbade5fda7"
12+
type: "Git"
13+
url: "https://github.com/spdx/tools"
14+
revision: "e179fae47590eccedc46186ea0ce20cbade5fda7"
15+
path: ""
1816
submodules:
19-
vcs_info:
20-
type: "Git"
21-
url: "https://github.com/oss-review-toolkit/ort-test-data-git-submodules"
22-
revision: "fcea94bab5835172e826afddb9f6427274c983b9"
23-
path: ""
24-
resolved_revision: "fcea94bab5835172e826afddb9f6427274c983b9"
17+
type: "Git"
18+
url: "https://github.com/oss-review-toolkit/ort-test-data-git-submodules"
19+
revision: "fcea94bab5835172e826afddb9f6427274c983b9"
20+
path: ""
2521
submodules/commons-text:
26-
vcs_info:
27-
type: "Git"
28-
url: "https://github.com/apache/commons-text.git"
29-
revision: "7643b12421100d29fd2b78053e77bcb04a251b2e"
30-
path: ""
31-
resolved_revision: "7643b12421100d29fd2b78053e77bcb04a251b2e"
22+
type: "Git"
23+
url: "https://github.com/apache/commons-text.git"
24+
revision: "7643b12421100d29fd2b78053e77bcb04a251b2e"
25+
path: ""
3226
submodules/test-data-npm:
33-
vcs_info:
34-
type: "Git"
35-
url: "https://github.com/oss-review-toolkit/ort-test-data-npm.git"
36-
revision: "ad0367b7b9920144a47b8d30cc0c84cea102b821"
37-
path: ""
38-
resolved_revision: "ad0367b7b9920144a47b8d30cc0c84cea102b821"
27+
type: "Git"
28+
url: "https://github.com/oss-review-toolkit/ort-test-data-npm.git"
29+
revision: "ad0367b7b9920144a47b8d30cc0c84cea102b821"
30+
path: ""
3931
submodules/test-data-npm/isarray:
40-
vcs_info:
41-
type: "Git"
42-
url: "https://github.com/juliangruber/isarray.git"
43-
revision: "63ea4ca0a0d6b0574d6a470ebd26880c3026db4a"
44-
path: ""
45-
resolved_revision: "63ea4ca0a0d6b0574d6a470ebd26880c3026db4a"
32+
type: "Git"
33+
url: "https://github.com/juliangruber/isarray.git"
34+
revision: "63ea4ca0a0d6b0574d6a470ebd26880c3026db4a"
35+
path: ""
4636
submodules/test-data-npm/long.js:
47-
vcs_info:
48-
type: "Git"
49-
url: "https://github.com/dcodeIO/long.js.git"
50-
revision: "941c5c62471168b5d18153755c2a7b38d2560e58"
51-
path: ""
52-
resolved_revision: "941c5c62471168b5d18153755c2a7b38d2560e58"
37+
type: "Git"
38+
url: "https://github.com/dcodeIO/long.js.git"
39+
revision: "941c5c62471168b5d18153755c2a7b38d2560e58"
40+
path: ""
5341
config: {}
5442
analyzer:
5543
start_time: "1970-01-01T00:00:00Z"

‎helper-cli/src/main/kotlin/utils/Extensions.kt

+2-4
Original file line numberDiff line numberDiff line change
@@ -693,10 +693,8 @@ internal fun OrtResult.getScanResultFor(packageConfiguration: PackageConfigurati
693693
internal fun OrtResult.getRepositoryPaths(): Map<String, Set<String>> {
694694
val result = mutableMapOf(repository.vcsProcessed.url to mutableSetOf(""))
695695

696-
repository.nestedRepositories.mapValues { (path, provenance) ->
697-
if (provenance is RepositoryProvenance) {
698-
result.getOrPut(provenance.vcsInfo.url) { mutableSetOf() } += path
699-
}
696+
repository.nestedRepositories.mapValues { (path, vcsInfo) ->
697+
result.getOrPut(vcsInfo.url) { mutableSetOf() } += path
700698
}
701699

702700
// For some Git-repo projects `OrtResult.repository.nestedRepositories´ misses some nested repositories for Git

‎helper-cli/src/main/kotlin/utils/Utils.kt

+16-27
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,8 @@ import java.io.File
2525

2626
import org.ossreviewtoolkit.downloader.VersionControlSystem
2727
import org.ossreviewtoolkit.model.Identifier
28-
import org.ossreviewtoolkit.model.KnownProvenance
2928
import org.ossreviewtoolkit.model.OrtResult
3029
import org.ossreviewtoolkit.model.PackageCuration
31-
import org.ossreviewtoolkit.model.RepositoryProvenance
3230
import org.ossreviewtoolkit.model.VcsInfo
3331
import org.ossreviewtoolkit.model.config.LicenseFindingCuration
3432
import org.ossreviewtoolkit.model.config.PathExclude
@@ -70,10 +68,8 @@ internal fun findRepositoryPaths(directory: File): Map<String, Set<String>> {
7068

7169
val result = mutableMapOf<String, MutableSet<String>>()
7270

73-
findRepositories(directory).forEach { (path, provenance) ->
74-
if (provenance is RepositoryProvenance) {
75-
result.getOrPut(provenance.vcsInfo.url.replaceCredentialsInUri()) { mutableSetOf() } += path
76-
}
71+
findRepositories(directory).forEach { (path, vcs) ->
72+
result.getOrPut(vcs.url.replaceCredentialsInUri()) { mutableSetOf() } += path
7773
}
7874

7975
return result
@@ -83,17 +79,14 @@ internal fun findRepositoryPaths(directory: File): Map<String, Set<String>> {
8379
* Search the given [directory] for repositories and return a mapping from paths where each respective repository was
8480
* found to the corresponding [VcsInfo].
8581
*/
86-
internal fun findRepositories(directory: File): Map<String, KnownProvenance> {
82+
internal fun findRepositories(directory: File): Map<String, VcsInfo> {
8783
require(directory.isDirectory)
8884

8985
val workingTree = VersionControlSystem.forDirectory(directory)
90-
val nestedVcs = workingTree?.getNested()?.filter { (path, _) ->
86+
return workingTree?.getNested()?.filter { (path, _) ->
9187
// Only include nested VCS if they are part of the analyzed directory.
9288
workingTree.getRootPath().resolve(path).startsWith(directory)
9389
}.orEmpty()
94-
return nestedVcs.map {
95-
it.key to RepositoryProvenance(it.value, it.value.revision)
96-
}.toMap()
9790
}
9891

9992
/**
@@ -173,17 +166,15 @@ internal data class ProcessedCopyrightStatement(
173166
*/
174167
internal fun getLicenseFindingCurationsByRepository(
175168
curations: Collection<LicenseFindingCuration>,
176-
nestedRepositories: Map<String, KnownProvenance>
169+
nestedRepositories: Map<String, VcsInfo>
177170
): RepositoryLicenseFindingCurations {
178171
val result = mutableMapOf<String, MutableList<LicenseFindingCuration>>()
179172

180-
nestedRepositories.forEach { (path, provenance) ->
181-
if (provenance is RepositoryProvenance) {
182-
val pathExcludesForRepository = result.getOrPut(provenance.vcsInfo.url) { mutableListOf() }
183-
curations.forEach { curation ->
184-
curation.path.withoutPrefix("$path/")?.let {
185-
pathExcludesForRepository += curation.copy(path = it)
186-
}
173+
nestedRepositories.forEach { (path, vcs) ->
174+
val pathExcludesForRepository = result.getOrPut(vcs.url) { mutableListOf() }
175+
curations.forEach { curation ->
176+
curation.path.withoutPrefix("$path/")?.let {
177+
pathExcludesForRepository += curation.copy(path = it)
187178
}
188179
}
189180
}
@@ -196,17 +187,15 @@ internal fun getLicenseFindingCurationsByRepository(
196187
*/
197188
internal fun getPathExcludesByRepository(
198189
pathExcludes: Collection<PathExclude>,
199-
nestedRepositories: Map<String, KnownProvenance>
190+
nestedRepositories: Map<String, VcsInfo>
200191
): RepositoryPathExcludes {
201192
val result = mutableMapOf<String, MutableList<PathExclude>>()
202193

203-
nestedRepositories.forEach { (path, provenance) ->
204-
if (provenance is RepositoryProvenance) {
205-
val pathExcludesForRepository = result.getOrPut(provenance.vcsInfo.url) { mutableListOf() }
206-
pathExcludes.forEach { pathExclude ->
207-
pathExclude.pattern.withoutPrefix("$path/")?.let {
208-
pathExcludesForRepository += pathExclude.copy(pattern = it)
209-
}
194+
nestedRepositories.forEach { (path, vcs) ->
195+
val pathExcludesForRepository = result.getOrPut(vcs.url) { mutableListOf() }
196+
pathExcludes.forEach { pathExclude ->
197+
pathExclude.pattern.withoutPrefix("$path/")?.let {
198+
pathExcludesForRepository += pathExclude.copy(pattern = it)
210199
}
211200
}
212201
}

‎model/src/main/kotlin/OrtResult.kt

+1-3
Original file line numberDiff line numberDiff line change
@@ -194,9 +194,7 @@ data class OrtResult(
194194
}
195195

196196
private val relativeProjectVcsPath: Map<Identifier, String?> by lazy {
197-
getProjects().associateBy({ it.id }, {
198-
repository.getRelativePath(RepositoryProvenance(it.vcsProcessed, it.vcsProcessed.revision))
199-
})
197+
getProjects().associateBy({ it.id }, { repository.getRelativePath(it.vcsProcessed) })
200198
}
201199

202200
/**

‎model/src/main/kotlin/Repository.kt

+11-5
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ data class Repository(
3939
* nested repository relative to the root of the main repository.
4040
*/
4141
@JsonInclude(JsonInclude.Include.NON_EMPTY)
42-
val nestedRepositories: Map<String, KnownProvenance> = emptyMap(),
42+
val nestedRepositories: Map<String, VcsInfo> = emptyMap(),
4343

4444
/**
4545
* The configuration of the repository, parsed from [ORT_REPO_CONFIG_FILENAME].
@@ -73,12 +73,18 @@ data class Repository(
7373
}
7474

7575
/**
76-
* Return the path of [provenance] relative to [Repository.provenance], or null if [provenance] is neither
76+
* Return the path of [vcs] relative to [Repository.provenance], or null if [vcs] is neither
7777
* [Repository.provenance] nor contained in [nestedRepositories].
7878
*/
79-
fun getRelativePath(provenance: KnownProvenance): String? {
80-
if (this.provenance.matches(provenance)) return ""
79+
fun getRelativePath(vcs: VcsInfo): String? {
80+
if (this.provenance !is RepositoryProvenance) return null
8181

82-
return nestedRepositories.entries.find { (_, nestedProvenance) -> nestedProvenance.matches(provenance) }?.key
82+
val normalizedVcs = vcs.normalize()
83+
84+
if (vcsProcessed.equalsDisregardingPath(normalizedVcs)) return ""
85+
86+
return nestedRepositories.entries.find { (_, nestedVcs) ->
87+
nestedVcs.normalize().equalsDisregardingPath(normalizedVcs)
88+
}?.key
8389
}
8490
}

‎model/src/main/kotlin/licenses/DefaultLicenseInfoProvider.kt

+1-4
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import java.util.concurrent.ConcurrentMap
2525
import org.ossreviewtoolkit.model.Identifier
2626
import org.ossreviewtoolkit.model.OrtResult
2727
import org.ossreviewtoolkit.model.Provenance
28-
import org.ossreviewtoolkit.model.RepositoryProvenance
2928
import org.ossreviewtoolkit.model.config.LicenseFindingCuration
3029
import org.ossreviewtoolkit.model.config.PathExclude
3130
import org.ossreviewtoolkit.model.utils.filterByVcsPath
@@ -107,9 +106,7 @@ class DefaultLicenseInfoProvider(val ortResult: OrtResult) : LicenseInfoProvider
107106
Configuration(
108107
ortResult.repository.config.curations.licenseFindings,
109108
ortResult.repository.config.excludes.paths,
110-
ortResult.repository.getRelativePath(
111-
RepositoryProvenance(project.vcsProcessed, project.vcsProcessed.revision)
112-
).orEmpty()
109+
ortResult.repository.getRelativePath(project.vcsProcessed).orEmpty()
113110
)
114111
} ?: ortResult.getPackageConfigurations(id, provenance).let { packageConfigurations ->
115112
Configuration(

‎model/src/main/kotlin/utils/OrtResultExtensions.kt

+5-2
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,11 @@ fun OrtResult.createLicenseInfoResolver(
8585
* Return the path where the repository given by [provenance] is linked into the source tree.
8686
*/
8787
fun OrtResult.getRepositoryPath(provenance: RepositoryProvenance): String {
88-
repository.nestedRepositories.forEach { (path, nestedProvenance) ->
89-
if (nestedProvenance.matches(provenance)) {
88+
repository.nestedRepositories.forEach { (path, vcsInfo) ->
89+
if (vcsInfo.type == provenance.vcsInfo.type
90+
&& vcsInfo.url == provenance.vcsInfo.url
91+
&& vcsInfo.revision == provenance.resolvedRevision
92+
) {
9093
return "/$path/"
9194
}
9295
}

‎model/src/test/kotlin/OrtResultTest.kt

+3-3
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,8 @@ class OrtResultTest : WordSpec({
108108
Repository(
109109
provenance = RepositoryProvenance(vcs, vcs.revision),
110110
nestedRepositories = mapOf(
111-
"path/1" to RepositoryProvenance(nestedVcs1, nestedVcs1.revision),
112-
"path/2" to RepositoryProvenance(nestedVcs2, nestedVcs2.revision)
111+
"path/1" to nestedVcs1,
112+
"path/2" to nestedVcs2
113113
)
114114
),
115115
AnalyzerRun.EMPTY.copy(
@@ -136,7 +136,7 @@ class OrtResultTest : WordSpec({
136136
Repository(
137137
provenance = RepositoryProvenance(vcs, vcs.revision),
138138
nestedRepositories = mapOf(
139-
"path/1" to RepositoryProvenance(nestedVcs1, nestedVcs1.revision)
139+
"path/1" to nestedVcs1
140140
)
141141
),
142142
AnalyzerRun.EMPTY.copy(

‎plugins/reporters/evaluated-model/src/funTest/assets/evaluated-model-reporter-test-deduplicate-expected-output.yml

+4-6
Original file line numberDiff line numberDiff line change
@@ -1168,12 +1168,10 @@ repository:
11681168
resolved_revision: "master"
11691169
nested_repositories:
11701170
sub/module:
1171-
vcs_info:
1172-
type: "Git"
1173-
url: "https://example.com/git"
1174-
revision: "master"
1175-
path: ""
1176-
resolved_revision: "master"
1171+
type: "Git"
1172+
url: "https://example.com/git"
1173+
revision: "master"
1174+
path: ""
11771175
config:
11781176
excludes:
11791177
paths:

‎plugins/reporters/evaluated-model/src/funTest/assets/evaluated-model-reporter-test-expected-output.json

+4-7
Original file line numberDiff line numberDiff line change
@@ -1267,13 +1267,10 @@
12671267
},
12681268
"nested_repositories" : {
12691269
"sub/module" : {
1270-
"vcs_info" : {
1271-
"type" : "Git",
1272-
"url" : "https://example.com/git",
1273-
"revision" : "master",
1274-
"path" : ""
1275-
},
1276-
"resolved_revision" : "master"
1270+
"type" : "Git",
1271+
"url" : "https://example.com/git",
1272+
"revision" : "master",
1273+
"path" : ""
12771274
}
12781275
},
12791276
"config" : {

‎plugins/reporters/evaluated-model/src/funTest/assets/evaluated-model-reporter-test-expected-output.yml

+4-6
Original file line numberDiff line numberDiff line change
@@ -1168,12 +1168,10 @@ repository:
11681168
resolved_revision: "master"
11691169
nested_repositories:
11701170
sub/module:
1171-
vcs_info:
1172-
type: "Git"
1173-
url: "https://example.com/git"
1174-
revision: "master"
1175-
path: ""
1176-
resolved_revision: "master"
1171+
type: "Git"
1172+
url: "https://example.com/git"
1173+
revision: "master"
1174+
path: ""
11771175
config:
11781176
excludes:
11791177
paths:

‎plugins/reporters/evaluated-model/src/funTest/assets/reporter-test-input.yml

+4-6
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,10 @@ repository:
99
resolved_revision: "master"
1010
nested_repositories:
1111
sub/module:
12-
vcs_info:
13-
type: "Git"
14-
url: "https://example.com/git"
15-
revision: "master"
16-
path: ""
17-
resolved_revision: "master"
12+
type: "Git"
13+
url: "https://example.com/git"
14+
revision: "master"
15+
path: ""
1816
config:
1917
excludes:
2018
paths:

‎plugins/reporters/freemarker/src/test/kotlin/FreeMarkerTemplateProcessorTest.kt

+2-3
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,11 @@ private val PROJECT_VCS_INFO = VcsInfo(
9696
url = "ssh://git@host/manifests/repo?manifest=path/to/manifest.xml",
9797
revision = PROJECT_REVISION
9898
)
99-
private const val NESTED_REVISION = "0000000000000000000000000000000000000000"
10099
private val NESTED_VCS_INFO = VcsInfo(
101100
type = VcsType.GIT,
102101
url = "ssh://git@host/project/repo",
103102
path = "",
104-
revision = NESTED_REVISION
103+
revision = "0000000000000000000000000000000000000000"
105104
)
106105

107106
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
111110
private val ORT_RESULT = OrtResult(
112111
repository = Repository(
113112
provenance = RepositoryProvenance(PROJECT_VCS_INFO, PROJECT_REVISION),
114-
nestedRepositories = mapOf("nested-vcs-dir" to RepositoryProvenance(NESTED_VCS_INFO, NESTED_REVISION))
113+
nestedRepositories = mapOf("nested-vcs-dir" to NESTED_VCS_INFO)
115114
),
116115
analyzer = AnalyzerRun.EMPTY.copy(
117116
result = AnalyzerResult.EMPTY.copy(

‎plugins/reporters/opossum/src/funTest/assets/reporter-test-input.yml

+4-6
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,10 @@ repository:
99
resolved_revision: "master"
1010
nested_repositories:
1111
sub/module:
12-
vcs_info:
13-
type: "Git"
14-
url: "https://example.com/git"
15-
revision: "master"
16-
path: ""
17-
resolved_revision: "master"
12+
type: "Git"
13+
url: "https://example.com/git"
14+
revision: "master"
15+
path: ""
1816
config:
1917
excludes:
2018
paths:

0 commit comments

Comments
 (0)