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

Support installer-paths nested into wordpress-install-dir #11

Closed
wants to merge 1 commit into from

Conversation

dzuelke
Copy link

@dzuelke dzuelke commented Jan 31, 2017

This allows an installer-paths entry for WordPress themes or plugins to be a subdirectory of wordpress-install-dir, which is nice, because it means no messing with WP_CONTENT_DIR and WP_CONTENT_URL is necessary:

"extra": {
	"wordpress-install-dir": "wordpress",
	"installer-paths": {
		"wordpress/wp-content/mu-plugins/{$name}/": ["type:wordpress-muplugin"],
		"wordpress/wp-content/plugins/{$name}/": ["type:wordpress-plugin"],
		"wordpress/wp-content/themes/{$name}/": ["type:wordpress-theme"]
	}
}

Without this change, running composer update to get a new version of johnpbloch/wordpress updates only that package, a process which wipes the whole wordpress/ directory, and, with that, the `wp-content/themes/' and 'wp-content/plugins/' directories that contain dependencies installed using Composer, so an additional 'composer install' is necessary to bring them back:

$ composer update
Loading composer repositories with package information
Updating dependencies (including require-dev)
Package operations: 0 installs, 1 update, 0 removals
  - Updating johnpbloch/wordpress (4.7.1 => 4.7.2) Loading from cache
Writing lock file
Generating autoload files
$ composer install
Loading composer repositories with package information
Installing dependencies (including require-dev) from lock file
Package operations: 4 installs, 0 updates, 0 removals
  - Installing dzuelke/twentyfifteen-child (1.0.11) Symlinked from themes/twentyfifteen-child
  - Installing wpackagist-plugin/amazon-s3-and-cloudfront (0.9.12) Loading from cache
  - Installing wpackagist-plugin/amazon-web-services (0.3.7) Loading from cache
  - Installing wpackagist-plugin/sendgrid-email-delivery-simplified (1.10.7) Loading from cache
Generating autoload files

Furthermore, it may happen that a plugin or theme is "alphabetically smaller" than "johnpbloch/wordpress", in which case a composer install from the lock file would result in such a package getting installed first, and then get overwritten because the wordpress/ folder gets replaced entirely:

$ composer install
Loading composer repositories with package information
Installing dependencies (including require-dev) from lock file
Package operations: 5 installs, 0 updates, 0 removals
  - Installing dzuelke/twentyfifteen-child (1.0.11) Symlinked from themes/twentyfifteen-child
  - Installing johnpbloch/wordpress (4.7.2) Loading from cache
  - Installing wpackagist-plugin/amazon-s3-and-cloudfront (0.9.12) Loading from cache
  - Installing wpackagist-plugin/amazon-web-services (0.3.7) Loading from cache
  - Installing wpackagist-plugin/sendgrid-email-delivery-simplified (1.10.7) Loading from cache
Generating autoload files
$ composer install
Loading composer repositories with package information
Installing dependencies (including require-dev) from lock file
Package operations: 1 install, 0 updates, 0 removals
  - Installing dzuelke/twentyfifteen-child (1.0.11) Symlinked from themes/twentyfifteen-child
Generating autoload files

The event handler fixes both cases, and supports all three composer/installers mapping types for installer-paths (literal package names, type: prefix, and vendor: prefix).

@dzuelke
Copy link
Author

dzuelke commented Jan 31, 2017

The second case could generally be avoided if johnpbloch/wordpress were to "provide": "wordpress/wordpress", and all themes and plugins, even on wpackagist.org, were to require wordpress/wordpress, but that's a completely different topic, and the first problem always remains, as Composer is simply not aware of such subdir trickery :)

This allows an 'installer-paths' entry for wordpress themes or plugins to be a subdirectory of 'wordpress-install-dir'.
Without this change, an update to the wordpress-core package updates only that package, which wipes 'wp-content/themes' and 'wp-content/plugins', and an additional 'composer install' is necessary to bring them back.
@Rarst
Copy link
Contributor

Rarst commented Jan 31, 2017

Composer packages are not meant to be ever nested inside one another. That's just trouble trouble trouble.

@dzuelke
Copy link
Author

dzuelke commented Jan 31, 2017

That's correct, but WordPress does use this structure by default, so... ¯\_(ツ)_/¯

@Rarst
Copy link
Contributor

Rarst commented Jan 31, 2017

I am of opinion that WordPress defaults of such kind shouldn't be dragged into a Composer stack. WordPress can work with extension directories separated, it's not a roadblock.

@johnpbloch
Copy link
Owner

Hi @dzuelke. First of all, thanks for taking the time to send code over. I appreciate the time and effort you've taken to improve this package.

This isn't a feature that I'm interested in introducing into the installer. @Rarst hit the nail on the head that some WordPress defaults simply don't make sense in a Composer-managed stack. For example, the default WordPress way of installing, updating, and deleting plugins and themes (and core itself) doesn't make sense in a composer stack. I don't think it's unreasonable to make a blanket decision to not support the WordPress defaults that don't make sense in a Composer stack.

I'm not going to close this quite yet. I'm interested in hearing your counter-argument, if any.

@dzuelke
Copy link
Author

dzuelke commented Jan 31, 2017

I believe do have counter-arguments ;)

  1. if you move wp-content by changing WP_CONTENT_DIR, then the johnpbloch/wordpress package no longer works out of the box, because the themes that are bundled are not there;
  2. you then have the additional challenge of having to set up more complicated web server configs or rewrites so wp-content/… is still web accessible - right now, you can just point the document root to wordpress/, and it all works immediately.

Of course WordPress gets managed differently once Composer is thrown into the mix. That's what WP-CLI and friends are for :)

My goal is to build as minimalist and standard a Twelve-Factor WordPress setup as possible (I am aware of Bedrock of course), in particular with the ability to both bundle themes and plugins from the same local project (using a Composer path repo), or from an external repo, without having at least two levels of nesting with web/app/… and web/wp/… (like Bedrock does).

Right now, that working minimal setup consists of only a .gitignore, a composer.json, a wp-cli.yml and a wp-config.php.

As a result, it's relatively easy to explain to somebody with a pure WordPress background - all you essentially have to convey is that Composer installs dependencies, WordPress lives in wordpress/, and the root folder wp-config.php gets picked up automatically. Which is nice, if you want to get people to learn about Composer, Twelve-Factor, how convenient Docker/PaaSes can be, etc :)

@Rarst
Copy link
Contributor

Rarst commented Jan 31, 2017

if you move wp-content by changing WP_CONTENT_DIR, then the johnpbloch/wordpress package no longer works out of the box, because the themes that are bundled are not there;

You can register core–bundled themes as additional theme directory and they will work even if content directory is elsewhere.

you then have the additional challenge of having to set up more complicated web server configs or rewrites

You don't?.. wordpress and wp-content folders in web root, no rewrite involved or needed.

@johnpbloch
Copy link
Owner

Regarding the default themes, you have two options: you could always ship an mu-plugin to register the default themes directory, or (probably even better) since you're shipping a composer manifest anyway, you could always just add the twenty* themes there and then they'll install into the correct default location anyway, especially since twenty sixteen doesn't even ship with the core package.

Regarding rewrites, they're not required unless you want to omit the wp subdirectory from URLs for admin/includes resources, but that's entirely optional. Just create an entrypoint index.php script to route traffic to WP. It does require the extra step of running the WP installation in the subdirectory and then changing the 'home' option to omit the subdirectory, but all of this could be scripted with WP CLI (maybe included as a dependency and using composer scripts?) and/or through wp-config constants.

@johnpbloch
Copy link
Owner

I'm Closing the pull request. This isn't a workflow that this package will support. Thanks again for your contribution and for the time you put into improving the project.

@johnpbloch johnpbloch closed this May 31, 2017
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.

None yet

3 participants