diff --git a/src/Command/UpdateRecipesCommand.php b/src/Command/UpdateRecipesCommand.php index 5bd9731e..2839559e 100644 --- a/src/Command/UpdateRecipesCommand.php +++ b/src/Command/UpdateRecipesCommand.php @@ -193,7 +193,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int ' After resolving, commit your changes like normal.', ]); } else { - if (!$patch->getPatch()) { + if (!$patch->getPatch() && !$patch->getDeletedFiles()) { // no changes were required $io->write([ ' No files were changed as a result of the update.', diff --git a/src/Update/RecipePatch.php b/src/Update/RecipePatch.php index bb6abdbc..4828a6ed 100644 --- a/src/Update/RecipePatch.php +++ b/src/Update/RecipePatch.php @@ -17,13 +17,15 @@ class RecipePatch private $blobs; private $deletedFiles; private $removedPatches; + private $keptFiles; - public function __construct(string $patch, array $blobs, array $deletedFiles, array $removedPatches = []) + public function __construct(string $patch, array $blobs, array $deletedFiles, array $removedPatches = [], array $keptFiles = []) { $this->patch = $patch; $this->blobs = $blobs; $this->deletedFiles = $deletedFiles; $this->removedPatches = $removedPatches; + $this->keptFiles = $keptFiles; } public function getPatch(): string @@ -49,4 +51,13 @@ public function getRemovedPatches(): array { return $this->removedPatches; } + + /** + * Files removed in the recipe that were not deleted because + * they have been modified in the user's project. + */ + public function getKeptFiles(): array + { + return $this->keptFiles; + } } diff --git a/src/Update/RecipePatcher.php b/src/Update/RecipePatcher.php index 66965ee0..c04f6d23 100644 --- a/src/Update/RecipePatcher.php +++ b/src/Update/RecipePatcher.php @@ -46,10 +46,6 @@ public function applyPatch(RecipePatch $patch, ?string $packageName = null): boo $nonRemovableFiles = []; foreach ($patch->getDeletedFiles() as $deletedFile) { - if (!file_exists($this->rootDir.'/'.$deletedFile)) { - continue; - } - if (isset($lockedFiles[$deletedFile])) { $nonRemovableFiles[] = $deletedFile; @@ -68,6 +64,15 @@ public function applyPatch(RecipePatch $patch, ?string $packageName = null): boo $this->io->writeError(''); } + if ($patch->getKeptFiles()) { + $this->io->writeError(' The following files were removed in the recipe, but they were kept because you modified them. You might need to remove them manually:'); + foreach ($patch->getKeptFiles() as $file) { + $this->io->writeError(' - '.$file); + } + + $this->io->writeError(''); + } + return $withConflicts; } @@ -85,12 +90,25 @@ public function generatePatch(array $originalFiles, array $newFiles): RecipePatc }, \ARRAY_FILTER_USE_BOTH); $deletedFiles = []; - // find removed files & record that they are deleted + $keptFiles = []; + // find removed files & record that they are deleted, unless they were modified in the project // unset them from originalFiles to avoid unnecessary blobs being added foreach ($originalFiles as $file => $contents) { - if (!isset($newFiles[$file])) { + if (isset($newFiles[$file])) { + continue; + } + + unset($originalFiles[$file]); + + $path = $this->rootDir.'/'.$file; + if (!file_exists($path)) { + continue; + } + + if (file_get_contents($path) === $contents) { $deletedFiles[] = $file; - unset($originalFiles[$file]); + } else { + $keptFiles[] = $file; } } @@ -136,7 +154,8 @@ public function generatePatch(array $originalFiles, array $newFiles): RecipePatc $patchString, $blobs, $deletedFiles, - $removedPatches + $removedPatches, + $keptFiles ); } finally { try { diff --git a/tests/Update/RecipePatchTest.php b/tests/Update/RecipePatchTest.php index 97f28769..dd98ce41 100644 --- a/tests/Update/RecipePatchTest.php +++ b/tests/Update/RecipePatchTest.php @@ -22,12 +22,14 @@ public function testBasicFunctioning() $blobs = ['blob1', 'blob2', 'beware of the blob']; $deletedFiles = ['old_file.txt']; $removedPatches = ['foo' => 'some diff']; + $keptFiles = ['modified_file.txt']; - $patch = new RecipePatch($thePatch, $blobs, $deletedFiles, $removedPatches); + $patch = new RecipePatch($thePatch, $blobs, $deletedFiles, $removedPatches, $keptFiles); $this->assertSame($thePatch, $patch->getPatch()); $this->assertSame($blobs, $patch->getBlobs()); $this->assertSame($deletedFiles, $patch->getDeletedFiles()); $this->assertSame($removedPatches, $patch->getRemovedPatches()); + $this->assertSame($keptFiles, $patch->getKeptFiles()); } } diff --git a/tests/Update/RecipePatcherTest.php b/tests/Update/RecipePatcherTest.php index 1b51e957..5a460355 100644 --- a/tests/Update/RecipePatcherTest.php +++ b/tests/Update/RecipePatcherTest.php @@ -11,6 +11,7 @@ namespace Symfony\Flex\Tests\Update; +use Composer\IO\BufferIO; use Composer\IO\IOInterface; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; @@ -37,10 +38,7 @@ public function testGeneratePatch(array $originalFiles, array $newFiles, string self::getFilesystem()->mkdir(FLEX_TEST_DIR); // original files need to be present to avoid patcher thinking they were deleting and skipping patch foreach ($originalFiles as $file => $contents) { - touch(FLEX_TEST_DIR.'/'.$file); - if ('.gitignore' === $file) { - file_put_contents(FLEX_TEST_DIR.'/'.$file, $contents); - } + file_put_contents(FLEX_TEST_DIR.'/'.$file, $contents ?? ''); } // make sure the test directory is a git repo @@ -195,6 +193,45 @@ public function testGeneratePatchOnDeletedFile() $this->assertSame('', $patch->getPatch()); } + public function testDeletedFileModifiedInAppIsKept() + { + (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); + $modifiedContents = <<mustRun(); + (new Process(['git', 'commit', '-m', 'Committing original files'], FLEX_TEST_DIR))->mustRun(); + + $io = new BufferIO(); + $patcher = new RecipePatcher(FLEX_TEST_DIR, $io, $this->createStub(Lock::class)); + + $patch = $patcher->generatePatch([ + 'config/packages/workflow.yaml' => "framework:\n workflows: null\n", + 'config/packages/untouched.yaml' => '# untouched', + 'config/packages/missing.yaml' => '# missing', + ], []); + + $this->assertSame('', $patch->getPatch()); + $this->assertSame(['config/packages/untouched.yaml'], $patch->getDeletedFiles()); + $this->assertSame(['config/packages/workflow.yaml'], $patch->getKeptFiles()); + + $this->assertTrue($patcher->applyPatch($patch)); + $this->assertSame($modifiedContents, file_get_contents($dir.'/workflow.yaml')); + $this->assertFileDoesNotExist($dir.'/untouched.yaml'); + $this->assertSame("D config/packages/untouched.yaml\n", (new Process(['git', 'status', '--porcelain'], FLEX_TEST_DIR))->mustRun()->getOutput()); + $this->assertStringContainsString("they were kept because you modified them. You might need to remove them manually:\n - config/packages/workflow.yaml\n", $io->getOutput()); + } + #[DataProvider('provideApplyPatchCases')] public function testApplyPatch(array $filesCurrentlyInApp, RecipePatch $recipePatch, array $expectedFiles, bool $expectedConflicts) { @@ -451,7 +488,7 @@ public function testIntegration(bool $useNullForMissingFiles) $this->assertSame($files['dot_env_clean']['expected'], file_get_contents(FLEX_TEST_DIR.'/.env')); $this->assertSame($files['package_json_conflict']['expected'], file_get_contents(FLEX_TEST_DIR.'/package.json')); $this->assertSame($files['webpack_encore_added']['expected'], file_get_contents(FLEX_TEST_DIR.'/config/packages/webpack_encore.yaml')); - $this->assertFileDoesNotExist(FLEX_TEST_DIR.'/security.yaml'); + $this->assertFileDoesNotExist(FLEX_TEST_DIR.'/config/packages/security.yaml'); } public static function getIntegrationTests(): iterable