bug #451 Lock copied files to prevent removing files needed by other …

…recipes (dbrumann)

This PR was merged into the 1.2-dev branch.

Discussion
----------

Lock copied files to prevent removing files needed by other recipes

This PR modifies the `CopyFromRecipeConfigurator` so that it will store a list of copied files in symfony.lock and when a recipe is about to be unconfigured, it will check the list of locked files to prevent removal. This should fix #428

## How to reproduce the issue

```
composer create-project symfony/skeleton example
cd example
composer remove console
```

This will remove both files installed through the recipe `bin/console` and `config/bootstrap.php`. As `symfony/framework-bundle` relies on the bootstrap-file as well, when trying to access the website, you will get an error due to the missing file.

## How to verify the fix

Create a new project based on `symfony/skeleton` and replace flex inside the example project with a local copy of this branch:

```
composer create-project symfony/skeleton example
cd example
rm -rf vendor/symfony/flex
ln -s path/to/local/flex vendor/symfony/flex
```

Since the `symfony.lock` was created during setup of the project it will not yet keep track of the copied files and needs to be updated first. An easy way to do this, is to remove the current file and run `composer fix-recipes` to create a new one:

```
rm symfony.lock
composer fix-recipes
```

Alternatively removing a recipe and then requiring it again will update the lock as well, but it needs to be done for each recipe, as otherwise the list of locked files would be incomplete.

Now when we remove the console again, opening the page in the browser should still work and the file `config/bootstrap.php` should still exist as it is locked by the framework-bundle.

## BC Breaks

None, as far as I can tell. For newly required recipes the files will be tracked. The new key for files in `symfony.lock` was not previously used and should not interfere with existing behavior.

## Known limitations

Unfortunately if any of the previously installed recipes has conflicting files, this will not be recognized unless the recipe is removed and then installed again.

Commits
-------

896e709 Adds file locking for CopyFromRecipe.

src/Configurator.php

@@ -44,22 +44,22 @@ public function __construct(Composer $composer, IOInterface $io, Options $option
         ];
     }
 
-    public function install(Recipe $recipe, array $options = [])
+    public function install(Recipe $recipe, Lock $lock, array $options = [])
     {
         $manifest = $recipe->getManifest();
         foreach (array_keys($this->configurators) as $key) {
             if (isset($manifest[$key])) {
-                $this->get($key)->configure($recipe, $manifest[$key], $options);
+                $this->get($key)->configure($recipe, $manifest[$key], $lock, $options);
             }
         }
     }
 
-    public function unconfigure(Recipe $recipe)
+    public function unconfigure(Recipe $recipe, Lock $lock)
     {
         $manifest = $recipe->getManifest();
         foreach (array_keys($this->configurators) as $key) {
             if (isset($manifest[$key])) {
-                $this->get($key)->unconfigure($recipe, $manifest[$key]);
+                $this->get($key)->unconfigure($recipe, $manifest[$key], $lock);
             }
         }
     }

src/Configurator/AbstractConfigurator.php

@@ -13,6 +13,7 @@
 
 use Composer\Composer;
 use Composer\IO\IOInterface;
+use Symfony\Flex\Lock;
 use Symfony\Flex\Options;
 use Symfony\Flex\Path;
 use Symfony\Flex\Recipe;
@@ -35,9 +36,9 @@ public function __construct(Composer $composer, IOInterface $io, Options $option
         $this->path = new Path($options->get('root-dir'));
     }
 
-    abstract public function configure(Recipe $recipe, $config, array $options = []);
+    abstract public function configure(Recipe $recipe, $config, Lock $lock, array $options = []);
 
-    abstract public function unconfigure(Recipe $recipe, $config);
+    abstract public function unconfigure(Recipe $recipe, $config, Lock $lock);
 
     protected function write($messages)
     {

src/Configurator/BundlesConfigurator.php

@@ -11,14 +11,15 @@
 
 namespace Symfony\Flex\Configurator;
 
+use Symfony\Flex\Lock;
 use Symfony\Flex\Recipe;
 
 /**
  * @author Fabien Potencier <[email protected]>
  */
 class BundlesConfigurator extends AbstractConfigurator
 {
-    public function configure(Recipe $recipe, $bundles, array $options = [])
+    public function configure(Recipe $recipe, $bundles, Lock $lock, array $options = [])
     {
         $this->write('Enabling the package as a Symfony bundle');
         $file = $this->getConfFile();
@@ -38,7 +39,7 @@ public function configure(Recipe $recipe, $bundles, array $options = [])
         $this->dump($file, $registered);
     }
 
-    public function unconfigure(Recipe $recipe, $bundles)
+    public function unconfigure(Recipe $recipe, $bundles, Lock $lock)
     {
         $this->write('Disabling the Symfony bundle');
         $file = $this->getConfFile();

src/Configurator/ComposerScriptsConfigurator.php

@@ -14,14 +14,15 @@
 use Composer\Factory;
 use Composer\Json\JsonFile;
 use Composer\Json\JsonManipulator;
+use Symfony\Flex\Lock;
 use Symfony\Flex\Recipe;
 
 /**
  * @author Fabien Potencier <[email protected]>
  */
 class ComposerScriptsConfigurator extends AbstractConfigurator
 {
-    public function configure(Recipe $recipe, $scripts, array $options = [])
+    public function configure(Recipe $recipe, $scripts, Lock $lock, array $options = [])
     {
         $json = new JsonFile(Factory::getComposerFile());
 
@@ -35,7 +36,7 @@ public function configure(Recipe $recipe, $scripts, array $options = [])
         file_put_contents($json->getPath(), $manipulator->getContents());
     }
 
-    public function unconfigure(Recipe $recipe, $scripts)
+    public function unconfigure(Recipe $recipe, $scripts, Lock $lock)
     {
         $json = new JsonFile(Factory::getComposerFile());
 

src/Configurator/ContainerConfigurator.php

@@ -11,20 +11,21 @@
 
 namespace Symfony\Flex\Configurator;
 
+use Symfony\Flex\Lock;
 use Symfony\Flex\Recipe;
 
 /**
  * @author Fabien Potencier <[email protected]>
  */
 class ContainerConfigurator extends AbstractConfigurator
 {
-    public function configure(Recipe $recipe, $parameters, array $options = [])
+    public function configure(Recipe $recipe, $parameters, Lock $lock, array $options = [])
     {
         $this->write('Setting parameters');
         $this->addParameters($parameters);
     }
 
-    public function unconfigure(Recipe $recipe, $parameters)
+    public function unconfigure(Recipe $recipe, $parameters, Lock $lock)
     {
         $this->write('Unsetting parameters');
         $target = $this->options->get('root-dir').'/'.$this->options->expandTargetDir('%CONFIG_DIR%/services.yaml');

src/Configurator/CopyFromPackageConfigurator.php

@@ -12,41 +12,45 @@
 namespace Symfony\Flex\Configurator;
 
 use LogicException;
+use Symfony\Flex\Lock;
 use Symfony\Flex\Recipe;
 
 /**
  * @author Fabien Potencier <[email protected]>
  */
 class CopyFromPackageConfigurator extends AbstractConfigurator
 {
-    public function configure(Recipe $recipe, $config, array $options = [])
+    public function configure(Recipe $recipe, $config, Lock $lock, array $options = [])
     {
         $this->write('Setting configuration and copying files');
         $packageDir = $this->composer->getInstallationManager()->getInstallPath($recipe->getPackage());
-        $this->copyFiles($config, $packageDir, $this->options->get('root-dir'), $options['force'] ?? false);
+        $options = array_merge($this->options->toArray(), $options);
+
+        $this->copyFiles($config, $packageDir, $options);
     }
 
-    public function unconfigure(Recipe $recipe, $config)
+    public function unconfigure(Recipe $recipe, $config, Lock $lock)
     {
         $this->write('Removing configuration and files');
         $packageDir = $this->composer->getInstallationManager()->getInstallPath($recipe->getPackage());
         $this->removeFiles($config, $packageDir, $this->options->get('root-dir'));
     }
 
-    private function copyFiles(array $manifest, string $from, string $to, bool $overwrite = false)
+    private function copyFiles(array $manifest, string $from, array $options)
     {
+        $to = $options['root-dir'] ?? '.';
         foreach ($manifest as $source => $target) {
             $target = $this->options->expandTargetDir($target);
             if ('/' === substr($source, -1)) {
-                $this->copyDir($this->path->concatenate([$from, $source]), $this->path->concatenate([$to, $target]), $overwrite);
+                $this->copyDir($this->path->concatenate([$from, $source]), $this->path->concatenate([$to, $target]), $options);
             } else {
                 $targetPath = $this->path->concatenate([$to, $target]);
                 if (!is_dir(\dirname($targetPath))) {
                     mkdir(\dirname($targetPath), 0777, true);
                     $this->write(sprintf('Created <fg=green>"%s"</>', $this->path->relativize(\dirname($targetPath))));
                 }
 
-                $this->copyFile($this->path->concatenate([$from, $source]), $targetPath, $overwrite);
+                $this->copyFile($this->path->concatenate([$from, $source]), $targetPath, $options);
             }
         }
     }
@@ -67,7 +71,7 @@ private function removeFiles(array $manifest, string $from, string $to)
         }
     }
 
-    private function copyDir(string $source, string $target, bool $overwrite)
+    private function copyDir(string $source, string $target, array $options)
     {
         if (!is_dir($target)) {
             mkdir($target, 0777, true);
@@ -82,13 +86,14 @@ private function copyDir(string $source, string $target, bool $overwrite)
                     $this->write(sprintf('Created <fg=green>"%s"</>', $this->path->relativize($targetPath)));
                 }
             } elseif (!file_exists($targetPath)) {
-                $this->copyFile($item, $targetPath, $overwrite);
+                $this->copyFile($item, $targetPath, $options);
             }
         }
     }
 
-    public function copyFile(string $source, string $target, bool $overwrite = false)
+    public function copyFile(string $source, string $target, array $options)
     {
+        $overwrite = $options['force'] ?? false;
         if (!$this->options->shouldWriteFile($target, $overwrite)) {
             return;
         }

src/Configurator/CopyFromRecipeConfigurator.php

@@ -11,51 +11,91 @@
 
 namespace Symfony\Flex\Configurator;
 
+use Symfony\Flex\Lock;
 use Symfony\Flex\Recipe;
 
 /**
  * @author Fabien Potencier <[email protected]>
  */
 class CopyFromRecipeConfigurator extends AbstractConfigurator
 {
-    public function configure(Recipe $recipe, $config, array $options = [])
+    public function configure(Recipe $recipe, $config, Lock $lock, array $options = [])
     {
         $this->write('Setting configuration and copying files');
-        $this->copyFiles($config, $recipe->getFiles(), $this->options->get('root-dir'), $options['force'] ?? false);
+        $options = array_merge($this->options->toArray(), $options);
+
+        $lock->add($recipe->getName(), ['files' => $this->copyFiles($config, $recipe->getFiles(), $options)]);
     }
 
-    public function unconfigure(Recipe $recipe, $config)
+    public function unconfigure(Recipe $recipe, $config, Lock $lock)
     {
         $this->write('Removing configuration and files');
-        $this->removeFiles($config, $recipe->getFiles(), $this->options->get('root-dir'));
+        $this->removeFiles($config, $this->getRemovableFilesFromRecipeAndLock($recipe, $lock), $this->options->get('root-dir'));
+    }
+
+    private function getRemovableFilesFromRecipeAndLock(Recipe $recipe, Lock $lock): array
+    {
+        $lockedFiles = array_unique(
+            array_reduce(
+                array_column($lock->all(), 'files'),
+                function (array $carry, array $package) {
+                    return array_merge($carry, $package);
+                },
+                []
+            )
+        );
+
+        $removableFiles = $recipe->getFiles();
+        foreach ($lockedFiles as $file) {
+            if (isset($removableFiles[$file])) {
+                unset($removableFiles[$file]);
+            }
+        }
+
+        return $removableFiles;
     }
 
-    private function copyFiles(array $manifest, array $files, string $to, bool $overwrite = false)
+    private function copyFiles(array $manifest, array $files, array $options): array
     {
+        $copiedFiles = [];
+        $to = $options['root-dir'] ?? '.';
+
         foreach ($manifest as $source => $target) {
             $target = $this->options->expandTargetDir($target);
             if ('/' === substr($source, -1)) {
-                $this->copyDir($source, $this->path->concatenate([$to, $target]), $files, $overwrite);
+                $copiedFiles = array_merge(
+                    $copiedFiles,
+                    $this->copyDir($source, $this->path->concatenate([$to, $target]), $files, $options)
+                );
             } else {
-                $this->copyFile($this->path->concatenate([$to, $target]), $files[$source]['contents'], $files[$source]['executable'], $overwrite);
+                $copiedFiles[] = $this->copyFile($this->path->concatenate([$to, $target]), $files[$source]['contents'], $files[$source]['executable'], $options);
             }
         }
+
+        return $copiedFiles;
     }
 
-    private function copyDir(string $source, string $target, array $files, bool $overwrite = false)
+    private function copyDir(string $source, string $target, array $files, array $options): array
     {
+        $copiedFiles = [];
         foreach ($files as $file => $data) {
             if (0 === strpos($file, $source)) {
                 $file = $this->path->concatenate([$target, substr($file, \strlen($source))]);
-                $this->copyFile($file, $data['contents'], $data['executable'], $overwrite);
+                $copiedFiles[] = $this->copyFile($file, $data['contents'], $data['executable'], $options);
             }
         }
+
+        return $copiedFiles;
     }
 
-    private function copyFile(string $to, string $contents, bool $executable, bool $overwrite = false)
+    private function copyFile(string $to, string $contents, bool $executable, array $options): string
     {
+        $overwrite = $options['force'] ?? false;
+        $basePath = $options['root-dir'] ?? '.';
+        $copiedFile = str_replace($basePath.\DIRECTORY_SEPARATOR, '', $to);
+
         if (!$this->options->shouldWriteFile($to, $overwrite)) {
-            return;
+            return $copiedFile;
         }
 
         if (!is_dir(\dirname($to))) {
@@ -68,6 +108,8 @@ private function copyFile(string $to, string $contents, bool $executable, bool $
         }
 
         $this->write(sprintf('Created <fg=green>"%s"</>', $this->path->relativize($to)));
+
+        return $copiedFile;
     }
 
     private function removeFiles(array $manifest, array $files, string $to)

src/Configurator/EnvConfigurator.php

@@ -11,14 +11,15 @@
 
 namespace Symfony\Flex\Configurator;
 
+use Symfony\Flex\Lock;
 use Symfony\Flex\Recipe;
 
 /**
  * @author Fabien Potencier <[email protected]>
  */
 class EnvConfigurator extends AbstractConfigurator
 {
-    public function configure(Recipe $recipe, $vars, array $options = [])
+    public function configure(Recipe $recipe, $vars, Lock $lock, array $options = [])
     {
         $this->write('Added environment variable defaults');
 
@@ -28,7 +29,7 @@ public function configure(Recipe $recipe, $vars, array $options = [])
         }
     }
 
-    public function unconfigure(Recipe $recipe, $vars)
+    public function unconfigure(Recipe $recipe, $vars, Lock $lock)
     {
         $this->unconfigureEnvFiles($recipe, $vars);
         $this->unconfigurePhpUnit($recipe, $vars);

src/Configurator/GitignoreConfigurator.php

@@ -11,14 +11,15 @@
 
 namespace Symfony\Flex\Configurator;
 
+use Symfony\Flex\Lock;
 use Symfony\Flex\Recipe;
 
 /**
  * @author Fabien Potencier <[email protected]>
  */
 class GitignoreConfigurator extends AbstractConfigurator
 {
-    public function configure(Recipe $recipe, $vars, array $options = [])
+    public function configure(Recipe $recipe, $vars, Lock $lock, array $options = [])
     {
         $this->write('Added entries to .gitignore');
 
@@ -39,7 +40,7 @@ public function configure(Recipe $recipe, $vars, array $options = [])
         }
     }
 
-    public function unconfigure(Recipe $recipe, $vars)
+    public function unconfigure(Recipe $recipe, $vars, Lock $lock)
     {
         $file = $this->options->get('root-dir').'/.gitignore';
         if (!file_exists($file)) {

src/Configurator/MakefileConfigurator.php

@@ -11,14 +11,15 @@
 
 namespace Symfony\Flex\Configurator;
 
+use Symfony\Flex\Lock;
 use Symfony\Flex\Recipe;
 
 /**
  * @author Fabien Potencier <[email protected]>
  */
 class MakefileConfigurator extends AbstractConfigurator
 {
-    public function configure(Recipe $recipe, $definitions, array $options = [])
+    public function configure(Recipe $recipe, $definitions, Lock $lock, array $options = [])
     {
         $this->write('Added Makefile entries');
 
@@ -53,7 +54,7 @@ public function configure(Recipe $recipe, $definitions, array $options = [])
         }
     }
 
-    public function unconfigure(Recipe $recipe, $vars)
+    public function unconfigure(Recipe $recipe, $vars, Lock $lock)
     {
         if (!file_exists($makefile = $this->options->get('root-dir').'/Makefile')) {
             return;