doctrine / data-fixtures

Doctrine2 ORM Data Fixtures Extensions
http://www.doctrine-project.org
MIT License
2.77k stars 224 forks source link

This should resolve changes made in the latest release of doctrine2 #212

Closed macnibblet closed 8 years ago

macnibblet commented 8 years ago

doctrine/doctrine2@8ea62b95b8e4a8b3de000a59f018ec250b41d714

stof commented 8 years ago

I prefer fixing the BC break in Doctrine ORM instead, as this should not happen in the first place (it breaks the semver policy): https://github.com/doctrine/doctrine2/pull/1570#discussion_r46291344

guilhermeblanco commented 8 years ago

WTH... This class was never meant to be used by anyone... not even other Doctrine projects. Oh private classes... how much I miss you.

guilhermeblanco commented 8 years ago

My suggestion is to copy the new class and make it part of data-fixtures project. New class performs better than old one and handles cyclic dependencies

guilhermeblanco commented 8 years ago

Let me explain why data-fixtures should not use ORM class at all and might actually copy the class from 2.0 branch https://github.com/doctrine/data-fixtures/blob/2.0/lib/Doctrine/Fixture/Sorter/TopologicalSorter.php

data-fixtures cannot accept cyclic dependencies. Old ORM implementation was loose and accepting cycles, and the implementation was relying which class got added first. Consuming the same implementation, a worse issue exist in data-fixtures project, because of FK constraints check could not be inferred depending of the filesystem order for loading the classes (related to order of add class on ORM), potentially creating an unusable scenario.

Also, data-fixtures cannot be loose on cycles and must notify the user he is making a mistake by breaking the execution. This is what 2.0 implementation does and 1.X version should too. Accepting loose cycles lead to unpredictable execution logic, which may enter in either deadlocks or FK constraint errors.

-1 on reverting on ORM and +1 on bringing the new, reliable execution checker to this version and release as another minor.

petrjirasek commented 8 years ago

Hello, I would like to ask you if there is any progress with this? I have updated my dependencies on doctrine and doctrine fixtures and now there is the error during the purging.

Thanks for answer

jurchiks commented 8 years ago

Hey guys, I'm updating my project from Symfony 2.7 to Symfony 3 and all is going well with the only exception being this error being thrown during doctrine:fixtures:load: Call to undefined method Doctrine\ORM\Internal\CommitOrderCalculator::addClass(). This library is marked as Symfony 3-compatible... But apparently it is not? Can you fix this or should I look for other fixture bundles?

I see people have been reporting this issue for over two months now, so why hasn't this been fixed yet?

Ocramius commented 8 years ago

@jurchiks that is a problem that affects master only. Also, it is to be fixed in fixtures (fixtures should not use a class from the Internal namespace

jurchiks commented 8 years ago

You mean master of this project or DoctrineFixturesBundle? Because I'm using version 2.3.0 of DoctrineFixturesBundle and it uses version 1.0 of this project: https://github.com/doctrine/DoctrineFixturesBundle/blob/2.3.0/composer.json#L26. There is no master anywhere, but I'm getting the same error.

jurchiks commented 8 years ago

Never mind, composer.lock contained a different version than composer.json, all because one dev-dependency had no stable versions and so "minimum-stability: dev" was required in composer.json...

Ocramius commented 8 years ago

We have ORM 2.6 blocked anyway until this is resolved.

The correct solution is the one suggested by @guilhermeblanco: duplicate the code into this package (and test it)

geofrwa commented 8 years ago

Thanks, that does the trick (should have read this better -__-' not to delete my question, that was about how to fix the bug without having actually tested the commit fix)

stevro commented 8 years ago

When could this fix be merged? Thank you.

guilhermeblanco commented 8 years ago

Closing this one in favor of #222