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

cc_shared_library ignores linkopts from header only cc_library deps #21884

Closed
keith opened this issue Apr 3, 2024 · 7 comments
Closed

cc_shared_library ignores linkopts from header only cc_library deps #21884

keith opened this issue Apr 3, 2024 · 7 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules type: bug

Comments

@keith
Copy link
Member

keith commented Apr 3, 2024

Description of the bug:

With this example:

cc_library(
    name = "foo",
    srcs = ["foo.cpp"],
    linkopts = ["-lcompression"],
)

cc_library(
    name = "baz",
    hdrs = ["baz.hpp"],
    linkopts = ["-blah"],
)

cc_binary(
    name = "main",
    srcs = ["main.cpp"],
    deps = [
        ":baz",
        ":foo",
    ],
)

cc_shared_library(
    name = "bar",
    deps = [
        ":baz",
        ":foo",
    ],
)

The cc_binary target fails to build, but the cc_shared_library doesn't, it ignores the invalid -blah linkopt. If you add an empty cpp file to baz it works correctly.

Which category does this issue belong to?

No response

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

No response

Which operating system are you running Bazel on?

macOS

What is the output of bazel info release?

7.1.1

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 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

@comius
Copy link
Contributor

comius commented Apr 10, 2024

cc @pzembrod for triage
cc @oquenchil for potential insights

@oquenchil
Copy link
Contributor

Not sure what's going on, I'd set a breakpoint (or print) here: https://cs.opensource.google/bazel/bazel/+/master:src/main/starlark/builtins_bzl/common/cc/cc_shared_library.bzl;drc=2abc982ca5a8d1e829613629c8e3b44cf61b58c5;l=295

to see whether the wrapped linker_input has the flag -blah. A different question which I won't go into is whether it makes sense to do this at all. Headers do not actually contribute anything here since cc_shared_library only does linking, no compilation and therefore the flag could be added to the cc_shared_library. The rule doesn't provide CcInfo either so that flag won't be propagated downstream.

In any case, if you want this to throw an error I'd look first around that code I linked.

@comius comius added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels May 3, 2024
@keith
Copy link
Member Author

keith commented Aug 5, 2024

#23053

@iancha1992
Copy link
Member

@bazel-io fork 7.4.0

@keith
Copy link
Member Author

keith commented Oct 9, 2024

fix PR is here #23215

bazel-io pushed a commit to bazel-io/bazel that referenced this issue Oct 16, 2024
Fixes bazelbuild#21884
Fixes bazelbuild#23053

Closes bazelbuild#23215.

PiperOrigin-RevId: 686518093
Change-Id: I1a6699b169fe4291ad0dda3be7e74f38c8e37bf1
@iancha1992
Copy link
Member

@bazel-io fork 8.0.0

bazel-io pushed a commit to bazel-io/bazel that referenced this issue Oct 16, 2024
Fixes bazelbuild#21884
Fixes bazelbuild#23053

Closes bazelbuild#23215.

PiperOrigin-RevId: 686518093
Change-Id: I1a6699b169fe4291ad0dda3be7e74f38c8e37bf1
iancha1992 pushed a commit to iancha1992/bazel that referenced this issue Oct 16, 2024
Fixes bazelbuild#21884
Fixes bazelbuild#23053

Closes bazelbuild#23215.

PiperOrigin-RevId: 686518093
Change-Id: I1a6699b169fe4291ad0dda3be7e74f38c8e37bf1
github-merge-queue bot pushed a commit that referenced this issue Oct 17, 2024
…ibrary` (#24017)

Fixes #21884
Fixes #23053

Closes #23215.

PiperOrigin-RevId: 686518093
Change-Id: I1a6699b169fe4291ad0dda3be7e74f38c8e37bf1

Commit
0fdb36f

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
@iancha1992
Copy link
Member

A fix for this issue has been included in Bazel 7.4.0 RC3. Please test out the release candidate and report any issues as soon as possible.
If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=7.4.0rc3. Thanks!

github-merge-queue bot pushed a commit that referenced this issue Oct 21, 2024
…ibrary` (#24005)

Fixes #21884
Fixes #23053

Closes #23215.

PiperOrigin-RevId: 686518093
Change-Id: I1a6699b169fe4291ad0dda3be7e74f38c8e37bf1

Commit
0fdb36f

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants
@keith @comius @oquenchil @iancha1992 @satyanandak and others