Added copts_ list + move -std=c++17 in BUILD

[ArthurBandaryk]

copts_ list contains -Werror option in order to turn all warnings into the
errors. -Wno-error=deprecated-declarations option turns syscall warning
into a warning even if -Werror is specified.
Also I created an issue in the google/glog repo about syscall warning
#717.

[rjhuijsman]

All our other compiler flags seem to live in .bazelrc - shall we put these there too?

[while-false]

All our other compiler flags seem to live in .bazelrc - shall we put these there too?

No. Having these flags "inside" bazel allow us to do conditional flags depending on platform and compiler. The flags in questions are common, but I think this rightfully belongs where Arthur put it.

[ArthurBandaryk]

On my way to finish another PR to add select with compiler specific options. I mean the things you @while-false Gorm mentioned in the comment above:

copts_ = copts_common_ + select(
 # os specific opts
) + select(
 # compiler specific opts
)

[rjhuijsman]

All our other compiler flags seem to live in .bazelrc - shall we put these there too?

No. Having these flags "inside" bazel allow us to do conditional flags depending on platform and compiler. The flags in questions are common, but I think this rightfully belongs where Arthur put it.

OK, agreed!

[while-false]

I am just moving an email thread to github here so, bare with the crudeness:

... if we wanna add one more select with compiler specific options, does it mean that we should remove this options from .bazelrc?

I mean we have our .bazelrc with the following content:

# Specific Bazel build/test options.
build --enable_platform_specific_config
build:windows --compiler="clang-cl" --cxxopt="/std:c++17"
build:macos --cxxopt=-std=c++17
build:linux --cxxopt=-std=c++17
build:clang --action_env=CC=clang

I think we should make a distinction between "nice to have" flags and "need to have" flags - even if the distinction will not be clear cut.

Need to have flags are flags needed to be able to compile the package with a given os/compiler setup, independently of whether the package is checked out locally or if it is being imported/compiled as an external dependency.

Nice to have flags are flags that helps us primarily or that are useful when compiling, testing, debugging in a locally checked out repo.

I think all the need-to-have flags should live in BAZEL.build, in an appropriate place, be it a select(..) or a [].
Nice to have flags should live in .bazelrc. I believe, though I am not sure, that the .bazelrc is anyway only picked up if bazel is started in that directory, i.e., if doing a local build.

To return to the question. The c++17 (and variants) flag is to my knowledge a need to have flag, we can not compile without it, and it should live in BUILD.bazel. Other flags discussed in another issue could well belong in a .bazelrc file instead of in a BUILD.bazel: The various options in there, (e.g., --cxxopt='-fstandalone-debug') is clang specific but not necessary for compilation, so it is a need to have and could well be added to build:clang in .buildrc.

I hope this answers your questions and if anything is unclear, just let me know!

[while-false]

LGTM. Thanks for getting stuff in there Arthur!

[benh]

Let's actually define these in a separate .bzl file, how about bazel/copts.bzl. Then we can load them in BUILD.bazel and test/BUILD.bazel so that we don't have to copy them between both places.

[benh]

Please address minor comment otherwise LGTM!