From 0af673491adc0a3ab61081ad8018da255d6e2223 Mon Sep 17 00:00:00 2001 From: Justin Boswell Date: Tue, 17 Aug 2021 19:06:14 -0700 Subject: [PATCH] Removed FFI support (#45) * Removed FFI support * run_tests and ci-test.sh should never have differed. Fixing now * run_tests can now update composer --- Makefile.frag | 6 +-- builder.json | 2 +- ci-test.sh | 11 ----- composer.json | 1 - run_tests | 9 ++++ src/AWS/CRT/CRT.php | 44 +++---------------- src/AWS/CRT/Internal/FFI.php | 82 ------------------------------------ tests/LogTest.php | 1 - tests/SigningTest.php | 7 --- tests/StreamTest.php | 1 - tests/common.inc | 6 --- 11 files changed, 18 insertions(+), 152 deletions(-) delete mode 100755 ci-test.sh delete mode 100644 src/AWS/CRT/Internal/FFI.php diff --git a/Makefile.frag b/Makefile.frag index 0d406c7..0b97206 100644 --- a/Makefile.frag +++ b/Makefile.frag @@ -12,8 +12,8 @@ CMAKE_BUILD = $(CMAKE) --build CMAKE_BUILD_TYPE ?= RelWithDebInfo CMAKE_TARGET = --config $(CMAKE_BUILD_TYPE) --target install -all: extension ffi -.PHONY: all extension ffi +all: extension +.PHONY: all extension # configure for static aws-crt-ffi.a build/aws-crt-ffi-static/CMakeCache.txt: @@ -51,7 +51,7 @@ vendor/bin/phpunit: composer update test-extension: vendor/bin/phpunit extension - AWS_CRT_PHP_EXTENSION=1 composer run test-extension + composer run test-extension # Use PHPUnit to run tests test: test-extension diff --git a/builder.json b/builder.json index 859a0ea..2c6f0be 100644 --- a/builder.json +++ b/builder.json @@ -32,6 +32,6 @@ "NO_INTERACTION": "1" }, "test_steps": [ - ["./ci-test.sh"] + ["./run_tests"] ] } diff --git a/ci-test.sh b/ci-test.sh deleted file mode 100755 index 6faf1b5..0000000 --- a/ci-test.sh +++ /dev/null @@ -1,11 +0,0 @@ -#!/usr/bin/env bash - -set -ex - -HAS_FFI=$(php -m | grep FFI | wc -l | xargs) - -make test-extension - -if [[ $HAS_FFI -gt 0 ]]; then - make test-ffi -fi \ No newline at end of file diff --git a/composer.json b/composer.json index f201d6a..3d7e6a9 100644 --- a/composer.json +++ b/composer.json @@ -31,7 +31,6 @@ "scripts": { "test": "./run_tests", "test-extension": "@test", - "test-ffi": "@php vendor/bin/phpunit tests", "test-win": "run_tests" }, "license": "Apache-2.0" diff --git a/run_tests b/run_tests index b234e0e..3d13c04 100755 --- a/run_tests +++ b/run_tests @@ -2,4 +2,13 @@ set -ex +if [ -z $PHP_BINARY ]; then + PHP_BINARY=$(which php) +fi + +if [ ! -d vendor ]; then + composer update +fi + $PHP_BINARY -c php.ini vendor/bin/phpunit tests --debug + diff --git a/src/AWS/CRT/CRT.php b/src/AWS/CRT/CRT.php index f33b16c..d196a47 100644 --- a/src/AWS/CRT/CRT.php +++ b/src/AWS/CRT/CRT.php @@ -22,30 +22,10 @@ final class CRT { function __construct() { if (is_null(self::$impl)) { - // Figure out what backends are/should be available - $backends = ['Extension']; - if (extension_loaded('ffi')) { - $backends = ['Extension', 'FFI']; - if (getenv('AWS_CRT_PHP_EXTENSION')) { - $backends = ['Extension']; - } else if (getenv('AWS_CRT_PHP_FFI')) { - $backends = ['FFI']; - } - } - - // Try to load each backend, give up if none succeed - $exceptions = []; - foreach ($backends as $backend) { - try { - $backend = 'AWS\\CRT\\Internal\\' . $backend; - self::$impl = new $backend(); - break; - } catch (RuntimeException $rex) { - array_push($exceptions, $rex); - } - } - if (is_null(self::$impl)) { - throw new RuntimeException('Unable to initialize AWS CRT via ' . join(', ', $backends) . ": \n" . join("\n", $exceptions), -1); + try { + self::$impl = new Extension(); + } catch (RuntimeException $rex) { + throw new RuntimeException("Unable to initialize AWS CRT via awscrt extension: \n$rex", -1); } } ++self::$refcount; @@ -58,7 +38,7 @@ final class CRT { } /** - * @return bool whether or not the CRT is currently loaded with an active backend + * @return bool whether or not the CRT is currently loaded */ public static function isLoaded() { return !is_null(self::$impl); @@ -76,20 +56,6 @@ final class CRT { } } - /** - * @return bool true if using PHP FFI (PHP 7.4+) - */ - public static function isFFI() { - return self::isLoaded() && strstr(get_class(self::$impl), 'FFI'); - } - - /** - * @return bool true if using PHP Extension awscrt - */ - public static function isExtension() { - return self::isLoaded() && strstr(get_class(self::$impl), 'Extension'); - } - /** * @return integer last error code reported within the CRT */ diff --git a/src/AWS/CRT/Internal/FFI.php b/src/AWS/CRT/Internal/FFI.php deleted file mode 100644 index ad1ac87..0000000 --- a/src/AWS/CRT/Internal/FFI.php +++ /dev/null @@ -1,82 +0,0 @@ - 0) { - $uint8_t = \FFI::type('uint8_t'); - $uint8_array = \FFI::arrayType($uint8_t, [$len]); - $buf = \FFI::new($uint8_array); - \FFI::memcpy($buf, $arg, $len); - $ffi_args [] = $buf; - $ffi_args [] = $len; - } else { - $ffi_args [] = null; - $ffi_args [] = 0; - } - } else if (is_resource($arg)) { - throw new RuntimeException("Resource types are not supported for FFI"); - } else { - $ffi_args []= $arg; - } - } - return call_user_func_array([self::$ffi, $name], $ffi_args); - } - - private static function init() { - return self::$ffi->aws_crt_init(); - } - - private static function clean_up() { - return self::$ffi->aws_crt_clean_up(); - } -} diff --git a/tests/LogTest.php b/tests/LogTest.php index 79456cf..8beff34 100644 --- a/tests/LogTest.php +++ b/tests/LogTest.php @@ -10,7 +10,6 @@ require_once('common.inc'); class LogTest extends CrtTestCase { public function testLogToStream() { - $this->skipFFI(); $log_stream = fopen("php://memory", "r+"); $this->assertNotNull($log_stream); Log::toStream($log_stream); diff --git a/tests/SigningTest.php b/tests/SigningTest.php index 31429d7..77399ab 100644 --- a/tests/SigningTest.php +++ b/tests/SigningTest.php @@ -39,7 +39,6 @@ final class SigningTest extends CrtTestCase { } public function testSignableFromChunkLifetime() { - $this->skipFFI(); $chunk = "THIS IS A TEST CHUNK IT CONTAINS MULTITUDES"; $stream = fopen("php://memory", 'r+'); fputs($stream, $chunk); @@ -66,8 +65,6 @@ final class SigningTest extends CrtTestCase { } public function testShouldSignHeader() { - $this->skipFFI(); - $credentials_provider = new StaticCredentialsProvider([ 'access_key_id' => self::SIGV4TEST_ACCESS_KEY_ID, 'secret_access_key' => self::SIGV4TEST_SECRET_ACCESS_KEY, @@ -107,8 +104,6 @@ final class SigningTest extends CrtTestCase { } public function testSigv4HeaderSigning() { - $this->skipFFI(); - $credentials_provider = new StaticCredentialsProvider([ 'access_key_id' => self::SIGV4TEST_ACCESS_KEY_ID, 'secret_access_key' => self::SIGV4TEST_SECRET_ACCESS_KEY, @@ -143,8 +138,6 @@ final class SigningTest extends CrtTestCase { } public function testSigV4aHeaderSigning() { - $this->skipFFI(); - $credentials_provider = new StaticCredentialsProvider([ 'access_key_id' => self::SIGV4TEST_ACCESS_KEY_ID, 'secret_access_key' => self::SIGV4TEST_SECRET_ACCESS_KEY, diff --git a/tests/StreamTest.php b/tests/StreamTest.php index 9c090a0..f8a116a 100644 --- a/tests/StreamTest.php +++ b/tests/StreamTest.php @@ -19,7 +19,6 @@ final class InputStreamTest extends CrtTestCase { } public function testMemoryStream() { - $this->skipFFI(); $mem_stream = $this->getMemoryStream(); $stream = new InputStream($mem_stream); $this->assertNotNull($stream, "Failed to create InputStream from PHP memory stream"); diff --git a/tests/common.inc b/tests/common.inc index 35601fa..1a7e8f8 100644 --- a/tests/common.inc +++ b/tests/common.inc @@ -31,10 +31,4 @@ abstract class CrtTestCase extends PHPUnit_Framework_TestCase { $this->setExpectedException($arguments[0]); } } - - public function skipFFI() { - if (CRT::isFFI()) { - $this->markTestSkipped(); - } - } }