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

grpc: Fix build #62916

Closed
wants to merge 1 commit into from
Closed

grpc: Fix build #62916

wants to merge 1 commit into from

Conversation

Mizux
Copy link
Contributor

@Mizux Mizux commented Oct 15, 2020

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

@Mizux
Copy link
Contributor Author

Mizux commented Oct 15, 2020

this is a fix for the PR #62803

@SMillerDev
Copy link
Member

Please also include a test that would indicate this in the future.

@Mizux
Copy link
Contributor Author

Mizux commented Oct 15, 2020

don't get it, sorry.
I mean:

  1. I won't fix Needs to rebuild gRPC package after abseil version bump #62589 which is for me, "have a bot job which recompile all abseil direct dependencies" (ed i also don't know if it's possible to do it with your package manager), so it should be done in a separate PR
  2. the build currently fail without this patch (missing abseil symbols)

@SMillerDev
Copy link
Member

Since grpc depends on abseil here: https://github.com/Homebrew/homebrew-core/pull/62916/files#diff-2325170da0acac021ad144f5df99ce0062d90d8b489de582e8bef1e3c0ad1008R29 that means that any abseil update will run the test block in the formula. Clearly the current test isn't good enough or it would have blocked your abseil PR. Please update the test block to add something that fails in the current situation.

@Mizux
Copy link
Contributor Author

Mizux commented Oct 15, 2020

[0]─[/usr/local/Cellar]
[^v^]─corentinl@corentinl-macbookair %otool -L grpc/1.32.0_1/lib/*.dylib | grep absl
[1]─[/usr/local/Cellar]

on my way to try to build a test, but at least no grpc dylib depends on abseil -> build dependency 🤔 ?

@Mizux
Copy link
Contributor Author

Mizux commented Oct 15, 2020

abseil is build as static libs (.a) need to check if grpc headers i.e. public include depends on abseil ones or not

@Mizux
Copy link
Contributor Author

Mizux commented Oct 15, 2020

no installed headers depends on abseil only the grpc-config.cmake and the .pc file

[0]─[/usr/local/Cellar]
[^v^]─corentinl@corentinl-macbookair %grep -rIn "absl_" grpc/1.32.0_1 
grpc/1.32.0_1/lib/pkgconfig/grpc.pc:11:Libs: -L${libdir} -lgrpc -laddress_sorting -lre2 -lupb -lcares -lz -labsl_raw_hash_set -labsl_hashtablez_sampler -labsl_exponential_biased -labsl_hash -labsl_bad_variant_access -labsl_city -labsl_status -labsl_cord -labsl_bad_optional_access -labsl_str_format_internal -labsl_synchronization -labsl_graphcycles_internal -labsl_symbolize -labsl_demangle_internal -labsl_stacktrace -labsl_debugging_internal -labsl_malloc_internal -labsl_time -labsl_time_zone -labsl_civil_time -labsl_strings -labsl_strings_internal -labsl_throw_delegate -labsl_int128 -labsl_base -labsl_spinlock_wait -labsl_raw_logging_internal -labsl_log_severity -labsl_dynamic_annotations
grpc/1.32.0_1/lib/pkgconfig/gpr.pc:11:Libs: -L${libdir} -lgpr -labsl_str_format_internal -labsl_synchronization -labsl_graphcycles_internal -labsl_symbolize -labsl_demangle_internal -labsl_stacktrace -labsl_debugging_internal -labsl_malloc_internal -labsl_time -labsl_time_zone -labsl_civil_time -labsl_strings -labsl_strings_internal -labsl_throw_delegate -labsl_int128 -labsl_base -labsl_spinlock_wait -labsl_raw_logging_internal -labsl_log_severity -labsl_dynamic_annotations
grpc/1.32.0_1/lib/pkgconfig/grpc++_unsecure.pc:11:Libs: -L${libdir} -lgrpc++_unsecure -labsl_status -labsl_cord -labsl_bad_optional_access -labsl_str_format_internal -labsl_synchronization -labsl_graphcycles_internal -labsl_symbolize -labsl_demangle_internal -labsl_stacktrace -labsl_debugging_internal -labsl_malloc_internal -labsl_time -labsl_time_zone -labsl_civil_time -labsl_strings -labsl_strings_internal -labsl_throw_delegate -labsl_int128 -labsl_base -labsl_spinlock_wait -labsl_raw_logging_internal -labsl_log_severity -labsl_dynamic_annotations
grpc/1.32.0_1/lib/pkgconfig/grpc++.pc:11:Libs: -L${libdir} -lgrpc++ -labsl_raw_hash_set -labsl_hashtablez_sampler -labsl_exponential_biased -labsl_hash -labsl_bad_variant_access -labsl_city -labsl_status -labsl_cord -labsl_bad_optional_access -labsl_str_format_internal -labsl_synchronization -labsl_graphcycles_internal -labsl_symbolize -labsl_demangle_internal -labsl_stacktrace -labsl_debugging_internal -labsl_malloc_internal -labsl_time -labsl_time_zone -labsl_civil_time -labsl_strings -labsl_strings_internal -labsl_throw_delegate -labsl_int128 -labsl_base -labsl_spinlock_wait -labsl_raw_logging_internal -labsl_log_severity -labsl_dynamic_annotations
grpc/1.32.0_1/lib/pkgconfig/grpc_unsecure.pc:11:Libs: -L${libdir} -lgrpc_unsecure -labsl_status -labsl_cord -labsl_bad_optional_access -labsl_str_format_internal -labsl_synchronization -labsl_graphcycles_internal -labsl_symbolize -labsl_demangle_internal -labsl_stacktrace -labsl_debugging_internal -labsl_malloc_internal -labsl_time -labsl_time_zone -labsl_civil_time -labsl_strings -labsl_strings_internal -labsl_throw_delegate -labsl_int128 -labsl_base -labsl_spinlock_wait -labsl_raw_logging_internal -labsl_log_severity -labsl_dynamic_annotations
grpc/1.32.0_1/lib/cmake/grpc/gRPCConfig.cmake:17:if(NOT absl_FOUND)
[0]─[/usr/local/Cellar]

so I'm not sure grpc once installed depends on abseil...
Feel free to do whatever you want with this PR...

@Mizux
Copy link
Contributor Author

Mizux commented Oct 16, 2020

@SMillerDev So after investigating grpc I think you need two separate PR, this one fix the grpc build, and an other one once grpc fix its cmake-config, pkg-config (i.e. release a new version) If i add a cmake/pc test in this PR it will just block it since its a bug in grpc extra file, while you could already at least fix the grpc build (concerning abseil c++17 bump) using this PR imho

ps: If you want, we can have a chat on discord (ortools discord https://discord.gg/ENkQrdf) or IIRC (freenode user: mizux) where we can talk...

@jeroen jeroen mentioned this pull request Nov 16, 2020
@jeroen
Copy link
Contributor

jeroen commented Nov 16, 2020

This was already merged in 8912ce9. Can be closed.

@Mizux Mizux closed this Nov 16, 2020
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Dec 17, 2020
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Dec 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants