Revise class loader #72
Conversation
…ages instead of directory scanning
Can go back to PSR4 folders once wintercms/storm#72 is merged.
|
This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days. |
bennothommo
added
Status: In Progress
Type: Maintenance
and removed
Status: Archived
labels
Co-authored-by: Ben Thomson <[email protected]>
* develop: (63 commits) Resync model getAttribute override with base Laravel functionality Convert Markdown parser to CommonMark (#133) Improve support for multiple database connections (#132) Make the SectionParser more extendable (#131) Fix static analysis errors Fixed generation of thumbnails for remote disks Get local root path from configured disk Use named arguments add more testing with pivot data Add Str::isJson() | is_json() helpers Code analysis fixes Register slug rule as part of Validation singleton registration Add slug validation rule Use Laravel's CLI components Use Laravel's CLI components Prioritize local dynamic methods over behavior-provided methods (#130) Delete unneeded PHPUnit config Re-enable code analysis on develop branch Fix PHPStan testing, minor tweaks to docs Pass the full model through add() and remove() methods ...
LukeTowers
added
Type: Enhancement
and removed
Status: In Progress
Type: Maintenance
labels
LukeTowers
added
Status: Testing Needed
Status: Completed
and removed
Status: Testing Needed
labels
|
@LukeTowers Is this PR responsible for the class loading issues in Windows? |
|
@damsfx most likely yes. We still haven't encountered the issues you were reporting nor have we heard from anyone else about them so if you're able to track it down on your machine and submit a PR I would appreciate that :) |
|
@LukeTowers When running the Winter.Test plugin migration, I've got this issue. In seed_tables.php line 31:
Class "Winter\Test\Models\Person" not foundThe $path = "P:\_Sites\_Labs\winterdev\plugins\winter\test\Models\Person.php"but the return value is $path = "P:\_Sites\_Labs\winterdev\P:\_Sites\_Labs\winterdev\plugins\winter\test\Models\Person.php"The method adds It may be necesary to include the base path in the test : /**
* Resolve the provided path, relative or absolute
*/
protected function resolvePath(string $path): string
{
if (!Str::startsWith($path, ['/', '\\', $this->basePath . DIRECTORY_SEPARATOR])) {
$path = $this->basePath . DIRECTORY_SEPARATOR . $path;
}
return $path;
} |
|
Oh, I see, on windows the basePath does not necessarily start with a slash or backslash... Care to submit a PR @damsfx ? |
On windows the basePath does not necessarily start with a slash or backslash. See : wintercms#72 (comment)
Fixes wintercms/winter#214.
This fixes a long standing issue with the ClassLoader that handles loading module and plugin classes where only lowercase paths to the (properly cased) class file were technically supported. This prevents PSR-4 folder structures in plugins and modules from working; and even worse it would usually only show up in production with an opaque error message since production machines typically have case sensitive filesystems and development machines tend to not.
The current logic uses a directory scanning approach of generating many different possible variations of a valid path for a given class and then looping over the "loaded directories" to scan each of them for a valid instance of a possible path.
The new logic instead switches to requiring class prefix to class path to be registered with the ClassLoader explicitly which not only makes the resolution logic much faster (since the prefix is directly matched with the request class instead of always having to scan for a potential match) but it also means that the autoloading can be completely disabled for plugins and modules that aren't currently enabled which isn't currently the case and will help with fully cutting off disabled plugins and modules from the application.