support nette/robot-loader@3.0

[adaamz]

This change is 

[janatjak]

There are two problems with robot loader 3.0 BC breaks

RobotLoader::setCacheStorage() is removed

Doctrine/src/Kdyby/Doctrine/Tools/RobotLoader.php

Line 36 in 98e4eff

$this->setCacheStorage($cacheStorage);

RobotLoader::$autoRebuild is private

Doctrine/src/Kdyby/Doctrine/Tools/RobotLoader.php

Line 39 in 98e4eff

$this->autoRebuild = TRUE;

[adaamz]

yes i know, currently fixing it :-)

[adaamz]

@janatjak i fixed those bc breaks. do you know why it is failing now?

[janatjak]

@CzechBoy Check travis...

btw IMHO using "$path/temp" directory is not good idea, because:

• there can be existing directory
• script does not have write access to this folder (same cases)

One question: Is your code compatible with robot-loader <= 3.0? New BC break?

[adaamz]

@janatjak Yes i know that it is not good idea for many reasons, but idk how to solve it in right way.
Upgrading will come from all libraries. Currently only phpstan requires robotloader>=3.0.2

And my app itself does not use robotloader.

[janatjak]

V ramci urychleni prejdu do cestiny.
Dneska jsem resil presne tehle problem a docasne to vyresil takto:

composer.json

"nette/robot-loader": "3.0.2 as 2.4"

Nova class NewAnnotationDriver, ktera jen prohazuje parametry konstruktoru

class NewAnnotationDriver extends Doctrine\ORM\Mapping\Driver\AnnotationDriver
{
    public function __construct(array $paths, Reader $reader)
    {
        parent::__construct($reader, $paths);
    }
}

Ve vlastnim AppExtension prepisuji OrmExtension::$metadataDriverClasses pole, je ale potreba ho umistit v configu pred OrmExtension.

...
public function loadConfiguration()
{
    $ormExtension = $this->compiler->getExtensions(OrmExtension::class)['doctrine'];
    $ormExtension->metadataDriverClasses[OrmExtension::ANNOTATION_DRIVER] = NewAnnotationDriver::class;
}

[adaamz]

Ano, jak píšeš dočasně. Chtěl bych problém vyřešit a nezaseknout celej balík jenom na jedný jednosouborový závislosti.
Ono by to šlo pořešit tak, že by se prostě vložil ten RobotLoader.php do balíčku celý z verze 2.4 (stejně už je vytvořený a používá se ten poděděný).

[enumag]

I'm not quite sure why is RobotLoader even used in the AnnotationDriver. Do you guys know? I'd prefer to remove the RobotLoader dependency completely.

[Majkl578]

https://github.com/Kdyby/Doctrine/blob/master/src/Kdyby/Doctrine/Mapping/AnnotationDriver.php#L95

[enumag]

@Majkl578 Yeah I found that... But I don't understand the purpose. I think it's some autoloading for proxies but why is such a thing necessary?

[fprochazka]

@enumag I'm not sure entirely, but IMHO RobotLoader was simply the easiest way to get all classes in the given path.

[enumag]

If I'm reading this correctly then this will create a temp dir in every directory configured for searching doctrine entities, right? The problem is that such directory can be set to read-only in production which is why I can't merge this PR.

[JanGalek]

so, we need get classes in path and remove dependency of nette/robot-loader ? We can try use Composer ClassLoader maybe ?

[Majkl578]

You can't use Composer's classmap, it may not exist. But you can use similar approach Composer uses to scan directory for classes: https://github.com/composer/composer/blob/master/src/Composer/Autoload/ClassMapGenerator.php

[enumag]

@Majkl578 Still why is some custom autoloading for proxies needed? I mean even Doctrine's native implementation would load them when needed, right?

[JanGalek]

@enumag ok you mean We should create Kdyby/Doctrine classLoader ?

[enumag]

@JanGalek I mean that I'd like to know why Doctrine's native implementation for proxies is not enough.

[JanGalek]

@enumag ok, I added log when it calls, and it calls with orm:info, I tried comments our loading and output is same. This is call in MappingDescribeCommand too

EDIT: orm:mapping:describe tested, and everything is same with and withou Kdyby AnnotationDriver method getAllClassNames

[enumag]

@JanGalek Can you check if this test passes with the native implementation?

[JanGalek]

@enumag I run test, fail:

actual

array(
	'KdybyTests\\Doctrine\\AnnotationDriver\\App\\Bar',
	'KdybyTests\\Doctrine\\AnnotationDriver\\App\\FooEntity',
	'KdybyTests\\Doctrine\\AnnotationDriver\\Something\\Baz',
)

expected

array(
	'KdybyTests\\Doctrine\\AnnotationDriver\\App\\FooEntity',
	'KdybyTests\\Doctrine\\AnnotationDriver\\Something\\Baz',
)

So I think that native not apply filter *Entity.php, maybe should call parent and return filtered ?

[JanGalek]

@enumag We need filter like path/to/file/*Entity.php ? , Native implementation find all classes with annotation @ORM\Entity.

[enumag]

@JanGalek Well apparently that's the reason for this. Whether we need it or not, personally I'd say no - it's a magic and if anyone needs it they can implement it outside of Kdyby/Doctrine. I'd like to simplify things in this package which in this case means to remove both the custom AnnotationDriver and robot-loader dependency.

@fprochazka @Majkl578 Your opinions?

[fprochazka]

I'd consider dropping the custom AnnotationDriver. It solved a very rare edge-case. I don't think anyone depends on it.

[enumag]

Good. Now I'm not talking to anyone in particular but if someone could send a PR with removal of AnnotationDriver and robot-loader it would be great. Please target the newly created branch v3.3.

[enumag]

Closed by #311.