bug #937 Updating Recipe Fails When Project Located in Subfolder in Git Repository (Peukku, Mikko Peltola)

This PR was merged into the 1.x branch.

Discussion
----------

Updating Recipe Fails When Project Located in Subfolder in Git Repository

Closes #918 and #864

Updating Recipe fails when symfony project is not in the root of git project.
As a developer the most problematic thing is that it fails without any errors. Some files are just not updated.

There is a simple project with commands needed to reproduce to see what is happening before the fix: https://github.com/Peukku/symfony-flex-reproducer-918-project-root/

I think there are two reasons for this to happen:
* $patchString is created with git diff in tmpPath, which has a temporary flat git repository (with no path prefix), but applied bit later in project context where path would be needed.
* Location of .git directory is assumed to be in project root, when it actually is in git root level. (or in .git/modules/[subproject] when using git submodules). The blobs do get written, but in the wrong location.

This PR uses git binary to resolve location of .git folder and current relative path to be used as the prefix for git diff.

Commits
-------

637b769 tweak coding standards
48e5039 Refactored getBlobPath() for clarity, added testApplyPatchOnSubfolder() using same dataprovider as is used for testApplyPatch()
ae675dc Use git binary to resolve location of .git folder and current relative path

Author: fabpot

src/Update/RecipePatcher.php

@@ -83,6 +83,8 @@ public function generatePatch(array $originalFiles, array $newFiles): RecipePatc
             }
         }
 
+        // Use git binary to get project path from repository root
+        $prefix = trim($this->execute('git rev-parse --show-prefix', $this->rootDir));
         $tmpPath = sys_get_temp_dir().'/_flex_recipe_update'.uniqid(mt_rand(), true);
         $this->filesystem->mkdir($tmpPath);
 
@@ -104,7 +106,7 @@ public function generatePatch(array $originalFiles, array $newFiles): RecipePatc
             $this->writeFiles($newFiles, $tmpPath);
             $this->execute('git add -A', $tmpPath);
 
-            $patchString = $this->execute('git diff --cached', $tmpPath);
+            $patchString = $this->execute(sprintf('git diff --cached --src-prefix "a/%s" --dst-prefix "b/%s"', $prefix, $prefix), $tmpPath);
             $removedPatches = [];
             $patchString = DiffHelper::removeFilesFromPatch($patchString, $deletedModifiedFiles, $removedPatches);
 
@@ -167,7 +169,7 @@ private function addMissingBlobs(array $blobs): array
     {
         $addedBlobs = [];
         foreach ($blobs as $hash => $contents) {
-            $blobPath = $this->rootDir.'/'.$this->getBlobPath($hash);
+            $blobPath = $this->getBlobPath($this->rootDir, $hash);
             if (file_exists($blobPath)) {
                 continue;
             }
@@ -192,18 +194,20 @@ private function generateBlobs(array $originalFiles, string $originalFilesRoot):
             }
 
             $hash = trim($this->execute('git hash-object '.ProcessExecutor::escape($filename), $originalFilesRoot));
-            $addedBlobs[$hash] = file_get_contents($originalFilesRoot.'/'.$this->getBlobPath($hash));
+            $addedBlobs[$hash] = file_get_contents($this->getBlobPath($originalFilesRoot, $hash));
         }
 
         return $addedBlobs;
     }
 
-    private function getBlobPath(string $hash): string
+    private function getBlobPath(string $gitRoot, string $hash): string
     {
+        $gitDir = trim($this->execute('git rev-parse --absolute-git-dir', $gitRoot));
+
         $hashStart = substr($hash, 0, 2);
         $hashEnd = substr($hash, 2);
 
-        return '.git/objects/'.$hashStart.'/'.$hashEnd;
+        return $gitDir.'/objects/'.$hashStart.'/'.$hashEnd;
     }
 
     private function _applyPatchFile(RecipePatch $patch)

tests/Update/RecipePatcherTest.php

@@ -233,9 +233,54 @@ public function testApplyPatch(array $filesCurrentlyInApp, RecipePatch $recipePa
         $this->assertSame($expectedConflicts, $hadConflicts);
     }
 
-    public function getApplyPatchTests(): iterable
+    /**
+     * @dataProvider getApplyPatchTests
+     */
+    public function testApplyPatchOnSubfolder(array $filesCurrentlyInApp, RecipePatch $recipePatch, array $expectedFiles, bool $expectedConflicts)
     {
-        $files = $this->getFilesForPatching();
+        $mainProjectPath = FLEX_TEST_DIR;
+        $subProjectPath = FLEX_TEST_DIR.'/ProjectA';
+
+        (new Process(['git', 'init'], $mainProjectPath))->mustRun();
+        (new Process(['git', 'config', 'user.name', 'Unit test'], $mainProjectPath))->mustRun();
+        (new Process(['git', 'config', 'user.email', ''], $mainProjectPath))->mustRun();
+
+        if (!file_exists($subProjectPath)) {
+            mkdir($subProjectPath, 0777, true);
+        }
+
+        foreach ($filesCurrentlyInApp as $file => $contents) {
+            $path = $subProjectPath.'/'.$file;
+            if (!file_exists(\dirname($path))) {
+                @mkdir(\dirname($path), 0777, true);
+            }
+            file_put_contents($path, $contents);
+        }
+        if (\count($filesCurrentlyInApp) > 0) {
+            (new Process(['git', 'add', '-A'], $subProjectPath))->mustRun();
+            (new Process(['git', 'commit', '-m', 'Committing original files'], $subProjectPath))->mustRun();
+        }
+
+        $patcher = new RecipePatcher($subProjectPath, $this->createMock(IOInterface::class));
+        $hadConflicts = !$patcher->applyPatch($recipePatch);
+
+        foreach ($expectedFiles as $file => $expectedContents) {
+            if (null === $expectedContents) {
+                $this->assertFileDoesNotExist($subProjectPath.'/'.$file);
+
+                continue;
+            }
+            $this->assertFileExists($subProjectPath.'/'.$file);
+            $this->assertSame($expectedContents, file_get_contents($subProjectPath.'/'.$file));
+        }
+
+        $this->assertSame($expectedConflicts, $hadConflicts);
+    }
+
+    public function getApplyPatchTests(string $testMethodName): iterable
+    {
+        $projectRootPath = ('testApplyPatchOnSubfolder' === $testMethodName) ? 'ProjectA/' : '';
+        $files = $this->getFilesForPatching($projectRootPath);
         $dotEnvClean = $files['dot_env_clean'];
         $packageJsonConflict = $files['package_json_conflict'];
         $webpackEncoreAdded = $files['webpack_encore_added'];
@@ -393,7 +438,7 @@ public function getIntegrationTests(): iterable
      *      * original_recipe
      *      * updated_recipe.
      */
-    private function getFilesForPatching(): array
+    private function getFilesForPatching(string $projectPath = ''): array
     {
         $files = [
             // .env
@@ -538,7 +583,7 @@ private function getFilesForPatching(): array
         foreach ($files as $key => $data) {
             $files[$key] = array_merge(
                 $data,
-                $this->generatePatchData($data['filename'], $data['original_recipe'], $data['updated_recipe'])
+                $this->generatePatchData($projectPath.$data['filename'], $data['original_recipe'], $data['updated_recipe'])
             );
         }