Skip to content

Fix implicit declaration error in zlib + macOS#24979

Merged
apolcyn merged 3 commits intogrpc:masterfrom
qnighy:darwin-zlib-implicit-declaration
Jan 11, 2021
Merged

Fix implicit declaration error in zlib + macOS#24979
apolcyn merged 3 commits intogrpc:masterfrom
qnighy:darwin-zlib-implicit-declaration

Conversation

@qnighy
Copy link
Copy Markdown
Contributor

@qnighy qnighy commented Dec 14, 2020

@qnighy
Copy link
Copy Markdown
Contributor Author

qnighy commented Jan 7, 2021

@apolcyn could you take a look at this too? This is similar to #24962 but the approach is different.

@apolcyn apolcyn added kokoro:force-run area/core release notes: yes Indicates if PR needs to be in release notes labels Jan 7, 2021
@apolcyn apolcyn self-assigned this Jan 7, 2021
@apolcyn
Copy link
Copy Markdown
Contributor

apolcyn commented Jan 8, 2021 •
edited
Loading

This seems reasonable, since we are basically assuming that zlib's probe for whether unistd.h exists will always be successful. But we are already assuming that unistd.h is available since grpc sources themselves won't compile without it.

But if I understand correctly, this isn't just an OSX issue but an issue whenever -Wimplicit-function-declaration is set. So, should we define HAVE_UNISTD_H for all zlib sources in the Makefile build, not just for OSX?

Also, "sanity checks" failed here because the Makefile is actually auto-generated from templates/Makefile.template. What you'll need to do is to hand edit templates/Makefile.template and then run tools/buildgen/generate_projects.sh (ping this PR if you need help to do that).

Adding @nicolasnoble for a second googler's review

qnighy added 2 commits January 8, 2021 14:39
@qnighy
Merge branch 'master' into darwin-zlib-implicit-declaration
6d6c5ca
@qnighy
Always apply -DHAVE_UNISTD_H
d158915 
This commit also moves the CPPFLAGS config from Makefile, which is
autogenerated, to build_handwritten.yaml, which is literally
handwritten.
@qnighy
Copy link
Copy Markdown
Contributor Author

qnighy commented Jan 8, 2021

Thanks! Removed ifeq ($(SYSTEM),Darwin) and it allowed me to move the configuration into build_handwritten.yaml.

@apolcyn apolcyn mentioned this pull request Jan 8, 2021
Closed
@apolcyn
Copy link
Copy Markdown
Contributor

apolcyn commented Jan 8, 2021

basic tests python windows: #25124

@apolcyn apolcyn merged commit b7df39f into grpc:master Jan 11, 2021
apolcyn pushed a commit to apolcyn/grpc that referenced this pull request Jan 11, 2021
@qnighy @apolcyn
Backport "Fix implicit declaration error in zlib + macOS grpc#24979"
58ef4d7
apolcyn added a commit that referenced this pull request Jan 12, 2021
@apolcyn
Merge pull request #25133 from apolcyn/backport_fix_zlib_implicit_dec…
96d2e95 
…laration

Backport "Fix implicit declaration error in zlib + macOS #24979"
@dependabot dependabot bot mentioned this pull request Mar 17, 2021
Closed
@lidizheng lidizheng mentioned this pull request May 26, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@apolcyn apolcyn apolcyn approved these changes

+1 more reviewer

@nicolasnoble nicolasnoble nicolasnoble approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@nicolasnoble nicolasnoble

@apolcyn apolcyn

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

can't compile grpc gem on MacOS BigSur

4 participants

@qnighy @apolcyn @nicolasnoble @grpc-kokoro