tmp_multisite_constants.php defines constant that may be redefined afterwards
#1025
Comments
|
Thanks for the great bug report! #766 explains why this is needed. More background in #754 and #554. But I'm still not sure the best solution here. |
|
Maybe it's something worth fixing in upstream wp cli, because the way I see it now is that the |
|
I tend to agree. I think WP-CLI is just running into a WP bug as well since it's an issue loading WordPress itself, but its their command so ideally it's dealt with there. |
|
@stefanotorresi how did you fix this? I am having the exact same issue, this fails at the point where $ wp core multisite-install --title="site title" --admin_user="username" --admin_password="password" --admin_email="[email protected]"then the site worked but the deploy script is still broken. How did #766 get merged when the process is still broken? #766 does solve one issue but creates another in my experience. |
|
@geekodour I don't think the presence of To get around the issue I described above, it is sufficient reverting to the usage of builtin |
|
It took me a while to remember the existance of |
|
As a new user of Trellis this problem is really annoying. For the moment I would be happy if at least I knew how to do the workaround to be able to deploy successfully, but I don't know how to revert to the usage of define() because I don't know how this was managed before. |
|
Finally I managed to deploy by commenting out two lines in trellis/roles/deploy/files/tmp_multisite_constants.php: |
|
@arusa thanks for posting your workaround. That's all it took it fix it? It was a one-step process? |
|
Yes, everything else is like documented as far as I remember. application.php contains the following constants now: and to be able to login to sites with different domains I also added the following: |
|
@swalkinshaw That doesn't completely avoid the issue with the config class throwing an error when it encounters a constant that's already defined. If a user defines |
|
I may be wrong here but I think this The multisite install procedure should be:
So long as this procedure is followed I'm under the impression the Edit: For more concise setup instructions follow #1025 (comment) |
|
Yep, confirmed that these constants don't need to be set at all for WP CLI when the correct multisite procedure is followed. I can work on a PR to remove the constants and update the Trellis docs with some more clear guidelines if that would help? Might be next week until I can do that |
|
Getting rid of this workaround would be amazing. Just to confirm what you're saying/proposing:
Is the latter the proper procedure even putting aside Trellis (or Bedrock)?
|
|
That's right, I went through step by step last night, all wp cli checks work absolutely fine when the above is followed. I think this just needs education / specific step by steps in the docs This procedure is the correct way for all multisites, you can get DB errors if you don't follow this. It's even common to run into issues when you have the allow multisites constant enabled before WP is installed. These tests were done on Trellis and Bedrock, and work exactly the same. In fact if constants were incorrectly defined an another stack you'd get the same table errors in WP CLI. This fix came about due to user error essentially. Ref: https://wordpress.org/support/article/create-a-network/ |
|
Question: What is the procedure to "taking over an existing multisite codebase"? Example:
It should throw errors during step 2 becuase Same goes to provsioning remote server the first time when Bedrock alredy coded in multisite (e.g: |
|
The procedure would be similar, it's erroneous to have a multisite constant and not a multisite database. The other way round however is fine. So to import an existing multisite DB you would first have a single install, you would import the multisite DB. At this point you have a multisite DB but the multisite tables are ignored by the dashboard. Until the constants are added. The mention of having Was your question "Why aren't I seeing errors with |
|
If the current process is just wrong, then yeah it should be changed. It sounds like right now this The counter point is that is has been "working" (as far as I know) and simplifies the process for people? |
|
Is there any reason for or against updating the Trellis documentation to reflect the steps @craigpearson has outlined? |
|
There's still a bit of uncertainty here. It would be better to solve this in Trellis itself and ideally remove |
|
@swalkinshaw That's true. Can you clarify the following a bit? On a single site:
On a multisite:
Apologies if I'm misunderstanding or asking dumb questions! |
|
I honestly don't know... I haven't tried multisite in forever. Though I think what you're asking is due to the problem described by this issue. If we removed the |
|
@ttamnedlog This is from memory but I believe if you have the multisite constants defined in bedrock. And you're provisioning / deploying to your live environment for the first time then the deploy will report a failure. This is due to I'm of the opinion that this shouldn't be bypassed as it leads people to break out of the normal multisite setup process, and causes confusion like this. So I personally remove the file from my projects. This means that you should do your first staging / production deploy in the following order:
/**
* (Required) Multisite Options
*/
Config::define('WP_ALLOW_MULTISITE', true);
// Config::define('MULTISITE', true);
// Config::define('SUBDOMAIN_INSTALL', false); // Set to true if using subdomains
// Config::define('DOMAIN_CURRENT_SITE', env('DOMAIN_CURRENT_SITE'));
// Config::define('PATH_CURRENT_SITE', env('PATH_CURRENT_SITE') ?: '/');
// Config::define('SITE_ID_CURRENT_SITE', env('SITE_ID_CURRENT_SITE') ?: 1);
// Config::define('BLOG_ID_CURRENT_SITE', env('BLOG_ID_CURRENT_SITE') ?: 1);
/**
* (Optional) Constants to help with assigning custom domains and prevent redirect login loops
*/
// Config::define('ADMIN_COOKIE_PATH', '/');
// Config::define('COOKIE_DOMAIN', '');
// Config::define('COOKIEPATH', '');
// Config::define('SITECOOKIEPATH', ''); Your multisite:
enabled: false
subdomains: false
/**
* (Required) Multisite Options
*/
Config::define('WP_ALLOW_MULTISITE', true);
Config::define('MULTISITE', true);
Config::define('SUBDOMAIN_INSTALL', false); // Set to true if using subdomains
Config::define('DOMAIN_CURRENT_SITE', env('DOMAIN_CURRENT_SITE'));
Config::define('PATH_CURRENT_SITE', env('PATH_CURRENT_SITE') ?: '/');
Config::define('SITE_ID_CURRENT_SITE', env('SITE_ID_CURRENT_SITE') ?: 1);
Config::define('BLOG_ID_CURRENT_SITE', env('BLOG_ID_CURRENT_SITE') ?: 1);
/**
* (Optional) Constants to help with assigning custom domains and prevent redirect login loops
*/
Config::define('ADMIN_COOKIE_PATH', '/');
Config::define('COOKIE_DOMAIN', '');
Config::define('COOKIEPATH', '');
Config::define('SITECOOKIEPATH', '');
multisite:
enabled: true
subdomains: false # Set to true if using sub domains
error_reporting(E_ALL & ~E_NOTICE);
//define('MULTISITE', false);
//define('SUBDOMAIN_INSTALL', false);
define('WPMU_PLUGIN_DIR', null);
define('WP_PLUGIN_DIR', null);
define('WP_USE_THEMES', false);I believe this to conflict with Bedrocks new
Just to clarify on development environments WordPress is automatically configured and the above errors don't take place, so this procedure can be ignored. But on production and staging environments WordPress must be configured via the http://example.com/blog/wp-admin/install.php URL, and this procedure should be followed |
|
@craigpearson When you say "no multisite constants" do you mean disabling all mention of multisite in both e.g. multisite:
enabled: false # true
subdomains: false # true/**
* Multisite
*/
// Config::define('WP_ALLOW_MULTISITE', true);
// Config::define('MULTISITE', true);
// Config::define('SUBDOMAIN_INSTALL', true); // Set to true if using subdomains
// Config::define('DOMAIN_CURRENT_SITE', env('DOMAIN_CURRENT_SITE'));
// Config::define('PATH_CURRENT_SITE', env('PATH_CURRENT_SITE') ?: '/');
// Config::define('SITE_ID_CURRENT_SITE', env('SITE_ID_CURRENT_SITE') ?: 1);
// Config::define('BLOG_ID_CURRENT_SITE', env('BLOG_ID_CURRENT_SITE') ?: 1);If so, I'm confused as to how I can "Run the multisite installation wizard"? I may be looking in the wrong places, but I'm not seeing anything... Don't some of these options need to be enabled for the multisite stuff to be visible? |
|
@shaneparsons Craig's response to me seems to be a bit more high level and omits some of the details provided in his earlier comment: #1025 (comment) I would follow Craig's steps as described there. In short, yes, disable all mention of multisite initially, and yes, you will then need to enable |
|
@ttamnedlog Thanks for that I've updated my previous comments to avoid any further confusion |
you forgot to mension that re-provision is also required |
|
Hello everyone, |
|
@craigpearson Thanks for the fix!
This step was required for me since it breaks the deploy with a |
|
Related discussion (WordPress |
|
Is there a workaround / fix for this for us on a old trellis version who wants to use wordpress 6.* ? |
|
It shouldn't be too hard to change up the syntax where necessary. Do you know which parts depend on newer Ansible versions? I don't off the top of my head. |
Bug report
What is the current behavior?
When using a multisite configuration, the deployment fails during the ansible task
WordPress Installed?.This is in turn caused by the
wp core is-installedcommand exiting with non zero status because of aConstantAlreadyDefinedException.This is related to roots/bedrock#380 and roots/docs#168.
What is the expected or desired behavior?
The deployment completes without error.
The
wp core is-installedcommand exits with 0.Please provide steps to reproduce, including full log output:
Note that the
MULTISITEandSUBDOMAIN_INSTALLredefinition is now prevented, by design, with the usage of the new configuration model.--debugflag yields the following output:This is caused by the explicit and one-time requirement of the file
roles/deploy/files/tmp_multisite_constants.php, which defines constant that may be re-defined later by the application config.Possible solutions:
Either of the following:
tmp_multisite_constants.php(the purpose of which is unclear to me at the moment) so that it avoids constant definitions. This is arguably the most desirable approach, but I'm not able to assess exactly what this would entail and how to proceed, because I don't even know why this file is being required in the first place during the affected Ansible task.Furthermore, I'm not sure using
Roots\WPConfig\Configinside this file is feasible, because the autoloader has not been loaded yet, and I'm not clear whenapplyshould then happen. Maybe this suggests a design oversight in the new configuration model.Also, IMO sneaking in a bunch of constants in such a non-transparent way is not a very sensible thing to do in general.
if definedblock to the multisite configuration docs. This would also require to explain accordingly the presence and purpose oftmp_multisite_constants.php, which as previously mentioned I am unable to do. IMO it also grants a kind of "wtf" moment to trellis users, which makes this solution undesirable.definefunction would make docs inconsistent.The text was updated successfully, but these errors were encountered: