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

Patches defined in dependencies are not applied unless the dependency is updated. #30

Merged
merged 1 commit into from
Aug 9, 2016

Conversation

mathieuhelie
Copy link

I've been struggling to rebuild a project of ours that defines patches for drupal modules in a custom drupal profile. The patch gathering process was only reading patches from the root package and packages that receive an update or install. The modified plugin in this PR stores patches definition in installed.json in $this->installedPatches, then adds them to the patch list unless the package that installed them is itself updated.

@mathieuhelie
Copy link
Author

The way the plugin currently decides whether or not to Uninstall and repatch a package also means that new patches defined by dependencies won't trigger an Uninstall since the updated composer.json of the dependencies hasn't yet been read in the PRE_INSTALL_CMD and PRE_UPDATE_CMD events.

Ideally the uninstalling of patched packages should take place after the dependencies have been resolved, and before the PRE_PACKAGE_INSTALL and PRE_PACKAGE_UPDATE events. I tried the InstallerEvents::POST_DEPENDENCIES_SOLVING event as the trigger but it failed to run, it would probably require substantial refactoring.

@cweagans
Copy link
Owner

Sorry for the slow response on this. I'm not 100% sure I understand the problem you're running into. Could you please provide a composer.json that causes the problem, along with any dependencies' composer.json and walk me through what you're seeing? If it's easier, I'm happy to jump on a Join.me with you or something.

@mathieuhelie
Copy link
Author

So our composer.json triggering the issue are pretty complicated, but here's an example scenario:

Root package

  • requires ourvendor/ourprofile
  • requires drupal/views ~7.3.1
  • adds patch A for drupal/views

Package ourvendor/ourprofile

  • requires drupal/views ~7.3.1
  • adds patch B for drupal/views

If running a composer update on our root package, composer may detect a version increase in drupal/views and trigger an update command on the package. The patches plugin will then gather patches from the root composer.json but will skip the patches from ourvendor/ourprofile because as far as composer tells it there has been no change in that package and it is irrelevant.

Since composer stores the composer.json for ourvendor/ourprofile in installed.json, we can gather all the patches for drupal/views from that source.

@chrfritsch
Copy link

This is exactly what we urgently need for our drupal thunder distribution

@grasmash
Copy link
Contributor

grasmash commented Aug 3, 2016

This is the cause of #26.

@cweagans
Copy link
Owner

cweagans commented Aug 9, 2016

👍

@cweagans cweagans merged commit ee6a6d0 into cweagans:master Aug 9, 2016
@grasmash
Copy link
Contributor

grasmash commented Aug 9, 2016

I have tested that this works on initial install, update, and with successive updates. It appears to work with root patches as well as patches that belong to dependencies.

@anotherjames
Copy link
Contributor

I honestly don't know why, but I still have the same problem. I'm using commit ee6a6d0 of this plugin. I had 2 patches listed for a required package (config_devel drupal module, as it happens, if that makes any difference). I added a 3rd patch, and ran composer update. The package was removed, and not reinstalled. I ran composer update a 2nd time, this time it was reinstalled and patched with all 3 patches.

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

Successfully merging this pull request may close these issues.

5 participants