search

google / benchmark

A microbenchmark support library
Apache License 2.0
8.94k stars 1.62k forks source link

Change nanobind linkage to response file approach on macOS #1638

Closed nicholasjng closed 11 months ago

nicholasjng commented 1 year ago

This change needs https://github.com/bazelbuild/bazel/pull/18952 to be merged first.

Fixes macOS linkage of GBM's nanobind bindings on macOS by supplying a linker response file instead of -undefined dynamic_lookup. The latter has since been deprecated on macOS.

nicholasjng commented 1 year ago

This is blocked by the mentioned Bazel PR. I will check if I can implement this with the linker input suggested in https://github.com/google/benchmark/issues/1636, but I will reopen this when we have a Bazel release supporting the linker input directly on the cc_library.

nicholasjng commented 11 months ago

Reopening, as Bazel 6.4.0 was released with the linker response file support merged in.

Give me some time to polish this again, I would request a review (hopefully) when I'm set :)

nicholasjng commented 11 months ago

Rebased on main, fixed the bazel_skylib checksum.

I think the changes from #1676 can be applied separately.

cc @dmah42

EDIT: Windows is failing, will investigate shortly.

EDIT2: windows-2019 does not have Bazel 6.4.0, bumping to latest (or merging #1676) should do the trick.

nicholasjng commented 11 months ago

Thanks for the merge, rebased on current main.

What's your take on bumping Bazel dependencies? Some of them are quite stale, so I updated a few in this PR (not rules_python though, since that is misconfigured right now and raises an error in the latest v0.26.0, I'm on it).

EDIT: Seems windows GH runners have not upgraded to Bazel v6.4.0 yet.

dmah42 commented 11 months ago

i have no concerns about bumping dependencies if they're stale. is this ready for me to re-review?

nicholasjng commented 11 months ago

i have no concerns about bumping dependencies if they're stale. is this ready for me to re-review?

yes, all set, GitHub won't let me request an initial review, that's all :)

Re: the windows-latest build failure, that should be out of the way in the next actions/runners Windows image release, you can check the release page here.

dmah42 commented 11 months ago

i have no concerns about bumping dependencies if they're stale. is this ready for me to re-review?

yes, all set, GitHub won't let me request an initial review, that's all :)

Re: the windows-latest build failure, that should be out of the way in the next actions/runners Windows image release, you can check the release page here.

should we go back to windows-2019 or would that not help? what's the timeline for the release? looks like the last one was 2 weeks ago.

nicholasjng commented 11 months ago

should we go back to windows-2019 or would that not help? what's the timeline for the release? looks like the last one was 2 weeks ago.

That won't help, it was actually the failure on windows-2019 that made me assume we need latest. Timeline is probably anywhere within this week, cadence seems to be <every 2 weeks.

nicholasjng commented 11 months ago

New windows-latest version is out, with Bazel 6.4.0: https://github.com/actions/runner-images/releases/tag/win22%2F20231023.1

Should be good to go now.