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

Make sure PHP executable is being resolved by composer #1701

Merged
merged 1 commit into from
Sep 18, 2018

Conversation

hertzg
Copy link
Contributor

@hertzg hertzg commented Sep 18, 2018

There are situations where php executable is not in path or is not called php in those cases composer update and composer install will break or might use php version which might not be the same as the one running the composer process.

One of the examples can be that on your system you have at least two versions of PHP. One available as php the other as php.7.1 and so on.
If you were to run php7.1 /path/to/composer.phar install in the project directory, the composer will be executed by php7.1 but the artisan scripts will be run under php executable and most likely will fail.

Defining scripts in composer.json which reference php executable using @php token avoids this issue.

/ref https://getcomposer.org/doc/articles/scripts.md#executing-php-scripts
/ref composer/composer@b000061
/ref https://github.com/symfony/process/blob/master/PhpExecutableFinder.php

Changes in this pull request:

  • replaced php artisan calls with @php artisan to ensure correct PHP executable resolution

@JC5

@hertzg hertzg changed the base branch from master to develop September 18, 2018 16:30
@JC5 JC5 merged commit 9bf43ce into firefly-iii:develop Sep 18, 2018
@JC5
Copy link
Member

JC5 commented Sep 18, 2018

Nice work, merged! Don't worry about the build, I think I am the one who broke it ;-).

@hertzg hertzg deleted the fix-composer-php-path-issue branch September 18, 2018 16:40
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants