Skip to content
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

Composer patches only respects composer.json #12

Closed
pingers opened this issue Aug 6, 2015 · 7 comments
Closed

Composer patches only respects composer.json #12

pingers opened this issue Aug 6, 2015 · 7 comments

Comments

@pingers
Copy link
Contributor

pingers commented Aug 6, 2015

When running composer install, I expect the composer.lock file to be respected, but instead, the patches defined in composer.json are always used. This isn't critical by any stretch, but it's not consistent with the way composer works.

Proposal:

  • Respect composer.json during composer update
  • Respect composer.lock during composer install
@cweagans
Copy link
Owner

cweagans commented Aug 8, 2015

👍 Yeah, I agree. This makes a lot of sense. I don't have a lot of time to work on this right now, but if you want to submit a PR, I'd be more than happy to merge it. It should be pretty straightforward.

@cspitzlay
Copy link

I think this behaviour is also at the bottom of an issue we are having.
Our setup uses both cweagans/composer-patches and wikimedia/composer-merge-plugin.
We have several composer.json files in subdirectories that are merged together by the merge plugin. These composer.jsons may contain patches.

We're seeing that patches are applied on composer update but not on composer install.
I guess the problem is that the logic to merge the composer.jsons does not run
on composer install, hence no patches from the subdirs are found.

Our current workaround is to move all the patches to the toplevel composer.json.
This seems to be the only file the patches plugin looks at for gathering patches.
This workaround is a bit ugly as it breaks the modularity we tried to achieve.

IIUC a composer.lock file is supposed to define the whole installation including versions (and by extension patches) and thus should be self-contained.
I think that fixing this issue here would be a step in that direction (and at the same time fix the incompatiblity with the merge plugin, too).
This makes me think that the patches plugin is the right place to fix this (instead of making the merge logic run on composer install which may be another way to solve this).

@whittaker007
Copy link

Sorry to reply on a closed issue, but I'm having the same issue as @cspitzlay above - that is we are using wikimedia/composer-merge-plugin so we can have a standard composer.json for certain kinds of project, and an instance-specific composer.local.json.

If we add patches to requirements in the composer.local.json they install fine, but building a new site instance using composer install with the current composer.lock (which does appear to contain references to the patches) the patches are not installed.

Manually running "composer update" for the requirement does not apply the patches on the new instance. The only workaround I've discovered to work reliably is to delete the destination folder which contains the requirement being patched and run "composer install" again.

I notice that this issue has been closed, but this particular issue doesn't seem to be resolved. Is the fix in the coming 2.x branch?

@cweagans
Copy link
Owner

cweagans commented Mar 2, 2018

This issue is still open :)

@cweagans cweagans mentioned this issue Jun 2, 2018
36 tasks
@bkosborne
Copy link

I think this is critical for anyone using the composer merge plugin. What's really strange is we can get it working correctly only if we manually remove the "patches_applied" section from the composer.lock file for each package that was patched from one of the merged composer.json files. If we do that, then when composer install is run, it will actually re-examine the composer.json file(s) for patches and include them.

So our current workaround is very hacky - we have to remember to manually remove the references made in composer.lock when we include new patches.

@eelkeblok
Copy link

Although this makes a lot of sense, what would be the workflow for just applying a patch to the current version of a dependency, without actually updating the "base" version of the dependency? Currently, that just requires adding the patch to composer.json and rerunning composer install.

I actually came here looking for an issue about composer.lock not being updated when you run composer install with a new patch, and having to run composer update --lock to get it to be added correctly.

Oscaner added a commit to ciandt-china-dev/composer-patches that referenced this issue Nov 24, 2020
Oscaner added a commit to ciandt-china-dev/composer-patches that referenced this issue Nov 24, 2020
Oscaner added a commit to ciandt-china-dev/composer-patches that referenced this issue Nov 24, 2020
Oscaner added a commit to ciandt-china-dev/composer-patches that referenced this issue Nov 24, 2020
Oscaner added a commit to ciandt-china-dev/composer-patches that referenced this issue Nov 24, 2020
Oscaner added a commit to ciandt-china-dev/composer-patches that referenced this issue Nov 25, 2020
Oscaner added a commit to ciandt-china-dev/composer-patches that referenced this issue Jun 9, 2021
Oscaner added a commit to ciandt-china-dev/composer-patches that referenced this issue Jun 11, 2021
Oscaner added a commit to ciandt-china-dev/composer-patches that referenced this issue Jun 11, 2021
@cweagans
Copy link
Owner

cweagans commented Feb 7, 2023

It's not composer.lock because that isn't super feasible, but main has a lock file now.

@cweagans cweagans closed this as completed Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants