Skip to content

[HttpKernel] Fix compile error when a legacy container is fresh again #25733

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

Merged
merged 1 commit into from
Jan 10, 2018

Conversation

nicolas-grekas
Copy link
Member

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

Noticed by @jpauli again: when reverting some configuration changes ends up generating the same container as the previously legacy one, the legacy flag should be removed.

@sroze
Copy link
Contributor

sroze commented Jan 9, 2018

You should hire this guy! Oh... wait... 😜

@jpauli
Copy link

jpauli commented Jan 9, 2018

👍

@fabpot
Copy link
Member

fabpot commented Jan 10, 2018

Thank you @nicolas-grekas.

@fabpot fabpot merged commit 8c3eadb into symfony:3.4 Jan 10, 2018
fabpot added a commit that referenced this pull request Jan 10, 2018
…fresh again (nicolas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[HttpKernel] Fix compile error when a legacy container is fresh again

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

Noticed by @jpauli again: when reverting some configuration changes ends up generating the same container as the previously legacy one, the legacy flag should be removed.

Commits
-------

8c3eadb [HttpKernel] Fix compile error when a legacy container is fresh again
This was referenced Jan 29, 2018
@phpwutz
Copy link

phpwutz commented Feb 14, 2018

Thanks for the fix, the bug was really annoying 👍

@nicolas-grekas nicolas-grekas deleted the fix-clear branch February 19, 2018 10:35
nico-incubiq added a commit to nico-incubiq/symfony that referenced this pull request Dec 18, 2018
Since symfony#25733 the Kernel attempts to unlink the legacy container while being built.
This throws an error if the file did not exist, for example on a clean install, on the build, which is then silenced.

That's fine on production systems, but on our build we have enabled "xdebug.scream" in order to visualise every errors, which basically un-silences the errors. I believe there should not be a need to silence anything on a usual, clean usage of the system.
Making this `unlink` conditional fixes it.

Could you please approve and merge this PR?
Thanks
nicolas-grekas pushed a commit to nico-incubiq/symfony that referenced this pull request Dec 19, 2018
Since symfony#25733 the Kernel attempts to unlink the legacy container while being built.
This throws an error if the file did not exist, for example on a clean install, on the build, which is then silenced.

That's fine on production systems, but on our build we have enabled "xdebug.scream" in order to visualise every errors, which basically un-silences the errors. I believe there should not be a need to silence anything on a usual, clean usage of the system.
Making this `unlink` conditional fixes it.

Could you please approve and merge this PR?
Thanks
nicolas-grekas added a commit that referenced this pull request Dec 19, 2018
…biq)

This PR was submitted for the master branch but it was merged into the 3.4 branch instead (closes #29645).

Discussion
----------

Allow running PHPUnit with "xdebug.scream" ON

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets |    <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        |  <!-- required for new features -->

Since #25733 the Kernel attempts to unlink the legacy container while being built.
This throws an error if the file did not exist, for example on a clean install, on the build, which is then silenced.

That's fine on production systems, but on our build we have enabled `xdebug.scream` in order to visualise every errors, which basically un-silences the errors. I believe there should not be a need to silence anything on a usual, clean usage of the system.
Making this `unlink` conditional fixes it.

Could you please approve and merge this PR?
Thanks

Commits
-------

7a7165e Allow running PHPUnit with "xdebug.scream" ON
symfony-splitter pushed a commit to symfony/http-kernel that referenced this pull request Dec 19, 2018
Since symfony/symfony#25733 the Kernel attempts to unlink the legacy container while being built.
This throws an error if the file did not exist, for example on a clean install, on the build, which is then silenced.

That's fine on production systems, but on our build we have enabled "xdebug.scream" in order to visualise every errors, which basically un-silences the errors. I believe there should not be a need to silence anything on a usual, clean usage of the system.
Making this `unlink` conditional fixes it.

Could you please approve and merge this PR?
Thanks
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.

6 participants