Skip to content

Commit

Permalink
Added new "try-discovering-cwebp" option for cwebp which tries "which…
Browse files Browse the repository at this point in the history 
… -a cwebp" and "whereis -b cwebp" to discover cwebp binaries. Closes #226
  • Loading branch information
@rosell-dk
rosell-dk committed Oct 3, 2019
1 parent 707ed68 commit d5fca19
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 11 deletions.
13 changes: 11 additions & 2 deletions docs/v2.0/converting/options.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,24 @@ Type: boolean
Default: true
Supported by: cwebp
```
If set, the converter will try to look for cwebp in locations such as `/usr/bin/cwebp`.
If set, the converter will try to look for cwebp in locations such as `/usr/bin/cwebp`. It is a limited list. It might find something that isn't found using `try-discovering-cwebp` if these common paths are not within PATH or neither `which` or `whereis` are available.

### `cwebp-try-cwebp`
```
Type: boolean
Default: true
Supported by: cwebp
```
If set, the converter will try the a pure "cwebp" command (without specifying a path).
If set, the converter will try the a plain "cwebp" command (without specifying a path).

### `try-discovering-cwebp`
```
Type: boolean
Default: true
Supported by: cwebp
```
If set, the converter will try to discover installed cwebp binaries using the `whereis -b cwebp` and `which -a cwebp` commands. These commands will find cwebp binaries residing in PATH. They might find cwebp binaries which are not found by enabling `cwebp-try-common-system-paths`


### `cwebp-try-supplied-binary-for-os`
```
Expand Down
85 changes: 76 additions & 9 deletions src/Convert/Converters/Cwebp.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ protected function createOptions()
new StringOption('command-line-options', ''),
new SensitiveStringOption('rel-path-to-precompiled-binaries', './Binaries'),
new BooleanOption('try-cwebp', true),
new BooleanOption('try-common-system-paths', true),
new BooleanOption('try-common-system-paths', false),
new BooleanOption('try-discovering-cwebp', true),
new BooleanOption('try-supplied-binary-for-os', true)
);
}
Expand Down Expand Up @@ -399,6 +400,65 @@ private function getSuppliedBinaryPathForOS()
return $result;
}

/**
* Discover installed binaries using "whereis -b cwebp"
*
* @return array Array of cwebp paths discovered (possibly empty)
*/
private function discoverBinariesUsingWhereIsCwebp()
{
// This method was added due to #226.
exec('whereis -b cwebp', $output, $returnCode);
if (($returnCode == 0) && (isset($output[0]))) {
$result = $output[0];
// Ie: "cwebp: /usr/bin/cwebp /usr/local/bin/cwebp"
if (preg_match('#^cwebp:\s(.*)$#', $result, $matches)) {
$paths = explode(' ', $matches[1]);
$this->logLn(
'Discovered ' . count($paths) . ' ' .
'installed versions of cwebp using "whereis -b cwebp" command. Result: "' . $result . '"'
);
return $paths;
}
}
return [];
}

/**
* Discover installed binaries using "which -a cwebp"
*
* @return array Array of cwebp paths discovered (possibly empty)
*/
private function discoverBinariesUsingWhichCwebp()
{
// As suggested by @cantoute here:
// https://wordpress.org/support/topic/sh-1-usr-local-bin-cwebp-not-found/
exec('which -a cwebp', $output, $returnCode);
if ($returnCode == 0) {
$paths = $output;
$this->logLn(
'Discovered ' . count($paths) . ' ' .
'installed versions of cwebp using "which -a cwebp" command. Result: "' . implode('", "', $paths) . '"'
);
return $paths;
}
return [];
}

private function discoverBinaries()
{
$paths = $this->discoverBinariesUsingWhereIsCwebp();
if (count($paths) > 0) {
return $paths;
}

$paths = $this->discoverBinariesUsingWhichCwebp();
if (count($paths) > 0) {
return $paths;
}
return [];
}

private function who()
{
exec('whoami', $whoOutput, $whoReturnCode);
Expand Down Expand Up @@ -476,8 +536,6 @@ private function detectVersions($binaries)
*/
private function getCwebpVersions()
{
// TODO: Check out if exec('whereis cwebp'); would be a good idea

if (defined('WEBPCONVERT_CWEBP_PATH')) {
$this->logLn('WEBPCONVERT_CWEBP_PATH was defined, so using that path and ignoring any other');
return $this->detectVersions([constant('WEBPCONVERT_CWEBP_PATH')]);
Expand All @@ -492,20 +550,29 @@ private function getCwebpVersions()
$versions = [];
if ($this->options['try-cwebp']) {
$this->logLn(
'Detecting version of cwebp command (it may not be available, but we try nonetheless)'
'Detecting version of plain cwebp command (it may not be available, but we try nonetheless)'
);
$versions = $this->detectVersions(['cwebp']);
}
if ($this->options['try-discovering-cwebp']) {
$versions = array_replace_recursive($versions, $this->detectVersions($this->discoverBinaries()));
}
if ($this->options['try-common-system-paths']) {
// Brute-force trying common system paths
// Note:
// We used to do a file_exists($binary) check.
// That was not a good idea because it could trigger open_basedir errors. The open_basedir
// restriction does not operate on the exec command. So note to self: Do not do that again.
// We used to do a file_exists($binary) check.
// That was not a good idea because it could trigger open_basedir errors. The open_basedir
// restriction does not operate on the exec command. So note to self: Do not do that again.
// BUT. Could we actually disable those errors? ie:
// WarningLoggerTrait::shutUp = true;
// if (@file_exists(...)) {...}
// WarningLoggerTrait::shutUp = false;

$this->logLn(
'Detecting versions of the cwebp binaries in common system paths ' .
'Brute force trying version detecting cwebp binaries in common system paths ' .
'(some may not be found, that is to be expected)'
);
$versions = array_merge_recursive($versions, $this->detectVersions(self::$cwebpDefaultPaths));
$versions = array_replace_recursive($versions, $this->detectVersions(self::$cwebpDefaultPaths));
}
if ($this->options['try-supplied-binary-for-os']) {
$versions = array_merge_recursive(
Expand Down
23 changes: 23 additions & 0 deletions tests/Convert/Converters/CwebpTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ public function testOperatinalityException()
'try-cwebp' => false,
'try-supplied-binary-for-os' => false,
'try-common-system-paths' => false,
'try-discovering-cwebp' => false,
];
$this->expectException(ConverterNotOperationalException::class);
//$cwebp = new Cwebp($source, $source . '.webp', $options);
Expand All @@ -205,8 +206,30 @@ public function testUsingSuppliedBinaryForOS()
{
$source = self::getImagePath('test.png');
$options = [
'try-cwebp' => false,
'try-supplied-binary-for-os' => true,
'try-common-system-paths' => false,
'try-discovering-cwebp' => false,
];
//$this->expectException(ConverterNotOperationalException::class);
//$cwebp = new Cwebp($source, $source . '.webp', $options);
try {
Cwebp::convert($source, $source . '.webp', $options);
} catch (ConversionFailedException $e) {
// this is ok.
// - but other exceptions are not!
}
$this->addToAssertionCount(1);
}

public function testUsingCommonSystemPaths()
{
$source = self::getImagePath('test.png');
$options = [
'try-cwebp' => false,
'try-supplied-binary-for-os' => false,
'try-common-system-paths' => true,
'try-discovering-cwebp' => false,
];
//$this->expectException(ConverterNotOperationalException::class);
//$cwebp = new Cwebp($source, $source . '.webp', $options);
Expand Down

0 comments on commit d5fca19

Please sign in to comment.