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

Building 1.63.0 or newer on macOS fails. #36654

Open
moubctez opened this issue May 17, 2024 · 12 comments
Open

Building 1.63.0 or newer on macOS fails. #36654

moubctez opened this issue May 17, 2024 · 12 comments

Comments

@moubctez
Copy link

What version of gRPC and what language are you using?

1.63.0 1.64.0

What operating system (Linux, Windows,...) and version?

macOS

What runtime / compiler are you using (e.g. python version or version of gcc)

clang

What did you do?

Try to build 1.63.0 or 1.64.0 (older versions are fine).

What did you expect to see?

Builds fine

What did you see instead?

Build fails with

Undefined symbols for architecture arm64:
 "_google__protobuf__EnumOptions_msg_init", referenced from:
     __upb_EnumDefs_New in enum_def.c.o
     __upb_EnumDefs_New in enum_def.c.o
     __upb_EnumDefs_New in enum_def.c.o
 "_google__protobuf__EnumValueOptions_msg_init", referenced from:
     __upb_EnumValueDefs_New in enum_value_def.c.o
     __upb_EnumValueDefs_New in enum_value_def.c.o
     __upb_EnumValueDefs_New in enum_value_def.c.o
 "_google__protobuf__ExtensionRangeOptions_msg_init", referenced from:
     __upb_ExtensionRanges_New in extension_range.c.o
     __upb_ExtensionRanges_New in extension_range.c.o
     __upb_ExtensionRanges_New in extension_range.c.o
 "_google__protobuf__FeatureSetDefaults_msg_init", referenced from:
     _upb_DefPool_SetFeatureSetDefaults in def_pool.c.o
     _upb_DefPool_SetFeatureSetDefaults in def_pool.c.o
 "_google__protobuf__FeatureSet_msg_init", referenced from:
     _upb_DefBuilder_AddFileToPool in def_pool.c.o
     __upb_FieldDef_Create in field_def.c.o
     __upb_DefBuilder_GetOrCreateFeatureSet in def_builder.c.o
     __upb_DefBuilder_DoResolveFeatures in def_builder.c.o
     __upb_DefBuilder_DoResolveFeatures in def_builder.c.o
 "_google__protobuf__FieldOptions_msg_init", referenced from:
     __upb_FieldDef_Create in field_def.c.o
     __upb_FieldDef_Create in field_def.c.o
 "_google__protobuf__FileDescriptorProto_msg_init", referenced from:
     __upb_DefPool_LoadDefInitEx in def_pool.c.o
     __upb_DefPool_LoadDefInitEx in def_pool.c.o
 "_google__protobuf__FileOptions_msg_init", referenced from:
     __upb_FileDef_Create in file_def.c.o
     __upb_FileDef_Create in file_def.c.o
 "_google__protobuf__MessageOptions_msg_init", referenced from:
     __upb_MessageDefs_New in message_def.c.o
     __upb_MessageDefs_New in message_def.c.o
     __upb_MessageDefs_New in message_def.c.o
 "_google__protobuf__MethodOptions_msg_init", referenced from:
     __upb_MethodDefs_New in method_def.c.o
     __upb_MethodDefs_New in method_def.c.o
     __upb_MethodDefs_New in method_def.c.o
 "_google__protobuf__OneofOptions_msg_init", referenced from:
     __upb_OneofDefs_New in oneof_def.c.o
     __upb_OneofDefs_New in oneof_def.c.o
     __upb_OneofDefs_New in oneof_def.c.o
 "_google__protobuf__ServiceOptions_msg_init", referenced from:
     __upb_ServiceDefs_New in service_def.c.o
     __upb_ServiceDefs_New in service_def.c.o
     __upb_ServiceDefs_New in service_def.c.o
ld: symbol(s) not found for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Anything else we should know about your project / environment?

Fix:

--- CMakeLists.txt.orig	2024-05-16 01:01:03.000000000 +0000
+++ CMakeLists.txt
@@ -3682,6 +3682,7 @@ target_include_directories(upb_json_lib
 )
 target_link_libraries(upb_json_lib
   ${_gRPC_ALLTARGETS_LIBRARIES}
+  grpc++_unsecure
   utf8_range_lib
   upb_message_lib
 )
@@ -3883,6 +3884,7 @@ target_include_directories(upb_textforma
 )
 target_link_libraries(upb_textformat_lib
   ${_gRPC_ALLTARGETS_LIBRARIES}
+  grpc++_unsecure
   utf8_range_lib
   upb_message_lib
 )
@eugeneo
Copy link
Contributor

eugeneo commented May 17, 2024

Thank you for this report. Can you clarify what targets were you trying to build? Please confirm if you obtained gRPC from our GitHub repository or from some other place.

@moubctez
Copy link
Author

I guess this is all targets with cmake argsuments: -DBUILD_SHARED_LIBS=ON -DgRPC_ABSL_PROVIDER=package -DgRPC_BENCHMARK_PROVIDER=package -DgRPC_CARES_PROVIDER=package -DgRPC_PROTOBUF_PROVIDER=package -DgRPC_RE2_PROVIDER=package -DgRPC_SSL_PROVIDER=package -DgRPC_ZLIB_PROVIDER=package.

I confirm the release tarball is from the official GitHub release.

To be more specific, I build the package using PkgSrc where I've incorporated the patch: http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/net/grpc/?only_with_tag=MAIN

@jcar87
Copy link

jcar87 commented Jun 5, 2024

Facing the same problem - building the shared library on macOS with all dependencies provided externally. Using protobuf 27

I can see homebrew's test bot is also experiencing this:
Homebrew/homebrew-core#170194

@h-vetinari
Copy link
Contributor

Anything else we should know about your project / environment?

Fix:

--- CMakeLists.txt.orig	2024-05-16 01:01:03.000000000 +0000
+++ CMakeLists.txt
@@ -3682,6 +3682,7 @@ target_include_directories(upb_json_lib
 )
 target_link_libraries(upb_json_lib
   ${_gRPC_ALLTARGETS_LIBRARIES}
+  grpc++_unsecure
   utf8_range_lib
   upb_message_lib
 )
@@ -3883,6 +3884,7 @@ target_include_directories(upb_textforma
 )
 target_link_libraries(upb_textformat_lib
   ${_gRPC_ALLTARGETS_LIBRARIES}
+  grpc++_unsecure
   utf8_range_lib
   upb_message_lib
 )

This doesn't work (for shared builds) because it creates a cyclic dependence, as grpc++_unsecure itself depends on those two libraries.

@yashykt
Copy link
Member

yashykt commented Jul 9, 2024

@veblush Does something need to change with respect to how we're vendoring upb?

@yashykt yashykt assigned veblush and unassigned yashykt Jul 9, 2024
@yashykt
Copy link
Member

yashykt commented Jul 9, 2024

I guess this is all targets with cmake argsuments: -DBUILD_SHARED_LIBS=ON -DgRPC_ABSL_PROVIDER=package -DgRPC_BENCHMARK_PROVIDER=package -DgRPC_CARES_PROVIDER=package -DgRPC_PROTOBUF_PROVIDER=package -DgRPC_RE2_PROVIDER=package -DgRPC_SSL_PROVIDER=package -DgRPC_ZLIB_PROVIDER=package.

Does this work without providing the additional arguments? -DBUILD_SHARED_LIBS=ON to be specific?

@h-vetinari
Copy link
Contributor

Does this work without providing the additional arguments? -DBUILD_SHARED_LIBS=ON to be specific?

I removed all the "package" specs and things compile on osx now. I'm assuming that the vendored upb (v26.1 since 34a7e76, until the so-far-unreleased 62401f6 bumped to v27.0) refers to symbols in protobuf that don't exist anymore or have been renamed.

That said, vendoring protobuf (much less all the other packages) is not an option for distribution, aside from the fact that the option -DgRPC_PROTOBUF_PROVIDER="package" provides "official" support for doing that, and it has been working for years.

One thing that surprises me, is that the failure is essentially the same no matter if the system-provided protobuf is older or newer than (or even the same as!) the vendored one:

grpc version vendored protobuf system protobuf Result
1.63.1 26.1 25.3
1.64.2 26.1 27.2
1.64.2 26.1 -
1.64.2 26.1 26.1 ❌❓❗

I haven't diffed the third_party/upb folder to see if it actually matches upb 26.1, but it's strange that there seems to be no combination of system protobuf that makes this work.

Finally this problem only seems to appear on osx, definitely not on linux (not sure about windows yet, running into #35794 there).

@jcar87
Copy link

jcar87 commented Jul 15, 2024

I guess this is all targets with cmake argsuments: -DBUILD_SHARED_LIBS=ON -DgRPC_ABSL_PROVIDER=package -DgRPC_BENCHMARK_PROVIDER=package -DgRPC_CARES_PROVIDER=package -DgRPC_PROTOBUF_PROVIDER=package -DgRPC_RE2_PROVIDER=package -DgRPC_SSL_PROVIDER=package -DgRPC_ZLIB_PROVIDER=package.

Does this work without providing the additional arguments? -DBUILD_SHARED_LIBS=ON to be specific?

@yashykt building it as a static library appears to work - is only the shared variant on macOS - when built against an external protobuf. But that would still be a regression, I think it's quite common to use grpc as a shared library
Also interesting that this only happens on macOS, but it could also be one of those cases where it's an issue on all platforms and only the macos linker surfaces this

@jcar87
Copy link

jcar87 commented Jul 15, 2024

I suspect the missing symbols are in descriptor.upb_minitable.c - which is built into the grpc and grpc_unsecure cmake targets.

The symbols are not built nor referenced in the upb_textformat_lib or upb_json_lib targets, so the linker on macOS complains when creating the dylib shared libraries for those targets.

Reasons it doesn't fail in other scenarios:

  • When all libraries are static (macOS or other) - the executable will link against grpc which also links against both upb_json_lib and upb_textformat_lib - so the linker can see all symbols when creating the executable.
  • When we are building shared libraries on Linux, the linker does not error out for "undefined references". On Linux, when creating shared libraries, the linker ignores undefined references (that is, --allow-shlib-undefined, see docs). When creating an executable, the missing symbols in upb_textformat_lib and upb_json_lib are visible and provided by libgprc.so so it works. The darwin linker has a different behaviour and it will fail on undefined references.
  • On Windows these two targets are explicitly set to be static, even if libgrpc isn't - so the shared libgrpc would have these static libraries "embedded" inside it - so it all works.

So there is a bit of circular dependency here:

  • libgprc.so and .dylib link against libupb_json_lib and libupb_textformat_lib
  • but those libraries refer to symbols that are only provided by libgrpc.so

I suppose one could pass -undefined dynamic_lookup when creating those two libupb targets, such that it behaves more like Linux - if those targets are not supposed to be used in a standalone manner (they're not "public") and are always brought in via libgrpc - then the downstreams consumers will link successfully - but it still begs the question as to whether this is the best approach.

TL;DR - on macOS, add this somewhere in the CMakeLists.txt - and it works, and it should work the same as Linux, but it's unclear whether something else needs to be properly fixed upstream:

            target_link_options(upb_textformat_lib PRIVATE -Wl,-undefined,dynamic_lookup)
            target_link_options(upb_json_lib PRIVATE -Wl,-undefined,dynamic_lookup)

@h-vetinari
Copy link
Contributor

So there is a bit of circular dependency here:

Yeah, there's a library layering problem there IMO. But good point about -Wl,-undefined,dynamic_lookup, I'll try that.

@yashykt
Copy link
Member

yashykt commented Jul 16, 2024

Update from @veblush: We don't have a good solution for this yet, but eventually this should go away once we are able to use CMake from upb instead of vendoring upb ourselves.

@saschasc
Copy link

Any progress on this issue? I am currently struggling source building the gRPC Conan package with clang 18.1.8 on macOS.

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

No branches or pull requests

8 participants