Make packagemanager PNPM configurable #11200

thomasscheer · 2025-12-08T15:31:42Z

To be able to configure the PNPM packagemanager via config.yml or ort.yml, it is necessary to provide the Config in Pnpm.kt

Signed-off-by: Thomas Scheer <scheer@semvox.de>

fviernau · 2025-12-08T15:36:58Z

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

+     * Ignore "pnpmfile" when analyzing projects. Defaults to ""--ignore-scripts"
+     */
+    @OrtPluginOption(defaultValue = "true")
+    val ignore_scripts: Boolean,


Would you have a rationale, why this needs to be configurable?
In particular, in which cases would one want to set these to false?

In particular, in which cases would one want to set these to false?

I've seen cases where scripts generate files for other package managers in a multi-package-manager-setup.

Can you please share one such case?

And also, are you sure this realted to --ignore-scripts, which to my understanding is for post installation, which in turn is for dependencies, not for (ORT) projects. And multi-package-manager-setup to me sounds like a project thing.

Can you please share one such case?

Unfortunately not, it has been a private repository. But this project has a similar concept where react-native integration is run from scripts.

And also, are you sure this realted to --ignore-scripts, which to my understanding is for post installation,

I'm not sure, but the docs are very generic and say "Do not execute any scripts defined in the project package.json and its dependencies" (emphasis mine), so it sounds not to be limited to post-install scripts.

And multi-package-manager-setup to me sounds like a project thing.

I'm not sure what that means. There certainly are project setups where you need to know in which order to build things to get the right results, i.e. use ORT's mustRunAfter functionality.

What's the problem with making this configurable?

In our case, the pnpmfile-mechanism is used to generate code during the build. The pnpm install won't work with --ignore-pnpmfile.

I added the second variable only for completness.

What's the problem with making this configurable?

If it's needed to make it configurable, there is no problem at all.
I just wanted to understand the need. If there is no need I believe then it shouldn't be done.
But for ignore-scripts I found further evidence that configuration makes sense, see https://www.npmjs.com/package/can-i-ignore-scripts.

How is the need for ignore_pnpmfile? Does this have to be configurable, or could it always be ignored?

I guess we generally should make things configurable if they deviate from the package manager's default. PNPM's default is to not ignore scripts, and if we'd hard-code to do ignore script, that might be different from what users expect.

I guess we generally should make things configurable if they deviate from the package manager's default.

I believe we used to follow the rule to make things configurable only if needed, and not if it deviates from default.
It would be relatively easy to come up with a list of counter examples, invalidating this rule. For exmaple, we do set the node-linker and this should not be configurable.

fviernau · 2025-12-08T15:37:22Z

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

+    /**
+     * The version of PNPM to use when analyzing projects. Defaults to the version 10.24.0
+     */
+    val pnpmVersion: String?,


This seems to be unused.

sschuberth · 2025-12-08T15:40:23Z

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

+     * Ignore "pnpmfile" when analyzing projects. Defaults to "--ignore-pnpmfile"
+     */
+    @OrtPluginOption(defaultValue = "true")
+    val ignore_pnpmfile: Boolean,


Please stick to camel-case option names.

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

+    /**
+     * Ignore "pnpmfile" when analyzing projects. Defaults to "--ignore-pnpmfile"
+     */
+    @OrtPluginOption(defaultValue = "true")
+    val ignore_pnpmfile: Boolean,


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

+    /**
+     * Ignore "pnpmfile" when analyzing projects. Defaults to ""--ignore-scripts"
+     */
+    @OrtPluginOption(defaultValue = "true")
+    val ignore_scripts: Boolean,


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

+     * Ignore "pnpmfile" when analyzing projects. Defaults to ""--ignore-scripts"
+     */
+    @OrtPluginOption(defaultValue = "true")
+    val ignore_scripts: Boolean,


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

+    @OrtPluginOption(defaultValue = "true")
+    val ignore_scripts: Boolean,
+
+


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

+    @OrtPluginOption(defaultValue = "true")
+    val ignore_scripts: Boolean,
+
+


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

    factory = PackageManagerFactory::class
 )
-class Pnpm(override val descriptor: PluginDescriptor = PnpmFactory.descriptor) :
+class Pnpm(override val descriptor: PluginDescriptor = PnpmFactory.descriptor, private val config: PnPMConfig) : 


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

-class Pnpm(override val descriptor: PluginDescriptor = PnpmFactory.descriptor) :
+class Pnpm(override val descriptor: PluginDescriptor = PnpmFactory.descriptor, private val config: PnPMConfig) : 
    NodePackageManager(NodePackageManagerType.PNPM) {
+


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

    }

    private fun installDependencies(workingDir: File, scopes: Collection<Scope>) {
+


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

    }

    private fun installDependencies(workingDir: File, scopes: Collection<Scope>) {
+


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

+     * Ignore "pnpmfile" when analyzing projects. Defaults to "--ignore-pnpmfile"
+     */
+    @OrtPluginOption(defaultValue = "true")
+    val ignore_pnpmfile: Boolean,


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

+     * Ignore "pnpmfile" when analyzing projects. Defaults to ""--ignore-scripts"
+     */
+    @OrtPluginOption(defaultValue = "true")
+    val ignore_scripts: Boolean,


codecov · 2025-12-08T15:55:19Z

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.48%. Comparing base (86da192) to head (ac51bc0).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...package-managers/node/src/main/kotlin/pnpm/Pnpm.kt	77.77%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##               main   #11200   +/-   ##
=========================================
  Coverage     57.47%   57.48%           
  Complexity     1704     1704           
=========================================
  Files           346      346           
  Lines         12855    12861    +6     
  Branches       1222     1224    +2     
=========================================
+ Hits           7389     7393    +4     
  Misses         4992     4992           
- Partials        474      476    +2

Flag	Coverage Δ
funTest-external-tools	`13.75% <77.77%> (+0.04%)`	⬆️
funTest-no-external-tools	`30.99% <0.00%> (-0.03%)`	⬇️
test-ubuntu-24.04	`42.47% <55.55%> (+0.01%)`	⬆️
test-windows-2025	`42.45% <55.55%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

fviernau · 2025-12-08T21:26:22Z

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

+    val pnpmVersion: String?,
+
+    /**
+     * Ignore "pnpmfile" when analyzing projects. Defaults to "--ignore-pnpmfile"


If the KDoc mention the default value, it should imho use the Boolean type.

thomasscheer and others added 2 commits December 8, 2025 15:51

spdx-utils: Make PNPM package manager configurable

0044d77

Signed-off-by: Thomas Scheer <scheer@semvox.de>

Merge branch 'oss-review-toolkit:main' into main

ac51bc0

thomasscheer requested a review from a team as a code owner December 8, 2025 15:31

fviernau reviewed Dec 8, 2025

View reviewed changes

sschuberth reviewed Dec 8, 2025

View reviewed changes

github-advanced-security bot found potential problems Dec 8, 2025

View reviewed changes

fviernau reviewed Dec 8, 2025

View reviewed changes

		@OrtPluginOption(defaultValue = "true")
		val ignore_scripts: Boolean,

		}

		private fun installDependencies(workingDir: File, scopes: Collection<Scope>) {

Make packagemanager PNPM configurable #11200

Are you sure you want to change the base?

Make packagemanager PNPM configurable #11200

Uh oh!

Conversation

thomasscheer commented Dec 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fviernau Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check notice

Check notice

codecov bot commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fviernau Dec 9, 2025 •

edited

Loading

codecov bot commented Dec 8, 2025 •

edited

Loading