Skip to content
This repository was archived by the owner on Nov 19, 2024. It is now read-only.

HowDoI: Create a tutorial for patching a composer project from github #1956

Closed
wants to merge 10 commits into from
Closed

HowDoI: Create a tutorial for patching a composer project from github #1956

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9a2e929
HowDoI: Create a tutorial for patching a composer project from github
dmanners May 23, 2018
54c4a03
Editorial revisions from tech writer
jeff-matthews May 23, 2018
ed1f664
Move to 2.1 and symlink to 2.2 and 2.3
jeff-matthews May 23, 2018
b10c253
Move to 2.1 and symlink to 2.2 and 2.3
jeff-matthews May 23, 2018
e358e87
Add topic to the left navigation
jeff-matthews May 23, 2018
47bd878
Prepend nav link with forward slash so it doesn't break when we merge…
jeff-matthews May 23, 2018
43fcab6
Revised per peer review
jeff-matthews May 23, 2018
f353f52
Revisions based on initial review feedback
jeff-matthews May 31, 2018
c52502b
Correct relative path info
jeff-matthews Jun 4, 2018
a775db1
Replaced patch with diff example
jeff-matthews Jun 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions _data/toc/how-do-i.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ pages:
- label: Contributor sign-up sheet
url: howdoi/howdoi_contribute.html

- label: Create a GitHub patch for a Composer installation
versions: ["2.1","2.2", "2.3"]
url: /howdoi/patch_from_github.html

- label: Configure Magento
url: howdoi/config/configure.html

Expand Down
61 changes: 61 additions & 0 deletions guides/v2.1/howdoi/patch_from_github.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
---
group: howdoi
title: Create a GitHub patch for a Composer installation
version: 2.2
github_link: howdoi/patch_from_github.md
functional_areas:
- Install
- System
- Setup
---

Sometimes it takes a while for us to include a bug fix made on GitHub in a Magento 2 Composer release. In the meantime, you can create a patch from GitHub and use the `composer-patches` plugin to apply it to your Composer-based Magento 2 installation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You sometimes see devs complaining about not wanting to install a patch but prefer to build a custom M2 module which fixes the bug by rewriting the original code.

This is a very personal opinion, but I think that using a patch is better in most cases, because if you upgrade Magento to a new version and it contains a fix for what you patched, composer will start complaining that it can no longer apply the patch. So you'll automatically go looking for a reason why it doesn't apply and you'll find that it was fixed in this particular new version and you can then remove the patch.
You wouldn't get notified about that if you use a custom build module. So you would have to keep that in mind to check all your custom modules with fixes after every Magento update.

Not sure if this makes a good addition to this page though, since it's a very personal opinion.

I would also add a link to the cweagans/composer-patches module, since its readme file contains useful information which explains all the options you can use: https://github.com/cweagans/composer-patches/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's great context. I'm not sure if it's necessary yet, but I may add it later. I'll definitely add a link to the package.

<div class="bs-callout bs-callout-warning" markdown="1">
Always perform comprehensive testing before deploying any unreleased patch.
</div>

## Create a patch

To create a patch file from a GitHub commit or pull request, append `.patch` to the url, [https://github.com/magento/magento2/commit/2d31571f1bacd11aa2ec795180abf682e0e9aede.patch](https://github.com/magento/magento2/commit/2d31571f1bacd11aa2ec795180abf682e0e9aede.patch).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This URL shows you the patch file, but doesn't actually create it for your purposes. I think you copy the contents, and paste it into the file described in line 31.

When take the link, I see this at the top:

From 2d31571f1bacd11aa2ec795180abf682e0e9aede Mon Sep 17 00:00:00 2001
From: Ievgen Sentiabov <[email protected]>
Date: Wed, 31 Aug 2016 18:46:44 +0300
Subject: [PATCH] MAGETWO-56934: Checkout page freezes when ordering with
 Authorize.net with invalid credit card

 - Added fail() method call
---
 app/code/Magento/Payment/view/frontend/web/js/view/payment/iframe.js | 1 +
 1 file changed, 1 insertion(+)

Does any of this need to be deleted? It's certainly not essential, but maybe the composer-patches tool knows to ignore it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmanners, deferring to you on this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, you can also add .diff to the url which results in a smaller file then the .patch variant.
But both work absolutely fine. People should pick the one they like best.

And yes, it's best if you then download that file and put it in the directory mentioned below: patches/composer (this directory is very arbitrary btw, you can choose which one you want to use yourself, not sure if this needs to be mentioned?)

Some other additions here:

  • Sometimes a PR doesn't contain the full fix, because sometimes a member of the magento community engineering team adds some extra code to the PR. If you would then add .patch or .diff to a PR, you won't get the full fix.
    An example of this is this PR: declare var to fix scope error magento2#15265, the correct fix is the commit mentioned at the very bottom (magento/magento2@d9dc66c) instead of the changes of the PR.

So maybe we'll need to add some section about how you find the correct commit (or even multiple commits) for a certain fix. Which will probably be pretty complex to write down...

  • If a fix is old, it's hard to find a single merge commit with only a fix for that problem, this is due to how the magento developers used to merge branches, this has improved more in recent months, and you'll usually find a single commit you can use for a patch.
    But if you can't find a single commit and you have multiple commits, what I usually do then is:

    • clone the Magento 2 repo
    • checkout the version I want to create a patch for
    • cherry-pick all commits you want to apply (and solve merge conflicts if they arise)
    • do a git diff 2.1.4...HEAD > my-patch-file.diff (where 2.1.4 is the version used in the second step)

  • It also will regularly happen that a commit won't apply cleanly to a particular version of Magento, happens most because there was some other code changed in another commit which is missing in that particular version of Magento somewhere around the same lines of that patch.
    If this happens, I also follow the 4 steps outlined above to create a new patch, but you will certainly have to solve merge conflicts in that case.

Feel free to ignore these things above if this would make the documentation more complex then it needs to. It might scare users or confuse them ...


<div class="bs-callout bs-callout-info" markdown="1">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Give an example of what needs to be changed. Would you change every instance of app/code/Magento/ to vendor/Magento?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmanners, deferring to you on this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, example would be very useful here.

BTW: the cweagans/composer-patches package's original intent was to apply patches relative to the root of the package, not to the root of the application. That particular "feature" was added afterwards, but I'm considering it a hack and never used it. So we always change the paths to the relative path of the module itself.

So in this specific case, you would replace all instances (5 times) of app/code/Magento/Payment/view/frontend/web/js/view/payment/iframe.js with view/frontend/web/js/view/payment/iframe.js

(But if it would be considered easier to understand, then just replace app/code/Magento/Payment/ with vendor/magento/module-payment/ like was mentioned below on line 29)

Btw, maybe good to mention as a warning: you shouldn't make these changes with an actual code editor, since they sometimes strip out trailing whitespace or adding new lines depending on how they are configured, which can sometimes mess up the patch file. I prefer a very old school editor which doesn't do any fancy stuff like that. On macOS I'm using the built-in TextEdit application for this. For Windows, I'm assuming you can use the built-in Notepad? And for Linux, just vim or nano probably?

You must change the paths in the patch file to correspond to the `vendor/***` directories.
</div>

## Apply a patch

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to see more of an intro paragraph.

The following example applies the X patch to ....

1. Create a `patches/composer` directory.
1. Prepare your patch file so that the paths are relative to the `vendor/<VENDOR>/<PACKAGE>` directory. For example `vendor/magento/module-payment`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment above about original intent of the cweagans/composer-patches package.

1. Name the patch file appropriately and move it to the `patches/composer` directory.
2. Add the `cweagans/composer-patches` plugin to the `composer.json` file.

```bash
composer require cweagans/composer-patches
```

1. Edit the `composer.json` file and add the following section, specifying the Composer package to apply the patch to, as well as a description of the patch and a reference to the file location:

```json
"extra": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the composer.json file contains an extra block already, do you empty it out, create a new block, or merge the info?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmanners, deferring to you on this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should merge the lines unless it doesn't already exist, then you have to create one.

"magento-force": "override",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove this magento-force setting here, it has nothing to do with the patches and it is already added by default in your Magento installation.

"composer-exit-on-patch-failure": true,
"patches": {
"magento/module-payment": {
"MAGETWO-56934: Checkout page freezes when ordering with Authorize.net with invalid credit card": "patches/composer/github-issue-6474.patch"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, it looks like there are 3 key elements that need to be called out:

Affected module: "magento/module-payment"
PR title: MAGETWO-56934: Checkout page freezes when ordering with Authorize.net with invalid credit card
Path to patch: patches/composer/github-issue-6474.patch

Line 31 should give an indication of what an appropriately named patch would be.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmanners, deferring to you on this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct!

The title is also arbitrary here, you can put in it what makes sense to you.

Also, if multiple modules are involved with a patch file, you need multiple patch files targeting multiple modules.
Maybe an example can be added as well for this case. Here is one which targets both the Quote and the Sales module: magento/magento2#11702

}
}
}
```

2. Apply the patch.

```bash
composer install
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a patch can't be applied, composer will complain with an error message: Could not apply patch! Skipping. The error was: Cannot apply patch patches/composer/github-issue-6474.patch

This is not very helpful, so sometimes it helps if you could get some more debugging output about why a patch doesn't want to apply.
And you can get more debugging output by adding the -v flag to composer install, so like this: composer -v install

This can maybe be added to some troubleshooting section?


1. Update the `composer.lock` file. The lock file tracks which patches have been applied to each Composer package in an `extra > patches_applied` object.

```bash
composer update <PACKAGE NAME>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer composer update --lock which updates nothing but just rewrites the composer.lock file with the correct information. Since adding patches can sometimes contain fixes for multiple modules, this is probably an easier command to use?

```
1 change: 1 addition & 0 deletions guides/v2.2/howdoi/patch_from_github.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../../v2.1/howdoi/patch_from_github.md
1 change: 1 addition & 0 deletions guides/v2.3/howdoi/patch_from_github.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../../v2.2/howdoi/patch_from_github.md