Use patch command at first #82

iGerchak · 2020-12-21T15:02:50Z

Description

On Magento 2.3.6 Open Source we had an issue when the patch MDVA-31168-V2 was silently failing with no changes in code. During debug session together with @ihor-sviziev we found that it executes the following command:

cat - <<'EOF' | git apply
diff --git a/vendor/magento/module-downloadable-import-export/Model/Export/RowCustomizer.php b/vendor/magento/module-downloadable-import-export/Model/Export/RowCustomizer.php
index daa874e829e..3b805fef734 100644
--- a/vendor/magento/module-downloadable-import-export/Model/Export/RowCustomizer.php
+++ b/vendor/magento/module-downloadable-import-export/Model/Export/RowCustomizer.php
@@ -82,7 +82,7 @@ class RowCustomizer implements RowCustomizerInterface
             ->addAttributeToSelect('samples_title');
         // set global scope during export
         $this->storeManager->setCurrentStore(Store::DEFAULT_STORE_ID);
-        foreach ($collection as $product) {
+        foreach ($productCollection as $product) {
             $productLinks = $this->linkRepository->getLinksByProduct($product);
             $productSamples = $this->sampleRepository->getSamplesByProduct($product);
             $this->downloadableData[$product->getId()] = [];
EOF

After adding -v argument - we found the following error, with 0 return code:
Skipped patch 'vendor/magento/module-downloadable-import-export/Model/Export/RowCustomizer.php'.

During some googling, I found that sometimes it's happening and it's recommended to use patch command instead:
https://stackoverflow.com/questions/42386491/git-apply-skips-patches

So my suggestion is to use patch command at first with a fallback to the git apply.

Note: git --version output:
git version 2.20.1

Also reproducting on git version 2.27.0

Fixed Issues (if relevant)

magento/magento-cloud-patches#<issue_number>: Issue title
...

Manual testing scenarios

...
...

Release notes

For user-facing changes, add a meaningful release note. For examples, see Magento Cloud Patches release notes.

Associated documentation updates

Add link to Magento DevDocs PR or Issue, if needed.

Contribution checklist

Pull request has a meaningful description of its purpose
Pull request introduces user-facing changes and includes meaningful updates for any required release notes and documentation changes
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)

ihor-sviziev · 2020-12-22T12:46:09Z

@oshmyheliuk could you review this PR?

bubasuma · 2020-12-23T17:38:39Z

Hi @iGerchak
I couldn't reproduce this issue in 2.3.6. I applied the patch using command ./vendor/bin/magento-patches apply MDVA-31168-V2. The patch was successfully applied and I could see the changes applied in /vendor/magento/module-downloadable-import-export/Model/Export/RowCustomizer.php:85.

Could you please help us to reproduce this issue?

Here is my concern about the proposed solution

There is no guarantee that there won't be other issues with PATCH and we won't be switching back and forth. Moreover, I don't know any reliable resources that suggest PATCH over GIT for applying patches. Maybe we should think of making it configurable (e.g using environment variable). Perhaps, we should handle this specific issue in GitDriver and consider it as a failure so it will fallback to PATCH?

ihor-sviziev · 2020-12-23T18:21:52Z

Hi, We were having issue with magento installation that is installed through composer and added to git repository. Please check that

…

On Wed, 23 Dec 2020 at 19:38 Buba Suma ***@***.***> wrote: Hi @iGerchak <https://github.com/iGerchak> I couldn't reproduce this issue in 2.3.6. I applied the patch using command ./vendor/bin/magento-patches apply MDVA-31168-V2. The patch was successfully applied and I could see the changes applied in /vendor/magento/module-downloadable-import-export/Model/Export/RowCustomizer.php:85. Could you please help us to reproduce this issue? Here is my concern about the proposed solution There is no guarantee that there won't be other issues with PATCH and we won't be switching back and forth. Moreover, I don't know any reliable resources that suggest PATCH over GIT for applying patches. Maybe we should think of making it configurable (e.g using environment variable). Perhaps, we should handle this specific issue in GitDriver and consider it as a failure so it will fallback to PATCH? — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#82 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOJOUIRDSYGSMLXLZBK3NTSWITK5ANCNFSM4VEKSJNQ> .

ihor-sviziev · 2021-01-04T20:12:46Z

@bubasuma any updates? We're having this issue both on Ubuntu based local machine and on CI/CD.

We're running apply command on top of checked out git repository. If you're not able to reproduce it - please contact me in slack - it will be much more effective

bubasuma · 2021-01-06T15:54:56Z

Hi @ihor-sviziev
I still can't reproduce this issue. Please see attached screenshot
Magento Version: 2.3.6
OS: Ubuntu 20.04
GIT: 2.25.1

Before Apply

After Apply

After Revert

ihor-sviziev · 2021-01-07T07:18:48Z

@bubasuma will try to provide you more details next week

bubasuma · 2021-01-07T12:55:40Z

@ihor-sviziev Could you please also check if there are other patches with the same issue in your environment?

viktym · 2021-01-07T16:02:40Z

@ihor-sviziev please try to modify the patch header.
Example:
replace

diff --git a/vendor/magento/module-negotiable-quote/Model/NegotiableQuoteConverter.php b/vendor/magento/module-negotiable-quote/Model/NegotiableQuoteConverter.php
index 050f3f6d4..9a918c0c5 100644

with

diff -Nuar a/vendor/magento/module-negotiable-quote/Model/NegotiableQuoteConverter.php b/vendor/magento/module-negotiable-quote/Model/NegotiableQuoteConverter.php

and apply using Git

ihor-sviziev · 2021-01-08T08:21:15Z

@viktym will try that one as well

magento-cloud-ft-jenkins-svc · 2021-01-09T04:25:15Z

Static Analysis & Code Style Results

❌ One or more static analysis or code style checks have failed.

PHP Codesniffer Output

Could not read phpcs output.

PHP Mess Detector Output

Could not read phpmd output.

PHPStan Output

Could not read phpstan output.

This comment was generated by Jenkins job magento-cloud-docker/static build 3.

magento-cloud-ft-jenkins-svc · 2021-01-09T04:25:16Z

Unit & Integration Test Results

❌ One or more unit or integration tests have failed.

Unit Test Output

Could not read unit test output.

Integration Test Output

Could not read integration test output.

This comment was generated by Jenkins job magento-cloud-docker/unit build 2.

magento-cloud-ft-jenkins-svc · 2021-01-09T22:26:29Z

Unit Test Results

✅ All unit tests have passed.

Unit Test Output

> phpunit --configuration tests/unit '--log-junit' 'tests/unit/tmp/junit.xml' '--coverage-clover' 'tests/unit/tmp/clover.xml'
PHPUnit 6.5.14 by Sebastian Bergmann and contributors.

...............................................................  63 / 159 ( 39%)
............................................................... 126 / 159 ( 79%)
.................................                               159 / 159 (100%)

Time: 1.71 seconds, Memory: 18.00MB

OK (159 tests, 392 assertions)

Generating code coverage report in Clover XML format ... done

This comment was generated by Jenkins job magento-cloud-patches/unit build 5.

magento-cloud-ft-jenkins-svc · 2021-01-09T22:26:55Z

Static Analysis & Code Style Results

✅ All static analysis and code style checks have passed.

PHP Codesniffer Output

> phpcs src --standard=tests/static/phpcs-ruleset.xml -p -n
............................................................  60 / 142 (42%)
............................................................ 120 / 142 (85%)
......................                                       142 / 142 (100%)


Time: 4.34 secs; Memory: 12MB

PHP Mess Detector Output


Found 0 violations and 0 errors in 2738ms

�[32mNo mess detected�[0m

This comment was generated by Jenkins job magento-cloud-patches/static build 6.

magento-cloud-ft-jenkins-svc · 2021-01-10T00:55:23Z

Functional Acceptance Test Results

❌ One or more functional acceptance tests have failed.

PHP 7.1

❌ Acceptance71Cest
❌ Acceptance71CeCest
❌ Acceptance71CePart2Cest
❌ Acceptance71Part2Cest

PHP 7.2

✅ Acceptance72CeCest
✅ Acceptance72Cest

PHP 7.3

✅ Acceptance73CeCest
✅ Acceptance73Cest

PHP 7.4

✅ AcceptanceCeCest
✅ AcceptanceCest

Output for failed tests is below. If many tests have failed only the first 5 will be included. If you need additional information please reach out to the Magento Cloud team for more details.