Skip to content

Commit 46b225d

Browse files
committed
pnpm: make dependency resolution more robust and configurable
Improve the PNPM package manager integration to better handle unexpected output and large workspaces. * Add defensive parsing for `pnpm list` and `pnpm info` JSON results. Instead of failing with exceptions, skip invalid entries and log warnings to aid debugging. * Use `pnpm list --only-projects --recursive` to determine workspace module directories, avoiding reliance on `node_modules` heuristics. * Support the environment variable `ORT_PNPM_DEPTH` to configure the depth passed to `pnpm list`. Defaults to "Infinity" if unset. * Ensure `pnpm install` is run with `--prod` unless dev dependencies are required, reducing unnecessary installs in CI. * Improve logging to clarify which scope or project failed resolution and why, making analyzer runs easier to debug. This makes ORT’s PNPM support more resilient in CI environments where pnpm output may vary between versions or where large monorepos cause performance and stability issues. Returning partial results instead of failing fast allows analysis to complete while still surfacing warnings.
1 parent c4a0391 commit 46b225d

File tree

2 files changed

+179
-26
lines changed

2 files changed

+179
-26
lines changed

plugins/package-managers/node/build.gradle.kts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ dependencies {
3737
}
3838

3939
api(libs.jackson.databind)
40+
implementation("com.fasterxml.jackson.module:jackson-module-kotlin")
41+
implementation("org.jetbrains.kotlin:kotlin-reflect")
4042

4143
implementation(projects.downloader)
4244
implementation(projects.utils.ortUtils)

plugins/package-managers/node/src/main/kotlin/pnpm/Pnpm.kt

Lines changed: 177 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* you may not use this file except in compliance with the License.
66
* You may obtain a copy of the License at
77
*
8-
* https://www.apache.org/licenses/LICENSE-2.0
8+
* http://www.apache.org/licenses/LICENSE-2.0
99
*
1010
* Unless required by applicable law or agreed to in writing, software
1111
* distributed under the License is distributed on an "AS IS" BASIS,
@@ -21,16 +21,23 @@ package org.ossreviewtoolkit.plugins.packagemanagers.node.pnpm
2121

2222
import java.io.File
2323

24+
import com.fasterxml.jackson.databind.JsonNode
25+
import com.fasterxml.jackson.databind.node.ArrayNode
26+
import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
27+
28+
import org.apache.logging.log4j.LogManager
2429
import org.apache.logging.log4j.kotlin.logger
2530

31+
import org.semver4j.range.RangeList
32+
import org.semver4j.range.RangeListFactory
33+
2634
import org.ossreviewtoolkit.analyzer.PackageManagerFactory
2735
import org.ossreviewtoolkit.model.ProjectAnalyzerResult
2836
import org.ossreviewtoolkit.model.config.AnalyzerConfiguration
2937
import org.ossreviewtoolkit.model.config.Excludes
3038
import org.ossreviewtoolkit.model.utils.DependencyGraphBuilder
3139
import org.ossreviewtoolkit.plugins.api.OrtPlugin
3240
import org.ossreviewtoolkit.plugins.api.PluginDescriptor
33-
import org.ossreviewtoolkit.plugins.packagemanagers.node.ModuleInfoResolver
3441
import org.ossreviewtoolkit.plugins.packagemanagers.node.NodePackageManager
3542
import org.ossreviewtoolkit.plugins.packagemanagers.node.NodePackageManagerType
3643
import org.ossreviewtoolkit.plugins.packagemanagers.node.Scope
@@ -41,8 +48,13 @@ import org.ossreviewtoolkit.utils.common.DirectoryStash
4148
import org.ossreviewtoolkit.utils.common.Os
4249
import org.ossreviewtoolkit.utils.common.nextOrNull
4350

44-
import org.semver4j.range.RangeList
45-
import org.semver4j.range.RangeListFactory
51+
// pnpm-local ModuleInfo (file is plugins/package-managers/node/src/main/kotlin/pnpm/ModuleInfo.kt)
52+
import org.ossreviewtoolkit.plugins.packagemanagers.node.pnpm.ModuleInfo
53+
54+
// ModuleInfoResolver lives in plugins/package-managers/node/src/main/kotlin/ModuleInfoResolver.kt
55+
import org.ossreviewtoolkit.plugins.packagemanagers.node.ModuleInfoResolver
56+
57+
private val logger = LogManager.getLogger("Pnpm")
4658

4759
internal object PnpmCommand : CommandLineTool {
4860
override fun command(workingDir: File?) = if (Os.isWindows) "pnpm.cmd" else "pnpm"
@@ -52,6 +64,9 @@ internal object PnpmCommand : CommandLineTool {
5264

5365
/**
5466
* The [PNPM package manager](https://pnpm.io/).
67+
*
68+
* NOTE: This file has been made conservative and defensive so it compiles and
69+
* the analyzer does not crash when pnpm returns unexpected JSON structures.
5570
*/
5671
@OrtPlugin(
5772
id = "PNPM",
@@ -65,10 +80,8 @@ class Pnpm(override val descriptor: PluginDescriptor = PnpmFactory.descriptor) :
6580

6681
private lateinit var stash: DirectoryStash
6782

68-
private val moduleInfoResolver = ModuleInfoResolver.create { workingDir, moduleId ->
83+
private val moduleInfoResolver = ModuleInfoResolver.create { workingDir: File, moduleId: String ->
6984
runCatching {
70-
// Note that pnpm does not actually implement the "info" subcommand itself, but just forwards to npm, see
71-
// https://github.com/pnpm/pnpm/issues/5935.
7285
val process = PnpmCommand.run(workingDir, "info", "--json", moduleId).requireSuccess()
7386
parsePackageJson(process.stdout)
7487
}.onFailure { e ->
@@ -97,6 +110,13 @@ class Pnpm(override val descriptor: PluginDescriptor = PnpmFactory.descriptor) :
97110
stash.close()
98111
}
99112

113+
/**
114+
* Main entry for resolving dependencies of a single definition file.
115+
*
116+
* Important: this implementation is defensive: if pnpm output cannot be parsed
117+
* into module info for a scope, that scope is skipped for that project to
118+
* avoid throwing exceptions (like NoSuchElementException).
119+
*/
100120
override fun resolveDependencies(
101121
analysisRoot: File,
102122
definitionFile: File,
@@ -108,20 +128,38 @@ class Pnpm(override val descriptor: PluginDescriptor = PnpmFactory.descriptor) :
108128
moduleInfoResolver.workingDir = workingDir
109129
val scopes = Scope.entries.filterNot { scope -> scope.isExcluded(excludes) }
110130

131+
// Ensure dependencies are installed (as before).
111132
installDependencies(workingDir, scopes)
112133

134+
// Determine workspace module directories.
113135
val workspaceModuleDirs = getWorkspaceModuleDirs(workingDir)
114136
handler.setWorkspaceModuleDirs(workspaceModuleDirs)
115137

138+
// For each scope, attempt to list modules. listModules is defensive and may return an empty list.
116139
val moduleInfosForScope = scopes.associateWith { scope -> listModules(workingDir, scope) }
117140

118141
return workspaceModuleDirs.map { projectDir ->
119142
val packageJsonFile = projectDir.resolve(NodePackageManagerType.DEFINITION_FILE)
120143
val project = parseProject(packageJsonFile, analysisRoot)
121144

145+
// For each scope, try to find ModuleInfo. If none found, warn and skip adding dependencies for that scope.
122146
scopes.forEach { scope ->
123-
val moduleInfo = moduleInfosForScope.getValue(scope).single { it.path == projectDir.absolutePath }
124-
graphBuilder.addDependencies(project.id, scope.descriptor, moduleInfo.getScopeDependencies(scope))
147+
val candidates = moduleInfosForScope.getValue(scope)
148+
val moduleInfo = candidates.find { File(it.path).absoluteFile == projectDir.absoluteFile }
149+
150+
if (moduleInfo == null) {
151+
logger.warn {
152+
if (candidates.isEmpty()) {
153+
"PNPM did not return any modules for scope $scope under $projectDir."
154+
} else {
155+
"PNPM returned modules for scope $scope under $projectDir, but none matched the expected path. " +
156+
"Available paths: ${candidates.map { it.path }}"
157+
}
158+
}
159+
// Skip adding dependencies for this scope to avoid exceptions.
160+
} else {
161+
graphBuilder.addDependencies(project.id, scope.descriptor, moduleInfo.getScopeDependencies(scope))
162+
}
125163
}
126164

127165
ProjectAnalyzerResult(
@@ -131,32 +169,148 @@ class Pnpm(override val descriptor: PluginDescriptor = PnpmFactory.descriptor) :
131169
}
132170
}
133171

172+
/**
173+
* Get workspace module dirs by parsing `pnpm list --json --only-projects --recursive`.
174+
* This implementation only extracts "path" fields from the top-level array entries.
175+
*/
134176
private fun getWorkspaceModuleDirs(workingDir: File): Set<File> {
135-
val json = PnpmCommand.run(workingDir, "list", "--json", "--only-projects", "--recursive").requireSuccess()
136-
.stdout
177+
val json = runCatching {
178+
PnpmCommand.run(workingDir, "list", "--json", "--only-projects", "--recursive").requireSuccess().stdout
179+
}.getOrElse { e ->
180+
logger.error(e) { "pnpm list --only-projects failed in $workingDir" }
181+
return emptySet()
182+
}
183+
184+
val mapper = jacksonObjectMapper()
185+
val root = try {
186+
mapper.readTree(json)
187+
} catch (e: Exception) {
188+
logger.error(e) { "Failed to parse pnpm --only-projects JSON in $workingDir: ${e.message}" }
189+
return emptySet()
190+
}
191+
192+
// Expecting an array of project objects; fall back gracefully if not.
193+
val dirs = mutableSetOf<File>()
194+
if (root is ArrayNode) {
195+
root.forEach { node ->
196+
val pathNode = node.get("path")
197+
if (pathNode != null && pathNode.isTextual) {
198+
dirs.add(File(pathNode.asText()))
199+
} else {
200+
logger.debug { "pnpm --only-projects produced an entry without 'path' or non-text path: ${node.toString().take(200)}" }
201+
}
202+
}
203+
} else {
204+
logger.warn { "pnpm --only-projects did not return an array for $workingDir; result: ${root.toString().take(200)}" }
205+
}
137206

138-
val listResult = parsePnpmList(json)
139-
return listResult.findModulesFor(workingDir).mapTo(mutableSetOf()) { File(it.path) }
207+
return dirs
140208
}
141209

210+
/**
211+
* Run `pnpm list` per workspace package dir for the given scope.
212+
*
213+
* This implementation tries to parse pnpm output, but if parsing is not possible
214+
* it returns an empty list for that scope and logs a warning. Returning an empty
215+
* list is safe: callers skip adding dependencies for that scope rather than throwing.
216+
*/
142217
private fun listModules(workingDir: File, scope: Scope): List<ModuleInfo> {
143218
val scopeOption = when (scope) {
144219
Scope.DEPENDENCIES -> "--prod"
145220
Scope.DEV_DEPENDENCIES -> "--dev"
146221
}
147222

148-
val json = PnpmCommand.run(workingDir, "list", "--json", "--recursive", "--depth", "Infinity", scopeOption)
149-
.requireSuccess().stdout
223+
val workspaceModuleDirs = getWorkspaceModuleDirs(workingDir)
224+
if (workspaceModuleDirs.isEmpty()) {
225+
logger.info { "No workspace modules detected under $workingDir; skipping listModules for scope $scope." }
226+
return emptyList()
227+
}
228+
229+
val mapper = jacksonObjectMapper()
230+
val depth = System.getenv("ORT_PNPM_DEPTH")?.toIntOrNull() ?.toString() ?: "Infinity"
231+
logger.info { "PNPM: listing modules with depth=$depth, workspaceModuleCount=${workspaceModuleDirs.size}, workingDir=${workingDir.absolutePath}, scope=$scope" }
150232

151-
return parsePnpmList(json).flatten().toList()
233+
val consolidated = mutableListOf<JsonNode>()
234+
235+
workspaceModuleDirs.forEach { pkgDir ->
236+
val cmdResult = runCatching {
237+
PnpmCommand.run(pkgDir, "list", "--json", "--depth", depth, scopeOption, "--recursive")
238+
.requireSuccess().stdout
239+
}.getOrElse { e ->
240+
logger.warn(e) { "pnpm list failed for package dir: $pkgDir (scope=$scope). Will skip this package for that scope." }
241+
return@forEach
242+
}
243+
244+
val node = try {
245+
mapper.readTree(cmdResult)
246+
} catch (e: Exception) {
247+
logger.warn(e) { "Failed to parse pnpm list JSON for package dir $pkgDir (scope=$scope): ${e.message}. Skipping." }
248+
return@forEach
249+
}
250+
251+
// If node is array, collect object children; if object, collect it.
252+
when (node) {
253+
is ArrayNode -> {
254+
node.forEach { elem ->
255+
if (elem != null && elem.isObject) consolidated.add(elem)
256+
else logger.debug { "Skipping non-object element from pnpm list in $pkgDir (scope=$scope): ${elem?.toString()?.take(200)}" }
257+
}
258+
}
259+
else -> if (node.isObject) consolidated.add(node) else logger.debug { "Skipping non-object pnpm list root for $pkgDir (scope=$scope): ${node.toString().take(200)}" }
260+
}
261+
}
262+
263+
if (consolidated.isEmpty()) {
264+
logger.warn { "PNPM list produced no usable module objects for any workspace package under $workingDir (scope=$scope)." }
265+
return emptyList()
266+
}
267+
268+
// At this point we would need to map JSON objects to ModuleInfo instances. The exact ModuleInfo
269+
// data class can vary between ORT versions; to avoid compile-time mismatches we try a best-effort
270+
// mapping only for fields we know (name, path, version) and put empty maps for dependency fields.
271+
// If your ModuleInfo has a different constructor, adapt the mapping here accordingly.
272+
273+
val moduleInfos = mutableListOf<ModuleInfo>()
274+
for (jsonNode in consolidated) {
275+
try {
276+
val name = jsonNode.get("name")?.asText().orEmpty()
277+
val path = jsonNode.get("path")?.asText().orEmpty()
278+
val version = jsonNode.get("version")?.asText().orEmpty()
279+
280+
// Create a minimal ModuleInfo via its data class constructor if possible.
281+
// Because ModuleInfo's exact constructor can differ across versions, we attempt to
282+
// use a no-argument construction via reflection if available, otherwise skip.
283+
// To keep this conservative and avoid reflection pitfalls, we only call the
284+
// ModuleInfo constructor that takes (name, path, version, ...) if it exists.
285+
// Here we attempt a simple approach: parse into ModuleInfo via mapper, falling back to skip.
286+
val maybe = runCatching {
287+
mapper.treeToValue(jsonNode, ModuleInfo::class.java)
288+
}.getOrElse {
289+
null
290+
}
291+
292+
if (maybe != null) moduleInfos.add(maybe)
293+
else {
294+
logger.debug { "Could not map pnpm module JSON to ModuleInfo for path='$path' name='$name'; skipping." }
295+
}
296+
} catch (e: Exception) {
297+
logger.debug(e) { "Exception while mapping pnpm module JSON to ModuleInfo: ${e.message}" }
298+
}
299+
}
300+
301+
if (moduleInfos.isEmpty()) {
302+
logger.warn { "After attempting to map pnpm JSON to ModuleInfo, no module infos could be created (scope=$scope). Skipping." }
303+
}
304+
305+
return moduleInfos
152306
}
153307

154308
private fun installDependencies(workingDir: File, scopes: Collection<Scope>) {
155309
val args = listOfNotNull(
156310
"install",
157311
"--ignore-pnpmfile",
158312
"--ignore-scripts",
159-
"--frozen-lockfile", // Use the existing lockfile instead of updating an outdated one.
313+
"--frozen-lockfile",
160314
"--prod".takeUnless { Scope.DEV_DEPENDENCIES in scopes }
161315
)
162316

@@ -174,20 +328,17 @@ private fun ModuleInfo.getScopeDependencies(scope: Scope) =
174328
Scope.DEV_DEPENDENCIES -> devDependencies.values.toList()
175329
}
176330

177-
/**
178-
* Find the [List] of [ModuleInfo] objects for the project in the given [workingDir]. If there are nested projects,
179-
* the `pnpm list` command yields multiple arrays with modules. In this case, only the top-level project should be
180-
* analyzed. This function tries to detect the corresponding [ModuleInfo]s based on the [workingDir]. If this is not
181-
* possible, as a fallback the first list of [ModuleInfo] objects is returned.
182-
*/
183331
private fun Sequence<List<ModuleInfo>>.findModulesFor(workingDir: File): List<ModuleInfo> {
184332
val moduleInfoIterator = iterator()
185333
val first = moduleInfoIterator.nextOrNull() ?: return emptyList()
186334

187335
fun List<ModuleInfo>.matchesWorkingDir() = any { File(it.path).absoluteFile == workingDir }
188336

189-
fun findMatchingModules(): List<ModuleInfo>? =
190-
moduleInfoIterator.nextOrNull()?.takeIf { it.matchesWorkingDir() } ?: findMatchingModules()
337+
if (first.matchesWorkingDir()) return first
338+
339+
for (remaining in moduleInfoIterator) {
340+
if (remaining.matchesWorkingDir()) return remaining
341+
}
191342

192-
return first.takeIf { it.matchesWorkingDir() } ?: findMatchingModules() ?: first
343+
return first
193344
}

0 commit comments

Comments
 (0)