Fixed autoloader patch to work with composer-installed setup

[colinmollenhour]

As per #3216 this adds back the automatic detection of vendor in the parent directory of BP.

[fballiano]

isnt't there a way to say "require dirname(DIR) . etc etc OR require DIR . etc etc", to keep it some sort of one liner?

[colinmollenhour]

isnt't there a way to say "require dirname(DIR) . etc etc OR require DIR . etc etc", to keep it some sort of one liner?

Maybe... I try to stick with common stuff that everyone can easily read, though. If it takes more than a few seconds to come up with it will probably be confusing to some readers. :)

[fballiano]

the thing with require that it triggers an error otherwise it would be super easy to read:
require ../vendor/autoload.php || require vendor/autoload.php
but...

[fballiano]

briefly tested

[m-overlund]

This patch seems to crash my site (non-composer install)

[elidrissidev]

This patch seems to crash my site (non-composer install)

Even the non-composer install should normally have the vendor folder. What's the error you're getting?

[m-overlund]

i believe it was a open basedir restriction - trying to load vendor dir outside public_html instead of in public_html.
Will check later on

[m-overlund]

@elidrissidev it was open basedir restriction for is_dir() function - trying to find vendor folder outside public_html.
It gives me a lot of errors every second.

did a dirty fix of the composer patch in get.php and app/Mage.php by including public_html in the path and the errors are gone.

(It however still doesn't seem to load things like redis session module for instance, from the vendor)

$autoloaderPath = getenv('COMPOSER_VENDOR_PATH');
if (!$autoloaderPath) {
    $autoloaderPath = dirname($bp) . $ds . 'public_html' . $ds .  'vendor';

[elidrissidev]

@m-overlund ahh got it. You could also avoid the issue by setting COMPOSER_VENDOR_PATH env variable in your vhost or htaccess.

@fballiano @colinmollenhour What do you think if the two lines were swapped instead, since most installs will likely have vendor within the same folder?

 $autoloaderPath = getenv('COMPOSER_VENDOR_PATH');
 if (!$autoloaderPath) {
-    $autoloaderPath = dirname(BP) . DS .  'vendor';
+    $autoloaderPath = BP . DS . 'vendor';
     if (!is_dir($autoloaderPath)) {
-        $autoloaderPath = BP . DS . 'vendor';
+        $autoloaderPath = dirname(BP) . DS .  'vendor';
     }
 }

[fballiano]

@elidrissidev I think it was done this way for some reason but I can recall it from top of my head now

[m-overlund]

@m-overlund ahh got it. You could also avoid the issue by setting COMPOSER_VENDOR_PATH env variable in your vhost or htaccess.

Thanks - that was a better solution.

Maybe it should be added to htaccess (commented out)
and to Readme, if it is required, and cannot be fixed otherwise

[addison74]

Creating a variable for Nginx webserver must also be provided. You can push a PR that includes the modification in .htaccess and a paragraph in the README file.

[colinmollenhour]

What do you think if the two lines were swapped instead, since most installs will likely have vendor within the same folder?

No objection. I think the motivation was to prevent an attacker from creating a vendor directory in the root which overrides the one in the parent directory, but I think if that prevents people from using open_basedir then it is preferable to have open_basedir supported.

[m-overlund]

Creating a variable for Nginx webserver must also be provided. You can push a PR that includes the modification in .htaccess and a paragraph in the README file.

I don't believe it will be necessary if switching the lines ends up being implemented, like @colinmollenhour suggests.