From 4fb6605cba4fbb8fb6806f055accd129a5f0a218 Mon Sep 17 00:00:00 2001 From: Takashi Matsuo Date: Tue, 27 Mar 2018 14:47:05 -0700 Subject: [PATCH] Change the way we check the required version of the dependency There are still some cases for false negative where the direct constrant is ok and other indirect dependency has a problem. Although the issue is not completely fixed, this logic change allows more legitimate cases which has been failing with the previous code. It will alleviate #426 --- .../gen-dockerfile/src/ValidateGoogleCloud.php | 15 ++++++++++----- .../tests/GenFilesCommandTest.php | 17 ++++++++++++++++- .../stackdriver_old_logging/composer.json | 2 +- .../test_data/stackdriver_wildcard/app.yaml | 6 ++++++ .../stackdriver_wildcard/composer.json | 5 +++++ 5 files changed, 38 insertions(+), 7 deletions(-) create mode 100644 builder/gen-dockerfile/tests/test_data/stackdriver_wildcard/app.yaml create mode 100644 builder/gen-dockerfile/tests/test_data/stackdriver_wildcard/composer.json diff --git a/builder/gen-dockerfile/src/ValidateGoogleCloud.php b/builder/gen-dockerfile/src/ValidateGoogleCloud.php index 563f8805..7d4dfcfd 100644 --- a/builder/gen-dockerfile/src/ValidateGoogleCloud.php +++ b/builder/gen-dockerfile/src/ValidateGoogleCloud.php @@ -91,14 +91,19 @@ public static function doCheck($workspace) "no available matching version of $package" ); } + $found = false; foreach ($filtered as $version) { - if (Comparator::lessThan($version, $minimumVersionMap[$package])) { - throw new GoogleCloudVersionException( - "stackdriver integration needs $package " - . $minimumVersionMap[$package] . ' or higher' - ); + if (Comparator::greaterThanOrEqualTo($version, $minimumVersionMap[$package])) { + $found = true; + break; } } + if ($found === false) { + throw new GoogleCloudVersionException( + "stackdriver integration needs $package " + . $minimumVersionMap[$package] . ' or higher' + ); + } } if (array_key_exists('google/cloud', $constraintsMap)) { diff --git a/builder/gen-dockerfile/tests/GenFilesCommandTest.php b/builder/gen-dockerfile/tests/GenFilesCommandTest.php index 0d37fbb4..feaee36a 100644 --- a/builder/gen-dockerfile/tests/GenFilesCommandTest.php +++ b/builder/gen-dockerfile/tests/GenFilesCommandTest.php @@ -202,6 +202,21 @@ public function dataProvider() "enable_stackdriver_integration.sh" ] ], + [ + // stackdriver wildcard dep + __DIR__ . '/test_data/stackdriver_wildcard', + null, + '', + '/app/web', + 'added by the php runtime builder', + 'gcr.io/google-appengine/php72:latest', + ["COMPOSER_FLAGS='--no-dev --prefer-dist' \\\n", + "FRONT_CONTROLLER_FILE='index.php' \\\n", + "DETECTED_PHP_VERSION='7.2' \\\n", + "IS_BATCH_DAEMON_RUNNING='true' \n", + "enable_stackdriver_integration.sh" + ] + ], [ // stackdriver individual packages __DIR__ . '/test_data/stackdriver_individual', @@ -224,7 +239,7 @@ public function dataProvider() '', '/app/web', 'added by the php runtime builder', - 'gcr.io/google-appengine/php71:latest', + 'gcr.io/google-appengine/php72:latest', [], '\\Google\\Cloud\\Runtimes\\Builder\\Exception\\GoogleCloudVersionException' ], diff --git a/builder/gen-dockerfile/tests/test_data/stackdriver_old_logging/composer.json b/builder/gen-dockerfile/tests/test_data/stackdriver_old_logging/composer.json index 1404ebff..99a943fc 100644 --- a/builder/gen-dockerfile/tests/test_data/stackdriver_old_logging/composer.json +++ b/builder/gen-dockerfile/tests/test_data/stackdriver_old_logging/composer.json @@ -1,6 +1,6 @@ { "require": { - "google/cloud-logging": "^1.2.0", + "google/cloud-logging": "<=1.2.0", "google/cloud-error-reporting": "^0.4.1" } } diff --git a/builder/gen-dockerfile/tests/test_data/stackdriver_wildcard/app.yaml b/builder/gen-dockerfile/tests/test_data/stackdriver_wildcard/app.yaml new file mode 100644 index 00000000..383f6885 --- /dev/null +++ b/builder/gen-dockerfile/tests/test_data/stackdriver_wildcard/app.yaml @@ -0,0 +1,6 @@ +env: flex +runtime: php + +runtime_config: + enable_stackdriver_integration: true + document_root: web diff --git a/builder/gen-dockerfile/tests/test_data/stackdriver_wildcard/composer.json b/builder/gen-dockerfile/tests/test_data/stackdriver_wildcard/composer.json new file mode 100644 index 00000000..c555cdb3 --- /dev/null +++ b/builder/gen-dockerfile/tests/test_data/stackdriver_wildcard/composer.json @@ -0,0 +1,5 @@ +{ + "require": { + "google/cloud": "*" + } +}