Skip to content

[FrameworkBundle] Fix autoloader in insulated clients #21922

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
Mar 8, 2017

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Mar 8, 2017

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

Related to the deprecation of the class loader component:
the Client already uses this code in HttpKernel, but FrameworkBundle is missing the same update.
Spotted while debugging the hhvm 3.18 issue (the chain is: ComposerResource sees different vendors, thus says the kernel cache is not fresh, thus it is rebuild, thus we hit facebook/hhvm#7722).

}

$path = str_replace("'", "\\'", $r->getFileName());
$requires .= "require_once '".str_replace("'", "\\'", (new \ReflectionObject($this->kernel))->getFileName())."';\n";
Copy link
Member

Choose a reason for hiding this comment

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

wouldn't var_export be better than doing manual escaping ? Thus, the escaping is incomplete in case there is a \ just before a quote

Copy link
Member Author

@nicolas-grekas nicolas-grekas Mar 8, 2017

Choose a reason for hiding this comment

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

Dunno :) Currently, this is consistent with other places (HttpKernel's Client) and with the previous code. I don't see the need to diverge here (in this PR).

fabpot added a commit that referenced this pull request Mar 8, 2017
This PR was merged into the 2.7 branch.

Discussion
----------

[travis] Test with hhvm 3.18

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

Needs #21922 on master to be green also.
Works around  facebook/hhvm#7722.

Commits
-------

7f1f0cb [travis] Test with hhvm 3.18
@fabpot
Copy link
Member

fabpot commented Mar 8, 2017

Thank you @nicolas-grekas.

@fabpot fabpot merged commit 414ac5d into symfony:master Mar 8, 2017
fabpot added a commit that referenced this pull request Mar 8, 2017
…olas-grekas)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[FrameworkBundle] Fix autoloader in insulated clients

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

Related to the deprecation of the class loader component:
the `Client` already uses this code in `HttpKernel`, but `FrameworkBundle` is missing the same update.
Spotted while debugging the hhvm 3.18 issue (the chain is: `ComposerResource` sees different vendors, thus says the kernel cache is not fresh, thus it is rebuild, thus we hit facebook/hhvm#7722).

Commits
-------

414ac5d [FrameworkBundle] Fix autoloader in insulated clients
@nicolas-grekas nicolas-grekas deleted the client-fix branch March 8, 2017 19:48
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