chore: use upstream e-dant/watcher headers and build system

dunglas commented 1 month ago

See https://github.com/e-dant/watcher/issues/58.

dunglas commented 4 weeks ago

I don't get why we get this linking issue https://github.com/dunglas/frankenphp/actions/runs/11543390970/job/32127719767?pr=1119 (same in the Docker image). It works on my computer (^^) but I'm on Mac.

Maybe do you have an idea @e-dant?

e-dant commented 4 weeks ago

I don't get why we get this linking issue https://github.com/dunglas/frankenphp/actions/runs/11543390970/job/32127719767?pr=1119 (same in the Docker image). It works on my computer (^^) but I'm on Mac.

Maybe do you have an idea @e-dant?

That looks correct to me. Running this locally in a container and strace'ing some stuff, the execve stuff also looks fine to me.

Attaching output from a run of

strace -e execve -v -s 9999 --follow-forks go run ./internal/testcli/main.go &> output.txt
with export CGO_CFLAGS='-I/usr/local/include -I/usr/include/php/20220829 -I/usr/include/php/20220829/main -I/usr/include/php/20220829/TSRM -I/usr/include/php/20220829/Zend -I/usr/include/php/20220829/ext -I/usr/include/php/20220829/ext/date/lib' ; export CGO_LDFLAGS='-L/usr/local/lib -lwatcher-c' in the environment
with watcher installed via git clone https://github.com/e-dant/watcher && (cd watcher && git checkout next) && cmake -S watcher/ -B watcher/out/tmp -DBUILD_BIN=ON -DBUILD_LIB=ON -DBUILD_HDR=ON && cmake --build watcher/out/tmp && cmake --install watcher/out/tmp

output.txt

The symbols must not be visible in the shared library. Everything else looks fine.

e-dant commented 4 weeks ago

The symbols must not be visible in the shared library. Everything else looks fine.

strings /usr/local/lib/libwatcher-c.so.0.13.1 | grep wtr | wc -l
0

What?

e-dant commented 4 weeks ago

One alternative is this:

$ wget https://github.com/e-dant/watcher/releases/download/0.13.1/$(arch)-unknown-linux-gnu.tar -qO- | tar -xvf - && nm -D $(arch)-unknown-linux-gnu/libwatcher-c.so.0.13.1 | rg ' T '
x86_64-unknown-linux-gnu/
x86_64-unknown-linux-gnu/tw
x86_64-unknown-linux-gnu/libwatcher-c.so
x86_64-unknown-linux-gnu/watcher-c.h
x86_64-unknown-linux-gnu/libwatcher-c.so.sha256sum
x86_64-unknown-linux-gnu/tw.sha256sum
x86_64-unknown-linux-gnu/watcher.sha256sum
x86_64-unknown-linux-gnu/watcher.hpp
x86_64-unknown-linux-gnu/watcher-c.h.sha256sum
x86_64-unknown-linux-gnu/libwatcher-c.so.0.13.1
x86_64-unknown-linux-gnu/watcher
x86_64-unknown-linux-gnu/libwatcher-c.so.0.13.1.sha256sum
x86_64-unknown-linux-gnu/watcher.hpp.sha256sum
000000000000799b T wtr_watcher_close
0000000000007e65 T wtr_watcher_open

Which does have these symbols...

Not sure how I broke the CMake build, but something is wrong there.

e-dant commented 4 weeks ago

So, it turns out that our entire library was being optimized away with -fwhole-program...

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 8f4a78e..98c6120 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -94,7 +94,6 @@ if(NOT WIN32 AND NOT IS_CC_MSVC)
     set(COMPILE_OPTIONS
       "${COMPILE_OPTIONS}"
       "-fexpensive-optimizations"
-      "-fwhole-program"
     )
   endif()
 endif()

This fixes it...

e-dant commented 4 weeks ago

So, it turns out that our entire library was being optimized away with -fwhole-program...

Version 0.13.2 fixes this:

$ cmake -S watcher -B wout && cmake --build wout && cmake --install wout && go build frankenphp/internal/testcli/main.go

Should work

dunglas commented 4 weeks ago

Thank you very much @e-dant, this works!

e-dant commented 3 weeks ago

❤️

dunglas / frankenphp

chore: use upstream e-dant/watcher headers and build system #1119