search

lkwdwrd / wp-muplugin-loader

A drop-in MU Plugin loader for WordPress
59 stars 26 forks source link

__DIR__ does not always make it into mu-require.php unescaped, and mu composer type lost on update #12

Open benvoynick opened 5 years ago

benvoynick commented 5 years ago

This PR addresses an issue I encountered where mu-require.php gets generated with an absolute start to the path. I believe this may be the (or a) cause of Issue #2. With an absolute path, the require statement is very likely to break if the code is moved elsewhere, or immediately depending on the configuration for web server, virtual machine, etc.

Specifically, in some circumstances mu-require.php ends up being written something like this: require_once '/path/to/repo/vendor/lkwdwrd/wp-muplugin-loader/src/lkwdwrd/Composer' . '/../../vendor/lkwdwrd/wp-muplugin-loader/src/lkwdwrd/mu-loader.php';

When it needs to always end up something like this: require_once __DIR__ . '/../../vendor/lkwdwrd/wp-muplugin-loader/src/lkwdwrd/mu-loader.php';

It seems like the DIR constant is being replaced by its actual value at composer run time in some circumstances, yet not in others. I can understand why PHP might grab it since it's inside double quotes, though I'm not sure why it's not consistent. composer require and composer remove seem to trigger it a lot, especially if I am not working in the same directory as composer.json at the time. Whereas composer install seems to pretty reliably NOT replace the DIR with the constant's value.

Breaking the string into two concatenated segments in the middle of the DIR constant works around this. Even on composer require and composer remove, I consistently get the actual DIR constant output in my mu-require.php file.

jonathan-dejong commented 4 years ago

Can this please be merged?? seeing this issue as well everytime our CI builds tests.

benvoynick commented 4 years ago

Added an additional commit addressing #16 - it looks to me like fixing that issue with update operations is as simple as processing the target package instead of the initial package.

jenkoian commented 4 years ago

@lkwdwrd hi! Just wondering if you're still maintaining this library and if so would mind taking a look at this PR :) thanks!

BramEsposito commented 4 years ago

One more vote for merging this PR. Would be usefull since I have to edit the mu-require.php 10's of times a day now :)