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

Avoid nested duplication #183

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Avoid nested duplication #183

wants to merge 2 commits into from

Conversation

olythy
Copy link

@olythy olythy commented Oct 25, 2019

Avoid nested duplication if a module folder exists when using copy strategy.

Avoid nested duplication if a module folder exists when using copy strategy
@kkrieger85
Copy link

Can confirm the duplication while copy deployment strategy

I wonder why this happens now. What changed, that this occurs?

@kkrieger85
Copy link

@olythy How can I test your changes?

@olythy
Copy link
Author

olythy commented Dec 16, 2019

Hi,

I have a fork here https://github.com/mage-better-m1/magento-composer-installer

Could you reproduce the problem?

@kkrieger85
Copy link

@olythy I tried your fork:

I deleted vendordir, added your repository to composer.json, did composer update & composer install

composer.json:

  "require": {
    "magento-hackathon/magento-composer-installer": "^3.2",
    "magento/core": "1.9.4.*",
...
  "repositories": [
    {
      "type": "vcs",
      "url": "[email protected]:mage-better-m1/magento-composer-installer.git"
    },

composer.lock:

{
            "name": "magento-hackathon/magento-composer-installer",
            "version": "3.2.0",
            "source": {
                "type": "git",
                "url": "https://github.com/mage-better-m1/magento-composer-installer.git",
                "reference": "fad9b319b425c14d9173732100c829bb310eb050"
            },
            "dist": {
                "type": "zip",
                "url": "https://api.github.com/repos/mage-better-m1/magento-composer-installer/zipball/fad9b319b425c14d9173732100c829bb310eb050",
                "reference": "fad9b319b425c14d9173732100c829bb310eb050",
                "shasum": ""
            },

But it did not fixed the problem. I still have nested duplication

@kkrieger85
Copy link

For better testing:
I face this problem with "avstudnitz/missing-translations": "dev-master#179905b",

result: htdocs/app/code/community/AvS/MissingTranslations/MissingTranslations/

Ich will try to generate a minium composer.json to reproduce this bug

@kkrieger85
Copy link

I tried installation with this minimal composer.json - and it worked fine!

composer.txt
composer.lock.txt

Maybe, the problem with nested duplication is something like a "sideeffekt" from other modules & dependencies.

@kkrieger85
Copy link

After this minimal composer.json I added aoepeople/aoe_scheduler via

composer require aoepeople/aoe_scheduler 

Then I deleted vendor dir and did composer install

Now, Avs/MissingTranslations becomes a nested duplicate.

CLI outputs:

composer require aoepeople/aoe_scheduler

Using version dev-master for aoepeople/aoe_scheduler
./composer.json has been updated
Loading composer repositories with package information               Updating dependencies (including require-dev)         
Package operations: 1 install, 1 update, 0 removals
  - Installing aoepeople/aoe_scheduler (dev-master 13dd3ef): Cloning 13dd3efc12 from cache
Writing lock file
Generating autoload files
./htdocs/app/Mage.php is not a file.

composer install

Loading composer repositories with package information
Installing dependencies (including require-dev) from lock file
Package operations: 11 installs, 0 updates, 0 removals
  - Installing symfony/polyfill-mbstring (dev-master 7b4aab9): Cloning 7b4aab9743 from cache
  - Installing psr/log (dev-master 5628725): Cloning 5628725d0e from cache
  - Installing symfony/debug (4.4.x-dev 89c3fd5): Cloning 89c3fd5c29 from cache
  - Installing symfony/console (3.4.x-dev 7c5bdd3): Cloning 7c5bdd346f from cache
  - Installing justinrainbow/json-schema (4.1.0): Loading from cache
  - Installing eloquent/enumeration (5.1.1): Loading from cache
  - Installing eloquent/composer-config-reader (2.1.0): Loading from cache
  - Installing magento-hackathon/magento-composer-installer (3.2.2): Loading from cache
  - Installing aoepeople/aoe_scheduler (dev-master 13dd3ef): Cloning 13dd3efc12 from cache
  - Installing magento/core (1.9.4.3): Loading from cache
  - Installing avstudnitz/missing-translations (dev-master 179905b): Cloning 179905b6a8 from cache
symfony/console suggests installing symfony/event-dispatcher
symfony/console suggests installing symfony/lock
symfony/console suggests installing symfony/process
magento-hackathon/magento-composer-installer suggests installing colinmollenhour/modman (*)
magento-hackathon/magento-composer-installer suggests installing theseer/autoload (~1.14)

@kkrieger85
Copy link

composer.json.txt
composer.lock.txt

with these file I can reproduce nested duplications:

  1. composer install=> Installation works fine
  2. remove vendor dir
  3. composer install => nested duplication for AvS/MissingTranslations but not Aoe/Scheduler

@kkrieger85
Copy link

kkrieger85 commented Jan 11, 2020

Comparison of modman file:

app/code/community/Aoe/Scheduler/ app/code/community/Aoe/Scheduler/
vs
src/app/code/community/AvS/* app/code/community/AvS/

@kkrieger85
Copy link

I updated modman file and re-installed module from my repo: Installation works fine, no nested duplication.

https://github.com/kkrieger85/AvS_MissingTranslations/commit/1be83f15f7f89ab6d54e585f09274c45aa5cf0c2

Question is now: Is this a magento-composer-installer bug or wrong usage of modman mapping syntax

@Flyingmana Flyingmana changed the base branch from master to main June 15, 2020 16:48
@sonarcloud
Copy link

sonarcloud bot commented Feb 17, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

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.

2 participants