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

fix: detect the auto-loader of the project you're installed in #91

Closed
wants to merge 1 commit into from

Conversation

dkarlovi
Copy link
Contributor

Relative to roave/backward-compatibility-check/bin/

Visible if installed using composer-bin.

Relative to roave/backward-compatibility-check/bin/

Visible if installed using composer-bin.
@Ocramius
Copy link
Member

@dkarlovi what do you mean by composer-bin here? How can we reproduce the issue?

@dkarlovi
Copy link
Contributor Author

dkarlovi commented Aug 20, 2018

$ mkdir rcc && cd rcc
$ composer require roave/backward-compatibility-check
$ cd vendor/ # if in root, it works as if by chance
$ roave/backward-compatibility-check/bin/roave-backward-compatibility-check 
PHP Fatal error:  Uncaught RuntimeException: Could not find Composer autoload.php 
$ ./bin/roave-backward-compatibility-check
PHP Fatal error:  Uncaught RuntimeException: Could not find Composer autoload.php 

etc.

@asgrim
Copy link
Member

asgrim commented Aug 20, 2018

You shouldn't need to cd vendor just run vendor/bin/roave-backward-compatibility-check in the root of your project after requiring it (side note: probably best to use --dev)

@dkarlovi
Copy link
Contributor Author

If you're using Composer bin plugin (directly or indirectly, like in referenced issue), it breaks.

@dkarlovi
Copy link
Contributor Author

dkarlovi commented Aug 20, 2018

@asgrim

You shouldn't need to cd vendor just run vendor/bin/roave-backward-compatibility-check in the root of your project after requiring it

This was just a reproducer, I'm not using it like that at all, it's installed in a Docker image used as a portable QA/tooling environment. It still can't rely on my project's autoloader, it needs to use its own, as here.

@dkarlovi
Copy link
Contributor Author

@asgrim this one is actually faulty since it loads an autoloader based on where in the filesystem you are. Roll a dice, load an autoloader! :)

@Ocramius
Copy link
Member

I'm closing this one as "won't fix" because of this:

If you're using Composer bin plugin (directly or indirectly, like in referenced issue), it breaks.

As much as I like supporting various use-cases, supporting plugins that move things around is not in my interest. I've already previously denied similar patches (in other projects) for people that use the vendor-dir config, because it just opens more rabbit holes.

As for the getcwd(), it should probably indeed be removed: opening a separate patch for that.

@dkarlovi
Copy link
Contributor Author

Fair, but in that case you should probably remove all the auto-loader paths except the one you support as a use case.

You can even remove the whole autoloader heuristic, come to think about it. 👍

@dkarlovi dkarlovi deleted the fix-autoloader-path branch August 21, 2018 07:27
@Ocramius
Copy link
Member

@dkarlovi currently removing all paths that are not in the pre-determined locations

@Ocramius
Copy link
Member

Note: manually merged this patch anyway, but the path is a replacement for existing ones, not an additional one 👍

Ocramius added a commit that referenced this pull request Aug 21, 2018
As per discussion in #91 (comment) (#91), the autoloader
is in a fixed position, relative to the executable entry-point,
so a `getcwd()` is unnecessary, and can actually
lead to loading the wrong composer autoloader,
which in turn may lead to loading analysed files,
which is exactly what we want to avoid.
@Ocramius Ocramius added this to the 1.1.0 milestone Aug 21, 2018
@Ocramius
Copy link
Member

Included in #94

uzibhalepu added a commit to uzibhalepu/BackwardCompatibilityCheck that referenced this pull request Aug 6, 2024
As per discussion in Roave/BackwardCompatibilityCheck#91 (comment) (#91), the autoloader
is in a fixed position, relative to the executable entry-point,
so a `getcwd()` is unnecessary, and can actually
lead to loading the wrong composer autoloader,
which in turn may lead to loading analysed files,
which is exactly what we want to avoid.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants