Lock copied files to prevent removing files needed by other recipes #451

dbrumann · 2018-12-13T12:15:55Z

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.

nicolas-grekas · 2018-12-20T12:15:10Z

(rebase needed)

nicolas-grekas · 2019-01-22T12:04:59Z

Hi @dbrumann, friendly ping to know the status here :)

dbrumann · 2019-01-23T13:52:46Z

Hi @nicolas-grekas. Sorry for not updating. I'm currently busy settling into a new client project.

Writing tests for this was a bit tricky due to the Lock-class requiring a file to write to. I think the rebase and the flexible root-dir made this a bit easier, but I had no time to confirm this. I don't feel comfortable mocking the calls to Lock as I want to ensure the file list is handled properly.

I won't have time until Wednesday next week though. I set the end of the first week of February as my personal deadline for this.

dbrumann · 2019-02-08T14:16:38Z

Hey @nicolas-grekas I think this is ready. Let me know if something is missing for you.

src/Configurator/CopyFromPackageConfigurator.php

src/Configurator/CopyFromRecipeConfigurator.php

tests/Configurator/EnvConfiguratorTest.php

tests/Configurator/MakefileConfiguratorTest.php

dbrumann

~~There seems to be an issue after my last changes. When manually trying to verify the fix right now, it did not seem to work. I will update as soon as I figure out what went wrong.~~

The issue is fixed. I messed up when replacing my array_map-construct with array_column.

nicolas-grekas

Tested locally, works like a charm!

src/Configurator/CopyFromRecipeConfigurator.php

- Allows passing lock to configurators. - Stores copied files in lock. - Ignores locked files on remove.

nicolas-grekas · 2019-02-21T11:31:39Z

Thank you @dbrumann.

…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.

…pes (nicolas-grekas) This PR was merged into the 2.x branch. Discussion ---------- Don't remove still-referenced files when unconfiguring recipes Instead of #706 Fixes broken logic added in #451, which didn't account for folders. The new logic uses only the symfony.lock file and not the recipe anymore to decide which files to remove. Commits ------- 6443e31 Don't remove still-referenced files when unconfiguring recipes

nicolas-grekas mentioned this pull request Jan 7, 2019

Removing test-pack deletes config/bootstrap.php file symfony/symfony#29805

Closed

dbrumann changed the title ~~[WIP] Lock copied files to prevent removing files needed by other recipes~~ Lock copied files to prevent removing files needed by other recipes Feb 8, 2019