Skip to content

Commit 3510642

Browse files
committed
fix: resolve cross-package C++ header staging issue (Issue #38)
CRITICAL FIX: Properly stage local CcInfo headers for cross-package dependencies Root Cause Analysis: - Code intentionally excluded CcInfo headers from staging, assuming they were external libraries - This broke local cross-package dependencies using relative includes like #include "foundation/types.h" - Headers within same component also needed directory structure preservation Solution: 1. Modified dependency collection logic in cpp/defs.bzl: - External CcInfo headers (path contains "external/") → Use include paths only (no staging) - Local CcInfo headers (same workspace) → Stage with directory structure for relative includes 2. Enhanced header staging in file_ops_actions.bzl: - Preserve directory structure for all headers to support relative includes - Handle cases where source files use #include "foundation/types.h" patterns Testing: - ✅ foundation_lib builds successfully with preserved directory structure - ✅ consumer_component successfully includes cross-package headers - ✅ External library dependencies continue to work via include paths Impact: - Resolves Issue #38 cross-package header staging problems - Maintains backward compatibility for external library dependencies - Enables proper modular C++ component development
1 parent 5ce385c commit 3510642

File tree

2 files changed

+42
-23
lines changed

2 files changed

+42
-23
lines changed

cpp/defs.bzl

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -88,15 +88,26 @@ def _cpp_component_impl(ctx):
8888
elif file.extension == "a":
8989
dep_libraries.append(file)
9090

91-
# Extract includes from CcInfo for proper transitive dependencies
92-
# but DON'T copy external headers to workspace (they use original paths)
91+
# Extract includes and headers from CcInfo for proper transitive dependencies
92+
# CRITICAL FIX for Issue #38: Distinguish between external vs local CcInfo headers
9393
if CcInfo in dep:
9494
cc_info = dep[CcInfo]
9595

9696
# Add all types of include paths (direct, system, and quote includes)
9797
dep_includes.extend(cc_info.compilation_context.includes.to_list())
9898
dep_includes.extend(cc_info.compilation_context.system_includes.to_list())
9999
dep_includes.extend(cc_info.compilation_context.quote_includes.to_list())
100+
101+
# CRITICAL FIX: Stage local CcInfo headers for cross-package dependencies
102+
# External libraries (path contains "external/") don't need staging - use original paths
103+
# Local libraries (same workspace) need staging for relative includes to work
104+
for hdr in cc_info.compilation_context.headers.to_list():
105+
if hdr.extension in ["h", "hpp", "hh", "hxx"]:
106+
# Check if this is an external dependency or local cross-package dependency
107+
if "external/" not in hdr.path:
108+
# Local cross-package header - stage it for relative includes to work
109+
dep_headers.append(hdr)
110+
# External headers are handled via include paths only (no staging needed)
100111

101112
# Generate bindings directory
102113
bindings_dir = ctx.actions.declare_directory(ctx.attr.name + "_bindings")
@@ -482,7 +493,7 @@ def _cc_component_library_impl(ctx):
482493
# Output library
483494
library = ctx.actions.declare_file("lib{}.a".format(ctx.attr.name))
484495

485-
# Collect dependency headers
496+
# Collect dependency headers with proper cross-package staging
486497
dep_headers = []
487498
for dep in ctx.attr.deps:
488499
# Check DefaultInfo first for direct file access (cc_component_library outputs)
@@ -491,12 +502,18 @@ def _cc_component_library_impl(ctx):
491502
if file.extension in ["h", "hpp", "hh", "hxx"]:
492503
dep_headers.append(file)
493504

494-
# For CcInfo dependencies (external libraries), don't copy headers to workspace
495-
# since they already provide proper include paths. Only copy headers from
496-
# DefaultInfo (our own cc_component_library targets that need cross-package staging)
497-
# if CcInfo in dep:
498-
# cc_info = dep[CcInfo]
499-
# dep_headers.extend(cc_info.compilation_context.headers.to_list())
505+
# CRITICAL FIX for Issue #38: Stage local CcInfo headers for cross-package dependencies
506+
# External libraries (path contains "external/") don't need staging - use original paths
507+
# Local libraries (same workspace) need staging for relative includes to work
508+
if CcInfo in dep:
509+
cc_info = dep[CcInfo]
510+
for hdr in cc_info.compilation_context.headers.to_list():
511+
if hdr.extension in ["h", "hpp", "hh", "hxx"]:
512+
# Check if this is an external dependency or local cross-package dependency
513+
if "external/" not in hdr.path:
514+
# Local cross-package header - stage it for relative includes to work
515+
dep_headers.append(hdr)
516+
# External headers are handled via include paths only (no staging needed)
500517

501518
# Set up workspace with proper header staging (CRITICAL FIX for issue #38)
502519
work_dir = setup_cpp_workspace_action(

tools/bazel_helpers/file_ops_actions.bzl

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -281,24 +281,26 @@ def setup_cpp_workspace_action(ctx, sources, headers, bindings_dir = None, dep_h
281281
"dependencies": [],
282282
}
283283

284-
# For local headers within the same component, use basename only for local headers
285-
# This matches how the source files include them (e.g., #include "simd_utils.h")
286-
# But preserve directory structure for cross-package headers
284+
# CRITICAL FIX for Issue #38: Preserve directory structure for all headers
285+
# Headers need their directory structure preserved if source files include them with paths
287286
for hdr in headers:
288-
relative_path = hdr.basename # Default to basename for local headers
289-
290-
# Check if this header is in a different package/directory structure
291-
# that requires preserving path (cross-package dependencies)
292-
if "/test/" in hdr.short_path or "/external/" in hdr.path:
293-
# For cross-package headers, preserve some directory structure
294-
path_parts = hdr.short_path.split("/")
295-
if len(path_parts) >= 2:
296-
# Take the last 2 parts to preserve subdirectory structure
297-
relative_path = "/".join(path_parts[-2:])
287+
# Always preserve directory structure by using short_path
288+
# This handles cases where source files use #include "foundation/types.h"
289+
relative_path = hdr.short_path
290+
291+
# For headers within the current target's directory, preserve the relative structure
292+
# This ensures "foundation/types.h" includes work correctly
293+
path_parts = relative_path.split("/")
294+
if len(path_parts) >= 2:
295+
# Preserve the directory structure (e.g., "foundation/types.h")
296+
relative_path = "/".join(path_parts[-2:])
297+
else:
298+
# Single-level headers use basename
299+
relative_path = hdr.basename
298300

299301
config["headers"].append({
300302
"source": hdr,
301-
"destination": relative_path, # Use basename for local headers
303+
"destination": relative_path, # Preserve directory structure
302304
"preserve_permissions": False,
303305
})
304306

0 commit comments

Comments
 (0)