Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
#25187 Lookup php binary in PHP_BINDIR first#25188
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| /** | ||
| * Replaces default suffixes of executable. | ||
| * | ||
| * @param array $suffixes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This can be removed as all the information are already part of the method signature.
nicolas-grekas commentedNov 29, 2017
Since this change is for a bugfix-only branch, I think the patch should be reduced to the strict minimum. |
36fb2b2 to4e8f028Compare4e8f028 to11ccd92Compare| } | ||
| $name ='php'; | ||
| foreach (array('','.exe','.bat','.cmd','.com')as$suffix) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
reading this, it feels like this logic belongs to the "executableFinder", isn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes, i had changed the executable finder component first to allow looking for executables ONLY in given direcories (currently it is only able to use the given directories as fallback if the executable is not found in the environment path), but you said that it should be reduced to the strict minimum. So the question is: Where should we out this functionality for the bugfix? In long term it would make sense to extend the executable finder with this functionality.
nicolas-grekas commentedFeb 4, 2018
Closing in favor of#26040, thank you@Deltachaos |
Refactored
Symfony\Component\Process\ExecutableFinder. ReusingSymfony\Component\Process\ExecutableFinder::findIninSymfony\Component\Process\PhpExecutableFinder::findtofix#25187.