NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.7k

[FrameworkBundle] fix using lock from service id when previous locks used env vars#61939

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

nicolas-grekas merged 1 commit intosymfony:7.3fromAydinHassan:lock-from-service-fix

Oct 6, 2025

Merged

[FrameworkBundle] fix using lock from service id when previous locks used env vars#61939

nicolas-grekas merged 1 commit intosymfony:7.3fromAydinHassan:lock-from-service-fix

Oct 6, 2025

+63 −0

Conversation

Copy link

Contributor

AydinHassan commentedOct 3, 2025•
edited by nicolas-grekas
Loading

Q	A
Branch?	7.3
Bug fix?	yes
New feature?	no
Deprecations?	no
Issues	n/a
License	MIT

When specifying multiple locks including one which references a service id, and a previous one used an env var, the service id is not converted in to a reference. This is becauseusedEnvs is not reset in the loop and is passed by reference. Not sure if this is intentional or not but it looks like a mistake to me. A similar reset is donehere.

fix using lock from service when previous locks used env vars

0cf7a9a

carsonbot added Status: Needs Review Bug FrameworkBundle labels

Oct 3, 2025

carsonbot added this to the7.3 milestone

Oct 3, 2025

nicolas-grekas modified the milestones:7.3,6.4

Oct 3, 2025

nicolas-grekas changed the base branch from7.3 to6.4

October 3, 2025 11:47

nicolas-grekas requested review fromOskarStark,chalasr,dunglas,jderusse,lyrixx,xabbuh andyceruto ascode owners

October 3, 2025 11:47

nicolas-grekas force-pushed thelock-from-service-fix branch 2 times, most recently from44e7ddc tofcfd9a7Compare

October 3, 2025 12:47

Copy link

ContributorAuthor

AydinHassan commentedOct 3, 2025•
edited
Loading

@nicolas-grekas I guess this test doesn't work because that feature was only added in 7.2, would you like me to do something or are you taking care?

AydinHassan mentioned this pull request

Oct 3, 2025

feat: improved app url validationshopware/shopware#12628

Draft

5 tasks

Copy link

Member

nicolas-grekas commentedOct 3, 2025

I'd be happy if you could have a look. To me, the issue exists on 6.4, even if it might manifest itself differently. That's why I rebased on 6.4. Thanks!

Copy link

ContributorAuthor

AydinHassan commentedOct 3, 2025

@nicolas-grekas we could certainly apply the fix to 6.4, but since the result of$usedEnvs is not used anywhere, I don't know how we could test that, effectively there is no change of behaviour with or without the fix in 6.4, at least I can't see.

Only in 7.2 where the new if condition is added can we use my test.