Merge branch 'v1-develop' into v1

Author: hostep

.gitattributes

@@ -0,0 +1,8 @@
+/.gitattributes    export-ignore
+/.gitignore        export-ignore
+/.php-auto-version export-ignore
+/.php-cs-fixer.php export-ignore
+/composer.lock     export-ignore
+/Makefile          export-ignore
+/phpstan.neon      export-ignore
+/vendor-bin        export-ignore

Css/PreProcessor/Adapter/Less/Processor.php

@@ -16,6 +16,9 @@
 use Magento\Framework\View\Asset\File as AssetFile;
 use Magento\Framework\View\Asset\Source;
 use Psr\Log\LoggerInterface;
+use Symfony\Component\Process\Exception\ProcessFailedException;
+use Symfony\Component\Process\Process;
+use Symfony\Component\Process\ProcessBuilder;
 
 class Processor implements ContentProcessorInterface
 {
@@ -58,17 +61,19 @@ public function processContent(AssetFile $asset)
             $content = (string) $this->assetSource->getContent($asset);
 
             if (trim($content) === '') {
-                return '';
+                throw new ContentProcessorException(
+                    new Phrase('Compilation from source: LESS file is empty: ' . $path)
+                );
             }
 
             $tmpFilePath = $this->temporaryFile->createFile($path, $content);
 
-            $content = $this->compileFile($tmpFilePath);
+            $content = $this->compileFile($tmpFilePath, $path);
 
             if (trim($content) === '') {
-                $this->logger->warning('Parsed less file is empty: ' . $path);
-
-                return '';
+                throw new ContentProcessorException(
+                    new Phrase('Compilation from source: CSS is empty from LESS file: ' . $path)
+                );
             } else {
                 return $content;
             }
@@ -85,37 +90,50 @@ public function processContent(AssetFile $asset)
      * Compiles less file and returns output as a string
      *
      * @param string $filePath
+     * @param string $assetPath
      *
      * @return string
      *
      * @throws NotFoundException if the nodejs or less compiler binaries can't be found
      * @throws LocalizedException if the shell command returns non-zero exit code
      */
-    protected function compileFile($filePath)
+    protected function compileFile($filePath, $assetPath)
     {
         $nodeCmdArgs = $this->getNodeArgsAsArray();
         $lessCmdArgs = $this->getCompilerArgsAsArray();
 
-        $cmd = '%s ' . str_repeat(' %s', count($nodeCmdArgs)) . ' %s' . str_repeat(' %s', count($lessCmdArgs)) . ' %s';
-        $arguments = [];
-        $arguments[] = $this->getPathToNodeBinary();
-        $arguments = array_merge($arguments, $nodeCmdArgs);
-        $arguments[] = $this->getPathToLessCompiler();
-        $arguments = array_merge($arguments, $lessCmdArgs);
-        $arguments[] = $filePath;
-
-        // to log or not to log, that's the question
-        // also, it would be better to use the logger in the Shell class,
-        // since that one will contain the exact correct command, and not this sprintf version
-        // $logArguments = array_map('escapeshellarg', $arguments);
-        // $this->logger->debug('Less compilation command: `'
-        //     . sprintf($cmd, ...$logArguments)
-        //     . '`');
-
-        return $this->shell->execute(
-            $cmd,
-            $arguments
-        );
+        $command = [];
+        $command[] = $this->getPathToNodeBinary();
+        $command = array_merge($command, $nodeCmdArgs);
+        $command[] = $this->getPathToLessCompiler();
+        $command = array_merge($command, $lessCmdArgs);
+        $command[] = $filePath;
+
+        $process = $this->getProcess($command);
+
+        try {
+            $process->mustRun();
+        } catch (ProcessFailedException $ex) {
+            throw new ContentProcessorException(
+                new Phrase('LESS compilation process failed with: %1', [$ex->getMessage()])
+            );
+        }
+
+        $errorOutput = $process->getErrorOutput();
+
+        if ($errorOutput !== '') {
+            $errorMessage = new Phrase('LESS compilation ran into some problems: %1', [$errorOutput]);
+            if ($this->isThrowOnErrorEnabled()) {
+                throw new ContentProcessorException($errorMessage);
+            } else {
+                $this->logger->error($errorMessage, [
+                    'asset' => $assetPath,
+                    'file'  => $filePath,
+                ]);
+            }
+        }
+
+        return $process->getOutput();
     }
 
     /**
@@ -258,4 +276,38 @@ private function getConfigValueFromPath($path)
 
         return null;
     }
+
+    /**
+     * @param string $path
+     *
+     * @return bool
+     */
+    private function getConfigValueFromPathAsBool($path)
+    {
+        return filter_var($this->scopeConfig->getValue($path), FILTER_VALIDATE_BOOLEAN);
+    }
+
+    /**
+     * @param array<string> $commandArgs
+     *
+     * @return Process
+     */
+    private function getProcess(array $commandArgs)
+    {
+        // We can't use Process class in symfony/process 2.x because it takes a string and not an array
+        // therefore we use ProcessBuilder, which exists only in symfony/process 2.x and was removed from 3.x and higher
+        if (class_exists(ProcessBuilder::class)) {
+            return (new ProcessBuilder($commandArgs))->getProcess();
+        }
+
+        return new Process($commandArgs);
+    }
+
+    /**
+     * @return bool
+     */
+    private function isThrowOnErrorEnabled()
+    {
+        return $this->getConfigValueFromPathAsBool('dev/less_js_compiler/throw_on_error');
+    }
 }

README.md

@@ -76,9 +76,24 @@ If you want to override the default arguments, you can do this by modifying your
     ]
 ```
 
-## Debugging less compilation errors
+You can also enable throwing an exception when the less compilation runs into warnings/errors, by default this is disabled and you should find the problems logged in your `var/log/system.log` file.
+But if you want to enable throwing an exception during the compilation process, you can by configuring this in your `app/etc/config.php` or `app/etc/env.php` file:
 
-When your `.less` files have a syntax error or contain something which doesn't allow it to compile properly, please have a look at the `var/log/system.log` file, it will contain the error what causes the problem.
+```php
+    'system' => [
+        'default' => [
+            'dev' => [
+                'less_js_compiler' => [
+                    'throw_on_error' => true,
+                ]
+            ]
+        ]
+    ]
+```
+
+## Investigating less compilation errors
+
+When your `.less` files have a syntax error or contain something which doesn't allow it to compile properly, please have a look at the `var/log/system.log` file, it will contain some output about what caused the problem.
 
 ## Remarks
 

composer.json

@@ -19,7 +19,8 @@
     "require": {
         "php": "~5.5.0 || ~5.6.0 || ~7.0.0 || ~7.1.0 || ~7.2.0 || ~7.3.0 || ~7.4.0 || ~8.1.0 || ~8.2.0 || ~8.3.0",
         "magento/framework": "^100.0.9 || ^101.0.0 || ^102.0.0 || ^103.0.0",
-        "magento/module-developer": "^100.0.5"
+        "magento/module-developer": "^100.0.5",
+        "symfony/process": "^2.8 || ^4.1 || ^5.0 || ^6.0"
     },
     "require-dev": {
         "bamarni/composer-bin-plugin": "^1.8",