Skip to content

"composer install" overwrite customizations outside of vendor folder #6221

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

Closed
vesan-83 opened this issue Aug 18, 2016 · 21 comments
Closed

"composer install" overwrite customizations outside of vendor folder #6221

vesan-83 opened this issue Aug 18, 2016 · 21 comments
Labels
improvement Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed

Comments

@vesan-83
Copy link

vesan-83 commented Aug 18, 2016

"composer install" command overwrite modifications in mapped files (files from composer.json -> extra.map node of magento/magento2-base module).

Preconditions

Magento 2.1.0

Steps to reproduce

  1. Clone the project where any of mapped file is customized e.g. pub/index.php or dev/tools/grunt/configs/themes.js
  2. Run "composer install"

Expected result

  1. Customized file will stay customized so that "composer install" will not touch existing files.

Actual result

  1. Customized file is replaced by default one
@maderlock
Copy link

I suspect that this is a feature, not a bug. Are there particular files that you think should be not part of the Magento core?

@hostep
Copy link
Contributor

hostep commented Aug 18, 2016

Duplicate for dev/tools/grunt/configs/themes.js file: #4697

For the pub/index.php file, this is very dangerous what you are asking. If Magento decides to update the index.php file in a new version, and you upgraded your project you would miss those updates on the index.php file.

@remkoj-ism
Copy link

For the pub/index.php file, this is very dangerous what you are asking. If Magento decides to update the index.php file in a new version, and you upgraded your project you would miss those updates on the index.php file.

The "composer install" command is IMHO intended to restore the dependencies of the application, but not modify the application in any way. The fact that it modifies a file which may contain additional pre-initialization logic (it may not be the most beautiful way, but it can be the most efficient way) breaks the ability to distribute an application without including its dependencies.

When talking about the "composer upgrade" command, especially when it updates the Magento base package, I agree with the argument above.

@hostep
Copy link
Contributor

hostep commented Aug 18, 2016

Ok that probably makes sense.

We currently have the same "problem" with the pub/.htaccess file, but we work around it in our deploy scripts, by doing this:

run('tools/composer.phar install --prefer-dist --no-dev')
run('git checkout pub/.htaccess')

@andimov
Copy link
Contributor

andimov commented Aug 22, 2016

This is a duplicate of #4697
I'm closing this one. Please feel free to reopen if you need.

@vesan-83
Copy link
Author

It's not duplicate for #4697, I'm talking about creating development environment (git clone and composer install after) but not about upgrade procedure.

@vesan-83
Copy link
Author

Please feel free to reopen if you need.
How?

@piotrekkaminski
Copy link
Contributor

Thank you for your submission.

We recently made some changes to the way we process GitHub submissions to more quickly identify and respond to core code issues.

Feature Requests and Improvements should now be submitted to the new Magento 2 Feature Requests and Improvements forum (see details here).

We are closing this GitHub ticket and have moved your request to the new forum.

@wpalmer
Copy link
Contributor

wpalmer commented Jan 10, 2018

Wouldn't this be a bug in the .gitignore file? Surely any files which are managed by "composer install", should not be tracked in git?

@magento-engcom-team magento-engcom-team added the Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed label Jan 10, 2018
@vesan-83
Copy link
Author

vesan-83 commented Jan 10, 2018

Sorry guys but your comments are totally pathetic.
Did you even think that developers need to fix thousands of your bugs in files which can't be extended by plugins or rewrites? We found a custom way to apply patches on this tones of bullshit.
Nobody even tried to understand core of the problem and provide acceptable answer how to handle such situations.
It doesn't make sense to report anything.
Good luck. Peace!

@hostep
Copy link
Contributor

hostep commented Jan 10, 2018

@vesan-83: the bug around the file dev/tools/grunt/configs/themes.js has been fixed in Magento 2.2.0 where you aren't supposed to overwrite that file any longer, you need to create a copy of that file and call it dev/tools/grunt/configs/local-themes.js and reference it from the file grunt-config.json which you can copy from grunt-config.json.sample. The problem does still exist on 2.1.x though...

Regarding the pub/index.php file, only a composer update should overwrite the file and not a composer install like @remkoj-ism proposes.

@wpalmer: sometimes you want to include pub/index.php in version control, for example to be able to run the same shop over multiple different domain names, you can modify the pub/index.php file to set the correct initialisation variables so Magento knows to initalise which website/storeview depending on the current domain name in use in the request. In this case it makes sense to include that file in version control, but not everyone needs this.

@wpalmer
Copy link
Contributor

wpalmer commented Jan 10, 2018

Yes, and for those cases where this is needed, you can force-add the file. But as a default, it would make sense to not have a file managed by two separate systems. It would make a lot more sense to need to explicitly specify "I want to do something odd" than to have the default behavior be odd.

Of course, if you think it's normal enough to modify the file that it should not be ignored by default, then it probably shouldn't default to being modified by composer. One, or the other, might make sense, but the current situation doesn't (note that this applies to more than just index.php)

@czettnersandor
Copy link

I found this issue when I realised our deployment process is reverting my changes on index.php when it's running composer install.

Please check the first 16 lines of this file:

<?php
/**
 * Application entry point
 *
 * Example - run a particular store or website:
 * --------------------------------------------
 * require __DIR__ . '/app/bootstrap.php';
 * $params = $_SERVER;
 * $params[\Magento\Store\Model\StoreManager::PARAM_RUN_CODE] = 'website2';
 * $params[\Magento\Store\Model\StoreManager::PARAM_RUN_TYPE] = 'website';
 * $bootstrap = \Magento\Framework\App\Bootstrap::create(BP, $params);
 * \/** @var \Magento\Framework\App\Http $app *\/
 * $app = $bootstrap->createApplication(\Magento\Framework\App\Http::class);
 * $bootstrap->run($app);
 * --------------------------------------------

It's suggesting to set these variables in index.php. I would remark that this was the common practice in Magento 1 as well and there's no any other easy way to do this (other than setting environmental variables in the webserver configuration). If this file is open to customisations, then a simple "composer install" should not overwrite it. Given the simplicity of this file, I would suggest do not change it upstream ever. Or if it's necessary, just alert the developers in the changelog to make those changes.

@hostep
Copy link
Contributor

hostep commented Jun 23, 2018

@czettnersandor: I've been meaning to test the following, but haven't found the time yet: apparently you can define a php file in your composer.json file, in the autoload files section, and that file is then getting included for every request before the index.php file is being executed, so you could - in theory - define your environment variables in that file and don't need to change your index.php file.
Again: all in theory and untested. If you would find some time to test and report back, that would be awesome! :)

@clemblanco
Copy link

There is a .htaccess.sample surely that file should be reverted to whatever the vanilla version is, not the .htaccess one.

This sounds pretty dumb to me. How are we supposed to deploy the application with composer when running composer install if the .htaccess gets reset?

@wpalmer
Copy link
Contributor

wpalmer commented Aug 30, 2018

Since this thread was originally opened, the list of files which "composer install" overwrites has only gotten larger. Much larger.

@clemblanco
Copy link

💩

@ragboyjr
Copy link

For those being plagued by this issue, You can disable this feature by setting the following in the composer.json:

  "extra": {
    "magento-deploystrategy": "none"
  }

This will update the magento composer installer code to not copy everything over on install.

@wpalmer
Copy link
Contributor

wpalmer commented Sep 27, 2018 via email

@ragboyjr
Copy link

ragboyjr commented Sep 27, 2018

@wpalmer

It depends on what you want. Personally, I prefer to install magento, then clean out all of the files not needed. Then on a magento version upgrade, just change that to magento-force: true, composer install and see what changed and then remove any unwanted files again.

If you want to just ignore a few files, that's possible too with magento-deploy-ignore, but there's not any documentation on the syntax for it: https://github.com/magento/magento-composer-installer/blob/master/src/MagentoHackathon/Composer/Magento/Installer.php#L325

@andrewdaluz
Copy link

andrewdaluz commented Sep 2, 2019

If anyone came here because of deployer/capistrano overwritten your files, I'd recommend you try:

I works like this, for example if you want to prevent the pub/.htaccess from being overwritten.

Add this to the composter.json

"extra": {
"magento-force": "override",
"magento-deploy-ignore": {
"*": [
"/pub/.htaccess"
]
}
}

Reference: https://stackoverflow.com/a/51961956/2620326

magento-engcom-team pushed a commit that referenced this issue Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed
Projects
None yet
Development

No branches or pull requests