open-telemetry / opentelemetry-collector

OpenTelemetry Collector
https://opentelemetry.io
Apache License 2.0
4.29k stars 1.42k forks source link

build failures on aarch64-darwin due to dependency shoenig/go-m1cpu #7668

Closed 06kellyjac closed 1 year ago

06kellyjac commented 1 year ago

Describe the bug A clear and concise description of what the bug is.

When building with macOS SDK >= 12.0, and targeting macOS Big Sur (10.16/11.0), the MAC_OS_VERSION_12_0 macro is defined, but we still can't use the kIOMainPortDefault definition, as it was only introduced on macOS 12.0. The previous patch only handles the case of compiling directly with macOS SDK versions < 12.0, but fails to address targeting earlier OS versions with new version of the SDK.

This has been fixed upstream: https://github.com/shoenig/go-m1cpu/pull/8

And https://github.com/shirou/gopsutil @ latest has been updated but there's not been a tagged release

mx-psi commented 1 year ago

We will update to the latest gopsutil version when released. Note that while we build for darwin/arm64 on CI, we do so with cgo disabled, since recommend against using cgo for portability reasons.

06kellyjac commented 1 year ago

the go-m1cpu dependency requires cgo though?

This package requires the use of CGO.

Or are you saying if you force gco off it avoids using go-m1cpu for the build?

mx-psi commented 1 year ago

Or are you saying if you force gco off it avoids using go-m1cpu for the build?

That's it, gopsutil can be used with cgo off (with reduced functionality), so I assume since our build is passing, the go-m1cpu dependency is not being used

06kellyjac commented 1 year ago

Yep disabling CGO avoided that codepath so the update patch was no-longer necessary.

https://github.com/NixOS/nixpkgs/pull/232231

We'll keep it GCO disabled in line with upstream so we'll hopefully not run into any extra issues not caught in upstream CI

I'm happy to close this

mx-psi commented 1 year ago

Thanks for following up, I agree that this seems resolved :)