wp-cli / extension-command

Manages plugins and themes, including installs, activations, and updates.
MIT License
87 stars 82 forks source link

Fatal because --exclude does not exclude for version request with --all #412

Closed kkmuffme closed 1 month ago

kkmuffme commented 3 months ago

Bug Report

Describe the current, buggy behavior

wp theme update --all --exclude=foo wp plugin update --all --exclude=foo

ParseThemeNameInput::check_optional_args_and_all (same for plugin) will try to get update info via themes_api for all themes/updates even the excluded ones. This is a problem if the theme/plugin is excluded because it will make the updater crash e.g. if it hooks onto filter themes_api_args in the themes_api function, which leads to nothing getting updated (since the add_filter function has a fatal error)

Describe what you would expect as the correct outcome

Don't check excluded plugins/themes for newer version/updates.

Let us know what environment you are running this on

WP CLI 2.10.0

Provide a possible solution

Change check_optional_args_and_all to skip excluded in $all case?

danielbachhuber commented 3 months ago

Thanks for the report, @kkmuffme ! Feel free to submit a pull request, if you'd like. Here is some guidance on our pull request best practices.

tfirdaus commented 3 months ago

Hey @danielbachhuber @kkmuffme, I couldn't replicate this issue. I tried it on both a live site with WP-CLI 2.10.0 and in a Behat test. Everything seems to be working fine. It updated some plugins and skipped the one specified in the --exclude parameter. I didn't encounter any fatal errors.

A live site

Screenshot 2024-04-08 at 10 43 07 PM

Behat test

@require-wp-5.2
  Scenario: Updating all plugins with some excluded
    Given a WP install
    And I run `wp plugin install wordpress-importer --version=0.8 --force`
    And I run `wp plugin install classic-editor --version=1.0 --force`

    When I try `wp plugin update --all --exclude=wordpress-importer`
    Then STDOUT should contain:
      """
      Success: Updated 1 of 1 plugins.
      Skipped updates for: wordpress-importer
      """

Result

Screenshot 2024-04-08 at 10 46 34 PM
ernilambar commented 3 months ago

I also tried with themes but could not reproduce the error.

wp theme install niya --version=1.0.0 --force
wp theme install obulma --version=1.0.5 --force
wp theme install simple-life --version=2.5.1 --force

Then:

wp theme update --all --exclude=niya                
Downloading update from https://downloads.wordpress.org/theme/obulma.1.0.11.zip...
Using cached file '/Users/nilambarsharma/.wp-cli/cache/theme/obulma-1.0.11.zip'...
Unpacking the update...
Installing the latest version...
Removing the old version of the theme...
Theme updated successfully.
Downloading update from https://downloads.wordpress.org/theme/simple-life.2.5.3.zip...
Using cached file '/Users/nilambarsharma/.wp-cli/cache/theme/simple-life-2.5.3.zip'...
Unpacking the update...
Installing the latest version...
Removing the old version of the theme...
Theme updated successfully.
+-------------+-------------+-------------+---------+
| name        | old_version | new_version | status  |
+-------------+-------------+-------------+---------+
| obulma      | 1.0.5       | 1.0.11      | Updated |
| simple-life | 2.5.1       | 2.5.3       | Updated |
+-------------+-------------+-------------+---------+
Success: Updated 2 of 2 themes.
Skipped updates for: niya
marksabbath commented 2 months ago

I think that I understand the problem. It feels like the plugin/theme in question is throwing a fatal error at the moment of the update. While inspecting the code, I was able to determine that the plugins/themes in the --exclude are properly removed before any attempt of the update.

What probably is causing this issue is that the plugin/theme is loading when the wp plugin update runs (and probably other commands), which as expected should show an error.

@kkmuffme have you tried to run that command appending at the end of that --skip-themes --skip-plugins? This might update your plugins (excluding the ones set in the --exclude) even though a plugin/theme is causing issues in WordPress.