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

Bug: Autodeploy failures with WordPress 6.0 #1385

Closed
5 tasks done
SteelWagstaff opened this issue May 25, 2022 · 13 comments · Fixed by #1388
Closed
5 tasks done

Bug: Autodeploy failures with WordPress 6.0 #1385

SteelWagstaff opened this issue May 25, 2022 · 13 comments · Fixed by #1388
Labels

Comments

@SteelWagstaff
Copy link

Terms

Description

What's wrong?

Our Trellis-based autodeploy pipeline is failing to create a WordPress multisite installation with the latest WordPress 6.0 release. In particular, this command is failing:

- name: WordPress Installed?
command: wp core is-installed --skip-plugins --skip-themes --require={{ deploy_helper.shared_path }}/tmp_multisite_constants.php

We think it's because the WP CLI command does like WPMU_PLUGIN_DIR being defined as null:

define('WPMU_PLUGIN_DIR', null);

We don't know what might be different about WordPress 6.0 that might be producing this problem/issue.

What have you tried?

We can complete the deploy by removing the --require parameter in

- name: WordPress Installed?
command: wp core is-installed --skip-plugins --skip-themes --require={{ deploy_helper.shared_path }}/tmp_multisite_constants.php

We can also complete the deploy by commenting out define('WPMU_PLUGIN_DIR', null); here

define('WPMU_PLUGIN_DIR', null);

However, neither of these seem like desirable solutions, considering the rationale provided by the author of this PR: #801. Not sure if others are experiencing similar problems or have more insight into how to resolve them.

What insights have you gained?

This WordPress multisite stuff is complicated 😂

Possible solutions

See the What have you tried? section

Temporary workarounds

See the What have you tried? section

Steps To Reproduce

  1. Attempt to deploy WordPress multisite using the latest Trellis repo and roots/bedrock
  2. Observe the output of the 'TASK [deploy: WordPress Installed?]` step
  3. It will include a fatal error in which the stderr output has several strpos(): Empty needle warnings that reference this line in wp-includes/link-template.php: https://github.com/WordPress/WordPress/blob/290742cada0b3761d2aac83db2a337756fee64eb/wp-includes/link-template.php#L3535

Expected Behavior

The deploy was able to install WordPress 6.0 in the same way that it was previously able to install WordPress 5.9.3, etc.

Actual Behavior

The 'TASK [deploy: WordPress Installed?]step fails with astderroutput which includes severalstrpos(): Empty needle` warnings that reference this line in wp-includes/link-template.php: https://github.com/WordPress/WordPress/blob/290742cada0b3761d2aac83db2a337756fee64eb/wp-includes/link-template.php#L3535

Relevant Log Output

TASK [deploy : Create file with multisite constants defined as false] **********
[0;33mchanged: [web_staging -> bibliotest.pressbooks.pub] 

TASK [deploy : WordPress Installed?] *******************************************
[0;37mSystem info:
[0;37m  Ansible 2.8.4; Linux
[0;37m  Trellis Head
[0;37m---------------------------------------------------
[0;31mWarning: strpos(): Empty needle in
[0;31m/srv/www/bibliotest.pressbooks.pub/releases/20220525212141/web/wp/wp-includes
[0;31m/link-template.php on line 3535
[0;31mWarning: strpos(): Empty needle in
[0;31m/srv/www/bibliotest.pressbooks.pub/releases/20220525212141/web/wp/wp-includes
[0;31m/link-template.php on line 3535
[0;31mfatal: [web_staging -> bibliotest.pressbooks.pub]: FAILED! => {"changed": false, "cmd": ["wp", "core", "is-installed", "--skip-plugins", "--skip-themes", "--require=/srv/www/bibliotest.pressbooks.pub/shared/tmp_multisite_constants.php"], "delta": "0:00:00.495290", "end": "2022-05-25 21:21:59.167648", "failed_when_result": true, "rc": 0, "start": "2022-05-25 21:21:58.672358", "stderr": "Warning: strpos(): Empty needle in /srv/www/bibliotest.pressbooks.pub/releases/20220525212141/web/wp/wp-includes/link-template.php on line 3535\nWarning: strpos(): Empty needle in /srv/www/bibliotest.pressbooks.pub/releases/20220525212141/web/wp/wp-includes/link-template.php on line 3535", "stderr_lines": ["Warning: strpos(): Empty needle in /srv/www/bibliotest.pressbooks.pub/releases/20220525212141/web/wp/wp-includes/link-template.php on line 3535", "Warning: strpos(): Empty needle in /srv/www/bibliotest.pressbooks.pub/releases/20220525212141/web/wp/wp-includes/link-template.php on line 3535"], "stdout": "", "stdout_lines": []}

PLAY RECAP *********************************************************************
[0;32mlocalhost�               : ok=0    changed=0    unreachable=0    failed=0    [0;36mskipped=1   rescued=0    ignored=0   
[0;33mweb_staging               : 0;32mok=6    [0;33mchanged=1    unreachable=0    failed=0    �[0;36mskipped=8    rescued=0    ignored=0   
[0;31mweb_staging -> bibliotest.pressbooks.pub : [0;32mok=20   ;33mchanged=9    unreachable=0    �[0;31mfailed=1  [0;36mskipped=21   rescued=0    ignored=0   

Build step 'Execute shell' marked build as failure
Finished: FAILURE

Versions

Not sure, sorry

@codepuncher
Copy link

Just to add more context: this fails for non-Multisite projects, too.

@mike-sheppard
Copy link

mike-sheppard commented May 26, 2022

Yup tried all 3 variations on a non-multisite, failing on WP installed check via standard trellis-cli deploy

  • roots/wordpress-full @ 6
  • roots/wordpress-no-content @ 6
  • roots/wordpress @ 6

Locked to 5.9.x for now which still works perfectly

EDIT
I take that back haha, sorry - they do work with trellis-cli deploy, ignore me I must have been doing something silly 🙈

@strarsis
Copy link
Contributor

strarsis commented May 27, 2022

Discussion about this issue: https://discourse.roots.io/t/wordpress-6-0-update-deploy-failed/23225/7

It boils down basically to this:

  1. WPMU_PLUGIN_DIR is set to null, which is something WordPress core isn't really designed to handle (PHP strpos empty needle warning).
  2. WPMU_PLUGIN_DIR is set to null to achieve something in regards to multisite, the filename of the required tmp_multisite_constants.php somehow indicates that.
  3. Without WPMU_PLUGIN_DIR set to null, that WPMU_PLUGIN_DIR constant is considered not defined by PHP, WordPress core assigns some default path to it in that case.
  4. Why must WPMU_PLUGIN_DIR be set to null? Does the default path assigned by WordPress core for a non-defined WPMU_PLUGIN_DIR interfere with multisite-related checks or something?

If WPMU_PLUGIN_DIR doesn't really have to be set to null, this line could be simply omitted from the Trellis template that generates the PHP file tmp_multisite_constants.php containing it, and the whole issue goes away.
If it needs it, then a (trac) ticket should be created for WordPress core, as WordPress should be able to handle the null value case of WPMU_PLUGIN_DIR without emitting PHP warnings. Until this is addressed, Trellis should be less strict with the wp core is-installed check, either allowing a non-empty stderr or at least tolerating the strpos non-empty needle PHP warnings caused by a WPMU_PLUGIN_DIR constant set to null.

@JJJ
Copy link

JJJ commented May 27, 2022

Tangentially related: as of WordPress 2.8, the WPMU_PLUGIN_DIR and WPMU_PLUGIN_URL (and the less used MUPLUGINDIR) constants are not multisite specific, and "must use" plugins in the mu-plugins directory will be loaded and be fully operational on single-site installations, making its placement inside of tmp_multisite_constants.php no longer precisely accurate.

@strarsis
Copy link
Contributor

@JJJ: So it would make sense to remove these lines for setting the WPMU_PLUGIN_DIR and WPMU_PLUGIN_URL constants to null.

The only problem that may arise in terms of backwards compatibility is when someone deploys a quite old, un-updated WordPress site with a current Trellis. Maybe check for the WordPress version to be deployed and cancel the deployment with a warning, or use a different tmp_multisite_constants.php with the WPMU_PLUGIN_DIR and WPMU_PLUGIN_URL constants set to null?

@JJJ
Copy link

JJJ commented May 28, 2022

@JJJ: So it would make sense to remove these lines for setting the WPMU_PLUGIN_DIR and WPMU_PLUGIN_URL constants to null.

I think: yes.

See also: https://github.com/WordPress/WordPress/blob/9a2a423eb6858cbd96017c2ce877e35604073ca5/wp-includes/default-constants.php#L192-L218

(Since WordPress 2.8, wp_plugin_directory_constants() gets called inside of wp-settings.php automatically setting them to wp-content/mu-plugins.)

It seems unlikely that WordPress will ever know what to do if WPMU_PLUGIN_DIR is null. Because it's a "default constant" that it expects to always have a value, rarely does it ever validate the contents of the constant before using it.

(get_mu_plugins() does if ( ! is_dir( WPMU_PLUGIN_DIR ) ) { which will produce a similar PHP warning as strpos(), etc...)

Personally, individually, I like the idea of a null value just silently turning the feature of mu-plugins completely off, much in the way that WP_USE_THEMES works inside of index.php. It's just code – it's achievable – but we'd need a decent use-case and loads of core unit tests. 🤣

@strarsis
Copy link
Contributor

strarsis commented Jun 1, 2022

@SteelWagstaff, @codepuncher, @mike-sheppard, @JJJ: Fixed with #1388.

@codepuncher
Copy link

Thanks for the work @strarsis!

@codepuncher
Copy link

@SteelWagstaff have you managed to test #1388 that was merged into master? I'm eager to hear your feedback for the multisite issue as I too am having issues there.

@strarsis
Copy link
Contributor

strarsis commented Jun 12, 2022

Summoning @codepuncher; @SteelWagstaff; @JJJ; @cfaria for the new PR that eliminates the need for those temporary PHP constants and allows for a normal deployment of multisite sites: #1391

@codepuncher
Copy link

Legend @strarsis !

All deployments (single and multi) work as expected.

Thanks for the work on that.

@JJJ
Copy link

JJJ commented Jun 13, 2022

@JJJ; for the new PR that eliminates the need for those temporary PHP constants and allows for a normal deployment of multisite sites: #1391

Works for me 💗

@swalkinshaw
Copy link
Member

Going to close this issue. I'll do a new release soon that will have this fix.

Thanks to everyone for the help and especially @strarsis for all this hard work.

reid added a commit to AwakeningSV/wordpress that referenced this issue Aug 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
6 participants