bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
23.21k stars 4.07k forks source link

CcInfo.compilation_context does not contain data from `implementation_deps` #19663

Open tpasternak opened 1 year ago

tpasternak commented 1 year ago

Description of the bug:

Implementation_deps doc: https://bazel.build/reference/be/c-cpp#cc_library.implementation_deps

If I understand correctly, if a target libB depends directly on libA via implementation_deps it should have access to A's includes, defines etc. Only the target that would depend on B (and transitively on A) would have them hidden. Unfortunately, from the perspective of CcInfo.compilation_context, the targets which are implementation_deps are just ignored.

Which category does this issue belong to?

C++/Objective-C Rules

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

File: libA/BUILD
cc_library(
    name = "libA",
    srcs = ["lib.cpp"],
    defines = ["FOO=1"],
    visibility = ["//visibility:public"],
)
File: libB/BUILD
cc_library(
    name = "libB",
    srcs = ["lib.cpp"],
    defines = ["BAR=0"],
    implementation_deps = ["//libA"],
    visibility = ["//visibility:public"],
)

Command:

bazel cquery //libB --output=starlark --starlark:expr="providers(target)['CcInfo'].compilation_context.defines"

Observed:

depset(["BAR=0"]

Expected:

depset(["BAR=0", "foo=1"])

EDIT: this also applies to other fields, like includes

Which operating system are you running Bazel on?

Fedora aarch64

What is the output of bazel info release?

release 6.3.2

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

brentleyjones commented 1 year ago

I see Intellij integration is having an issue with this. rules_xcodeproj had to work around this as well: https://github.com/MobileNativeFoundation/rules_xcodeproj/blob/f7e1fd996e1677760f7e8efc49b66a7a62a7d79b/xcodeproj/internal/compilation_providers.bzl#L20-L55

thii commented 1 year ago

This looks WAI to me. Implementation deps have to be propagated separately (as they are currently), otherwise they would be visible to downstream targets. If any, there should be a CcInfo.implementation_compilation_context.

tpasternak commented 1 year ago

the downstream targets wouldn't necessarily see the deps in their own compilation contexts

thii commented 1 year ago

But it's not possible to hide those if we merge implementation deps' compilation context into the normal deps' compilation context.

tpasternak commented 1 year ago

Ok, but when bazel build things it somehow can distinguish sources of the flags and not use transitive ones in case of implementation_deps. Can't it be done in providers, too?

pcjanzen commented 1 year ago

What is the real problem you're trying to solve? The CcInfo for libB should be, and is, exactly what is required by the clients of libB, it's not supposed to be the compilation context for compiling the sources of libB itself.

sfc-gh-abalik commented 1 year ago

The real problem comes from this issue in the IntelliJ Bazel plugin where implementation_deps are not propagated into the struct that the plugin uses to build a given target.

I proposed a fix for this in https://github.com/bazelbuild/intellij/pull/5220, which sounds a lot like what @thii is suggesting:

Implementation deps have to be propagated separately (as they are currently), otherwise they would be visible to downstream targets. If any, there should be a CcInfo.implementation_compilation_context.

tpasternak commented 1 year ago

@iancha1992 @Pavank1992 @sgowroji Would it be possible for you to tell as if the current behavior is expected or not? To it looks like a bug, because I'd expect that the compilation_context provides all the flags that are required to actually compile the file

keertk commented 1 year ago

cc @buildbreaker2021 who may be able to help with this question