Skip to content

Commit c0c28f8

Browse files
committed
bug #885 fix: Recipe update skip ignored files (maxime-aknin, Max)
This PR was squashed before being merged into the 1.x branch. Discussion ---------- fix: Recipe update skip ignored files I ran into an issue with the `composer recipes:update` command. While trying to update `doctrine/doctrine-bundle` recipe from 2.0 to 2.4, the command ouput stated that the recipe was updated with conflicts, but there were no actual changes made in files (except the symfony lock file): ![Screen Shot 2022-03-15 at 5 15 21 PM](https://user-images.githubusercontent.com/8659993/158423936-b9000e32-9e92-42f8-b74d-d7f41e710905.png) What's happening is that this recipe update contains changes that are applied to `phpunit.xml` by ` Symfony\Flex\Configurator\EnvConfigurator`, but this file is also added to `gitignore` by `phpunit/phpunit` [recipe](https://github.com/symfony/recipes/blob/2d1ba2cf32556d9c01ff7bd05bb02ec9bfb44e5c/phpunit/phpunit/9.3/manifest.json#L8) The fact that there is an ignored file in the recipe patch makes the `git apply` command in [RecipePatcher](https://github.com/symfony/flex/blob/b66b0dd8f5099002f4863aa0bba0c3e0c9e8523b/src/Update/RecipePatcher.php#L55) fail without applying the patch, and this failure is not detected by the rest of the function. ![Screen Shot 2022-03-15 at 5 31 10 PM](https://user-images.githubusercontent.com/8659993/158426089-956a3475-24af-4b90-8dc1-31c00d3c52d9.png) My proposal is to filter out ignored files from the beginning of the `generatePatch` function to fix this issue. Commits ------- cba1b4a fix: Recipe update skip ignored files
2 parents 2c857c0 + cba1b4a commit c0c28f8

File tree

2 files changed

+41
-6
lines changed

2 files changed

+41
-6
lines changed

src/Update/RecipePatcher.php

+18-6
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,16 @@ public function applyPatch(RecipePatch $patch): bool
5151

5252
public function generatePatch(array $originalFiles, array $newFiles): RecipePatch
5353
{
54+
$ignoredFiles = $this->getIgnoredFiles(array_keys($originalFiles) + array_keys($newFiles));
55+
5456
// null implies "file does not exist"
55-
$originalFiles = array_filter($originalFiles, function ($file) {
56-
return null !== $file;
57-
});
58-
$newFiles = array_filter($newFiles, function ($file) {
59-
return null !== $file;
60-
});
57+
$originalFiles = array_filter($originalFiles, function ($file, $fileName) use ($ignoredFiles) {
58+
return null !== $file && !\in_array($fileName, $ignoredFiles);
59+
}, \ARRAY_FILTER_USE_BOTH);
60+
61+
$newFiles = array_filter($newFiles, function ($file, $fileName) use ($ignoredFiles) {
62+
return null !== $file && !\in_array($fileName, $ignoredFiles);
63+
}, \ARRAY_FILTER_USE_BOTH);
6164

6265
$deletedFiles = [];
6366
// find removed files & record that they are deleted
@@ -240,4 +243,13 @@ private function _applyPatchFile(RecipePatch $patch)
240243
}
241244
}
242245
}
246+
247+
private function getIgnoredFiles(array $fileNames): array
248+
{
249+
$args = implode(' ', array_map([ProcessExecutor::class, 'escape'], $fileNames));
250+
$output = '';
251+
$this->processExecutor->execute(sprintf('git check-ignore %s', $args), $output, $this->rootDir);
252+
253+
return $this->processExecutor->splitLines($output);
254+
}
243255
}

tests/Update/RecipePatcherTest.php

+23
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,18 @@ public function testGeneratePatch(array $originalFiles, array $newFiles, string
3838
// original files need to be present to avoid patcher thinking they were deleting and skipping patch
3939
foreach ($originalFiles as $file => $contents) {
4040
touch(FLEX_TEST_DIR.'/'.$file);
41+
if ('.gitignore' === $file) {
42+
file_put_contents(FLEX_TEST_DIR.'/'.$file, $contents);
43+
}
44+
}
45+
46+
// make sure the test directory is a git repo
47+
(new Process(['git', 'init'], FLEX_TEST_DIR))->mustRun();
48+
(new Process(['git', 'config', 'user.name', '"Flex Updater"'], FLEX_TEST_DIR))->mustRun();
49+
(new Process(['git', 'config', 'user.email', '""'], FLEX_TEST_DIR))->mustRun();
50+
if (0 !== \count($originalFiles)) {
51+
(new Process(['git', 'add', '-A'], FLEX_TEST_DIR))->mustRun();
52+
(new Process(['git', 'commit', '-m', '"original files"'], FLEX_TEST_DIR))->mustRun();
4153
}
4254

4355
$patcher = new RecipePatcher(FLEX_TEST_DIR, $this->createMock(IOInterface::class));
@@ -46,6 +58,11 @@ public function testGeneratePatch(array $originalFiles, array $newFiles, string
4658
$this->assertSame($expectedPatch, rtrim($patch->getPatch(), "\n"));
4759
$this->assertSame($expectedDeletedFiles, $patch->getDeletedFiles());
4860

61+
// when testing ignored files the patch is empty
62+
if ('' === $expectedPatch) {
63+
return;
64+
}
65+
4966
// find all "index 7d30dc7.." in patch
5067
$matches = [];
5168
preg_match_all('/index\ ([0-9|a-z]+)\.\./', $patch->getPatch(), $matches);
@@ -158,6 +175,12 @@ public function getGeneratePatchTests(): iterable
158175
,
159176
['will_be_deleted.txt'],
160177
];
178+
179+
yield 'ignored_file' => [
180+
['file1.txt' => 'Original contents', '.gitignore' => 'file1.txt'],
181+
['file1.txt' => 'Updated contents', '.gitignore' => 'file1.txt'],
182+
'',
183+
];
161184
}
162185

163186
public function testGeneratePatchOnDeletedFile()

0 commit comments

Comments
 (0)