phpstan / extension-installer

Composer plugin for automatic installation of PHPStan extensions.
MIT License
426 stars 27 forks source link

App doesn't work outside Docker if plugin is installed inside it and vice versa #5

Closed grongor closed 5 years ago

grongor commented 5 years ago

During plugin installation, there are some absolute paths being stored inside configuration/autoloader. This causes errors if you install a plugin inside Docker but then later want to use your app outside Docker.

Or if you rename the project folder, or your home folder, whatever. I think you shouldn't be using absolute paths at all - they all should be relative to the project root.

ondrejmirtes commented 5 years ago

I think that things like Docker and global installation are exactly the argument why you want to use absolute paths. It's hard to determine what actually project root is.

lookyman commented 5 years ago

We could maybe use currentWorkingDirectory and call it a day..

ondrejmirtes commented 5 years ago

That would be also really non-deterministic and WTF in some cases. I think that users of non-conventioná installation ways like Docker need to be more careful about what they’re doing, and always run the tools in the same context. This issue is the equivalent of “I moved this file to another computer and it doesn’t work there”, although we’re talking about virtual ones...

On Fri, 24 May 2019 at 09:57, Lukáš Unger notifications@github.com wrote:

We could maybe use currentWorkingDirectory and call it a day..

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/phpstan/extension-installer/issues/5?email_source=notifications&email_token=AAAZTOHSW6J3LLUYL7IRRETPW6NYDA5CNFSM4HO64BKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWEOSPA#issuecomment-495511868, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAZTOGPHAX3IETVEKHJRZLPW6NYDANCNFSM4HO64BKA .

--

Ondřej Mirtes

ondrejmirtes commented 5 years ago

You can always run composer install which fixes this issue.

On Fri, 24 May 2019 at 10:00, Ondřej Mirtes ondrej@mirtes.cz wrote:

That would be also really non-deterministic and WTF in some cases. I think that users of non-conventioná installation ways like Docker need to be more careful about what they’re doing, and always run the tools in the same context. This issue is the equivalent of “I moved this file to another computer and it doesn’t work there”, although we’re talking about virtual ones...

On Fri, 24 May 2019 at 09:57, Lukáš Unger notifications@github.com wrote:

We could maybe use currentWorkingDirectory and call it a day..

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/phpstan/extension-installer/issues/5?email_source=notifications&email_token=AAAZTOHSW6J3LLUYL7IRRETPW6NYDA5CNFSM4HO64BKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWEOSPA#issuecomment-495511868, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAZTOGPHAX3IETVEKHJRZLPW6NYDANCNFSM4HO64BKA .

--

Ondřej Mirtes

--

Ondřej Mirtes

lookyman commented 5 years ago

Yea, I agree with you, that was just a suggestion what could be done if we decided to do something..

lookyman commented 5 years ago

Also, this whole thing is optional and pretty experimental still. If it starts to cause problems, just don't use it. No functionality is lost, just convenience.

ondrejmirtes commented 5 years ago

Yes, exactly why I want to keep it simple 😊

On Fri, 24 May 2019 at 10:24, Lukáš Unger notifications@github.com wrote:

Also, this whole thing is optional and pretty experimental still. If it starts to cause problems, just don't use it. No functionality is lost, just convenience.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/phpstan/extension-installer/issues/5?email_source=notifications&email_token=AAAZTOAHPESBWO7O3NY4SXTPW6Q3BA5CNFSM4HO64BKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWEQQ5Q#issuecomment-495519862, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAZTOF25LQRYS3D7UIN6NDPW6Q3BANCNFSM4HO64BKA .

--

Ondřej Mirtes

grongor commented 5 years ago

Another example where this fails is a deploy process.

You build an app on the build machine and then deploy code to a completely different machine, where the path to the application is probably different than on the build machine. And now your application is broken.

I strongly disagree that tools should persist absolute paths during build time (if the paths are relative to the project).

ondrejmirtes commented 5 years ago

You shouldn’t deploy dev dependencies into production.

On Fri, 24 May 2019 at 12:43, Jakub Chábek notifications@github.com wrote:

Another example where this fails is a deploy process.

You build an app on the build machine and then deploy code to a completely different machine, where the path to the application is probably different than on the build machine. And now your application is broken.

I strongly disagree that tools should persist absolute paths during build time (if the paths are relative to the project).

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/phpstan/extension-installer/issues/5?email_source=notifications&email_token=AAAZTOEKDN6HRMDF5OJWKGDPW7BGBA5CNFSM4HO64BKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWE4RSA#issuecomment-495569096, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAZTOCHC6HLXILYC6IWXQDPW7BGBANCNFSM4HO64BKA .

--

Ondřej Mirtes

grongor commented 5 years ago

Yeah, I didn't think this one through ... we don't do that :joy:

Anyway, do as you think is best...I just won't be able to use this feature if you use absolute paths :(

ondrejmirtes commented 5 years ago

This causes errors if you install a plugin inside Docker but then later want to use your app outside Docker.

This is just a weird use case. Your build environment should match the runtime environment. You'd have the same problems for example with compiling a DI container that bakes in absolute paths - I think Nette does it. You should both install and run PHPStan in Docker or outside Docker.

ovidals commented 4 years ago

I have the same problem. I use captain-hook to execute phpstan and It is fired on git precommit hook from my host, that does not have access to the path it was concatenated during the vendor installation inside docker.

ondrejmirtes commented 3 years ago

This is solved as of extension-installer 1.1.0 and phpstan/phpstan 0.12.62. https://github.com/phpstan/extension-installer/issues/17#issuecomment-744005407

github-actions[bot] commented 3 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.