MAGECLOUD-4004: Move Docker-related functionality to separate package #33

shiftedreality · 2019-07-31T18:54:48Z

Description

Fixed Issues (if relevant)

https://magento2.atlassian.net/browse/MAGECLOUD-4004

Manual testing scenarios

!!! Use branch https://github.com/magento/ece-tools/tree/MAGECLOUD-4004
!!! ./vendor/bin/ece-docker build:compose is equivalent to ./vendor/bin/ece-tools docker:build

Contribution checklist

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages

magento-cicd2 · 2019-07-31T18:54:52Z

All committers have signed the CLA.

- Add PHP 7.3

- Add Varnish 6.0

arhiopterecs · 2019-08-02T01:54:17Z

autoload.php

+    define('ECE_BP', BP);
+}
+
+foreach ([__DIR__ . '/../../autoload.php', __DIR__ . '/vendor/autoload.php'] as $file) {


I think it will look better

foreach ([ '/../../autoload.php', '/vendor/autoload.php'] as $file) { if (file_exists(__DIR__.$file)) { return require $file; }

You won't be able to navigate to those files from IDE

arhiopterecs · 2019-08-02T02:04:09Z

dist/bin/docker

@@ -0,0 +1,63 @@
+#!/bin/bash


I think will be better if you change the name of this file to docker.sh or mc-docker
Because the current name can be confused with docker command

It would be BC change, as we already released this file

I'd actually submitted a Jira ticket about this as well. This file creates some really frustrating conflicts if you add your project's bin directory to $PATH. It's not just confusing; it actually breaks things.

arhiopterecs · 2019-08-02T04:12:09Z

images/php/7.3-fpm/Dockerfile

@@ -0,0 +1,149 @@
+FROM php:7.3-fpm-stretch


It looks like the image php:7.3-cli-stretch has problems with libsodium too

$ docker run -it php:7.3-cli-stretch bash root@92cc980b6e64:/# php -v PHP 7.3.7 (cli) (built: Jul 17 2019 22:25:08) ( NTS ) Copyright (c) 1997-2018 The PHP Group Zend Engine v3.3.7, Copyright (c) 1998-2018 Zend Technologies root@92cc980b6e64:/# php -i | grep libsodium libsodium headers version => 1.0.11 libsodium library version => 1.0.11 root@92cc980b6e64:/# php -r 'echo SODIUM_CRYPTO_PWHASH_ALG_ARGON2ID13;' Warning: Use of undefined constant SODIUM_CRYPTO_PWHASH_ALG_ARGON2ID13 - assumed 'SODIUM_CRYPTO_PWHASH_ALG_ARGON2ID13' (this will throw an Error in a future version of PHP) in Command line code on line 1 SODIUM_CRYPTO_PWHASH_ALG_ARGON2ID13

Thanks, fixed

arhiopterecs · 2019-08-02T04:19:51Z

images/php/php-extensions.php

+pecl install -o -f libsodium
+BASH
+        ],
+        '~7.2.0' => [


You need to change the value of constraint because the image php:7.3-cli-stretch also has an outdated library libsodium.

arhiopterecs · 2019-08-02T04:23:55Z

src/Command/BuildCompose.php

+            [ComposeFactory::COMPOSE_DEVELOPER, ComposeFactory::COMPOSE_PRODUCTION],
+            false
+        )) {
+


Delete empty line, please

arhiopterecs · 2019-08-02T15:27:11Z

src/Compose/PhpExtension.php

+    /**
+     * Extensions which can be installed or uninstalled
+     */
+    const AVAILABLE_PHP_EXTENSIONS = [


this constant can be deleted since this information is contained in the file 'images/php/php-extensions.php'

It's used in this class. It seems like separate task

arhiopterecs · 2019-08-07T15:54:56Z

src/Compose/Php/ExtensionResolver.php

+        'soap' => '>=7.0',
+        'sockets' => '>=7.0',
+        'sodium' => '>=7.0',
+        'ssh2' => '>=7.0',


It must be indicated that the package is not available for PHP 7.3

https://github.com/magento/magento-cloud-docker/pull/33/files#diff-9f51bf098cc0ff04fa9850efeefe1780R170

bbatsche · 2019-08-06T20:36:48Z

autoload.php

+ * See COPYING.txt for license details.
+ */
+error_reporting(E_ALL);
+date_default_timezone_set('UTC');


Why are we setting a timezone?

It's always good to set timezone in the beginning of php script to not depend on user settings.

bbatsche · 2019-08-06T20:40:21Z

images/php/cli/bin/cloud-build

+
+[ "$DEBUG" = "true" ] && set -x
+
+MAGENTO_COMMAND="magento-command"


I could be missing something, but I can't see how this variable is used anywhere?

Thanks, removed

bbatsche · 2019-08-06T20:49:05Z

src/Command/Image/GeneratePhp.php


 use Composer\Semver\Constraint\Constraint;
+use Composer\Semver\Semver;
 use Composer\Semver\VersionParser;


No longer used

bbatsche · 2019-08-06T20:50:39Z

src/Command/Image/GeneratePhp.php

-namespace Mcd\Command\Generate;
+declare(strict_types=1);
+
+namespace Magento\CloudDocker\Command\Image;

 use Composer\Semver\Constraint\Constraint;


No longer used

bbatsche · 2019-08-06T20:50:57Z

src/Command/Image/GeneratePhp.php

@@ -87,26 +94,33 @@ class Php extends Command
    /**
     * @var VersionParser


should be Semver

bbatsche · 2019-08-06T20:54:15Z

composer.json

+        "ext-json": "*",
+        "composer/semver": "^1.0",
+        "illuminate/config": "^5.5",
+        "illuminate/filesystem": "^5.5",


Perhaps we should consider using symfony/filesystem since it's already included with ECE tools, and would thus cut down on the number of extra dependencies

It lacks a bunch of methods that the previous implementation was relying on

src/Command/Image/GeneratePhp.php

…agento/magento-cloud-docker into MAGECLOUD-4004

andriyShevtsov · 2019-08-13T21:15:06Z

QA approved

… v1.2.0 (#33)

shiftedreality added 10 commits July 31, 2019 10:00

MAGECLOUD-4004: Move Docker-related functionality to separate package

f1dc7a6

MAGECLOUD-4004: Move Docker-related functionality to separate package

dabb330

MAGECLOUD-4004: Move Docker-related functionality to separate package

3b5f626

MAGECLOUD-4004: Move Docker-related functionality to separate package

f99ebc4

MAGECLOUD-4004: Move Docker-related functionality to separate package

14a5488

MAGECLOUD-4004: Move Docker-related functionality to separate package

1a9fcd9

MAGECLOUD-4004: Move Docker-related functionality to separate package

f933896

MAGECLOUD-4004: Move Docker-related functionality to separate package

7f2a066

MAGECLOUD-4004: Move Docker-related functionality to separate package

0bdf0d5

MAGECLOUD-4004: Move Docker-related functionality to separate package

3fa6a5a

shiftedreality added 3 commits July 31, 2019 14:44

MAGECLOUD-4004: Move Docker-related functionality to separate package

e662edd

MAGECLOUD-4004: Move Docker-related functionality to separate package

5bc7ea2

MAGECLOUD-4004: Move Docker-related functionality to separate package

f1644e8

shiftedreality added the Progress: review PR/issue status label Aug 1, 2019

shiftedreality mentioned this pull request Aug 1, 2019

MAGECLOUD-4004: Move Docker-related functionality to separate package magento/ece-tools#576

Merged

4 tasks

shiftedreality added 2 commits August 1, 2019 14:50

MAGECLOUD-4004: Move Docker-related functionality to separate package

bb3bf5b

- Add PHP 7.3

MAGECLOUD-4004: Move Docker-related functionality to separate package

b10aa04

- Add Varnish 6.0

arhiopterecs reviewed Aug 2, 2019

View reviewed changes

MAGECLOUD-4004: Move Docker-related functionality to separate package

c0d578d

mveeramneni requested a review from bbatsche August 6, 2019 14:47

shiftedreality added 6 commits August 6, 2019 10:12

MAGECLOUD-4004: Move Docker-related functionality to separate package

f756bae

MAGECLOUD-4004: Move Docker-related functionality to separate package

a2556c8

MAGECLOUD-4004: Move Docker-related functionality to separate package

9d5d421

MAGECLOUD-4004: Move Docker-related functionality to separate package

d47031b

MAGECLOUD-4004: Move Docker-related functionality to separate package

38066ee

MAGECLOUD-4004: Move Docker-related functionality to separate package

2dba894

arhiopterecs suggested changes Aug 7, 2019

View reviewed changes

MAGECLOUD-4004: Move Docker-related functionality to separate package

c1f66ed

arhiopterecs approved these changes Aug 7, 2019

View reviewed changes

bbatsche suggested changes Aug 7, 2019

View reviewed changes

MAGECLOUD-4004: Move Docker-related functionality to separate package

d39b3b8

bbatsche approved these changes Aug 7, 2019

View reviewed changes

shiftedreality added Progress: testing in progress and removed Progress: review PR/issue status labels Aug 7, 2019

bbatsche mentioned this pull request Aug 7, 2019

MAGECLOUD-4022 - PHP 7.3 Test Coverage magento/ece-tools#585

Merged

4 tasks

shiftedreality added 3 commits August 8, 2019 08:31

Merge branches 'MAGECLOUD-4004' and 'develop' of https://github.com/m…

8a68e75

…agento/magento-cloud-docker into MAGECLOUD-4004

MAGECLOUD-4004: Move Docker-related functionality to separate package

7b8b08f

MAGECLOUD-4004: Move Docker-related functionality to separate package

db168ec

shiftedreality changed the base branch from develop to master August 9, 2019 14:16

shiftedreality assigned andriyShevtsov Aug 9, 2019

shiftedreality added Progress: accept PR/issue status and removed Progress: testing in progress labels Aug 14, 2019

shiftedreality merged commit 14a8859 into master Aug 14, 2019

shiftedreality deleted the MAGECLOUD-4004 branch August 14, 2019 18:45

YPyltiai added the Release: 1.0.0 Magento-Cloud-Docker Release label Aug 15, 2019

magento-cicd2 pushed a commit that referenced this pull request Apr 13, 2021

MCLOUD-7254: sync_engine native no longer references local folders in…

e19c56a

… v1.2.0 (#33)


		[ "$DEBUG" = "true" ] && set -x

		MAGENTO_COMMAND="magento-command"

		@@ -87,26 +94,33 @@ class Php extends Command
		/**
		* @var VersionParser

MAGECLOUD-4004: Move Docker-related functionality to separate package #33

MAGECLOUD-4004: Move Docker-related functionality to separate package #33

Uh oh!

Conversation

shiftedreality commented Jul 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Fixed Issues (if relevant)

Manual testing scenarios

Contribution checklist

Uh oh!

magento-cicd2 commented Jul 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

andriyShevtsov commented Aug 13, 2019

Uh oh!

Uh oh!

shiftedreality commented Jul 31, 2019 •

edited

Loading

magento-cicd2 commented Jul 31, 2019 •

edited

Loading