-
Notifications
You must be signed in to change notification settings - Fork 885
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.
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
Dependency Injection: fix custom loader on Windows #16364
Dependency Injection: fix custom loader on Windows #16364
Conversation
Both the returned value from `glob` as well as the classmap generated by Composer contain file paths with a mix of forward and backward slashes when run on Windows. This was breaking the `compile-di` command. Previously it was _silently_ broken - it wouldn't work, but without throwing an error. Since the addition of the `Schema_Blocks` it would fail with an exception. This commit fixes this by: 1. Adding a `normalize_slashes()` method. 2. Calling that method in the relevant places in the script to make sure that paths are normalized before they are compared based on their string value.
The `Custom_Loader::getClassFromClassMap()` would do a loop through all classes registered in the classmap via a foreach for _each_ class requested (which could be many). By flipping the array and using `isset()` instead, the functionality remains the same, but is infinitely faster.
CR 👍 Note: on Windows, the build fails for an unrelated, new, error on the task |
@enricobattocchi was so kind to test on his Linux machine: |
works great on windows 👍 |
Context
Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
Compile-DI: fix custom loader on Windows
Both the returned value from
glob
as well as the classmap generated by Composer contain file paths with a mix of forward and backward slashes when run on Windows.This was breaking the
compile-di
command.Previously it was silently broken - it wouldn't work, but without throwing an error.
Since the addition of the
Schema_Blocks
it would fail with an exception.This commit fixes this by:
normalize_slashes()
method.Custom_Loader::getClassFromClassMap(): efficiency fix
The
Custom_Loader::getClassFromClassMap()
would do a loop through all classes registered in the classmap via a foreach for each class requested (which could be many).By flipping the array and using
isset()
instead, the functionality remains the same, but is infinitely faster.Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
This should be tested on the three OS-es used by devs: Linux, MacOS and Windows.
trunk
, runcomposer install
.On Linux and Mac this should be successful already, on Windows it will fail with an exception during the
compile-di
step.src/generated
folder contains both thecontainer.php
and thecontainer.php.meta
file and that those are not empty.If all is good, the command should now finish successfully on all three OSes.