pestle icon indicating copy to clipboard operation
pestle copied to clipboard

Certain upgrade scripts cause setup:di:compile to fatal error

Open jantzenw opened this issue 7 years ago • 6 comments

Preconditions

  1. Magento 2.2.0 EE
  2. Pestle 1.4.2

Steps to reproduce

  1. Use Pestle to create an upgrade script containing something similar to the following structure:

$eavSetup = $this->eavSetupFactory->create(['setup' => $setup]);

$eavSetup->addAttribute(
    \Magento\Catalog\Model\Product::ENTITY,
    'test',
    [
        ...
        'source' => \Magento\Eav\Model\Entity\Attribute\Source\Boolean::class,  \Magento\Eav\Model\Entity\Attribute\ScopedAttributeInterface::SCOPE_GLOBAL,
        ...
    ]
);
  1. Run setup:di:compile.

Expected result

DI is compiled.

Actual result

[Exception]
Notice: Undefined property: Magento\Setup\Module\Di\Code\Reader\ClassesScanner::$eavSetupFactory 
in ... upgrade_scripts/data/0.0.5.php

Analysis

At the end of one line, there is the word 'class'. In the following line, there is the word '\Magento'. This triggers the following events:

  1. \Magento\Setup\Module\Di\Code\Reader\FileClassScanner::extract thinks the file contains class Magento,
  2. Causing \Magento\Setup\Module\Di\Code\Reader\ClassesScanner::includeClasses to try to include the file,
  3. Causing the file to be included with $this being \Magento\Setup\Module\Di\Code\Reader\ClassesScanner,
  4. Causing $this->eavSetupFactory to fatal error.

So upgrade scripts containing the following properties will fail:

  1. Contain the word "class" that is parsed as its own token.
  2. Contain another token that can be parsed as a class name.
  3. Call a property or function on $this that Magento\Setup\Module\Di\Code\Reader\ClassesScanner does not contain.

I am not sure if Pestle can fix this. I have filed a bug with Magento here.

jantzenw avatar Oct 13 '17 22:10 jantzenw

makes long painful sigh that contains multitude of things better not said in public

Thanks for the bug report. I don't see a Magento\Setup\Module\Di\Code\Reader\FileClassScanner class in Magento -- did you mean Magento\Setup\Module\Di\Code\Reader\FileScanner? Or another class?

If I'm reading that all correctly it sounds like

  1. The presence of Magento\Eav\Model\Entity\Attribute\Source\Boolean::class, followed (eventually) by Magento makes Magento's class scanner think this is a class source file.
  2. Magento tries including the script in the context of its own class
  3. Explodo

I'm not sure there's much we can do here -- if Magento thinks a class file is not a class file and runs the code inside then bad undefined variable things will happen. I can think of some hack solutions (something like $pestleSetup = $this in the class, replace all references to $this with $pestleSetup, and an if(isset($pestleSetup)) check at the top of the script but -- no.

We might be able to investigate if there's a way to format the include such that Magento identifies it as something other than a class.

The work-around is to use a 'Magento\Eav\Model\Entity\Attribute\Source\Boolean' in your source instead of the more elegant Magento\Eav\Model\Entity\Attribute\Source\Boolean::class

I'll leave this to the billion dollar ecommence company to fix, but am open to pull requests from anyone that can come up with a solution.

astorm avatar Oct 14 '17 02:10 astorm

Maybe an option to produce a script with something like this at the top (and a $setup = $this in the include function)

if(!isset($setup) || !is_object($setup) || !isset($setup->pulseStormUpgradeClass))
{
    return;
}

astorm avatar Oct 17 '17 02:10 astorm

@jantzenw can you report the issue here https://github.com/magento/magento2/issues ? it is a bug in the Magento compiler.

piotrekkaminski avatar Oct 17 '17 13:10 piotrekkaminski

@piotrekkaminski Yes it was reported here: https://github.com/magento/magento2/issues/11442

jantzenw avatar Oct 17 '17 15:10 jantzenw

@astorm Magento\Setup\Module\Di\Code\Reader\FileClassScanner is the correct class. I see that it exists in 2.2.0 EE but does not exist in at least 2.1.7 CE.

jantzenw avatar Oct 17 '17 15:10 jantzenw

@jantzenw Looks like the class is there in M2.2 as well -- I must have been looking at an older version/the wrong folder when I was diagnosing things before. Thank you for clarifying!

https://github.com/magento/magento2/blob/2.2-develop/setup/src/Magento/Setup/Module/Di/Code/Reader/FileClassScanner.php

astorm avatar Oct 18 '17 05:10 astorm