Skip to content

Add a link script to ease debugging Flex apps #24746

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
Nov 20, 2017

Conversation

dunglas
Copy link
Member

@dunglas dunglas commented Oct 29, 2017

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

(Reopened because of mishandling in the previous PR)

It's painful to debug and patch Flex apps because symfony/symfony isn't installed by default (only components are) but PRs must be opened against the monolithic repository.

This tiny tool, inspired by npm link, scan the vendor/ directory of the project, and replace symfony/ dependencies by symlinks to the local clone of the symfony/symfony repositories.

Usage:

git clone git@github.com:symfony/symfony.git
cd symfony
./link /path/to/the/project

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Oct 29, 2017

Could that be written in PHP, so that Windows user could use it easily also?

Copy link
Contributor

@sroze sroze left a comment

Choose a reason for hiding this comment

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

Definitely 👍. It helped me already!

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Oct 30, 2017
@nicolas-grekas
Copy link
Member

@dunglas ok to rewrite it in PHP?

@dunglas
Copy link
Member Author

dunglas commented Nov 1, 2017

Yes I’ll try.

@dunglas dunglas changed the title Add a link.sh script to ease debugging Flex apps Add a link script to ease debugging Flex apps Nov 4, 2017
@dunglas
Copy link
Member Author

dunglas commented Nov 4, 2017

@nicolas-grekas done.

I don't have a Windows computer. It would be nice that someone test the new version on this OS.

link Outdated
}

$sfPackages = array();
foreach (glob(__DIR__.'/src/Symfony/{Bundle,Bridge,Component}/*', GLOB_BRACE | GLOB_ONLYDIR | GLOB_NOSORT) as $dir) {
Copy link
Member Author

Choose a reason for hiding this comment

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

GLOB_BRACE doesn't work on some systems (Solaris), but I think it doesn't matter for this script.

Copy link
Member

@lyrixx lyrixx left a comment

Choose a reason for hiding this comment

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

May be we should put this binary in a bin/ folder?

link Outdated
if ($argc < 2) {
echo 'Link dependencies to components to a local clone of the monolithic Symfony repository'.PHP_EOL.PHP_EOL;
echo 'Usage: link.php /path/to/the/project'.PHP_EOL;
exit;
Copy link
Member

Choose a reason for hiding this comment

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

exit(1); the usage is wrong.

link Outdated

if ($argc < 2) {
echo 'Link dependencies to components to a local clone of the monolithic Symfony repository'.PHP_EOL.PHP_EOL;
echo 'Usage: link.php /path/to/the/project'.PHP_EOL;
Copy link
Member

Choose a reason for hiding this comment

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

the name of the file is link so Usage: php link /path/

@dunglas
Copy link
Member Author

dunglas commented Nov 9, 2017

@lyrixx I've added this comment to the root directory for consistency with the phpunit script and because it's easier to find for a new contributor. But we can change that.

Copy link
Contributor

@ro0NL ro0NL left a comment

Choose a reason for hiding this comment

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

works like a charm! (ubuntu)

link Outdated
}

if (!is_dir("$argv[1]/vendor/symfony")) {
echo "The directory \"$argv[1]\" does not exist or the dependencies are not installed, did you forget to run \"composer install\" in your project?";
Copy link
Contributor

Choose a reason for hiding this comment

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

missing PHP_EOL?

given The directory "foo" does not exist or the dependencies are not installed, did you forget to run "composer install" in your project?user@docker:/some/path$

link Outdated
if (isset($sfPackages["symfony/$package"])) {
rename($dir, "$dir.back");
symlink($sfDir = $sfPackages["symfony/$package"], $dir);
echo "\"symfony/$package\" has been linked to \"$sfDir\".";
Copy link
Contributor

Choose a reason for hiding this comment

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

same.. missing EOL. Output is terrible ;-)

Copy link
Member Author

Choose a reason for hiding this comment

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

I update (I didn't noticed because I use ZSH 😛).

link Outdated

$package = basename($dir);
if (isset($sfPackages["symfony/$package"])) {
rename($dir, "$dir.back");
Copy link
Contributor

@ro0NL ro0NL Nov 9, 2017

Choose a reason for hiding this comment

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

intended as .bak? https://en.wikipedia.org/wiki/Bak_file

Can/should we have an option to skip backup files, so no double classes are indexed in Phpstorm and such.

i'd prefer a ./link /some/path --backup, but also fine for a --no-backup :)

Copy link
Member

Choose a reason for hiding this comment

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

I don't care about the backup too.
Composer install is really fast when all packages are in cache

Copy link
Contributor

@sroze sroze Nov 9, 2017

Choose a reason for hiding this comment

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

Same, I don't see the point of the backup. I'm always doing a rm -rf vendor/symfony/*.back as it also messes up PhpStorm...

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not for the speed, it's just in case the developper has done some changes in the vendor (common when trying to debug something). Ok for the --no-backup flag.

Copy link
Member

Choose a reason for hiding this comment

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

but this fails when run twice

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh... maybe after adding another dependency...

Copy link
Member Author

@dunglas dunglas Nov 9, 2017

Choose a reason for hiding this comment

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

but this fails when run twice

No because the second time it will be skipped (the symlink will already exists)

Copy link
Member Author

@dunglas dunglas Nov 9, 2017

Choose a reason for hiding this comment

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

Anyway I'm ok to remove the backup feature if nobody except me finds this hazardous :)

Copy link
Member

Choose a reason for hiding this comment

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

drop :)

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@sroze sroze left a comment

Choose a reason for hiding this comment

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

👍

link Outdated
*/

if (2 !== $argc) {
echo 'Link dependencies to components to a local clone of the monolithic Symfony repository.' . PHP_EOL . PHP_EOL;
Copy link
Member

Choose a reason for hiding this comment

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

I would not use "monolithic Symfony repository" as that's not something we are using anywhere else. What about just "the main symfony/symfony Github repository"?

Copy link
Member Author

Choose a reason for hiding this comment

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

updated

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

(even on 3.3 as that's the lowest supported Flex)

@ro0NL
Copy link
Contributor

ro0NL commented Nov 11, 2017

@dunglas is it possible to create relative symlinks? That would be convenient with docker.

@nicolas-grekas
Copy link
Member

@ro0NL technically yes, but not on Windows AFAIK, so there need to be special care here.

@ro0NL
Copy link
Contributor

ro0NL commented Nov 11, 2017

Rigth, but otherwise we could just exec ln -rs ... ... no? That seems to do it :)

@nicolas-grekas
Copy link
Member

"symlink" works just fine on Linux and Windows, no need for "exec" at all.
The only diff is that the target on Windows has to be a full path.

@dunglas
Copy link
Member Author

dunglas commented Nov 11, 2017

How does it work with Docker with relative links? You put symfony/symfony in your project’s volume?

What we can do is using relative links on Linux (for Docker) and absolute links on Windows.

@ro0NL
Copy link
Contributor

ro0NL commented Nov 11, 2017

You put symfony/symfony in your project’s volume?

Yep :) basically i mount my entire ~/dev dir on a generic container. So relative to that all paths remain the same.

What we can do is using relative links on Linux (for Docker) and absolute links on Windows.

Sounds good :) im not sure if generating a rel path is a hassle? hence i suggested ln -rs

continue;
}

if (!isset($sfPackages[$package])) {
Copy link
Member

Choose a reason for hiding this comment

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

maybe add a message here: "Skipping ..., not a symfony/symfony package."?
also, should we make the script work with symfony/symfony? I'm sure it would help standard edition users.
Also wondering: where should we tell ppl that this script exists and when it should be used?

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 for symlinking symfony/symfony as well. I think it fits the contributing section. That's what i'd use it for... cloning my symfony fork (so symlinks pointing here), adding symfony as 2nd upstream remote.

Copy link
Contributor

Choose a reason for hiding this comment

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

👎 for this "Skipping .. not a symfony package", it would be far too verbose and the output would be unreadable IMHO. If you really want to know, as a user, put a echo in the PHP script or something I'd say.

👍 for symfony/symfony tho

Copy link
Member Author

@dunglas dunglas Nov 12, 2017

Choose a reason for hiding this comment

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

You can already rely on composer install --prefer-source for symfony/symfony.

Copy link
Contributor

Choose a reason for hiding this comment

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

given this tool requires you to have a centralized clone somewhere, it's tempting to link it to multiple projects and switch branches on the fly :) i dont know if thats a supported use case, but if so symfony/symfony should be detected IMHO.

Copy link
Member

Choose a reason for hiding this comment

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

You can already rely on composer install --prefer-source for symfony/symfony.

I do as @ro0NL: one repo for dev, never touch directly inside vendor

@GromNaN
Copy link
Member

GromNaN commented Nov 12, 2017

Does the same can be achevied with composer local path repository ? https://getcomposer.org/doc/05-repositories.md#path

@ro0NL
Copy link
Contributor

ro0NL commented Nov 15, 2017

@GromNaN yes, possible :) but i still think this script should work independent from composer.

@derrabus
Copy link
Member

Works like a charm and it really saves the hack day for me. 👍

@ro0NL
Copy link
Contributor

ro0NL commented Nov 18, 2017

well.. no :)

image

linked on docker.. but i see if i can patch now :)

@ro0NL
Copy link
Contributor

ro0NL commented Nov 18, 2017

my use case is

$ pwd
/home/me/dev/symfony-src

$ run-on-docker ./link ../project
$sfDir = '/var/www/html/symfony-src/src/Symfony/Component/Debug';
$dir = '../project/vendor/symfony/debug';

for now i did

-    symlink($sfDir = $sfPackages[$package], $dir);
+    exec('ln -rs '.escapeshellarg($sfDir = $sfPackages[$package]).' '.escapeshellarg($dir));
debug -> ../../../symfony-src/src/Symfony/Component/Debug/

@dunglas
Copy link
Member Author

dunglas commented Nov 18, 2017

@ro0NL why don't you run the link command from inside your container?

@ro0NL
Copy link
Contributor

ro0NL commented Nov 18, 2017

i do that 😉 i wrap docker run CMD

therefor i get /var/www/html/symfony-src/... targets, which should be /home/me/dev/symfony-src/... for the host

@dunglas
Copy link
Member Author

dunglas commented Nov 18, 2017

@ro0NL ln -r is a (recent) GNU option, it doesn't work on Mac and BSD, and it looks like there is no easy alternative for those systems.

@ro0NL
Copy link
Contributor

ro0NL commented Nov 18, 2017

perhaps worth detecting availability... dunno. Or enable explicitly by link -r.

👍 for merging as is also.. i can change this manually now and then.

@ostrolucky
Copy link
Contributor

Please merge with relative links support. We containerize each project we are working on and I believe lot of other people too. It will be very common use case.

@dunglas
Copy link
Member Author

dunglas commented Nov 18, 2017

We don’t have a solution working both on Linux, BSD and Mac right now.

I suggest to merge this PR as is because it’s a pain to debug Flex apps without, then we can try to add relative links support in another PR.

@ro0NL
Copy link
Contributor

ro0NL commented Nov 18, 2017

We could simply include src/Symfony/Component/Filesystem/Filesystem.php. Using ->makePathRelative as well as ->symlink() perhaps.

I can confirm makePathRelative($sfDir, dirname(realpath($dir)) works (before doing rmdir($dir)).

link Outdated

if (2 !== $argc) {
echo 'Link dependencies to components to a local clone of the main symfony/symfony GitHub repository.'.PHP_EOL.PHP_EOL;
echo 'Usage: ./link /path/to/the/project'.PHP_EOL;
Copy link

Choose a reason for hiding this comment

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

maybe echo "Usage: $argv[0] /path/to/the/project".PHP_EOL; should be used?

@dunglas
Copy link
Member Author

dunglas commented Nov 18, 2017

@ro0NL good idea, the same trick was proposed by @lyrixx. It's implemented!

link Outdated
* file that was distributed with this source code.
*/

require 'src/Symfony/Component/Filesystem/Filesystem.php';
Copy link

Choose a reason for hiding this comment

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

maybe require __DIR__.'/src/...' ?

link Outdated
* file that was distributed with this source code.
*/

require __DIR__.'src/Symfony/Component/Filesystem/Filesystem.php';
Copy link

Choose a reason for hiding this comment

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

sorry, but it seems '/' is missing: __DIR__.'/src/

Copy link
Contributor

@ro0NL ro0NL left a comment

Choose a reason for hiding this comment

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

works otherwise 👍

link Outdated
exit(1);
}

$sfPackages = array('symfony' => __DIR__);
Copy link
Contributor

Choose a reason for hiding this comment

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

symfony/symfony

Copy link
Contributor

@ro0NL ro0NL left a comment

Choose a reason for hiding this comment

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

real nice 👍 (any reason to not merge into lowest maintained branch?)

@dunglas
Copy link
Member Author

dunglas commented Nov 19, 2017

@ro0NL, no we'll merge it in the lowest branch directly using gh.

link Outdated
}

$sfPackages = array('symfony/symfony' => __DIR__);
foreach (glob(__DIR__.'/src/Symfony/{Bundle,Bridge,Component}/*', GLOB_BRACE | GLOB_ONLYDIR | GLOB_NOSORT) as $dir) {
Copy link
Member

Choose a reason for hiding this comment

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

missing Security sub-components: src/Symfony/{Bundle,Bridge,Component,Component/Security}/*

@nicolas-grekas
Copy link
Member

👍 for merging this into 2.7, this is a tool for dev

@nicolas-grekas
Copy link
Member

(squashing + rebasing before merging would be great)

@dunglas dunglas changed the base branch from 3.4 to 2.7 November 20, 2017 21:19
@nicolas-grekas
Copy link
Member

Thank you @dunglas.

@nicolas-grekas nicolas-grekas merged commit 381f5d1 into symfony:2.7 Nov 20, 2017
nicolas-grekas added a commit that referenced this pull request Nov 20, 2017
This PR was merged into the 2.7 branch.

Discussion
----------

Add a link script to ease debugging Flex apps

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md files -->
| Tests pass?   | yes
| Fixed tickets | #24708
| License       | MIT
| Doc PR        | n/a

(Reopened because of mishandling in the previous PR)

It's painful to debug and patch Flex apps because `symfony/symfony` isn't installed by default (only components are) but PRs must be opened against the monolithic repository.

This tiny tool, inspired by `npm link`, scan the `vendor/` directory of the project, and replace `symfony/` dependencies by symlinks to the local clone of the `symfony/symfony` repositories.

Usage:

```
git clone git@github.com:symfony/symfony.git
cd symfony
./link /path/to/the/project
```

Commits
-------

381f5d1 Add a "link" script to ease debugging Flex apps
@dunglas dunglas deleted the linker branch November 20, 2017 21:27
weaverryan added a commit to symfony/symfony-docs that referenced this pull request Nov 24, 2017
This PR was merged into the 2.7 branch.

Discussion
----------

Add example usage of the link script

Docs for symfony/symfony#24746.

Commits
-------

6051229 Add example usage of the link script
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.