magento-composer-installer icon indicating copy to clipboard operation
magento-composer-installer copied to clipboard

Fix too many files being ignored when providing an ignore path containing directories

Open jantzenw opened this issue 2 years ago • 2 comments

Problem: Providing an ignore path to "magento-deploy-ignore" containing directories causes other files in those directories to be ignored.

Set of steps that I used to reproduce the problem:

  1. Install Magento B2B module.
  2. Add these lines to "magento-deploy-ignore" in composer.json. For those curious, this is being done on a project because its composer installation removes the NegotiableQuote module (and several others, a la https://github.com/yireo/magento2-replace-tools) but the B2B base module has this Negotiable Quote fixture file. Its presence breaks setup:di:compile. This project does not use integration tests so this fixture file's absence will not be missed. This is of course one example to reproduce the problem. There are likely other better examples that do not have other solutions:
        "magento-deploy-ignore": {
            ...
            "magento/magento2-b2b-base": [
                "/setup/src/Magento/Setup/Fixtures/NegotiableQuotesFixture.php"
            ]
        },
  1. rm -rf setup vendor/magento/magento2-base/ vendor/magento/magento2-b2b-base && composer install
  2. See that other files are being ignored:
setup/src/Magento/Setup/Test/Unit/Fixtures/NegotiableQuotesFixtureTest.php
setup/src/Magento/Setup/Test/Unit/Fixtures/CompaniesFixtureTest.php
setup/src/Magento/Setup/Fixtures/_files/quote_fixture_data.json
setup/src/Magento/Setup/Fixtures/NegotiableQuotesFixture.php
setup/src/Magento/Setup/Fixtures/Quote/NegotiableQuoteConfiguration.php
setup/src/Magento/Setup/Fixtures/CompaniesFixture.php
setup/src/Magento/Setup/Fixtures/SharedCatalogsFixture.php
setup/performance-toolkit/files/1mb.pdf
setup/performance-toolkit/profiles/b2b
setup/performance-toolkit/profiles/b2b/extra_small.xml
setup/performance-toolkit/profiles/b2b/manufacture_standard.xml
setup/performance-toolkit/profiles/b2b/small.xml
setup/performance-toolkit/profiles/b2b/singlesite_small.xml
setup/performance-toolkit/profiles/b2b/medium.xml

This is because, in the strpos($destination,$ignored code, $destination is these values:

/setup/src/Magento
/setup/src/Magento/Setup
/setup/src/Magento/Setup/Test
/setup/src/Magento/Setup/Test/Unit
/setup/src/Magento/Setup/Test/Unit/Fixtures
/setup/src/Magento/Setup/Test/Unit/Fixtures/NegotiableQuotesFixtureTest.php
/setup/src/Magento/Setup/Test/Unit/Fixtures/CompaniesFixtureTest.php
/setup/src/Magento/Setup/Fixtures/_files
/setup/src/Magento/Setup/Fixtures/_files/quote_fixture_data.json
/setup/src/Magento/Setup/Fixtures/NegotiableQuotesFixture.php
/setup/src/Magento/Setup/Fixtures/Quote
/setup/src/Magento/Setup/Fixtures/Quote/NegotiableQuoteConfiguration.php
/setup/src/Magento/Setup/Fixtures/CompaniesFixture.php
/setup/src/Magento/Setup/Fixtures/SharedCatalogsFixture.php

strpos( '/setup/src/Magento/Setup/Fixtures/SharedCatalogsFixture.php', '/setup') returns 0, so the entire setup folder is ignored! This is because strpos takes the haystack as the first argument, not the second. So the arguments are reversed.

This seems like a significant change, so I hope that this will be well-reviewed. I am surprised that this bug exists but maybe this use case is rarer than I assumed, or this change will cause some other unintended effect or break some major use case that I am unaware of. The bug does go unnoticed for simple ignores not containing directories, e.g. "/.gitignore", "/.editorconfig", etc.

jantzenw avatar Apr 29 '22 21:04 jantzenw

Trying to close and reopen pull request after signing Adobe CLA, per Adobe's instructions.

jantzenw avatar Apr 29 '22 21:04 jantzenw

Cross linking with https://magento.stackexchange.com/a/350046/17176 just so I can share some notes more appropriately.

convenient avatar Apr 14 '23 09:04 convenient