-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[2.1] Fixed "assets:install" to create relative instead of absolute symlinks #1173
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
[2.1] Fixed "assets:install" to create relative instead of absolute symlinks #1173
Conversation
@@ -74,14 +74,16 @@ EOT | |||
|
|||
foreach ($this->container->get('kernel')->getBundles() as $bundle) { | |||
if (is_dir($originDir = $bundle->getPath().'/Resources/public')) { | |||
$targetDir = $input->getArgument('target').'/bundles/'.preg_replace('/bundle$/', '', strtolower($bundle->getName())); | |||
$bundlesDir = $input->getArgument('target').'/bundles/'; | |||
$targetDir = $bundlesDir.preg_replace('/bundle$/', '', strtolower($bundle->getName())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why changing this ? You add a variable used only on the next line which is useless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line #85 uses it as well.
I added it so the function call to makePathRelative
wouldn't require duplicate concatenation: realpath($input->getArgument('target').'/bundles/')
Any other thoughts/updates on this? |
We have such a feature in symfony1 and IIRC it does not work very well. One problem is when you use symlink for some bundles. Then, you should not use a relative symlink as there is a common path between the two. |
Sorry, I didn't think that we would be an issue since the Bundle "Best Practices" states to not include other bundles as dependencies. If absolute links are a must, then the next alternative is for collaborators to add "/web/bundles" to .gitignore and each person run "assets:install" upon installation/update. I was personally hoping there were a way to have this versioned for easier deployment. On Jun 5, 2011, at 12:31 AM, fabpotreply@reply.github.com wrote:
|
<?php
// ...
->addOption('relative', null, InputOption::VALUE_NONE, 'The --symlink option will generate relative paths')
// ... and just default to absolute paths ? |
I'm very supportive of that compromise. Up to @fabpot if I should add this back in, since relative paths were apparently problematic with symfony1. |
+1 I'm developing on Mac and the files are mounted on a Linux box which serves the project. The paths are not the same on those two systems. If I accidentally install assets on my Mac the absolute paths won't work on the Linux webserver. Other scenario: one teammate could add those symlinks by accident to the git repository, which breaks all other installations. Relative symlinks could help a lot in these cases. |
The handling (calculation) of relative symlinks IMO fits better to the symfony/src/Symfony/Component/HttpKernel/Util/Filesystem.php: <?php
// ...
/**
* Creates a symbolic link or copy a directory.
*
* @param string $originDir The origin directory path
* @param string $targetDir The symbolic link name
* @param Boolean $copyOnWindows Whether to copy files if on Windows
* @param Boolean $makeRelative Whether to try to create a relative link
*/
public function symlink($originDir, $targetDir, $copyOnWindows = false, $makeRelative = false)
{ And what about changing the symfony/src/Symfony/Bundle/FrameworkBundle/Command/AssetsInstallCommand.php: <?php
// ...
class AssetsInstallCommand extends ContainerAwareCommand
{
/**
* @see Command
*/
protected function configure()
{
$this
->setDefinition(array(
new InputArgument('target', InputArgument::REQUIRED, 'The target directory (usually "web")'),
))
->addOption('symlink', null, InputOption::VALUE_OPTIONAL, 'Symlinks the assets instead of copying it. Allowed values: "absolute" (default) and "relative".', 'absolute')
->setHelp(<<<EOT
The <info>assets:install</info> command installs bundle assets into a given
directory (e.g. the web directory).
<info>./app/console assets:install web [--symlink]</info>
A "bundles" directory will be created inside the target directory, and the
"Resources/public" directory of each bundle will be copied into it.
To create a symlink to each bundle instead of copying its assets, use the
<info>--symlink</info> option. Use <info>--symlink=relative</info> for relative symlinks.
EOT
)
->setName('assets:install')
;
}
/**
* @see Command
*
* @throws \InvalidArgumentException When the target directory does not exist
*/
protected function execute(InputInterface $input, OutputInterface $output)
{
if (!is_dir($input->getArgument('target'))) {
throw new \InvalidArgumentException(sprintf('The target directory "%s" does not exist.', $input->getArgument('target')));
}
if ($input->hasOption('symlink'))
{
if (!function_exists('symlink')) {
throw new \InvalidArgumentException('The symlink() function is not available on your system. You need to install the assets without the --symlink option.');
}
if (!in_array($input->getOption('symlink'), array('absolute', 'relative'))) {
throw new \InvalidArgumentException(sprintf('Invalid value "%s" for option "symlink"', $input->getOption('symlink')));
}
}
$filesystem = $this->getContainer()->get('filesystem');
// Create the bundles directory otherwise symlink will fail.
$filesystem->mkdir($input->getArgument('target').'/bundles/', 0777);
foreach ($this->getContainer()->get('kernel')->getBundles() as $bundle) {
$originDir = $bundle->getPath().'/Resources/public';
if (is_dir($originDir)) {
$targetDir = $input->getArgument('target').'/bundles/'.preg_replace('/bundle$/', '', strtolower($bundle->getName()));
$output->writeln(sprintf('Installing assets for <comment>%s</comment> into <comment>%s</comment>', $bundle->getNamespace(), $targetDir));
$filesystem->remove($targetDir);
if ($input->hasOption('symlink')) {
$filesystem->symlink($originDir, $targetDir, false, $input->getOption('symlink') == 'relative');
} else {
$filesystem->mkdir($targetDir, 0777);
$filesystem->mirror($originDir, $targetDir);
}
}
}
} |
@ericclemmons: yes, that's our current workaround. I started with manually converting absolute links to relative ones, but that quickly got very annoying ;-) After that I tried to implement the generation of relative links by myself (where the proposals of my previous comment come from) until I found your PR. |
@sbush if it defaults to something how would you turn it off ? |
@henrikbjorn a default value for the option is used when using |
In fact no. The default value seems to be also used when the option is not set at all. @fabpot is this intended ? |
Symlinks on windows, although technically possible, don't quite work with PHP on most setups. Also git doesn't seem to support them either on windows (not sure why not). For those reasons, and although I'm sure this doesn't apply to every project, I would recommend you just have everyone run |
Nobody is even entertaining --relative? On Jul 19, 2011, at 7:18 AM, Seldaekreply@reply.github.com wrote:
|
I agree with the idea of proposing a --relative option, I'm currently working on a Samba mounted filesystem and I'm forced to create manually symlinks to get things working since the paths are not the same |
Commits ------- dd20f01 Fixed assets:install to use a relative path instead of an absolute Discussion ---------- [2.1] Fixed "assets:install" to create relative instead of absolute symlinks This is a fairly simple fix so that the symlinks are relative to the resources rather than an absolute path that breaks from machine-to-machine or upon deployment. We were trying to figure out why styles were messed up for other contributors, but then found that the paths were hard-coded for my machine :) --------------------------------------------------------------------------- by ericclemmons at 2011/06/04 09:44:11 -0700 Any other thoughts/updates on this? --------------------------------------------------------------------------- by fabpot at 2011/06/04 22:31:39 -0700 We have such a feature in symfony1 and IIRC it does not work very well. One problem is when you use symlink for some bundles. Then, you should not use a relative symlink as there is a common path between the two. --------------------------------------------------------------------------- by ericclemmons at 2011/06/05 09:55:00 -0700 Sorry, I didn't think that we would be an issue since the Bundle "Best Practices" states to not include other bundles as dependencies. If absolute links are a must, then the next alternative is for collaborators to add "/web/bundles" to .gitignore and each person run "assets:install" upon installation/update. I was personally hoping there were a way to have this versioned for easier deployment. On Jun 5, 2011, at 12:31 AM, fabpot<reply@reply.github.com> wrote: > We have such a feature in symfony1 and IIRC it does not work very well. One problem is when you use symlink for some bundles. Then, you should not use a relative symlink as there is a common path between the two. > > -- > Reply to this email directly or view it on GitHub: > #1173 (comment) --------------------------------------------------------------------------- by henrikbjorn at 2011/06/27 04:56:58 -0700 ``` php <?php // ... ->addOption('relative', null, InputOption::VALUE_NONE, 'The --symlink option will generate relative paths') // ... ``` and just default to absolute paths ? --------------------------------------------------------------------------- by ericclemmons at 2011/06/27 08:37:50 -0700 I'm very supportive of that compromise. Up to @fabpot if I should add this back in, since relative paths were apparently problematic with symfony1. --------------------------------------------------------------------------- by sbusch at 2011/07/15 08:46:01 -0700 +1 I'm developing on Mac and the files are mounted on a Linux box which serves the project. The paths are not the same on those two systems. If I accidentally install assets on my Mac the absolute paths won't work on the Linux webserver. Other scenario: one teammate could add those symlinks by accident to the git repository, which breaks all other installations. Relative symlinks could help a lot in these cases. --------------------------------------------------------------------------- by ericclemmons at 2011/07/15 08:47:53 -0700 @sbusch Your issues are the same as mine, which prompted this ticket :) Until this gets @fabpot's blessing, it's best to simply add `web/bundles` to your `.gitignore` file and tell your users to always run `assets:install --symlink` each time they pull down code & something breaks ;) --------------------------------------------------------------------------- by sbusch at 2011/07/15 08:58:33 -0700 The handling (calculation) of relative symlinks IMO fits better to the `symlink()` method of `\Symfony\Component\HttpKernel\Util\Filesystem`. Possible method signature: symfony/src/Symfony/Component/HttpKernel/Util/Filesystem.php: ```php <?php // ... /** * Creates a symbolic link or copy a directory. * * @param string $originDir The origin directory path * @param string $targetDir The symbolic link name * @param Boolean $copyOnWindows Whether to copy files if on Windows * @param Boolean $makeRelative Whether to try to create a relative link */ public function symlink($originDir, $targetDir, $copyOnWindows = false, $makeRelative = false) { ``` And what about changing the `--symlink` option to optionally have a value, instead of adding a new depending option? E.g. `--symlink[=absolute|relative]`, with "absolute" as default: symfony/src/Symfony/Bundle/FrameworkBundle/Command/AssetsInstallCommand.php: ```php <?php // ... class AssetsInstallCommand extends ContainerAwareCommand { /** * @see Command */ protected function configure() { $this ->setDefinition(array( new InputArgument('target', InputArgument::REQUIRED, 'The target directory (usually "web")'), )) ->addOption('symlink', null, InputOption::VALUE_OPTIONAL, 'Symlinks the assets instead of copying it. Allowed values: "absolute" (default) and "relative".', 'absolute') ->setHelp(<<<EOT The <info>assets:install</info> command installs bundle assets into a given directory (e.g. the web directory). <info>./app/console assets:install web [--symlink]</info> A "bundles" directory will be created inside the target directory, and the "Resources/public" directory of each bundle will be copied into it. To create a symlink to each bundle instead of copying its assets, use the <info>--symlink</info> option. Use <info>--symlink=relative</info> for relative symlinks. EOT ) ->setName('assets:install') ; } /** * @see Command * * @throws \InvalidArgumentException When the target directory does not exist */ protected function execute(InputInterface $input, OutputInterface $output) { if (!is_dir($input->getArgument('target'))) { throw new \InvalidArgumentException(sprintf('The target directory "%s" does not exist.', $input->getArgument('target'))); } if ($input->hasOption('symlink')) { if (!function_exists('symlink')) { throw new \InvalidArgumentException('The symlink() function is not available on your system. You need to install the assets without the --symlink option.'); } if (!in_array($input->getOption('symlink'), array('absolute', 'relative'))) { throw new \InvalidArgumentException(sprintf('Invalid value "%s" for option "symlink"', $input->getOption('symlink'))); } } $filesystem = $this->getContainer()->get('filesystem'); // Create the bundles directory otherwise symlink will fail. $filesystem->mkdir($input->getArgument('target').'/bundles/', 0777); foreach ($this->getContainer()->get('kernel')->getBundles() as $bundle) { $originDir = $bundle->getPath().'/Resources/public'; if (is_dir($originDir)) { $targetDir = $input->getArgument('target').'/bundles/'.preg_replace('/bundle$/', '', strtolower($bundle->getName())); $output->writeln(sprintf('Installing assets for <comment>%s</comment> into <comment>%s</comment>', $bundle->getNamespace(), $targetDir)); $filesystem->remove($targetDir); if ($input->hasOption('symlink')) { $filesystem->symlink($originDir, $targetDir, false, $input->getOption('symlink') == 'relative'); } else { $filesystem->mkdir($targetDir, 0777); $filesystem->mirror($originDir, $targetDir); } } } } ``` --------------------------------------------------------------------------- by sbusch at 2011/07/15 09:04:46 -0700 @ericclemmons: yes, that's our current workaround. I started with manually converting absolute links to relative ones, but that quickly got very annoying ;-) After that I tried to implement the generation of relative links by myself (where the proposals of my previous comment come from) until I found your PR. --------------------------------------------------------------------------- by henrikbjorn at 2011/07/18 00:20:38 -0700 @sbush if it defaults to something how would you turn it off ? --------------------------------------------------------------------------- by stof at 2011/07/18 00:26:16 -0700 @henrikbjorn a default value for the option is used when using ``--symlink`` without the value. If you don't use the option at all, it is disabled. --------------------------------------------------------------------------- by stof at 2011/07/18 11:58:29 -0700 In fact no. The default value seems to be also used when the option is not set at all. @fabpot is this intended ? --------------------------------------------------------------------------- by Seldaek at 2011/07/19 05:18:29 -0700 Symlinks on windows, although technically possible, don't quite work with PHP on most setups. Also git doesn't seem to support them either on windows (not sure why not). For those reasons, and although I'm sure this doesn't apply to every project, I would recommend you just have everyone run `assets:install [--symlink]` on their local machine, and make that command run on the server as part of your deployment process. --------------------------------------------------------------------------- by ericclemmons at 2011/07/19 06:15:34 -0700 Nobody is even entertaining --relative? On Jul 19, 2011, at 7:18 AM, Seldaek<reply@reply.github.com> wrote: > Symlinks on windows, although technically possible, don't quite work with PHP on most setups. Also git doesn't seem to support them either on windows (not sure why not). For those reasons, and although I'm sure this doesn't apply to every project, I would recommend you just have everyone run `assets:install [--symlink]` on their local machine, and make that command run on the server as part of your deployment process. > > -- > Reply to this email directly or view it on GitHub: > #1173 (comment) --------------------------------------------------------------------------- by Gregwar at 2011/08/10 08:56:27 -0700 I agree with the idea of proposing a --relative option, I'm currently working on a Samba mounted filesystem and I'm forced to create manually symlinks to get things working since the paths are not the same
Commits ------- dd20f01 Fixed assets:install to use a relative path instead of an absolute Discussion ---------- [2.1] Fixed "assets:install" to create relative instead of absolute symlinks This is a fairly simple fix so that the symlinks are relative to the resources rather than an absolute path that breaks from machine-to-machine or upon deployment. We were trying to figure out why styles were messed up for other contributors, but then found that the paths were hard-coded for my machine :) --------------------------------------------------------------------------- by ericclemmons at 2011/06/04 09:44:11 -0700 Any other thoughts/updates on this? --------------------------------------------------------------------------- by fabpot at 2011/06/04 22:31:39 -0700 We have such a feature in symfony1 and IIRC it does not work very well. One problem is when you use symlink for some bundles. Then, you should not use a relative symlink as there is a common path between the two. --------------------------------------------------------------------------- by ericclemmons at 2011/06/05 09:55:00 -0700 Sorry, I didn't think that we would be an issue since the Bundle "Best Practices" states to not include other bundles as dependencies. If absolute links are a must, then the next alternative is for collaborators to add "/web/bundles" to .gitignore and each person run "assets:install" upon installation/update. I was personally hoping there were a way to have this versioned for easier deployment. On Jun 5, 2011, at 12:31 AM, fabpot<reply@reply.github.com> wrote: > We have such a feature in symfony1 and IIRC it does not work very well. One problem is when you use symlink for some bundles. Then, you should not use a relative symlink as there is a common path between the two. > > -- > Reply to this email directly or view it on GitHub: > symfony/symfony#1173 (comment) --------------------------------------------------------------------------- by henrikbjorn at 2011/06/27 04:56:58 -0700 ``` php <?php // ... ->addOption('relative', null, InputOption::VALUE_NONE, 'The --symlink option will generate relative paths') // ... ``` and just default to absolute paths ? --------------------------------------------------------------------------- by ericclemmons at 2011/06/27 08:37:50 -0700 I'm very supportive of that compromise. Up to @fabpot if I should add this back in, since relative paths were apparently problematic with symfony1. --------------------------------------------------------------------------- by sbusch at 2011/07/15 08:46:01 -0700 +1 I'm developing on Mac and the files are mounted on a Linux box which serves the project. The paths are not the same on those two systems. If I accidentally install assets on my Mac the absolute paths won't work on the Linux webserver. Other scenario: one teammate could add those symlinks by accident to the git repository, which breaks all other installations. Relative symlinks could help a lot in these cases. --------------------------------------------------------------------------- by ericclemmons at 2011/07/15 08:47:53 -0700 @sbusch Your issues are the same as mine, which prompted this ticket :) Until this gets @fabpot's blessing, it's best to simply add `web/bundles` to your `.gitignore` file and tell your users to always run `assets:install --symlink` each time they pull down code & something breaks ;) --------------------------------------------------------------------------- by sbusch at 2011/07/15 08:58:33 -0700 The handling (calculation) of relative symlinks IMO fits better to the `symlink()` method of `\Symfony\Component\HttpKernel\Util\Filesystem`. Possible method signature: symfony/src/Symfony/Component/HttpKernel/Util/Filesystem.php: ```php <?php // ... /** * Creates a symbolic link or copy a directory. * * @param string $originDir The origin directory path * @param string $targetDir The symbolic link name * @param Boolean $copyOnWindows Whether to copy files if on Windows * @param Boolean $makeRelative Whether to try to create a relative link */ public function symlink($originDir, $targetDir, $copyOnWindows = false, $makeRelative = false) { ``` And what about changing the `--symlink` option to optionally have a value, instead of adding a new depending option? E.g. `--symlink[=absolute|relative]`, with "absolute" as default: symfony/src/Symfony/Bundle/FrameworkBundle/Command/AssetsInstallCommand.php: ```php <?php // ... class AssetsInstallCommand extends ContainerAwareCommand { /** * @see Command */ protected function configure() { $this ->setDefinition(array( new InputArgument('target', InputArgument::REQUIRED, 'The target directory (usually "web")'), )) ->addOption('symlink', null, InputOption::VALUE_OPTIONAL, 'Symlinks the assets instead of copying it. Allowed values: "absolute" (default) and "relative".', 'absolute') ->setHelp(<<<EOT The <info>assets:install</info> command installs bundle assets into a given directory (e.g. the web directory). <info>./app/console assets:install web [--symlink]</info> A "bundles" directory will be created inside the target directory, and the "Resources/public" directory of each bundle will be copied into it. To create a symlink to each bundle instead of copying its assets, use the <info>--symlink</info> option. Use <info>--symlink=relative</info> for relative symlinks. EOT ) ->setName('assets:install') ; } /** * @see Command * * @throws \InvalidArgumentException When the target directory does not exist */ protected function execute(InputInterface $input, OutputInterface $output) { if (!is_dir($input->getArgument('target'))) { throw new \InvalidArgumentException(sprintf('The target directory "%s" does not exist.', $input->getArgument('target'))); } if ($input->hasOption('symlink')) { if (!function_exists('symlink')) { throw new \InvalidArgumentException('The symlink() function is not available on your system. You need to install the assets without the --symlink option.'); } if (!in_array($input->getOption('symlink'), array('absolute', 'relative'))) { throw new \InvalidArgumentException(sprintf('Invalid value "%s" for option "symlink"', $input->getOption('symlink'))); } } $filesystem = $this->getContainer()->get('filesystem'); // Create the bundles directory otherwise symlink will fail. $filesystem->mkdir($input->getArgument('target').'/bundles/', 0777); foreach ($this->getContainer()->get('kernel')->getBundles() as $bundle) { $originDir = $bundle->getPath().'/Resources/public'; if (is_dir($originDir)) { $targetDir = $input->getArgument('target').'/bundles/'.preg_replace('/bundle$/', '', strtolower($bundle->getName())); $output->writeln(sprintf('Installing assets for <comment>%s</comment> into <comment>%s</comment>', $bundle->getNamespace(), $targetDir)); $filesystem->remove($targetDir); if ($input->hasOption('symlink')) { $filesystem->symlink($originDir, $targetDir, false, $input->getOption('symlink') == 'relative'); } else { $filesystem->mkdir($targetDir, 0777); $filesystem->mirror($originDir, $targetDir); } } } } ``` --------------------------------------------------------------------------- by sbusch at 2011/07/15 09:04:46 -0700 @ericclemmons: yes, that's our current workaround. I started with manually converting absolute links to relative ones, but that quickly got very annoying ;-) After that I tried to implement the generation of relative links by myself (where the proposals of my previous comment come from) until I found your PR. --------------------------------------------------------------------------- by henrikbjorn at 2011/07/18 00:20:38 -0700 @sbush if it defaults to something how would you turn it off ? --------------------------------------------------------------------------- by stof at 2011/07/18 00:26:16 -0700 @henrikbjorn a default value for the option is used when using ``--symlink`` without the value. If you don't use the option at all, it is disabled. --------------------------------------------------------------------------- by stof at 2011/07/18 11:58:29 -0700 In fact no. The default value seems to be also used when the option is not set at all. @fabpot is this intended ? --------------------------------------------------------------------------- by Seldaek at 2011/07/19 05:18:29 -0700 Symlinks on windows, although technically possible, don't quite work with PHP on most setups. Also git doesn't seem to support them either on windows (not sure why not). For those reasons, and although I'm sure this doesn't apply to every project, I would recommend you just have everyone run `assets:install [--symlink]` on their local machine, and make that command run on the server as part of your deployment process. --------------------------------------------------------------------------- by ericclemmons at 2011/07/19 06:15:34 -0700 Nobody is even entertaining --relative? On Jul 19, 2011, at 7:18 AM, Seldaek<reply@reply.github.com> wrote: > Symlinks on windows, although technically possible, don't quite work with PHP on most setups. Also git doesn't seem to support them either on windows (not sure why not). For those reasons, and although I'm sure this doesn't apply to every project, I would recommend you just have everyone run `assets:install [--symlink]` on their local machine, and make that command run on the server as part of your deployment process. > > -- > Reply to this email directly or view it on GitHub: > symfony/symfony#1173 (comment) --------------------------------------------------------------------------- by Gregwar at 2011/08/10 08:56:27 -0700 I agree with the idea of proposing a --relative option, I'm currently working on a Samba mounted filesystem and I'm forced to create manually symlinks to get things working since the paths are not the same
This is a fairly simple fix so that the symlinks are relative to the resources rather than an absolute path that breaks from machine-to-machine or upon deployment.
We were trying to figure out why styles were messed up for other contributors, but then found that the paths were hard-coded for my machine :)