Skip to content

Add supports_interface_shared_libraries to cc_shared_library#680

Open
keith wants to merge 1 commit into
bazelbuild:mainfrom
keith:ks/add-supports_interface_shared_libraries-to-cc_shared_library
Open

Add supports_interface_shared_libraries to cc_shared_library#680
keith wants to merge 1 commit into
bazelbuild:mainfrom
keith:ks/add-supports_interface_shared_libraries-to-cc_shared_library

Conversation

@keith
Copy link
Copy Markdown
Member

@keith keith commented Apr 3, 2026

This also backfills tests and fixes this in the default unix toolchain

@keith keith requested review from c-mita, pzembrod and trybka as code owners April 3, 2026 17:01
@keith
Copy link
Copy Markdown
Member Author

keith commented Apr 3, 2026

i could split the unix toolchain fix up but it was nice to verify with this new support

@keith
Copy link
Copy Markdown
Member Author

keith commented Apr 3, 2026

this is still off by default and requires users pass --features=supports_interface_shared_libraries

@keith keith force-pushed the ks/add-supports_interface_shared_libraries-to-cc_shared_library branch 3 times, most recently from 03edcaa to c7e4c50 Compare April 3, 2026 17:15
@keith
Copy link
Copy Markdown
Member Author

keith commented Apr 3, 2026

this working on macOS depends on #675

@keith
Copy link
Copy Markdown
Member Author

keith commented Apr 6, 2026

cc @fmeum in case there was a reason not to do this originally

@fmeum
Copy link
Copy Markdown
Collaborator

fmeum commented Apr 7, 2026

I don't see why it shouldn't work.

@keith keith force-pushed the ks/add-supports_interface_shared_libraries-to-cc_shared_library branch from c7e4c50 to db5bb61 Compare April 30, 2026 22:20
lilygorsheneva
lilygorsheneva previously approved these changes May 12, 2026
@lilygorsheneva
Copy link
Copy Markdown
Collaborator

Getting a failure in tests that are still in main Bazel repo.
https://github.com/bazelbuild/bazel/blob/708b25f5716af27cce53c797d149a73bf2dbdf66/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test/BUILD.builtin_test#L367

value of: collection
1 expected elements missing from values:
  0: -lprivate_lib_so

Investigating.

@lilygorsheneva
Copy link
Copy Markdown
Collaborator

lilygorsheneva commented May 12, 2026

Disabling interface_shared_objects in that test fixes it; that being said is the change in default behavior expected? Still waiting on additional testing to see if this breaks actual users.

@keith
Copy link
Copy Markdown
Member Author

keith commented May 12, 2026

can you tell from the log if there's a nodeps version of that lib in there? i think disabling that feature is fine for the tests, i guess that test is using the default --dynamic_mode value so it really is changing what is linked

@lilygorsheneva
Copy link
Copy Markdown
Collaborator

A .../libprivate_lib_so.ifso appears in the list

@keith
Copy link
Copy Markdown
Member Author

keith commented May 12, 2026

yea i think that behavior is an expected change here. but if that test also works on macOS we probably still need to disable this feature for it. i think we should just disable that feature since it's unrelated to simplify

@pzembrod
Copy link
Copy Markdown
Collaborator

Hi Keith, could you sync this PR to head again? It looks like I'll need to manually import it to double-check internal test failures, and the import fails with "Cannot find a merge reference for Pull Request 680. It might have a conflict with head."

This also backfills tests and fixes this in the default unix toolchain
@keith keith force-pushed the ks/add-supports_interface_shared_libraries-to-cc_shared_library branch from a255001 to 8ac79d4 Compare May 13, 2026 18:03
@keith
Copy link
Copy Markdown
Member Author

keith commented May 13, 2026

rebased and green! conflicts were just adding new features in the tests in 2 prs at once

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants