Laravel-Backpack / basset

Better asset helpers for Laravel apps.
MIT License
151 stars 10 forks source link

Update BassetInstall to use provided PHP process #112

Closed o15a3d4l11s2 closed 4 months ago

o15a3d4l11s2 commented 4 months ago

Story

While deploying a new app on a shared hosting (has PHP 7.4 in CLI, PHP 8.1 inside the served app), I noticed an issue - the command from "post-install-cmd": ["php artisan storage:link --quiet"] fails, complaining that Composer expects PHP 8.1, but PHP is version 7.4. The command I execute is /opt/cpanel/php81/root/usr/bin/php composer install which explicitly uses PHP 8.1 for executing the composer install. But it is ignored by php artisan storage:link --quiet,which uses the default PHP 7.4 instead of the one I explicitly specified.

Proposed fix

Prefix the command with @.

Proof of similar usage:

  1. The composer.json that comes with Laravel uses @php instead of php.
  2. The official composer documentation about executing PHP scripts

Warning

This is currently not an issue and with composer commands provided in an array format this will not be an issue, but worth mentioning:

One limitation of this is that you can not call multiple commands in a row like @php install && @php foo. You must split them up in a JSON array of commands.

karandatwani92 commented 4 months ago

Hey @o15a3d4l11s2

Thanks for reporting the issue and submitting a PR. I appreciate your efforts. I have assigned my colleague @pxpm to review the PR & merge.

Thanks🤝

pxpm commented 4 months ago

Thanks @o15a3d4l11s2 🙏

Hope you are doing well man. Thanks for the contribution and for an excellent explanation of the issue as always! 👍

Merged & tagged!

Cheers