#16953 closed enhancement (fixed)
Allow symlinked plugins
Reported by: | scribu | Owned by: | rmccue |
---|---|---|---|
Milestone: | 3.9 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Plugins | Keywords: | needs-patch |
Focuses: | Cc: |
Description (last modified by )
There are many scenarios where one would like to have a plugin's folder symlinked to another location.
A couple of these scenarios are described in #13550.
However, when using symlinks, code such as this fails:
plugins_url( 'script.js', __FILE__ );
This happens because __FILE__
resolves to the real path, which confuses plugin_basename().
The most simple and most flexible solution is to add a filter to plugin_basename(), leaving individual devs to handle symlinked paths, depending on their environment.
Attachments (22)
Change History (149)
#1
@
14 years ago
- Keywords has-patch added
- Owner set to scribu
- Status changed from new to accepted
Test files coming up.
#2
@
14 years ago
- Description modified (diff)
- Keywords commit added
- Milestone changed from Awaiting Review to 3.2
To test:
The archive already contains a symlink, so it probably won't work on Windows out of the box.
#6
@
14 years ago
- Milestone changed from 3.2 to Future Release
Not a 3.2 feature moving to Future Release.
#7
@
14 years ago
- Milestone Future Release deleted
- Resolution set to worksforme
- Status changed from accepted to closed
Well what'ya know... there's already a 'plugins_url' filter which can be used just as well.
#8
@
14 years ago
- Milestone set to Future Release
- Resolution worksforme deleted
- Status changed from closed to reopened
Found a case that's not covered by the 'plugins_url' filter: register_*_hook().
#9
@
14 years ago
Yes, very annoying, it boils down to lack of feature in PHP, see PHP bug 46260 (I actually believe best way to fix PHP bug is to create new constant like __FILELINK__
to keep backwards compatibility). But getting that fixed anytime soon is not reality.
@scribu, can you elaborate what kind of problems you encountered using plugins_url filter hack for symlinkked plugins? The problem on those register_*_hook()
seems to be in plugin_basename()
.
Alternative for plugins_url
filter that converts plugin realpath to WP plugin path is naturally to make all symlinked plugins to create their URL differently, e.g. using plugin WPMU_PLUGIN_URL
and WP_PLUGIN_URL
.
#10
@
14 years ago
Well, anything that uses plugin_basename(__FILE__)
won't work properly and can't be fixed by hooking into 'plugins_url'.
Changing each plugin to pass to plugin_basename() what it would expect is not a good solution.
Hence the proposed filter, which would allow to handle this from a single point.
#11
@
13 years ago
- Keywords commit removed
An alternative solution would be to allow plugins from multiple directories, like we do for themes.
@
13 years ago
Fix for correctly extracting the local directory and filename regardless if the file comes from a symlink or not
#12
@
13 years ago
- Resolution set to worksforme
- Status changed from reopened to closed
the above patch has been tested in our local and production environments. The patch is really simple as all that's needed is to use the basename() and dirname() functions to extract the local directory and filename from the passed in file.
dirname doc = http://www.php.net/manual/en/function.dirname.php
basename doc = http://www.php.net/manual/en/function.basename.php
or use pathinfo to get all the information = http://www.php.net/manual/en/function.pathinfo.php
#13
@
13 years ago
- Resolution worksforme deleted
- Status changed from closed to reopened
The ticket will be closed automatically when a fix is commited.
What's up with WP_fix_for_plugin_basename_to_allow_symlinks_2011-07-28.2.patch?
#14
follow-up:
↓ 16
@
13 years ago
this patch is incorrect. it only works two levels deep. my issue is that that a lot of the jetpack modules are acting screwy when symlinked, and their JS and CSS files are 3/4 levels deep in the plugins folder.
#16
in reply to:
↑ 14
@
13 years ago
To clarify, the WP_fix_for_plugin_basename_to_allow_symlinks_2011-07-28.2.patch is a patch to the WP_fix_for_plugin_basename_to_allow_symlinks_2011-07-28.patch so apply the WP_fix_for_plugin_basename_to_allow_symlinks_2011-07-28.patch first then the WP_fix_for_plugin_basename_to_allow_symlinks_2011-07-28.2.patch.
Replying to yincrash:
this patch is incorrect. it only works two levels deep. my issue is that that a lot of the jetpack modules are acting screwy when symlinked, and their JS and CSS files are 3/4 levels deep in the plugins folder.
This patch is not incorrect. You're welcome to contribute to it if you have found scenarios that have nested paths, but I'll bet those scenarios are easily solved by using relative paths to the directory determined by this plugin basename patch.
#17
@
13 years ago
- Cc paulschreiber@… added
I keep getting bit by this and end up patching individual plugins (Raw HTML Pro, Advanced Custom Fields, More Fields, Post Page Association, etc.) They end up generating bad paths to CSS and JS — the <link> and <script> tags have the filesystem path embedded in them.
It would be nice if WordPress' plugins_url() did the right thing out of the box.
There are other complaints about symlinked plugins:
I urged you to seriously consider fixing this for 3.3.x or 3.4.
#22
@
13 years ago
Had the same issue - symlinks are defintely important if one is managing a complex WP farm.
Applying the patches did solve the issue, so I propose that these are included upstream.
#27
@
12 years ago
- Cc mitcho@… added
Patching up plugin_basename()
(or allowing for that patching) isn't the only issue here; symlinked files will show up in the plugins listing in wp-admin
, but symlinked directories are not. I just attached a patch which also touches get_plugins()
to support this.
My fix to plugin_basename()
is also a little more robust, but perhaps more overhead than wanted. I hope this patch is helpful for others.
In the mean time, if this functionality should come as a plugin, scribu's pre_plugin_basename
isn't the only hook needed; a filter in get_plugins()
will also be necessary. If these filters are added, I'd be happy to release a "Symlink Plugins Support" plugin.
#29
@
12 years ago
- Cc hertzog@… added
FTR, this feature is also needed for the setup picked by the official Debian package where we wanted to have packaged plugins in and manually installed plugins kept in two different directories (which is not really possible, so we put symlink to packaged plugins in the directory which is controlled by the local administrator). It would thus be nice to see this issue fixed.
See http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=686228 for a report of this bug on the Debian side.
#30
@
12 years ago
- Cc kwisatz added
Weirdly enough, while augustash's patch works for me (debian wp 3.4.2 package), mitchoyoshitaka's (included in there) does not.
#31
@
12 years ago
This is also an issue for WordPress sites deployed via Capistrano.
Capistrano keeps previous deployments of the site so you can easily roll back changes. So the actual filesystem path to the site might be:
/home/username/web/example.com/releases/20121018101521/public
But it's symlinked as:
/home/username/web/example.com/current/public
The latter is the path in my web server config. The current directory is a symlink to the latest deployment, which is actually the datestamped releases/20121018101521
directory.
As a result, the entire site is in a symlinked directory. This isn't an issue for most plugins but it's come up more than once in my experience running a handful of sites this way.
#35
@
12 years ago
Since #22802 was reopened anyway, let's keep this ticket focused on a solution that would enable symlinked plugins without plugin authors having to do anything special.
#39
follow-up:
↓ 57
@
12 years ago
I resolved all my plugin problems caused by symlinks in the wp-content path by using the realpath() function in my wp-config.php. I simply replace this:
define('WP_CONTENT_DIR',$_SERVER['DOCUMENT_ROOT'] . '/wp-content');
With this:
define('WP_CONTENT_DIR', realpath($_SERVER['DOCUMENT_ROOT'] . '/wp-content'));
Thought that might help anyone ending up in this ticket when troubleshooting.
#41
@
12 years ago
- Cc mike@… added
- Keywords dev-feedback added
- Version set to trunk
I've uploaded a path with another approach. The patch hooks 'pre_update_option_active_plugins'
so anytime 'active_plugins'
is saved it finds which active plugins have been symlinked and saves an array to a 'symlinked_plugins'
key of wp_options
. The array has an element for each of the symlinked plugins with the array key being realpath( $virtual_plugin_file )
and the value being virtual_plugin_file
, i.e.
update_options( 'symlinked_plugins', array( '/Users/user/Plugins/my-plugin-1/my-plugin-1.php' => '/Users/user/Sites/test/wp-content/plugins/my-plugin-1/my-plugin-1.php', '/Users/user/Plugins/my-plugin-2/my-plugin-2.php' => '/Users/user/Sites/test/wp-content/plugins/my-plugin-2/my-plugin-2.php', ));
Then plugins_url()
simply checks to see if the value of the $plugin
parameter -- which is typically passed in as __FILE__
-- is a key in the array returned by get_option( 'symlinked_plugins' )
and if yes it assigns $plugin
with that array element's value.
I think that's all it takes. I've been circling this problem for a while so maybe this is all we need to enable plugins_url()
to work correctly with symlinked plugins. But I might have missed a requirement and if so will need others to point out what I've missed.
Also please consider this a starting point for discussion; I don't expect that it's completely ready for production, but crossing fingers it might be.
#42
follow-up:
↓ 43
@
12 years ago
Re symlinked_plugins.diff: the check you added to plugin_url() should be moved to plugin_basename(), since that's the more general function (and also the one that doesn't have a filter).
#43
in reply to:
↑ 42
@
12 years ago
Replying to scribu:
Re symlinked_plugins.diff: the check you added to plugin_url() should be moved to plugin_basename(), since that's the more general function (and also the one that doesn't have a filter).
Originally I didn't put in plugin_basename()
because it looked like that would not have worked but I just tested and you are correct, it works. I also tested it with a symlinked plugin where the virtual directory was different than the real directory and it still worked correctly, i.e. my-plugin-dev/my-plugin.php
vs. my-plugin/my-plugin.php
:
Virtual Path => /Users/my-user/Site/my-site/wp-content/plugins/my-plugin-dev/my-plugin.php Real Path => /Users/my-user/Plugins/my-plugin/my-plugin.php
I also added an enhancement that omits the 'symlinked_plugins'
option if there are no symlinked plugins so that the options table won't won't be polluted with that entry when there are no plugins that have been symlinked.
Attached is a new patch with the check moved to plugins_url()
.
#50
@
12 years ago
This has been working pretty well over the past few months.
Is there any chance of this making it into 3.6 so that I can update WordPress without fear of loosing all these code changes?
It has made a HUGE difference for me on the organization of my server files and makes maintaining dozens of sites on a single server so much easier. I no longer have to worry about any clients getting lazy and not updating their plugins and potentially leading to a security breach.
Has anyone at all come up with a problem that would cause this not to work properly in time for 3.6?
*If for some reason it can't be done in time, is it possible to turn this into an 'mu' plugin in the meantime?
#51
@
12 years ago
Looks like MikeSchinkels code works for me.
Agree with everyone else here, please add this to the core. Putting it into 3.6 would be great so we don't have to wait 6 months.
#52
follow-up:
↓ 53
@
12 years ago
If adding my patch to the core is too much for 3.6, can you please consider adding a 'pre_plugin_basename'
hook that would allow us to create an mu-plugin to do the rest? I've attached a patch for that, above.
#53
in reply to:
↑ 52
@
12 years ago
Replying to MikeSchinkel:
If adding my patch to the core is too much for 3.6, can you please consider adding a
'pre_plugin_basename'
hook that would allow us to create an mu-plugin to do the rest? I've attached a patch for that, above.
+1 if we can't have a core patch, this would work just fine for now!
#54
follow-up:
↓ 55
@
12 years ago
16953.diff could go in before the beta, but we don't typically add new hooks after feature freeze.
#55
in reply to:
↑ 54
@
12 years ago
Replying to SergeyBiryukov:
16953.diff could go in before the beta, but we don't typically add new hooks after feature freeze.
Given how this ticket has been languishing for 2 years with lots of interest and having come full circle, it sure would be nice. :)
#57
in reply to:
↑ 39
;
follow-up:
↓ 64
@
12 years ago
Replying to markus.magnuson:
I resolved all my plugin problems caused by symlinks in the wp-content path by using the realpath() function in my wp-config.php. I simply replace this:
define('WP_CONTENT_DIR',$_SERVER['DOCUMENT_ROOT'] . '/wp-content');With this:
define('WP_CONTENT_DIR', realpath($_SERVER['DOCUMENT_ROOT'] . '/wp-content'));Thought that might help anyone ending up in this ticket when troubleshooting.
I can confirm that the below solution worked for me in using:
define('WP_PLUGIN_DIR', realpath('/home/152858/domains/example.com/html/all/wp-content/plugins') ); define('WP_PLUGIN_URL', 'http://example.com/all/wp-content/plugins'); define('WP_THEMES_DIR', realpath('/home/152858/domains/example.com/html/all/wp-content/themes') ); define('WP_THEMES_URL', 'http://example.com/all/wp-content/themes'); define('WP_CONTENT_DIR', realpath('/home/152858/domains/example.com/html/all/wp-content') ); define('WP_CONTENT_URL', 'http://example.com/all/wp-content');
But, I have been using this as of late: https://gist.github.com/aubreypwd/7828624
#64
in reply to:
↑ 57
@
12 years ago
Worked for me as well.
Replying to aubreypwd:
I can confirm that the below solution worked for me in using:
define('WP_PLUGIN_DIR', realpath('/home/152858/domains/example.com/html/all/wp-content/plugins') ); define('WP_PLUGIN_URL', 'http://example.com/all/wp-content/plugins'); define('WP_THEMES_DIR', realpath('/home/152858/domains/example.com/html/all/wp-content/themes') ); define('WP_THEMES_URL', 'http://example.com/all/wp-content/themes'); define('WP_CONTENT_DIR', realpath('/home/152858/domains/example.com/html/all/wp-content') ); define('WP_CONTENT_URL', 'http://example.com/all/wp-content');Replying to markus.magnuson:
I resolved all my plugin problems caused by symlinks in the wp-content path by using the realpath() function in my wp-config.php. I simply replace this:
define('WP_CONTENT_DIR',$_SERVER['DOCUMENT_ROOT'] . '/wp-content');With this:
define('WP_CONTENT_DIR', realpath($_SERVER['DOCUMENT_ROOT'] . '/wp-content'));Thought that might help anyone ending up in this ticket when troubleshooting.
#65
follow-up:
↓ 66
@
12 years ago
I didn't see until now there was a consensus on a simple filter. My question: What's more useful? pre_plugin_basename, or one just before the return? I'd think the final filter would be more useful, since it smooths out the processing?
#66
in reply to:
↑ 65
;
follow-up:
↓ 67
@
11 years ago
Replying to nacin:
My question: What's more useful? pre_plugin_basename, or one just before the return? I'd think the final filter would be more useful, since it smooths out the processing?
Yes, given that you are passing in both a post-processed value and a pre-processed value a 'plugin_basename'
would definitely be more generally useful and also still meet the need stated on comment 52 of this thread.
#67
in reply to:
↑ 66
@
11 years ago
Replying to MikeSchinkel:
Replying to nacin:
My question: What's more useful? pre_plugin_basename, or one just before the return? I'd think the final filter would be more useful, since it smooths out the processing?
Yes, given that you are passing in both a post-processed value and a pre-processed value a
'plugin_basename'
would definitely be more generally useful and also still meet the need stated on comment 52 of this thread.
UPDATE: Except, the hook would have to duplicate the logic in lines 567-574 in 16953.2.diff when changing the path, so when the goal is to meet the need stated on comment 52 a 'pre_plugin_basename'
would be a better choice because it would limit the potential for error on the part of the plugin author.
#70
@
11 years ago
Generally I'm 100% against what @MikeSchinkel proposes: Syncing one option set with another one. To elaborte the why: We already got that problem in other places, so let's please not add another one. Example: Parent/Child relationships are stored inside get_option( "{$taxonomy}_children" );
. Bring that out of sync per accident and you're screwed. A bug that's buried inside a DB option is harder to trace back. And so you already know that you then need will something like _get_term_hierarchy( $tax );
to fix syncing in case.
To stick with Mikes example from #22802 and his directory structure:
/home/myacct/mysite1/ <-- Site #1 goes here /home/myacct/mysite2/ <-- Site #2 goes here /home/myacct/plugins/ <-- All the shared plugins go here # 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
Wouldn't it be easier to read out the full path and check where exactly you are? Doing this on the fly won't be easier than any other approach, but over all easier to debug. And PHP already got stuff like getcwd()
that we can use alongside the directory constants and ABSPATH
to compare real paths and symlinked paths.
#71
@
11 years ago
Discussed on IRC with nacin, best path forward looks to be creating a path cache.
I know exactly how to write the patch, but haven't the time yet. Patches welcome if anyone does.
The approach I'd take is to create a new global (ick, I know, but it can be edited on plugins_loaded
) containing a map of dirname( $plugin ) => dirname( realpath( $plugin ) )
in wp-settings.php, then changing plugin_basename
to use it. It's simple and it's fairly fast on load (realpath
is already hit in include
, so the stat cache should fix that). Bonus points for rolling the filter in as well.
#73
@
11 years ago
- Cc chatfielddaniel@… added
A very inelegant solution, but it is working for me:
https://github.com/danielchatfield/wordpress-symlink-fixer
Can either be a regular plugin or a must-use plugin (some plugins will still not work unless this is a must-use plugin, but mine do).
#74
@
11 years ago
- Keywords needs-testing added
- Owner set to rmccue
Attached patch creates a realpath cache of sorts to match real directories up with their original version. Also introduces a wp_normalize_path()
function, since there's a lot of duplicate code in core doing exactly that (I only converted plugin_basename
while I was there, but there's plenty more places ripe for patching that in a separate ticket).
Tested locally (Windows system with junction points) and it works. I'd like to have a few more people test it though to make sure.
The only concern I have here is performance, which is why it only stores differing paths (plugins are loaded once, but plugin_basename
is called many times). Calling realpath
shouldn't be a concern, since it's called on the filename itself which should push it into the stat cache, and is just repeating the realpath
call that happens in include
anyway.
#75
@
11 years ago
Added a filter at the end, same format as nacin's. (Missing hook documentation at the moment though. Added in .4.2.diff, although I gaffed the naming right up.)
#77
@
11 years ago
@rmccue — 16953.4.2.diff:
- Short description needs a period
- Wrap the long description at the comma
- Periods at the end of the @param and @return lines
:-)
#78
@
11 years ago
The docs are ready in 16953.6.diff.
@rmccue: Inline Documentation Standards
#80
@
11 years ago
- Keywords needs-testing removed
16953.7.diff now checks on activation as well, and splits it out into a separate function. This should cover every case in core, I believe.
#83
in reply to:
↑ 82
;
follow-up:
↓ 84
@
11 years ago
Replying to rmccue:
16953.7.diff now checks on activation as well, and splits it out into a separate function. This should cover every case in core, I believe.
Great work. However you missed uninstall_plugin()
. Patch 16953.8.diff attached.
#84
in reply to:
↑ 83
@
11 years ago
- Milestone changed from Future Release to 3.8
Replying to MikeSchinkel:
Great work. However you missed
uninstall_plugin()
. Patch 16953.8.diff attached.
Thanks for catching that. This is in dd32's hands for committing to the 3.8 branch.
#85
@
11 years ago
Have been working on a solution for a private project, but the latest patch seems to cover it all. Works great for me, love to have it in a future release.
#91
@
11 years ago
- Keywords 3.9-early added; 3.8-early removed
- Milestone changed from 3.8 to Future Release
Bumping now that we're in beta. Had this have been a longer cycle I'd have gladly put this in for testing, so lets see come 3.9.
Did anyone investigate if this would cause any performance issues if plugins_url()
is called often?
#93
follow-up:
↓ 94
@
11 years ago
16953.9.diff refreshes 16953.8.diff and adds support for network-active plugins in multisite. I moved the initialization of $GLOBALS['wp_plugin_paths']
to up above where the mu-plugins are loaded. Basically it needs to be initialized as an array before there are any calls to plugin_basename()
, otherwise that will give an "Invalid argument supplied for foreach" error. Maybe there is a better place to initialize that?
#94
in reply to:
↑ 93
;
follow-up:
↓ 95
@
11 years ago
Replying to jdgrimes:
16953.9.diff refreshes 16953.8.diff and adds support for network-active plugins in multisite. I moved the initialization of
$GLOBALS['wp_plugin_paths']
to up above where the mu-plugins are loaded. Basically it needs to be initialized as an array before there are any calls toplugin_basename()
, otherwise that will give an "Invalid argument supplied for foreach" error. Maybe there is a better place to initialize that?
I didn't think plugin_basename()
could safely be used in mu-plugins? If so, +1 on your patch then.
#95
in reply to:
↑ 94
@
11 years ago
Replying to rmccue:
I didn't think
plugin_basename()
could safely be used in mu-plugins? If so, +1 on your patch then.
It wouldn't be used by the mu-plugin for its own path, but I know for example that sometimes mu-plugins are just used to load a regular plugin, which will likely be using plugin_basename()
.
#96
follow-up:
↓ 98
@
11 years ago
I have tested 16953.9.diff and it works well for a situation where I was bit by this "bug". My situation involves a Capistrano-like "releases" directory that is symlinked to a web root. Within the WordPress installation, the wp-content directory has been moved. I experienced problems with WordPress SEO and this fixed it.
Now that 3.9 is moving along, can this be further tested and integrated. I think that we should aim to build a suite of unit tests that address the different scenarios. This will likely be tricky given that to perform the tests, the environment needs to be configured in a special way. It's probably worth-while to increase the confidence that the chosen solution is backwards compatible, yet solves the issues.
#97
@
11 years ago
16953.9.diff also worked great in an environment I was testing. It's got a WordPress Skeleton-esque setup with core and wp-content in the same directory; inside wp-content: mu-plugins, plugins, and themes are all symlinked. Installing the patch fixed all the 404's, and I've yet to run into any issues.
#98
in reply to:
↑ 96
;
follow-up:
↓ 101
@
11 years ago
Replying to tollmanz:
I have tested 16953.9.diff and it works well for a situation where I was bit by this "bug". My situation involves a Capistrano-like "releases" directory that is symlinked to a web root. Within the WordPress installation, the wp-content directory has been moved.
This isn't really Capistrano-specific, or really related at all, as Capistrano is symlinked from the web root. At least where I have seen this, wp-content is not typically then symlinked from elsewhere.
I found a few missing wp_register_plugin_realpath() calls. Specifically around plugin sandboxing and also for mu-plugin inclusion. Network plugins are in the normal plugins directory. mu-plugins are separate; they are indeed supported by plugin_basename() and are represented by the WPMU_PLUGIN_DIR directory.
Trying this out sans-filter.
#101
in reply to:
↑ 98
;
follow-up:
↓ 102
@
11 years ago
Replying to nacin:
This isn't really Capistrano-specific, or really related at all, as Capistrano is symlinked from the web root. At least where I have seen this, wp-content is not typically then symlinked from elsewhere.
I think my situation was similar to what you described. "wp-content" is not symlinked elsewhere. It follows a typical Capistrano setup.
Thanks for landing this. I'll test my scenario and report back to you.
#102
in reply to:
↑ 101
@
11 years ago
Replying to tollmanz:
I think my situation was similar to what you described. "wp-content" is not symlinked elsewhere. It follows a typical Capistrano setup.
This is more about handling if a plugin itself is symlinked; e.g. wp-content/plugins/myplugin/
-> /var/www/common/myplugin/
Moving wp-content
is separate (and usually done by defining WP_CONTENT_DIR
/WP_CONTENT_URL
) and probably won't be affected here.
Also, <3 nacin.
#103
@
11 years ago
This. is. great! Thanks!
Just briefly tested this, and it seems that load_plugin_textdomain()
(and related functions) might also need some treatment.
#104
@
11 years ago
- Resolution set to fixed
- Status changed from assigned to closed
load_plugin_textdomain() is fine; it's just for inclusion. The only point of this is for plugin_basename() accuracy.
#105
follow-ups:
↓ 106
↓ 107
@
11 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
I upgraded a 3.8.1 install to 3.9-beta1 via WP-CLI today and got about a dozen of these:
PHP Warning: Invalid argument supplied for foreach() in wp-includes/plugin.php on line 620
Attaching 16953.10.diff to check if it's an array before trying to iterate over it.
#106
in reply to:
↑ 105
@
11 years ago
Replying to iandunn:
I upgraded a 3.8.1 install to 3.9-beta1 via WP-CLI today and got about a dozen of these:
PHP Warning: Invalid argument supplied for foreach() in wp-includes/plugin.php on line 620Attaching 16953.10.diff to check if it's an array before trying to iterate over it.
I am trying and failing to determine when this would occur.
#107
in reply to:
↑ 105
@
11 years ago
Replying to iandunn:
I upgraded a 3.8.1 install to 3.9-beta1 via WP-CLI today and got about a dozen of these:
PHP Warning: Invalid argument supplied for foreach() in wp-includes/plugin.php on line 620
Confirmed (with WP-CLI). Only got two of them though, and only on subsequent upgrades to the current nightly build, not on the initial one.
#108
@
11 years ago
I suspect this could be a WP-CLI problem. Relevant PR over there is 1019.
Only got two of them though, and only on subsequent upgrades to the current nightly build, not on the initial one.
My guess: on first load, wp_register_plugin_realpath
doesn't exist (or WP-CLI doesn't realise it does), so the plugin paths never get loaded. On second load, the files have been updated. Still not sure how that would occur, but something along those lines could explain it.
#109
@
11 years ago
Actually, looks like the above PR already fixes it; original ticket is 1015. We could add the check for WP-CLI specifically, but not sure if we should.
#110
@
11 years ago
- Resolution set to fixed
- Status changed from reopened to closed
So: updating WP-CLI fixes things, and technically nothing is broken (you just get a warning out of it). With that I'm fine with calling this fixed again.
#111
@
11 years ago
FWIW, I tested with WP-CLI 0.14.1, freshly downloaded from http://wp-cli.org/.
Looks like it includes the commit mentioned in comment:108, but I still got two warnings.
#112
@
11 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
I have a symlink that looks like this:
wp-content/mu-plugins/p2p-debug -> wp-content/plugins/posts-to-posts/debug.php
$wp_plugin_paths
ends up looking like this:
[ 'wp-content/mu-plugins/' => 'wp-content/plugins/posts-to-posts' ]
This breaks all calls to plugins_url()
inside the Posts 2 Posts plugin, but I'm assuming that most plugins don't have helper files like that.
What worries me is that all symlinks inside mu-plugins end up pointing to the directory of the first symlink. That doesn't seem sane.
So, I propose we remove the wp_register_plugin_realpath( $mu_plugin );
line.
#113
@
11 years ago
Agreed with scribu; mu-plugins
can't be handled the same way (since they're all in the same directory).
Makes me wonder if the same can occur if you have e.g. plugins/hello.php -> /usr/hello.php
without a directory?
#114
@
11 years ago
Replying to rmccue:
Makes me wonder if the same can occur if you have e.g.
plugins/hello.php -> /usr/hello.php
without a directory?
I've tested and can confirm this is the case.
mu-symlinks.diff resolves symlinked MU plugins or single file plugins. I'm not sure if that's necessary as far as plugins_url()
, but if it doesn't cause problems it might be good for consistency.
#115
follow-up:
↓ 116
@
11 years ago
It looks like mu-symlinks.diff would need to normalize paths on both sides of the conditional.
I'm open to suggestions as to what to do here. Do we resolve symlinks for single files, or only for full directories? I'm surprised it took us this long to notice. What's the best path forward? We'll have no choice but to revert the whole thing for 3.9 unless there is a plan of action on how to proceed.
#116
in reply to:
↑ 115
@
11 years ago
Replying to nacin:
It looks like mu-symlinks.diff would need to normalize paths on both sides of the conditional.
The paths are normalized on 645-646 in the patch.
I'm open to suggestions as to what to do here. Do we resolve symlinks for single files, or only for full directories?
As I said above, IMO, we should resolve symlinks for single files for consistency. Though it may not be useful for plugins_url()
, it would standardize the behavior of plugin_basename()
for all symlinks.
I'm surprised it took us this long to notice. What's the best path forward? We'll have no choice but to revert the whole thing for 3.9 unless there is a plan of action on how to proceed.
I'd really hate to see this get reverted now. I think the reason we took so long to notice was that it really isn't that important in terms of plugins_url()
. We could skip them for now and add them in later if needed. But I don't see any reason not to go ahead and resolve them. In the rare situation where a single-file plugin might need to use plguins_url()
relative to itself it can't.
#117
@
11 years ago
- Keywords needs-patch added; has-patch dev-feedback 3.9-early removed
I'd really hate to see this get reverted now.
I agree, but all features come with a maintenance cost. I've discussed this before, quite possibly in the context of this ticket. If WordPress core is to do something here, then we are committing to maintain it in the future. Right now, it's not even clear if we can maintain it in the present. In this case, the feature hasn't even shipped, so I have little qualms of pulling it out if we can't figure out how to best move forward here. For more on incorporating features, read this essay from 2004.
I don't have the time to do an in-depth investigation on this to be able to articulate the exact rationale for fixing this one way or the other. Someone else will need to do that and articulate exactly what needs to be done for 3.9. Otherwise it needs to be removed. There are 116 comments on this ticket and 52 people watching it. If this is a shallow bug, then I imagine this should not be difficult.
I'm reverting this on Wednesday, April 9th if we don't have a fix. This isn't meant to be vindictive; this just isn't my priority; rather, it's meant to be motivating. If it belongs in core, then it needs to become at least one other person's priority. If you want this feature, then fight for it in the form of fixing it. Thanks. :-)
#118
@
11 years ago
Looking into this now.
I'm in favour of the ignore-single-files solution. If we ignore all mu-plugins (since they're all single file; setups with mu-plugins in subfolders can call wp_register_plugin_realpath
in their custom loaders) and single-file plugins, we should be OK here. You can still symlink either of these, but any references to plugin_url
will break; I don't forsee this being a huge problem, since single-file plugins with external resources are _doing_it_wrong and should use a directory.
#119
@
11 years ago
Added patch that uses scribu's change to remove mu-plugins registration, and also ignore files living in WP_PLUGIN_DIR
and WPMU_PLUGIN_DIR
from being registered.
wp_plugin_register_realpath
now returns a boolean that indicates whether the path could be registered, for those of you using it in your own plugins.
The following now work correctly:
mu-plugins/y.php -> plugins/x/y.php
plugins/y.php -> plugins/x/y.php
Confirmed that the following work:
plugins/x/ -> /arbitrary/path/x/
plugins/ -> plugins-x/
plugins/ -> plugins-x/
andplugins-x/y/ -> /arbitrary/path/y
Any other paths we need to handle?
#120
@
11 years ago
Also worth mentioning, if you use a pattern like mu-plugins/x/x.php
with a mu-plugins/loader.php
file, you should add the following to your loader file:
$plugins = array( 'my-mu-plugin/my-mu-plugin.php', ); foreach ( $plugins as $plugin ) { $path = dirname( __FILE__ ) . '/' . $plugin; // Add this line to ensure mu-plugins subdirectories can be symlinked wp_register_plugin_realpath( $path ); include $path; }
#122
@
11 years ago
16953.no-single-file.2.diff looks good, but it should update wp_register_plugin_realpath()
's docblock with a @return
tag.
'pre_plugin_basename' filter