Flex: Removing test-pack deletes config/bootstrap.php

[leongersen]

This is a re-report of #498, which was closed as solved. In that report, the issue was an outdated symfony.lock file. This issue persists with a lock file written by the most recent version of flex.

Version:

symfony/flex       v1.2.6

Reproducing:

# Setup Symfony
composer create-project symfony/website-skeleton my-project

# Remove test-pack
composer remove symfony/test-pack --dev

Output:

Symfony operations: 1 recipe (d383c6fabd7f692fcebff8abb7bad1c1)
- Unconfiguring symfony/phpunit-bridge (>=4.3): From github.com/symfony/recipes:master
Executing script cache:clear [KO]
[KO]
Script cache:clear returned with error code 255
!!
!!  Warning: require(C:\flex\my-project/config/bootstrap.php): failed to open stream: No such file or directory in C:\flex\my-project\bin\console on line 30                                                                                                                    

[leongersen]

From what I can tell, the Lock that is passed to \Symfony\Flex\Configurator\CopyFromPackageConfigurator::unconfigure isn't used, whereas CopyFromRecipeConfigurator has a method getRemovableFilesFromRecipeAndLock which checks for requirements in other recipes.

[dbrumann]

Yes, in newer versions this should be fixed by keeping a list of files created through the recipe inside symfony.lock. Unfortunately, if you created the project before this was introduced to Flex, then checking for locked files will not work and Flex might remove files used by multiple recipes.

For reference this was implemented in #451

edit: If this is still happening with a new project and a current version of Symfony Flex, I will have to check if there is an edge case I missed.

[leongersen]

@dbrumann, thanks for responding. I did see that PR earlier, and while CopyFromRecipeConfigurator was adapted to respect the lock file, it is CopyFromPackageConfigurator that removes the bootstrap.php file.

I tried it again just now with a completely fresh install of the website-skeleton, and the issue still occurs in the current release of flex.

[dbrumann]

I have tried this by removing symfony/console. I did not find a case where this behavior was needed for packages, but considering your bug, I guess it should also be implemented there. I will see when I have time to make a fix, probably on Friday, but feel free to take this over, if you like.

[leongersen]

What is the desired behavior here? bootstrap.php does contain logic specific to phpunit. Simply not removing the file doesn't properly unconfigure the recipe, does it?

[dbrumann]

I have to look at it in more detail, but are you sure this file is only needed by phpunit? I was thinking that config/bootstrap.php is required by the FrameworkBundle and the Console. Flex should not remove this file when unconfiguring the test-pack.

Basically my idea was, that the initial concept still holds. We track each file that was created during require and ought to be deleted when executing composer remove. If a file is used by more than one package/recipe it should not be deleted. It might be that I am overlooking something that works different with packs, though.

[leongersen]

only needed by phpunit

No, but there is added configuration to handle the .env files for the test environment. To do this in a clean way, the bootstrap file should not be modified by individual packages/packs, but there should be an extension opportunity (e.g. including all files in a bootstrap folder?).

[dbrumann]

Are you sure about that or maybe I misunderstand? As far as I know, once a file was copied Flex will not change it, as you might have made project specific customizations. The only exception is .env where there are special markers to ensure that adding/removing lines will not produce conflicts.

I just checked the recipes and it looks like the bootstrap.php provided both by phpunit/phpunit and symfony/phpunit-bridge will just reference the original file from the FrameworkBundle:

• https://github.com/symfony/recipes/blob/master/phpunit/phpunit/4.7/config/bootstrap.php
• https://github.com/symfony/recipes/blob/master/symfony/phpunit-bridge/4.3/config/bootstrap.php

[dbrumann]

Hey, I just wanted to look at this and I can't reproduce this.

I created a project both with composer create-project symfony/website-skeleton and symfony new --full and then removed the test pack. Since symfony.lock contains entries both for the pack itself and PhpUnitBridge the latter keeps track of the configured files, including bootstrap.php, preventing it from being deleted. This matches my initial assumption that it's (usually) not necessary to keep tracks of files from a package, but rather from the recipes installed via that package.

I can't quite explain why you run into this issue. I used Composer 1.8.5 with Flex 1.2.6 and my project did not specify a version constraint, so I got Symfony 4.3.1

[leongersen]

Are you sure about that or maybe I misunderstand?

No, you are correct. I misunderstood how that aspect worked.

I just tried to reproduce this again, and this does happen for me on Windows 10 with Flex 1.2.7, Composer 1.8.5, Symfony 4.3.1:

$ composer remove symfony/test-pack --dev
Restricting packages listed in "symfony/symfony" to "4.3.*"
Restricting packages listed in "symfony/symfony" to "4.3.*"
Dependency "symfony/http-client" is also a root requirement, but is not explicitly whitelisted. Ignoring.
Dependency "symfony/process" is also a root requirement, but is not explicitly whitelisted. Ignoring.
Dependency "symfony/process" is also a root requirement, but is not explicitly whitelisted. Ignoring.
Loading composer repositories with package information
Updating dependencies (including require-dev)
Package operations: 0 installs, 0 updates, 7 removals
  - Removing symfony/test-pack (v1.0.5)
  - Removing symfony/phpunit-bridge (v4.3.1)
  - Removing symfony/panther (v0.4.1)
  - Removing symfony/dom-crawler (v4.3.1)
  - Removing symfony/css-selector (v4.3.1)
  - Removing symfony/browser-kit (v4.3.1)
  - Removing facebook/webdriver (1.7.1)
Writing lock file
Generating autoload files
ocramius/package-versions:  Generating version class...
ocramius/package-versions: ...done generating version class
Symfony operations: 1 recipe (0d0b547d9c284e7a753e9c0d4ac95482)
  - Unconfiguring symfony/phpunit-bridge (>=4.3): From github.com/symfony/recipes:master
Executing script cache:clear [KO]
 [KO]
Script cache:clear returned with error code 255
!!
!!  Warning: require(xxx\my-project/config/bootstrap.php): failed to open stream: No such file or directory in xxx\my-project\bin\console on line 30

[dbrumann]

Could be that I have to make sure DIRECTORY_SEPARATOR is used to ensure compatibility with Windows. I don't have a Windows PC, but I will have a look at it again to see whether I can spot the cause.

[leongersen]

I'll spin up a VM, just a second.

[leongersen]

I just tried the same steps in a linux environment, and the issue doesn't happen there.

[leongersen]

The issue isn't in the DIRECTORY_SEPARATOR, the paths have a leading ./.

In \Symfony\Flex\Configurator\CopyFromRecipeConfigurator::getRemovableFilesFromRecipeAndLock:

$lockedFiles:

array(41) {
...
  [10]=>
  string(22) "./config/bootstrap.php"
...

array_keys($removableFiles):

array(5) {
...
  [2]=>
  string(20) "config/bootstrap.php"
...
}

[nicolas-grekas]

Anyone willing to work on this?
I just noticed CopyFromPackageConfigurator is missing files tracking also!

[dbrumann]

I might have some time near the weekend to have a look at this, but since I don't have a Windows PC I am not sure if I can reliably verify if it fixes the issue.

Do you have an example where tracking files in CopyFromPackageConfigurator is required? So far, each package I have encountered will install 1 or more recipes, which means files are tracked through those recipes.

[dbrumann]

@leongersen I created a pull-request that should hopefully fix the problem with the relative paths beginning with "./". I don't have a Windows PC, could you please verify for me if this works? The original PR #451 contains a quick guide to switch out flex in a demo app to make sure the (hopefully) fixed version is being used.

The updates for CopyFromPackageConfigurator I will do in a follow up PR if it's still considered necessary.

[leongersen]

@dbrumann I'll try it now.

[leongersen]

I'm afraid this doesn't work, I still end up with $lockedFiles being prefixed with ./, whereas array_keys($removableFiles) does not have them. If I change the checks to realpath both sides, I get the expected result:

        $lockedFiles = array_map('realpath', $lockedFiles);

        foreach (array_keys($removableFiles) as $file) {
            if (in_array(realpath($file), $lockedFiles)) {
                unset($removableFiles[$file]);
            }
        }

[dbrumann]

Thanks for testing. I have updated the PR to normalize paths containing "./", which hopefully should fix the issue and also work on a symfony.lock already containing the "wrong" path. Could you please do another test run for me?

[leongersen]

I tried it again, and unfortunately I still end up with a removed bootstrap.php. I opened a PR for comparing the files to be removed against the locked files by comparing their real path.