[HttpKernel] removed absolute paths from the generated container #10999
Conversation
We should get __DIR__ instead of __DIR__ . '/.'
|
@fabpot A long time ago I solved the problem of absolute paths by defining a root dir of the application by a define inside of front controller. In the symfony's case the root dir would be the dir where composer.json resides. |
|
@mvrhov That won't work so well when you're running console commands in the standard dist |
|
For the console command it could be setup inside command file. |
|
The logic available in Composer might help to solve the issue of |
|
Practice of putting the cache folder outside of the project root directory is not uncommon. I think we should consider such scenarios (see #11011 for example). |
|
Hello, The bug seems to be corrected with Windows but not with Mac. I have this message with the config.php file: And when I try to create a bundle with console: Does anyone have the solution? |
|
Any chance to roll a release soon with the revert, @fabpot? We're seeing a lot of users hit it with 2.4.5. |
|
We ran into the same problem on 2.4.5. Now 2.4.6 has been released and the revert is included ;) |
|
Also the Heroku infrastructure seems to be affected by this issue, heroku/heroku-buildpack-php#64 |
|
+1 on removing absolute paths from the files in the cache folder and make everything relative using If you want to build deb / rpm packages or similar, the absolute paths force you to run Ok, you can have a build server matching the same folders structure but this is a hacky solution Having said that, agreed that there are many players and infrastructures to take into account so 2.6 or even 2.7 should be the target for that |
|
Looking at this issue again, I think there is no easy fix. Using I cannot see any way to make it reliable in all cases, so I propose to make this an opt-in feature. You would opt-in by defining a constant (something like We can add this easily in Symfony Standard Edition (in // using realpath here is required
define('SYMFONY_ROOT_DIR', realpath(__DIR__.'/...')); |
|
A refinement of the above could be to not require the constant when the cache dir is indeed under the root_dir. |
|
@fabpot why do we need the constant in most cases ? We have The real pain point is actually being able to identify all paths in the container needing to be rewritten, because not all the code is inside the kernel root dir. Or were you suggesting to have And rather than a constant, couldn't we use a method in the Kernel class ? This way, it would not require any change for existing projects (considering that the root of the project is |
|
The logic is here as well, right? |
|
Has this been resolved, as still as issue on Heroku deployment? |
|
Two steps that help mitigate this:
|
|
Thanks. I have step 1 already. Step 2 removed that error but now I get no routes are found, always 404 on Heroku, locally & amazon work fine. |
|
@eddiejaoude can you please test #12784 and see if it helps? |
|
closing in favor of #12784 |
|
👍 |
1 similar comment
|
👍 |
…e generated container (nicolas-grekas) This PR was merged into the 2.3 branch. Discussion ---------- [DependencyInjection] make paths relative to __DIR__ in the generated container | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #6484, #3079, partially #9238, #10894, #10999 | License | MIT | Doc PR | n/a This is an alternative approach to #10999 for removing absolute paths from the generated container: instead of trying to fix the container file after it has been dumped, telling to the PhpDumper where its output will be written allows it to replace parts of strings by an equivalent value based on `__DIR__`. This should be safe, thus the PR is on 2.3. Commits ------- edd7057 [DependencyInjection] make paths relative to __DIR__ in the generated container
This code was merged to 2.3 in (#10894, #10937, and #10979) but it caused too many problems and so was reverted. This PR targets master (aka 2.6) to get plenty of time to figure out the best way to implement this feature without too many BC breaks.
Some issues that should be taken care of before re-merging this into master: #10976, #10943, symfony/symfony-standard#659.
The biggest problem is the usage of
dirnamewhich is naive per the PHP documentation: "dirname() operates naively on the input string, and is not aware of the actual filesystem, or path components such as "..".")". So, basically, as we have now..in the root and cache dirs, using dirname on them is not possible anymore. One solution would be to usedirnamecalls instead of adding..specifically for the root and cache dir, but other paths could be used a problem as well. A better fix would be to check usage of alldirnamecalls and be sure that we operate on a safe path or callrealpathwhen needed.From the original PR:
This PR converts absolute paths to relative ones in the dumped container. The code is a bit "ugly", but it gets the job done and I'm not sure that there is a more elegant way without breaking everything.