Skip to content

[DI] Fix dumping Doctrine-like service graphs (bis) #30096

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jul 29, 2019
Merged

[DI] Fix dumping Doctrine-like service graphs (bis) #30096

merged 2 commits into from
Jul 29, 2019

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Feb 6, 2019 •
edited
Loading

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #30091, #29637
License MIT
Doc PR -

Dumping the container while accounting for circular references is hard :)

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Feb 6, 2019
@ro0NL
Copy link
Contributor

ro0NL commented Feb 6, 2019

the required service reference graph is too crazy

well, thank you :}

conceptually it evolves around using PSR4 prototyped + autowired services, where EM service is bound with a private alias pointing to the actual doctrine EM service. Cool huh 😅

bind: https://github.com/msgphp/msgphp/blob/61f5678b2ab764fb94bd912e6eceab5f3546a8b3/src/UserBundle/Resources/config/doctrine.php#L27
load: https://github.com/msgphp/msgphp/blob/61f5678b2ab764fb94bd912e6eceab5f3546a8b3/src/UserBundle/Resources/config/doctrine.php#L36
alias: https://github.com/msgphp/msgphp/blob/61f5678b2ab764fb94bd912e6eceab5f3546a8b3/src/Domain/Infra/DependencyInjection/BundleHelper.php#L138

Im not sure it's related to the scope where services are registered, in my case from an extension config file, but the alias is registered during bundle booting.

Ill try to replicate it in a minimal DI definition 👍 renamed the repo meanwhile.

Confirmed the patch works. Thanks 💯

@nicolas-grekas
Copy link
Member Author

Status: needs work
I'll try to fix #29637 here also, it's related somehow, and not fixed yet.

@ro0NL ro0NL mentioned this pull request Feb 7, 2019
Open
@alcaeus alcaeus mentioned this pull request Apr 6, 2019
Closed
@nicolas-grekas
Copy link
Member Author

🎉
I've been chasing this for more than one year almost day-to-day.
This PR is now green and fixes all the reproducers provided in linked issues + the test cases from @weaverryan (thank you dude!)

@nicolas-grekas nicolas-grekas mentioned this pull request Jul 29, 2019
Closed
@nicolas-grekas
Copy link
Member Author

Thank you @weaverryan.

@nicolas-grekas nicolas-grekas merged commit a37f3e0 into symfony:3.4 Jul 29, 2019
nicolas-grekas added a commit that referenced this pull request Jul 29, 2019
@nicolas-grekas
bug #30096 [DI] Fix dumping Doctrine-like service graphs (bis) (weave…
6763172 
…rryan, nicolas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[DI] Fix dumping Doctrine-like service graphs (bis)

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #30091, #29637
| License       | MIT
| Doc PR        | -

Dumping the container while accounting for circular references is hard :)

Commits
-------

a37f3e0 [DI] Fix dumping Doctrine-like service graphs (bis)
ee49144 Failing test case for complex near-circular situation + lazy
@nicolas-grekas nicolas-grekas deleted the di-fix branch July 29, 2019 17:28
This was referenced Aug 26, 2019
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Bug DependencyInjection Status: Needs Review
Projects
None yet
Milestone
3.4
Development

Successfully merging this pull request may close these issues.
4 participants
@nicolas-grekas @ro0NL @carsonbot @weaverryan