Skip to content

Add windows static build support#164

Merged
brettmc merged 4 commits intoopen-telemetry:mainfrom
crazywhalecc:patch-1
Feb 10, 2025
Merged

Add windows static build support#164
brettmc merged 4 commits intoopen-telemetry:mainfrom
crazywhalecc:patch-1

Conversation

@crazywhalecc
Copy link
Contributor

@crazywhalecc crazywhalecc commented Feb 6, 2025

This patch will allow opentelemetry compiling with php-src statically on Windows.

@crazywhalecc crazywhalecc requested a review from a team as a code owner February 6, 2025 04:20
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Feb 6, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@brettmc
Copy link
Contributor

brettmc commented Feb 6, 2025

Hi @crazywhalecc thanks for this! Is there a way we can test this build as part of our github actions, and do you think we should also build/publish a binary for it?

@crazywhalecc
Copy link
Contributor Author

Thanks for the quick reply. I don't know much about the structure and CI of the opentelemetry project at the moment, but this change should only affect people who want to statically compile extensions to PHP, while the traditional way of distributing dlls is not affected. That is, we don't need to redistribute dlls or other binaries.

To test this PR, we need to start by downloading php-src locally or writing an additional GHA. I'm working on adding opentelemetry support to the static-php-cli project, where the results are already checked by a simple sanity check, but I haven't tested each method in depth or hooked into more comprehensive extension testing.

If the project requires additional code to fully support static compilation, please point it out.

@brettmc
Copy link
Contributor

brettmc commented Feb 6, 2025

It looks like the win/8.0/ts build is broken with this change.

@crazywhalecc
Copy link
Contributor Author

crazywhalecc commented Feb 6, 2025

I may have found the reason.

In the config.w32 of the PHP build, the definition of EXTENSION() is function EXTENSION(extname, file_list, shared, cflags, dllname, obj_dir), and the cflags parameter was incorrectly set to shared before, which made the real /DZEND_ENABLE_STATIC_TSRMLS_CACHE=1 parameter unable to take effect all the time.

When dynamically compiling opentelemetry locally, the following line can also be successfully built in ZTS mode:

EXTENSION('opentelemetry', 'opentelemetry.c otel_observer.c', PHP_OPENTELEMETRY_SHARED);

At the same time, I found that this CI not only had problems with 8.0-ts, but all versions of ts tests failed, but the checks of other versions were skipped.

@brettmc
Copy link
Contributor

brettmc commented Feb 7, 2025

I may have found the reason.

Can you add that fix to this PR and we will see if it un-breaks ts builds?

@crazywhalecc
Copy link
Contributor Author

I've tried to fix this, and so far I've been able to compile and test locally and it seems to work fine.

@brettmc brettmc merged commit e8f494a into open-telemetry:main Feb 10, 2025
25 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.

2 participants