Skip to content

WIP: Install WP directly into /public, remove the config midlayer #375

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

Closed
wants to merge 17 commits into from

Conversation

stian-overasen
Copy link
Member

@stian-overasen stian-overasen commented Apr 8, 2022

The overall goal of this PR is to rethink and reavaluate why we're placing wp in a problematic subdirectory and why we're adding on two layers of environment variables with both config/ and .env when we potentially can can get away with just having our custom wp-config.php with applied optional config from .env.

Main suggested changes:

  • Install WP clean directly into /public and not /wp. Avoid our over engineered setup for keeping WP in a subdir. Back to basics.
  • Avoid the problematic /wp rewrite in nginx and Local.
  • Move all config to a single wp-config.php inside /public that uses optional overrides from .env. Copy wp-config-template.php into /public after wp is installed.
  • Delete old config files in /config folder.
  • Have default config variables for quickly setting up a site in Local without need for changes to .env.
  • Add support for default return value in env().
  • Add support for boolean return in env().
  • No need for the extra complexity in Add local environment support. #374.

Edited: Configuration changes moved to #388.

@stian-overasen stian-overasen changed the title WIP: Install WP directly into /public WIP: Install WP directly into /public, remove the config midlayer Apr 8, 2022
@stian-overasen
Copy link
Member Author

What do you think about this approach @Clorith ?

@olethomas olethomas marked this pull request as ready for review April 11, 2022 11:10
@olethomas olethomas requested a review from dekode-andre April 11, 2022 12:05
* WP_HOME and WP_SITEURL

* Remove unnecessary comment.

* Fallback to WP_HOME for WP_SITEURL
@Clorith
Copy link
Member

Clorith commented Apr 20, 2022

Interesting idea, I'd like us to add some research into why bedrock chose to add the subdirectory before we swap it out all over, there may be a technical, or security related, reasoning behind it that we want to keep in mind.

I'm all in favor of making things as core-like as possible to ensure compatibility with plugins etc., without needing workarounds in the future as well though, especially as some of the constants utilized were not originally intended for this use.

I'd still argue there's a place for a zero-config environment (re: #374), but it could be simplified a lot if the ideas from this PR were to be implemented.

Copy link
Contributor

@dekode-andre dekode-andre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved my comments to be a reply on the thread above.

@pederan
Copy link
Contributor

pederan commented Apr 25, 2022

Good to question our structure, but gathering all wp files in its own folder makes it easy to git ignore the whole folder. Also think structuring installed packages in a public/wp (vendor folder) makes sense and is a good structure. By using default wp install paths, at least we need to git ignore all the files installed at root level.

@stian-overasen
Copy link
Member Author

Good to question our structure, but gathering all wp files in its own folder makes it easy to git ignore the whole folder. Also think structuring installed packages in a public/wp (vendor folder) makes sense and is a good structure. By using default wp install paths, at least we need to git ignore all the files installed at root level.

We can still ignore the whole public folder @pederan.

@pederan
Copy link
Contributor

pederan commented Apr 25, 2022

Good to question our structure, but gathering all wp files in its own folder makes it easy to git ignore the whole folder. Also think structuring installed packages in a public/wp (vendor folder) makes sense and is a good structure. By using default wp install paths, at least we need to git ignore all the files installed at root level.

We can still ignore the whole public folder @pederan.

Ok @stian-overasen I misunderstood. You want default wp structure inside /public instead of having two folders /public/content and /public/wp and symlink/install packages in default /public/wp-content/* folders. That makes sense. Inherited the bedrock structure, so as @Clorith said, should have a look first if there is any reasoning behind the split.

@stian-overasen
Copy link
Member Author

Did you find any security related issues @Clorith ?

@olethomas
Copy link
Member

olethomas commented May 19, 2022

We need to get some traction on this PR. Any volunteers? As far as I can see we are as secure as Bedrock keeping data in the .env file one level up from the public dir, but I might be missing some details.

@pederan
Copy link
Contributor

pederan commented May 23, 2022

I would like to get some traction, but would @Clorith must answer the security related first. But I have a question: Since we symlink local plugins, mu-plugins and themes into the wp (composer installed) package. Then when there is a new version of wp, won't that delete the symlinks created when installing the new version/overwrite the wp folder? Or does it keep the symlinks?

@stian-overasen
Copy link
Member Author

I would like to get some traction, but would @Clorith must answer the security related first. But I have a question: Since we symlink local plugins, mu-plugins and themes into the wp (composer installed) package. Then when there is a new version of wp, won't that delete the symlinks created when installing the new version/overwrite the wp folder? Or does it keep the symlinks?

I just tested this; Updating WP will flush the public-folder and delete all it's content. After running composer install all plugins and themes was put back. Is this a concern? There cannot be any git controlled files inside the public folder.

@olethomas
Copy link
Member

I would like to get some traction, but would @Clorith must answer the security related first. But I have a question: Since we symlink local plugins, mu-plugins and themes into the wp (composer installed) package. Then when there is a new version of wp, won't that delete the symlinks created when installing the new version/overwrite the wp folder? Or does it keep the symlinks?

I just tested this; Updating WP will flush the public-folder and delete all it's content. After running composer install all plugins and themes was put back. Is this a concern? There cannot be any git controlled files inside the public folder.

Was the uploads folder preserved?

@stian-overasen stian-overasen marked this pull request as draft August 15, 2022 06:39
@stian-overasen stian-overasen deleted the wp-directly-in-public branch November 8, 2022 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants