-
Notifications
You must be signed in to change notification settings - Fork 77
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
composer: fixes autoload section #35
base: master
Are you sure you want to change the base?
Conversation
This entry in composer file is necessary if you want to use image generation in PHPPdf library. There is a comment why this entry is necessary: https://github.com/psliwa/PHPPdf/blob/master/lib/Imagine/Image/Box.php#L19. Thanks to this entry the Box class implementation is replaced by implementation provided by PHPPdf library. There is no other way to achieve this without changing Imagine library itself, because all concrete classes are final and there are no extension point to replace Box class implementation. I will try to do a pull request to Imagine library that will add extension point to replace Box implementation and then clean fix in PHPPdf will be possible (because current solution is hack) |
Thanks for the explanations. But IMO you'd better fork the whole library and make it a new composer dependency to avoid side effects of the current hack. |
I think this would be a worse hack that current solution. What when in project you want to use both PHPPdf library and Imagine? When I will make fork and declare dependency in PHPPdf to Imagine fork, you won't be able to use original Imagine library in your project because there will be namespace conflict. PR to Imagine: php-imagine/Imagine#279 |
Yes you're right I've forgotten namespace conflict... Anyway, PR Imagine is the only viable solution. The current hack simply breaks box-dependent Imagine's features. |
Yes, it breaks those features but thanks to this PR this broken Box class will be only used by internals of PHPPdf library. Only two features in Imagine uses |
because it sounds relevant: http://blog.naderman.de/2014/02/17/replace-conflict-forks-explained/ |
This article isn't related to this issue, I don't want to use Imagine fork as dependency because this is nastier than current solution. By the way, this article is good and true is you shouldn't use composer update on production server (because this is critical security issue for several reasons) and after composer update on dev you should check what changed in new versions of vendor libraries. |
This patch removes the definition of namespace "Imagine" (mapped on local lib/) as it is an external library and could corrupt main application's autoloader