Skip to content

Conversation

@vonosmas
Copy link
Contributor

Extend Bazel rule implementation to enforce that all transitive dependencies of libc_header_library targets (used to implement hand-in-hand code sharing via headers) indeed only contain header files.

This fixes Bazel portion of PR #133126.

Extend Bazel rule implementation to enforce that all transitive
dependencies of libc_header_library targets (used to implement
hand-in-hand code sharing via headers) indeed only contain header files.

This fixes Bazel portion of PR llvm#133126.
@llvmbot llvmbot added libc bazel "Peripheral" support tier build system: utils/bazel labels Apr 17, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 17, 2025

@llvm/pr-subscribers-libc

Author: Alexey Samsonov (vonosmas)

Changes

Extend Bazel rule implementation to enforce that all transitive dependencies of libc_header_library targets (used to implement hand-in-hand code sharing via headers) indeed only contain header files.

This fixes Bazel portion of PR #133126.


Full diff: https://github.com/llvm/llvm-project/pull/136219.diff

1 Files Affected:

  • (modified) utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl (+15-13)
diff --git a/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl b/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl
index 60add23a46c48..7fe263ab08f71 100644
--- a/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl
+++ b/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl
@@ -123,12 +123,15 @@ _get_libc_info_aspect = aspect(
 )
 
 def _libc_srcs_filegroup_impl(ctx):
-    return DefaultInfo(
-        files = depset(transitive = [
-            fn[LibcLibraryInfo].srcs
-            for fn in ctx.attr.libs
-        ]),
-    )
+    srcs = depset(transitive = [
+        fn[LibcLibraryInfo].srcs
+        for fn in ctx.attr.libs
+    ])
+    if ctx.attr.enforce_headers_only:
+        paths = [f.short_path for f in srcs.to_list() if f.extension != "h"]
+        if paths:
+            fail("Unexpected non-header files: {}".format(paths))
+    return DefaultInfo(files = srcs)
 
 _libc_srcs_filegroup = rule(
     doc = "Returns all sources for building the specified libraries.",
@@ -138,6 +141,7 @@ _libc_srcs_filegroup = rule(
             mandatory = True,
             aspects = [_get_libc_info_aspect],
         ),
+        "enforce_headers_only": attr.bool(default = False),
     },
 )
 
@@ -218,6 +222,7 @@ def libc_header_library(name, hdrs, deps = [], **kwargs):
     _libc_srcs_filegroup(
         name = name + "_hdr_deps",
         libs = deps,
+        enforce_headers_only = True,
     )
 
     _libc_textual_hdrs_filegroup(
@@ -231,13 +236,10 @@ def libc_header_library(name, hdrs, deps = [], **kwargs):
 
     native.cc_library(
         name = name,
-        # Technically speaking, we should put _hdr_deps in srcs, as they are
-        # not a part of this cc_library interface. However, we keep it here to
-        # workaround the presence of .cpp files in _hdr_deps - we need to
-        # fix that and enforce their absence, since libc_header_library
-        # should be header-only and not produce any object files.
-        # See PR #133126 which tracks it.
-        hdrs = hdrs + [":" + name + "_hdr_deps"],
+        hdrs = hdrs,
+        # We put _hdr_deps in srcs, as they are not a part of this cc_library
+        # interface, but instead are used to implement shared headers.
+        srcs = [":" + name + "_hdr_deps"],
         deps = [":" + name + "_textual_hdr_library"],
         # copts don't really matter, since it's a header-only library, but we
         # need proper -I flags for header validation, which are specified in

Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vonosmas vonosmas merged commit bb67de6 into llvm:main Apr 17, 2025
12 checks passed
@vonosmas vonosmas deleted the libc-bazel-hdr-lib branch April 17, 2025 23:34
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…lvm#136219)

Extend Bazel rule implementation to enforce that all transitive
dependencies of libc_header_library targets (used to implement
hand-in-hand code sharing via headers) indeed only contain header files.

This fixes Bazel portion of PR llvm#133126.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bazel "Peripheral" support tier build system: utils/bazel libc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants