Skip to content

improve and make more strict autoload.php #28923

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Aug 15, 2020
Merged

improve and make more strict autoload.php #28923

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 24 additions & 12 deletions app/autoload.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,36 +5,48 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

use Magento\Framework\Autoload\AutoloaderRegistry;
use Magento\Framework\Autoload\ClassLoaderWrapper;

/**
* Shortcut constant for the root directory
*/
define('BP', dirname(__DIR__));
\define('BP', \dirname(__DIR__));

define('VENDOR_PATH', BP . '/app/etc/vendor_path.php');
\define('VENDOR_PATH', BP . '/app/etc/vendor_path.php');

if (!file_exists(VENDOR_PATH)) {
if (!\is_readable(VENDOR_PATH)) {
throw new \Exception(
'We can\'t read some files that are required to run the Magento application. '
. 'This usually means file permissions are set incorrectly.'
);
}

$vendorDir = require VENDOR_PATH;
$vendorAutoload = BP . "/{$vendorDir}/autoload.php";
$vendorAutoload = (
static function (): ?string {
$vendorDir = require VENDOR_PATH;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @rvitaliy. Thank you for your collaboration. Could I kindly ask you to provide a comment for these two declarations of $vendorAutoload with highlighting what is a particular block for? At first glance, these blocks of code look very similar (almost the same) and it's hard to guess why we need such an approach. I see that in first case we have the trailing slash, but let's make developers life a little bit easier and highlight this case in the comments. Thank you!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rogyar
sorry, but i can't do it.
personally i don't undestand why we need the second check without BP prefix.
i'm refactor the old code without change to the logics, make it more strict and readable.
IMHO: we can remove the second block of check, app/{$vendor} is not a valid path for vendor.

Copy link
Contributor

@rogyar rogyar Jul 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I see that this functionality has been recently added. So I assume it was done for a purpose.

2054fab

Unfortunately, there's no additional information about this change. Well then, let's leave it as it is. Maybe it's required for some particular circumstances (like CI system or so).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rogyar, that commit is from #18849, so there is your missing information 🙂

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so the second check is need to autoload vendors from the global path

$vendorAutoload = BP . "/{$vendorDir}/autoload.php";
if (\is_readable($vendorAutoload)) {
return $vendorAutoload;
}

$vendorAutoload = "{$vendorDir}/autoload.php";
if (\is_readable($vendorAutoload)) {
return $vendorAutoload;
}

return null;
}
)();

/* 'composer install' validation */
if (file_exists($vendorAutoload)) {
$composerAutoloader = include $vendorAutoload;
} else if (file_exists("{$vendorDir}/autoload.php")) {
$vendorAutoload = "{$vendorDir}/autoload.php";
$composerAutoloader = include $vendorAutoload;
} else {
if ($vendorAutoload === null) {
throw new \Exception(
'Vendor autoload is not found. Please run \'composer install\' under application root directory.'
);
}

$composerAutoloader = include $vendorAutoload;
AutoloaderRegistry::registerAutoloader(new ClassLoaderWrapper($composerAutoloader));