[feature] Automatically recover from trivial merge conflicts in composer.lock
#11517
Conversation
src/Composer/Json/JsonFile.php
Outdated
| if (isset($lines[9]) && strpos($lines[9], '"content-hash": "') !== false && strpos($lines[7], '"content-hash": "') !== false) { | ||
| // We may have found a git merge conflict in composer.lock. Remove the offending lines and try again. | ||
| unset($lines[6], $lines[7], $lines[9], $lines[10]); | ||
| $lines[8] = ' "content-hash": "VCS merge conflict detected. Please run `composer update --lock`.",'; |
There was a problem hiding this comment.
I'm not sure here.. is this better than just picking one? IMO if anything we should rather parse the composer.json to get the current hash and make sure we update the hash in composer.lock, but that will not guarantee that the merge was done correctly in terms of dependencies, and you may still be in an inconsistent state.
Plus you have the problem that git will still mark the file conflicted until you add it again, so all in all I am not sure if this helps more than it causes/hides potential problems.
I'd recommend using https://github.com/balbuf/composer-git-merge-driver if these conflicts are a problem for you.
There was a problem hiding this comment.
I'm not sure here.. is this better than just picking one? IMO if anything we should rather parse the composer.json to get the current hash and make sure we update the hash in composer.lock, but that will not guarantee that the merge was done correctly in terms of dependencies, and you may still be in an inconsistent state.
This string is intentionally not a valid hash. I don't think it will ever be written to disk, but is instead only used in-memory within Composer. Picking either of the two hashes would give the same result (ie, the hash isn't valid), but be somewhat misleading. When running composer install any invalid hash will trigger the "outdated" warning. When running composer update, the correct hash will be calculated and written out.
Plus you have the problem that git will still mark the file conflicted until you add it again, so all in all I am not sure if this helps more than it causes/hides potential problems.
Yes, git will pause with conflicts and require these to be marked as resolved. Nothing's changing in git here. The changes here are an attempt to make the "resolve conflict" process smoother, not bypass it.
I'd recommend using https://github.com/balbuf/composer-git-merge-driver if these conflicts are a problem for you.
Using a git merge driver requires specific set-up, which isn't always compatible with how/where composer runs. Yes, that merge driver looks very useful, but doesn't work when the system PHP isn't compatible with the application being manipulated.
For example, many of the projects that I work on require specific (read: older) versions of PHP with various extensions available (to match production). I use docker to get the correct version of PHP etc, and run Composer in those containers. Running composer update --lock on my host system results in a failure due to the mismatch of PHP versions. I run git on my host system.
Using a git merge driver would require me to configure a different driver for each project so that it could run in the correct environment. This means that the set-up for each would need to be in each individual project. I'm not the only person working on these projects, so anyone else trying to do the same would have conflicting configuration. Anyone without the git merge driver installed may not be able to work on the project trivially. Some of this could be mitigated by using git within the container/environment where PHP works, but this then prevents users from using their much-loved graphical tools for working with git.
I've updated the pull request description to demonstrate the steps in my workflow which this pull request would remove.
There was a problem hiding this comment.
Ok thanks, I guess I'll have to try this out to see a bit clearly more how it behaves.
|
#MarkDown |
c2ebc95 to
dc2c28e
Compare
ad61b4d to
90a49e9
Compare
|
Alright, rebased and added support for extended conflict markers with a cc @TimWolla I'd say it's good to merge now but will leave this for a bit in case anyone wants to review. |
Related to: #6929
This is a very naive approach to try to gracefully handle trivial merge conflicts in
composer.lockfiles. This does nothing for conflicts incomposer.jsonfiles, and does not handle "complex" merge conflicts where more than just thecontent-hashhas conflicted.In the scenario where only the
content-hashhas conflicted, and when using thegitversion-control system*, the change in this pull request is enough to resolve this situation gracefully. A user runningcomposer installin this case will get the expected packages installed (most of the time), and a suitable notice saying that the content-hash is not up to date. A user runningcomposer update --lockin this case will have a "fixed" lock file produced (most of the time).* I've not tested other version control systems. This approach could be adapted to handle other version control systems's merge conflict markers.
This pull request includes tests and an update to the documentation.
Workflow before this pull request
git merge ...results in a conflict incomposer.lock.git diff composer.lockshows that only the content-hash lines are conflicting.composer update --lockhere throws an exception because thecomposer.lockfile no longer contains valid JSON.composer.lockin an editor.content-hashvalue with an invalid hash (egx). This isn't strictly necessary, but makes it clear that Composer did something in the following steps.git diff composer.locknow shows no merge conflict markers are present.composer update --lockto generate a newcontent-hashand validate that the packages are installable together.git diff composer.lockshows that the content-hash line has been updated to be different from both sides of the merge.git add composer.lockgit commit -vas normal, or anothergitcommand to continue the process (eg,git rebase --continue)Workflow after this pull request
git merge ...results in a conflict incomposer.lock.git diff composer.lockshows that only the content-hash lines are conflicting.Note: runningcomposer update --lockhere throws an exception because thecomposer.lockfile no longer contains valid JSON.Opencomposer.lockin an editor.Delete lines 6, 7, 8, 10. This makes the file valid JSON again.(Optional) Replacecontent-hashvalue with an invalid hash (egx). This isn't strictly necessary, but makes it clear that Composer did something in the following steps.Save the file, close editor.Note:git diff composer.locknow shows no merge conflict markers are present.composer update --lockto generate a newcontent-hashand validate that the packages are installable together.git diff composer.lockshows that the content-hash line has been updated to be different from both sides of the merge.git add composer.lockgit commit -vas normal, or anothergitcommand to continue the process (eg,git rebase --continue)