-
Notifications
You must be signed in to change notification settings - Fork 24
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.
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
Build Linux packages for the extension #107
Build Linux packages for the extension #107
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! A few comments and observations from me. Do you suggest that we should run this build as part of each push, and also do the publish when we create a release? If so, we will also need some actions added under .github
#### Function php_ini_file_path ################################################ | ||
function php_ini_file_path() { | ||
php_command -i \ | ||
| grep 'Configuration File (php.ini) Path =>' \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could this be replaced by php-config --ini-path
?
|
||
################################################################################ | ||
#### Function php_config_d_path ################################################ | ||
function php_config_d_path() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
php-config --ini-dir
?
|
||
echo "Uninstalling ${OPENTELEMETRY_INI_FILE_NAME} for supported SAPI's" | ||
|
||
# Detect installed SAPI's |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
php-config --php-sapis
?
BUILD_EXT_DIR=instrumentation/native/_build/linux-x86-64-release/ext/ | ||
createPackage | ||
|
||
NAME="${NAME_BACKUP}-debugsymbols-linux-x86-64" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor, but I think just debug
would be ok here?
PHP_INSTRUMENTATION_DIR=/opt/open-telemtry/php-instrumentation | ||
EXTENSION_DIR="${PHP_INSTRUMENTATION_DIR}/extensions" | ||
EXTENSION_CFG_DIR="${PHP_INSTRUMENTATION_DIR}/etc" | ||
BOOTSTRAP_FILE_PATH="${PHP_INSTRUMENTATION_DIR}/src/bootstrap_php_part.php" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think BOOTSTRAP_FILE_PATH is vestigial from elastic apm, and not needed here
################################################################################ | ||
#### Function php_api ########################################################## | ||
function php_api() { | ||
php_command -i \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if there was a php-config --api
, we could get everything more effectively from there...
dont clobber the dockerfile arg for PHP_CONFIG_OPTS, which includes --with-pear. this will allow us to pear package within the build
Hopefully this will integrate nicely with #109 when it's merged - we can publish these packages as build artifacts for each github actions run, and also as release artifacts. |
test that post hooks still execute after die/exit is called
…lemetry#109) we already build windows binaries, so let's make them available to users. On each github action, the binaries are published as build artifacts. When a tag is created, we now create a draft release from the workflow (instead of manually creating the release via the GUI), which means we can also upload the binaries as release artifacts. Give PECL package the same treatment, so that maintainers don't have to build it manually and upload. After this is merged, the release process for the extension will change slightly: it relies on a tag being created (but not a release). You can't do that from the GUI, so there's a change coming to dev-tools which will create the tag, and from there the workflow will take care of creating the release in draft form. The final check is for a maintainer to eyeball the release, make any modifications to the text, and publish it.
Co-authored-by: Brett McBride <[email protected]>
adds 8.3 local builds, and also github build of our debug/dev image
* Include PHP 8.3 in build matrix * Bump deps in actions * Upgrade default php version * Switch to php/setup-php-sdk * Keep actions up to date --------- Co-authored-by: Brett McBride <[email protected]>
* Isolate exception state for hooks * Consistent naming. * Fix formatting and test typo * Fixed test list in PECL package.xml * Save and restore execute_data->opline
* adding test for post hook type error if an invalid typehint is provided for first param of post callback, this would previously cause a hang. fixed by open-telemetry#118, this test confirms the fix works in this scenario. * improve test, add to package.xml * remove try/catch
* Support modifying named params * Removed use of function not available in all PHP versions
…n-telemetry#122) * Unregister INI in MSHUTDOWN instead of RSHUTDOWN * Added test * Fix test name typo
…etry#120) * Fix modifying extra args, limit argument expansion * Added some comments and one more test * Fix func_get_args crash on excess internal func params * Fix typo in comment
- warning: unused variable '...' [-Wunused-variable] - warning: format '%u' expects argument of type 'unsigned int', but argument 4 has type 'zend_ulong' {aka 'long unsigned int'} [-Wformat=]
* clang format * update checkout action to v4
…hook (open-telemetry#127) * Use E_CORE_WARNING for logging * Add test, fix other tests --------- Co-authored-by: Brett McBride <[email protected]>
Co-authored-by: Brett McBride <[email protected]>
Going to close this for now; please reopen if you're interested in finishing this out. |
Closes open-telemetry/opentelemetry-php#1151