Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix PECL publish #85

Merged
merged 41 commits into from Mar 1, 2023
Merged

Fix PECL publish #85

Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
e5e998a
require composer to load awscrt extension
TingDaoK Feb 21, 2023
31a5785
load extension before run composer
TingDaoK Feb 22, 2023
82abc2d
make test after the composer nonsense
TingDaoK Feb 22, 2023
1a0efbf
try to just use make test?
TingDaoK Feb 22, 2023
17b6a93
run test
TingDaoK Feb 22, 2023
9ff650e
clean up
TingDaoK Feb 22, 2023
9de07ef
restructure
TingDaoK Feb 22, 2023
826974a
change dir from run test
TingDaoK Feb 22, 2023
f6314a2
update the script path from ci
TingDaoK Feb 22, 2023
1f6f814
create release action when we cut a new release
TingDaoK Feb 22, 2023
9de1d33
more fix
TingDaoK Feb 22, 2023
f183523
make a github action to test pecl before release
TingDaoK Feb 22, 2023
093cf4e
why not find
TingDaoK Feb 22, 2023
b6a3a29
use mac
TingDaoK Feb 22, 2023
08aa34b
skip verification
TingDaoK Feb 22, 2023
3015509
build and test as well
TingDaoK Feb 22, 2023
279f5d2
just make sure it builds
TingDaoK Feb 22, 2023
a7e8407
use mac to release
TingDaoK Feb 22, 2023
02a6591
trivial
TingDaoK Feb 22, 2023
7d56d18
oh, it was a typo..
TingDaoK Feb 22, 2023
26ab397
windows CI
TingDaoK Feb 23, 2023
8208b31
get the right directory
TingDaoK Feb 23, 2023
311edc6
why?
TingDaoK Feb 23, 2023
2aae069
this?
TingDaoK Feb 23, 2023
8455b69
why cannot find dll
TingDaoK Feb 23, 2023
babc4bf
one more test
TingDaoK Feb 23, 2023
3ad39d5
fine
TingDaoK Feb 24, 2023
72c24d9
generate php-win.ini
TingDaoK Feb 24, 2023
7c12d40
get the right php
TingDaoK Feb 24, 2023
a28388f
give me echo
TingDaoK Feb 24, 2023
ce4a00c
I don't know
TingDaoK Feb 24, 2023
8c66121
I don't know
TingDaoK Feb 24, 2023
e361f03
Windows ci fix (#86)
TingDaoK Feb 24, 2023
b116a18
it should not be needed for the config platform
TingDaoK Feb 27, 2023
c0ab711
Revert "it should not be needed for the config platform"
TingDaoK Feb 27, 2023
c2f584a
remove release.yml and leave it as todo to follow up quick
TingDaoK Feb 27, 2023
ff20f95
Rewrite script (#87)
TingDaoK Feb 27, 2023
b4d352c
Merge branch 'native-extension-rewrite' of github.com:awslabs/aws-crt…
TingDaoK Feb 27, 2023
a63ac28
try CMD to run multiple lines of code
TingDaoK Feb 28, 2023
a9ab031
try to not force the special version of phpunit
TingDaoK Feb 28, 2023
6bc427d
update comments
TingDaoK Feb 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion .clang-format-ignore
View file
Open in desktop
@@ -1,4 +1,3 @@
# ignore generated files
ext/api.h
ext/*_arginfo.h
src/api.h
113 changes: 73 additions & 40 deletions .github/workflows/ci.yml
View file
Open in desktop
Expand Up @@ -30,12 +30,6 @@ jobs:
with:
submodules: recursive

- name: Install ancient PHPUnit
run: composer require --dev --ignore-platform-reqs phpunit/phpunit "4.8.36"

- name: Install dependencies
run: composer update --no-interaction

- name: Build for PHP 5.5
env:
CC: clang
Expand All @@ -45,6 +39,13 @@ jobs:
./configure
make
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debateable: // TODO run tests (if we are officially supporting 5.5 for now)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should ask when we can drop support for older versions of PHP. If it's something like 1-2 months away, we can drop support now and sdk-php can pick up new changes later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's leave the php version related stuff as a follow up item, I have listed some TODOs in the description of the PR.


- name: Install dependencies
run: |
php -r "copy('https://getcomposer.org/installer', 'composer-setup.php');"
php composer-setup.php
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this necessary?

ubuntu-latest comes with composer preinstalled

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To have composer.phar in the same directory. So, that we don't need to script to get the path of composer.phar.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

installing composer twice seems like it could have weird consequences. let's just find the path. maybe something like?:

# get path to composer.phar so we can run it with custom php.ini
COMPOSER_PHAR=$(realpath $(which composer))
php -c php.ini $COMPOSER_PHAR require ...

php -c php.ini composer.phar require --dev --ignore-platform-reqs phpunit/phpunit "4.8.36"
graebm marked this conversation as resolved.
Show resolved Hide resolved
php -c php.ini composer.phar update --no-interaction

php-linux-x64:
runs-on: ubuntu-latest
strategy:
Expand All @@ -71,9 +72,6 @@ jobs:
with:
submodules: recursive

- name: Install dependencies
run: composer update --no-interaction

- name: Run tests
env:
CC: clang
Expand All @@ -82,37 +80,50 @@ jobs:
phpize
./configure
make
make test


# linux-arm:
# name: ARM (${{ matrix.arch }})
# runs-on: ubuntu-latest
# strategy:
# matrix:
# arch: [armv6, armv7, arm64]
# steps:
# - name: Build ${{ env.PACKAGE_NAME }}
# run: |
# python3 -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder')"
# chmod a+x builder
# ./builder build -p ${{ env.PACKAGE_NAME }} --target=linux-${{ matrix.arch }} --spec=downstream

# windows-vc16:
# runs-on: windows-latest
# strategy:
# matrix:
# arch: [x64]
# steps:
# - uses: ilammy/msvc-dev-cmd@v1
# with:
# arch: ${{ matrix.arch }}
# uwp: false
# spectre: true
# - name: Build ${{ env.PACKAGE_NAME }} + consumers
# run: |
# python -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder.pyz')"
# python builder.pyz build -p ${{ env.PACKAGE_NAME }} --spec=downstream
./dev-scripts/run_tests.sh

# linux-arm:
# name: ARM (${{ matrix.arch }})
# runs-on: ubuntu-latest
# strategy:
# matrix:
# arch: [armv6, armv7, arm64]
# steps:
# - name: Build ${{ env.PACKAGE_NAME }}
# run: |
# python3 -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder')"
# chmod a+x builder
# ./builder build -p ${{ env.PACKAGE_NAME }} --target=linux-${{ matrix.arch }} --spec=downstream

windows-vc16:
runs-on: windows-2019
defaults:
run:
shell: cmd # use CMD instead of powershell to catch error from bat script
strategy:
matrix:
arch: [x64]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confused: Why only x64? Is this a todo for 32bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, maybe should leave it as a TODO. Again, just being lazy as no current ask and it's probably work.

steps:
- uses: actions/checkout@v3
with:
submodules: recursive
- id: setup-php-sdk
uses: cmb69/setup-php-sdk@v0.6
with:
version: '8.0'
arch: x64
TingDaoK marked this conversation as resolved.
Show resolved Hide resolved
ts: ts
waahm7 marked this conversation as resolved.
Show resolved Hide resolved
deps: openssl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confused: What is this action? I could not find anything about it using a quick Google search (https://github.com/marketplace?type=actions&query=Setup+PHP+). Can you please add some documentation for why we need a different action for Windows CI? It appears that shivammathur/setup-php@v2 also supports Windows.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH, I didn't dig deep. I followed the PR initialed from Mike, https://github.com/awslabs/aws-crt-php/pull/77/files#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fR114.
And I believe Mike found it from https://github.com/Imagick/imagick/blob/master/.github/workflows/windows.yml

I was really just want to have something working...

- uses: ilammy/msvc-dev-cmd@v1
with:
arch: ${{ matrix.arch }}
toolset: ${{steps.setup-php-sdk.outputs.toolset}}
# CMD only execute one commend per run
graebm marked this conversation as resolved.
Show resolved Hide resolved
- run: phpize
- run: .\configure --with-prefix=${{steps.setup-php-sdk.outputs.prefix}} --enable-awscrt=shared --enable-cli --enable-openssl
- run: nmake
- run: nmake generate-php-ini
- run: .\dev-scripts\run_tests.bat ${{steps.setup-php-sdk.outputs.prefix}}\php

# windows-vc14:
# runs-on: windows-latest
Expand Down Expand Up @@ -154,3 +165,25 @@ jobs:
python3 -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder')"
chmod a+x builder
./builder build -p ${{ env.PACKAGE_NAME }} --spec=downstream

pecl-package-test:
runs-on: ubuntu-latest
steps:
- name: Setup PHP
uses: shivammathur/setup-php@v2
with:
php-version: '8.0'

- name: Checkout
uses: actions/checkout@v2
with:
submodules: recursive

- name: Test PECL package build
run: |
python3 dev-scripts/prepare_pecl_release.py --name aws-crt --user aws-crt --version 1.0.0
tar -zxf *.tgz
cd awscrt-1.0.0
phpize
./configure
make
5 changes: 4 additions & 1 deletion .gitignore
View file
Open in desktop
@@ -1,4 +1,3 @@

# Created by https://www.toptal.com/developers/gitignore/api/autotools,cmake,phpstorm
# Edit at https://www.toptal.com/developers/gitignore?templates=autotools,cmake,phpstorm

Expand Down Expand Up @@ -182,6 +181,8 @@ fabric.properties
build/
configure.in
configure.ac
configure.bat
configure.js
mkinstalldirs
run-tests.php
Makefile.global
Expand All @@ -191,6 +192,7 @@ modules/
*.lo
config.h
config.nice
config.nice.bat
*.la
Makefile*
!Makefile.am
Expand All @@ -204,6 +206,7 @@ PHP-Parser*/
src/*.so
src/*.dylib
src/*.dll
/x64/

# ignoring output of package.xml as it needs to be generated from ./prepare_release.sh in each publishing
package.xml
Expand Down
16 changes: 7 additions & 9 deletions Makefile.frag
View file
Open in desktop
Expand Up @@ -27,8 +27,8 @@ CMAKE_BUILD = CMAKE_BUILD_PARALLEL_LEVEL='' $(CMAKE) --build
CMAKE_BUILD_TYPE ?= RelWithDebInfo
CMAKE_TARGET = --config $(CMAKE_BUILD_TYPE) --target install

all: extension
.PHONY: all extension
all: extension
.PHONY: all extension

# configure for static aws-crt-ffi.a
build/aws-crt-ffi-static/CMakeCache.txt:
Expand All @@ -46,19 +46,17 @@ ext/awscrt.lo: ext/awscrt.c

ext/awscrt.c: build/aws-crt-ffi-static/libaws-crt-ffi.a ext/api.h ext/awscrt_arginfo.h

ext/awscrt_arginfo.h: ext/awscrt.stub.php gen_stub.php
ext/awscrt_arginfo.h: awscrt.stub.php gen_stub.php
ifeq ($(GENERATE_STUBS),1)
# install awscrt.stub.php to ext/
cp -v awscrt.stub.php ext/awscrt.stub.php
Comment on lines +51 to +52
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? We moved this to the main folder instead of ext/ and making a copy here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PECL removes .php from subdirectory, but not from the root directory. So, move it outside the ext/ is the easiest way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this in ext/? I am just confused about why do we need this file at two places.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

        # generate awscrt_arginfo.h
	php gen_stub.php --minimal-arginfo ext/awscrt.stub.php

This part will generate the awscrt_arginfo.h into the same folder as awscrt.stub.php.
The gen_sub.php is from the PHP build scripts, I didn't dig deep into it. We need awscrt_arginfo.h in ext/, so I just copy it around here.

# generate awscrt_arginfo.h
php gen_stub.php --minimal-arginfo ext/awscrt.stub.php
endif

# transform/install api.h from FFI lib
src/api.h: crt/aws-crt-ffi/src/api.h
php gen_api.php crt/aws-crt-ffi/src/api.h > src/api.h

# install api.h to ext/ as well
ext/api.h : src/api.h
cp -v src/api.h ext/api.h
ext/api.h : crt/aws-crt-ffi/src/api.h
php gen_api.php crt/aws-crt-ffi/src/api.h > ext/api.h

ext/php_aws_crt.h: ext/awscrt_arginfo.h ext/api.h

Expand Down
20 changes: 8 additions & 12 deletions Makefile.frag.w32
View file
Open in desktop
@@ -1,6 +1,5 @@

CMAKE=cmake.exe
COMPOSER_PHAR=C:\ProgramData\ComposerSetup\bin\composer.phar
PHP_BINARY=$(PHP_PREFIX)\php.exe

CMAKE_CONFIGURE = $(CMAKE) -DCMAKE_INSTALL_PREFIX=$(AWSCRT_DIR)\build\install -DCMAKE_PREFIX_PATH=$(AWSCRT_DIR)\build\install -DBUILD_TESTING=OFF -DCMAKE_BUILD_TYPE=$(CMAKE_BUILD_TYPE)
Expand All @@ -22,14 +21,11 @@ $(BUILD_DIR)\php_awscrt.dll: $(AWSCRT_DIR)\ext\awscrt.c
$(AWSCRT_DIR)\ext\awscrt.c: $(AWSCRT_DIR)\build\libaws-crt-ffi.lib $(AWSCRT_DIR)\ext\api.h $(AWSCRT_DIR)\ext\awscrt_arginfo.h

# transform\install api.h from FFI lib
$(AWSCRT_DIR)\src\api.h: $(AWSCRT_DIR)\crt\aws-crt-ffi\src\api.h
php $(AWSCRT_DIR)\gen_api.php $(AWSCRT_DIR)\crt\aws-crt-ffi\src\api.h > $(AWSCRT_DIR)\src\api.h

# install api.h to ext/ as well
$(AWSCRT_DIR)\ext\api.h : $(AWSCRT_DIR)\src\api.h
copy $(AWSCRT_DIR)\src\api.h $(AWSCRT_DIR)\ext\api.h

# Use PHPUnit to run tests
test-awscrt: install $(AWSCRT_DIR)\src\api.h $(BUILD_DIR)\php_awscrt.dll
$(PHP_BINARY) -c $(AWSCRT_DIR)\php-win.ini $(COMPOSER_PHAR) --working-dir=$(AWSCRT_DIR) update
$(PHP_BINARY) -c $(AWSCRT_DIR)\php-win.ini $(COMPOSER_PHAR) --working-dir=$(AWSCRT_DIR) run test-win
$(AWSCRT_DIR)\ext\api.h: $(AWSCRT_DIR)\crt\aws-crt-ffi\src\api.h
php $(AWSCRT_DIR)\gen_api.php $(AWSCRT_DIR)\crt\aws-crt-ffi\src\api.h > $(AWSCRT_DIR)\ext\api.h

# Get the dll directory to load
generate-php-ini:
@echo extension=$(BUILD_DIR)\php_awscrt.dll > php-win.ini
@echo extension=$(PHP_PREFIX)\ext\php_openssl.dll >> php-win.ini
@echo extension=$(PHP_PREFIX)\ext\php_mbstring.dll >> php-win.ini
0 ext/awscrt.stub.php → awscrt.stub.php
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion builder.json
View file
Open in desktop
Expand Up @@ -32,6 +32,6 @@
"NO_INTERACTION": "1"
},
"test_steps": [
["./run_tests"]
["./dev-scripts/run_tests.sh"]
]
}
8 changes: 4 additions & 4 deletions composer.json
View file
Open in desktop
Expand Up @@ -11,24 +11,24 @@
}
],
"config": {
"platform": {"php": "5.6"}
"platform": {"php": "7.4"}
waahm7 marked this conversation as resolved.
Show resolved Hide resolved
},
"minimum-stability": "alpha",
"require": {
"php": ">=5.5"
},
"require-dev": {
"phpunit/phpunit":"^4.8.35|^5.6.3"
"phpunit/phpunit":"^5.6.3"
},
"autoload": {
"classmap": [
"src/"
]
},
"scripts": {
"test": "./run_tests",
"test": "./dev-scripts/run_tests",
"test-extension": "@test",
"test-win": "run_tests"
"test-win": ".\\dev-scripts\\run_tests.bat"
},
"license": "Apache-2.0"
}
59 changes: 59 additions & 0 deletions dev-scripts/cleanup_build.py
View file
Open in desktop
@@ -0,0 +1,59 @@
import os
import glob
import shutil

TOOLS_DIR = os.path.dirname(os.path.abspath(__file__))
WORK_DIR = os.path.join(TOOLS_DIR, '..')

# Remove specified directories
DIRS_TO_REMOVE = [
'.deps',
'.libs',
'build',
'include',
'modules',
'vendor',
'autom4te.cache']

# Remove specified files
FILES_TO_REMOVE = [
'Makefile',
'Makefile.fragments',
'Makefile.global',
'Makefile.objects',
'config.guess',
'config.h',
'config.h.in',
'config.log',
'config.nice',
'config.status',
'config.sub',
'configure',
'configure.in',
'configure.ac',
'install-sh',
'libtool',
'ltmain.sh',
'missing',
'mkinstalldirs',
'run-tests.php',
'awscrt.la',
'composer.lock',
'ext/awscrt.stub.php',
'acinclude.m4',
'aclocal.m4',
'**/*.lo',
'**/*.o',
'**/*.la',
'**/*.a',
'*.tgz']

os.chdir(WORK_DIR)


for directory in DIRS_TO_REMOVE:
shutil.rmtree(directory, ignore_errors=True)

for pattern in FILES_TO_REMOVE:
for filepath in glob.glob(pattern):
os.remove(filepath)
graebm marked this conversation as resolved.
Show resolved Hide resolved
0 package.xml-template_post → dev-scripts/package.xml-template_post
View file
Open in desktop
File renamed without changes.
0 package.xml-template_pre → dev-scripts/package.xml-template_pre
View file
Open in desktop
File renamed without changes.