Skip to content

Conversation

@yoeunes
Copy link
Contributor

@yoeunes yoeunes commented Nov 28, 2025

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

Adding a parameter containing placeholders to an already resolved bag resulted in the bag remaining marked as resolved, preventing the new parameter from being resolved.

This PR ensures that ParameterBag marks itself as unresolved when a new parameter is set.

$bag = new ParameterBag(['kernel.project_dir' => '/var/www/app']);
$bag->resolve();

$bag->set('app.uploads_dir', '%kernel.project_dir%/public/uploads');
$bag->resolve();

// before: '%kernel.project_dir%/public/uploads'
// after:  '/var/www/app/public/uploads' 

@yoeunes yoeunes force-pushed the di-parameter-bag-resolved-state branch from dfd97a3 to ad3b789 Compare November 28, 2025 06:24
@nicolas-grekas
Copy link
Member

Thank you @yoeunes.

@nicolas-grekas nicolas-grekas merged commit 808365f into symfony:6.4 Dec 1, 2025
5 of 11 checks passed
nicolas-grekas added a commit to nicolas-grekas/symfony that referenced this pull request Dec 5, 2025
…when setting a parameter (yoeunes)"

This reverts commit 808365f, reversing
changes made to 3678f5c.
@nicolas-grekas
Copy link
Member

Reverted in #62665

nicolas-grekas added a commit that referenced this pull request Dec 5, 2025
…ate when setting a parameter" (nicolas-grekas)

This PR was merged into the 6.4 branch.

Discussion
----------

[DependencyInjection] Revert "bug #62541  Reset resolved state when setting a parameter"

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #62541
| License       | MIT

This reverts commit 808365f, reversing changes made to 3678f5c.

PR #62541 introduced this. But the change makes the CI red on 7.3.
And looking at the code again, changing the "resolved" state means already resolved parameters are going to be resolved twice.
We could eg resolve immediately when setting a new value inside a resolved bag.
I didn't do it since things were working before so I see no hurry to think about how to fix the fix. I prefer reverting for now.
/cc `@yoeunes` FYI

Commits
-------

f4e6e99 Revert "bug #62541 [DependencyInjection] Reset resolved state when setting a parameter (yoeunes)"
nicolas-grekas added a commit that referenced this pull request Dec 5, 2025
* 6.4:
  Revert "bug #62541 [DependencyInjection] Reset resolved state when setting a parameter (yoeunes)"
nicolas-grekas added a commit that referenced this pull request Dec 5, 2025
* 7.3:
  Revert "bug #62541 [DependencyInjection] Reset resolved state when setting a parameter (yoeunes)"
nicolas-grekas added a commit that referenced this pull request Dec 5, 2025
* 7.4:
  Fix merge
  [HttpKernel] Make `#[Cache]` respect all explicit cache directives set in controller
  Revert "bug #62541 [DependencyInjection] Reset resolved state when setting a parameter (yoeunes)"
nicolas-grekas added a commit that referenced this pull request Dec 5, 2025
* 8.0:
  Fix merge
  [HttpKernel] Make `#[Cache]` respect all explicit cache directives set in controller
  Revert "bug #62541 [DependencyInjection] Reset resolved state when setting a parameter (yoeunes)"
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