[cordova-cli #578] plugin/remove.js: don't stomp opts.cli_variables in removePluginFromPlatform()

sebastian-onlea commented 1 year ago

Platforms affected

CLI - adding and removing plugins with differing required variables per platform (e.g. cordova-plugin-googleplus )

Motivation and Context

resolves apache/cordova-cli#578 : cordova plugin rm plugin-package --variable "IOS_ONLY_REQUIRED_VARIABLE=variable-value" will fail if the project includes e.g. both android and ios platforms.

Description

In removePluginFromPlatform(), at cordova/plugin/remove.js:105, opts.cli_variables is destructively reassigned to the result of mergeVariables(). As mergeVariables() selects only keys specified for a specific platform, this can result in CLI-specified values being lost before the platform that requires them is processed.

In the change, instead call uninstallPlatform() with a copy of opts that includes the platform-specific mergeVariables() result.

Testing

Ran existing tests (npm run test)
verified the change fixes my local use case by npm linking

Checklist

[x] I've run the tests to see all new and existing tests pass
[ ] I added automated test coverage as appropriate for this change

I'm not familiar enough with how to create a reproduction case in the test library for this project.
[x] Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
[x] If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
[x] I've updated the documentation if necessary

No doc update should be required

breautek commented 1 year ago

Thank you for the PR.

We generally do not make back ports releases. If applicable, the PR should target the master branch so that when merged it will be included in our next release. I'm not sure if GitHub allows you to re-target a PR, it may need to be closed and recreated.

sebastian-onlea commented 1 year ago

Will do! I've been working on the released version 11 and I wasn't sure whether to target the patch branch or master. Thanks.

apache / cordova-lib