Skip to content

Don't build a shared library for embedded data - #4993

Open
marketchedai wants to merge 1 commit into
google:mainfrom
marketchedai:fix/embed-data-no-shared-lib
Open

marketchedai wants to merge 1 commit into
google:mainfrom
marketchedai:fix/embed-data-no-shared-lib

Conversation

@marketchedai

@marketchedai marketchedai commented Sep 16, 2026 •

Copy link
Copy Markdown

Fixes #4992.

xls_cc_embed_data produces both a static and a shared library. The shared
variant fails to link on macOS, because the toolchain passes the ELF-only
-soname flag to the Mach-O linker:

ld64.lld: error: unknown argument '-soname=libxls_Sbuild_Urules_Stests_Slibarbitrary_Udata_Ufile.dylib'

This blocks any test target that depends on embedded data, because cc_test
links its dependencies dynamically by default. On current main that includes
the rule's own test — //xls/build_rules/tests:embed_data_test cannot be built
on macOS at all.

Embedded data is only ever linked statically into its consumers, so the shared
library is an unused artifact: nothing in the tree depends on it. Passing
disallow_dynamic_library = True skips producing it, which avoids the broken
link on macOS and drops a dead artifact on every platform.

If you would prefer this fixed in the toolchain configuration so the shared
variant links correctly on Mach-O, I am happy to redo it that way; I took the
minimal route.

Testing

Tested on macOS (arm64, macOS 26.6.2) only, with Bazel 8.7.0, rebased onto
924b9fd. I have not built this on Linux — relying on CI for that.
The
change is platform-independent Starlark; its effect on Linux is that the .so
is no longer produced, and consumers link the static library as they already
did.

  • //xls/build_rules/tests/... passes 31/31 with this change, and
    embed_data_test fails to build without it on macOS.
  • //xls/common/... passes 37/37.
  • interpreter_main, ir_converter_main, opt_main and codegen_main build,
    and a DSLX -> IR -> opt -> Verilog run produces correct output.
  • //xls/common:subprocess_helper_embedded still builds under
    --dynamic_mode=fully.

Only two targets use this rule (//xls/common:subprocess_helper_embedded and
the test's arbitrary_data_file), and rdeps shows no consumer of the shared
variant.

🤖 Generation assisted with Claude Code

@google-cla

google-cla Bot commented Sep 16, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@marketchedai

Copy link
Copy Markdown
Author

Rebased onto 924b9fd07 and corrected the justification.

When I opened this, I cited //xls/common:subprocess_test as the blocked target. After #4972 merged, that is no longer accurate: macOS no longer links the embedded helper into subprocess_test, so that test now builds and passes on main without this change.

The rule itself is still broken on macOS, though — the failing target is now the rule's own test, //xls/build_rules/tests:embed_data_test, which fails to build on unpatched main with the -soname error above. I have updated the description and commit message accordingly.

@allight

allight commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

The Google cla must be agreed to before we can review or merge this pr.

@allight

allight commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

@marketchedai The Google cla must be agreed to before we can review or merge this pr.

@marketchedai
marketchedai force-pushed the fix/embed-data-no-shared-lib branch from 1884b34 to ad5a30e Compare September 22, 2026 20:01
xls_cc_embed_data produces both a static and a shared library. Linking the
shared variant fails on macOS because the toolchain passes the ELF-only
-soname flag to the Mach-O linker:

  ld64.lld: error: unknown argument
  '-soname=libxls_Sbuild_Urules_Stests_Slibarbitrary_Udata_Ufile.dylib'

This blocks any test target depending on embedded data, since tests link
their dependencies dynamically by default. On current main that includes the
rule's own test: //xls/build_rules/tests:embed_data_test cannot be built on
macOS at all.

Embedded data is only ever linked statically into its consumers, so the
shared library is an unused artifact -- no target in the tree depends on it.
Passing disallow_dynamic_library skips producing it, which avoids the broken
link on macOS and drops a dead artifact on every platform.

Verified on macOS arm64 against 924b9fd: //xls/build_rules/tests/... passes
31/31 with this change and embed_data_test fails to build without it;
//xls/common/... passes 37/37; the four main tools still build; and
//xls/common:subprocess_helper_embedded still builds under
--dynamic_mode=fully.

Co-authored with Claude Code (https://claude.com/claude-code).
@marketchedai
marketchedai force-pushed the fix/embed-data-no-shared-lib branch from ad5a30e to 341fd64 Compare September 22, 2026 20:03
@allight

allight commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Again you must agree to the CLA for PRs to be considered: https://github.com/google/xls/pull/4993/checks?check_run_id=107370647803

This branch has not been deployed

No deployments
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.

xls_cc_embed_data: shared-library link fails on macOS — ELF-only -soname passed to the Mach-O linker

2 participants