-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[feature] Automatically recover from trivial merge conflicts in composer.lock
#11517
base: main
Are you sure you want to change the base?
Conversation
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok thanks, I guess I'll have to try this out to see a bit clearly more how it behaves.
#MarkDown |
Related to: #6929
This is a very naive approach to try to gracefully handle trivial merge conflicts in
composer.lock
files. This does nothing for conflicts incomposer.json
files, and does not handle "complex" merge conflicts where more than just thecontent-hash
has conflicted.In the scenario where only the
content-hash
has conflicted, and when using thegit
version-control system*, the change in this pull request is enough to resolve this situation gracefully. A user runningcomposer install
in 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 --lock
in 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.lock
shows that only the content-hash lines are conflicting.composer update --lock
here throws an exception because thecomposer.lock
file no longer contains valid JSON.composer.lock
in an editor.content-hash
value 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.lock
now shows no merge conflict markers are present.composer update --lock
to generate a newcontent-hash
and validate that the packages are installable together.git diff composer.lock
shows that the content-hash line has been updated to be different from both sides of the merge.git add composer.lock
git commit -v
as normal, or anothergit
command to continue the process (eg,git rebase --continue
)Workflow after this pull request
git merge ...
results in a conflict incomposer.lock
.git diff composer.lock
shows that only the content-hash lines are conflicting.Note: runningcomposer update --lock
here throws an exception because thecomposer.lock
file no longer contains valid JSON.Opencomposer.lock
in an editor.Delete lines 6, 7, 8, 10. This makes the file valid JSON again.(Optional) Replacecontent-hash
value 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.lock
now shows no merge conflict markers are present.composer update --lock
to generate a newcontent-hash
and validate that the packages are installable together.git diff composer.lock
shows that the content-hash line has been updated to be different from both sides of the merge.git add composer.lock
git commit -v
as normal, or anothergit
command to continue the process (eg,git rebase --continue
)