magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.58k stars 9.32k forks source link

setup:static-content:deploy, setup:di:compile and deploy:mode:set will not return a non-zero exit code if any error occurs #3060

Closed moleman closed 8 years ago

moleman commented 8 years ago

When you run commands in an build environment, like Jenkins, you want the commands to return a non-zero exit code if an error occurred so that Jenkins can fail the build.

Currently if any error occurs when you run setup:static-content:deploy it will not return a non-zero exit code which makes the build succeed even though the build is incomplete.

Example:

=== frontend -> Vaimo/XXX -> en_US ===
...........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
Compilation from source:
frontend/Vaimo/XXX/en_US/css/email-inline.less
variable @media-common is undefined in file /XXX/en_US/css/source/_theme.less in _theme.less on line 24, column 9
22| //  Global
23| //  _____________________________________________
24| & when (@media-common = true) {
25|     .svg-link {
26|         pointer-events: none;
27|     }>

This will make the generation of static files to stop right in the middle because of an error but it will still not return a non-zero exit code which makes the build incomplete and Jenkins will not know about it.

The same behaviour occurs when you run setup:di:compile and when you switch deploy mode with deploy:mode:set. All of these commands should return a non-zero exit code if any error occurs.

erikhansen commented 8 years ago

+1 for fixing this issue. I just experienced the problem with failures not returning a non-zero return code. I am using Capistrano to deploy to stage/production servers. I run bin/magento setup:static-content:deploy -q as a part of my deployment process. I ran into an issue where there was a LESS compilation error, but since the return code was 0, Capistrano continued with the deployment. This resulted in the site site being updated with a broken frontend since the CSS did not compile.

erikhansen commented 8 years ago

@Magento - I did some digging into this, and it looks like what needs to happen is that the execute methods (that use the Symfony Console library) need to return "1" when there is an error. This isn't documented in the Symfony documentation, but you can see an example of it on this page: http://fideloper.com/laravel-symfony-console-commands-stderr

So if you search for all instances of $output->writeln('<error>, you'll see at least most of the places that you need to update.

For example, the \Magento\Setup\Console\Command\AdminUserCreateCommand::execute method (https://github.com/magento/magento2/blob/develop/setup/src/Magento/Setup/Console/Command/AdminUserCreateCommand.php#L59) should change from this:

if ($errors) {
    $output->writeln('<error>' . implode('</error>' . PHP_EOL .  '<error>', $errors) . '</error>');
    return;
}

to this:

if ($errors) {
    $output->writeln('<error>' . implode('</error>' . PHP_EOL .  '<error>', $errors) . '</error>');
    return 1;
}
fooman commented 8 years ago

Seems there is already a PR for this https://github.com/magento/magento2/pull/3189

erikhansen commented 8 years ago

@fooman Yes, you are correct. Thanks for sharing. Once that PR gets merged, this ticket should be closed.

piotrekkaminski commented 8 years ago

Thank you for your submission.

We recently made some changes to the way we process GitHub submissions to more quickly identify and respond to core code issues.

Feature Requests and Improvements should now be submitted to the new Magento 2 Feature Requests and Improvements forum (see details here).

We are closing this GitHub ticket and have moved your request to the new forum.

davidalger commented 8 years ago

FTR - It appears based on the history of #3189 that this bug was fixed in 2.1.0

magento-team commented 7 years ago

Internal ticket to track issue progress: MAGETWO-67500