Add ability to apply patches after "composer install" about capistrano-magento2 HOT 14 CLOSED

If you are considering applying patches after pulling in code from composer, I'd suggest committing your vendor folder and applying your patches there. I think this would be the lesser of two evils. If you are aghast at the idea of committing a vendor folder, ask yourself why and do not accept anything with the words "everyone" or "always" in it.

from capistrano-magento2.

@danslo Not sure @erikhansen knows this or not, but we at CL use per-project rake files for SCSS compilation on a few projects, and recently this very thing as well. And I feel I can pick on him a little bit since he sits across the wall from me ;)

I recently setup an M2 project config with the following to apply patches after getting some launch-critical patches from Magento Support. It appears to be working very well:

lib/capistrano/tasks/patches.rake

after 'magento:composer:install', 'magento:apply:patches' do
  on release_roles :all do
    within release_path do
      output = capture :find, "patches -maxdepth 1 -type f -name *.patch || true"
      output.split("\n").each do |patch_file|
        execute :git, "apply -v --directory vendor #{patch_file}"
      end
    end
  end
end

@danslo Is there a particular reason you think this should not be in this gem? It seems to me to make a lot of sense as it keeps the core logic of something common to Magento 2 deployments out of the project specific configuration, leaving the project to just that, configuring what it needs. I'm open to any thoughts you might have.

This is what I have in mind here:

1. Add a task to apply .patch files found in the patches directory in a separate patches.rb

2. Projects which needed this functionality would include it by including the following line in the project Capfile:

   require 'capistrano/magento2/patches'

3. A new var :magento_deploy_patches_dir which defaulted to patches/ in the project repository. Projects could override this if one desired to place patch files in a different location.

from capistrano-magento2.

I'm not exactly sure about this either. Thinking about it: applying a patch means changing your code, and changing code belongs in version control. I don't particularly like having my codebase change as part of a build step, especially when they are prone to failure (will this patch apply, without human intervention?). Also considering it takes ~10 lines of ruby for those that do appreciate such a feature.

Then again, not my project -> not my rules. I'm fine with whatever decision is made here.

from capistrano-magento2.

@davidalger I had the need to apply a patch after deploying an M2 site using Cap, so I decided to test out the https://github.com/cweagans/composer-patches plugin. Some notes:

1. Applying patches from the filesystem/Git repo worked well
2. Patches can only be applied to specific Composer packages, so if you want to apply a patch that spans multiple Magento extensions, you have to split the patch into multiple files. It looks like the Composer plugin maintainer has no plans to add support for being able to patch the root directory (see this), although he might reconsider if he understood the Magento 2 patching use case and saw there was a demand for it.
3. After adding a patch for the magento/module-catalog module and running composer install locally, the patch was immediately applied:
   
4. If a patch cannot be applied, the Composer plugin notifies the user, but it doesn't cause composer install to return a non-zero exit code. In my mind, if patches can't be successfully applied, the Capistrano deployment should fail. Something to be considered if we decide to use this plugin in lieu of implementing patching directly within capistrano-magento2.
   

Overall I'm optimistic about using the composer-patches plugin instead of implementing patch support to capistrano-magento2—since it would handle applying patches to local development environments, it seems like this solution is a more holistic one. If you agree with this, I'll see if the composer-patches maintainer would be willing to add "root patching support", which would eliminate the primary downside to this approach.

from capistrano-magento2.

Committing the vendor directory adds tremendous bulk to your git repository in addition to a large amount of noise to diffs.

Committing patches provide a clean and safe way to change vendor code and gives you an explicit list of all core code changes you've made for that specific project. This makes life much easier, especially when upgrading to a new version, which would override your committed vendor folder changes.

from capistrano-magento2.

I'm not sure if that functionality should belong in this repository. However, it's quite easy to make your own capistrano task and have it executed after certain deployment steps. For example, we use gulp to compile SASS files in our build process and it goes something like this:

lib/capistrano/tasks/gulp.rake:

namespace :magento do
  task :gulp do
    on release_roles :all do
      within release_path.join('dev/tools') do
        execute 'yarn', 'install'
        execute './node_modules/.bin/gulp'
      end
    end
  end
end

And then we register it to be executed after deploying static content:

config/deploy.rb:

[...]
after 'magento:setup:static-content:deploy', 'magento:gulp'

You could make something similar for applying patches :)

from capistrano-magento2.

@danslo Fair point. I did consider just implementing this as a Capistrano task that is committed to the repos of my projects that need patch support. However after talking about this with the senior engineers at Classy Llama, there was a consensus that this is something that would be useful across multiple projects.

@davidalger Let me know your thoughts.

from capistrano-magento2.

@davidalger I saw that nifty patch file you created last week, but I haven't seen any of the M1 SCSS rake files.

I think your proposal sounds great. In this issue's description, I proposed adding support for applying shell scripts like the ones provided by Magento Enterprise support, but it would be easy enough to extract the content below the __PATCHFILE_FOLLOWS__ line in those files into their own .patch files. And automatically running shell scripts could be abused.

I would think requiring explicit inclusion of this feature via require 'capistrano/magento2/patches' should alleviate @danslo's concerns.

from capistrano-magento2.

Just FYI, we recently started using this composer plugin: https://github.com/cweagans/composer-patches, which allows us to add patches using composer's extra section. This just works when you call composer install or composer update and there is no need for an additional step in the deployment process.

Maybe this helps here?

from capistrano-magento2.

@hostep Thanks for sharing. I assume this solution also works well on local development environments where Capistrano is not being used?

The examples in the readme demo patches being applied from a web location—do patches from the local file system (relative to the Composer root) work just as well?

from capistrano-magento2.

It looks like the composer-patches plugin already supports failing when patches can't be applied. You just have to set a COMPOSER_EXIT_ON_PATCH_FAILURE env variable: https://github.com/AmazeeLabs/composer-patches/blob/master/src/Patches.php#L273

There is also a pending PR that would allow you to add composer-exit-on-patch-failure to the extra object in composer.json to enable this behavior.

from capistrano-magento2.

@davidalger FYI - the owner of the composer-patches plugin is open to accepting a PR to add support for patching at the root level. See cweagans/composer-patches#76 (comment)

from capistrano-magento2.

Or add your behaviour to your composer.json file using:
"scripts": {
"post-install-cmd": [
]
}

from capistrano-magento2.

I agree with @jaywilliams.

I've been successfully using the composer-patches plugin for the majority of my patching needs over the past months. I've also used the rake file that @davidalger shared above. I'm going to close this issue since I no longer feel there is a need to have this issue addressed in the capistrano-magento2 gem.

from capistrano-magento2.