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

[HttpKernel] removed absolute paths from the generated container #10894

Merged
merged 1 commit into from May 17, 2014

Conversation

Projects
None yet
6 participants
@fabpot
Member

fabpot commented May 12, 2014

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets first step to resolve #6484, #3079, and #9238
License MIT
Doc PR n/a

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.

@stof

This comment has been minimized.

Member

stof commented May 12, 2014

This should clearly be covered by tests

return $content;
}
$cacheDir = $this->getCacheDir();

This comment has been minimized.

@staabm

staabm May 12, 2014

Contributor

Could be moved below the while loop, because not used before

// find the "real" root dir (by finding the composer.json file)
$rootDir = $this->getRootDir();
while (!file_exists($rootDir.'/composer.json')) {
if ('/' === $rootDir = realpath($rootDir.'/..')) {

This comment has been minimized.

@yguedidi

yguedidi May 12, 2014

Contributor

I think this won't work on Windows platform.

This comment has been minimized.

This comment has been minimized.

@yguedidi

yguedidi May 12, 2014

Contributor

But paths on Windows starts by X: no ?

@fabpot fabpot changed the title from [WIP] [HttpKernel] removed absolute paths from the generated container to [HttpKernel] removed absolute paths from the generated container May 17, 2014

@fabpot

This comment has been minimized.

Member

fabpot commented May 17, 2014

This one should be ready now.

$rootDir = $this->getRootDir();
$previous = $rootDir;
while (!file_exists($rootDir.'/composer.json')) {
if ($previous === $rootDir = realpath($rootDir.'/..')) {

This comment has been minimized.

@stof

stof May 17, 2014

Member

shoudln't $previous be updated in the loop too ?

This comment has been minimized.

@fabpot

fabpot May 17, 2014

Member

right, fixed it now and added a test about that.

This comment has been minimized.

@tvlooy

tvlooy May 30, 2014

Contributor

@fabpot I think this stuff (the preg_replace_callback below) breaks the console. kernel.rootpath is now at app/cache/dev/../src can be reproduced by generating a bundle

This comment has been minimized.

@wouterj

wouterj May 30, 2014

Member

@tvlooy this is already reverted see #10943

This comment has been minimized.

@tvlooy

tvlooy May 30, 2014

Contributor

@wouterj thanks for the pointer

@fabpot fabpot merged commit c1450b4 into symfony:2.3 May 17, 2014

2 checks passed

continuous-integration/travis-ci The Travis CI build passed
Details
default Success: Travis, fabbot
Details

fabpot added a commit that referenced this pull request May 17, 2014

bug #10894 [HttpKernel] removed absolute paths from the generated con…
…tainer (fabpot)

This PR was merged into the 2.3 branch.

Discussion
----------

[HttpKernel] removed absolute paths from the generated container

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | first step to resolve #6484, #3079, and #9238
| License       | MIT
| Doc PR        | n/a

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.

Commits
-------

c1450b4 [HttpKernel] removed absolute paths from the generated container

@fabpot fabpot deleted the fabpot:absolute-paths branch May 17, 2014

fabpot added a commit that referenced this pull request May 26, 2014

Revert "bug #10894 [HttpKernel] removed absolute paths from the gener…
…ated container (fabpot)"

This reverts commit 735e9a4, reversing
changes made to 5c91dc1.

fabpot added a commit that referenced this pull request May 26, 2014

Merge branch '2.3' into 2.4
* 2.3:
  Revert "bug #10894 [HttpKernel] removed absolute paths from the generated container (fabpot)"
  Revert "bug #10937 [HttpKernel] Fix "absolute path" when we look to the cache directory (BenoitLeveque)"
  Revert "fixed CS"
  Revert "bug #10979 Make rootPath part of regex greedy (artursvonda)"
  Revert "[HttpKernel] simplified some tests"
  [HttpKernel] simplified some tests
  Make rootPath part of regex greedy

Conflicts:
	src/Symfony/Component/HttpKernel/Tests/KernelTest.php

fabpot added a commit that referenced this pull request May 26, 2014

Merge branch '2.4' into 2.5
* 2.4:
  Revert "bug #10894 [HttpKernel] removed absolute paths from the generated container (fabpot)"
  Revert "bug #10937 [HttpKernel] Fix "absolute path" when we look to the cache directory (BenoitLeveque)"
  Revert "fixed CS"
  Revert "bug #10979 Make rootPath part of regex greedy (artursvonda)"
  Revert "[HttpKernel] simplified some tests"
  [HttpKernel] simplified some tests
  Make rootPath part of regex greedy

fabpot added a commit that referenced this pull request May 26, 2014

Merge branch '2.5'
* 2.5:
  Revert "bug #10894 [HttpKernel] removed absolute paths from the generated container (fabpot)"
  Revert "bug #10937 [HttpKernel] Fix "absolute path" when we look to the cache directory (BenoitLeveque)"
  Revert "fixed CS"
  Revert "bug #10979 Make rootPath part of regex greedy (artursvonda)"
  Revert "[HttpKernel] simplified some tests"
  [HttpKernel] simplified some tests
  Make rootPath part of regex greedy

fabpot added a commit that referenced this pull request Dec 2, 2014

bug #12784 [DependencyInjection] make paths relative to __DIR__ in th…
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment