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

Build failure: phpExtensions.opentelemetry 1.0.2 on MacOS Sonoma #304809

Open
avrahamappel opened this issue Apr 17, 2024 · 15 comments · Fixed by #304986
Open

Build failure: phpExtensions.opentelemetry 1.0.2 on MacOS Sonoma #304809

avrahamappel opened this issue Apr 17, 2024 · 15 comments · Fixed by #304986
Assignees

Comments

@avrahamappel
Copy link

Steps To Reproduce

Steps to reproduce the behavior:

  1. build opentelemetry extension on MacOS
nix build github:r-ryantm/nixpkgs/a399db43527b481d2af275ba058a4dc1d36ed373#php81Extensions.opentelemetry

Build log

error: builder for '/nix/store/gsma784mx1iy2sbnbvd2wdxkld1asc3z-php-opentelemetry-1.0.2.drv' failed with exit code 2;
       last 10 log lines:
       >              ~~~~~~~~~~^~~~~~~~~~~
       > /private/tmp/nix-build-php-opentelemetry-1.0.2.drv-1/source/ext/otel_observer.c:231:24: note: remove extraneous parentheses around the comparison to silence this warning
       >         if ((type_mask == IS_UNDEF)) {
       >             ~          ^          ~
       > /private/tmp/nix-build-php-opentelemetry-1.0.2.drv-1/source/ext/otel_observer.c:231:24: note: use '=' to turn this equality comparison into an assignment
       >         if ((type_mask == IS_UNDEF)) {
       >                        ^~
       >                        =
       > 1 error generated.
       > make: *** [Makefile:211: otel_observer.lo] Error 1
       For full logs, run 'nix-store -l /nix/store/gsma784mx1iy2sbnbvd2wdxkld1asc3z-php-opentelemetry-1.0.2.drv'.

Additional context

This occurred after this PR was merged: #303437

Notify maintainers

@drupol

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

 - system: `"aarch64-darwin"`
 - host os: `Darwin 23.4.0, macOS 14.4.1`
 - multi-user?: `yes`
 - sandbox: `no`
 - version: `nix-env (Nix) 2.18.1`
 - channels(root): `"nixpkgs"`
 - nixpkgs: `/Users/avraham.appel/.nix-defexpr/channels/nixpkgs`

Add a 👍 reaction to issues you find important.

@drupol
Copy link
Contributor

drupol commented Apr 17, 2024

Sadly I don't have a Mac to debug and fix this, sorry :(.

However, I'm gladly accepting sponsors so I can buy one, I'm lurking on the new Mac Mini M4 arriving at the end of this year.

@avrahamappel
Copy link
Author

@drupol

Thanks

I often run into compilation errors like these on macos due to stricter clang errors, is there any well-known workaround for this kind of thing?

@avrahamappel
Copy link
Author

So I can get it to compile by disabling the parentheses-equality check:

opentelemetry.overrideAttrs (attrs: {
  NIX_CFLAGS_COMPILE = (attrs.NIX_CFLAGS_COMPILE or "") + " -Wno-parentheses-equality";
})

I assume that at some point the upstream package may fix the code that causes the error. What should nixpkgs do in the meantime? Add the workaround? Mark it as broken?

@drupol
Copy link
Contributor

drupol commented Apr 18, 2024

That's great! I will provide the fix before lunch then. I'll ping you when it's done.

drupol added a commit to drupol/nixpkgs that referenced this issue Apr 18, 2024
@gaelreyrol
Copy link
Contributor

@drupol I believe this issue should be reported on the original repository. Maybe we should also make sure the compilation is done with gcc instead of clang, as I don't see any reference to clang in the original repository.

@gaelreyrol
Copy link
Contributor

gaelreyrol commented Apr 22, 2024

@drupol An issue has been created about this: open-telemetry/opentelemetry-php#1283

@gaelreyrol gaelreyrol reopened this Apr 22, 2024
@drupol
Copy link
Contributor

drupol commented Apr 22, 2024

Thank you @gaelreyrol !

@brettmc
Copy link

brettmc commented Apr 24, 2024

Hello from upstream. I'm one of the maintainers of the opentelemetry PHP extension. I'm happy to fix that error and get a new beta out, but I'd also like to update our CI/checks to pick future issues up. We test against aline:3.16 and debian:bookworm using the compiler that's provided by their respective package managers.

I can't find any way to trigger the parentheses warning mentioned here, does anybody know how to dial up the strictness to be closer/equivalent to what macos has?

@gaelreyrol
Copy link
Contributor

Hi @brettmc! I was wondering if it was not coming from clang which is the default CC compiler on MacOS. Have you tried with it instead of gcc?

@gaelreyrol gaelreyrol self-assigned this Apr 24, 2024
@drupol
Copy link
Contributor

drupol commented Apr 24, 2024

Just my 2 cents: clang is indeed the default C compiler on MacOS and it is stricter than GCC. Usually GCC is a bit more relax before adopting clang constraints.

@brettmc
Copy link

brettmc commented Apr 28, 2024

Thanks for the info, I've since added macos to our builds, and it successfully revealed the issue reported here. There should be a new version in the coming days, and hopefully this will be the last macos incompatibility that makes it out into the wild.

@brettmc
Copy link

brettmc commented May 1, 2024

I've just now published opentelemetry 1.0.3beta1 to pecl. Could somebody verify that it works on macos, please?

@avrahamappel
Copy link
Author

@brettmc I can check in a couple days, when I get back to my work computer

@avrahamappel
Copy link
Author

@brettmc Works for me (using nixpkgs obv :)

With the new version it builds without needing to disable the compiler flag 👍

@brettmc
Copy link

brettmc commented May 4, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants