-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
RFC: Automatically resolve conflicts in composer.lock #6929
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
Comments
Maybe not very clean but - separate folder like '.composer.lock' having files separate per library or just remove content-hash. In bigger projects with large teams, this becomes a real pain. |
@adanilowski I think this is quite a drastic change compared to how the lock file currently works and is a breaking change. My suggestion is around the current implementation and it is similar to how other package managers work. I really appreciate having all dependencies locked into a single file and the hash of dependencies for the warnings it produces. |
The content-hash serves a real purpose and removing it is not a solution, it might resolve some merge conflicts but would introduce other issues. Adding a merge functionality that checks the two lock files and sees if they are compatible and still match the current requirements though sounds like something that's feasible and definitely should be looked. |
This will be harder than in Yarn though, because our composer.lock file is a JSON file, but a merge conflict makes it unparseable, and so would require to build a special parser for conflicting files (not impossible though). |
I suppose we need to detect just the following:
If it is, we just need to apply a merge strategy to choose one of the sides without parsing the JSON (it doesn't matter which is in this case). Then run |
Maybe composer could also automatically inject git hook to run this post merge? |
I have created issue WI-39466. In PHPStorm tracker, I think solving this problem in editor might be more proper place. |
I have created a custom git merge driver that can be used to minimize merge conflicts in https://github.com/balbuf/composer-git-merge-driver Please check it out! |
thanks @balbuf, I also think that handling it in as git merge driver is the way to go. I was about to write my own, but luckily found this issue thread first! |
@Seldaek would you accept a PR which incorporates https://github.com/balbuf/composer-git-merge-driver into the composer codebase (solving the problem vcs independently)? IMO solving merge conflicts of composer.json/lock is pain point for composer usage and should not be something the composer user should need additional setup for (setup the git merge-driver etc). I guess a lot of composer users dont know git merge drivers at all and therefore deal with those problems manually. additionally the git merge driver thing doesnt work for mercurial or svn etc. |
@staabm tbh I am medium interested in spending time on this right now (even if you do a PR..) as the way I understand it it still require setting up the git merge driver and so most people still won't benefit from it. Or how do you imagine this? |
I think he means solving this problem without the user having to set up anything / not depending on the version control system used. |
@vierlex how would that work ? The previous discussion concluded that things are much easier to handle when implement a git merge driver than when trying to resolve conflicts after running the default git merge. You are asking to give up that solution. So what would you suggest otherwise ? |
Having the lock data in a directory with distinct file per package could be opt-in, then we would not have to worry about BC as much. Imo these files could also be more slim, I think the version number is really the only relevant piece of information, right? Perhaps if it was downloaded from an alternative repo.. |
The secure solution when having conflicts is to keep other developpers changes, then you replay your changes on top of this. |
I came here to also suggest this :) My problem is usually:
I usually solve this by removing composer.lock and running composer install/update. But that unavoidably updates more stuff than I want to - because when using loose constraints like ^1.0 of a library which version I didn't want to touch, it will also update that library. It looks like Composer could use the current |
I'm also aware this can be solved with a little bit of git-fu:
I wonder if there's a way for Composer to do the same thing out of the box... |
That's wrong. the composer.lock file is designed so that an install from lock has all the necessary metadata to run the solver and the installer. It will not load metadata from Packagist anymore (which is fortunate, otherwise locking dev versions would not actually lock them as their metadata gets updated to newer commits on packagist). |
But could this not be cached in some place that is not tracked with git? Either on system level or on project level.
So are we afraid that a package maintainer would force-push and change the metadata for a specific release tag? Or what else could cause the upstream metadata to change, if we target a specific version number?
if we target a specific version number, then newer commits should not matter, right? unless there is a force-push. |
Even without a force push, git allows deleting a tag and publishing the same version on a new commit. Also, force pushes in libraries do happen. But I think this is beside the point in this conversation. This is mostly about multiple developers working on the same project having conflicts in |
I've been using the Composer Git merge driver for some time and it works well for simple cases. It however causes more confusion with more complex cases and it can lead you to think things are OK when they are not. For example, when there are new and removed libraries between the two versions of composer.lock touching the same lines. If this could be improved in a Git merge driver and if that could be installed in one step, I'm all for resolving it as a Git merge driver instead of as part of Composer. If these cases require an in-house handling in Composer, let's discuss that. In terms of developer experience, I particularly like the Yarn approach as you'd normally need to run If the capabilities of a Git merge driver are limited by Git, we could also handle that as a Composer plugin hooking to To summarise - potential approaches I can see: A. Git merge driver for Composer - needs improvements on installation and complex cases. Which one do you see most viable and acceptable to Composer core? /cc @Seldaek @balbuf |
being tracked by git is precisely what allows reproducible installs in all environments. Other environments must have the metadata too.
This one might be hard to implement, because composer loads the composer.json file quite early (as it contains some config stuff), and would fail there in case of an invalid file.
For that, I suggest opening issues on the merge driver repo for the different cases causing issues. |
Conflicts in |
Right. Conflicts on the lock file should be something that a plugin can handle. |
@hkdobrev, echoing @stof, go ahead and open an issue on the repo if you can (and even better if you can open a PR, because I'm not a full-time dev anymore and don't have a lot of bandwidth to work on these projects). But re:
I'm not sure why there would be any issue here, because this was the primary use-case I had in mind when writing the merge driver. The merge driver parses the JSON files and merges the contents as assoc arrays, rather than (and having no concept of) lines from the files. As far as comparing lockfile changes, packages are keyed by package name and compared rudimentarily as JSON encoded strings before being converted back. The only issue I'm aware of is an edge case where you can be left with invalid JSON file because of an errant comma after resolving a conflict at the end of the package list, as described here: balbuf/composer-git-merge-driver#14 |
And just to clarify, just because you are left with merge conflicts after the driver runs doesn't mean the driver is not properly handling complex cases: when there are merge conflicts that means (or should mean) that a given package was changed in multiple development histories involved in the merge. The driver doesn't know which of the two legitimate, conflicting changes of a given dependency you'd like to keep, thus you must manually resolve them. However, the advantage of this merge driver over the line-based comparison of the default merge driver is that the conflicts are much easier to resolve because the markers are placed around the whole chunk of JSON corresponding to the conflicting versions of a given package. The readme touches on what I've described above:
(Forgive me if this is not the situation you are alluding to--but perhaps someone else reading this will find it useful to understand what the merge driver does and doesn't do. There, of course, remains a distinct possibility that there is a bug in which certain cases are not handled properly.) |
What I've been recently doing to overcome composer.lock conflicts in feature branches is:
If Composer would let me do this in an easier way without doing all the git-fu, that would be great! |
Maybe I'm missing something but to me it sounds like one of the issues is that we have a single file (
Would it be an option to extract 3. into a separate file? This file could also be added to VCS. If that was not done, intentionally or not, Composer may to download the metadata of all installed packages. This way a file containing 1. and 2. may have little to no risk of having conflicts in the mentioned scenarios. More things to consider:
Disclaimer: this may oversimplify things given that I know little about the Composer internals. But maybe it is food for some thoughts. |
@mbrodala the composer.lock is not about the installed packages. It is all about the resolved packages. Storing the metadata of resolved packages is what allows to do a
An install from lock does not load the repositories (at least not the Composer repositories like packagist.org where Composer can ask the repository for metadata about a specific package. For VCS repositories, I'm not sure it is skipped entirely due to the fact that composer itself is the one extracting the metadata from the VCS to know what is inside the repository).
Yarn moved away from their custom format in Yarn 2+ and uses Yaml instead. |
Thanks for the info. (And yes, we still use Yarn 1.x 😅 ) So maybe rephrased: does Composer really need the whole content of the |
As said before, the |
Uh oh!
There was an error while loading. Please reload this page.
When merging/rebasing branches it's not that rare that you may encounter merge conflicts in
composer.lock
. Most of the time the conflict would be in the hash or in a timestamp or in some cases you'd need to choose both blocks on the same lines.However, it's quite rare you'd actually need to manually resolve something in
composer.lock
in a custom way.The usual workflow:
composer.lock
git-mergetool
and chose one of the hashes - doesn't matter which one.composer update --lock
git add composer.lock
git rebase --continue
orgit commit
(if you're merging).How about automatically fix the merge conflicts when running
composer install
orcomposer update --lock
if possible?This was already implemented in Yarn:
What do you think?
The text was updated successfully, but these errors were encountered: