Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

setup:di:compile returns exit code 0 if errors are found #7780

Merged

Conversation

pivulic
Copy link
Contributor

@pivulic pivulic commented Dec 13, 2016

The problem

As a follow up to #3060 and #3189, running bin/magento setup:di:compile doesn't always return a non-zero exit code.

Steps to reproduce

  1. Cause an Incorrect dependency error.
  2. Run setup:di:compile
  3. Check exit code echo $?

Expected result

Since there are Errors during compilation, the exit code should not be 0

Actual result - before this PR

Exit code was 0, since we never entered the catch (OperationException $e) in Magento\Setup\Console\Command\DiCompileCommand::execute()

See screenshot: http://www.vaimo.com/snaps/pablo.ivulic/exit-code-0-not-ok.png

Actual result - after this PR

Exit code is now 1.
exit-code-1

@pivulic pivulic changed the title Throw exception if errors are found setup:di:compile returing exit code 0 instead of throwing exception if errors are found Dec 13, 2016
@pivulic pivulic changed the title setup:di:compile returing exit code 0 instead of throwing exception if errors are found setup:di:compile returns exit code 0 if errors are found Dec 13, 2016
@@ -70,6 +70,7 @@ public function write(array $data)

if ($errorsCount) {
$this->console->writeln('<error>' . 'Total Errors Count: ' . $errorsCount . '</error>');
throw new \Magento\Framework\Validator\Exception(__('Error durring compilation'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log writer is incorrect place to throw this exception: nothing went wrong in log writer. Please fix it on command level.

@vrann
Copy link
Contributor

vrann commented Mar 6, 2017

@pivulic Thank you for contribution. I see there are outstanding comments from the previous reviewers, is there any chance you can fix them?

@pivulic
Copy link
Contributor Author

pivulic commented Mar 7, 2017

I'm not quite sure where to exactly fix it. If you know where to fix it on the command level, could maybe you do it?

@vrann
Copy link
Contributor

vrann commented Mar 14, 2017

@pivulic the comment meant to say to move the exception one level up, to the code which invokes error log writer.

The best place as I see would be in the \Magento\Setup\Module\Di\Code\Reader\Decorator\Interceptions::getList, just before $this->log->report();, you can add the code to throw the exception for anything in the $_errorEntries

@vrann vrann self-assigned this Mar 14, 2017
@pivulic
Copy link
Contributor Author

pivulic commented Mar 20, 2017

@vrann, do you mean in https://github.com/magento/magento2/blob/2.1.5/setup/src/Magento/Setup/Module/Di/Code/Reader/Decorator/Interceptions.php#L85? I can't find the $_errorEntries variable there.

Did you mean something like this:

public function getList($path)
{
    ...
    ...
    ...

    if ($_errorEntries) {
        throw new \Magento\Framework\Validator\Exception(__('Error during compilation'));
    }

    $this->log->report();

    return $nameList;
}

@vrann
Copy link
Contributor

vrann commented Mar 23, 2017

@pivulic sorry taking for the long time to respond. Yes, I mean this:

    public function report()
    {
        $this->_successWriter->write($this->_successEntries);
        $this->_errorWriter->write($this->_errorEntries);
        if (count($this->_errorEntries) > 0) {
              throw new \Magento\Framework\Validator\Exception(__('Error durring compilation'));
        } 
    }

@vrann
Copy link
Contributor

vrann commented Apr 12, 2017

@pivulic please let me know if you need any help with this

@pivulic
Copy link
Contributor Author

pivulic commented Apr 13, 2017

@vrann, does it look better now?

@vrann
Copy link
Contributor

vrann commented Apr 14, 2017

@pivulic looks good

@vrann vrann self-requested a review April 14, 2017 21:31
@vrann vrann added this to the April 2017 milestone Apr 14, 2017
@pivulic
Copy link
Contributor Author

pivulic commented Apr 18, 2017

Can we merge it? :)

@okorshenko okorshenko modified the milestones: April 2017, May 2017 May 9, 2017
@okorshenko okorshenko modified the milestones: May 2017, June 2017 Jun 1, 2017
@okorshenko okorshenko assigned okorshenko and unassigned vrann Jun 9, 2017
@magento-team magento-team merged commit 658a30a into magento:develop Jun 12, 2017
magento-team pushed a commit that referenced this pull request Jun 12, 2017
AntonEvers pushed a commit to AntonEvers/magento2 that referenced this pull request Jun 14, 2017
…-9924

* upstream/develop: (60 commits)
  Fix typo in comment
  Move prefix and suffix default values to a new PR
  MAGETWO-68877: Issue magento#7988 Typo changed also added comments for each index, getters and setters. magento#9484
  Revert "MAGETWO-69728: Fixes layered navigation options being cached using the wrong store id. magento#9873"
  MAGETWO-67500: setup:di:compile returns exit code 0 if errors are found magento#7780
  Fix prefix, middle name and suffix were not prefilled in the checkout
  add middle name to checkout address html templates magento#8878
  Using Command output as message which actually provides more information for debugging than just the path
  Handling CLI error as a failure when validating composer.json file
  MAGETWO-69805: Return array of blocks as items instead of array of arrays magento#9157
  MAGETWO-69666: Return array of pages as items instead of array of arrays magento#9823
  MAGETWO-69723: Email to a Friend feature magento#9824
  MAGETWO-69539: PHP "soap" extension is not declared in composer.json but can be used by Magento modules
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-63054: [Catalog] MSRP field is not displayed for bundle products with fixed price
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  ...
AntonEvers pushed a commit to AntonEvers/magento2 that referenced this pull request Jun 14, 2017
* develop: (60 commits)
  Fix typo in comment
  Move prefix and suffix default values to a new PR
  MAGETWO-68877: Issue magento#7988 Typo changed also added comments for each index, getters and setters. magento#9484
  Revert "MAGETWO-69728: Fixes layered navigation options being cached using the wrong store id. magento#9873"
  MAGETWO-67500: setup:di:compile returns exit code 0 if errors are found magento#7780
  Fix prefix, middle name and suffix were not prefilled in the checkout
  add middle name to checkout address html templates magento#8878
  Using Command output as message which actually provides more information for debugging than just the path
  Handling CLI error as a failure when validating composer.json file
  MAGETWO-69805: Return array of blocks as items instead of array of arrays magento#9157
  MAGETWO-69666: Return array of pages as items instead of array of arrays magento#9823
  MAGETWO-69723: Email to a Friend feature magento#9824
  MAGETWO-69539: PHP "soap" extension is not declared in composer.json but can be used by Magento modules
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-63054: [Catalog] MSRP field is not displayed for bundle products with fixed price
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  ...
magento-devops-reposync-svc pushed a commit that referenced this pull request Jul 25, 2022
…end-oauth

[Pyrrans] AC-5928: Replace Zend_Oauth with Laminas\Oauth
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants