When webkitdirs.pm runs 'which' it redirects its stdout to /dev/null. At least GNU which prints to stderr when a binary is not found, so we should redirect stderr as well. This fixes an annoying warning that prints at the top of the output of build-webkit on distros that rename ninja to ninja-build.
Created attachment 244315 [details] Patch
Comment on attachment 244315 [details] Patch We should take this opportunity to write these functions in terms of webkitdirs::commandExists() instead of duplicating effort. Notice that webkitdirs::commandExists() assumes that a command that exists will emit a non-empty string to stdout when passed --version. We may need to improve the implementation of webkitdirs::commandExists() if it's insufficient to use to test for the existence of ninja, eclipse, and ninja-build.
The existing function works fine for ninja, but in the Java tradition, eclipse does not support --version.
Created attachment 244412 [details] Patch
The Windows support is speculative, based on [1] and [2]. I hope that will be tested by the EWS? [1] http://technet.microsoft.com/en-us/library/cc753148.aspx [2] http://support.microsoft.com/kb/110930
Comment on attachment 244412 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=244412&action=review > Tools/Scripts/webkitdirs.pm:896 > + return $? == 0; $? encodes both the exit status and signal code that caused termination of the child process and I'm unclear whether the signal code would be zero under normal circumstances on Windows. Regardless, we should make use of exitStatus() extract the exit status of the child before comparing to 0. Specifically, we should pass the return value of system() to exitStatus(). > Tools/Scripts/webkitdirs.pm:1774 > + return commandExists('ninja') || commandExists('ninja-build'); Nit: ' (single quote) => " (double quote) > Tools/Scripts/webkitdirs.pm:1779 > + return commandExists('eclipse'); Ditto.
The Windows portion of this patch looks sane to me given the referenced documents in comment 5. CC'ed Brent Fulgham who is familiar with the Windows command line environment.
Created attachment 244414 [details] Patch
Comment on attachment 244414 [details] Patch Clearing flags on attachment: 244414 Committed r178243: <http://trac.webkit.org/changeset/178243>
All reviewed patches have been landed. Closing bug.