diff --git a/src/Command/UpdateRecipesCommand.php b/src/Command/UpdateRecipesCommand.php index 96d7d0d8..1b4491bb 100644 --- a/src/Command/UpdateRecipesCommand.php +++ b/src/Command/UpdateRecipesCommand.php @@ -153,11 +153,11 @@ 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()); - $hasConflicts = !$patcher->applyPatch($patch); + $hasConflicts = !$patcher->applyPatch($patch, $packageName); } catch (\Throwable $throwable) { $io->writeError([ 'There was an error applying the recipe update patch', diff --git a/src/Update/RecipePatcher.php b/src/Update/RecipePatcher.php index 680fb0f3..66965ee0 100644 --- a/src/Update/RecipePatcher.php +++ b/src/Update/RecipePatcher.php @@ -15,6 +15,7 @@ use Composer\Util\ProcessExecutor; use Symfony\Component\Filesystem\Exception\IOException; use Symfony\Component\Filesystem\Filesystem; +use Symfony\Flex\Lock; class RecipePatcher { @@ -22,12 +23,14 @@ class RecipePatcher 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); } @@ -36,14 +39,33 @@ public function __construct(string $rootDir, IOInterface $io) * * @return bool returns true if fully successful, false if conflicts were encountered */ - public function applyPatch(RecipePatch $patch): bool + public function applyPatch(RecipePatch $patch, ?string $packageName = null): bool { $withConflicts = $this->_applyPatchFile($patch); + $lockedFiles = $packageName ? array_count_values(array_merge(...array_column(array_filter($this->symfonyLock->all(), fn ($package) => $package !== $packageName, \ARRAY_FILTER_USE_KEY), '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(' The following files were removed in the recipe, but are still referenced by other recipes. You might need to adjust them manually:'); + foreach ($nonRemovableFiles as $file) { + $this->io->writeError(' - '.$file); + } + + $this->io->writeError(''); } return $withConflicts; diff --git a/tests/Update/RecipePatcherTest.php b/tests/Update/RecipePatcherTest.php index c4f76207..0dceb6e8 100644 --- a/tests/Update/RecipePatcherTest.php +++ b/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) { @@ -233,6 +234,38 @@ public function testApplyPatch(array $filesCurrentlyInApp, RecipePatch $recipePa $this->assertSame($expectedConflicts, $hadConflicts); } + public function testApplyPatchFileOwnedByMultipleRecipes() + { + (new Process(['git', 'init'], FLEX_TEST_DIR))->mustRun(); + (new Process(['git', 'config', 'user.name', 'Unit test'], FLEX_TEST_DIR))->mustRun(); + (new Process(['git', 'config', 'user.email', ''], FLEX_TEST_DIR))->mustRun(); + + $dir = FLEX_TEST_DIR.'/config/packages'; + @mkdir($dir, 0777, true); + file_put_contents($dir.'/security.yaml', '# contents'); + (new Process(['git', 'add', '-A'], FLEX_TEST_DIR))->mustRun(); + (new Process(['git', 'commit', '-m', 'Committing original files'], FLEX_TEST_DIR))->mustRun(); + + + $lock = $this->createMock(Lock::class); + $lock->expects($this->any())->method('all')->willReturn([ + 'symfony/security-bundle' => ['files' => ['config/packages/security.yaml']], + 'symfony/security' => ['files' => ['config/packages/security.yaml']], + ]); + $patcher = new RecipePatcher(FLEX_TEST_DIR, $this->createMock(IOInterface::class), $lock); + + $patchData = $this->generatePatchData('config/packages/security.yaml', '# contents', null); + $hadConflicts = !$patcher->applyPatch(new RecipePatch( + '', + [$patchData['hash'] => $patchData['blob']], + ['config/packages/security.yaml'] + ), 'symfony/security-bundle'); + + $this->assertFileExists($dir.'/security.yaml'); + $this->assertSame('# contents', file_get_contents($dir.'/security.yaml')); + $this->assertFalse($hadConflicts); + } + /** * @dataProvider getApplyPatchTests */ @@ -261,7 +294,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 +423,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'],