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.

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

fix 8.4 segfaults, test against PHP nightly #153

Merged
merged 9 commits into from
Sep 9, 2024

Conversation

brettmc
Copy link
Collaborator

@brettmc brettmc commented Sep 6, 2024

  • fix segfaults on 8.4, via zend_get_internal_function_extension_handle (see php/php-src@62ebe82)
  • build against PHP nightly on PR and daily schedule
  • use setup-php for linux builds (which drops Alpine builds)

Fixes: open-telemetry/opentelemetry-php#1376

@brettmc brettmc requested a review from a team September 6, 2024 00:47
- build against PHP nightly on PR and daily schedule
- use setup-php for linux builds (which drops Alpine builds)
@brettmc brettmc changed the title test against PHP nightly fix 8.4 segfaults, test against PHP nightly Sep 7, 2024
@brettmc brettmc marked this pull request as draft September 7, 2024 07:37
@brettmc brettmc marked this pull request as ready for review September 7, 2024 08:04
@andypost
Copy link
Contributor

andypost commented Sep 7, 2024

FYI using this patch it shows failure on 32-bits like this

TEST 53/67 [tests/span_attribute/function_params_non_simple.phpt]
========DIFF========
--
       }
       ["three"]=>
       object(Closure)#2 (%d) {%A
013+     ["name"]=>
014+     string(148) "{closure:/builds/alpine/aports/testing/php84-pecl-opentelemetry/src/opentelemetry-1.1.0beta2/tests/span_attribute/function_params_non_simple.php:24}"
015+     ["file"]=>
016+     string(135) "/builds/alpine/aports/testing/php84-pecl-opentelemetry/src/opentelemetry-1.1.0beta2/tests/span_attribute/function_params_non_simple.php"
017+     ["line"]=>
018+     int(24)
       }
       ["four"]=>
       NULL
     }
     string(3) "foo"
018- string(4) "post"
024+ zend_mm_heap corrupted
========DONE========
FAIL Check if function non-simple types can be passed as function params [tests/span_attribute/function_params_non_simple.phpt] 

@brettmc
Copy link
Collaborator Author

brettmc commented Sep 7, 2024

FYI using this patch it shows failure on 32-bits like

Was able to replicate this using i386/alpine docker image...

- remove the ability to pass complex items as attributes (since the spec already
  disallows them, they'd get dropped later anyway)
- update alpine dockerfile to also support 32bit builds, which enables local
  testing against a 32bit platform
@brettmc
Copy link
Collaborator Author

brettmc commented Sep 8, 2024

It should be green now against 32bit builds, @andypost
I'm open to any suggestions on how we could add 32bit builds to our pipeline, but I've added the capability to our local builds for now, so it's at least possible to be caught in future.

docker/Dockerfile.alpine Outdated Show resolved Hide resolved
@andypost
Copy link
Contributor

andypost commented Sep 8, 2024

Thank you! It passed on all arches!

btw I noticed that CI here is not using latest stable Alpine version (3.20)

@brettmc brettmc merged commit d8d9246 into open-telemetry:main Sep 9, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[opentelemetry-php-instrumentation] PHP 8.4 compatibility (10 failed tests)
3 participants