bazel: similar build flags to cmake for corresponding build types (#4… #720

htuch · 2017-04-07T19:51:24Z

…15).

Bazel has 3 compilation modes out-of-the-box {fastbuild, opt, dbg}, see
https://bazel.build/versions/master/docs/bazel-user-manual.html#flag--compilation_mode.

The cmake build flags for normal/debug/server_only/asan/coverage are at
#415 (comment).

Below are the relevant flags (ignoring -include etc.) from the various builds. Lines that begin with

are those that include additional flags we're injecting.

fastbuild:

-U_FORTIFY_SOURCE -fstack-protector -Wall
-Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer '-std=c++0x'
-frandom-seed=...
-fPIC
*-DBAZEL_BRINGUP -Wall -Wextra -Werror -Wnon-virtual-dtor -Woverloaded-virtual
*-Wold-style-cast '-std=c++0x' -includeprecompiled/precompiled.h -DDEBUG
-fno-canonical-system-headers
-Wno-builtin-macro-redefined '-D__DATE__="redacted"' '-D__TIMESTAMP__="redacted"'
'-D__TIME__="redacted"'

dbg:

-U_FORTIFY_SOURCE -fstack-protector -Wall
-Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -g '-std=c++0x'
-frandom-seed=...
-fPIC
*-DBAZEL_BRINGUP -Wall -Wextra -Werror -Wnon-virtual-dtor
*-Woverloaded-virtual -Wold-style-cast '-std=c++0x' -includeprecompiled/precompiled.h -ggdb3 -DDEBUG
-fno-canonical-system-headers
-Wno-builtin-macro-redefined '-D__DATE__="redacted"' '-D__TIMESTAMP__="redacted"'
'-D__TIME__="redacted"'

opt:

-U_FORTIFY_SOURCE -fstack-protector -Wall
-Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -g0 -O2
'-D_FORTIFY_SOURCE=1' -DNDEBUG -ffunction-sections -fdata-sections '-std=c++0x'
-frandom-seed=...
*-DBAZEL_BRINGUP -Wall -Wextra -Werror -Wnon-virtual-dtor
*-Woverloaded-virtual -Wold-style-cast '-std=c++0x' -includeprecompiled/precompiled.h
-fno-canonical-system-headers
-Wno-builtin-macro-redefined '-D__DATE__="redacted"' '-D__TIMESTAMP__="redacted"'
'-D__TIME__="redacted"'

This PR requires #716 to merge, since #716 provides some important changes to how the headers are
exported from the 3rd party deps to allow -Wextra and -Wold-style-cast to work.

I've verified that we now have a 100% static envoy-static build, which ldd tells me is not a dynamic
binary.

% bazel build -c opt //source/exe:envoy-static.stripped
Target //source/exe:envoy-static.stripped up-to-date:
bazel-bin/source/exe/envoy-static.stripped

% ldd bazel-bin/source/exe/envoy-static.stripped
not a dynamic executable

% ls -lh bazel-bin/source/exe/envoy-static.stripped
-r-xr-xr-x 1 htuch eng 8.1M Apr 7 15:46 bazel-bin/source/exe/envoy-static.stripped

…voyproxy#415). Bazel has 3 compilation modes out-of-the-box {fastbuild, opt, dbg}, see https://bazel.build/versions/master/docs/bazel-user-manual.html#flag--compilation_mode. The cmake build flags for normal/debug/server_only/asan/coverage are at envoyproxy#415 (comment). Below are the relevant flags (ignoring -include etc.) from the various builds. Lines that begin with * are those that include additional flags we're injecting. fastbuild: -U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer '-std=c++0x' -frandom-seed=... -fPIC *-DBAZEL_BRINGUP -Wall -Wextra -Werror -Wnon-virtual-dtor -Woverloaded-virtual *-Wold-style-cast '-std=c++0x' -includeprecompiled/precompiled.h -DDEBUG -fno-canonical-system-headers -Wno-builtin-macro-redefined '-D__DATE__="redacted"' '-D__TIMESTAMP__="redacted"' '-D__TIME__="redacted"' dbg: -U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -g '-std=c++0x' -frandom-seed=... -fPIC *-DBAZEL_BRINGUP -Wall -Wextra -Werror -Wnon-virtual-dtor *-Woverloaded-virtual -Wold-style-cast '-std=c++0x' -includeprecompiled/precompiled.h -ggdb3 -DDEBUG -fno-canonical-system-headers -Wno-builtin-macro-redefined '-D__DATE__="redacted"' '-D__TIMESTAMP__="redacted"' '-D__TIME__="redacted"' opt: -U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -g0 -O2 '-D_FORTIFY_SOURCE=1' -DNDEBUG -ffunction-sections -fdata-sections '-std=c++0x' -frandom-seed=... *-DBAZEL_BRINGUP -Wall -Wextra -Werror -Wnon-virtual-dtor *-Woverloaded-virtual -Wold-style-cast '-std=c++0x' -includeprecompiled/precompiled.h -fno-canonical-system-headers -Wno-builtin-macro-redefined '-D__DATE__="redacted"' '-D__TIMESTAMP__="redacted"' '-D__TIME__="redacted"' This PR requires envoyproxy#716 to merge, since envoyproxy#716 provides some important changes to how the headers are exported from the 3rd party deps to allow -Wextra and -Wold-style-cast to work. I've verified that we now have a 100% static envoy-static build, which ldd tells me is not a dynamic binary. % bazel build -c opt //source/exe:envoy-static.stripped Target //source/exe:envoy-static.stripped up-to-date: bazel-bin/source/exe/envoy-static.stripped % ldd bazel-bin/source/exe/envoy-static.stripped not a dynamic executable % ls -lh bazel-bin/source/exe/envoy-static.stripped -r-xr-xr-x 1 htuch eng 8.1M Apr 7 15:46 bazel-bin/source/exe/envoy-static.stripped

mattklein123 · 2017-04-07T20:02:22Z

ldd bazel-bin/source/exe/envoy-static.stripped
not a dynamic executable

That seems not right, or at least not what I'm used to seeing.

mklein@localhost:~/Source/envoy-private$ ldd build_local/envoy/source/exe/envoy
	linux-vdso.so.1 =>  (0x00007ffe86975000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f81f5b05000)
	librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007f81f58fd000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f81f55f3000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f81f522a000)
	/lib64/ld-linux-x86-64.so.2 (0x00005582f81a7000)

Are you somehow statically linking everything? I've had a lot of issues with full static linking and would prefer we still dynamically link to system libraries (or at least have that option).

htuch · 2017-04-07T20:03:21Z

Yes, I statically linked everything. I'll switch back to what you had before.

mattklein123 · 2017-04-08T00:44:30Z

I dropped a note into the build GH issue, but FYI we also need to link in build ID. (This change doesn't need to cover that but since we were discussing parity here I thought i would mention it).

htuch · 2017-04-11T20:56:50Z

Waiting for input on bazelbuild/bazel#2805 for the --build-id link issue.

mattklein123 · 2017-04-14T03:34:21Z

bazel/envoy_build_system.bzl

@@ -3,22 +3,19 @@ load("@protobuf_git//:protobuf.bzl", "cc_proto_library")
 ENVOY_COPTS = [
    # TODO(htuch): Remove this when Bazel bringup is done.
    "-DBAZEL_BRINGUP",
-    "-fno-omit-frame-pointer",


Is this default in bazel?

mattklein123 · 2017-04-14T03:35:25Z

bazel/envoy_build_system.bzl

    "-std=c++0x",
    "-includeprecompiled/precompiled.h",
-]
+] + select({
+    "//bazel:opt_build": [],


-DNDEBUG? Still need debug symbols?

Bazel automatically sets NDEBUG in opt, see the output in the above commit messages. We're now actually in a strange situation where Bazel is setting NDEBUG in opt and we set DEBUG in fastbuild/dbg. Shouldn't we just have one define indicating debug and use the negated logic where necessary?

I think DEBUG is just habit from previous jobs. I don't think we use it anywhere and I agree it's silly. Let's just kill it. Can you grep for it though just to make sure we don't use it anywhere?

Confirmed it's not used, switching to NDEBUG only.

htuch · 2017-04-14T15:44:22Z

The --build-id fix is in #767, so we can ship this one once it's approved and tests pass.

mattklein123 · 2017-04-14T23:56:11Z

What's preventing this from passing tests? build image upgrade?

htuch · 2017-04-17T13:43:19Z

It wasn't building as the headers included from external deps were subject to the -Wold-style-cast -Wextra treatment. I've changed how they get included so that they are now treated as "system" headers, by using includes rather than strip_include_prefix to reference them in ci/prebuild/BUILD. Will push after validating on all Bazel flows and build types.

…ra etc. applying to external dep headers.

htuch added 2 commits April 7, 2017 15:50

fix_format

2423d5f

Don't statically link system libraries.

63a3b1c

htuch force-pushed the build-flags-pr branch from 0bb7ad2 to a57fc16 Compare April 11, 2017 20:53

htuch added 2 commits April 11, 2017 16:54

Bump hash for CI image.

b55e524

Merge remote-tracking branch 'upstream/master' into build-flags-pr

f47df63

htuch force-pushed the build-flags-pr branch from a57fc16 to f47df63 Compare April 11, 2017 20:55

mattklein123 reviewed Apr 14, 2017

View reviewed changes

Merge remote-tracking branch 'upstream/master' into build-flags-pr

1a75897

mattklein123 mentioned this pull request Apr 15, 2017

bazel: rewrite MD5 .note.gnu.build-id with truncated git SHA1. #767

Merged

htuch added 2 commits April 17, 2017 09:31

Merge remote-tracking branch 'upstream/master' into build-flags-pr

4969692

Remove DEBUG for fastbuild/dbg in favor of NDEBUG on opt.

b28d32e

htuch added 2 commits April 17, 2017 09:44

s/strip_include_prefix/includes/g in ci/prebuilt/BUILD to avoid -Wext…

619dd3c

…ra etc. applying to external dep headers.

Add --define debug_symbols=yes support.

e4baffe

mattklein123 approved these changes Apr 17, 2017

View reviewed changes

mattklein123 merged commit 7bce1eb into envoyproxy:master Apr 17, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bazel: similar build flags to cmake for corresponding build types (#4… #720

bazel: similar build flags to cmake for corresponding build types (#4… #720

htuch commented Apr 7, 2017

mattklein123 commented Apr 7, 2017

htuch commented Apr 7, 2017

mattklein123 commented Apr 8, 2017

htuch commented Apr 11, 2017

mattklein123 Apr 14, 2017

htuch Apr 14, 2017

mattklein123 Apr 14, 2017 •

edited

Loading

htuch Apr 14, 2017

mattklein123 Apr 14, 2017

htuch Apr 17, 2017

htuch commented Apr 14, 2017

mattklein123 commented Apr 14, 2017

htuch commented Apr 17, 2017

bazel: similar build flags to cmake for corresponding build types (#4… #720

bazel: similar build flags to cmake for corresponding build types (#4… #720

Conversation

htuch commented Apr 7, 2017

mattklein123 commented Apr 7, 2017

htuch commented Apr 7, 2017

mattklein123 commented Apr 8, 2017

htuch commented Apr 11, 2017

mattklein123 Apr 14, 2017

Choose a reason for hiding this comment

htuch Apr 14, 2017

Choose a reason for hiding this comment

mattklein123 Apr 14, 2017 • edited Loading

Choose a reason for hiding this comment

htuch Apr 14, 2017

Choose a reason for hiding this comment

mattklein123 Apr 14, 2017

Choose a reason for hiding this comment

htuch Apr 17, 2017

Choose a reason for hiding this comment

htuch commented Apr 14, 2017

mattklein123 commented Apr 14, 2017

htuch commented Apr 17, 2017

mattklein123 Apr 14, 2017 •

edited

Loading