Skip to content

Fix implicit decalaration in rb_event_thread.c#24962

Merged
apolcyn merged 1 commit intogrpc:masterfrom
qnighy:ruby-extension-implicit-declaration
Jan 6, 2021
Merged

Fix implicit decalaration in rb_event_thread.c#24962
apolcyn merged 1 commit intogrpc:masterfrom
qnighy:ruby-extension-implicit-declaration

Conversation

@qnighy
Copy link
Copy Markdown
Contributor

@qnighy qnighy commented Dec 11, 2020

As seen in #24881, the current grpc-ruby doesn't build on recent macOS.

This PR does not fix the exact problem mentioned above (because the problematic file is in ./third_party), but the similar problem in rb_event_thread.c.

@nicolasnoble

@thiagoss
Copy link
Copy Markdown

thiagoss commented Jan 6, 2021

Easy to reproduce the issue just by running rake on the ruby directory.

The issue is:

rb_event_thread.c:118:3: error: implicit declaration of function 'grpc_ruby_init' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
  grpc_ruby_init();
  ^
rb_event_thread.c:132:3: error: implicit declaration of function 'grpc_ruby_shutdown' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
  grpc_ruby_shutdown();

Tested and this fix works for me.

@apolcyn apolcyn self-assigned this Jan 6, 2021
@apolcyn apolcyn added release notes: yes Indicates if PR needs to be in release notes lang/ruby kokoro:force-run labels Jan 6, 2021
@apolcyn apolcyn self-requested a review January 6, 2021 19:31
Copy link
Copy Markdown
Contributor

@apolcyn apolcyn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm -- I'll merge once tests pass

@apolcyn apolcyn merged commit c95ed7a into grpc:master Jan 6, 2021
@qnighy qnighy deleted the ruby-extension-implicit-declaration branch January 7, 2021 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lang/ruby release notes: yes Indicates if PR needs to be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants