From 4454630f0a031bd1b7866bceee27ae2f3b95204d Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Fri, 19 Jul 2024 07:44:44 +0200 Subject: [PATCH 1/7] chore(model): Consistently use HTTPS for `example.com` URLs Signed-off-by: Sebastian Schuberth --- model/src/test/kotlin/licenses/LicenseInfoResolverTest.kt | 2 +- model/src/test/kotlin/utils/PurlExtensionsTest.kt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/model/src/test/kotlin/licenses/LicenseInfoResolverTest.kt b/model/src/test/kotlin/licenses/LicenseInfoResolverTest.kt index f3b7ce429828e..2abee45e4d12d 100644 --- a/model/src/test/kotlin/licenses/LicenseInfoResolverTest.kt +++ b/model/src/test/kotlin/licenses/LicenseInfoResolverTest.kt @@ -337,7 +337,7 @@ class LicenseInfoResolverTest : WordSpec({ "apply path excludes" { val sourceArtifact = RemoteArtifact( - url = "http://example.com", + url = "https://example.com/", hash = Hash("", HashAlgorithm.NONE) ) val sourceArtifactProvenance = ArtifactProvenance( diff --git a/model/src/test/kotlin/utils/PurlExtensionsTest.kt b/model/src/test/kotlin/utils/PurlExtensionsTest.kt index 8a93cd6694cd5..7975dfcff1937 100644 --- a/model/src/test/kotlin/utils/PurlExtensionsTest.kt +++ b/model/src/test/kotlin/utils/PurlExtensionsTest.kt @@ -36,7 +36,7 @@ class PurlExtensionsTest : StringSpec({ "Artifact provenance can be converted to PURL extras and back" { val provenance = ArtifactProvenance( sourceArtifact = RemoteArtifact( - url = "http://example.com/sources.zip", + url = "https://example.com/sources.zip", hash = Hash( value = "ddce269a1e3d054cae349621c198dd52", algorithm = HashAlgorithm.MD5 @@ -49,7 +49,7 @@ class PurlExtensionsTest : StringSpec({ val purl = id.toPurl(extras.qualifiers, extras.subpath) purl shouldBe "pkg:maven/com.example/sources@1.2.3?" + - "download_url=http%3A%2F%2Fexample.com%2Fsources.zip&" + + "download_url=https%3A%2F%2Fexample.com%2Fsources.zip&" + "checksum=md5%3Addce269a1e3d054cae349621c198dd52" purl.toProvenance() shouldBe provenance } From b6bdbfe5f906d1591f4bdf81a1165237dd575159 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Fri, 19 Jul 2024 00:06:29 +0200 Subject: [PATCH 2/7] test: Use realistic `RemoteArtifact` placeholder values This avoids an error with upcoming hash sanity checks. Signed-off-by: Sebastian Schuberth --- .../kotlin/utils/PostgresProvenanceFileStorageFunTest.kt | 2 +- model/src/test/kotlin/licenses/LicenseInfoResolverTest.kt | 3 +-- .../package-managers/pub/src/funTest/kotlin/PubFunTest.kt | 5 ++--- reporter/src/testFixtures/kotlin/TestData.kt | 8 ++++---- utils/test/src/main/kotlin/Utils.kt | 2 +- 5 files changed, 9 insertions(+), 11 deletions(-) diff --git a/model/src/funTest/kotlin/utils/PostgresProvenanceFileStorageFunTest.kt b/model/src/funTest/kotlin/utils/PostgresProvenanceFileStorageFunTest.kt index 01ca43cc064ed..90ae5b08404b7 100644 --- a/model/src/funTest/kotlin/utils/PostgresProvenanceFileStorageFunTest.kt +++ b/model/src/funTest/kotlin/utils/PostgresProvenanceFileStorageFunTest.kt @@ -35,7 +35,7 @@ import org.ossreviewtoolkit.model.VcsType import org.ossreviewtoolkit.model.config.FileArchiverConfiguration import org.ossreviewtoolkit.utils.test.PostgresListener -private val SOURCE_ARTIFACT_PROVENANCE = ArtifactProvenance(sourceArtifact = RemoteArtifact("url", Hash.create("hash"))) +private val SOURCE_ARTIFACT_PROVENANCE = ArtifactProvenance(RemoteArtifact("https://example.com/", Hash.NONE)) private val VCS_PROVENANCE = RepositoryProvenance( vcsInfo = VcsInfo( type = VcsType.GIT, diff --git a/model/src/test/kotlin/licenses/LicenseInfoResolverTest.kt b/model/src/test/kotlin/licenses/LicenseInfoResolverTest.kt index 2abee45e4d12d..db6774f723377 100644 --- a/model/src/test/kotlin/licenses/LicenseInfoResolverTest.kt +++ b/model/src/test/kotlin/licenses/LicenseInfoResolverTest.kt @@ -39,7 +39,6 @@ import java.lang.IllegalArgumentException import org.ossreviewtoolkit.model.ArtifactProvenance import org.ossreviewtoolkit.model.CopyrightFinding import org.ossreviewtoolkit.model.Hash -import org.ossreviewtoolkit.model.HashAlgorithm import org.ossreviewtoolkit.model.Identifier import org.ossreviewtoolkit.model.LicenseFinding import org.ossreviewtoolkit.model.LicenseSource @@ -338,7 +337,7 @@ class LicenseInfoResolverTest : WordSpec({ "apply path excludes" { val sourceArtifact = RemoteArtifact( url = "https://example.com/", - hash = Hash("", HashAlgorithm.NONE) + hash = Hash.NONE ) val sourceArtifactProvenance = ArtifactProvenance( sourceArtifact = sourceArtifact diff --git a/plugins/package-managers/pub/src/funTest/kotlin/PubFunTest.kt b/plugins/package-managers/pub/src/funTest/kotlin/PubFunTest.kt index f647479da9cca..9777709a65fdf 100644 --- a/plugins/package-managers/pub/src/funTest/kotlin/PubFunTest.kt +++ b/plugins/package-managers/pub/src/funTest/kotlin/PubFunTest.kt @@ -31,7 +31,6 @@ import org.ossreviewtoolkit.analyzer.create import org.ossreviewtoolkit.analyzer.resolveSingleProject import org.ossreviewtoolkit.model.AnalyzerResult import org.ossreviewtoolkit.model.Hash -import org.ossreviewtoolkit.model.HashAlgorithm import org.ossreviewtoolkit.model.toYaml import org.ossreviewtoolkit.utils.test.getAssetFile import org.ossreviewtoolkit.utils.test.matchExpectedResult @@ -122,8 +121,8 @@ private fun AnalyzerResult.patchPackages(): AnalyzerResult { pkg.takeUnless { it.id.toCoordinates().startsWith("Maven:com.android.tools.build:aapt2:") } ?: pkg.copy( binaryArtifact = pkg.binaryArtifact.copy( - url = "***", - hash = Hash("***", HashAlgorithm.SHA1) + url = "https://example.com/", + hash = Hash.NONE ) ) } diff --git a/reporter/src/testFixtures/kotlin/TestData.kt b/reporter/src/testFixtures/kotlin/TestData.kt index dfc61b6667b62..ada437762d457 100644 --- a/reporter/src/testFixtures/kotlin/TestData.kt +++ b/reporter/src/testFixtures/kotlin/TestData.kt @@ -272,7 +272,7 @@ val ORT_RESULT = OrtResult( provenance = ArtifactProvenance( sourceArtifact = RemoteArtifact( url = "https://example.com/license-file-1.0.tgz", - hash = Hash(value = "", algorithm = HashAlgorithm.SHA1) + hash = Hash.NONE ) ), scanner = ScannerDetails(name = "scanner", version = "1.0", configuration = ""), @@ -305,7 +305,7 @@ val ORT_RESULT = OrtResult( provenance = ArtifactProvenance( sourceArtifact = RemoteArtifact( url = "https://example.com/license-file-and-additional-licenses-1.0.tgz", - hash = Hash(value = "", algorithm = HashAlgorithm.SHA1) + hash = Hash.NONE ) ), scanner = ScannerDetails(name = "scanner", version = "1.0", configuration = ""), @@ -346,7 +346,7 @@ val ORT_RESULT = OrtResult( provenance = ArtifactProvenance( sourceArtifact = RemoteArtifact( url = "https://example.com/concluded-license-1.0.tgz", - hash = Hash(value = "", algorithm = HashAlgorithm.SHA1) + hash = Hash.NONE ) ), scanner = ScannerDetails(name = "scanner", version = "1.0", configuration = ""), @@ -379,7 +379,7 @@ val ORT_RESULT = OrtResult( provenance = ArtifactProvenance( sourceArtifact = RemoteArtifact( url = "https://example.com/declared-license-1.0.tgz", - hash = Hash(value = "", algorithm = HashAlgorithm.SHA1) + hash = Hash.NONE ) ), scanner = ScannerDetails(name = "scanner", version = "1.0", configuration = ""), diff --git a/utils/test/src/main/kotlin/Utils.kt b/utils/test/src/main/kotlin/Utils.kt index b458ee18f30a0..2d7f211cd7b6f 100644 --- a/utils/test/src/main/kotlin/Utils.kt +++ b/utils/test/src/main/kotlin/Utils.kt @@ -156,7 +156,7 @@ fun scannerRunOf(vararg pkgScanResults: Pair>): Sca provenance = ArtifactProvenance( sourceArtifact = RemoteArtifact( url = id.toPurl(), - hash = Hash(value = "", algorithm = HashAlgorithm.NONE) + hash = Hash.NONE ) ) ) From 39acf3e0e1891f7a8bb38f82bd798b5c84a800f6 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Thu, 18 Jul 2024 22:59:04 +0200 Subject: [PATCH 3/7] refactor(model)!: Use a secondary `Hash` constructor instead of `create()` Move the sanity check from the `create()` function overload that takes strings to an `init` block so that it affects all constructors, and add a secondary constructor to replace that function. Change calling code accordingly and simplify the cases for `Conan` and `Pub` where the hash is known to be SHA256. Signed-off-by: Sebastian Schuberth --- model/src/main/kotlin/Hash.kt | 20 +++++++++---------- model/src/main/kotlin/utils/PurlExtensions.kt | 2 +- model/src/test/kotlin/HashTest.kt | 4 ++-- .../conan/src/main/kotlin/Conan.kt | 2 +- .../src/main/kotlin/GradleInspector.kt | 2 +- .../src/main/kotlin/utils/MavenSupport.kt | 2 +- .../src/test/kotlin/utils/MavenSupportTest.kt | 4 ++-- .../pub/src/main/kotlin/Pub.kt | 4 ++-- .../main/kotlin/utils/SpdxResolvedDocument.kt | 2 +- 9 files changed, 21 insertions(+), 21 deletions(-) diff --git a/model/src/main/kotlin/Hash.kt b/model/src/main/kotlin/Hash.kt index 4f5b6df55b2d2..9f0d3f82bc698 100644 --- a/model/src/main/kotlin/Hash.kt +++ b/model/src/main/kotlin/Hash.kt @@ -67,19 +67,19 @@ data class Hash( Hash(value.lowercase(), HashAlgorithm.create(value)) } } + } - /** - * Create a [Hash] instance from a known hash [value] and [algorithm]. This is mostly used for deserialization - * to verify the algorithm matches the one determined by the value. - */ - fun create(value: String, algorithm: String): Hash = - create(value).also { hash -> - require(hash.algorithm == HashAlgorithm.fromString(algorithm)) { - "'$value' is not a $algorithm hash." - } - } + init { + require(value.length == algorithm.size || algorithm == HashAlgorithm.UNKNOWN) { + "'$value' is not a $algorithm hash." + } } + /** + * Construct a [Hash] instance from hash [value] and [algorithm] strings. + */ + constructor(value: String, algorithm: String) : this(value, HashAlgorithm.fromString(algorithm)) + /** * Return the hash in Support Subresource Integrity (SRI) format. */ diff --git a/model/src/main/kotlin/utils/PurlExtensions.kt b/model/src/main/kotlin/utils/PurlExtensions.kt index faf81723dfbe4..3e70a5dc8d426 100644 --- a/model/src/main/kotlin/utils/PurlExtensions.kt +++ b/model/src/main/kotlin/utils/PurlExtensions.kt @@ -121,7 +121,7 @@ fun String.toProvenance(): Provenance { ArtifactProvenance( sourceArtifact = RemoteArtifact( url = URLDecoder.decode(encodedUrl, "UTF-8"), - hash = Hash.create(value, algorithm) + hash = Hash(value, algorithm) ) ) } diff --git a/model/src/test/kotlin/HashTest.kt b/model/src/test/kotlin/HashTest.kt index 4635b8cc93090..91356bcbf4225 100644 --- a/model/src/test/kotlin/HashTest.kt +++ b/model/src/test/kotlin/HashTest.kt @@ -73,12 +73,12 @@ class HashTest : WordSpec({ "Passing a string value and name to create()" should { "succeed if the name is valid for the created hash" { - Hash.create("6a7d2814506e9801f13e767964ae3a8f", "MD5").algorithm shouldBe HashAlgorithm.MD5 + Hash("6a7d2814506e9801f13e767964ae3a8f", "MD5").algorithm shouldBe HashAlgorithm.MD5 } "fail if the name is invalid for the created hash" { shouldThrow { - Hash.create("0123456789", "MD5") + Hash("0123456789", "MD5") } } } diff --git a/plugins/package-managers/conan/src/main/kotlin/Conan.kt b/plugins/package-managers/conan/src/main/kotlin/Conan.kt index 947d96af20ff4..9e1ce2a14a65b 100644 --- a/plugins/package-managers/conan/src/main/kotlin/Conan.kt +++ b/plugins/package-managers/conan/src/main/kotlin/Conan.kt @@ -381,7 +381,7 @@ class Conan( private fun parseSourceArtifact(conanData: ConanData): RemoteArtifact { val url = conanData.url ?: return RemoteArtifact.EMPTY val hashValue = conanData.sha256.orEmpty() - val hash = Hash.NONE.takeIf { hashValue.isEmpty() } ?: Hash.create(hashValue, HashAlgorithm.SHA256.name) + val hash = Hash.NONE.takeIf { hashValue.isEmpty() } ?: Hash(hashValue, HashAlgorithm.SHA256) return RemoteArtifact(url, hash) } diff --git a/plugins/package-managers/gradle-inspector/src/main/kotlin/GradleInspector.kt b/plugins/package-managers/gradle-inspector/src/main/kotlin/GradleInspector.kt index 31370e060dc25..51704f6620ce4 100644 --- a/plugins/package-managers/gradle-inspector/src/main/kotlin/GradleInspector.kt +++ b/plugins/package-managers/gradle-inspector/src/main/kotlin/GradleInspector.kt @@ -403,7 +403,7 @@ private fun GradleInspector.createRemoteArtifact( */ private fun parseChecksum(checksum: String, algorithm: String) = checksum.splitOnWhitespace().firstNotNullOfOrNull { - runCatching { Hash.create(it, algorithm) }.getOrNull() + runCatching { Hash(it, algorithm) }.getOrNull() } ?: Hash.NONE // See http://maven.apache.org/pom.html#SCM. diff --git a/plugins/package-managers/maven/src/main/kotlin/utils/MavenSupport.kt b/plugins/package-managers/maven/src/main/kotlin/utils/MavenSupport.kt index 3f85d0e44ec8a..518d7e10e2f3a 100644 --- a/plugins/package-managers/maven/src/main/kotlin/utils/MavenSupport.kt +++ b/plugins/package-managers/maven/src/main/kotlin/utils/MavenSupport.kt @@ -307,7 +307,7 @@ class MavenSupport(private val workspaceReader: WorkspaceReader) { */ internal fun parseChecksum(checksum: String, algorithm: String) = checksum.splitOnWhitespace().firstNotNullOfOrNull { - runCatching { Hash.create(it, algorithm) }.getOrNull() + runCatching { Hash(it, algorithm) }.getOrNull() } ?: Hash.NONE /** diff --git a/plugins/package-managers/maven/src/test/kotlin/utils/MavenSupportTest.kt b/plugins/package-managers/maven/src/test/kotlin/utils/MavenSupportTest.kt index e732fece76f1a..68350d396fd1d 100644 --- a/plugins/package-managers/maven/src/test/kotlin/utils/MavenSupportTest.kt +++ b/plugins/package-managers/maven/src/test/kotlin/utils/MavenSupportTest.kt @@ -200,14 +200,14 @@ class MavenSupportTest : WordSpec({ MavenSupport.parseChecksum( checksum = "868c0792233fc78d8c9bac29ac79ade988301318 7de43522ca1a2a65d7c3b9eacb802a51745b245c", algorithm = "SHA1" - ) shouldBe Hash.create("868c0792233fc78d8c9bac29ac79ade988301318", "SHA1") + ) shouldBe Hash("868c0792233fc78d8c9bac29ac79ade988301318", "SHA1") } "ignore prefixes and suffixes" { MavenSupport.parseChecksum( checksum = "prefix 868c0792233fc78d8c9bac29ac79ade988301318 suffix", algorithm = "SHA1" - ) shouldBe Hash.create("868c0792233fc78d8c9bac29ac79ade988301318", "SHA1") + ) shouldBe Hash("868c0792233fc78d8c9bac29ac79ade988301318", "SHA1") } } }) diff --git a/plugins/package-managers/pub/src/main/kotlin/Pub.kt b/plugins/package-managers/pub/src/main/kotlin/Pub.kt index 83530a7c2987d..d3fac9d8035d9 100644 --- a/plugins/package-managers/pub/src/main/kotlin/Pub.kt +++ b/plugins/package-managers/pub/src/main/kotlin/Pub.kt @@ -616,11 +616,11 @@ class Pub( val hostUrl = pkgInfoFromLockfile["description"]["url"].textValueOrEmpty() val sourceArtifact = if (source == "hosted" && hostUrl.isNotEmpty() && version.isNotEmpty()) { - val sha256 = pkgInfoFromLockfile["description"]["sha256"].textValueOrEmpty() + val sha256 = pkgInfoFromLockfile["description"]["sha256"].textValue() RemoteArtifact( url = "$hostUrl/packages/$rawName/versions/$version.tar.gz", - hash = Hash.create(sha256, HashAlgorithm.SHA256.name) + hash = Hash(sha256, HashAlgorithm.SHA256) ) } else { RemoteArtifact.EMPTY diff --git a/plugins/package-managers/spdx/src/main/kotlin/utils/SpdxResolvedDocument.kt b/plugins/package-managers/spdx/src/main/kotlin/utils/SpdxResolvedDocument.kt index 8823212d11a4c..f7151fa377cb8 100644 --- a/plugins/package-managers/spdx/src/main/kotlin/utils/SpdxResolvedDocument.kt +++ b/plugins/package-managers/spdx/src/main/kotlin/utils/SpdxResolvedDocument.kt @@ -394,7 +394,7 @@ private fun SpdxExternalDocumentReference.resolveFromDownload( * checksum. If not, return an [Issue] based on the document [uri] and [managerName]. */ private fun SpdxExternalDocumentReference.verifyChecksum(file: File, uri: URI, managerName: String): Issue? { - val hash = Hash.create(checksum.checksumValue, checksum.algorithm.name) + val hash = Hash(checksum.checksumValue, checksum.algorithm.name) if (hash.verify(file)) return null return SpdxResolvedDocument.createAndLogIssue( From 3278c2b91933a26fabaacf90e6e6ebae60f6ae13 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Thu, 18 Jul 2024 08:41:29 +0200 Subject: [PATCH 4/7] docs(conan): Explain why a temporary file is required for `inspect` Signed-off-by: Sebastian Schuberth --- plugins/package-managers/conan/src/main/kotlin/Conan.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/package-managers/conan/src/main/kotlin/Conan.kt b/plugins/package-managers/conan/src/main/kotlin/Conan.kt index 9e1ce2a14a65b..cade116232876 100644 --- a/plugins/package-managers/conan/src/main/kotlin/Conan.kt +++ b/plugins/package-managers/conan/src/main/kotlin/Conan.kt @@ -325,6 +325,8 @@ class Conan( */ private fun inspectField(pkgName: String, workingDir: File, field: String): String = pkgInspectResults.getOrPut(pkgName) { + // Note: While Conan 2 supports inspect output to stdout, Conan 1 does not and a temporary file is required, + // see https://github.com/conan-io/conan/issues/6972. val jsonFile = createOrtTempDir().resolve("inspect.json") run(workingDir, "inspect", pkgName, "--json", jsonFile.absolutePath) Json.parseToJsonElement(jsonFile.readText()).jsonObject.also { From 5f9326911e0d265a7940a6edac7aef9e1bacd321 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Thu, 18 Jul 2024 23:31:40 +0200 Subject: [PATCH 5/7] refactor(conan): Inline the `parsePackageField()` one-liner function This function did not really reduce the amount of code. Signed-off-by: Sebastian Schuberth --- .../package-managers/conan/src/main/kotlin/Conan.kt | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/plugins/package-managers/conan/src/main/kotlin/Conan.kt b/plugins/package-managers/conan/src/main/kotlin/Conan.kt index cade116232876..fbac01bf282ee 100644 --- a/plugins/package-managers/conan/src/main/kotlin/Conan.kt +++ b/plugins/package-managers/conan/src/main/kotlin/Conan.kt @@ -311,7 +311,7 @@ class Conan( id = id, authors = parseAuthors(pkgInfo), declaredLicenses = pkgInfo.license.toSet(), - description = parsePackageField(pkgInfo, workingDir, "description"), + description = inspectField(pkgInfo.displayName, workingDir, "description"), homepageUrl = homepageUrl, binaryArtifact = RemoteArtifact.EMPTY, // TODO: implement me! sourceArtifact = parseSourceArtifact(conanData), @@ -356,8 +356,8 @@ class Conan( Identifier( type = "Conan", namespace = "", - name = parsePackageField(pkgInfo, workingDir, "name"), - version = parsePackageField(pkgInfo, workingDir, "version") + name = inspectField(pkgInfo.displayName, workingDir, "name"), + version = inspectField(pkgInfo.displayName, workingDir, "version") ) /** @@ -370,12 +370,6 @@ class Conan( return if (revision == "0") vcsInfo else vcsInfo.copy(revision = revision) } - /** - * Return the value of [field] from the output of `conan inspect --raw` for the package in [pkgInfo]. - */ - private fun parsePackageField(pkgInfo: PackageInfo, workingDir: File, field: String): String = - inspectField(pkgInfo.displayName, workingDir, field) - /** * Return the source artifact contained in [conanData], or [RemoteArtifact.EMPTY] if no source artifact is * available. From 10e16defa7b16e1b87f89f6c3cf00801bbee2bf0 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Thu, 18 Jul 2024 23:35:08 +0200 Subject: [PATCH 6/7] refactor(conan): Rename `inspectOrNull()` to `inspectPyFile()` Make clear from the name that the type of Conan file plays a role. Signed-off-by: Sebastian Schuberth --- plugins/package-managers/conan/src/main/kotlin/Conan.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/package-managers/conan/src/main/kotlin/Conan.kt b/plugins/package-managers/conan/src/main/kotlin/Conan.kt index fbac01bf282ee..f53939c831f28 100644 --- a/plugins/package-managers/conan/src/main/kotlin/Conan.kt +++ b/plugins/package-managers/conan/src/main/kotlin/Conan.kt @@ -392,19 +392,19 @@ class Conan( * from the `requires` field. Need to investigate whether this is a sure thing before implementing. */ private fun generateProjectPackage(pkgInfo: PackageInfo, definitionFile: File, workingDir: File): Package { - fun inspectOrNull(field: String) = + fun inspectPyFile(field: String) = definitionFile.name.takeIf { it == "conanfile.py" }?.let { inspectField(it, workingDir, field) } return Package( id = Identifier( type = managerName, namespace = "", - name = inspectOrNull("name") ?: pkgInfo.reference.orEmpty(), - version = inspectOrNull("version").orEmpty() + name = inspectPyFile("name") ?: pkgInfo.reference.orEmpty(), + version = inspectPyFile("version").orEmpty() ), authors = parseAuthors(pkgInfo), declaredLicenses = pkgInfo.license.toSet(), - description = inspectOrNull("description").orEmpty(), + description = inspectPyFile("description").orEmpty(), homepageUrl = pkgInfo.homepage.orEmpty(), binaryArtifact = RemoteArtifact.EMPTY, // TODO: implement me! sourceArtifact = RemoteArtifact.EMPTY, // TODO: implement me! From 0f9e680172dcfdcb1b44384400349e96adc64b12 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Thu, 18 Jul 2024 23:39:06 +0200 Subject: [PATCH 7/7] refactor(conan): Make the return type of `inspectField()` nullable Let the caller decide how to handle a non-existing field, for which null is a more natural value than an empty string. This now also better fits the `inspectPyFile()` use-case which already is nullable. Signed-off-by: Sebastian Schuberth --- .../conan/src/main/kotlin/Conan.kt | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/plugins/package-managers/conan/src/main/kotlin/Conan.kt b/plugins/package-managers/conan/src/main/kotlin/Conan.kt index f53939c831f28..16c54734520c5 100644 --- a/plugins/package-managers/conan/src/main/kotlin/Conan.kt +++ b/plugins/package-managers/conan/src/main/kotlin/Conan.kt @@ -311,7 +311,7 @@ class Conan( id = id, authors = parseAuthors(pkgInfo), declaredLicenses = pkgInfo.license.toSet(), - description = inspectField(pkgInfo.displayName, workingDir, "description"), + description = inspectField(pkgInfo.displayName, workingDir, "description").orEmpty(), homepageUrl = homepageUrl, binaryArtifact = RemoteArtifact.EMPTY, // TODO: implement me! sourceArtifact = parseSourceArtifact(conanData), @@ -321,10 +321,10 @@ class Conan( } /** - * Return the value `conan inspect` reports for the given [field]. + * Return the value `conan inspect` reports for the given [field], or null if the field does not exist. */ - private fun inspectField(pkgName: String, workingDir: File, field: String): String = - pkgInspectResults.getOrPut(pkgName) { + private fun inspectField(pkgName: String, workingDir: File, field: String): String? { + val results = pkgInspectResults.getOrPut(pkgName) { // Note: While Conan 2 supports inspect output to stdout, Conan 1 does not and a temporary file is required, // see https://github.com/conan-io/conan/issues/6972. val jsonFile = createOrtTempDir().resolve("inspect.json") @@ -332,7 +332,10 @@ class Conan( Json.parseToJsonElement(jsonFile.readText()).jsonObject.also { jsonFile.parentFile.safeDeleteRecursively(force = true) } - }[field]?.jsonPrimitive?.content.orEmpty() + } + + return results[field]?.jsonPrimitive?.content + } /** * Find the [PackageInfo] that represents the project defined in the definition file. @@ -356,8 +359,8 @@ class Conan( Identifier( type = "Conan", namespace = "", - name = inspectField(pkgInfo.displayName, workingDir, "name"), - version = inspectField(pkgInfo.displayName, workingDir, "version") + name = inspectField(pkgInfo.displayName, workingDir, "name").orEmpty(), + version = inspectField(pkgInfo.displayName, workingDir, "version").orEmpty() ) /**