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

[V8] getsectdatafromheader_64 is deprecated in macOS 13.0 #45

Closed
KianNH opened this issue Sep 28, 2022 · 11 comments · Fixed by #276
Closed

[V8] getsectdatafromheader_64 is deprecated in macOS 13.0 #45

KianNH opened this issue Sep 28, 2022 · 11 comments · Fixed by #276
Labels
bug Something isn't working macos present in workerd on macOS

Comments

@KianNH
Copy link
Contributor

KianNH commented Sep 28, 2022

Builds on macOS 13.0 will fail since getsectdatafromheader_64 is deprecated as of 13.0, replaced by getsectiondata(). This is in V8's codebase as opposed to workerd, so it may just be worth noting in the README (or not, since 13.0 is still in beta).

Culprit file

V8: external/v8/src/base/platform/platform-darwin.cc:56:22

Build output

external/v8/src/base/platform/platform-darwin.cc:56:22: error: 'getsectdatafromheader_64' is deprecated: first deprecated in macOS 13.0 [-Werror,-Wdeprecated-declarations]
    char* code_ptr = getsectdatafromheader_64(
                     ^~~~~~~~~~~~~~~~~~~~~~~~
                     use getsectiondata()
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/mach-o/getsect.h:130:14: note: 'getsectdatafromheader_64' has been explicitly marked deprecated here
extern char *getsectdatafromheader_64(
             ^
1 error generated.
Target //src/workerd/server:workerd failed to build

System information

System Version:	macOS 13.0 (22A5352e)
Kernel Version:	Darwin 22.1.0
@jasnell jasnell added bug Something isn't working macos present in workerd on macOS labels Sep 28, 2022
@kentonv
Copy link
Member

kentonv commented Sep 28, 2022

Hmm, we should figure out how to build V8 with warnings not treated as errors, otherwise we're going to be persistently running into problems like this where something was deprecated but V8 hasn't handled it yet.

Or maybe we should blanket disable warnings when compiling dependencies... it's not like we're going to go fix their warnings.

@jasnell
Copy link
Member

jasnell commented Sep 29, 2022

Disabling them makes sense but I'd still like to be able to bring them back optionally. Can we turn them off by default and have an option to show them? They often give us a good heads up on upcoming changes.

@kentonv
Copy link
Member

kentonv commented Sep 29, 2022

@jasnell To be clear I think we still want warnings when our own code uses deprecated V8 APIs. What we don't want is warnings when V8 internal code uses deprecated APIs from its own dependencies.

@dio
Copy link
Contributor

dio commented Oct 27, 2022

@KianNH I can successfully compile on Ventura (macOS 13) by using: https://github.com/cloudflare/workerd/compare/main...dio:workerd:allow-catalina-ventura.patch?full_index=1 (basically to achieve this: #45 (comment)). Not sure if this is the right way here (by introducing another v8 patch).

Note: the above patch also contains -Wno-range-loop-analysis just in case people want to build it on Catalina (10.15) as well.
Relevant: https://bugs.chromium.org/p/v8/issues/detail?id=13428.

Also, do you have a plan allowing people to install this via homebrew? I can try to set up the formula if it is desirable (it has releases).

Thank you!

@caass
Copy link
Contributor

caass commented Nov 8, 2022

Just bumping to say this now breaks building workerd on the latest Xcode

ETA workaround:

  1. Uninstall Xcode 14.1 (drag from Applications -> Trash)
  2. Install Xcode 14.0.1 (Download link, needs Apple Developer Account)
  3. Extract archive and drag to Applications
  4. Open new (old) Xcode once
  5. Run bazel clean --expunge in workerd directory
  6. bazel build should work now

@dio
Copy link
Contributor

dio commented Nov 8, 2022

Or you can apply the patch here: https://github.com/cloudflare/workerd/compare/main...dio:workerd:allow-catalina-ventura.patch?full_index=1

image

@lgyhit
Copy link

lgyhit commented Dec 23, 2022

Or you can apply the patch here: https://github.com/cloudflare/workerd/compare/main...dio:workerd:allow-catalina-ventura.patch?full_index=1

image

how to use this patch?

@lgyhit
Copy link

lgyhit commented Dec 23, 2022

Disabling them makes sense but I'd still like to be able to bring them back optionally. Can we turn them off by default and have an option to show them? They often give us a good heads up on upcoming changes.

how can i disable them, i can't turn it off, this makes me feel confused.

@kentonv
Copy link
Member

kentonv commented Dec 23, 2022

@lgyhit Nice patch, if you send a pull request I'll merge it.

@mrbbot
Copy link
Contributor

mrbbot commented Jan 16, 2023

Hey @dio! Would you be interested in submitting a PR containing your patch?

@dio
Copy link
Contributor

dio commented Jan 17, 2023

@mrbbot I have submitted it here: #276.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working macos present in workerd on macOS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants