Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix wp-cli incompatibility #1387

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix wp-cli incompatibility #1387

wants to merge 1 commit into from

Conversation

IJMacD
Copy link

@IJMacD IJMacD commented Nov 8, 2023

As mentioned in #1344 the plugin is incompatible with wp-cli.

For example, the following command fails:

wp-cli plugin activate qtranslate-xt

Any action with wp-cli results in an error such as the one below, where functions haven't been defined.

call_user_func(): Argument #1 ($callback) must be a valid callback

In my case the missing function happened to be function "qtranxf_default_default_language" not found.

The reason for this, is the functions were defined in /src/admin/activation_hook.php which isn't included when the WP function is_admin() returns false, as it does in wp-cli. Any user who has access to the wp-cli binary clearly has administrative privileges over the installation, so not including those function definitions is presumably not the intended behviour.

This pull request ensures those functions have been defined if the WP_CLI php const is defined, fixing compatibility with wp-cli.

call_user_func(): Argument qtranslate#1 ($callback) must be a valid callback
@icaliman
Copy link

+1 for this

@hirasso
Copy link

hirasso commented Jan 9, 2024

@herrvigg are you still maintaining qtranslate-xt? It would be amazing to get this fixed sometime 🙏

@herrvigg
Copy link
Collaborator

Sorry for late answer, I've been extremely busy these last months.
Great you found this fix!

@herrvigg herrvigg added the core Core functionalities, including the admin section label Jan 13, 2024
@herrvigg
Copy link
Collaborator

Before merge I need to understand better the different issues. I don't manage to reproduce any of the cases.

@IJMacD qtranxf_default_default_language is included in activation_hooks.php. The PR makes a change to include it with WP_CLI. But how could this function be invoked before the change, if the hooks were not even loaded? If I run wp plugin calls I see no issue.

@hirasso does this fix #1344? The problem described there seems to be while loading front filters. If I run wp core update I see no issue.

@hirasso
Copy link

hirasso commented Jan 13, 2024

Great to have you back @herrvigg 🤠

I'll try to provide more info tomorrow.

@IJMacD
Copy link
Author

IJMacD commented Jan 14, 2024

@herrvigg Thanks for getting back to us on this.

I have tried to create a minimal test case.

https://gist.github.com/IJMacD/683b0f6b4413b779d501fa53269e78dd

In this gist are two docker compose files. One installs the plugin from the qtranslate/qtranslate-xt master branch; the other from IJMacD/qtranslate-xt master branch (the pull request branch). (Line 53 in the gist files)

The plugin installs and activates correctly in both versions; however any subsequent wp-cli command will fail.

I haven't investigated deeper as to why the hooks get called, but it seems wp-cli does the action plugins_loaded which qtranslate registers a filter for.

Run the gists as:

docker compose -f .\docker-compose.yaml up

And be sure to delete volumes when taking it down.

docker compose -f .\docker-compose.yaml down --volumes
bin-wordpress-cli-1  | Success: WordPress installed successfully.
bin-wordpress-cli-1  | Downloading installation package from https://github.com/qtranslate/qtranslate-xt/archive/refs/tags/3.15.2.zip...
bin-wordpress-cli-1  | Unpacking the package...
bin-wordpress-cli-1  | Installing the plugin...
bin-wordpress-cli-1  | Renamed Github-based project from 'qtranslate-xt-3.15.2' to 'qtranslate-xt'.
bin-wordpress-cli-1  | Plugin installed successfully.
bin-wordpress-cli-1  | Activating 'qtranslate-xt'...
bin-wordpress-cli-1  | Plugin 'qtranslate-xt' activated.
bin-wordpress-cli-1  | Success: Installed 1 of 1 plugins.
bin-wordpress-cli-1  | [14-Jan-2024 04:12:06 UTC] PHP Fatal error:  Uncaught TypeError: call_user_func(): Argument #1 ($callback) must be a valid callback, function "qtranxf_default_default_language" not found or invalid function name in /var/www/html/wp-content/plugins/qtranslate-xt/src/options.php:336
bin-wordpress-cli-1  | Stack trace:
bin-wordpress-cli-1  | #0 /var/www/html/wp-content/plugins/qtranslate-xt/src/options.php(410): qtranxf_load_option_func('default_languag...')
bin-wordpress-cli-1  | #1 /var/www/html/wp-content/plugins/qtranslate-xt/src/init.php(29): qtranxf_load_config()
bin-wordpress-cli-1  | #2 /var/www/html/wp-includes/class-wp-hook.php(324): qtranxf_init_language('')
bin-wordpress-cli-1  | #3 /var/www/html/wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters(NULL, Array)
bin-wordpress-cli-1  | #4 /var/www/html/wp-includes/plugin.php(517): WP_Hook->do_action(Array)
bin-wordpress-cli-1  | #5 /var/www/html/wp-settings.php(506): do_action('plugins_loaded')
bin-wordpress-cli-1  | #6 phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php(1363): require('/var/www/html/w...')
bin-wordpress-cli-1  | #7 phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php(1282): WP_CLI\Runner->load_wordpress()
bin-wordpress-cli-1  | #8 phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Bootstrap/LaunchRunner.php(28): WP_CLI\Runner->start()
bin-wordpress-cli-1  | #9 phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/bootstrap.php(83): WP_CLI\Bootstrap\LaunchRunner->process(Object(WP_CLI\Bootstrap\BootstrapState))
bin-wordpress-cli-1  | #10 phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/wp-cli.php(32): WP_CLI\bootstrap()
bin-wordpress-cli-1  | #11 phar:///usr/local/bin/wp/php/boot-phar.php(20): include('phar:///usr/loc...')
bin-wordpress-cli-1  | #12 /usr/local/bin/wp(4): include('phar:///usr/loc...')
bin-wordpress-cli-1  | #13 {main}
bin-wordpress-cli-1  |   thrown in /var/www/html/wp-content/plugins/qtranslate-xt/src/options.php on line 336
bin-wordpress-cli-1  | Fatal error: Uncaught TypeError: call_user_func(): Argument #1 ($callback) must be a valid callback, function "qtranxf_default_default_language" not found or invalid function name in /var/www/html/wp-content/plugins/qtranslate-xt/src/options.php:336
bin-wordpress-cli-1  | Stack trace:
bin-wordpress-cli-1  | #0 /var/www/html/wp-content/plugins/qtranslate-xt/src/options.php(410): qtranxf_load_option_func('default_languag...')
bin-wordpress-cli-1  | #1 /var/www/html/wp-content/plugins/qtranslate-xt/src/init.php(29): qtranxf_load_config()
bin-wordpress-cli-1  | #2 /var/www/html/wp-includes/class-wp-hook.php(324): qtranxf_init_language('')
bin-wordpress-cli-1  | #3 /var/www/html/wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters(NULL, Array)
bin-wordpress-cli-1  | #4 /var/www/html/wp-includes/plugin.php(517): WP_Hook->do_action(Array)
bin-wordpress-cli-1  | #5 /var/www/html/wp-settings.php(506): do_action('plugins_loaded')
bin-wordpress-cli-1  | #6 phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php(1363): require('/var/www/html/w...')
bin-wordpress-cli-1  | #7 phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php(1282): WP_CLI\Runner->load_wordpress()
bin-wordpress-cli-1  | #8 phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Bootstrap/LaunchRunner.php(28): WP_CLI\Runner->start()
bin-wordpress-cli-1  | #9 phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/bootstrap.php(83): WP_CLI\Bootstrap\LaunchRunner->process(Object(WP_CLI\Bootstrap\BootstrapState))
bin-wordpress-cli-1  | #10 phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/wp-cli.php(32): WP_CLI\bootstrap()
bin-wordpress-cli-1  | #11 phar:///usr/local/bin/wp/php/boot-phar.php(20): include('phar:///usr/loc...')
bin-wordpress-cli-1  | #12 /usr/local/bin/wp(4): include('phar:///usr/loc...')
bin-wordpress-cli-1  | #13 {main}
bin-wordpress-cli-1  |   thrown in /var/www/html/wp-content/plugins/qtranslate-xt/src/options.php on line 336
bin-wordpress-cli-1  | Error: There has been a critical error on this website.Learn more about troubleshooting WordPress. There has been a critical error on this website.  
bin-wordpress-cli-1 exited with code 1

@hirasso
Copy link

hirasso commented Jan 14, 2024

@IJMacD

This didn't work for me (404):

wp plugin install "https://github.com/qtranslate/qtranslate-xt/archive/refs/tags/master.zip" --activate;

This works:

wp plugin install "https://github.com/qtranslate/qtranslate-xt/archive/master.zip" --activate

@IJMacD
Copy link
Author

IJMacD commented Jan 14, 2024

This didn't work for me (404):

wp plugin install "https://github.com/qtranslate/qtranslate-xt/archive/refs/tags/master.zip" --activate;

Thanks for catching this. I was sure to test with the latest release as well as the master branch. I just ended up with a copy error in the gist. I've updated the gist now.

Here are equivalent URLs which all exhibit the failing behavior:

@hirasso
Copy link

hirasso commented Jan 14, 2024

So... I did a bit more testing.

I installed WordPress in three different ways because I was suspecting a custom wordpress folder structure to be causing the problem. After each installation, I ran the command wp plugin list to see if it would pass.

Test 1: Vanilla install

-> wp plugin list does work without errors for me

EDIT: It only works if qtranslate-xt was previously installed manually from the WordPress admin, as described in #1387 (comment)

Test 2: Install from Bedrock

Bedrock uses a custom directory structure like this:

.
├── composer.json
├── composer.lock
├── config
│  ├── application.php
│  └── environments
├── LICENSE.md
├── phpcs.xml
├── README.md
├── vendor
│  ├── autoload.php
│  ├── bin
│  ├── composer
│  ├── graham-campbell
│  ├── oscarotero
│  ├── phpoption
│  ├── roots
│  ├── squizlabs
│  ├── symfony
│  └── vlucas
├── web
│  ├── app # The wp-content equivalent
│  ├── index.php
│  ├── wp # The WordPress core files
│  └── wp-config.php
└── wp-cli.yml

-> With an install from Bedrock, I am getting the same error as reported by @IJMacD :

❯ wp plugin list
Fatal error: Uncaught TypeError: call_user_func(): Argument #1 ($callback) must be a valid callback, function "qtranxf_default_default_language" not found or invalid function name in /Users/me/Sites/bedrock/web/app/plugins/qtranslate-xt/src/options.php:336
Stack trace:
#0 /Users/me/Sites/bedrock/web/app/plugins/qtranslate-xt/src/options.php(410): qtranxf_load_option_func('default_languag...')
#1 /Users/me/Sites/bedrock/web/app/plugins/qtranslate-xt/src/init.php(29): qtranxf_load_config()
#2 /Users/me/Sites/bedrock/web/wp/wp-includes/class-wp-hook.php(324): qtranxf_init_language('')
#3 /Users/me/Sites/bedrock/web/wp/wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters(NULL, Array)
#4 /Users/me/Sites/bedrock/web/wp/wp-includes/plugin.php(517): WP_Hook->do_action(Array)
#5 /Users/me/Sites/bedrock/web/wp/wp-settings.php(506): do_action('plugins_loaded')
#6 phar:///Applications/MAMP/Library/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php(1363): require('/Users/me/Site...')
#7 phar:///Applications/MAMP/Library/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php(1282): WP_CLI\Runner->load_wordpress()
#8 phar:///Applications/MAMP/Library/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Bootstrap/LaunchRunner.php(28): WP_CLI\Runner->start()
#9 phar:///Applications/MAMP/Library/bin/wp/vendor/wp-cli/wp-cli/php/bootstrap.php(83): WP_CLI\Bootstrap\LaunchRunner->process(Object(WP_CLI\Bootstrap\BootstrapState))
#10 phar:///Applications/MAMP/Library/bin/wp/vendor/wp-cli/wp-cli/php/wp-cli.php(32): WP_CLI\bootstrap()
#11 phar:///Applications/MAMP/Library/bin/wp/php/boot-phar.php(20): include('phar:///Applica...')
#12 /Applications/MAMP/Library/bin/wp(4): include('phar:///Applica...')
#13 {main}
  thrown in /Users/me/Sites/bedrock/web/app/plugins/qtranslate-xt/src/options.php on line 336

Test 3: Only changing the folder structure from a vanilla install

I changed the folder structure from my vanilla install to this:

.
├── content
│  ├── index.php
│  ├── languages
│  ├── plugins
│  ├── themes
│  ├── upgrade
│  └── uploads
├── core
│  ├── index.php
│  ├── license.txt
│  ├── readme.html
│  ├── wp-activate.php
│  ├── wp-admin
│  ├── wp-blog-header.php
│  ├── wp-comments-post.php
│  ├── wp-config-sample.php
│  ├── wp-cron.php
│  ├── wp-includes
│  ├── wp-links-opml.php
│  ├── wp-load.php
│  ├── wp-login.php
│  ├── wp-mail.php
│  ├── wp-settings.php
│  ├── wp-signup.php
│  ├── wp-trackback.php
│  └── xmlrpc.php
├── index.php
└── wp-config.php

The wp-config.php needs to receive a few custom defines to make this work:

// ** Custom folder structure in wp-config.php:

define('WP_HOME', "http://qtranslate-xt.test");
define('WP_SITEURL', WP_HOME . '/core');
define('WP_CONTENT_URL', WP_HOME . '/content');
define('WP_CONTENT_DIR', dirname(__FILE__) . '/content');

-> Turns out this also results in an error, but it's a different one than with the bedrock install:

❯ wp plugin list
Warning: Undefined array key "wp-path" in /Users/rah/Sites/wpnaked/wwwroot/content/plugins/qtranslate-xt/src/frontend.php on line 13
Fatal error: Uncaught TypeError: qtranxf_parse_page_config(): Argument #2 ($url_path) must be of type string, null given, called in /Users/rah/Sites/wpnaked/wwwroot/content/plugins/qtranslate-xt/src/frontend.php on line 25 and defined in /Users/rah/Sites/wpnaked/wwwroot/content/plugins/qtranslate-xt/src/utils.php:317
Stack trace:
#0 /Users/rah/Sites/wpnaked/wwwroot/content/plugins/qtranslate-xt/src/frontend.php(25): qtranxf_parse_page_config(Array, NULL, '')
#1 /Users/rah/Sites/wpnaked/wwwroot/content/plugins/qtranslate-xt/src/frontend.php(898): qtranxf_get_front_page_config()
#2 /Users/rah/Sites/wpnaked/wwwroot/content/plugins/qtranslate-xt/src/init.php(126): qtranxf_add_front_filters()
#3 /Users/rah/Sites/wpnaked/wwwroot/core/wp-includes/class-wp-hook.php(324): qtranxf_init_language('')
#4 /Users/rah/Sites/wpnaked/wwwroot/core/wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters(NULL, Array)
#5 /Users/rah/Sites/wpnaked/wwwroot/core/wp-includes/plugin.php(517): WP_Hook->do_action(Array)
#6 /Users/rah/Sites/wpnaked/wwwroot/core/wp-settings.php(506): do_action('plugins_loaded')
#7 phar:///Applications/MAMP/Library/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php(1363): require('/Users/rah/Site...')
#8 phar:///Applications/MAMP/Library/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php(1282): WP_CLI\Runner->load_wordpress()
#9 phar:///Applications/MAMP/Library/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Bootstrap/LaunchRunner.php(28): WP_CLI\Runner->start()
#10 phar:///Applications/MAMP/Library/bin/wp/vendor/wp-cli/wp-cli/php/bootstrap.php(83): WP_CLI\Bootstrap\LaunchRunner->process(Object(WP_CLI\Bootstrap\BootstrapState))
#11 phar:///Applications/MAMP/Library/bin/wp/vendor/wp-cli/wp-cli/php/wp-cli.php(32): WP_CLI\bootstrap()
#12 phar:///Applications/MAMP/Library/bin/wp/php/boot-phar.php(20): include('phar:///Applica...')
#13 /Applications/MAMP/Library/bin/wp(4): include('phar:///Applica...')
#14 {main}
  thrown in /Users/rah/Sites/wpnaked/wwwroot/content/plugins/qtranslate-xt/src/utils.php on line 317

This one actually looks more like it's related to the other error I posted in #1344, although it's not the same one 😅

@herrvigg I think the simplest way for you to start tackling this issue is by installing bedrock and test qtranslate-xt with that setup.

@IJMacD
Copy link
Author

IJMacD commented Jan 14, 2024

For completeness the docker image seems to be a pretty standard folder layout.

.
|-- index.php
|-- license.txt
|-- readme.html
|-- wp-activate.php
|-- wp-admin
|    `-- ...
|-- wp-blog-header.php
|-- wp-comments-post.php
|-- wp-config-docker.php
|-- wp-config-sample.php
|-- wp-config.php
|-- wp-content
|    |-- cache
|    |-- index.php
|    |-- plugins
|    |-- themes
|    |-- upgrade
|    `-- uploads
|-- wp-cron.php
|-- wp-includes
|    `-- ...
|-- wp-links-opml.php
|-- wp-load.php
|-- wp-login.php
|-- wp-mail.php
|-- wp-settings.php
|-- wp-signup.php
|-- wp-trackback.php
`-- xmlrpc.php

@hirasso
Copy link

hirasso commented Jan 14, 2024

👍 I'm out of time for today, will give the docker examples a try tomorrow.

@hirasso
Copy link

hirasso commented Jan 15, 2024

@IJMacD I tested both your docker images. I had to add platform: linux/amd64 to the msyql entry to get it to work on my Apple M2, as described here:

version: "3.3"
services:
  db:
    image: mysql:5.7
    platform: linux/amd64 # < needed to add this to make it work on my mac
    # ...

I can reproduce the issue on my end. The .broken.yaml fails, the .pr.yaml passes. Now I'm pretty confused why it passed in my manual Test 1: Vanilla install above, but breaks in the docker environment. What could be the difference between the two? 🤔

@hirasso
Copy link

hirasso commented Jan 15, 2024

I've found a way to reliably reproduce the failing <-> working setup. It seems to be related to a first install of qtranslate-xt on a WordPress site where qtranslate-xt was never installed before. It's NOT related to the issue described in #1344 (that one is related to a custom folder structure).

How to reproduce

  1. Install WordPress completely from scratch, with a fresh database
  2. Install qtranslate using wp plugin install "https://github.com/qtranslate/qtranslate-xt/archive/master.zip" --activate
  3. Run any wp command, for example wp plugin list, or just visit http://example.test/wp-login.php
    --> fails with the described error
  4. Delete the qtranslate-xt folder from the plugins folder manually
  5. Login to /wp-admin
  6. Re-install qtranslate using wp plugin install "https://github.com/qtranslate/qtranslate-xt/archive/master.zip" --activate
  7. Reload /wp-admin
  8. Run any wp command like for example wp plugin list
    --> works!

So qtranslate-xt seems to be attempting to do some one-time task after first being installed. That fails if the user is not at an admin URL, as it is the case if installing qtranslate-xt from wp-cli.

Exactly what this PR solves 🤠

To be sure, I deleted my database, re-installed WordPress from scratch and then repeated the above steps 1-3 with the version from this PR:

wp plugin install "https://github.com/ijmacd/qtranslate-xt/archive/refs/heads/master.zip" --activate

This works like a charm! @herrvigg hope this helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core functionalities, including the admin section
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants