Updating Recipe Fails When Project Located in Subfolder in Git Repository

[Peukku]

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.

[weaverryan]

Big apologies for the slow review on this! Amazing then, when I hit this problem today, you had a PR ready for this complex problem ❤️ .

I've just tried this code and it works beautifully.

It would also be awesome to add a simple test case for this in https://github.com/symfony/flex/blob/1.x/tests/Update/RecipePatcherTest.php - you could duplicate testApplyPatch, but make it work without a data provider (just initialize the git repo, add a file, then try to apply some simple patch from a sub-directory).

[weaverryan]

Small code simplification. Move $originalFilesGitDir = trim($this->execute('git rev-parse --absolute-git-dir', $originalFilesRoot)); into THIS function, then return:

return $originalFilesGitDir.'/objects/'.$hashStart.'/'.$hashEnd;

If you do this, then the two methods above need zero changes (their diffs can be reverted).

[Peukku]

Hmm, this seems not to work in case of subfolders.
I think the reason is that this getBlobPath() is called from two different places which are refering to different git paths:

• addMissingBlobs() is modifying the current project in $this->rootDir (git rev-parse resulting to either .git or ../.git/modules/projectName)
• generateBlobs() is manually generating the blobs for patch in temp location $tmpPath (git rev-parse always resulting to .git).

I'll try a bit different approach simplifying the code

[Peukku]

Thank you for the feedback!I'm extremely busy this week, but i will continue after that

[weaverryan]

I see some new work on this - awesome :). I don't know if you're finished, yet, but 2 small things:

A) Please rebase the PR to remove the merge commit
B) On fabbot, there is a curl command you can run to tweak any coding standards - d429f654e42ab691c1fb684d99501d8fcf8903d

Thanks!

[Peukku]

Extremely busy week turned into two .. But now I was finally able to continue with this!

I moved the "git rev-parse" into getBlobPath() as you suggested, makes much more sense that getBlobPath() returns the full path. The full blob path consists of two parts; git root path (which can be project repository or the temp location used in creating the patch) and the hash. These are now the arguments for the getBlobPath(string $gitRoot, string $hash).

I also made the duplicate of testApplyPatch; testApplyPatchOnSubfolder. And made it use the same dataprovider as the original one. Same files and patches can be used, but subfolder had to be added in the generated patch.
For that I made the dataprovider getApplyPatchTests to check the name of the test method calling it to prefix the patches with required path.

[weaverryan]

I just tested this locally and it works! I also tested on a traditional project (where the git root is the project root) and there were no problems there either. This should be merged. Great work and big thanks @Peukku 👍

[fabpot]

Thank you @Peukku.