[PhpUnitBridge] Declare relative path for phpunit-bridge repository #40879
Conversation
|
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
e850602 to
f0a95e6
Compare
…t-bridge repository
f0a95e6 to
b1a83c6
Compare
|
Hey! I think @gennadigennadigennadi has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
| @@ -180,4 +180,5 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c | |||
| * feature #38596 [BrowserKit] Add jsonRequest function to the browser-kit client (alexander-schranz) | |||
| * feature #38998 [Messenger][SQS] Make sure one can enable debug logs (Nyholm) | |||
| * feature #38974 [Intl] deprecate polyfills in favor of symfony/polyfill-intl-icu (nicolas-grekas) | |||
| * feature #37099 [PhpUnitBridge] Declare relative path for phpunit-bridge repository (toilal) | |||
There was a problem hiding this comment.
should be removed (will be added later while releasing)
| $from = is_dir($from) ? rtrim($from, '\/').'/' : $from; | ||
| $to = is_dir($to) ? rtrim($to, '\/').'/' : $to; | ||
| $from = str_replace('\\', '/', $from); | ||
| $to = str_replace('\\', '/', $to); |
There was a problem hiding this comment.
all \ above should be replaced by \DIRECTORY_SEPARATOR
| $passthruOrFail("$COMPOSER require --no-update symfony/phpunit-bridge \"*@dev\""); | ||
| $passthruOrFail("$COMPOSER config repositories.phpunit-bridge path ".escapeshellarg(str_replace('/', \DIRECTORY_SEPARATOR, $path))); | ||
| $passthruOrFail("$COMPOSER config repositories.phpunit-bridge path ".escapeshellarg(str_replace('/', \DIRECTORY_SEPARATOR, $relativePath))); |
There was a problem hiding this comment.
we don't need the str_replace anymore, do we?
|
@Toilal Do you have time to take comments into accounts? |
|
Closing in favor of #43799, thanks for raising the issue. |
…its path relative (nicolas-grekas) This PR was merged into the 4.4 branch. Discussion ---------- [PhpUnitBridge] fix symlink to bridge in docker by making its path relative | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #40879 | License | MIT | Doc PR | - This is a simpler and less generic version of #40879 that should fit the goal: > When running symfony from a docker container with a volume mapped to the host, it may cause issues when symlinks are generated from container root, as absolute links. > For composer to generate a relative symlink, declaration of the path repository should be relative. Commits ------- 3672ee2 [PhpUnitBridge] fix symlink to bridge in docker by making its path relative
I'm not sure it's a bugfix or a minor feature, please tell me if I need to rebase this on 4.4 branch.
When running symfony from a docker container with a volume mapped to the host, it may cause issues when symlinks are generated from container root, as absolute links.
For composer to generate a relative symlink, declaration of the path repository should be relative. Anyway, it seems better to configure this repository with relative, i.e. to avoid breaking the link when moving the project directory around the filesystem.
This change compute the relative path before invoking composer to configure phpunit-bridge repository.