Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 3 years ago

#22802 closed enhancement (wontfix)

Empower Plugin Developers to make Symlink Compatible Plugins

Reported by: mikeschinkel's profile MikeSchinkel Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Plugins Keywords: has-patch dev-feedback 2nd-opinion
Focuses: Cc:

Description

Currently it's effectively impossible to use symlinked plugins reliably in a WordPress installation. For those not familiar with the problems with symlinks, let's assume we have a structure like the following where two sites are on the same server, and then there is a plugins directory that is on peer with the site directories:

/home/myacct/mysite1/  <-- Site #1 goes here
/home/myacct/mysite2/  <-- Site #2 goes here
/home/myacct/plugins/  <-- All the shared plugins go here

Now let's assume one of those plugins use plugins_url( 'js/myajax.js', __FILE__ ) to get a URL for this external file. But the problem is that PHP resolves symlinks before returning the value of __FILE__ but it does not provide any way to translate the value from FILE back into the symlinked virtual directory. So when you need a URL that looks like this:

http://mysite1.com/wp-content/plugins/myplugin/js/myajax.js

You instead get this URL because of the symlink:

http://mysite1.com/wp-content/plugins/home/myacct/plugins/myplugin/js/myajax.js

This problem has been discussed on tickets #16953 and #13550 and in both cases the discussion apparently stalled because there wasn't a good viable way to automatically support plugins that have been symlinked into the expected directories.

But what CAN be done is for the plugin developer to be proactive and handle write their plugin to be symlinkable. Alex King proposed a solution as did Jan Fabry which captures the file path value from the global variables $plugin, $mu_plugin or $network_plugin, whichever is applicable.

Unfortunately that works in many cases, but not all. Specifically those fail during plugin activation and plugin deactivation as you can see in the code I've extracted from a working class file. This class file shows how frought with peril it is to try to make this work in all applicable use-cases (For those interested here is the real file that uses this code from its GitHub repo.)

Forunately there is a very simple modification to WordPress core that will empower plugin developers to make their plugins symlinkable by simply replacing __FILE__ with $GLOBALS['wp_plugin_file']; easy peasy. Rather than WordPress core call include_once() every place where it needs to load a plugin WordPress core could call wp_load_plugin() instead. Here's how simple wp_load_plugin() can be:

function wp_load_plugin( $plugin_file ) {
  global $wp_plugin_file;
  include_once( $wp_plugin_file = $plugin_file );
  unset( $wp_plugin_file );
}

Simple, bulletproof, and here's what plugin code could look like with this change (compared to this.)

Note I decided to use a (disappearing) global variable because that would be the most performant method of capturing the plugin file value vs. assigning to a static object properties or similar, and because I think it will be the easiest syntax for people to use, i.e.:

global wp_plugin_file;
$url = plugins_url( 'js/myajax.js', $wp_plugin_file );

And being "disappearing" the code releases $wp_plugin_file from memory immediately after loading the plugin so chance of future conflict with anything else in WordPress is extremely unlikely.

I've attached a patch with includes this wp_load_plugin() function. The patch also calls wp_load_plugin() in all the places I've identified that load plugins. The version in the patch is actually a bit more complicated; it has logic to calculate and assign a $wp_plugin_slug too, because the plugins slug is needed at times and it would be really convenient if it could be made available during plugin load too (but of course adding $wp_plugin_slug is not nearly as important as adding $wp_plugin_file):

function wp_load_plugin( $plugin_file, $plugin_type = 'plugin' ) {
  global $wp_plugin_file, $wp_plugin_type, $wp_plugin_slug;
  $wp_plugin_type = $plugin_type;
  $dir = preg_quote( 'plugin' == $plugin_type ? WP_PLUGIN_DIR : WPMU_PLUGIN_DIR );
  $wp_plugin_slug = preg_replace( "#^{$dir}/(.+)$#", '$1', str_replace( '\\', '/', $plugin_file ) );
  include_once( $wp_plugin_file = $plugin_file );
  unset( $wp_plugin_file, $wp_plugin_type, $wp_plugin_slug );
}

In summary this addresses symlinking by enabling a new best practice that, when following, would empower plugin developers to build symlinkable plugins without going to extremely fragile lengths and that would work reliably in all cases.

BTW, if you've never had this problem try using some of your own plugins in a symlinked directory first before jumping to conclusions about what's required; it wasn't obvious to me how difficult this problem is to address until after I needed to.

Attachments (3)

wp-load-plugin.patch (3.7 KB) - added by MikeSchinkel 11 years ago.
Adding wp_load_plugin() function and appropriate calls.
plugin-file-hooks.diff (1.1 KB) - added by MikeSchinkel 11 years ago.
Adds 'uninstall_plugin_file' and 'activate_plugin_file' hooks.
make-plugins-symlinkable.diff (1.3 KB) - added by MikeSchinkel 11 years ago.
Make Plugins Symlinkable.

Download all attachments as: .zip

Change History (45)

#1 @MikeSchinkel
11 years ago

  • Cc mike@… added

TL;DR: Ticket basically says if WordPress will load plugins via a function that sets a global var containing the plugin filepath instead of directly using include_once() for each plugin then we can write plugins that can be reliably symlinked.

#2 @mordauk
11 years ago

  • Cc pippin@… added

@MikeSchinkel
11 years ago

Adding wp_load_plugin() function and appropriate calls.

#3 @Bueltge
11 years ago

  • Cc frank@… added

#4 @leewillis77
11 years ago

  • Cc junk@… added

#5 follow-up: @scribu
11 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

This is an interesting proposal, but it should have been posted as a comment on #16953.

#6 in reply to: ↑ 5 ; follow-up: @MikeSchinkel
11 years ago

Replying to scribu:

This is an interesting proposal, but it should have been posted as a comment on #16953.

You should not have closed this since #16953 focuses on the wrong thing.

And that's exactly what you said about westi's ticket #13550 that you closed 21 months ago when you opened #16953. What I should have done was first close your ticket as you closed westi's, but I'll let you close it instead.

Last edited 11 years ago by MikeSchinkel (previous) (diff)

#7 follow-up: @toscho
11 years ago

  • Cc info@… added
  • Resolution duplicate deleted
  • Status changed from closed to reopened

I like this idea. Not sure about the global variables – these are hard to debug. And I think wp_include_plugin() would be easier to understand than load.

#8 in reply to: ↑ 6 ; follow-up: @scribu
11 years ago

Replying to MikeSchinkel:

Replying to scribu:
You should not have closed this since #16953 focuses on the wrong thing.

And that's exactly what you said about westi's ticket #13550 that you closed 21 months ago when you opened #16953.

Looking back at it, you're right; I probably shouldn't have closed #13550, as #16953 covered almost exactly the same ground.

#9 @scribu
11 years ago

  • Milestone set to Awaiting Review
  • Type changed from defect (bug) to enhancement

#10 in reply to: ↑ 7 @MikeSchinkel
11 years ago

Replying to toscho:

I like this idea. Not sure about the global variables – these are hard to debug. And I think wp_include_plugin() would be easier to understand than load.

The patch was more to propose the concept than any specific code, and to identify where plugins are loaded. Anything that addresses the core concept will be helpful; to capture the known filename at the time of include so it can be made available to the plugin.

Global vars are easy but there are other ways to handle it, all depends on what the core team/community prefers today; I know preferred practices have evolved over the years.

BTW, there is an existing load_theme() function which is why I named it wp_load_plugin(), but I really have no opinion what it's called because it'd (mostly?) only be used internally in core.

Last edited 11 years ago by MikeSchinkel (previous) (diff)

#11 in reply to: ↑ 8 @MikeSchinkel
11 years ago

Replying to scribu:

Looking back at it, you're right; I probably shouldn't have closed #13550, as #16953 covered almost exactly the same ground.

Thanks for the ack. I sincerely appreciate it.

#12 follow-up: @scribu
11 years ago

there is an existing load_theme() function

I can't find it.

In summary this addresses symlinking by enabling a new best practice

Could you give an example of that best practice, from the POW of a plugin author?

#13 in reply to: ↑ 12 @MikeSchinkel
11 years ago

Replying to scribu:

there is an existing load_theme() function

I can't find it.

<redfaced> I can't find it now either. Maybe it was in a plugin I had in my test site. Or maybe I was confused when I saw load_theme_textdomain(). Fortunately though, the name of the function is not important to the concept.

In summary this addresses symlinking by enabling a new best practice

Could you give an example of that best practice, from the POW of a plugin author?

Sure.

I implied it above in the example but explicitly stated it would be for the plugin author to capture the value of global $wp_plugin_file into a constant at the top of the plugin's entry point. So instead of this which is common in plugins:

define( 'MY_PLUGIN_FILE', __FILE__ );

This new "best practice" would be to do this instead:

global $wp_plugin_file; 
define( 'MY_PLUGIN_FILE', $wp_plugin_file );

Then anywhere in the plugin where __FILE__ might be otherwise used the plugin developer would use MY_PLUGIN_FILE instead.

Of course it doesn't have to be captured and made available as a global, it could be stored as a class property or something else, and the value doesn't have to be captured as a constant, it could be captured into a class property or another variable as well. The key point is simply to somehow make sure that there is always a simple method to capture the plugin's file name that WordPress uses instead of the real path that has been symlinked.

#14 @kchrist
11 years ago

  • Cc kenn@… added

#15 @rmccue
11 years ago

+1. I've always thought it silly that WordPress knows exactly where the file lives under its system, yet we still use __FILE__ in plugins. I didn't really have a great solution, but this is probably a good use of global variables (if there is such a thing).

#16 @boonebgorges
11 years ago

  • Cc boonebgorges@… added

#17 @naomicbush
11 years ago

  • Cc naomicbush added

#18 follow-up: @nacin
11 years ago

Not saying this isn't a problem, but the solution as proposed won't work for core. For compatibility reasons, plugins need to be included in global scope, not from within a function. That's why we have a function to return to wp-settings.php an array of PHP files to then include (in global scope), versus a function to just include the files.

The use of a global is weird, because it means the plugin has to immediately act on its value. That doesn't make much sense, especially since an ideal plugin should have only add_action/add_filter calls in global scope, and potentially even just one (waiting until plugins_loaded or init to do everything).

We probably have to do this the other way around: wp_get_active_and_valid_plugins() should store its results somewhere, so we can later look to it to see which plugins are registered and where they are. That won't help for deactivated plugins (i.e. the uninstall process, and on activation), but we can separately register those paths at the beginning of each of those processes. Then, we can use a plugin slug (say, my-plugin in lieu of __FILE__) in plugins_url() and check the registered paths before continuing.


Now, cast all of that aside for a moment. Why do we actually need to do this? Rather than symlinking, just check the code out in multiple locations, or define a single WP_PLUGIN_DIR and WP_PLUGIN_URL, or just filter plugins_url(). Having a symlinked setup is environment-specific, so why should we go through so much trouble to try to fix this? You could easily filter plugins_url() and have it work for all plugins, even ones not adjusted for the environment...

#19 in reply to: ↑ 18 @MikeSchinkel
11 years ago

Replying to nacin:

Not saying this isn't a problem, but the solution as proposed won't work for core....

ok.

The use of a global is weird...

Well, the the fact that PHP won't let us discover the symlinked value is weird wouldn't you say it might require a "weird" approach to fix?

We probably have to do this the other way around: wp_get_active_and_valid_plugins() should store its results somewhere...

It'd be (partially) happy with a hook in wp_get_active_and_valid_plugins() to allow me to build an array of filenames keyed by the file's realname(). Having that function attach the same information to $current_screen would be even better.

That won't help for deactivated plugins (i.e. the uninstall process, and on activation), but we can separately register those paths at the beginning of each of those processes.

Yes, I need to know plugin filenames during activation and deactivation too. Maybe they could also be attached to $current_screen?

Now, cast all of that aside for a moment. Why do we actually need to do this? Rather than symlinking, just check the code out in multiple locations, or define a single WP_PLUGIN_DIR and WP_PLUGIN_URL, or just filter plugins_url(). Having a symlinked setup is environment-specific, so why should we go through so much trouble to try to fix this? You could easily filter plugins_url() and have it work for all plugins, even ones not adjusted for the environment...

In theory yes. But in practice it's a nightmare to manage. That's what I was doing and it was so painful I finally had to move to symlinked plugins. I have written code to allow it but the code is outrageously ugly, I continue to find new use-cases where it breaks and so I'd really scared of discovering new ways it breaks. And all I need would be a simple way to get the plugin's expected filename vs. the realname().

The problems are the duplicate symbols in the IDE cause all sorts of headache, and I accidentally end up updating the wrong code before the IDE takes me to the wrong version of the code. Doing what you propose has become such a nightmare I've had to move away from it, and yet all it would take would be a tiny bit of new code in core.

Like I said on Twitter if this is only one (1) thing I ask for 3.6, please let it be a solution to allow developers to write symlinkable plugins. Please...

#20 follow-up: @nacin
11 years ago

I see four approaches:

  1. Something fairly convoluted in core to allow a plugin to work in a symlinked environment.
  1. Something fairly straightforward in an environment to allow all plugins to work.
  1. Avoid symlinks, and use VCS tools to check out the same code into multiple places (git submodules, svn externals, checkouts, etc)
  1. Avoid symlinks, and use a shared plugin directory via WP_PLUGIN_DIR and WP_PLUGIN_URL (it's why these constants are there)

The first approach is particularly painful, and still won't help you for every other plugin that uses __FILE__, and because it will require plugin authors to do a decent amount of work to leverage it, it probably won't become a best practice.

The other three are far easier, far more reliable, and require no core changes. Option 2 even sounds like a great plugin.

Yes, PHP is clearly lacking a tool here. But I don't think we should return the volley with a weird fix that plugin developers will probably struggle with. If there's a proposal that is sufficiently straightforward (both in core's implementation, and in how plugins use it), this could be considered. Until then, there seem to be a number of ways to do this inside one's environment, and those ways seem to be more efficient than a core solution anyway.

#21 @goldenapples
11 years ago

  • Cc goldenapplesdesign@… added

#22 @MikeSchinkel
11 years ago

Replying to nacin:

I see four approaches:

I agree that #1 is not worth doing. I have no idea what #2 is. And I've already explained that #3 is not practical and #4 doesn't address the need either, as I'll explain below.

But you left out a 5th option, which is the option you already described in your prior comment. Record the virtual filename and associate it with the realname() such as via a keyed array or as a variable that is accessible when the plugin is loaded, activated, deactivated and uninstalled. That's all, and you already suggested it. That's all it would take to empower developers. There are numerous ways to get there too, we only need one.

Maybe what I need to explain is the problem really arises when using shared code across many plugins in the same site so #4 is not a solution. If we keep multiple copies of the same shared code it becomes too hard to keep track of which one(s) are being modified resulting in lost changes and a lot of wasted time. So we need to symlink the libraries of shared code to be able to keep only one copy but that's really difficult if can't get access to the virtual filename that WordPress expects vs. the realname() that PHP uses for __FILE__ and __DIR__.

All we need is to know the plugin's virtual filename when it loads. Somehow. Is that really too much to ask?

#23 @naomicbush
11 years ago

As a plugin developer, this really is a real problem. Every time there's a new version of WordPress, I spin up up to 5 separate installs for testing just that version (and I'm sure others do more but I've chosen to not support ridiculously old versions of PHP and WordPress). And then, any time I make a change to the plugin, I have to test it in each of those 5 installs, not to mention the 5 installs for each earlier version (depending on how far I want to go back). Maybe I'm doing_it_wrong, but for me, symlinking is the only reasonable way to continually test a plugin in multiple WordPress installs. Checking out and updating the code in each of those locations every time I make a change and want to test it is a bit much, to put it mildly. I'm currently squeaking by with Alex's code but a real solution would be so much better. Please?

#24 in reply to: ↑ 20 ; follow-up: @kchrist
11 years ago

Replying to nacin:

  1. Avoid symlinks, and use VCS tools to check out the same code into multiple places (git submodules, svn externals, checkouts, etc)

I just want to note here that while this could work in some situations where people are symlinking either individual plugins or the entire plugins directory, it doesn't address the issue where plugins break when deployed via Capistrano (as noted in comment:ticket:16953:31). Any situation where the entire document root is symlinked is affected by this same problem.

#25 in reply to: ↑ 24 @MikeSchinkel
11 years ago

Replying to kchrist:

I just want to note here that while this could work in some situations where people are symlinking either individual plugins or the entire plugins directory, it doesn't address the issue where plugins break when deployed via Capistrano (as noted in comment:ticket:16953:31). Any situation where the entire document root is symlinked is affected by this same problem.

Interesting. While I understand my use-case causes issues I don't fully understand yours. Do any of the solutions proposed address your use-case issues? If not, is there an idea you have for a solution that would address it?

Last edited 11 years ago by MikeSchinkel (previous) (diff)

#26 follow-up: @markjaquith
11 years ago

The fact that __FILE__ is magic and we can't fix it, is a rather impressive hurdle to this proposed functionality having any practical effect. That is, it's not going to solve the problem MikeSchinkel is having because most plugins will just continue using __FILE__ which has long been best practice. We have trouble getting them to stop hardcoding /wp-content/plugins/my-plugin-name/ — and that has much more measurable negative effects.

I'm much more interested in seeing if we can make it possible (or easier) for this situation to be supported without plugin authors having to change their code, which is what #16953 is focusing on.

#27 in reply to: ↑ 26 @MikeSchinkel
11 years ago

Replying to markjaquith:

That is, it's not going to solve the problem MikeSchinkel is having because most plugins will just continue using __FILE__ which has long been best practice.

The problem I'm having is with my own plugins that are under heavy development. I myself don't have an issue with having multiple copies of the same plugins from other authors that I don't happen to be modifying, but I need to be able to write my own so that I can symlink them.

I'm much more interested in seeing if we can make it possible (or easier) for this situation to be supported without plugin authors having to change their code, which is what #16953 is focusing on.

Understood, but please don't let perfect be the enemy of good.

#28 follow-up: @markjaquith
11 years ago

Replying to kchrist:

Replying to nacin:

  1. Avoid symlinks, and use VCS tools to check out the same code into multiple places (git submodules, svn externals, checkouts, etc)

I just want to note here that while this could work in some situations where people are symlinking either individual plugins or the entire plugins directory, it doesn't address the issue where plugins break when deployed via Capistrano (as noted in comment:ticket:16953:31). Any situation where the entire document root is symlinked is affected by this same problem.

All of my sites are deployed with Capistrano, and I have no problems with this. The only way you'd have an issue is if some stupid plugin is storing a full system path somewhere in the database (or in a config file it writes to disk). The issue MikeSchinkel is talking about comes into play because the plugin path doesn't contain ABSPATH. But with a Capistrano deployment, it is the docroot that is symlinked. So while ABSPATH changes on every deploy, that doesn't matter, because nothing in WordPress hardcodes full system paths. Happy to chat with you about this privately, as Capistrano and WordPress deployment systems are one of my specialties (see WP Stack ). Same username on Skype.

#29 @kchrist
11 years ago

It's the same problem, really; in the Capistrano case the symlink is a couple levels above the plugins directory but the effect is the same.

As in the other use cases described, avoiding use of __FILE__ will fix the issue with Capistrano deployments. I'm currently filtering plugins_url and that does the trick, but it took a while to come up with that solution and it still feels like a bit of a kludge.

#30 in reply to: ↑ 28 ; follow-up: @kchrist
11 years ago

Replying to markjaquith:

It's a rare plugin that causes this problem for me but off the top of my head I can name Editorial Calendar, YARPP, and the official Facebook plugin as a few that break. The breakage in this cause is always JS/CSS paths; in some cases it only affects admin pages, other cases it affects the plugin working at all.

#31 in reply to: ↑ 30 @MikeSchinkel
11 years ago

Replying to kchrist:

The breakage in this cause is always JS/CSS paths; in some cases it only affects admin pages, other cases it affects the plugin working at all.

So your issue is when others don't follow best practices, I get that. Yes, that's another issue but to fix it's both easier (if you are writing the plugin) and harder (if you are using someone else's plugin).

#32 follow-up: @markjaquith
11 years ago

Replying to MikeSchinkel:

The problem I'm having is with my own plugins that are under heavy development. I myself don't have an issue with having multiple copies of the same plugins from other authors that I don't happen to be modifying, but I need to be able to write my own so that I can symlink them.

If it's private code on systems you control, that you have any number of solutions available to you. Heck, you could just hardcode the path based on ABSPATH.

Understood, but please don't let perfect be the enemy of good.

I honestly don't think what you've proposed is good. It only helps people who are in your specific situation. And as a subset of plugin symlinking desires, I don't think it's a very common one, and it is one that — due to your control over the plugins and environments in question — you have alternative ways of solving.

#33 in reply to: ↑ 32 ; follow-up: @MikeSchinkel
11 years ago

Replying to markjaquith:

If it's private code on systems you control, that you have any number of solutions available to you. Heck, you could just hardcode the path based on ABSPATH.

I'm writing plugins for clients to distribute on wordpress.org. So no, I don't have other solutions.

I honestly don't think what you've proposed is good. It only helps people who are in your specific situation. And as a subset of plugin symlinking desires, I don't think it's a very common one...

Is it really that hard to make the virtual plugin filename available to the plugin on activation and uninstall? Hell, I'll settle for that. New patch coming...

#34 in reply to: ↑ 33 ; follow-up: @nacin
11 years ago

Replying to MikeSchinkel:

Replying to markjaquith:

If it's private code on systems you control, that you have any number of solutions available to you. Heck, you could just hardcode the path based on ABSPATH.

I'm writing plugins for clients to distribute on wordpress.org. So no, I don't have other solutions.

Then who is doing the symlinking?

#35 in reply to: ↑ 34 @MikeSchinkel
11 years ago

Replying to nacin:

Then who is doing the symlinking?

I'm doing the symlinking of shared libraries packaged on my local system for plugins that I need to distribute via wordpress.org.

This is become a huge problem for me. All I'm asking for is to make the virtual filename available. Please at least consider my next patch.

@MikeSchinkel
11 years ago

Adds 'uninstall_plugin_file' and 'activate_plugin_file' hooks.

#36 @MikeSchinkel
11 years ago

  • Version changed from 3.4.2 to trunk

New proposed approach: Add two (2) hooks: 'uninstall_plugin_file' and 'activate_plugin_file'. It's too tiny additions to core that have no performance impact and neither could cause any compatibility issues. See attached patch: plugin-file-hooks.diff​.

@MikeSchinkel
11 years ago

Make Plugins Symlinkable.

#37 @MikeSchinkel
11 years ago

Replying to MikeSchinkel:

New proposed approach: Add two (2) hooks: 'uninstall_plugin_file' and 'activate_plugin_file'. It's too tiny additions to core that have no performance impact and neither could cause any compatibility issues. See attached patch: plugin-file-hooks.diff​.

Actually I'm an idiot. Hooks won't work. Ugh. See make-plugins-symlinkable.diff​ instead.

#38 @emzo
11 years ago

  • Cc wordpress@… added

#39 @mindctrl
11 years ago

  • Cc mindctrl added

#40 @SergeyBiryukov
11 years ago

  • Version trunk deleted

#41 @markjaquith
11 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from reopened to closed

Closing. This approach just isn't realistic and doesn't solve the problem in a way that is generally useful. We should continue working on #16953

This ticket was mentioned in Slack in #core by peterwilsoncc. View the logs.


3 years ago

Note: See TracTickets for help on using tickets.