Skip to content

Commit

Permalink
Avoid leaking LFS flags to reverse dependencies (#1730)
Browse files Browse the repository at this point in the history
Follow up of #1725.

`defines` propagates to reverse dependencies, while `local_defines`
don't. If we use `defines` then there's risk of ODR violation:

Suppose a user have a cc_library foo that depends on bar and benchmark:

    cc_library(name = "foo", deps = [":bar", "@com_github_google_benchmark//:benchmark"])

And bar has a class that has LFS-dependant ABI:

    cc_library(name = "foo")

    class Bar {
        off_t member;
    };

Bar would be compiled without LFS, but linked to foo when assuming LFS is enabled.

So we limit LFS to within the library only. benchmark does not have
LFS dependant public ABIs so it should be fine.
  • Loading branch information
afq984 authored Jan 5, 2024
1 parent e3824e7 commit 07c98d5
Showing 1 changed file with 6 additions and 4 deletions.
10 changes: 6 additions & 4 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,6 @@ cc_library(
}),
defines = [
"BENCHMARK_STATIC_DEFINE",
# Turn on Large-file Support
"_FILE_OFFSET_BITS=64",
"_LARGEFILE64_SOURCE",
"_LARGEFILE_SOURCE",
] + select({
":perfcounters": ["HAVE_LIBPFM"],
"//conditions:default": [],
Expand All @@ -67,6 +63,12 @@ cc_library(
# Using `defines` (i.e. not `local_defines`) means that no
# dependent rules need to bother about defining the macro.
linkstatic = True,
local_defines = [
# Turn on Large-file Support
"_FILE_OFFSET_BITS=64",
"_LARGEFILE64_SOURCE",
"_LARGEFILE_SOURCE",
],
strip_include_prefix = "include",
visibility = ["//visibility:public"],
deps = select({
Expand Down

0 comments on commit 07c98d5

Please sign in to comment.