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

ci: upgrade to Bazel 0.27.0 #116

Closed
junr03 opened this issue Jun 17, 2019 · 7 comments
Closed

ci: upgrade to Bazel 0.27.0 #116

junr03 opened this issue Jun 17, 2019 · 7 comments
Assignees
Labels

Comments

@junr03
Copy link
Member

junr03 commented Jun 17, 2019

Bazel released a new version. CI currently runs on 0.26.1.

There is a starter PR to do so. However, we encountered build issues with Android NDK on Linux due to missing pipe2 definitions.

@junr03 junr03 self-assigned this Jun 17, 2019
@junr03 junr03 added the ci label Jun 17, 2019
@jin
Copy link

jin commented Jun 19, 2019

Bazel 0.27 comes with support for Android NDK 19 and 20, which promoted some warnings into errors according to the NDK build system maintainers guide. These were promoted because they could lead into bugs with NDK-Clang compilations.

See https://source.bazel.build/bazel/+/00e29b7cd80df6a2762bc8b4b035f5a99466f5f6:src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/r19/AndroidNdkCrosstoolsR19.java;l=65-100 for more information.

@junr03
Copy link
Member Author

junr03 commented Jul 3, 2019

Getting back to this. Yep, @jin that is what I am seeing:

/var/tmp/_bazel_josenino/9e680707426f56f890add48ff19d437d/execroot/__main__/external/com_github_libevent_libevent/evutil.c:2637:6: error: implicit declaration of function 'pipe2' is invalid in C99 [-Werror,-Wimplicit-function-declaration]

So definitely the promoted warnings.

@rebello95
Copy link
Contributor

Just adding a note here - #155 is on hold until we update to bazel 0.27.0

@keith keith self-assigned this Jul 8, 2019
@keith
Copy link
Member

keith commented Jul 8, 2019

Filed bazel-contrib/rules_foreign_cc#289 upstream about our current inability to easily check in CMake configuration if we're targeting Android. If we could do that I believe this patch would be enough libevent/libevent#850

In the meantime I will try and do this in envoy only for Android

keith added a commit to keith/envoy that referenced this issue Jul 8, 2019
The Android NDK doesn't assume `_GNU_SOURCE`, clang only assumes it for
C++ builds, not C builds. libevent uses `pipe2` which in the NDK is only
exposed in the header when these variables are set. In bazel 0.27.0
these implicit use became an error.

More details:

- envoyproxy/envoy-mobile#116
- libevent/libevent#850

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith
Copy link
Member

keith commented Jul 8, 2019

Here's the temporary envoy fix envoyproxy/envoy#7497

lizan pushed a commit to envoyproxy/envoy that referenced this issue Jul 9, 2019
The Android NDK doesn't assume `_GNU_SOURCE`, clang only assumes it for
C++ builds, not C builds. libevent uses `pipe2` which in the NDK is only
exposed in the header when these variables are set. In bazel 0.27.0
these implicit use became an error.

More details:

- envoyproxy/envoy-mobile#116
- libevent/libevent#850

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith
Copy link
Member

keith commented Jul 9, 2019

This gives us that fix #236

@keith
Copy link
Member

keith commented Jul 12, 2019

#246

@keith keith closed this as completed Jul 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants