Skip to content

Change nanobind linkage to response file approach on macOS#1638

Merged
dmah42 merged 3 commits intogoogle:mainfrom
nicholasjng:nanobind-response-file
Oct 24, 2023
Merged

Change nanobind linkage to response file approach on macOS#1638
dmah42 merged 3 commits intogoogle:mainfrom
nicholasjng:nanobind-response-file

Conversation

@nicholasjng
Copy link
Contributor

@nicholasjng nicholasjng commented Jul 20, 2023

This change needs bazelbuild/bazel#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
Copy link
Contributor Author

nicholasjng commented Jul 20, 2023

This is blocked by the mentioned Bazel PR. I will check if I can implement this with the linker input suggested in #1636, but I will reopen this when we have a Bazel release supporting the linker input directly on the cc_library.

@nicholasjng nicholasjng reopened this Oct 20, 2023
@nicholasjng
Copy link
Contributor Author

nicholasjng commented Oct 20, 2023

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 nicholasjng force-pushed the nanobind-response-file branch from 2fe54cf to 92dca2f Compare October 23, 2023 09:56
@nicholasjng
Copy link
Contributor Author

nicholasjng commented Oct 23, 2023 •
edited
Loading

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
Change nanobind linkage to response file approach
e5cf23f 
This change needs bazelbuild/bazel#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
Fix bazel_skylib checksum, bump skylib version in MODULE.bazel
cd3d4f5
@nicholasjng
Bump Bazel to version 6.4.0 for linker response file support
ec025b4
@nicholasjng nicholasjng force-pushed the nanobind-response-file branch from 92dca2f to ec025b4 Compare October 23, 2023 12:18
@nicholasjng
Copy link
Contributor Author

nicholasjng commented Oct 23, 2023 •
edited
Loading

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
Copy link
Member

dmah42 commented Oct 23, 2023

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

@nicholasjng
Copy link
Contributor Author

nicholasjng commented Oct 23, 2023

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
Copy link
Member

dmah42 commented Oct 23, 2023

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
Copy link
Contributor Author

nicholasjng commented Oct 23, 2023

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
Copy link
Contributor Author

nicholasjng commented Oct 24, 2023

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.

@dmah42 dmah42 merged commit e45585a into google:main Oct 24, 2023
@nicholasjng nicholasjng deleted the nanobind-response-file branch April 15, 2024 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmah42 dmah42 dmah42 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nicholasjng @dmah42