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

[FrameworkBundle] resolve service locators in `debug:*` commands #34755

Merged
merged 1 commit into from Dec 6, 2019

Conversation

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Dec 2, 2019

Q A
Branch? 3.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #34470
License MIT
Doc PR -

Because of the way ServiceClosureArgument are dumped, we need to resolve locators after loading the xml dump of the container:
https://github.com/symfony/symfony/blob/3.4/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php#L273

@chalasr
chalasr approved these changes Dec 2, 2019
@chalasr

This comment has been minimized.

Copy link
Member

chalasr commented Dec 2, 2019

Adding a doc comment or updating the command test case would be nice though :)

@nicolas-grekas

This comment has been minimized.

Copy link
Member Author

nicolas-grekas commented Dec 3, 2019

Adding a doc comment or updating the command test case would be nice though :)

This command has no test case, I'd prefer moving on here, this is a debug tool, nothing critical.

@chalasr

This comment has been minimized.

Copy link
Member

chalasr commented Dec 6, 2019

Thank you @nicolas-grekas.

chalasr added a commit that referenced this pull request Dec 6, 2019
…mmands (nicolas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[FrameworkBundle] resolve service locators in `debug:*` commands

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #34470
| License       | MIT
| Doc PR        | -

Because of the way ServiceClosureArgument are dumped, we need to resolve locators after loading the xml dump of the container:
https://github.com/symfony/symfony/blob/3.4/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php#L273

Commits
-------

820da66 [FrameworkBundle] resolve service locators in `debug:*` commands
@chalasr chalasr merged commit 820da66 into symfony:3.4 Dec 6, 2019
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details
@graftak

This comment has been minimized.

Copy link

graftak commented Dec 13, 2019

I encountered the same issue in Symfony 4.4.1 and tried the same fix as in this commit, which worked as well. Can I help by creating a pull request for this? (This would be my first contribution to Symfony b.t.w.!)

Edit: Oh sorry, I think these fixes are already being merged with later versions, like what was done here for 4.3: 2ac5609
Never mind, I'll try to find another way to contribute :)

@nicolas-grekas nicolas-grekas deleted the nicolas-grekas:fwb-load-locators branch Dec 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.