Skip to content
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

Allow the composer class loader to be absent, for composer/composer:2.2.0 compatibility #90

Merged
merged 2 commits into from Dec 21, 2021

Conversation

boesing
Copy link
Member

@boesing boesing commented Dec 21, 2021

Q A
Documentation no
Bugfix yes
BC Break no
New Feature no

Description

With composer 2.2.0, some autoloaders are loaded even before the vendor/autoload.php file is dumped.
This leads to installation crashes in some circumstances due to the fact that this component throws a RuntimeException in case the autoloader could not be found.

Due to the fact, that this works properly after the autoloader got dumped, we can safely not register the prepend/append autoloader registration for this case.

Closes #89

With composer 2.2.0, some autoloaders are loaded even before the `vendor/autoload.php` file is dumped.
This leads to installation crashes in some circumstances due to the fact that this component throws a `RuntimeException` in case the autoloader could not be found.

Due to the fact, that this works properly after the autoloader got dumped, we can safely not register the prepend/append autoloader registration for this case.

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@boesing boesing added the Bug Something isn't working label Dec 21, 2021
@boesing boesing linked an issue Dec 21, 2021 that may be closed by this pull request
@boesing boesing added this to the 1.4.1 milestone Dec 21, 2021
This was referenced Dec 21, 2021
src/Autoloader.php Outdated Show resolved Hide resolved
src/Autoloader.php Outdated Show resolved Hide resolved
…calls

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@boesing boesing force-pushed the bugfix/handle-not-existing-autoloader branch from 916d5a1 to e71bcbb Compare December 21, 2021 14:00
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

LGTM, although I only reviewed the code structure, not sure if this fixes the actual issue

@boesing
Copy link
Member Author

boesing commented Dec 21, 2021

LGTM, although I only reviewed the code structure, not sure if this fixes the actual issue

Will see if that works after we created release. Actually quite tough to test this without having a "stable" release due to the fact that manipulating composer.json to add this PR via repositories almost always is quite a bunch of work for what I do not have time now.

Since we are getting rid of this component anyways, I think this bugfix should do its trick.
If there are no vetos to merge this, I'd continue releasing this this evening.

@Ocramius
Copy link
Member

We can release right away, unless other patches are needed

@Seldaek
Copy link

Seldaek commented Dec 21, 2021

Looks good to me too for as much as I'm familiar with this code.. At least the intent sounds good :)

Copy link
Member

@internalsystemerror internalsystemerror left a comment

Choose a reason for hiding this comment

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

LGTM. The only thoughts I have are if the file exists but is not readable, but I guess the error for that would be descriptive enough

@Ocramius Ocramius changed the title Allow the class loader to be absent Allow the composer class loader to be absent, for composer/composer:2.2.0 compatibility Dec 21, 2021
Copy link
Member

@weierophinney weierophinney left a comment

Choose a reason for hiding this comment

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

Makes perfect sense to me. 🚢

@Ocramius Ocramius merged commit 88bf037 into laminas:1.4.x Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incompatibility with composer 2.2.0RC1
5 participants