Don't remove still-referenced files when updating recipes

Author: wouterj

src/Command/UpdateRecipesCommand.php

@@ -153,7 +153,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         $recipeUpdate = new RecipeUpdate($originalRecipe, $newRecipe, $symfonyLock, $this->rootDir);
         $this->configurator->populateUpdate($recipeUpdate);
         $originalComposerJsonHash = $this->flex->getComposerJsonHash();
-        $patcher = new RecipePatcher($this->rootDir, $io);
+        $patcher = new RecipePatcher($this->rootDir, $io, $symfonyLock);
 
         try {
             $patch = $patcher->generatePatch($recipeUpdate->getOriginalFiles(), $recipeUpdate->getNewFiles());

src/Update/RecipePatcher.php

@@ -15,19 +15,22 @@
 use Composer\Util\ProcessExecutor;
 use Symfony\Component\Filesystem\Exception\IOException;
 use Symfony\Component\Filesystem\Filesystem;
+use Symfony\Flex\Lock;
 
 class RecipePatcher
 {
     private $rootDir;
     private $filesystem;
     private $io;
     private $processExecutor;
+    private $symfonyLock;
 
-    public function __construct(string $rootDir, IOInterface $io)
+    public function __construct(string $rootDir, IOInterface $io, Lock $symfonyLock)
     {
         $this->rootDir = $rootDir;
         $this->filesystem = new Filesystem();
         $this->io = $io;
+        $this->symfonyLock = $symfonyLock;
         $this->processExecutor = new ProcessExecutor($io);
     }
 
@@ -39,11 +42,30 @@ public function __construct(string $rootDir, IOInterface $io)
     public function applyPatch(RecipePatch $patch): bool
     {
         $withConflicts = $this->_applyPatchFile($patch);
+        $lockedFiles = array_count_values(array_merge(...array_column($this->symfonyLock->all(), 'files')));
 
+        $nonRemovableFiles = [];
         foreach ($patch->getDeletedFiles() as $deletedFile) {
-            if (file_exists($this->rootDir.'/'.$deletedFile)) {
-                $this->execute(\sprintf('git rm %s', ProcessExecutor::escape($deletedFile)), $this->rootDir);
+            if (!file_exists($this->rootDir.'/'.$deletedFile)) {
+                continue;
+            }
+
+            if (isset($lockedFiles[$deletedFile])) {
+                $nonRemovableFiles[] = $deletedFile;
+
+                continue;
             }
+
+            $this->execute(\sprintf('git rm %s', ProcessExecutor::escape($deletedFile)), $this->rootDir);
+        }
+
+        if ($nonRemovableFiles) {
+            $this->io->writeError('  <warning>The following files were removed in the recipe, but are still referenced by other recipes. You might need to adjust them manually:</warning>');
+            foreach ($nonRemovableFiles as $file) {
+                $this->io->writeError('      - '.$file);
+            }
+
+            $this->io->writeError('');
         }
 
         return $withConflicts;

tests/Update/RecipePatcherTest.php

@@ -15,6 +15,7 @@
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Filesystem\Filesystem;
 use Symfony\Component\Process\Process;
+use Symfony\Flex\Lock;
 use Symfony\Flex\Update\RecipePatch;
 use Symfony\Flex\Update\RecipePatcher;
 
@@ -52,7 +53,7 @@ public function testGeneratePatch(array $originalFiles, array $newFiles, string
             (new Process(['git', 'commit', '-m', '"original files"'], FLEX_TEST_DIR))->mustRun();
         }
 
-        $patcher = new RecipePatcher(FLEX_TEST_DIR, $this->createMock(IOInterface::class));
+        $patcher = new RecipePatcher(FLEX_TEST_DIR, $this->createMock(IOInterface::class), $this->createMock(Lock::class));
 
         $patch = $patcher->generatePatch($originalFiles, $newFiles);
         $this->assertSame($expectedPatch, rtrim($patch->getPatch(), "\n"));
@@ -189,7 +190,7 @@ public function testGeneratePatchOnDeletedFile()
         $this->getFilesystem()->remove(FLEX_TEST_DIR);
         $this->getFilesystem()->mkdir(FLEX_TEST_DIR);
 
-        $patcher = new RecipePatcher(FLEX_TEST_DIR, $this->createMock(IOInterface::class));
+        $patcher = new RecipePatcher(FLEX_TEST_DIR, $this->createMock(IOInterface::class), $this->createMock(Lock::class));
 
         // try to update a file that does not exist in the project
         $patch = $patcher->generatePatch(['.env' => 'original contents'], ['.env' => 'new contents']);
@@ -217,7 +218,7 @@ public function testApplyPatch(array $filesCurrentlyInApp, RecipePatch $recipePa
             (new Process(['git', 'commit', '-m', 'Committing original files'], FLEX_TEST_DIR))->mustRun();
         }
 
-        $patcher = new RecipePatcher(FLEX_TEST_DIR, $this->createMock(IOInterface::class));
+        $patcher = new RecipePatcher(FLEX_TEST_DIR, $this->createMock(IOInterface::class), $this->createMock(Lock::class));
         $hadConflicts = !$patcher->applyPatch($recipePatch);
 
         foreach ($expectedFiles as $file => $expectedContents) {
@@ -261,7 +262,7 @@ public function testApplyPatchOnSubfolder(array $filesCurrentlyInApp, RecipePatc
             (new Process(['git', 'commit', '-m', 'Committing original files'], $subProjectPath))->mustRun();
         }
 
-        $patcher = new RecipePatcher($subProjectPath, $this->createMock(IOInterface::class));
+        $patcher = new RecipePatcher($subProjectPath, $this->createMock(IOInterface::class), $this->createMock(Lock::class));
         $hadConflicts = !$patcher->applyPatch($recipePatch);
 
         foreach ($expectedFiles as $file => $expectedContents) {
@@ -390,7 +391,7 @@ public function testIntegration(bool $useNullForMissingFiles)
         (new Process(['git', 'add', '-A'], FLEX_TEST_DIR))->mustRun();
         (new Process(['git', 'commit', '-m', 'committing in app start files'], FLEX_TEST_DIR))->mustRun();
 
-        $patcher = new RecipePatcher(FLEX_TEST_DIR, $this->createMock(IOInterface::class));
+        $patcher = new RecipePatcher(FLEX_TEST_DIR, $this->createMock(IOInterface::class), $this->createMock(Lock::class));
         $originalFiles = [
             '.env' => $files['dot_env_clean']['original_recipe'],
             'package.json' => $files['package_json_conflict']['original_recipe'],