Updating Recipe Fails When Project Located in Subfolder in Git Repository#937
Conversation
weaverryan
left a comment
There was a problem hiding this comment.
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).
| $hashEnd = substr($hash, 2); | ||
|
|
||
| return '.git/objects/'.$hashStart.'/'.$hashEnd; | ||
| return 'objects/'.$hashStart.'/'.$hashEnd; |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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
|
Thank you for the feedback!I'm extremely busy this week, but i will continue after that
|
|
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 Thanks! |
…() using same dataprovider as is used for testApplyPatch()
3d429f6 to
637b769
Compare
|
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. |
weaverryan
left a comment
There was a problem hiding this comment.
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 👍
|
Thank you @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:
This PR uses git binary to resolve location of .git folder and current relative path to be used as the prefix for git diff.