Removing environment specific switch from tool

[davidalger]

Toolchains should work uniformly regardless of what environment they are run within. That is, with the same settings, composer should do the same things everywhere it is run.

Should someone not want file-marshaling to run when deploying Magento ECE, they should add "magento-deploystrategy": "none" to the extra section of their composer config.

In a recent setup where I prepped an existing site to deploy into the ECE environment, I was forced to explicitly request the use of the copy strategy in my composer.json (using same setting as noted above) to workaround this seemingly arbitrary condition which was added to this module at the behest of the platform.sh and/or Magento team(s).

[magento-cicd2]

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

---

David Alger seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.

[vrann]

@davidalger This option is used to prevent any type of files copying over existing root directory of the git repo when deploying on the cloud. Root directory of the git repo for the cloud should be prepared locally: "composer install" should be run, this will copy files, any conflicts should be resolved locally as well. After that, all copied files should be added to the cloud git and comited/pushed.

The reason behind this is that cloud will not be able to resolve conflicts in case if you have changed files from the base package. Deployment becomes unpredictable in this case.

Can you describe more why did you need to run the marshalling on the cloud itself, not locally?

[davidalger]

@vrann The environment variable check is not an option, it's the cloud forcing a different behavior on ECE users. The configuration I noted in the ticket description is however an option. If committing all the files to the repository for ECE deploys is going to be recommended, then my suggestion would be to with that direct users to have "magento-deploystrategy": "none" in the extra section of their composer.json file to disable the file-marshaling.

The reason behind this is that cloud will not be able to resolve conflicts in case if you have changed files from the base package.

To be clear, this isn't an issue of conflict resolution, but a measure to prevent the file-marshaling from overwriting modified versions of the files checked into the repository. And there is already actually an existing solution for that using the composer configuration as well :)

Deployment becomes unpredictable in this case.

I would beg to disagree with this. It certainly does result in some potentially unexpected behavior if one both does not configure composer properly AND have committed modified versions of files which composer normally marshals into place. The predictability however is uniform, as in the deployment of the same source repository will result in the same code in production every time it's run.

Can you describe more why did you need to run the marshalling on the cloud itself, not locally?

We do not commit the files installed by composer to our repositories. The repository of a project should remain lean. There are many benefits to doing this, and we aren't the only ones. This was even in accordance with the official guidance Magento distributed to partners shortly after GA. That aside though :) It's current and working practice which many advanced users of Magento 2 are following and is how we deploy all of our builds everywhere currently.

A very valid deployment procedure has been implemented (and is beginning to be pretty widely used) in the form of the capistrano-magento2 plugin for capistrano. The tool allows for someone to commit all their code to the repository (and skip the composer install on deploy), but the primary use scenario (and the one we all use over here) is to only check in files specific to the project. If a file composer installs needs to be customized, we change it and then exclude it from the list of files marshaled in during the composer install.

On the ECE build I'm working on I have the following in my composer.json:

    "extra": {
        "magento-force": "override",
        "magento-deploystrategy": "copy",
        "magento-deploy-ignore": {
            "*": [
                "/dev/tools/grunt/configs/themes.js"
            ]
        }
    }

Turns out explicitly defining the deploy strategy works around this heavy-handed behavior change in the tool chain. But that last bit is the import part: magento-deploy-ignore, as with it the theme.js is no longer copied when composer install runs.

My main premise is this: Elements of a tool-chain should function uniformly across all environments, with the same set of defaults everywhere, and provide users the ability to tune them to their preferences.

Since the behavior of composer file-marshaling has been well established by the community who originally wrote the magento-composer-installer which was then taken on and adapted by Magento for the purposes of Magento 2, the behavior that is there needs to be accepted and not override in a heavy handed fashion.

Please remove this check, and leave the tool behavior to the configuration found in the composer.json!

[piotrekkaminski]

approved

[davidalger]

@rmsundar1 I'm not sure why you can't see the change. I submitted the PR by editing the file on Github I believe (I don't have a fork of this repo, public or private) 🤷‍♂️

Here is a patch from this PR though (simply what Github gives me when appending .patch to the PR's URL):

From f0e962dee884ca43b603ed5f5f1796c8705e899d Mon Sep 17 00:00:00 2001
From: David Alger <david@classyllama.com>
Date: Fri, 23 Sep 2016 14:49:34 -0500
Subject: [PATCH] Removing environment specific switch from tool

Toolchains should work uniformly regardless of what environment they are run within. That is, with the same settings, composer should do the same things everywhere it is run.

Should someone not want file-marshaling to run when deploying Magento ECE, they should add `"magento-deploystrategy": "none"` to the `extra` section of their composer config.

In a recent setup where I prepped an existing site to deploy into the ECE environment, I was forced to explicitly request the use of the `copy` strategy in my composer.json (using same setting as noted above) to workaround this seemingly arbitrary condition which was added to this module at the behest of the platform.sh and/or Magento team(s).
---
 src/MagentoHackathon/Composer/Magento/Installer.php | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/src/MagentoHackathon/Composer/Magento/Installer.php b/src/MagentoHackathon/Composer/Magento/Installer.php
index deef73e..2b59003 100644
--- a/src/MagentoHackathon/Composer/Magento/Installer.php
+++ b/src/MagentoHackathon/Composer/Magento/Installer.php
@@ -175,10 +175,6 @@ public function __construct(IOInterface $io, Composer $composer, $type = 'magent
             $this->isForced = (bool)$extra['magento-force'];
         }
 
-        if (false !== getenv('MAGENTO_CLOUD_PROJECT')) {
-            $this->setDeployStrategy('none');
-        }
-
         if (isset($extra['magento-deploystrategy'])) {
             $this->setDeployStrategy((string)$extra['magento-deploystrategy']);
         }