Skip to content

[DependencyInjection] Fix self-referenced 'service_container' service#14445

Closed
hacfi wants to merge 1 commit into
symfony:masterfrom
hacfi:di_service_container
Closed

[DependencyInjection] Fix self-referenced 'service_container' service#14445
hacfi wants to merge 1 commit into
symfony:masterfrom
hacfi:di_service_container

Conversation

@hacfi

@hacfi hacfi commented Apr 22, 2015

Copy link
Copy Markdown
Contributor

..to allow garbage collection.

Q A
Bug fix? yes
New feature? no
BC breaks? yes
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

Follow up of #11422

I created the constant Symfony\Component\DependencyInjection\ContainerInterface::CONTAINER_ID while I was at it and removed an unused use statement in a test. Let me know if I should remove the constant again or create a separate PR.

@hacfi

hacfi commented Apr 22, 2015

Copy link
Copy Markdown
Contributor Author

@stof

stof commented Apr 22, 2015

Copy link
Copy Markdown
Member

it is weird if it is not in newer versions. this looks like a bad conflict resolution.

The proper fix would be to reapply the bugfix in 2.6.

And I don't think the new constant is worth it

@xabbuh

xabbuh commented Apr 22, 2015

Copy link
Copy Markdown
Member

It seems that it's only the constructor that wasn't merged properly. The rest seems to be present (see for example https://github.com/symfony/symfony/blob/2.6/src/Symfony/Component/DependencyInjection/Container.php#L200-205).

@hacfi

hacfi commented Apr 22, 2015

Copy link
Copy Markdown
Contributor Author

@stof This PR is for 3.0 because #11422 had some code in it to keep BC.

Reapplying the bugfix in 2.6 and 2.7 is still required.

@dosten

dosten commented Apr 22, 2015

Copy link
Copy Markdown
Contributor

This is going to be changed in #14155

@stof

stof commented Apr 22, 2015

Copy link
Copy Markdown
Member

The issue was the conflict resolution in 3bed1b7

@hacfi

hacfi commented Apr 22, 2015

Copy link
Copy Markdown
Contributor Author

@xabbuh Exactly..see 3bed1b7#diff-8

@stof

stof commented Apr 22, 2015

Copy link
Copy Markdown
Member

@hacfi reapplying the bugfix must be done first, before doing anything for 3.0 (because the reapplied bugfix will then also reach the master branch)

@hacfi

hacfi commented Apr 22, 2015

Copy link
Copy Markdown
Contributor Author

@dosten My bad..closing then.

@xabbuh

xabbuh commented Apr 22, 2015

Copy link
Copy Markdown
Member

see #14446

nicolas-grekas added a commit that referenced this pull request Apr 27, 2015
This PR was merged into the 2.6 branch.

Discussion
----------

[DependencyInjection] resolve circular reference

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

This issue has been fixed in #11422. Due to a bad merge in 3bed1b7 it partially appeared again in Symfony 2.6 or higher.

Commits
-------

8b3b3ce [DependencyInjection] resolve circular reference
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants