Skip to content

Conversation

@aschempp
Copy link
Contributor

@aschempp aschempp commented Dec 1, 2025

Q A
Branch? 7.4
Bug fix? yes
New feature? no
Deprecations? no
Issues
License MIT

#60597 introduced a BC break in that it assumes the ContainerBuilder constructor accepts the parameter bag as its first argument. However, in our application, we have a custom version of the ContainerBuilder.

Since the container is already built at this state, I understand that calling __construct() again is just a hack to re-set the parameter bag. A direct setter function for this case should fix the issue.

PS: Kernel::buildContainer must always return an instance of ContainerBuilder, so the setter method must always be there (our class inherits from it as well).

Links:

@aschempp
Copy link
Contributor Author

aschempp commented Dec 1, 2025

Failing tests seem unrelated to me

$container->setDefinitions($dumpedContainer->getDefinitions());
$container->setAliases($dumpedContainer->getAliases());
$container->__construct($dumpedContainer->getParameterBag());
$container->setParameterBag($dumpedContainer->getParameterBag());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather not add a new method. Can't we use a foreach instead, copying all parameters one by one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how a foreach would work? Would you use reflections instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't we iterate over the parameters from the bag of the dumped container and call set() on the bag from the empty one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, that could work as well! I'll investigate

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed in 02551f3. This would fail if we have a FrozenParameterBag, but I don't think that is ever the case as long as the container is not compiled.

@nicolas-grekas nicolas-grekas force-pushed the fix/container-constructor branch from 02551f3 to 843fbca Compare December 4, 2025 17:16
@nicolas-grekas
Copy link
Member

Thank you @aschempp.

@nicolas-grekas nicolas-grekas merged commit f0c6c79 into symfony:7.4 Dec 4, 2025
11 of 12 checks passed
This was referenced Dec 7, 2025
@aschempp aschempp deleted the fix/container-constructor branch December 9, 2025 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants